-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
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
go: do not wrap with go get
tools
#48284
Conversation
@GrahamcOfBorg build grafana prometheus_2 |
Success on aarch64-linux (full log) Attempted: go Partial log (click to expand)
|
Failure on aarch64-linux (full log) Attempted: grafana, prometheus_2 Partial log (click to expand)
|
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.
LGTM. People using go
imperatively would have to make sure the required SCM tools are installed and in PATH. It's mostly git anyway, which is usually installed. I think that's fine and the reduction in closure size is worth a minor inconvenience.
Success on x86_64-linux (full log) Attempted: go Partial log (click to expand)
|
Success on x86_64-darwin (full log) Attempted: go Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: grafana, prometheus_2 Partial log (click to expand)
|
Success on x86_64-darwin (full log) Attempted: prometheus_2 The following builds were skipped because they don't evaluate on x86_64-darwin: grafana Partial log (click to expand)
|
I agree, also not those systems are used frequently. Most packages can be probably build with git + bazar. |
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.
Seeing the difference in the dependecy tree and the resulting decreased closure size, I definitely agree with @xeji and think it's worth this minor inconvenience 👍
# before
$ nix path-info -r /nix/store/6p2nyb3kvamrjd3xkc8dym3f9l9pqfcw-go-1.10.3 | wc -l
95
# after
$ nix path-info -r /nix/store/yijwdk569c4fj361vg8f9788i1n8mv7m-go-1.10.3 | wc -l
12
Motivation for this change
Related: #46603
These tools are not required for
buildGoPackage
, while significantly affecting closure size and build time. This essentially reverts 458895d.cc @fpletz @Mic92 @WilliButz @xeji
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)