buildGoModule: fix GOFLAGS for structuredAttrs#470709
Conversation
|
Almost no rebuilds so this could go to master, is this targeting staging for a reason? |
|
Mostly because it's a followup to #470403 which did have to go onto staging. But I can retarget this one if needed. |
|
People on matrix noticed that this managed to break at least 3 previously cached Logs for `go-away.goModules`Revert seems to fix it: diffdiff --git a/pkgs/build-support/go/module.nix b/pkgs/build-support/go/module.nix
index ea266422c6..30542b1d16 100644
--- a/pkgs/build-support/go/module.nix
+++ b/pkgs/build-support/go/module.nix
@@ -223,20 +223,19 @@
GOTOOLCHAIN = "local";
CGO_ENABLED = args.env.CGO_ENABLED or go.CGO_ENABLED;
-
- GOFLAGS = toString (
- GOFLAGS
- ++
- lib.warnIf (lib.any (lib.hasPrefix "-mod=") GOFLAGS)
- "use `proxyVendor` to control Go module/vendor behavior instead of setting `-mod=` in GOFLAGS"
- (lib.optional (!finalAttrs.proxyVendor) "-mod=vendor")
- ++
- lib.warnIf (builtins.elem "-trimpath" GOFLAGS)
- "`-trimpath` is added by default to GOFLAGS by buildGoModule when allowGoReference isn't set to true"
- (lib.optional (!finalAttrs.allowGoReference) "-trimpath")
- );
};
+ GOFLAGS =
+ GOFLAGS
+ ++
+ lib.warnIf (lib.any (lib.hasPrefix "-mod=") GOFLAGS)
+ "use `proxyVendor` to control Go module/vendor behavior instead of setting `-mod=` in GOFLAGS"
+ (lib.optional (!finalAttrs.proxyVendor) "-mod=vendor")
+ ++
+ lib.warnIf (builtins.elem "-trimpath" GOFLAGS)
+ "`-trimpath` is added by default to GOFLAGS by buildGoModule when allowGoReference isn't set to true"
+ (lib.optional (!finalAttrs.allowGoReference) "-trimpath");
+
inherit enableParallelBuilding;
# If not set to an explicit value, set the buildid empty for reproducibility.Logs for `go-away.goModules` with revert |
Not running |
|
So, we revert for now? Or what would be wise here? |
|
Is there a reason we’re inheriting (Side note: why does this change break running |
|
Seems like moving # now on master:
nix eval --impure --json --expr 'with import ./. {}; go-away.GOFLAGS'
"-mod=vendor -trimpath"
nix eval --impure --json --expr 'with import ./. {}; go-away.goModules.GOFLAGS'
"-mod=vendor -trimpath"
# with reverted PR:
nix eval --impure --json --expr 'with import ./. {}; go-away.GOFLAGS'
["-mod=vendor","-trimpath"]
nix eval --impure --json --expr 'with import ./. {}; go-away.goModules.GOFLAGS'
error:
...
error: attribute 'GOFLAGS' missing |
|
After updating from packages = rec {
tests = pkgs.buildGoModule {
pname = "tests";
version = "0.0.1";
src = goSrc;
vendorHash = null;
buildTestBinaries = true;
GOFLAGS = [ "-race" ];
};
};Any idea how we can fix this? |
You should probably change it to checkFlags checkFlags = [ "-race" ]; |
|
Are you sure this would help? https://nixos.org/manual/nixpkgs/stable/#var-stdenv-checkFlags says the following:
We don't use |
Yep, it's used by buildGoModule to pass those to the |
|
let
nixpkgs = import ./. {
overlays = [
(final: prev: {
jsonschema = prev.jsonschema.overrideAttrs (_: oldAttrs: {
CGO_ENABLED = 0;
ldflags = oldAttrs.ldflags ++ [ "-extldflags=-static" ];
env.GOWORK = "off";
});
})
];
};
in
nixpkgs.jsonschemaOn or after 83549e3, the build fails with: |
jsonschema = prev.jsonschema.overrideAttrs (_: oldAttrs: {
ldflags = oldAttrs.ldflags ++ [ "-extldflags=-static" ];
env = oldAttrs.env // { GOWORK = "off"; CGO_ENABLED = 0; };
}); |
|
Brilliant, thank you. I leave it to the maintainers to decide whether the old way should remain supported or not. |
|
Sorry for getting back to this so late. When I made this PR I just moved After having encountered the pattern multiple times now, I think the proper solution may be to change the merge order: #492019 if anyone would care to take a look / check if it helps their problem? |
|
I hit this exact issue on so many statically linked out of tree packages for various projects: #470709 (comment) This commit didn't only break overridden packages but also anyone in the community using buildgomodule to define their own packages in flakes, e.g. https://github.com/squat/generic-device-plugin :( |
|
This commit works around issues introduced by NixOS/nixpkgs#470709 that broke the way that environment variables attributes are extended in `buildGoModule` and thus prevented bumping the nixpkgs flake input beyond December. Signed-off-by: squat <lserven@gmail.com>
This commit works around issues introduced by NixOS/nixpkgs#470709 that broke the way that environment variables attributes are extended in `buildGoModule` and thus prevented bumping the nixpkgs flake input beyond December. Signed-off-by: squat <lserven@gmail.com>
This commit works around issues introduced by NixOS/nixpkgs#470709 that broke the way that environment variables attributes are extended in `buildGoModule` and thus prevented bumping the nixpkgs flake input beyond December. Signed-off-by: squat <lserven@gmail.com>
Related: #470403
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.