go: Adding a derivation for the 1.8 Go compiler #23122

ixmatus · 2017-02-23T21:11:23Z

Motivation for this change

Go 1.8 was recently released.

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
- Linux
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.

This should resolve issue #20082.

I haven't fully tested all packages that may depend on this (Real Life time constraints). If there are people using Go that are interested in this, I'd appreciate the help testing that this doesn't produce major breakage.

Maintainers: not sure if it's "correct" for me to go = go_1_8 instead of keeping it at go_1_7? Maybe if I didn't retarget that attribute it would be more "okay" to not test as many packages as may depend on Go?

copumpkin · 2017-02-24T02:45:25Z

pkgs/development/compilers/go/1.8.nix

+  CGO_ENABLED = 1;
+  GOROOT_BOOTSTRAP = "${goBootstrap}/share/go";
+
+  # The go build actually checks for CC=*/clang and does something different, so we don't


❤️ seeing comments like this

copumpkin · 2017-02-24T02:46:42Z

I get one test failure on Darwin, but otherwise this looks good:

##### Testing packages.
ok  	archive/tar	0.043s
ok  	archive/zip	2.061s
ok  	bufio	0.108s
ok  	bytes	0.294s
ok  	compress/bzip2	0.058s
ok  	compress/flate	0.812s
ok  	compress/gzip	0.027s
ok  	compress/lzw	0.014s
ok  	compress/zlib	0.026s
ok  	container/heap	0.013s
ok  	container/list	0.012s
ok  	container/ring	0.026s
ok  	context	1.030s
ok  	crypto/aes	0.045s
ok  	crypto/cipher	0.035s
ok  	crypto/des	0.026s
ok  	crypto/dsa	0.016s
ok  	crypto/ecdsa	0.095s
ok  	crypto/elliptic	0.053s
ok  	crypto/hmac	0.037s
ok  	crypto/md5	0.013s
ok  	crypto/rand	0.032s
ok  	crypto/rc4	0.130s
ok  	crypto/rsa	0.121s
ok  	crypto/sha1	0.039s
ok  	crypto/sha256	0.014s
ok  	crypto/sha512	0.016s
ok  	crypto/subtle	0.016s
ok  	crypto/tls	0.525s
ok  	crypto/x509	1.210s
ok  	database/sql	0.454s
ok  	database/sql/driver	0.012s
ok  	debug/dwarf	0.027s
ok  	debug/elf	0.037s
ok  	debug/gosym	0.552s
ok  	debug/macho	0.015s
ok  	debug/pe	0.021s
ok  	debug/plan9obj	0.013s
ok  	encoding/ascii85	0.013s
ok  	encoding/asn1	0.014s
ok  	encoding/base32	0.014s
ok  	encoding/base64	0.015s
ok  	encoding/binary	0.015s
ok  	encoding/csv	0.010s
ok  	encoding/gob	0.055s
ok  	encoding/hex	0.011s
ok  	encoding/json	0.495s
ok  	encoding/pem	0.031s
ok  	encoding/xml	0.021s
ok  	errors	0.012s
ok  	expvar	0.023s
ok  	flag	0.014s
ok  	fmt	0.128s
ok  	go/ast	0.017s
ok  	go/build	0.289s
ok  	go/constant	0.014s
ok  	go/doc	0.065s
ok  	go/format	0.018s
ok  	go/internal/gccgoimporter	0.019s
ok  	go/internal/gcimporter	1.291s
ok  	go/parser	0.037s
ok  	go/printer	0.434s
ok  	go/scanner	0.016s
ok  	go/token	0.031s
ok  	go/types	0.605s
ok  	hash/adler32	0.018s
ok  	hash/crc32	0.017s
ok  	hash/crc64	0.014s
ok  	hash/fnv	0.013s
ok  	html	0.015s
ok  	html/template	0.057s
ok  	image	0.131s
ok  	image/color	0.081s
ok  	image/draw	0.080s
ok  	image/gif	0.102s
ok  	image/jpeg	0.180s
ok  	image/png	0.048s
ok  	index/suffixarray	0.019s
ok  	internal/pprof/profile	0.013s
ok  	internal/singleflight	0.023s
ok  	internal/trace	0.454s
ok  	io	0.030s
ok  	io/ioutil	0.019s
ok  	log	0.012s
ok  	log/syslog	1.262s
ok  	math	0.015s
ok  	math/big	1.921s
ok  	math/cmplx	0.010s
ok  	math/rand	0.074s
ok  	mime	0.017s
ok  	mime/multipart	0.270s
ok  	mime/quotedprintable	0.126s
ok  	net	4.260s
ok  	net/http	3.391s
ok  	net/http/cgi	0.692s
ok  	net/http/cookiejar	0.027s
ok  	net/http/fcgi	0.025s
ok  	net/http/httptest	0.027s
ok  	net/http/httptrace	0.024s
ok  	net/http/httputil	0.053s
ok  	net/http/internal	0.016s
ok  	net/internal/socktest	0.016s
ok  	net/mail	0.024s
ok  	net/rpc	0.036s
ok  	net/rpc/jsonrpc	0.026s
ok  	net/smtp	0.029s
ok  	net/textproto	0.018s
ok  	net/url	0.023s
ok  	os	0.457s
ok  	os/exec	0.630s
ok  	os/signal	1.637s
ok  	os/user	0.018s
ok  	path	0.015s
ok  	path/filepath	0.044s
ok  	reflect	0.116s
ok  	regexp	0.150s
ok  	regexp/syntax	0.846s
ok  	runtime	22.355s
ok  	runtime/debug	0.020s
ok  	runtime/internal/atomic	0.140s
ok  	runtime/internal/sys	0.012s
ok  	runtime/pprof	1.876s
ok  	runtime/pprof/internal/protopprof	0.015s
ok  	runtime/trace	2.241s
ok  	sort	0.077s
ok  	strconv	0.796s
ok  	strings	0.116s
ok  	sync	0.247s
ok  	sync/atomic	0.039s
ok  	syscall	0.126s
ok  	testing	2.267s
ok  	testing/quick	0.064s
ok  	text/scanner	0.013s
ok  	text/tabwriter	0.013s
ok  	text/template	0.513s
ok  	text/template/parse	0.019s
--- FAIL: TestParseInLocation (0.00s)
	format_test.go:264: ParseInLocation(Feb 01 2013 AST, Baghdad) = 2013-02-01 00:00:00 +0000 AST, want 2013-02-01 00:00:00 +0300 +03
FAIL
FAIL	time	2.395s
ok  	unicode	0.014s
ok  	unicode/utf16	0.013s
ok  	unicode/utf8	0.014s
ok  	vendor/golang_org/x/crypto/chacha20poly1305	0.415s
ok  	vendor/golang_org/x/crypto/chacha20poly1305/internal/chacha20	0.017s
ok  	vendor/golang_org/x/crypto/curve25519	0.029s
ok  	vendor/golang_org/x/crypto/poly1305	0.018s
ok  	vendor/golang_org/x/net/http2/hpack	0.023s
ok  	vendor/golang_org/x/net/idna	0.021s
ok  	vendor/golang_org/x/net/lex/httplex	0.017s
ok  	vendor/golang_org/x/net/route	0.036s
ok  	cmd/addr2line	1.145s
ok  	cmd/api	0.015s
ok  	cmd/asm/internal/asm	0.200s
ok  	cmd/asm/internal/lex	0.018s
ok  	cmd/compile	6.837s
ok  	cmd/compile/internal/gc	8.161s
ok  	cmd/compile/internal/ssa	0.116s
ok  	cmd/compile/internal/syntax	0.015s
ok  	cmd/compile/internal/test	0.011s [no tests to run]
ok  	cmd/cover	2.307s
ok  	cmd/doc	0.038s
ok  	cmd/fix	0.021s
ok  	cmd/go	57.802s
ok  	cmd/gofmt	0.068s
ok  	cmd/internal/goobj	0.011s
ok  	cmd/internal/obj	0.011s
ok  	cmd/internal/obj/arm64	0.012s
ok  	cmd/internal/obj/x86	0.012s
ok  	cmd/link	0.014s
ok  	cmd/nm	1.057s
ok  	cmd/objdump	1.428s
ok  	cmd/pack	2.919s
ok  	cmd/trace	0.025s
ok  	cmd/vendor/golang.org/x/arch/arm/armasm	0.016s
ok  	cmd/vendor/golang.org/x/arch/ppc64/ppc64asm	0.014s
ok  	cmd/vendor/golang.org/x/arch/x86/x86asm	0.184s
ok  	cmd/vet	4.194s
ok  	cmd/vet/internal/cfg	0.014s

LnL7 · 2017-02-24T07:23:49Z

My builds both passed.

I don't think switching the go attribute to 1.8 will have a big impact, but it should probably match with the version that's used by buildGoPackage to avoid confusion.

ixmatus · 2017-02-24T17:53:52Z

@LnL7 and @copumpkin since you both got different test results, how do you want to proceed?

@LnL7 updating buildGoPackage now.

ixmatus · 2017-02-24T18:04:37Z

Okay, updated buildGoPackage I tried a test build on my EC2 machine but it doesn't seem like any derivations wanted to be rebuilt? Not sure how to test this...

ixmatus · 2017-02-24T18:33:37Z

I tested that the compiler built by this derivation builds our own internal application and its vendor libraries and all of our tests pass.

LnL7 · 2017-02-24T18:39:40Z

