-
-
Notifications
You must be signed in to change notification settings - Fork 15.3k
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
go: make compiler usable for cross compiles #50223
Conversation
@@ -133,12 +133,12 @@ stdenv.mkDerivation rec { | |||
|
|||
GOOS = if stdenv.isDarwin then "darwin" else "linux"; | |||
GOARCH = if stdenv.isDarwin then "amd64" | |||
else if stdenv.targetPlatform.isi686 then "386" |
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.
This should be correct. GOARCH is the architecture that the go compiler will produce code /for/. targetPlatform is used correctly here. The stdenv.isDarwin stuff looks wrong though. We should be able to use targetPlatform.parsed.kernel.name
for GOOS and a simple map for GOARCH:
GOOS = targetPlatform.parsed.kernel.name;
GOARCH = {
"i686" = "386";
"x86_64" = "amd64";
"aarch64" = "arm64";
"arm" = "arm";
"armv5tel" = "arm";
"armv6l" = "arm";
"armv7l" = "arm";
}.${stdenv.targetPlatform.parsed.cpu.name} or (throw "Unsupported system");
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.
The go compiler, after it's built, is able to produce code for any platform by changing those environment variables. So you'd set those appropriately for the host platform when doing a cross compile of a go package like terraform. But when building the go compiler itself, setting it to anything other than the build platform results in "invalid instruction" build errors.
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.
Makes sense, but that restructuring that Matt wrote (the "switch-case") is also a good idea.
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.
Ok sounds good!
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.
Updated to use the switch-case version
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.
Edit: oops, meant to reply in thread.
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.
Thanks!!
Motivation for this change
Allow go compiler to be used in cross compiles
This makes the go compiler usable for cross compiles. Previously, specifying go as
nativeBuildsInputs
ordepsBuildBuild
dependencies resulted in a go compiler built for the wrong platform. Now it pulls in the appropriate build platform version.I don't think
buildGoPackage
supports cross compiles, so existing go packages liketerraform
will still not cross compile. But one could now manually set up a go build that cross compiles without usingbuildGoPackage
.And go itself currently doesn't seem to cross compile; this PR doesn't change that.
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)