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.

Sign up for GitHub

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

Jump to bottom

go: Refactor to generic builder #35381

Closed
wants to merge 1 commit into from
Closed

go: Refactor to generic builder #35381

wants to merge 1 commit into from

Conversation

adisbladis
Copy link
Member

@adisbladis adisbladis commented Feb 23, 2018 •
edited

Motivation for this change

The go versions build process is more similar than different.
I'm not sure why this causes darwin rebuilds currently.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

cc @cstrahan @orivej @velovix @Mic92

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Partial log (click to expand)

these paths will be fetched (118.55 MiB download, 663.73 MiB unpacked):
  /nix/store/r7p21cmhq0jimq2f8c8y5sm2vqg2ap99-go-1.10
fetching path ‘/nix/store/r7p21cmhq0jimq2f8c8y5sm2vqg2ap99-go-1.10’...

*** Downloading ‘https://cache.nixos.org/nar/0vvv568kyrg93mn86lp8rilinmyg44l5m9bwfw3g6rmi63lfvgaw.nar.xz’ (signed by ‘cache.nixos.org-1’) to ‘/nix/store/r7p21cmhq0jimq2f8c8y5sm2vqg2ap99-go-1.10’...
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
  3  118M    3 4456k    0     0  6062k      0  0:00:20 --:--:--  0:00:20 6054k
 13  118M   13 16.4M    0     0  9747k      0  0:00:12  0:00:01  0:00:11 9741k
 23  118M   23 27.8M    0     0  10.2M      0  0:00:11  0:00:02  0:00:09 10.2M
 33  118M   33 39.2M    0     0  10.4M      0  0:00:11  0:00:03  0:00:08 10.4M
 42  118M   42 50.5M    0     0  10.6M      0  0:00:11  0:00:04  0:00:07 10.6M
 52  118M   52 61.9M    0     0  10.8M      0  0:00:10  0:00:05  0:00:05 11.5M
 61  118M   61 73.3M    0     0  10.9M      0  0:00:10  0:00:06  0:00:04 11.3M
 71  118M   71 84.6M    0     0  10.9M      0  0:00:10  0:00:07  0:00:03 11.3M
 81  118M   81 96.0M    0     0  10.9M      0  0:00:10  0:00:08  0:00:02 11.3M
 90  118M   90  107M    0     0  11.0M      0  0:00:10  0:00:09  0:00:01 11.3M
100  118M  100  118M    0     0  11.0M      0  0:00:10  0:00:10 --:--:-- 11.2M
100  118M  100  118M    0     0  11.0M      0  0:00:10  0:00:10 --:--:-- 11.2M

/nix/store/r7p21cmhq0jimq2f8c8y5sm2vqg2ap99-go-1.10

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Partial log (click to expand)

these paths will be fetched (95.49 MiB download, 554.56 MiB unpacked):
  /nix/store/sja9gpkj05paj4j9gvxzx5vrnghfdaja-go-1.10
copying path '/nix/store/sja9gpkj05paj4j9gvxzx5vrnghfdaja-go-1.10' from 'https://cache.nixos.org'...
/nix/store/sja9gpkj05paj4j9gvxzx5vrnghfdaja-go-1.10

@GrahamcOfBorg
Copy link

Success on x86_64-darwin (full log)

Partial log (click to expand)


ALL TESTS PASSED
---
Installed Go for darwin/amd64 in /nix/store/2h3y3gc6c9ir9nr7j33i2i11ascpcakp-go-1.10/share/go
Installed commands in /nix/store/2h3y3gc6c9ir9nr7j33i2i11ascpcakp-go-1.10/share/go/bin
post-installation fixup
strip is /nix/store/4sdh09gmvl15cy0zb6i7mbvxh5syz206-cctools-binutils-darwin/bin/strip
stripping (with command strip and flags -S) in /nix/store/2h3y3gc6c9ir9nr7j33i2i11ascpcakp-go-1.10/bin
patching script interpreter paths in /nix/store/2h3y3gc6c9ir9nr7j33i2i11ascpcakp-go-1.10
/nix/store/2h3y3gc6c9ir9nr7j33i2i11ascpcakp-go-1.10

@Mic92
Copy link
Member

Mic92 commented Feb 23, 2018

Mhm. I am not sure if those generic approaches are very useful when building different versions of compilers. It is not very common, that you change something in a compiler expression after it has been added to nixpkgs. Sometimes abstraction is cool if you can change something in a central place, but it can be also make things harder to read. Or like the Rob Pike say: A little copying is better then a little dependency quote
What is your experience from updating the llvm infrastructure in nixpkgs? @dtzWill

@Mic92
Copy link
Member

Mic92 commented Feb 23, 2018

Also the last release did not change a lot, that might change again in future releases.

@velovix
Copy link
Contributor

velovix commented Feb 23, 2018

I agree with your concerns @Mic92 and I'm a sucker for Go proverbs, but I feel like the good outweighs the bad here. It expresses to readers that Go 1.9 and 1.10 are built in largely the same way.

If Go 1.11 requires a significantly different build process, then I don't think that will be a problem. At that point, we'll have removed 1.9 anyway (hopefully) and so this code will end up back in the Go 1.10 expression instead of as a generic expression. If we end up having to maintain 1.9, 1.10, and 1.11 this might get unwieldy, but we can revisit this at that time.

@orivej
Copy link
Contributor

orivej commented Feb 24, 2018

Go 1.10 expression may diverge more from Go 1.9 expression when buildGoPackage is switched to Go 1.10 (without breaking any dependent packages). The copy of 1.8 expression had to be slightly adjusted in 1.8 -> 1.9 transition: 07d2a5b (although this change would not have hurt 1.8 if the expression was already generalized).

@adisbladis adisbladis closed this Jun 12, 2018
@adisbladis
Copy link
Member Author

I want to revisit this.

When #68101 is merged we'll have 4 versions of Go in nixpkgs that only differ slightly.
This makes reviewing version bumps harder than it has to be.

@rvolosatovs rvolosatovs mentioned this pull request Sep 5, 2019
Merged
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nlewo nlewo nlewo approved these changes

Assignees
No one assigned
Labels
6.topic: golang 10.rebuild-darwin: 101-500 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@adisbladis @GrahamcOfBorg @Mic92 @velovix @orivej @nlewo