That test failure looks like it's something timezone related, setting export TZ=UTC in preCheck might fix it.

copumpkin · 2017-02-24T20:42:51Z

Didn't fix it 😦

This looks related though: golang/go#8814

Supposedly fixed ages ago, but perhaps the way we inject TZ data is weird? I'm on US East Coast time, if that helps.

ixmatus · 2017-02-24T20:52:51Z

I'm not sure if it does help, I'd appreciate help in figuring it out if you feel it's important as I can't reproduce (I'm on a NixOS laptop not MacOS).

copumpkin · 2017-02-24T20:55:29Z

Sure, I'll poke at it a bit later or over the weekend. Given that this is a compiler, I assume you also tested that a few common go tools compile properly with it? I'd check vault, terraform, docker, etc.

Once (or if 😉) I get this working on my Mac, I'll do the same.

ixmatus · 2017-02-24T20:57:58Z

@copumpkin I have not tested building common tools, I was going to do so this weekend or tonight or see if anyone else could get to it before me.

I have tested that an internal web application with a large dependency graph does build successfully and all tests pass, so that's a start. But it isn't strong enough confidence.

Thank you for helping :)

copumpkin · 2017-02-24T21:01:10Z

Thank you for the contribution!

LnL7 · 2017-02-24T21:04:57Z

We could also just disable the test if everything works properly.
@copumpkin I tested a few basic things, I'll add those to my jobset.

LnL7 · 2017-02-24T21:46:49Z

Those also look good on both NixOS and darwin.

LnL7 · 2017-02-25T18:39:07Z

pkgs/development/compilers/go/1.8.nix

+  '';
+
+  patches =
+    [ ./remove-tools-1.8.patch


Are these patches all different from the 1.7 versions?

No not all, I think one of them is the same but I wanted to "copy it" just to make sure it was distinct because the others needed tweaks.

LnL7 · 2017-02-25T18:41:04Z

pkgs/development/compilers/go/1.8.nix

+  hardeningDisable = [ "all" ];
+
+  preCheck = ''
+    export TZ=UTC


I just realised that the go build doesn't have a checkPhase, adding it to preInstall might still fix the timezone issue in the tests.

Ah! I hadn't realized that, adding now.

CC: @copumpkin

LnL7 · 2017-02-27T12:07:20Z

Turns out the timezone is actually already set by the stdenv.

copumpkin · 2017-02-27T13:49:57Z

Anyway, not really sure what causes the failure. Perhaps let's just turn off that test for now until we figure it out? Then merge with a comment in there?

LnL7

Looks good, can you squash this.

ixmatus · 2017-03-02T17:59:13Z

Hmm, I think I confused myself trying to squash by rebasing. I will try again.

ixmatus · 2017-03-05T02:28:48Z

@LnL7 finally got around to cleaning up the history.

the-kenny · 2017-03-05T10:43:15Z

@ixmatus LGTM. Can you please just change the commit message to match contributing.md? You can do this via git commit --amend.

ixmatus · 2017-03-05T20:12:26Z

@the-kenny sure, I can. Are you wanting me to add more detail like removing the test for the time formatting failure? This line in the contributing.md had me thinking my one-liner was fine:

For package version upgrades and such a one-line commit message is usually sufficient.

the-kenny · 2017-03-05T22:57:36Z

@ixmatus No the one liner is fine :) I just think it's beneficial to follow the usual go: 1.7 -> 1.8 or scheme. However, it's a bit tricky here as we're introducing pkgs.go_1_8 and at the same time setting pkgs.go to it.
In hindsight the current commit message sounds fine to me after all.

LnL7 · 2017-03-05T23:00:20Z

I think go: 1.7 -> 1.8 makes more sense, since go and buildGoPackage changed.

grahamc · 2017-03-06T13:48:12Z

Applied in 8d6fbd0, thank you!

ixmatus · 2017-03-06T14:18:15Z

@grahamc, thanks! Also why did you close (and commit elsewhere) instead of merging this PR? Is there something I should have done differently?

grahamc · 2017-03-06T14:55:35Z

I have some tools to apply-and-test in one go which is easier, and also I reworded the commit message. You did nothing wrong, really :)

copumpkin · 2017-03-08T18:38:40Z

For what it's worth, I ran into the same timezone issue with the go 1.4 bootstrap on my machine, so I think it's an impurity everywhere 😄

This is from 1.4

	format_test.go:202: ParseInLocation(Feb 01 2013 AST, Baghdad) = 2013-02-01 00:00:00 +0000 AST, want 2013-02-01 00:00:00 +0300 +03

copumpkin · 2017-03-15T17:06:41Z

@ixmatus I figured it out, and I'll push a fix soon. See golang/go@91563ce for details 😄

