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: Introduce ldflags argument #125520

Merged
merged 1 commit into from
Jun 4, 2021
Merged

Conversation

infinisil
Copy link
Member

Motivation for this change

Previously it was not possible to define multiple ldflags, since only
the last definition applies, and there's some quoting issues with
buildFlagsArray. With the new ldflags argument it's possible to do
this, e.g.

ldflags = drv.ldflags or [] ++ [
  "-X main.Version=1.0"
]

can now properly append a flag without clearing all previous ldflags.

Pretty much fixes #112054, ping @jtojnar @Mic92 @kalbasit @kvtb

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Added a release notes entry if the change is major or breaking
  • Fits CONTRIBUTING.md.

@r-rmcgibbo
Copy link

Result of nixpkgs-review pr 125520 at be257e24 run on aarch64-linux 1

3 packages marked as broken and skipped:
  • gb-backup
  • linuxPackages_testing_bcachefs.oci-seccomp-bpf-hook
  • secretscanner
696 packages skipped due to time constraints:
  • _3mux
  • act
  • age
  • air
  • alertmanager-bot
  • alertmanager-irc-relay
  • amass
  • amfora
  • antibody
  • archiver
  • ...
4 packages built successfully:
  • nixopsUnstable
  • poetry
  • rmfuse
  • yj

@kalbasit
Copy link
Member

kalbasit commented Jun 3, 2021

Can this be backported to 21.05 or is it too late?

@zowoq
Copy link
Contributor

zowoq commented Jun 4, 2021

Can this be backported to 21.05 or is it too late?

I think backporting this would be fine and may even be somewhat necessary so that go package backports aren't a hassle.

@fabaff fabaff mentioned this pull request Jun 4, 2021
8 tasks
Previously it was not possible to define multiple ldflags, since only
the last definition applies, and there's some quoting issues with
`buildFlagsArray`. With the new `ldflags` argument it's possible to do
this, e.g.

    ldflags = drv.ldflags or [] ++ [
      "-X main.Version=1.0"
    ]

can now properly append a flag without clearing all previous ldflags.
@zowoq
Copy link
Contributor

zowoq commented Jun 4, 2021

I won't be doing the backport myself but I'd suggest that it be backported with #125597 to limit the number of rebuilds.

@infinisil infinisil deleted the go-ldflags branch June 8, 2021 12:40
@fabaff fabaff mentioned this pull request Jun 9, 2021
11 tasks
@terinjokes
Copy link
Contributor

@zowoq Should it be backported with the next Go release then?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants