implement module compatibility check#4723
Open
thaJeztah wants to merge 2 commits intodocker:masterfrom
Open
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
a385e26 to
2fc96b2
Compare
c0a89d6 to
b6a9390
Compare
Member
|
This is going to get rid of so many headaches in the future! Thanks @thaJeztah :') |
Member
Author
|
Was still deciding whether we should merge this one as-is (but we don't have a flow yet to run it cleanly, which could be some stage in the Dockerfile), or if we could make this work without that; per moby/moby#46941 (comment)
|
b6a9390 to
5fa9bea
Compare
cd0d419 to
557418d
Compare
557418d to
ea9bce2
Compare
cc3ffde to
d0457fa
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds an automated “module consumer compatibility” verification by generating a temporary module that imports all importable packages, ensuring files that use newer Go language features are properly guarded with //go:build constraints when the repo is consumed as a dependency in module mode.
Changes:
- Add
internal/gocompatgenerator + Makefile to build a synthetic module and compile-import all packages. - Vendor
golang.org/x/modand updatevendor.mod/vendor.sumto support parsing/formatting go.mod content. - Add a new GitHub Actions job to run the gocompat check in CI.
Reviewed changes
Copilot reviewed 7 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/validate.yml | Adds a CI job to run the new gocompat verification. |
| internal/gocompat/.gitignore | Ignores generated module/test artifacts in internal/gocompat. |
| internal/gocompat/Makefile | Adds generate and verify targets for the gocompat check. |
| internal/gocompat/generate.go | Adds go:generate entrypoint for the generator. |
| internal/gocompat/modulegenerator.go | Implements the generator that writes a synthetic go.mod and imports all packages. |
| vendor.mod | Adds golang.org/x/mod (and a tool line) to support the generator. |
| vendor.sum | Adds checksums for the new golang.org/x/mod version. |
| vendor/modules.txt | Registers vendored golang.org/x/mod packages. |
| vendor/golang.org/x/mod/LICENSE | Adds vendored license for golang.org/x/mod. |
| vendor/golang.org/x/mod/PATENTS | Adds vendored patents notice for golang.org/x/mod. |
| vendor/golang.org/x/mod/internal/lazyregexp/lazyre.go | Vendored x/mod internal helper used by the new dependency. |
| vendor/golang.org/x/mod/modfile/print.go | Vendored x/mod/modfile formatting support. |
| vendor/golang.org/x/mod/modfile/read.go | Vendored x/mod/modfile parsing support. |
| vendor/golang.org/x/mod/modfile/rule.go | Vendored x/mod/modfile editing/AST logic. |
| vendor/golang.org/x/mod/modfile/work.go | Vendored x/mod/modfile go.work support. |
| vendor/golang.org/x/mod/module/module.go | Vendored x/mod/module utilities. |
| vendor/golang.org/x/mod/module/pseudo.go | Vendored x/mod/module pseudo-version helpers. |
| vendor/golang.org/x/mod/semver/semver.go | Vendored x/mod/semver utilities. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
internal/gocompat/modulegenerator.go
Outdated
| // | ||
| // [DefaultGoModVersion]: https://github.com/golang/go/blob/58c28ba286dd0e98fe4cca80f5d64bbcb824a685/src/cmd/go/internal/gover/version.go#L15-L24 | ||
| // [2]: https://go.dev/doc/toolchain | ||
| func TestModuleCompatibllity(t *testing.T) { |
f544958 to
1be10f4
Compare
There was a problem hiding this comment.
Pull request overview
Adds an internal “module mode compatibility” check that simulates consuming this repository as a dependency (with an implicit go1.16 language default) by generating a small module that imports all importable packages and running it in CI. This helps ensure files using newer Go language features are guarded with appropriate //go:build go1.xx constraints so external consumers don’t hit unexpected compile failures.
Changes:
- Add
internal/gocompatgenerator + Makefile to build a generated module that imports all non-main, non-internalpackages. - Add a new CI job to run the gocompat verification.
- Vendor
golang.org/x/mod(and updatevendor.mod/vendor.sum) to generate/parse go.mod content for the check.
Reviewed changes
Copilot reviewed 7 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/validate.yml | Adds a CI job to run the gocompat verification. |
| internal/gocompat/.gitignore | Ignores generated module artifacts (go.mod/go.sum/main_test.go). |
| internal/gocompat/Makefile | Adds generate and verify targets to produce and test the generated module. |
| internal/gocompat/generate.go | Adds go:generate entrypoint to run the module generator. |
| internal/gocompat/modulegenerator.go | Implements generation of the test file importing all packages and the generated go.mod. |
| vendor.mod | Adds golang.org/x/mod (and a tool entry) to support go.mod generation/parsing for gocompat. |
| vendor.sum | Adds checksums for golang.org/x/mod v0.32.0. |
| vendor/modules.txt | Records vendored golang.org/x/mod packages. |
| vendor/golang.org/x/mod/LICENSE | Vendors upstream license text for golang.org/x/mod. |
| vendor/golang.org/x/mod/PATENTS | Vendors upstream patents grant for golang.org/x/mod. |
| vendor/golang.org/x/mod/internal/lazyregexp/lazyre.go | Vendors lazy regexp helper used by x/mod. |
| vendor/golang.org/x/mod/modfile/print.go | Vendors go.mod printer used by generator/parsing. |
| vendor/golang.org/x/mod/modfile/read.go | Vendors go.mod parser/AST used by generator/parsing. |
| vendor/golang.org/x/mod/modfile/rule.go | Vendors go.mod rule handling needed by modfile.Parse and edits. |
| vendor/golang.org/x/mod/modfile/work.go | Vendors go.work support used by x/mod/modfile. |
| vendor/golang.org/x/mod/module/module.go | Vendors module path/version logic used by x/mod/modfile. |
| vendor/golang.org/x/mod/module/pseudo.go | Vendors pseudo-version helpers used by module logic. |
| vendor/golang.org/x/mod/semver/semver.go | Vendors semver helpers used by module logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This package imports all "importable" packages, i.e., packages that:
- are not applications ("main")
- are not internal
- and that have non-test go-files
We do this to verify that our code can be consumed as a dependency
in "module mode". When using a dependency that does not have a go.mod
(i.e.; is not a "module"), go implicitly generates a go.mod. Lacking
information from the dependency itself, it assumes "go1.16" language
(see [DefaultGoModVersion]). Starting with Go1.21, go downgrades the
language version used for such dependencies, which means that any
language feature used that is not supported by go1.16 results in a
compile error;
# github.com/docker/cli/cli/context/store
/go/pkg/mod/github.com/docker/cli@v25.0.0-beta.2+incompatible/cli/context/store/storeconfig.go:6:24: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
/go/pkg/mod/github.com/docker/cli@v25.0.0-beta.2+incompatible/cli/context/store/store.go:74:12: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
These errors do NOT occur when using GOPATH mode, nor do they occur
when using "pseudo module mode" (the "-mod=mod -modfile=vendor.mod"
approach used in this repository).
As a workaround for this situation, we must include "//go:build" comments
in any file that uses newer go-language features (such as the "any" type
or the "min()", "max()" builtins).
From the go toolchain docs (https://go.dev/doc/toolchain):
> The go line for each module sets the language version the compiler enforces
> when compiling packages in that module. The language version can be changed
> on a per-file basis by using a build constraint.
>
> For example, a module containing code that uses the Go 1.21 language version
> should have a go.mod file with a go line such as go 1.21 or go 1.21.3.
> If a specific source file should be compiled only when using a newer Go
> toolchain, adding //go:build go1.22 to that source file both ensures that
> only Go 1.22 and newer toolchains will compile the file and also changes
> the language version in that file to Go 1.22.
This file is a generated module that imports all packages provided in
the repository, which replicates an external consumer using our code
as a dependency in go-module mode, and verifies all files in those
packages have the correct "//go:build <go language version>" set.
To test this package:
make shell
make -C ./internal/gocompat/
make: Entering directory '/go/src/github.com/docker/cli/internal/gocompat'
GO111MODULE=off go generate .
GO111MODULE=on go mod tidy
GO111MODULE=on go test -v
# github.com/docker/cli/templates
../../templates/templates.go:13:17: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
# github.com/docker/cli/cli/compose/template
../../cli/compose/template/template.go:98:45: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../cli/compose/template/template.go:105:27: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../cli/compose/template/template.go:141:28: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
# github.com/docker/cli/cli/compose/types
../../cli/compose/types/types.go:53:22: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../cli/compose/types/types.go:86:34: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../cli/compose/types/types.go:105:22: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../cli/compose/types/types.go:137:34: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../cli/compose/types/types.go:211:20: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../cli/compose/types/types.go:343:35: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../cli/compose/types/types.go:442:40: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../cli/compose/types/types.go:469:24: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../cli/compose/types/types.go:490:24: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../cli/compose/types/types.go:587:28: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../cli/compose/types/types.go:442:40: too many errors
# github.com/docker/cli/cli/context/store
../../cli/context/store/storeconfig.go:6:24: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../cli/context/store/store.go:74:12: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../cli/context/store/store.go:75:23: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../cli/context/store/metadatastore.go:43:58: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../cli/context/store/metadatastore.go:48:22: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../cli/context/store/metadatastore.go:80:30: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
# github.com/docker/cli/cli/command/idresolver
../../cli/command/idresolver/idresolver.go:6:2: "github.com/docker/docker/api/types" imported and not used
../../cli/command/idresolver/idresolver.go:7:2: "github.com/docker/docker/api/types/swarm" imported and not used
../../cli/command/idresolver/idresolver.go:9:2: "github.com/pkg/errors" imported and not used
../../cli/command/idresolver/idresolver.go:28:49: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../cli/command/idresolver/idresolver.go:58:53: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
# github.com/docker/cli/cli/compose/schema
../../cli/compose/schema/schema.go:20:46: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../cli/compose/schema/schema.go:27:53: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../cli/compose/schema/schema.go:45:32: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../cli/compose/schema/schema.go:66:33: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
FAIL gocompat [build failed]
make: *** [Makefile:3: verify] Error 1
make: Leaving directory '/go/src/github.com/docker/cli/internal/gocompat'
[DefaultGoModVersion]: https://github.com/golang/go/blob/58c28ba286dd0e98fe4cca80f5d64bbcb824a685/src/cmd/go/internal/gover/version.go#L15-L24
[2]: https://go.dev/doc/toolchain
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
There was a problem hiding this comment.
Pull request overview
Adds an internal/gocompat verification module intended to catch Go “language version downgrades” that happen when this repository is consumed as a dependency in module mode without an upstream go.mod, and updates vendoring to support generating/parsing the required module files.
Changes:
- Introduces
internal/gocompatgenerator + Makefile to build a synthetic module that imports all non-internal, non-mainpackages and runsgo testto surface language-version/build-tag issues. - Vendors
golang.org/x/mod(and updatesvendor.mod,vendor.sum,vendor/modules.txt) so the generator can manipulatevendor.modviamodfile. - Adds a GitHub Actions job to run the gocompat verification in CI.
Reviewed changes
Copilot reviewed 7 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| vendor/modules.txt | Adds vendored module listing entries for golang.org/x/mod. |
| vendor/golang.org/x/mod/semver/semver.go | New vendored x/mod semver implementation. |
| vendor/golang.org/x/mod/module/pseudo.go | New vendored x/mod pseudo-version helpers. |
| vendor/golang.org/x/mod/module/module.go | New vendored x/mod module path/version validation utilities. |
| vendor/golang.org/x/mod/modfile/work.go | New vendored x/mod go.work parsing/editing. |
| vendor/golang.org/x/mod/modfile/rule.go | New vendored x/mod go.mod parsing/editing logic (used by generator). |
| vendor/golang.org/x/mod/modfile/read.go | New vendored x/mod parser infrastructure. |
| vendor/golang.org/x/mod/modfile/print.go | New vendored x/mod formatter/printer. |
| vendor/golang.org/x/mod/internal/lazyregexp/lazyre.go | New vendored lazy regexp wrapper. |
| vendor/golang.org/x/mod/PATENTS | New vendored licensing file for x/mod. |
| vendor/golang.org/x/mod/LICENSE | New vendored licensing file for x/mod. |
| vendor.sum | Adds sums for golang.org/x/mod v0.32.0. |
| vendor.mod | Adds golang.org/x/mod dependency and a tool entry for the generator’s modfile usage. |
| internal/gocompat/modulegenerator.go | Generator that builds the import-all test and synthesizes a module based on vendor.mod. |
| internal/gocompat/generate.go | Adds go:generate entrypoint to run the generator. |
| internal/gocompat/Makefile | Adds generate/verify/clean targets for running the compatibility check. |
| internal/gocompat/.gitignore | Ignores generated module/test artifacts. |
| .github/workflows/validate.yml | Adds CI job to run the gocompat verification. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| _, err := os.Stat(modFile) | ||
| if err == nil { | ||
| return errors.New("go.mod exists in the repository root") | ||
| } |
Comment on lines
+59
to
+75
| modFile := filepath.Join(rootDir, "go.mod") | ||
| _, err := os.Stat(modFile) | ||
| if err == nil { | ||
| return errors.New("go.mod exists in the repository root") | ||
| } | ||
|
|
||
| // create an empty go.mod without go version. | ||
| // | ||
| // this go.mod must exist when running the test. | ||
| err = os.WriteFile(modFile, []byte("module github.com/docker/cli\n"), 0o644) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to write go.mod: %w", err) | ||
| } | ||
| defer func() { | ||
| if retErr != nil { | ||
| _ = os.Remove(modFile) | ||
| } |
Comment on lines
+3
to
+5
| GO111MODULE=on go test -v; status=$$?; \ | ||
| rm -f go.mod go.sum main.go main_test.go ../../go.mod; \ | ||
| exit $$status |
| shell: 'script --return --quiet --command "bash {0}"' | ||
| working-directory: ${{ github.workspace }}/src/github.com/docker/cli | ||
| run: | | ||
| make -C ./internal/gocompat verify clean |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This package imports all "importable" packages, i.e., packages that:
We do this to verify that our code can be consumed as a dependency in "module mode". When using a dependency that does not have a go.mod (i.e.; is not a "module"), go implicitly generates a go.mod. Lacking information from the dependency itself, it assumes "go1.16" language (see DefaultGoModVersion). Starting with Go1.21, go downgrades the language version used for such dependencies, which means that any language feature used that is not supported by go1.16 results in a compile error;
These errors do NOT occur when using GOPATH mode, nor do they occur when using "pseudo module mode" (the "-mod=mod -modfile=vendor.mod" approach used in this repository).
As a workaround for this situation, we must include "//go:build" comments in any file that uses newer go-language features (such as the "any" type or the "min()", "max()" builtins).
From the go toolchain docs (https://go.dev/doc/toolchain):
This file is a generated module that imports all packages provided in the repository, which replicates an external consumer using our code as a dependency in go-module mode, and verifies all files in those packages have the correct "//go:build " set.
To test this package:
- What I did
- How I did it
- How to verify it
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)