luckily for us, the patch actually applies to older versions of Go as well, so the change to nixpkgs is pretty easy.

LnL7 added the 6.topic: golang label Feb 23, 2017

copumpkin reviewed Feb 24, 2017

View reviewed changes

LnL7 reviewed Feb 25, 2017

View reviewed changes

LnL7 approved these changes Mar 1, 2017

View reviewed changes

ixmatus force-pushed the parnell/add-go1.8 branch from 4b31a9d to 91abe63 Compare March 2, 2017 17:56

ixmatus force-pushed the parnell/add-go1.8 branch from 91abe63 to 51f3f77 Compare March 5, 2017 02:28

go: compiler 1.7 => 1.8

d753ba4

ixmatus force-pushed the parnell/add-go1.8 branch from 51f3f77 to d753ba4 Compare March 6, 2017 00:50

grahamc closed this Mar 6, 2017

ixmatus deleted the parnell/add-go1.8 branch March 6, 2017 15:20

Artturin mentioned this pull request May 25, 2022

go_1_18: fix pkgsCross.raspberryPi.pkgsBuildHost.go_1_18 #174612

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

go: Adding a derivation for the 1.8 Go compiler #23122

go: Adding a derivation for the 1.8 Go compiler #23122

ixmatus commented Feb 23, 2017 •

edited by LnL7

copumpkin Feb 24, 2017

copumpkin commented Feb 24, 2017

LnL7 commented Feb 24, 2017

ixmatus commented Feb 24, 2017

ixmatus commented Feb 24, 2017

ixmatus commented Feb 24, 2017

LnL7 commented Feb 24, 2017

copumpkin commented Feb 24, 2017 •

edited

ixmatus commented Feb 24, 2017

copumpkin commented Feb 24, 2017

ixmatus commented Feb 24, 2017

copumpkin commented Feb 24, 2017

LnL7 commented Feb 24, 2017 •

edited

LnL7 commented Feb 24, 2017

LnL7 Feb 25, 2017

ixmatus Feb 25, 2017

LnL7 Feb 25, 2017

ixmatus Feb 25, 2017

LnL7 commented Feb 27, 2017

copumpkin commented Feb 27, 2017

LnL7 left a comment

ixmatus commented Mar 2, 2017

ixmatus commented Mar 5, 2017

the-kenny commented Mar 5, 2017 •

edited

ixmatus commented Mar 5, 2017

the-kenny commented Mar 5, 2017

LnL7 commented Mar 5, 2017

grahamc commented Mar 6, 2017

ixmatus commented Mar 6, 2017

grahamc commented Mar 6, 2017 via email

copumpkin commented Mar 8, 2017

copumpkin commented Mar 15, 2017 •

edited

go: Adding a derivation for the 1.8 Go compiler #23122

go: Adding a derivation for the 1.8 Go compiler #23122

Conversation

ixmatus commented Feb 23, 2017 • edited by LnL7

Motivation for this change

Things done

copumpkin Feb 24, 2017

Choose a reason for hiding this comment

copumpkin commented Feb 24, 2017

LnL7 commented Feb 24, 2017

ixmatus commented Feb 24, 2017

ixmatus commented Feb 24, 2017

ixmatus commented Feb 24, 2017

LnL7 commented Feb 24, 2017

copumpkin commented Feb 24, 2017 • edited

ixmatus commented Feb 24, 2017

copumpkin commented Feb 24, 2017

ixmatus commented Feb 24, 2017

copumpkin commented Feb 24, 2017

LnL7 commented Feb 24, 2017 • edited

LnL7 commented Feb 24, 2017

LnL7 Feb 25, 2017

Choose a reason for hiding this comment

ixmatus Feb 25, 2017

Choose a reason for hiding this comment

LnL7 Feb 25, 2017

Choose a reason for hiding this comment

ixmatus Feb 25, 2017

Choose a reason for hiding this comment

LnL7 commented Feb 27, 2017

copumpkin commented Feb 27, 2017

LnL7 left a comment

Choose a reason for hiding this comment

ixmatus commented Mar 2, 2017

ixmatus commented Mar 5, 2017

the-kenny commented Mar 5, 2017 • edited

ixmatus commented Mar 5, 2017

the-kenny commented Mar 5, 2017

LnL7 commented Mar 5, 2017

grahamc commented Mar 6, 2017

ixmatus commented Mar 6, 2017

grahamc commented Mar 6, 2017 via email

copumpkin commented Mar 8, 2017

copumpkin commented Mar 15, 2017 • edited

ixmatus commented Feb 23, 2017 •

edited by LnL7

copumpkin commented Feb 24, 2017 •

edited

LnL7 commented Feb 24, 2017 •

edited

the-kenny commented Mar 5, 2017 •

edited

copumpkin commented Mar 15, 2017 •

edited