migrate to go modules #716

thaJeztah · 2021-03-25T10:55:04Z

replaces / closes #704

migrate to go modules

This switches the repository to use go modules. This commit is a "mechanical"
migration, keeping all dependencies to the same version as is currently used,
and is the result of:

go mod init
go mod tidy

After that, I updated the go version in go.mod to 1.11, which is the oldest
version currently tested against (and first go release with go modules, although
go modules should not be required, as vendoring is used).

~~It looks like there's a bug in go mod vendor, causing it to omit nested LICENSE~~
~~files, and LICENSE files with an extension (e.g. LICENSE.txt). As a result,~~
~~two licenses are removed in this commit;~~

~~vendor/github.com/xeipuuv/gojsonschema/json_schema_test_suite/LICENSE~~
update: looks like the corresponding files were never vendored, so it makes sense to not vendor the LICENSE
~~vendor/github.com/stretchr/testify/LICENCE.txt~~

~~The first one definitely is a bug, because modules may include separate licenses~~
~~for specific parts (for example, when copying/forking code from other sources).~~

It looks like go mod vendor ignores LICENSE files with an extension, as a result,
the license for testify is removed in this commit;
vendor/github.com/stretchr/testify/LICENCE.txt

Another LICENSE is removed in gojsonschema, which is correct, as the license
covered files that were not vendored;
vendor/github.com/xeipuuv/gojsonschema/json_schema_test_suite/LICENSE

The testify license is "to be discussed"; license with an extension is non-standard,
but may occur; the issue will be resolved when updating to a more recent version
of testify; the version currently used is very old (pre v1.2.0, Aug 10, 2017);
https://github.com/stretchr/testify/tree/890a5c3458b43e6104ff5da8dfa139d013d77544),
and current version have renamed the file to LICENSE.### Remove GoDeps

removes the GoDeps directory and removes it from .gitignore

Add vendor make target, and vendor validation

Add a make vendor target for convenience, and add a vendor validation
target (and run it in CI) to verify the vendored files match the expected
versions.

thaJeztah · 2021-03-25T10:56:03Z

vendor/github.com/stretchr/testify/LICENCE.txt

@@ -1,22 +0,0 @@
-Copyright (c) 2012 - 2013 Mat Ryer and Tyler Bunnell


Removal of this file looks to be a bug in Go modules; but (as outlined above) should be fixed after updating this to a more current version (I'll open a follow-up PR to do so)

thaJeztah · 2021-03-25T10:57:51Z

vendor/github.com/xeipuuv/gojsonschema/json_schema_test_suite/LICENSE

@@ -1,19 +0,0 @@
-Copyright (c) 2012 Julian Berman


This definitely looks like a bug in go modules (also as outlined above); this file has a standard name, so should not be removed by go mod vendor; I'll create a minimal reproducer and open a bug-report.

Ah, actually; ignore this: looks like the associated files are not vendored, so it makes sense to not vendor the LICENSE as well

thaJeztah · 2021-03-25T11:00:00Z

Interesting; I see some file permissions changed when venturing with go mod vendor; let me check which one is the correct one / matching with upstream (old or new);

thaJeztah · 2021-03-25T11:01:16Z

Ah... hm.. go 1.11 is used in CI, which didn't have go modules by default;

The command "git-validation -run DCO,short-subject -v -range ${TRAVIS_COMMIT_RANGE}" exited with 0.
$ make validate-vendor
go: unknown subcommand "mod"
Run 'go help' for usage.
Makefile:67: recipe for target 'vendor' failed
make: *** [vendor] Error 2
The command "make validate-vendor" exited with 2.

thaJeztah · 2021-03-25T12:11:15Z

For the permission bits; looks like go mod messes that up. Checking with the upstream source, it looks like they should have their executable bit set;

I checked if I didn't override git config to ignore;

git config --get-all core.fileMode
true

Clone upstream repository and check the file permissions at the given commit:

git clone https://github.com/golang/sys.git
cd sys
git checkout f3918c30c5c2cb527c0b071a27c35120a6c0719a

git ls-files -s -- unix/*.pl
100755 34f8ef829bb4389f5bbff68527fc5c9c3877d0a9 0	unix/mksyscall.pl
100755 939c8a79157927194779e757c349a4a96efc95ba 0	unix/mksyscall_solaris.pl
100755 be67afa4175f92bae6ccf5f2788dcd583a475f62 0	unix/mksysctl_openbsd.pl
100755 d3e5147fcb3790e7b6903346bcb0e36169b0f1e5 0	unix/mksysnum_darwin.pl
100755 266a248c75c1cf5c994a8f2801534843bb055f40 0	unix/mksysnum_dragonfly.pl
100755 b767e124cc271e3f5f3c90a3bb6f90bf093d7728 0	unix/mksysnum_freebsd.pl
100755 872ae8c520e8de84175bc117cd57734a8d561d8d 0	unix/mksysnum_linux.pl
100755 e74616a65aefe69191954e0569929ee51c1f1f8e 0	unix/mksysnum_netbsd.pl
100755 ae5aad58660845c98d30984ab843b67d9c71cf05 0	unix/mksysnum_openbsd.pl

git ls-files -s -- unix/mkall.sh
100755 c1fc2adb88dd45bf5adca3deb7ffdd4ed827eaf0 0	unix/mkall.sh

Also tried if that goes lost in translation in the proxy, but trying to vendor with GOPROXY=direct produces the same.

To prevent `go get` from updating the project's go.mod and go.sum. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

This switches the repository to use go modules. This commit is a "mechanical" migration, keeping all dependencies to the same version as is currently used, and is the result of: go mod init go mod tidy After that, I updated the go version in go.mod to 1.11, which is the oldest version currently tested against (and first go release with go modules, although go modules should not be required, as vendoring is used). It looks like `go mod vendor` ignores LICENSE files with an extension, as a result, the license for testify is removed in this commit; vendor/github.com/stretchr/testify/LICENCE.txt Another LICENSE is removed in gojsonschema, which is correct, as the license covered files that were not vendored; vendor/github.com/xeipuuv/gojsonschema/json_schema_test_suite/LICENSE The testify license is "to be discussed"; license with an extension is non-standard, but may occur; the issue will be resolved when updating to a more recent version of testify; the version currently used is very old (pre v1.2.0, Aug 10, 2017); https://github.com/stretchr/testify/tree/890a5c3458b43e6104ff5da8dfa139d013d77544), and current version have renamed the file to LICENSE. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Add a `make vendor` target for convenience, and add a vendor validation target (and run it in CI) to verify the vendored files match the expected versions. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah · 2021-03-25T14:47:40Z

@crosbymichael @tianon @mrunalp ptal 🤗

I'll have a look at updating some of the old dependencies after this; trying to keep vendoring close to the current versions

dqminh · 2021-03-25T14:52:30Z

.travis.yml

-    && go get golang.org/x/lint/golint
-  - go get github.com/vbatts/git-validation
+    && GO111MODULE=off go get golang.org/x/lint/golint
+  - GO111MODULE=off go get github.com/vbatts/git-validation


instead of turning it on/off manually, you can probably just move to a directory that doesn't have go.mod such as /tmp

I thought this was slightly cleaner (not having to create a tmp dir, or assume /tmp is present), but happy to update if you prefer that.

Isn't this variable going away completely in 1.17? 😬

If we remove the old go versions, we can probably use go install for these. I tried to keep this PR focused on just the switch to go mod; but have some branches for follow ups.

I added go1.16 to the matrix in this PR as there's differences in vendoring / module resolution / go.sum between go versions, and I wanted to prevent additional churn if I switched in a follow up.

Happy to make changes in this PR though, if that's preferred

Travis is no more, so we can drop this entirely.

It seems that we only use vbatts/git-validation for DCO and commit subject length checks. First can be replaced by github native DCO checker, and for the second I mocked up something here: https://github.com/opencontainers/runc/blob/97875b0abfaec23923d9506901073bf4da4b033e/.github/workflows/validate.yml#L108-L124 (initially added by opencontainers/runc@8ccd39a)

dqminh · 2021-03-25T14:52:42Z

Makefile

+
+vendor:
+	GO111MODULE=on go mod tidy
+	GO111MODULE=on go mod vendor


do we need to turn this on explicitly ?

So, I added it, because go 1.11 won't use go modules if we're anywhere inside GOPATH. That said; contributors very likely won't be on go 1.11, and I now exclude this in CI, so I can remove it now, or in the same PR where I will remove Go 1.11 from the matrix (I think go 1.11 could probably be removed by now, unless we want to continue verifying that older go versions still build)

thaJeztah commented Mar 25, 2021

View reviewed changes

thaJeztah force-pushed the go_modules branch 2 times, most recently from e2fe9e8 to e560f05 Compare March 25, 2021 11:40

thaJeztah force-pushed the go_modules branch from e560f05 to 4d847fc Compare March 25, 2021 12:15

thaJeztah added 2 commits March 25, 2021 13:58

disable go modules when installing test-utilities

13e8789

To prevent `go get` from updating the project's go.mod and go.sum. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

travis: add go1.16

c3b18ec

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah force-pushed the go_modules branch from 4d847fc to 15f0618 Compare March 25, 2021 13:01

thaJeztah force-pushed the go_modules branch 2 times, most recently from 9888f8a to 24ea75b Compare March 25, 2021 13:23

thaJeztah added 2 commits March 25, 2021 14:25

Add vendor make target, and vendor validation

a8b321a

Add a `make vendor` target for convenience, and add a vendor validation target (and run it in CI) to verify the vendored files match the expected versions. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Remove GoDeps

1257f22

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah force-pushed the go_modules branch from 24ea75b to 1257f22 Compare March 25, 2021 13:26

dqminh reviewed Mar 25, 2021

View reviewed changes

cyphar mentioned this pull request May 1, 2021

On a clean clone, Makefile.. doesn't #718

Closed

kolyshkin mentioned this pull request Oct 14, 2021

Migrate to go mod #727

Merged

tianon closed this in #727 Oct 19, 2021

thaJeztah deleted the go_modules branch October 20, 2021 14:11

		@@ -1,22 +0,0 @@
		Copyright (c) 2012 - 2013 Mat Ryer and Tyler Bunnell

migrate to go modules #716

migrate to go modules #716

Uh oh!

Conversation

thaJeztah commented Mar 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!