Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gecko: Use stdenv's clang for clang paths. #196

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nbp
Copy link
Collaborator

@nbp nbp commented Jul 23, 2019

This change fixes the issue reported in #191 , by making sure that we use the same clang as the one provided in the stdenv when building Gecko, and when using bindgen.

@nbp nbp requested a review from glasserc July 23, 2019 16:30
libclang_path =
if stdenv.cc.isGNU then "${llvmPackages.clang.cc.lib}/lib"
else "${stdenv.cc.cc.lib}/lib";

genMozConfig = ''
cxxLib=$( echo -n ${gcc}/include/c++/* )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also had to change this -- I got confusing errors which seemed to have to do with clang including GCC headers.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that if we do not do that, then I think we default to libstdc++ from LLVM, which never worked for compiling Firefox, as far as I experienced.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be a dumb question, but why not? It seems from https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Linux_Prerequisites#Other_distros_and_other_Unix-based_systems that both gcc and clang are supported.

@@ -129,7 +138,7 @@ let
export CC="${stdenv.cc}/bin/cc";
export CXX="${stdenv.cc}/bin/c++";
# To be used when building the JS Shell.
export NIX_EXTRA_CONFIGURE_ARGS="--with-libclang-path=${llvmPackages.clang.cc.lib}/lib --with-clang-path=${llvmPackages.clang}/bin/clang"
export NIX_EXTRA_CONFIGURE_ARGS="--with-libclang-path=${libclang_path}/lib --with-clang-path=${clang_path}"
cxxLib=$( echo -n ${gcc}/include/c++/* )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also changed it here. I'm not sure if both were really necessary.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is mostly needed for me, where I use this environment variable for only compiling SpiderMonkey using the ./configure $NIX_EXTRA_CONFIGURE_ARGS command line. (more complex in practice, but this is the idea)

genMozConfig = ''
cxxLib=$( echo -n ${gcc}/include/c++/* )
archLib=$cxxLib/$( ${gcc}/bin/gcc -dumpmachine )

cat - > $MOZCONFIG <<EOF
mk_add_options AUTOCONF=${autoconf213}/bin/autoconf
ac_add_options --with-libclang-path=${llvmPackages.clang.cc.lib}/lib
ac_add_options --with-clang-path=${llvmPackages.clang}/bin/clang
ac_add_options --with-libclang-path=${libclang_path}/lib
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This /lib should be removed, because it ends in /lib/lib else.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least when i try to build within nix-shell ../../nixpkgs-overlays/nixpkgs-mozilla/release.nix -A gecko.x86_64-linux.gcc, that is.

@@ -129,7 +138,7 @@ let
export CC="${stdenv.cc}/bin/cc";
export CXX="${stdenv.cc}/bin/c++";
# To be used when building the JS Shell.
export NIX_EXTRA_CONFIGURE_ARGS="--with-libclang-path=${llvmPackages.clang.cc.lib}/lib --with-clang-path=${llvmPackages.clang}/bin/clang"
export NIX_EXTRA_CONFIGURE_ARGS="--with-libclang-path=${libclang_path}/lib --with-clang-path=${clang_path}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one /lib is also extraneous. See above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants