-
Notifications
You must be signed in to change notification settings - Fork 350
Fix toolchain paths on certain systems. #871
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
base: main
Are you sure you want to change the base?
Conversation
Certain systems, e.g. Guix, have e.g. $ARCH-unknown-linux-gnu-gcc rather than $ARCH-linux-gnu-gcc. Thus, use the former as a fallback if the latter isn't found.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me - just a nit.
Co-authored-by: Florian Lehner <[email protected]>
Co-authored-by: Florian Lehner <[email protected]>
| export CC = $(shell command -v $(ARCH_PREFIX)-linux-gnu-gcc || command -v $(ARCH_PREFIX)-unknown-linux-gnu-gcc) | ||
| export OBJCOPY = $(shell command -v $(ARCH_PREFIX)-linux-gnu-objcopy || command -v $(ARCH_PREFIX)-unknown-linux-gnu-objcopy) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don't use these when building the artifacts (as we dropped CGo), they're only used in tests where we explicitly enable CGo. In which case, shouldn't we just run the tests through the Docker image as well for reproducibility (and also avoiding adding environment-specific fallbacks)?
One issue with this implementation is that if no compiler is found, the errors read weird as CC is empty:
test.c -g -o with-debug-syms
make[1]: test.c: No such file or directory
make[1]: *** [Makefile:21: with-debug-syms] Error 127
make[1]: *** Waiting for unfinished jobs....
/bin/sh: 1: test.c: not found
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are also used when running make generate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are also used when running
make generate.
I missed this because make generate doesn't fail when the command -v invocations don't find a compiler (make test will however fail as shown in my previous comment):
$ (cd ./support; CC=$(command -v false) ./generate.sh)
$ (cd ./support; CC=$(command -v foo) ./generate.sh)
types_gen.go
strace is showing Go searching for and executing gcc when CC=. So essentially, there are multiple points of discussion here:
- We want
maketo fail when no compiler matching our criteria is found instead of introducing indeterminism. - Rather than adding environment-specific fallbacks (Guix, Nix, ..), we should probably strive for a cleaner
Makefileand strongly recommend people use our build container.
Certain systems, e.g. Guix, have e.g. $ARCH-unknown-linux-gnu-gcc rather than $ARCH-linux-gnu-gcc.
Thus, use the former as a fallback if the latter isn't found.