-
-
Notifications
You must be signed in to change notification settings - Fork 14.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
buildGoModule: pass GOFLAGS
with env
and construct from goFlags
#359744
base: staging
Are you sure you want to change the base?
Conversation
ff75e86
to
eefacb3
Compare
1cced9b
to
088f5dd
Compare
088f5dd
to
182f790
Compare
(lib.optional (!allowGoReference) "-trimpath"); | ||
env = args.env or { } // { | ||
|
||
GOFLAGS = lib.concatStringsSep " " finalAttrs.goFlags; |
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'm not sure if we should do this from nix.
Can't we perhaps have goFlags
be used as a bash array and set GOFLAGS
later in bash?
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 am thinking about it. Operating on goFlags
and forming GOFLAGS as needed makes the interface more consistent.
Still, as GOFLAGS
acts as the "default flags for all Go commands if applicable," people are likely to expect GOFLAGS
to be defined before the first execution of the go
command. Defining and exporting GOFLAGS
in one of the prePhases
seems to defeat the purpose of making goFlags
modifiable after evaluation.
By the way, is the rationale behind having |
It's because a list is more structured than a string, and I hope to keep having a way to specify GOFLAGS with a list. As you said, the main benefit would be a smoother |
Pass
GOFLAGS
asenv.GOFLAGS
to support__structuredAttrs = true
.Introduce a new argument,
goFlags
, to formGOFLAGS
at evaluation time. (Environment variables passed viaenv
cannot be a list.)This PR depends on #359641. Once the dependent PR is merged, I'll rebase this feature branch and drop the
[placeholder]
commit.Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.