Skip to content
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

Draft
wants to merge 3 commits into
base: staging
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions doc/languages-frameworks/go.section.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,14 @@ The root directory of the Go module that contains the `go.mod` file.

Defaults to `./`, which is the root of `src`.

### `goFlags` {#var-go-goFlags}

A string list of flags to form the environment variable `GOFLAGS` during evaluation, overridable via `overrideAttrs`.

::: {.note}
Build-time modification should be performed directly onto the `GOFLAGS` environment variable instead of `goFlags`.
:::

### `ldflags` {#var-go-ldflags}

A string list of flags to pass to the Go linker tool via the `-ldflags` argument of `go build`. Possible values can be retrieved by running `go tool link --help`.
Expand Down
8 changes: 8 additions & 0 deletions nixos/doc/manual/release-notes/rl-2505.section.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@
- `binwalk` was updated to 3.1.0, which has been rewritten in rust. The python module is no longer available.
See the release notes of [3.1.0](https://github.com/ReFirmLabs/binwalk/releases/tag/v3.1.0) for more information.

- `buildGoModule` now pases environment variables via the `env` attribute. `CGO_ENABLED` should now be specified with `env.CGO_ENABLED` when passing to buildGoModule. Direct specification of `CGO_ENABLED` is now redirected by a compatibility layer with a warning, but will become an error in future releases.

The flags to form `GOFLAGS` should now be passed to the `goFlags` argument. The build-time modification of `GOFLAGS` should still target `GOFLAGS` instead of `goFlags`.

Go-related environment variables previously shadowed by `buildGoModule` now results in errors when specified directly. Such variables include `GOOS` and `GOARCH`.

Third-party projects supporting both stable and unstable channels could detect this change through the absence of the `CGO_ENABLED` function argument in `buildGoModule` (`!((lib.functionArgs buildGoModule) ? CGO_ENABLED)`).

- `buildGoPackage` has been removed. Use `buildGoModule` instead. See the [Go section in the nixpkgs manual](https://nixos.org/manual/nixpkgs/unstable/#sec-language-go) for details.

- `timescaledb` requires manual upgrade steps.
Expand Down
28 changes: 22 additions & 6 deletions pkgs/build-support/go/module.nix
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
# Go linker flags.
, ldflags ? [ ]
# Go build flags.
, GOFLAGS ? [ ]
, goFlags ? [ ]

# Needed for buildFlags{,Array} warning
, buildFlags ? "" # deprecated
Expand Down Expand Up @@ -171,16 +171,32 @@ in

inherit (go) GOOS GOARCH;

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 (!allowGoReference) "-trimpath");
env = args.env or { } // {

GOFLAGS = lib.concatStringsSep " " finalAttrs.goFlags;
Copy link
Contributor

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?

Copy link
Contributor Author

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.

};

inherit CGO_ENABLED enableParallelBuilding GO111MODULE GOTOOLCHAIN;

# If not set to an explicit value, set the buildid empty for reproducibility.
ldflags = ldflags ++ lib.optional (!lib.any (lib.hasPrefix "-buildid=") ldflags) "-buildid=";

goFlags =
let
# Compatibility layers for direct specification of the GOFLAGS environment variable
goFlags =
lib.optionals (args?GOFLAGS) (
if lib.isString args.GOFLAGS then
lib.splitString args.GOFLAGS
else args.GOFLAGS
) ++ args.goFlags or [ ];
in
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 (!allowGoReference) "-trimpath");

configurePhase = args.configurePhase or (''
runHook preConfigure

Expand Down