-
-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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: warn about buildFlags only when using buildPhase provided by buildGoModule #298847
Conversation
c45a9e8
to
2c17e25
Compare
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 for the fix! Looks good to me.
Warn about buildFlags and ldflags only when using buildPhase provided by buildGoModule. This allows developers to use buildFlags in custom buildPhase.
…ation Avoid "global" warnings that complicates fixed-point arguments support.
2c17e25
to
8d861e6
Compare
Bump onto Add a commit that moves @katexochen Could you help me take a look again? |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review/3032/3842 |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-already-reviewed/2617/1608 |
This would have merge conflicts with #279391 on staging (fixed) |
Sorry @wegank , missed that. Let me know if you need an additional pair of eyes on it. |
Description of changes
The warning that suggests using
ldflags
instdead ofbuildFlags
orbuildFlagsArray
is relevant only whenbuildPhase
provided bybuildGoModule
. It becomes annoying if developers specifies a Make-based build phase and try to usebuildFlags
there.This PR fixes this issue by attaching the warning to the
buildPhase
implementation bybuildGoModule
. If applied, the warning won't be triggered ifbuildPhase
is specified otherwise, during packaging or overriding.This PR also moves
GOFLAGS
-related warnings where the attributeGOFLAGS
is specified. This clears away the remaining "global" warnings, simplifying the work to fix overriding and adopt fixed-point arguments.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.