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.

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: fix installation of external tools with Go 1.7 #2874

Closed
wants to merge 2 commits into from

Conversation

rasky
Copy link
Contributor

@rasky rasky commented Jul 9, 2016

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same formula update/change?
  • Have you built your formula locally prior to submission with brew install <formula> (where <formula> is the name of the formula you're submitting)?
  • Does your submission pass brew audit --strict --online <formula> (after doing brew install <formula>)?

Go 1.7 integrated go vet in the standard toolchain, so it's not part of the external tools repo anymore. This PR fixes two related bugs in the formula:

  • Make sure that we checkout the correct tools branch when building devel; previously, it would always build the branch of the stable version. This is a potential bug because you can't use old tools (aligned with a previous version) with a newer Go version and expect them to work. See for instance go 1.7beta1: go vet: import failed: fmt.a:1:4 (byte offset = 3): expected keyword package, got "cn" golang/go#16044 (that is fixed by this patch).
  • Add a workaround to avoid failing when trying to install go vet from the tools directory. Once Go 1.7 is released and the whole formula cleaned up, the whole codeblock can just be removed.

@DomT4
Copy link
Member

DomT4 commented Jul 10, 2016

I'm not sure this is working as intended. It's building the 1.7 branch even for brew install go (i.e. the stable spec):

==> Cloning https://go.googlesource.com/tools.git
git clone --branch release-branch.go1.7 https://go.googlesource.com/tools.git /usr/local/var/homebrew/cache/go--gotools--git
Cloning into '/usr/local/var/homebrew/cache/go--gotools--git'...
remote: Sending approximately 11.37 MiB ...
remote: Counting objects: 770, done
remote: Finding sources: 100% (10/10)
remote: Total 16308 (delta 11045), reused 16303 (delta 11045)
Receiving objects: 100% (16308/16308), 11.34 MiB | 7.16 MiB/s, done.
Resolving deltas: 100% (11045/11045), done.
Checking connectivity... done.
git config homebrew.cacheversion 0
==> Checking out branch release-branch.go1.7
git checkout -f release-branch.go1.7 --
Already on 'release-branch.go1.7'
Your branch is up-to-date with 'origin/release-branch.go1.7'.

@rasky
Copy link
Contributor Author

rasky commented Jul 10, 2016

Oops thanks for noticing.

Any advice on how to fix it? How can I checkout different branches depending on whether the user requested the stable or devel version?

Giovanni Bajo
Develer S.r.l.
http://www.develer.com
Sent from mobile

Il giorno 10 lug 2016, alle ore 14:21, Dominyk Tiller notifications@github.com ha scritto:

I'm not sure this is working as intended. It's building the 1.7 branch even for brew install go (i.e. the stable spec):

==> Cloning https://go.googlesource.com/tools.git
git clone --branch release-branch.go1.7 https://go.googlesource.com/tools.git /usr/local/var/homebrew/cache/go--gotools--git
Cloning into '/usr/local/var/homebrew/cache/go--gotools--git'...
remote: Sending approximately 11.37 MiB ...
remote: Counting objects: 770, done
remote: Finding sources: 100% (10/10)
remote: Total 16308 (delta 11045), reused 16303 (delta 11045)
Receiving objects: 100% (16308/16308), 11.34 MiB | 7.16 MiB/s, done.
Resolving deltas: 100% (11045/11045), done.
Checking connectivity... done.
git config homebrew.cacheversion 0
==> Checking out branch release-branch.go1.7
git checkout -f release-branch.go1.7 --
Already on 'release-branch.go1.7'
Your branch is up-to-date with 'origin/release-branch.go1.7'.

You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@DomT4
Copy link
Member

DomT4 commented Jul 12, 2016

Oops thanks for noticing

'Tis why I'm here 😉.

It should just be a case of moving the resource inside respective stable do & devel do blocks with go_version set there. Will check it out more later today.

@DomT4 DomT4 added the go Go use is a significant feature of the PR or issue label Jul 12, 2016
@DomT4 DomT4 closed this in 0e12b73 Jul 18, 2016
DomT4 added a commit that referenced this pull request Jul 18, 2016
Fixes the issues discussed in #2874.
@DomT4
Copy link
Member

DomT4 commented Jul 18, 2016

Pushed some extra changes to this myself afterwards in ac2b85f, but merged your commit in 0e12b73. Apologies for the slow review here, things have been hectic lately. Appreciate your contribution to Homebrew 🙇

@Homebrew Homebrew locked and limited conversation to collaborators May 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
go Go use is a significant feature of the PR or issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants