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

build: Upgrade to Go 1.12 #35637

Closed
5 tasks done
knz opened this issue Mar 12, 2019 · 12 comments
Closed
5 tasks done

build: Upgrade to Go 1.12 #35637

knz opened this issue Mar 12, 2019 · 12 comments
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. meta-issue Contains a list of several other issues.

Comments

@knz
Copy link
Contributor

knz commented Mar 12, 2019 •
edited by benesch
Loading

Open standing issues:
@knz knz added the meta-issue Contains a list of several other issues. label Mar 12, 2019
This was referenced Mar 12, 2019
Merged
Merged
craig bot pushed a commit that referenced this issue Mar 12, 2019
@knz
Merge #35638
51179af 
35638: build: reject builds with go 1.12. r=knz a=knz

Informs #35637

We need this patch because people (eg. brew) are trying to build crdb with 1.12.

Release note (build change): CockroachDB will provisionally refuse to build with
go 1.12, as this is known to produce incorrect code inside CockroachDB.

Co-authored-by: Raphael 'kena' Poss <[email protected]>
@bdarnell bdarnell mentioned this issue Mar 12, 2019
Closed
@bdarnell
Copy link
Contributor

When built in the homebrew CI environment, the go 1.12 linker segfaults: Homebrew/homebrew-core#37788. We haven't seen that elsewhere so I don't know what, if anything, we can do about that.

This issue also makes the homebrew-core cockroach package unbuildable (since brew insists on using the latest version of everything). We'll need to either fix the go 1.12 issues or go back to our own tap for 19.1: #35657.

@knz knz added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Mar 12, 2019
@benesch
Copy link
Contributor

benesch commented Mar 25, 2019

Linker bug appears to be known upstream: golang/go#30908

@bdarnell
Copy link
Contributor

make lint also fails if you're using go 1.12:

    --- FAIL: TestLint/TestVet (0.04s)
        lint_test.go:1357: 
            vet: flag "-shadow" not defined
        lint_test.go:1357: 
            Run "go help vet" for more information

bdarnell added a commit to bdarnell/cockroach that referenced this issue Apr 1, 2019
@bdarnell
build: Allow go 1.12 again
053aa1b 
Fears of invalid builds have been resolved, so we no longer need to
prohibit this.

Note that it is difficult to be lint-clean with both 1.11 and 1.12
simultaneously, so we'll have to wait to fix the new lint issues until
we move to 1.12 as the minimum version (post CRDB 19.1)

Updates cockroachdb#35637

Release note: None
@bdarnell bdarnell mentioned this issue Apr 1, 2019
Merged
craig bot pushed a commit that referenced this issue Apr 1, 2019
@bdarnell
Merge #36389
4421c14 
36389: build: Allow go 1.12 again r=petermattis a=bdarnell

Fears of invalid builds have been resolved, so we no longer need to
prohibit this.

Note that it is difficult to be lint-clean with both 1.11 and 1.12
simultaneously, so we'll have to wait to fix the new lint issues until
we move to 1.12 as the minimum version (post CRDB 19.1)

Updates #35637

Release note: None

Co-authored-by: Ben Darnell <[email protected]>
bdarnell added a commit to bdarnell/cockroach that referenced this issue Apr 1, 2019
@bdarnell
build: Allow go 1.12 again
63eaf7b 
Fears of invalid builds have been resolved, so we no longer need to
prohibit this.

Note that it is difficult to be lint-clean with both 1.11 and 1.12
simultaneously, so we'll have to wait to fix the new lint issues until
we move to 1.12 as the minimum version (post CRDB 19.1)

Updates cockroachdb#35637

Release note: None
@bdarnell bdarnell mentioned this issue Apr 1, 2019
Merged
@bdarnell bdarnell changed the title build: go 1.12 produces invalid CockroachDB builds build: Upgrade to Go 1.12 Apr 2, 2019
@dt
Copy link
Member

dt commented Apr 3, 2019

What's the difference between this and #35197?

@knz
Copy link
Contributor Author

knz commented Apr 3, 2019

The other issue was a plan to do things with an attempt to identify problem areas ahead of time.

This issue here is the list of actual problems that fell out of having some folk trying go 1.12 "by accident".

I agree that given where we are we can probably combine them. Will close the other issue.

@knz knz mentioned this issue Apr 3, 2019
Closed
4 tasks
@dt
Copy link
Member

dt commented Apr 4, 2019

I think lint is happy now with #36486 and #36527

@chenrui333
Copy link

It looks like there is some issue with homebrew 2.1.7 upgrade: Homebrew/homebrew-core#40048

13:24:41 tar xof /Users/brew/Library/Caches/Homebrew/downloads/af0196d886dbaae1bb8c23b91f4cc38ef4dabdee0ede47364776c6f4094859ca--cockroach-v2.1.7.src.tgz -C /private/tmp/d20190519-47502-nel76u
13:24:41 cp -pR /private/tmp/d20190519-47502-nel76u/cockroach-v2.1.7/. /private/tmp/cockroach-20190519-47502-12kwzdy/cockroach-v2.1.7
13:24:41 chmod -Rf +w /private/tmp/d20190519-47502-nel76u
13:24:41 ==> make buildoss
13:24:41 make -C src/github.com/cockroachdb/cockroach buildoss
13:24:41 make[1]: Entering directory '/private/tmp/cockroach-20190519-47502-12kwzdy/cockroach-v2.1.7/src/github.com/cockroachdb/cockroach'
13:24:41 GOPATH set to /private/tmp/cockroach-20190519-47502-12kwzdy/cockroach-v2.1.7
13:24:41 go 1.12+ is known to produce invalid crdb builds, see https://github.com/cockroachdb/cockroach/issues/35637
13:24:41 Disable this check with IGNORE_GOVERS=1.
13:24:41 make[1]: *** [Makefile:660: build/defs.mk] Error 1
13:24:41 make[1]: Leaving directory '/private/tmp/cockroach-20190519-47502-12kwzdy/cockroach-v2.1.7/src/github.com/cockroachdb/cockroach'
13:24:41 make: *** [Makefile:6: buildoss] Error 2
13:24:41 
13:24:41 ==> Formula
13:24:41 Path: /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/cockroach.rb

@zbeekman
Copy link

Can someone please explain your versioning system? Chronologically 2.1.7 is the most recent release, but 19.... is the version you're shipping on your website.

For packagers this is not easy to find or well documented info.

Cheers 🍻

@knz
Copy link
Contributor Author

knz commented May 20, 2019

The most recent version is 19.1. However we also provide bug fixes in the previous releases 2.1 and security fixes in 2.0.
This is akin to how Postgres still provides bug fixes to version 10.8 and 9.6 etc. even though version 11.3 is the latest one.

@knz
Copy link
Contributor Author

knz commented May 20, 2019

@zbeekman @chenrui333
Please also read the linked issue: #35657
We do not support nor recommend building CockroachDB with any other version of Go than the one specifically allowed by our Makefile.
Thanks

@zbeekman
Copy link

This is akin to how Postgres still provides bug fixes to version 10.8 and 9.6 etc. even though version 11.3 is the latest one.

Sure, I'm familiar with supporting maintenance branches and backporting. The skip from 2 to 19 was the source of my confusion combined with it being somewhat unclear which versions are currently supported, and which is the newest, and where the major version numbers are coming from.

As far as building against older gos goes, I'm not sure if this will be possible in core, and what the best way to handle it is... You can always recommend (in your own documentation and site) that users install from your own tap if they want the most up-to-date version. In that case, we will probably continue to ship versions that we can, when they can be built against the latest go, and for newer releases that require older gos, we will probably just end up lagging behind the most recent release.

It would be really great if we could find a way to build the latest releases with the latest go for homebrew. I do understand, that it can be hard to keep up with the latest compilers and tooling though. Hopefully we can get to a place that is good for everyone.

🍻

@chenrui333 chenrui333 mentioned this issue May 20, 2019
Closed
@bdarnell
Copy link
Contributor

Another go 1.12 issue: If cockroach is built with 1.12, it will fail to start on systems where the file descriptor hard limit is below our recommended amount (previously it would log a warning and continue). See #37685 and golang/go#30401

andreimatei added a commit to andreimatei/cockroach that referenced this issue Jun 24, 2019
@andreimatei
build: require go1.12
521a52d 
Since the builder was upgraded a while ago and we already have code
needing 1.12.

Closes cockroachdb#35637

Release note: The minimum compiler version for building CRDB is now go
1.12.
@craig craig bot closed this as completed in a8b7483 Jun 26, 2019
irfansharif pushed a commit to irfansharif/cockroach that referenced this issue Nov 5, 2019
@bdarnell @irfansharif
build: Allow go 1.12 again
3fd067e 
Fears of invalid builds have been resolved, so we no longer need to
prohibit this.

Note that it is difficult to be lint-clean with both 1.11 and 1.12
simultaneously, so we'll have to wait to fix the new lint issues until
we move to 1.12 as the minimum version (post CRDB 19.1)

Updates cockroachdb#35637

Release note: None
@irfansharif irfansharif mentioned this issue Nov 5, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. meta-issue Contains a list of several other issues.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@dt @bdarnell @zbeekman @knz @benesch @chenrui333