nytimes / gziphandler

Go middleware to gzip HTTP responses
https://godoc.org/github.com/NYTimes/gziphandler
Apache License 2.0
868 stars 129 forks source link

go mod depends on 1.12 #79

Closed jared2501 closed 5 years ago

jared2501 commented 5 years ago

The go module currently depends on an unreleased version of go. Can we bump the requires go version down to a released version?

jprobinson commented 5 years ago

Is the use of 1.12 negatively affecting you in some way?

It's in RC1 so it's the default version of Go internally to Google at this point, so should be fine to use in pipelines.

jared2501 commented 5 years ago

We use go 1.11 in all of our builds and don't wish to upgrade to 1.12 until it's released. In general, it's probably wise for libraries to have a lower bound on the go compiler version that they depend on since not all users are able to upgrade.

On Thu, Feb 21, 2019, 2:30 PM JP Robinson notifications@github.com wrote:

Is the use of 1.12 negatively affecting you in some way?

It's in RC1 so it's the default version of Go internally to Google at this point, so should be fine to use in pipelines.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/NYTimes/gziphandler/issues/79#issuecomment-466193948, or mute the thread https://github.com/notifications/unsubscribe-auth/ACu-MgY78GQEmup7E_kfFafjBDrU1pjqks5vPx3sgaJpZM4bIdOQ .

fsouza commented 5 years ago

@jared2501 the version of go specified in go.mod is informative and doesn't actually do anything. It's used by Go to suggest that you may need to upgrade you version in case the module doesn't compile, but gziphandler doesn't actually require 1.12.

See https://tip.golang.org/doc/go1.12#modules

The go directive in a go.mod file now indicates the version of the language used by the files within that module. It will be set to the current release (go 1.12) if no existing version is present. If the go directive for a module specifies a version newer than the toolchain in use, the go command will attempt to build the packages regardless, and will note the mismatch only if that build fails.

jared2501 commented 5 years ago

Hmm I'm not seeing this on go 1.11:

✓ jnewman@Jareds-MacBook-Pro:~/dev/front-door | jn/gzip● $ CGO_ENABLED=0 GOOS=darwin go build -v -o build/darwin-amd64/front-door ./cmd/front-door
github.com/NYTimes/gziphandler
go build github.com/NYTimes/gziphandler: module requires Go 1.12
✕ jnewman@Jareds-MacBook-Pro:~/dev/front-door | jn/gzip● $ go version
go version go1.11.2 darwin/amd64
jared2501 commented 5 years ago

If you're not keen to downgrade the suggested version, no worries, I can use the replace direct and depend on my fork until go 1.12 comes out and then move over.

fsouza commented 5 years ago

@jared2501 oh interesting. We can downgrade, but if the Go docs are telling the truth, the build is failing for some other reason and Go is trying to guess it's because of the language version? Is that code open source?

fsouza commented 5 years ago

(and if I have to guess, I'd go with golang/go#29278, which will likely require you to upgrade to Go 1.11.4)

fsouza commented 5 years ago

Reopening as we wait for more info.

jared2501 commented 5 years ago

Ah nice one @fsouza, that was it!

jared2501 commented 5 years ago

I upgraded to go 1.11.5, and could build v1.1.0 successfully

fsouza commented 5 years ago

Cool, thanks for confirming! It sucks that go swallows the original error message though, so let's keep go 1.11 in the mod file for now and v1.1.1.