Skip to content

vendor: eliminate some of the replace directives #3048

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.

Sign up for GitHub

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

Closed
errordeveloper wants to merge 4 commits into from
Closed

vendor: eliminate some of the replace directives #3048

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1646833
vendor: fix issue with permissions
errordeveloper Dec 15, 2021
4ee4f47
vendor: fix dependency path resolution in `make go-mod-vendor`
errordeveloper Dec 15, 2021
25e88b5
vendor: eliminate some of the `replace` directives
errordeveloper Dec 14, 2021
4b579ac
misc: add `.gitattributes`
errordeveloper Dec 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitattributes
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
vendor.mod linguist-language=Go-Module
vendor.sum linguist-language=Go-Checksums
6 changes: 4 additions & 2 deletions direct.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,12 @@ dep-validate: go-mod-vendor
go.mod:
@printf "module github.com/docker/swarmkit\n\ngo 1.17\n" > $@

GOGO_PROTOBUF_PATH=$(shell go env GOPATH)/pkg/mod/github.com/gogo/protobuf@$(shell go list -modfile vendor.mod -m -f '{{.Version}}' github.com/gogo/protobuf)/protobuf

.PHONY: go-mod-vendor
go-mod-vendor: go.mod
@go mod vendor -modfile vendor.mod
@# ensure that key protobuf spec files are in vendor dir
@cp -a $(GOGO_PROTOBUF_PATH) vendor/github.com/gogo/protobuf/
@module=github.com/gogo/protobuf ; \
prefix=$$(go env GOPATH)/pkg/mod/$${module} ; \
version=$$(go list -modfile vendor.mod -m -f '{{.Version}}' $${module}) ; \
cp -a $${prefix}@$${version}/protobuf vendor/$${module}/ && chmod -R u+w vendor/$${module}
30 changes: 4 additions & 26 deletions vendor.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,18 +56,19 @@ require (
github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/docker/libkv v0.2.2-0.20180912205406-458977154600 // indirect
github.com/fsnotify/fsnotify v1.4.9 // indirect
github.com/google/certificate-transparency-go v1.0.20 // indirect
github.com/google/uuid v1.2.0 // indirect
github.com/gorilla/mux v1.8.0 // indirect
github.com/hashicorp/go-immutable-radix v1.0.0 // indirect
github.com/hashicorp/golang-lru v0.5.3 // indirect
github.com/hashicorp/uuid v0.0.0-20160311170451-ebb0a03e909c // indirect
github.com/hpcloud/tail v1.0.0 // indirect
github.com/inconshreveable/mousetrap v1.0.0 // indirect
github.com/ishidawataru/sctp v0.0.0-20210226210310-f2269e66cdee // indirect
github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369 // indirect
github.com/moby/term v0.0.0-20210619224110-3f7ff695adc6 // indirect
github.com/morikuni/aec v1.0.0 // indirect
github.com/nxadm/tail v1.4.4 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/prometheus/client_model v0.2.0 // indirect
github.com/prometheus/common v0.10.0 // indirect
Expand All @@ -78,37 +79,20 @@ require (
go.uber.org/zap v1.10.0 // indirect
golang.org/x/sys v0.0.0-20210823070655-63515b42dcdf // indirect
golang.org/x/text v0.3.4 // indirect
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
google.golang.org/genproto v0.0.0-20201110150050-8816d57aaa9a // indirect
gopkg.in/fsnotify.v1 v1.4.7 // indirect
gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c // indirect
Copy link
Member

@thaJeztah thaJeztah Jan 20, 2022

Choose a reason for hiding this comment

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

Was wondering why this didn't use a tagged release, but looks like no v3 releases exist yet; https://github.com/go-yaml/yaml/tags

Copy link
Contributor Author

@errordeveloper errordeveloper Jan 20, 2022

Choose a reason for hiding this comment

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

eh... also gopkg.in does some magic, which I never had the time to try and understand, very few libs use it anyway.

sigs.k8s.io/yaml v1.3.0 // indirect
)

replace (
github.com/Microsoft/go-winio => github.com/Microsoft/go-winio v0.4.15
github.com/akutz/gosync => github.com/akutz/gosync v0.1.0
github.com/containerd/containerd => github.com/containerd/containerd v1.5.2
Copy link
Member

@thaJeztah thaJeztah Jan 20, 2022

Choose a reason for hiding this comment

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

Was this replace still needed? Wondering if we can just update the indirect dependency to the latest v1.5.x release

I should note that if we need to keep some of the replace rules, then it might be good to add a comment in the corresponding require section, and to add a comment that "the actual version is overridden in the replace section", similar to https://github.com/moby/buildkit/blob/390c6886f4a310af5ee0d45eaab35dc36b21f23e/go.mod#L22-L23

It's easy to overlook that there's a replace rule for a dependency, and people may then forget to update (or remove) them when updating dependencies.

Copy link
Contributor Author

@errordeveloper errordeveloper Jan 20, 2022

Choose a reason for hiding this comment

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

As the time of the PR states, the goal is to remove only some of the replace directives, i.e. seemingly trivial version bumps that won't affect downstream in any way. So runc and containerd are untouched for the purpose of this PR, as I cannot assume those are trivial and won't affect Moby.

github.com/coreos/go-semver => github.com/coreos/go-semver v0.2.0
github.com/docker/distribution => github.com/docker/distribution v2.7.1-0.20190205005809-0d3efadf0154+incompatible
github.com/golang/protobuf => github.com/golang/protobuf v1.3.5
github.com/google/uuid => github.com/google/uuid v1.0.0
github.com/hashicorp/go-immutable-radix => github.com/tonistiigi/go-immutable-radix v0.0.0-20170803185627-826af9ccf0fe
github.com/hpcloud/tail => github.com/hpcloud/tail v1.0.0
github.com/matttproud/golang_protobuf_extensions => github.com/matttproud/golang_protobuf_extensions v1.0.1
github.com/onsi/ginkgo => github.com/onsi/ginkgo v1.8.0
github.com/onsi/gomega => github.com/onsi/gomega v1.5.0
github.com/opencontainers/runc => github.com/opencontainers/runc v1.0.0-rc95
Copy link
Member

@thaJeztah thaJeztah Jan 20, 2022

Choose a reason for hiding this comment

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

Same for this one; if we update containerd, this may not be needed (I see containerd 1.5.x now uses run v1.0.2); https://github.com/containerd/containerd/blob/v1.5.9/go.mod#L43

github.com/prometheus/client_golang => github.com/prometheus/client_golang v1.6.0
github.com/prometheus/common => github.com/prometheus/common v0.9.1
github.com/prometheus/procfs => github.com/prometheus/procfs v0.0.11
github.com/stretchr/testify => github.com/stretchr/testify v1.3.0
golang.org/x/crypto => golang.org/x/crypto v0.0.0-20201117144127-c1f2f97bffc9
golang.org/x/net => golang.org/x/net v0.0.0-20201224014010-6772e930b67b
golang.org/x/text => golang.org/x/text v0.3.3
golang.org/x/time => golang.org/x/time v0.0.0-20191024005414-555d28b269f0
google.golang.org/genproto => google.golang.org/genproto v0.0.0-20200227132054-3f1135a288c9
gopkg.in/fsnotify.v1 => gopkg.in/fsnotify.v1 v1.4.7
)

// NOTE(dperny,cyli): there is some error handling, found in the
Expand All @@ -120,9 +104,3 @@ replace (
// After updating GRPC, if integration test failures occur, verify that the
// string matching there is correct.
replace google.golang.org/grpc => google.golang.org/grpc v1.23.0
Copy link
Member

@thaJeztah thaJeztah Jan 20, 2022

Choose a reason for hiding this comment

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

Do we need to / should we copy the replace rules from the containerd version we're using? Or does it break with v1.27.1 ? There's some other related replace in the containerd go.mod as well; https://github.com/containerd/containerd/blob/v1.5.9/go.mod#L75-L80

	github.com/gogo/googleapis => github.com/gogo/googleapis v1.3.2
	github.com/golang/protobuf => github.com/golang/protobuf v1.3.5
	// urfave/cli must be <= v1.22.1 due to a regression: https://github.com/urfave/cli/issues/1092
	github.com/urfave/cli => github.com/urfave/cli v1.22.1
	google.golang.org/genproto => google.golang.org/genproto v0.0.0-20200224152610-e50cd9704f63
	google.golang.org/grpc => google.golang.org/grpc v1.27.1

Copy link
Contributor Author

@errordeveloper errordeveloper Jan 20, 2022

Choose a reason for hiding this comment

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

This is another non-trivial one, so I'd leave it for another PR :)

Copy link
Member

@crazy-max crazy-max Jan 20, 2022

Choose a reason for hiding this comment

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

Yeah this is the hard one that blocks buildkit vendoring on moby.

Copy link
Member

@thaJeztah thaJeztah Jan 20, 2022

Choose a reason for hiding this comment

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

Yeah, we can do in a follow-up. I was looking at the moby code, which already included those replace rules (linking to the vendor.conf file for convenience) and versions of containerd/runc

For these I was more concerned about not having replace rules for some of them, but CI is green so at least currently it works 😂


// go-systemd v17 is required by github.com/coreos/pkg/capnslog/journald_formatter.go
replace (
github.com/coreos/go-systemd => github.com/coreos/go-systemd v0.0.0-20180511133405-39ca1b05acc7
github.com/coreos/pkg => github.com/coreos/pkg v0.0.0-20180108230652-97fdf19511ea
)
Loading