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

Allow to build with Go 1.17 and Go 1.16 #618

Conversation

klingtnet
Copy link
Contributor

@klingtnet klingtnet commented Sep 14, 2021

Without Go 1.17 in the list of supported build tags the iofs package is
not importable in a Go 1.17 project. A minimal example can be found in
this playground. The example will show the following build error:

prog.go:9:5: module github.com/golang-migrate/migrate/v4@latest found
(v4.14.1), but does not contain package
github.com/golang-migrate/migrate/v4/source/iofs

Without Go 1.17 in the list of supported build tags the iofs package is
not importable in a Go 1.17 project.  A minimal example can be found in
this [playground][1].  The example will show the following build error:

    prog.go:9:5: module github.com/golang-migrate/migrate/v4@latest found
    (v4.14.1), but does not contain package
    github.com/golang-migrate/migrate/v4/source/iofs

[1]: https://play.golang.org/p/d6t91tWfYlJ
@klingtnet klingtnet force-pushed the allow-source-iofs-with-Go-1.17 branch from fa0cce6 to ccfdae6 Compare September 14, 2021 14:23
@coveralls
Copy link

coveralls commented Sep 14, 2021

Pull Request Test Coverage Report for Build 1234022515

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 57.709%

Totals Coverage Status
Change from base Build 1223561164: 0.0%
Covered Lines: 3713
Relevant Lines: 6434

💛 - Coveralls

@dhui
Copy link
Member

dhui commented Sep 15, 2021

Weird, go1.16 should be sufficient since it means Go 1.16 onwards.
This might be an issue with modules which hasn't always played well with build constraints...

@klingtnet
Copy link
Contributor Author

klingtnet commented Sep 15, 2021

@dhui I might really be an issue with how Go Modules handles build tags. Go's build constraint documentation states:

  • a term for each Go major release, through the current version:
    "go1.1" from Go version 1.1 onward, "go1.12" from Go 1.12, and so on.

But, I cannot import this source/iofs without specifying Go 1.17 explicitly. Here's a proof that the forked version builds fine:

$ make
CGO_ENABLED=0 go build -o inject -ldflags="-X main.Version=2.7.5-1-g1dcf197" ./cmd/inject
cmd/inject/inject.go:30:2: no required module provides package github.com/golang-migrate/migrate/v4/source/iofs; to add it:
        go get github.com/golang-migrate/migrate/v4/source/iofs
make: *** [Makefile:12: inject] Error 1
$ go mod edit -replace=github.com/golang-migrate/migrate/v4=github.com/spreadshirt/migrate/[email protected]
$ make
CGO_ENABLED=0 go build -o inject -ldflags="-X main.Version=2.7.5-1-g1dcf197" ./cmd/inject
go: updates to go.mod needed; to update it:
        go mod tidy
make: *** [Makefile:12: inject] Error 1
$ go mod tidy
$ git diff go.mod
diff --git a/go.mod b/go.mod
index 8d90c31..ffc54a1 100644
--- a/go.mod
+++ b/go.mod
@@ -76,3 +76,5 @@ require (
        gopkg.in/square/go-jose.v2 v2.6.0 // indirect
        gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect
 )
+
+replace github.com/golang-migrate/migrate/v4 => github.com/spreadshirt/migrate/v4 v4.6.1-0.20210914142308-ccfdae602432
$ make
CGO_ENABLED=0 go build -o inject -ldflags="-X main.Version=2.7.5-1-g1dcf197" ./cmd/inject

I think we can close this and I need to raise an issue in the golang/go repository.

@dhui
Copy link
Member

dhui commented Sep 15, 2021

Closing since other versions (not v4.14.1) should work. See: golang/go#48397 (comment)

@dhui dhui closed this Sep 15, 2021
@klingtnet klingtnet deleted the allow-source-iofs-with-Go-1.17 branch September 16, 2021 05:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants