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

Autoconfigured unix crosstool should respect standard environment variables #5186

Closed
hlopko opened this issue May 11, 2018 · 9 comments
Closed
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Rules-CPP Issues for C++ rules type: feature request

Comments

@hlopko
Copy link
Member

hlopko commented May 11, 2018

Autoconfigured C++ toolchain doesn't follow standard gnu environment variables. There is no reason why we couldn't use the standard, therefore let's do it.

Bazel currently inspects some environment variables, but those don't have standard naming (even within bazel), are hard to discover (you have to go read the source code) and are completely undocumented.

Since gnu env vars are widely understood, using them would lower the entry barrier plus would make the life of package maintainers simpler, as they would not have to special case packages built by bazel.

This is very simple to implement, the cost is mostly the need to migrate existing users.

The most important environment variables are:

  • Programs:
    • CC: the C compiler (not C++), eg x86_64-pc-linux-gnu-gcc
    • CPP: The C pre-processor (also not C++) eg, x86_64-pc-linux-gnu-gcc -E
    • CXX: C++ compiler, eg x86_64-pc-linux-gnu-g++
    • FC: Fortran compiler
    • LD: the linker, eg x86_64-pc-linux-gnu-ld

Flags for the programs:

  • CFLAGS,CXXFLAGS: these are passed right after $CC for any optimizations and
    whatnot, make's implicit compile rule basically does:
    $(CC) $(CFLAGS) foo.c -o foo.o. You are more than welcome to
    append to any of these variables just overwriting is problematic.

  • LDFLAGS: These are the flags for the linker itself.

  • LDLIBS: these are the libraries that the binary needs to be linked
    against. Basically stuff with -l goes in LDLIBS and stuff with
    -L goes in LDFLAGS. make's rules for linking are roughly:
    $(LD) $(LDFLAGS) -o out foo.o bar.o $(LDLIBS). mixing up LDFLAGS
    and LDLIBS causes issues when --as-needed is used too.

  • SYSROOT: this is for cross-compiling and autoconf passes it as gcc
    --sysroot= i think. I don't cross-compile that often but can get
    you more info if it'd help.

@hlopko hlopko added type: feature request P2 We'll consider working on this in future. (Assignee optional) category: rules > C++ labels May 11, 2018
@hlopko hlopko self-assigned this May 11, 2018
@hlopko
Copy link
Member Author

hlopko commented May 11, 2018

This is currently blocked by:

@rongjiecomputer
Copy link
Contributor

I think there is no need to restrict this to Unix only. CMake on Windows will honour CC, CXX, LD and other environment variables so that user don't need to pass -DCMAKE_C_COMPILER, -DCMAKE_LINKER. flags in configure step. GCC/Mingw64 and GCC/MSYS users should generally be familiar with these environment variables. It is a nice-to-have feature for MSVC users too since these environment variables are harmless.

I am willing to implement the Windows part later. I have some other improvements to upstream after C++ Crosstool migration is done.

@hlopko
Copy link
Member Author

hlopko commented Jun 15, 2018

Good points. We have to bring our windows and darwin-with-Xcode toolchains to the same level of configurability as unix toolchain is.

@jasal82
Copy link

jasal82 commented Sep 17, 2018

Can you tell me when this is going to be implemented? I'm currently evaluating Bazel as replacement for our existing CMake setup but the configuration of cross toolchains is way too complicated. We could generate the necessary files from our Yocto environment but as there is already a environment setup script written by Yocto containing the CC/CXX/etc. paths and flags it would be good if Bazel could work with that instead.

Please note that Bazel must be able to accept CC/CXX paths with additional compiler flags appended because this is how Yocto inserts the sysroot and some architecture flags. Here is an example:

CC="powerpc64-fsl-linux-gcc -mhard-float -m64 -mcpu=e6500 --sysroot=$SDKTARGETSYSROOT"

@perfinion
Copy link

@jasal82 I'm not sure for the timing, its got a few deps that arn't done yet i think. In the mean time, I do this, feel free to copy. Its kind of ugly but it works decently well. https://github.com/gentoo/gentoo/blob/0d7adf51d80763d2ca8c3e5ffac4dd9b3edbe388/sci-libs/tensorflow/tensorflow-1.11.0_rc0.ebuild#L125-L141

@jasal82
Copy link

jasal82 commented Sep 18, 2018

@perfinion Your sample code is setting the compiler flags but I don't see where the cross compiler binaries are configured. Maybe I'm missing something.

@hlopko hlopko added team-Rules-CPP Issues for C++ rules and removed team-Rules-CPP Issues for C++ rules category: rules > C++ labels Oct 11, 2018
@detailyang
Copy link

+1 please

@hlopko hlopko removed their assignment Nov 28, 2018
@dslomov dslomov removed the bazel 1.0 label Jul 24, 2019
@c-mita c-mita added P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) and removed P2 We'll consider working on this in future. (Assignee optional) labels Nov 23, 2020
@github-actions
Copy link

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 2+ years. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Apr 12, 2023
@github-actions
Copy link

This issue has been automatically closed due to inactivity. If you're still interested in pursuing this, please reach out to the triage team (@bazelbuild/triage). Thanks!

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Rules-CPP Issues for C++ rules type: feature request
Projects
None yet
Development

No branches or pull requests

7 participants