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 1.13.5 #47510

Closed
wants to merge 2 commits into from
Closed

go 1.13.5 #47510

wants to merge 2 commits into from

Conversation

leonklingele
Copy link
Contributor

  • 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?
  • Is your test running fine brew test <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>)?

@twchn
Copy link

twchn commented Dec 5, 2019

What happened?

@chenrui333 chenrui333 changed the title Go 1.13.5 go 1.13.5 Dec 6, 2019
@chenrui333
Copy link
Member

Compared with what we done in the previous PRs, removed all the revision bumps. Will go straight for the merge.

@beauhoyt
Copy link

beauhoyt commented Dec 9, 2019

@leonklingele and @chenrui333 dont you need to bump godep version too?

Example from 1.13.4: https://github.com/Homebrew/homebrew-core/pull/46046/files

@leonklingele
Copy link
Contributor Author

I don't know why @chenrui333 has reverted the godep bump.

@chenrui333
Copy link
Member

sorry that is my fault. the godep is certainly needed. i was just trying to revert my change.

@chenrui333
Copy link
Member

Just re-commit the godep change.

While I was testing this PR yesterday, I did not find anything weird from my side.

@dxvgef
Copy link

dxvgef commented Dec 11, 2019

Has this PR not progressed? Many days have passed 😟

@SMillerDev
Copy link
Member

However magical software development may seem, it does actually involve humans making time and doing work. So no, it hasn't progressed. But you're always welcome to make a pull request that has all the issues fixed.

@davidlanouette
Copy link
Contributor

Hi all. I've partially tracked down the issue, but don't have time to dig in further right now.

The build is including the borg formula (not sure why or how yet). That formula builds and tests borg, using it's defaults. But, it's defaults point to a web site that nolonger exists.

From borg's README:
PLEASE READ: The website (https://ok-b.org) is down, because I didn't have time to maintain it. You can host borg yourself, and we plan to resurrect the version hosted by us on 1backend (https://github.com/1backend/1backend). The ETA for this is a couple of months.

That README was last updated 2 years ago, so it doesn't look like this is a new issue with borg itself.

Conjecture:

  1. The domain that borg was using finally was taken down, or the domain registry expired, or something similar that would make it inaccessible.

  2. Or, borg as added as a dependency in the recent past.

Potential solutions:

  1. Remove borg as a build dependency. Somebody would have to find out why it was added, and what it's used for.

  2. Remove or fix the test task in the borg formula. You can pass in a server to use (the -s option.)

I'll try to get back to it later.

@mat
Copy link

mat commented Dec 11, 2019

Given that the test in the current borg formula will fail anyway (because of the missing host), it makes sense to adjust the test there to e.g. borg —help.

(I agree that finding out what depends on borg would be great, too, regardless :-))

@tmm1
Copy link
Contributor

tmm1 commented Dec 12, 2019

  • Remove borg as a build dependency. Somebody would have to find out why it was added, and what it's used for.

Where is this dependency defined?

@poffe
Copy link

poffe commented Dec 12, 2019

Hi,

I've looked at the console output for the Jenkins job and it looks like after brew test go --verbose it starts to test every brew formula that is dependent on go.

So it goes

==> brew fetch --retry acmetool

==> brew install --only-dependencies acmetool

==> brew install acmetool

==> brew install --only-dependencies acmetool

==> brew linkage --test acmetool

==> brew install --only-dependencies --include-test acmetool

==> brew test --verbose acmetool

And so on... and borg is dependent on go and it fails the tests.

So borg is not a build dependency for go, it's the other way around. Is this the expected behaviour to fail the test of the go formula if any formula dependent on go fails it tests?

@SMillerDev
Copy link
Member

Yes, otherwise an update could break all formula that use go.

@iwittkau
Copy link
Contributor

How about changing borg's formula to test with borg --help instead of borg -p brew?
Another option is to remove borg entirely until the backend situation is resolved.

@iwittkau
Copy link
Contributor

Seems like there is no easy fix other than removing borg entirely for now.

I changed the test command to borg --help and brew audit --strict borg throws an error:

borg:
  * C: 21: col 3: `go_resource`s are deprecated. Please ask upstream to implement Go vendoring
  * C: 26: col 3: `go_resource`s are deprecated. Please ask upstream to implement Go vendoring
  * C: 31: col 3: `go_resource`s are deprecated. Please ask upstream to implement Go vendoring
Error: 3 problems in 1 formula detected

IMHO it's kind of pointless to try to fix borg, when the future of the project is unclear. See ok-borg/borg/README.md.

@iwittkau iwittkau mentioned this pull request Dec 13, 2019
2 tasks
@chenrui333
Copy link
Member

Now we have borg removed (Thanks @iwittkau), going to rebase the formula and retest.

@iwittkau
Copy link
Contributor

That did about nothing. We need to track down why these other formulas still fail.

Failed tests:

brew-test-bot.high_sierra.test dashing
brew-test-bot.high_sierra.test faas-cli
brew-test-bot.high_sierra.test gost
brew-test-bot.high_sierra.install iron-functions
brew-test-bot.high_sierra.test launch_socket_server
brew-test-bot.high_sierra.test piknik
brew-test-bot.high_sierra.test ssh-vault
brew-test-bot.high_sierra.test warp
brew-test-bot.catalina.test dashing
brew-test-bot.catalina.test gost
brew-test-bot.catalina.install iron-functions
brew-test-bot.catalina.test launch_socket_server
brew-test-bot.catalina.test ssh-vault
brew-test-bot.catalina.test warp
brew-test-bot.mojave.test dashing
brew-test-bot.mojave.test gost
brew-test-bot.mojave.install iron-functions
brew-test-bot.mojave.test launch_socket_server
brew-test-bot.mojave.test ssh-vault
brew-test-bot.mojave.test warp

@beauhoyt
Copy link

beauhoyt commented Dec 13, 2019

For Dashing looks like contents of https://ruby-doc.org/downloads/ruby_2_5_0_core_rdocs.tgz changed:

07:55:26 Error: dashing: failed
07:55:26 An exception occurred within a child process:
07:55:26   ChecksumMismatchError: SHA256 mismatch
07:55:26 Expected: 219e171641e979a5c8dee1b63347a1a26b94ba648aec96f7e6ed915d12bcaa15
07:55:26   Actual: 017d402805eecfb7638bcf92831c915d02dccaacb947fdbe4016edf1a2eff95c
07:55:26  Archive: 

I guess we only have to update this hash here:
https://github.com/Homebrew/homebrew-core/blob/master/Formula/dashing.rb#L21

@iwittkau
Copy link
Contributor

@beauhoyt it's a 403, seems like the URL has changed

@beauhoyt
Copy link

@iwittkau huh weird it worked for me 🤔

[13:32:27][beau@Beaus-MacBook-Pro][~](master)$ curl -v https://ruby-doc.org/downloads/ruby_2_5_0_core_rdocs.tgz > /dev/null
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0*   Trying 108.168.213.2...
* TCP_NODELAY set
* Connected to ruby-doc.org (108.168.213.2) port 443 (#0)
* ALPN, offering h2
* ALPN, offering http/1.1
* Cipher selection: ALL:!EXPORT:!EXPORT40:!EXPORT56:!aNULL:!LOW:!RC4:@STRENGTH
* successfully set certificate verify locations:
*   CAfile: /etc/ssl/cert.pem
  CApath: none
* TLSv1.2 (OUT), TLS handshake, Client hello (1):
} [512 bytes data]
* TLSv1.2 (IN), TLS handshake, Server hello (2):
{ [93 bytes data]
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0* TLSv1.2 (IN), TLS handshake, Certificate (11):
{ [4708 bytes data]
* TLSv1.2 (IN), TLS handshake, Server key exchange (12):
{ [333 bytes data]
* TLSv1.2 (IN), TLS handshake, Server finished (14):
{ [4 bytes data]
* TLSv1.2 (OUT), TLS handshake, Client key exchange (16):
} [70 bytes data]
* TLSv1.2 (OUT), TLS change cipher, Client hello (1):
} [1 bytes data]
* TLSv1.2 (OUT), TLS handshake, Finished (20):
} [16 bytes data]
* TLSv1.2 (IN), TLS change cipher, Client hello (1):
{ [1 bytes data]
* TLSv1.2 (IN), TLS handshake, Finished (20):
{ [16 bytes data]
* SSL connection using TLSv1.2 / ECDHE-RSA-AES256-GCM-SHA384
* ALPN, server did not agree to a protocol
* Server certificate:
*  subject: OU=Domain Control Validated; OU=PositiveSSL; CN=ruby-doc.org
*  start date: Apr 10 00:00:00 2018 GMT
*  expire date: Apr 27 23:59:59 2020 GMT
*  subjectAltName: host "ruby-doc.org" matched cert's "ruby-doc.org"
*  issuer: C=GB; ST=Greater Manchester; L=Salford; O=COMODO CA Limited; CN=COMODO RSA Domain Validation Secure Server CA
*  SSL certificate verify ok.
> GET /downloads/ruby_2_5_0_core_rdocs.tgz HTTP/1.1
> Host: ruby-doc.org
> User-Agent: curl/7.54.0
> Accept: */*
>
< HTTP/1.1 200 OK
< Date: Fri, 13 Dec 2019 21:32:31 GMT
< Server: Apache/2.2.22 (Ubuntu)
< Last-Modified: Tue, 01 Oct 2019 22:45:09 GMT
< ETag: "13e1137-177e0b-593e11abbe740"
< Accept-Ranges: bytes
< Content-Length: 1539595
< Content-Type: application/x-gzip
<
{ [16384 bytes data]
100 1503k  100 1503k    0     0  2304k      0 --:--:-- --:--:-- --:--:-- 2302k
* Connection #0 to host ruby-doc.org left intact
[13:32:31][beau@Beaus-MacBook-Pro][~](master)$

@iwittkau
Copy link
Contributor

@beauhoyt mysterious, now it works for me as well. Maybe the error here is caused by a flaky webserver.

@beauhoyt
Copy link

beauhoyt commented Dec 13, 2019

For Fass-cli (OpenFaaS CLI) https://github.com/openfaas/faas-cli
They seem to only be doing their travis-ci build against golang1.11.0 and not golang1.13.5 or newer. Which maybe the reason their tests are failing on the newer version:

08:08:53 Error: faas-cli: failed
08:08:53 An exception occurred within a child process:
08:08:53   Test::Unit::AssertionFailedError: <0> expected but was
08:08:53 <1>.

Is there anyway to force this Formula to only run against golang1.11.x?
https://github.com/Homebrew/homebrew-core/blob/master/Formula/faas-cli.rb

@iwittkau
Copy link
Contributor

https://github.com/wilhelm-murdoch/gost is also kind of outdated and uses deprecated formula fields. CI error might be related to a GitHub API change:

High Sierra / go 1.13.4:

brew test --verbose gost
Testing gost
/usr/bin/sandbox-exec -f /private/tmp/homebrew20191213-23709-26cdo0.sb ruby -W0 -I $LOAD_PATH -- /usr/local/Homebrew/Library/Homebrew/test.rb /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/gost.rb --verbose
==> /usr/local/Cellar/gost/1.2.0/bin/gost --file=test.txt
Gosting Gist ...
Unable to create gist: POST https://api.github.com/gists: 401 Requires authentication []
Error: gost: failed

@beauhoyt
Copy link

As for Gost (Some Gist posting tool) https://github.com/wilhelm-murdoch/gost
This doesn't actually have any golang test that are failing. It's just doing a very lame test write 42 to a temp file and try and post that on gist, which is failing because it requires authentication credentials. It's not failing because of golang1.13.5 just bad Formula IMHO.

08:27:29 Gosting Gist ... 
08:27:29 Unable to create gist: POST https://api.github.com/gists: 401 Requires authentication []
08:27:29 Error: gost: failed
08:27:29 Failed executing:

I think we should just remove this test or remove the whole thing 🤷‍♂
https://github.com/Homebrew/homebrew-core/blob/master/Formula/gost.rb#L61-L64

@iwittkau
Copy link
Contributor

@beauhoyt *sigh* I'll try removing it ...

@mistydemeo
Copy link
Member

I think launch_socket_server was just a buggy test, the software itself works fine.

@mistydemeo
Copy link
Member

Figured it out, it's the sandbox! The sandbox is preventing launch_socket_server from being able to launch the service; that explains why it works in the real world but not in brew test.

@mistydemeo mistydemeo mentioned this pull request Dec 15, 2019
5 tasks
@iwittkau
Copy link
Contributor

iwittkau commented Dec 15, 2019

Latest test results:

brew-test-bot.high_sierra.test cql
brew-test-bot.high_sierra.test dashing
brew-test-bot.high_sierra.test piknik
brew-test-bot.high_sierra.test sonobuoy
brew-test-bot.catalina.test dashing
brew-test-bot.catalina.test sonobuoy
brew-test-bot.mojave.test dashing
brew-test-bot.mojave.test sonobuoy

dashing and piknik were expected but cql and sonobuoy are new.

@iwittkau iwittkau mentioned this pull request Dec 15, 2019
5 tasks
@atakanyenel
Copy link

Hello, I'm trying to understand the above process and I have a question. So when golang updates, you also update every package that has go as It's dependency and point it to latest version. So when an error occurs, you debug them yourselves because update might break them. It seems that some tests are not working anymore, and the reasons are unrelated to go. Now comes the question. Can you make formulas pinpoint to a certain semver version like language package managers do ?

If I want to install a package that has dependency [email protected], and my machine has [email protected], just allow it because golang semver assures that they are compatible, as it's a patch. It's golang core team's responsibility. So if someone wants to use latest version of go, let them update their own package, do the tests and open a PR.

I'm probably missing some vital part as I don't know internals of homebrew very well , thus the question.

@SMillerDev
Copy link
Member

Homebrew generally only ships the last version so when Go updates, we check if that update doesn't break anything that we're already shipping (because users of homebrew shouldn't notice the difference). When errors occur we're debugging because it isn't always an issue with the software. Sometimes the test or CI is just flaky.

As for the versioning, that's pretty much what we do already. Versions depend on go which is always the latest. Or on go@12, which will lock them to the 1.12 releases.

As for letting people test their own stuff, that doesn't work for package managers. If you install a language dependent library, you probably know the language. If you install teleport from homebrew, how can you be expected to be fluent in Go?

@iwittkau
Copy link
Contributor

I also want to add that most of the failed formulas also failed on the previous version of Go. It just so happened that this was brought to attention through this PR because the CI tests everything when the Go version changes.
I think that the CI test should be run more often between releases of Go to detect broken formulas earlier and to avoid blocking situations like this.

@SMillerDev
Copy link
Member

I also want to add that most of the failed formulas also failed on the previous version of Go. It just so happened that this was brought to attention through this PR because the CI tests everything when the Go version changes.
I think that the CI test should be run more often between releases of Go to detect broken formulas earlier and to avoid blocking situations like this.

There's about 5000 formula in homebrew, just building the python one already maxes out our CI for the better part of a day. Testing all of them regularly simply isn't a viable solution.

@iwittkau
Copy link
Contributor

iwittkau commented Dec 16, 2019

There's about 5000 formula in homebrew, just building the python one already maxes out our CI for the better part of a day. Testing all of them regularly simply isn't a viable solution.

This may be true for python but doesn't the Go pipeline run in just under 3 hours? Anyway, this is probably the wrong place to discuss this ...

@poffe
Copy link

poffe commented Dec 16, 2019

First of all, thanks for all the hard work put into this.

I have a couple of reflections and questions about this.

First of all, isn't these dependencies on go from the other formulas only a build dependency? How about only testing that all the dependent formulas build ok instead of running the full test suite for them all?

Btw, are you all aware that Go 1.x kind of promises that a new release of Go should never break an existing program. I'm not saying that it will never happen but it's actually quite unlikely. You can read more about it here: https://golang.org/doc/go1compat

Second, there seems like most of the failing tests have dependencies on external systems, e.g. sites that has to be up and available. This doesn't feel like a sustainable solution. Are there guidelines on how testing should be designed?

@davidlanouette
Copy link
Contributor

davidlanouette commented Dec 16, 2019

So when golang updates, you also update every package that has go as It's dependency and point it to latest version.

@atakanyenel It's not that the build is updating every package that uses Go. It's just verifying that they all work with the new version of the Go formula. For Go specifically, this is generally not an issue. But other languages it could be an issue, and signal that more work needs to be done.

In this case, it seems that there happen to be a bunch of other formula that are just broken - not because of the change in Go, just because they haven't been maintained.

Once this change is committed, the other formula that depend on Go will not change. The only way they would change on a users machine is if the user chose to reinstall and used the --build-from-source flag.

HTH.

@davidlanouette
Copy link
Contributor

isn't these dependencies on go from the other formulas only a build dependency?

Hi @poffe. You are correct that Go is a build-only dependency. Once the binary is created, the end user has no need for any version of Go. But, if we only build, and don't test, those dependent formula, we could miss subtle bugs.

Yes, the Go team strives to maintain backwards compatibility. But they aren't perfect. I think it's better to deal with issues like this every now and then, then to have packages not install for end users (Yes, I know that the only way they wouldn't install is if the users included the --build-from-source flag. But it's still a possibility that would turn users off of using brew.)

Second, there seems like most of the failing tests have dependencies on external systems, e.g. sites that has to be up and available. This doesn't feel like a sustainable solution. Are there guidelines on how testing should be designed?

Your are right. The issues we are seeing now are not related to Go, but to various other things. This could be because brew is just getting too big to have a flat structure. But I don't think that's going to get resolved in this PR.

@iwittkau
Copy link
Contributor

Go 1.14 beta 1 released

The macOS release of Go 1.14 enables the Hardened Runtime.

Looking forward to this 😬

@iwittkau
Copy link
Contributor

iwittkau commented Dec 18, 2019

brew install --build-from-source dashing fails on go 1.13.4
brew test --verbose dashing fails on go 1.13.4

It is not because of go 1.13.5 (I encourage everybody to test this on their own machine).

So can we move on with the merge?

@BrewTestBot test this please

@poffe
Copy link

poffe commented Dec 18, 2019

brew install --build-from-source dashing fails on go 1.13.4
brew test --verbose dashing fails on go 1.13.4

I've tested this and it fails due to this versions of dashing is using glide in a build non-repeatable way. The glide.lock file is not committed and included in the tar. So when installing, glide is trying to fetch the latest versions for all the package dependencies and one of them fails.

Btw, dashing is now at version 0.4.0 and has moved from glide to using go modules (in a build repeatable fashion).

It is not because of go 1.13.5 (I encourage everybody to test this on their own machine).

No, it's not due to go 1.13.5, again... so how should this be resolved? Update the dashing formula?

So can we move on with the merge?

We could have moved on with the merge days ago if we change the strict dependency view of go. 😄

@BrewTestBot test this please

@iwittkau
Copy link
Contributor

@poffe, yes i've been tracking this here: #47842 and there technosophos/dashing#54

@mtibben
Copy link
Contributor

mtibben commented Dec 18, 2019

Go 1.14 beta 1 released

The macOS release of Go 1.14 enables the Hardened Runtime.

Looking forward to this 😬

Homebrew compiles go itself and does not codesign. As the runtime hardening flag is set during code signing, a Homebrew release of go will not have the hardened runtime. To get the hardened runtime, go would have to be released via Cask using the official binaries.

@poffe
Copy link

poffe commented Dec 18, 2019

@poffe, yes i've been tracking this here: #47842 and there technosophos/dashing#54

@iwittkau Great! So we can release the blocking of this pull request then?

@iwittkau
Copy link
Contributor

@iwittkau Great! So we can release the blocking of this pull request then?

@poffe That's what I would suggest. The homebrew maintainers have to decide this.

@iwittkau
Copy link
Contributor

Latest results: only dashing failed

@fxcoudert
Copy link
Member

Probably not worth blocking this for dashing, if the failure is known and being worked on. Merging, thanks everyone!

@fxcoudert fxcoudert closed this in 920e4c5 Dec 18, 2019
@lock lock bot added the outdated PR was locked due to age label Jan 17, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Jan 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.