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

[WIP] implement module compatibility check #46941

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Dec 15, 2023

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 " set.

To test this package:

make -C ./internal/gocompat/
GO111MODULE=off go generate .
go mod tidy
go test -v
# github.com/docker/docker/libnetwork/options
../../libnetwork/options/options.go:45:25: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
# github.com/docker/docker/libnetwork/internal/setmatrix
../../libnetwork/internal/setmatrix/setmatrix.go:13:16: type parameter requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../libnetwork/internal/setmatrix/setmatrix.go:13:18: predeclared comparable requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../libnetwork/internal/setmatrix/setmatrix.go:14:20: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../libnetwork/internal/setmatrix/setmatrix.go:20:10: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../libnetwork/internal/setmatrix/setmatrix.go:31:10: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../libnetwork/internal/setmatrix/setmatrix.go:43:10: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../libnetwork/internal/setmatrix/setmatrix.go:59:10: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../libnetwork/internal/setmatrix/setmatrix.go:80:10: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../libnetwork/internal/setmatrix/setmatrix.go:93:10: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../libnetwork/internal/setmatrix/setmatrix.go:104:10: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../libnetwork/internal/setmatrix/setmatrix.go:104:10: too many errors
# github.com/docker/docker/libnetwork/config
../../libnetwork/config/config.go:35:47: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../libnetwork/config/config.go:47:41: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../libnetwork/config/config.go:63:55: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../libnetwork/config/config.go:95:63: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
# github.com/docker/docker/testutil
../../testutil/helpers.go:80:9: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
# github.com/docker/docker/builder/builder-next/adapters/containerimage
../../builder/builder-next/adapters/containerimage/pull.go:72:4: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../builder/builder-next/adapters/containerimage/pull.go:200:19: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
FAIL	gocompat [build failed]
make: *** [Makefile:5: verify] Error 1

temporarily skip some packages

one, or combinations of these still makes it fail, but unfortunately
it doesn't print what file it is:

go test -v
# github.com/docker/docker/daemon
embedding interface element ~[]string requires go1.18 or later (-lang was set to go1.16; check go.mod)
FAIL	gocompat [build failed]

no external consumer should be importing the daemon-code, so skipping
checks for these (for now).

- 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)

daemon/info.go Outdated
Comment on lines 354 to 361
// // promoteNil converts a nil slice to an empty slice of that type.
// // A non-nil slice is returned as is.
// func promoteNil[S ~[]E, E any](s S) S {
// if s == nil {
// return S{}
// }
// return s
// }
Copy link
Member Author

@thaJeztah thaJeztah Dec 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if there's a bug in go; even with //go:build 1.19 this fails. I also tried //go:build go1.20 (and 1.21) but same;

make -C ./internal/gocompat/
make: Entering directory '/go/src/github.com/docker/docker/internal/gocompat'
GO111MODULE=off go generate .
GO111MODULE=on go mod tidy
GO111MODULE=on go test -v
# github.com/docker/docker/daemon
embedding interface element ~[]string 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/docker/internal/gocompat'

Some discussions related to that error (one was a bug in go);

Copy link
Contributor

@corhere corhere Dec 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lack of a position in the error message is suspicious. As is the error message: seems like it should only apply to embedded types in interfaces, though I could be wrong. I wonder if the type-checker is synthesizing a virtual interface type to represent the function's type constraint and neglecting to annotate it with the source position of the function, causing the type-checker to fall back to the default language version. Is gotip also affected? Only Go 1.21 is affected, not gotip.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a compiler bug. I have a minimal reproducer; will report upstream.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, thanks!! 🙏

Yes, after trying all angles, I started to suspect it could be a bug, but I failed to create a minimal reproducer so far, so wanted to give it more tries during the weekend.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@corhere
Copy link
Contributor

corhere commented Jun 21, 2024

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 -C ./internal/gocompat/
    GO111MODULE=off go generate .
    go mod tidy
    go test -v
    # github.com/docker/docker/libnetwork/options
    ../../libnetwork/options/options.go:45:25: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
    # github.com/docker/docker/libnetwork/internal/setmatrix
    ../../libnetwork/internal/setmatrix/setmatrix.go:13:16: type parameter requires go1.18 or later (-lang was set to go1.16; check go.mod)
    ../../libnetwork/internal/setmatrix/setmatrix.go:13:18: predeclared comparable requires go1.18 or later (-lang was set to go1.16; check go.mod)
    ../../libnetwork/internal/setmatrix/setmatrix.go:14:20: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
    ../../libnetwork/internal/setmatrix/setmatrix.go:20:10: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
    ../../libnetwork/internal/setmatrix/setmatrix.go:31:10: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
    ../../libnetwork/internal/setmatrix/setmatrix.go:43:10: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
    ../../libnetwork/internal/setmatrix/setmatrix.go:59:10: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
    ../../libnetwork/internal/setmatrix/setmatrix.go:80:10: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
    ../../libnetwork/internal/setmatrix/setmatrix.go:93:10: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
    ../../libnetwork/internal/setmatrix/setmatrix.go:104:10: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
    ../../libnetwork/internal/setmatrix/setmatrix.go:104:10: too many errors
    # github.com/docker/docker/libnetwork/config
    ../../libnetwork/config/config.go:35:47: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
    ../../libnetwork/config/config.go:47:41: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
    ../../libnetwork/config/config.go:63:55: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
    ../../libnetwork/config/config.go:95:63: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
    # github.com/docker/docker/testutil
    ../../testutil/helpers.go:80:9: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
    # github.com/docker/docker/builder/builder-next/adapters/containerimage
    ../../builder/builder-next/adapters/containerimage/pull.go:72:4: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
    ../../builder/builder-next/adapters/containerimage/pull.go:200:19: type instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
    FAIL	gocompat [build failed]
    make: *** [Makefile:5: verify] Error 1

[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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants