go 1.8 #10048

ttaylorr · 2017-02-16T21:15:24Z

Have you followed the guidelines for contributing?
Have you checked that there aren't other open pull requests for the same formula update/change?
Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

wendigo · 2017-02-16T21:19:48Z

That was fast :)

brew bump-formula-pr --strict go
Error: These open pull requests may be duplicates:
go 1.8 https://github.com/Homebrew/homebrew-core/pull/10048
Duplicate PRs should not be opened. Use --force to override this error.

bcomnes · 2017-02-16T21:24:04Z

Formula/go.rb

probably need to update the comment here

Thanks, fixed 👍

tsuna · 2017-02-16T21:43:33Z

The godep check failed with a weird error:

==> /usr/local/Cellar/godep/79/bin/godep restore
godep: [WARNING]: godep should only be used inside a valid go package directory and
godep: [WARNING]: may not function correctly. You are probably outside of your $GOPATH.
godep: [WARNING]:	Current Directory: /private/tmp/godep-test-20170216-53415-iarcjp
godep: [WARNING]:	$GOPATH: /private/tmp/godep-test-20170216-53415-iarcjp
godep: Dep (golang.org/x/tools/cover) restored, but was unable to load it with error:
	Package (bufio) not found
godep: Error checking some deps.
Error: godep: failed
Failed executing: /usr/local/Cellar/godep/79/bin/godep restore

ttaylorr · 2017-02-16T21:52:52Z

The godep check failed with a weird error:

Strange. Anyone else getting this?

twexler · 2017-02-16T22:35:01Z

@ttaylorr I was able to replicate this failure. I am not entirely sure why it's happening yet, it looks like there is some issue with runtime.GOROOT()/$GOROOT. godep doesn't seem to be searching those paths for imports on 1.8.

DomT4 · 2017-02-16T22:37:58Z

godep at the very least will need a revision bump, as it always does on major bumps.

cblecker · 2017-02-16T22:38:29Z

The caveat should also be edited, as there is a functionality change in GOPATH:

    As of go 1.8, a default GOPATH will be created for you if it is not manually
    set:
      https://golang.org/doc/code.html#GOPATH

DomT4 · 2017-02-16T22:40:47Z

Formula/go.rb

Unless 1.7 has known issues bootstrapping 1.8 I'd probably leave this how it was. The original intention here was that no version of go should be bootstrapped by the same version of go, so that if there's a problem bootstrapping you know it's with the new from-source release rather than having to work out whether the problem is the binary or the from-source part.

(I appreciate when I tweaked this blob of code I made the comment confusing as all hell, apologies)

I think this argument makes more sense to me than #10048 (comment). Fixed!

Makes sense when you put it this way ;)

Apologies 😸. I've noticed with some pain that since I left the project code comments I added as a maintainer made sense sometimes exclusively to me, heh.

I don't think I really follow this argument. Does upstream actually say x.y should not be built with x.y itself or is this brew superstition?

It's just a tool we've used in the past for easier build failure debugging, but upstream themselves the last time I checked used Go 1.4 to bootstrap. We moved off of that system because for a while the 1.4 branch died on Sierra, which may or may not still be the case.

(More broadly I think you might as well scrap the code comment and replace it with something along the lines of don't update this unless this version cannot bootstrap the new version, and effectively pin it in place)

I believe they've fixed that (definitely MacPorts has). At some point (not this PR, of course), I think we should consider building our own 1.4 bootstrapper from source and avoid the upstream binaries altogether.

In any case, assuming for the sake of argument that I accept that this use-the-prior-version-to-bootstrap approach is useful, why 1.7 not 1.7.5?

I believe they quietly cut a new release, and we just never moved back to 1.4 because 1.7 wasn't causing any issues and changes to the go formula are rarely simple 😄, but I could be misremembering. I know there was talk of cutting a new 1.4 release simply for the sake of bootstrapping at least.

But yes, you're correct that having a gobootstrap or such from source would've allowed us to patch the 1.4 build failure at the time and keep using that branch for the process. The downside is that building even Go 1.4 is not ultra-quick in comparison to the existing process, and there hasn't been many clear upsides in the past.

In any case, assuming for the sake of argument that I accept that this use-the-prior-version-to-bootstrap approach is useful, why 1.7 not 1.7.5?

No reason at all. I simply wanted to avoid the confusion & hassle of people updating the bootstrap for every minor release. Less moving parts, less potential for breakage, etc etc.

paulocoutinhox · 2017-02-16T22:46:07Z

+1
Waiting for it!!!!

owenthereal · 2017-02-16T22:46:48Z

👍

twexler · 2017-02-16T22:49:32Z

godep at the very least will need a revision bump, as it always does on major bumps.

This is the issue, looks like godep just needs to be built from source again with 1.8.

twexler · 2017-02-16T22:55:59Z

Alternatively the formula could be modified to always set ENV['GOROOT'] from the output of go env GOROOT.

ttaylorr · 2017-02-16T23:02:33Z

godep at the very least will need a revision bump, as it always does on major bumps.

This is the issue, looks like godep just needs to be built from source again with 1.8.

I was unable to confirm that this is a correct fix. After bumping the version in godep's formula:

diff --git a/Formula/godep.rb b/Formula/godep.rb
index 6ee335ae..a9c003cc 100644
--- a/Formula/godep.rb
+++ b/Formula/godep.rb
@@ -25,7 +25,7 @@ class Godep < Formula
     (testpath/"Godeps/Godeps.json").write <<-EOS.undent
       {
         "ImportPath": "github.com/tools/godep",
-        "GoVersion": "go1.7",
+        "GoVersion": "go1.8",
         "Deps": [
           {
             "ImportPath": "golang.org/x/tools/cover",

and rebuilding it from source:

/u/l/H/L/T/h/homebrew-core (master!) $ go version
go version go1.8 darwin/amd64
/u/l/H/L/T/h/homebrew-core (master!) $ brew install --build-from-source godep
Updating Homebrew...
==> Using the sandbox
==> Downloading https://github.com/tools/godep/archive/v79.tar.gz
Already downloaded: /Users/ttaylorr/Library/Caches/Homebrew/godep-79.tar.gz
==> go build -o /usr/local/Cellar/godep/79/bin/godep
🍺  /usr/local/Cellar/godep/79: 3 files, 8.9M, built in 4 seconds
/u/l/H/L/T/h/homebrew-core (master!) $ godep version
godep v79 (darwin/amd64/go1.8)

the tests still failed:

/u/l/H/L/T/h/homebrew-core (master!) $ brew test --verbose godep
Testing godep
==> Using the sandbox
/usr/bin/sandbox-exec -f /tmp/homebrew20170216-14330-1dawnu0.sb /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/bin/ruby -W0 -I /usr/local/Homebrew/Library/Homebrew -- /usr/local/Homebrew/Library/Homebrew/test.rb /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/godep.rb --verbose
==> /usr/local/Cellar/godep/79/bin/godep restore
godep: [WARNING]: godep should only be used inside a valid go package directory and
godep: [WARNING]: may not function correctly. You are probably outside of your $GOPATH.
godep: [WARNING]:       Current Directory: /private/tmp/godep-test-20170216-14331-1fam7n9
godep: [WARNING]:       $GOPATH: /private/tmp/godep-test-20170216-14331-1fam7n9

@twexler -- were you able to spot anything I missed?

polds · 2017-02-16T23:03:29Z

I just looked and fossies.org doesn't have the 1.8 package yet either.

twexler · 2017-02-16T23:07:52Z

@twexler -- were you able to spot anything I missed?

This is a bit backwards(the first test is not the bottled version) because I screwed up, but I believe this demonstrates that --build-from-source fixes it.

[twexler:~/dev/tmp] 2 $ godep version
godep v79 (darwin/amd64/go1.8)

[twexler:~/dev/tmp] $ brew test godep
Testing godep
==> Using the sandbox
==> /usr/local/Cellar/godep/79/bin/godep restore

[twexler:~/dev/tmp] $ brew reinstall godep
==> Reinstalling godep
==> Downloading https://homebrew.bintray.com/bottles/godep-79.sierra.bottle.tar.gz
Already downloaded: /Users/twexler/Library/Caches/Homebrew/godep-79.sierra.bottle.tar.gz
==> Pouring godep-79.sierra.bottle.tar.gz
🍺  /usr/local/Cellar/godep/79: 3 files, 8.6M

[twexler:~/dev/tmp] $ brew test godep
Testing godep
==> Using the sandbox
==> /usr/local/Cellar/godep/79/bin/godep restore
Last 15 lines from /Users/twexler/Library/Logs/Homebrew/godep/test.01.godep:
2017-02-16 15:04:58 -0800

/usr/local/Cellar/godep/79/bin/godep
restore

godep: [WARNING]: godep should only be used inside a valid go package directory and
godep: [WARNING]: may not function correctly. You are probably outside of your $GOPATH.
godep: [WARNING]:	Current Directory: /private/tmp/godep-test-20170216-59376-z4jxm2
godep: [WARNING]:	$GOPATH: /private/tmp/godep-test-20170216-59376-z4jxm2
godep: Dep (golang.org/x/tools/cover) restored, but was unable to load it with error:
	Package (bufio) not found
godep: Error checking some deps.
Error: godep: failed
Failed executing: /usr/local/Cellar/godep/79/bin/godep restore
/usr/local/Homebrew/Library/Homebrew/formula.rb:1834:in `block in system'
/usr/local/Homebrew/Library/Homebrew/formula.rb:1772:in `open'
/usr/local/Homebrew/Library/Homebrew/formula.rb:1772:in `system'
/usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/godep.rb:37:in `block in <class:Godep>'
/usr/local/Homebrew/Library/Homebrew/formula.rb:1671:in `block (2 levels) in run_test'
/usr/local/Homebrew/Library/Homebrew/formula.rb:884:in `with_logging'
/usr/local/Homebrew/Library/Homebrew/formula.rb:1670:in `block in run_test'
/usr/local/Homebrew/Library/Homebrew/extend/fileutils.rb:14:in `block in mktemp'
/usr/local/Homebrew/Library/Homebrew/extend/fileutils.rb:74:in `block in run'
/usr/local/Homebrew/Library/Homebrew/extend/fileutils.rb:74:in `chdir'
/usr/local/Homebrew/Library/Homebrew/extend/fileutils.rb:74:in `run'
/usr/local/Homebrew/Library/Homebrew/extend/fileutils.rb:13:in `mktemp'
/usr/local/Homebrew/Library/Homebrew/formula.rb:1664:in `run_test'
/usr/local/Homebrew/Library/Homebrew/test.rb:28:in `block in <main>'
/System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/timeout.rb:66:in `timeout'
/usr/local/Homebrew/Library/Homebrew/test.rb:27:in `<main>'

[twexler:~/dev/tmp] 1 $ godep version
godep v79 (darwin/amd64/go1.7.5)
[twexler:~/dev/tmp] $

EDIT: Cleaned up my shell log

DomT4 · 2017-02-16T23:09:06Z

@twexler -- were you able to spot anything I missed?

Heh, the test actually passed there on godep 😉. The warning about GOPATH is expected. A failure more explicitly blows up:

Error: godep: failed
Failed executing: /usr/local/Cellar/godep/79_1/bin/godep restore

Running this process:

brew pull https://github.com/Homebrew/homebrew-core/pull/10048
brew upgrade -s -v go
brew edit godep

diff --git a/Formula/godep.rb b/Formula/godep.rb
index 6ee335aec5..bdf5d8459c 100644
--- a/Formula/godep.rb
+++ b/Formula/godep.rb
@@ -3,6 +3,8 @@ class Godep < Formula
   homepage "https://godoc.org/github.com/tools/godep"
   url "https://github.com/tools/godep/archive/v79.tar.gz"
   sha256 "3dd2e6c4863077762498af98fa0c8dc5fedffbca6a5c0c4bb42b452c8268383d"
+  revision 1
+
   head "https://github.com/tools/godep.git"
 
   bottle do
@@ -25,7 +27,7 @@ class Godep < Formula
     (testpath/"Godeps/Godeps.json").write <<-EOS.undent
       {
         "ImportPath": "github.com/tools/godep",
-        "GoVersion": "go1.7",
+        "GoVersion": "go1.8",
         "Deps": [
           {
             "ImportPath": "golang.org/x/tools/cover",

brew install godep -s -v
brew test godep -v

Produces no problems locally.

ttaylorr · 2017-02-16T23:14:18Z

@DomT4, @twexler -- thanks for your help, both of your comments clarified things a bunch. Judging from past PRs (in particular: #3924), it looks like the thing to do is increment the revision on all formulae that depend on Go.

I'll do that now.

twexler · 2017-02-16T23:15:23Z

I'll do that now.

Sweet! Excited for this.

DomT4 · 2017-02-16T23:15:59Z

it looks like the thing to do is increment the revision on all formulae that depend on Go.

Note: Only the ones that use go at runtime. Any that are :build don't need bumping, or we'll be here for days.

ttaylorr · 2017-02-16T23:16:19Z

Note: Only the ones that use go at runtime.

Thanks. I gathered from running ack 👍

twexler · 2017-02-16T23:16:33Z

it looks like the thing to do is increment the revision on all formulae that depend on Go.

@ttaylorr You should also add a comment to go.rb mentioning this.

ttaylorr · 2017-02-16T23:25:18Z

Thanks again, all. For posterity, Go 1.8 still supports Mountain Lion, but:

Go 1.8 now only supports OS X 10.8 or later. This is likely the last Go release to support 10.8. Compiling Go or running binaries on older OS X versions is untested.

(ref: https://beta.golang.org/doc/go1.8#ports)

So Go 1.9 will need to update this in the formula.

twexler · 2017-02-16T23:27:24Z

Formula/go.rb

Is it all version bumps, or just major/minor?

#6096, #7452, and #9304 would seem to indicate it's only major/minor and not patch versions

I assume it would vary between formulae, so I left it unspecific.

Having patch version bumped go myself multiple times, these extra revision bumps on dependant formula were not needed. Based on what we know now, I'd recommend clarifying this to just major/minor.

In the case of godep, it's currently required every time even on revision bumps. The other revision bumps in this PR are probably unnecessary altogether.

wkschwartz · 2017-02-16T23:49:17Z

Formula/go.rb

s/those formula's revision./those formulas' revisions./

If you want to be particularly consistent about it, Homebrew's preferred plural for formula is formulae 😉, so something like:

This can be done by bumping the revision of the `brew uses go` formulae.

Also this comment should ideally be at the top of the formula rather than nested, and I'd perhaps limit it to "major version bumps" given those are the most problematic. But at this point I'd be tempted to leave it and let @ilovezfs tidy up when he merges 😈

Thanks for the suggestions. I don't want to bikeshed this longer than we need to, so I'm alright with deferring to @ilovezfs's judgement, if he's OK with contributing here.

I've ticked the 'Allow edits from maintainers.' box, so anyone from Homebrew core is welcome to touch this.

Probably no comment is needed since nothing has changed.

silentred · 2017-02-17T02:44:58Z

Looking forward to this upgrade!

ilovezfs

Looks like bootstrap wasn't updated to

https://storage.googleapis.com/golang/go1.8.darwin-amd64.tar.gz

ilovezfs · 2017-02-17T09:06:08Z

Formula/go.rb

This appears to be the hash of the tree not the top commit, and I think specifying branch at all is actually incorrect, since that's really only for head specs that use a branch other than master.

ilovezfs · 2017-02-17T09:07:42Z

Formula/go.rb

ilovezfs · 2017-02-17T09:11:04Z

Formula/go.rb

Probably no comment is needed since nothing has changed.

ilovezfs · 2017-02-17T10:51:46Z

@DomT4 PR refreshed.

DomT4 · 2017-02-17T11:09:22Z

👍 LGTM, for what that's worth these days heh.

ilovezfs · 2017-02-17T11:11:06Z

LGTM, for what that's worth these days heh.

I still have your badge in the drawer whenever you'd like it back :)

DomT4 · 2017-02-17T11:15:50Z

I still have your badge in the drawer whenever you'd like it back :)

But retirement has been such a quiet place where I get to randomly work on my own mildly obscure side projects 🙈. But the desire to put up with me again, which I don't quite understand why you would want to heh, is appreciated as always.

ilovezfs · 2017-02-17T11:15:58Z

@ttaylorr thanks for the PR! 🚀

ilovezfs · 2017-02-17T11:20:22Z

But the desire to put up with me again, which I don't quite understand why you would want to heh, is appreciated as always.

🙇‍♂️

freeformz · 2017-02-17T18:46:01Z

FWIW ....

Given this error message:

godep: [WARNING]: godep should only be used inside a valid go package directory and
godep: [WARNING]: may not function correctly. You are probably outside of your $GOPATH.
godep: [WARNING]:	Current Directory: /private/tmp/godep-test-20170216-59376-z4jxm2
godep: [WARNING]:	$GOPATH: /private/tmp/godep-test-20170216-59376-z4jxm2

The current directory (/private/tmp/godep-test-20170216-59376-z4jxm2) is not a valid location for go packages to exist at. The minimal filepath would be: /private/tmp/godep-test-20170216-59376-z4jxm2/src/<anything>. So godep is telling you that it should "only be used inside of a valid go package directory". This may not be the best wording and I'm open to suggestions to fix the wording. I should also add a test to see if ! strings.HasPrefix(os.Getwd(), $GOPATH) and only then make the $GOPATH suggestion.

ilovezfs · 2017-02-17T18:58:12Z

That's usually a warning (not an error) that occurs when we use the symlink black magic.

bcomnes reviewed Feb 16, 2017

View reviewed changes

ttaylorr mentioned this pull request Feb 16, 2017

all: build using Go 1.8 git-lfs/git-lfs#1952

Merged

DomT4 reviewed Feb 16, 2017

View reviewed changes

twexler reviewed Feb 16, 2017

View reviewed changes

wkschwartz reviewed Feb 16, 2017

View reviewed changes

ilovezfs suggested changes Feb 17, 2017

View reviewed changes

ttaylorr added 3 commits February 17, 2017 02:50

go 1.8

14d6c06

godep: revision for go

06593c8

govendor: revision for go

1ee9d3d

ilovezfs approved these changes Feb 17, 2017

View reviewed changes

ilovezfs closed this in 86c2148 Feb 17, 2017

Homebrew locked and limited conversation to collaborators May 4, 2018

Uh oh!

go 1.8 #10048

go 1.8 #10048

Uh oh!

Conversation

ttaylorr commented Feb 16, 2017

Uh oh!

wendigo commented Feb 16, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tsuna commented Feb 16, 2017

Uh oh!

ttaylorr commented Feb 16, 2017

Uh oh!

twexler commented Feb 16, 2017

Uh oh!

DomT4 commented Feb 16, 2017

Uh oh!

cblecker commented Feb 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paulocoutinhox commented Feb 16, 2017

Uh oh!

owenthereal commented Feb 16, 2017

Uh oh!

twexler commented Feb 16, 2017

Uh oh!

twexler commented Feb 16, 2017

Uh oh!

ttaylorr commented Feb 16, 2017

Uh oh!

polds commented Feb 16, 2017

Uh oh!

twexler commented Feb 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DomT4 commented Feb 16, 2017

Uh oh!

ttaylorr commented Feb 16, 2017

Uh oh!

twexler commented Feb 16, 2017

Uh oh!

DomT4 commented Feb 16, 2017

Uh oh!

ttaylorr commented Feb 16, 2017

Uh oh!

twexler commented Feb 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ttaylorr commented Feb 16, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cblecker commented Feb 16, 2017 •

edited

Loading

twexler commented Feb 16, 2017 •

edited

Loading

twexler commented Feb 16, 2017 •

edited

Loading