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.

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

Enable Go workspace #19314

Closed
wants to merge 21 commits into from
Closed

Enable Go workspace #19314

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
da1b47a
Enable Go workspace
ivanvc Dec 11, 2024
c0206ac
Add verify-go-workspace make target
ivanvc Dec 20, 2024
697abcf
Migrate verify-go-versions to use the Go workspace
ivanvc Dec 19, 2024
0a0d565
Migrate verify-lint to use the Go workspace
ivanvc Dec 11, 2024
0a94f9d
Migrate verify-gofmt make target to use the Go workspace
ivanvc Dec 11, 2024
f2a0ee4
Migrate verify-bom make target to use the Go workspace
ivanvc Dec 11, 2024
601ba5e
Migrate verify-goword make target to use the Go workspace
ivanvc Feb 1, 2025
0d2f05c
Migrate verify-license-header make target to use the Go workspace
ivanvc Dec 11, 2024
dad115f
Migrate verify-govet make target to use the Go workspace
ivanvc Dec 11, 2024
4d43c75
Migrate verify-mod-tidy make target to use the Go workspace
ivanvc Dec 12, 2024
ae4d6a3
Migrate verify-govet-shadow make target to use the Go workspace
ivanvc Dec 13, 2024
8b3f523
Migrate release scripts to use the Go workspace
ivanvc Dec 13, 2024
c342fc6
Migrate updatebom.sh to use the Go workspace
ivanvc Dec 17, 2024
0e05186
Migrate scripts/fix.sh to use the Go workspace
ivanvc Dec 18, 2024
73bb43f
Migrate run-govulncheck to use the Go workspace
ivanvc Dec 18, 2024
23ae621
Migrate build script to use the Go workspace
ivanvc Dec 18, 2024
d7ff06d
Migrate tests to use the Go workspace
ivanvc Dec 18, 2024
3be8aba
Migrate cov make target to use the Go workspace
ivanvc Dec 19, 2024
277ffd7
Migrate sync-toolchain-directive make target to use the Go workspace
ivanvc Dec 19, 2024
a639591
Migrate fix-yamllint to use the Go workspace
ivanvc Dec 22, 2024
9396f68
Migrate robustness makefile to use the Go workspace
ivanvc Feb 1, 2025
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
16 changes: 12 additions & 4 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,10 @@ fuzz:
verify: verify-gofmt verify-bom verify-lint verify-dep verify-shellcheck verify-goword \
verify-govet verify-license-header verify-mod-tidy \
verify-shellws verify-proto-annotations verify-genproto verify-yamllint \
verify-govet-shadow verify-markdown-marker verify-go-versions
verify-govet-shadow verify-markdown-marker verify-go-versions verify-go-workspace

.PHONY: fix
fix: fix-bom fix-lint fix-yamllint sync-toolchain-directive
fix: fix-bom fix-lint fix-yamllint sync-toolchain-directive update-go-workspace
./scripts/fix.sh

.PHONY: verify-gofmt
Expand Down Expand Up @@ -151,7 +151,7 @@ verify-govet-shadow:
verify-markdown-marker:
PASSES="markdown_marker" ./scripts/test.sh

YAMLFMT_VERSION = $(shell cd tools/mod && go list -m -f '{{.Version}}' github.com/google/yamlfmt)
YAMLFMT_VERSION = $(shell go list -m -f '{{.Version}}' github.com/google/yamlfmt)

.PHONY: fix-yamllint
fix-yamllint:
Expand All @@ -169,7 +169,7 @@ endif

# Tools

GOLANGCI_LINT_VERSION = $(shell cd tools/mod && go list -m -f {{.Version}} github.com/golangci/golangci-lint)
GOLANGCI_LINT_VERSION = $(shell go list -m -f {{.Version}} github.com/golangci/golangci-lint)
.PHONY: install-golangci-lint
install-golangci-lint:
ifeq (, $(shell which golangci-lint))
Expand Down Expand Up @@ -207,6 +207,14 @@ clean:
verify-go-versions:
./scripts/verify_go_versions.sh

.PHONY: verify-go-workspace
verify-go-workspace:
PASSES="go_workspace" ./scripts/test.sh

.PHONY: sync-toolchain-directive
sync-toolchain-directive:
./scripts/sync_go_toolchain_directive.sh

.PHONY: update-go-workspace
update-go-workspace:
./scripts/update_go_workspace.sh
10 changes: 5 additions & 5 deletions api/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ require (
// Bad imports are sometimes causing attempts to pull that code.
// This makes the error more explicit.
replace (
go.etcd.io/etcd => ./FORBIDDEN_DEPENDENCY
go.etcd.io/etcd/api/v3 => ./FORBIDDEN_DEPENDENCY
go.etcd.io/etcd/pkg/v3 => ./FORBIDDEN_DEPENDENCY
go.etcd.io/etcd/tests/v3 => ./FORBIDDEN_DEPENDENCY
go.etcd.io/etcd/v3 => ./FORBIDDEN_DEPENDENCY
go.etcd.io/etcd => ../FORBIDDEN_DEPENDENCY
go.etcd.io/etcd/api/v3 v3.0.0 => ../FORBIDDEN_DEPENDENCY
Copy link
Member

Choose a reason for hiding this comment

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

why v3.0.0?

Copy link
Member

Choose a reason for hiding this comment

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

Also I don't see a need to change ./FORBIDDEN_DEPENDENCY to ../FORBIDDEN_DEPENDENCY, although not very important.

Copy link
Member Author

Choose a reason for hiding this comment

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

why v3.0.0?

For the workspace to work, dependencies need to be consistent. The problem is that we have the same dependency used and also in a replacement for the forbidden dependencies, then go complains with the following error:

conflicting replacements found for go.etcd.io/etcd/api/v3@v3.6.0-alpha.0 in workspace modules defined by [...]/etcd/etcd/go.mod and [...]/etcd/etcd/api/go.mod

It could technically be anything, but if we set it to a version like 3.6.0, we would need to change that when we release the 3.6.0 version.

Also I don't see a need to change ./FORBIDDEN_DEPENDENCY to ../FORBIDDEN_DEPENDENCY, although not very important.

In my first attempt to enable the Go workspace, I couldn't figure out why the forbidden dependency trick wasn't working. I removed it and implemented how k/k does the forbidden dependencies. However, Tim pointed out that it works if you point this virtual dependency to the exact location across the different submodules. So, the top-level points them to ./FORBIDDEN_DEPENDENCY, for the first-level (i.e., ./api), it should point to ../FORBIDDEN_DEPENDENCY and so on.

Copy link
Member

Choose a reason for hiding this comment

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

I removed it and implemented how k/k does the forbidden dependencies.

How does K8s implement the forbidden dependencies? I don't see any forbidden directive in K8s's go.mod files at all.

It could technically be anything, but if we set it to a version like 3.6.0, we would need to change that when we release the 3.6.0 version.

Even if we use "v3.0.0", we also need to manually update it when we release new major release, i.e. v4.0.0, but it's less frequent.

Can we raise a question in golang community to ask for best practice?

Copy link
Member Author

Choose a reason for hiding this comment

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

How does K8s implement the forbidden dependencies? I don't see any forbidden directive in K8s's go.mod files at all.

They use an internal tool import-boss, which works with .import-restrictions files.

go.etcd.io/etcd/pkg/v3 v3.0.0 => ../FORBIDDEN_DEPENDENCY
go.etcd.io/etcd/tests/v3 v3.0.0 => ../FORBIDDEN_DEPENDENCY
go.etcd.io/etcd/v3 v3.0.0 => ../FORBIDDEN_DEPENDENCY
)
8 changes: 4 additions & 4 deletions client/internal/v2/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ replace (
// Bad imports are sometimes causing attempts to pull that code.
// This makes the error more explicit.
replace (
go.etcd.io/etcd => ./FORBIDDEN_DEPENDENCY
go.etcd.io/etcd/pkg/v3 => ./FORBIDDED_DEPENDENCY
go.etcd.io/etcd/tests/v3 => ./FORBIDDEN_DEPENDENCY
go.etcd.io/etcd/v3 => ./FORBIDDEN_DEPENDENCY
go.etcd.io/etcd => ../../../FORBIDDEN_DEPENDENCY
go.etcd.io/etcd/pkg/v3 v3.0.0 => ../../../FORBIDDEN_DEPENDENCY
go.etcd.io/etcd/tests/v3 v3.0.0 => ../../../FORBIDDEN_DEPENDENCY
go.etcd.io/etcd/v3 v3.0.0 => ../../../FORBIDDEN_DEPENDENCY
)
8 changes: 4 additions & 4 deletions client/v3/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ replace (
// Bad imports are sometimes causing attempts to pull that code.
// This makes the error more explicit.
replace (
go.etcd.io/etcd => ./FORBIDDEN_DEPENDENCY
go.etcd.io/etcd/pkg/v3 => ./FORBIDDEN_DEPENDENCY
go.etcd.io/etcd/v3 => ./FORBIDDEN_DEPENDENCY
go.etcd.io/tests/v3 => ./FORBIDDEN_DEPENDENCY
go.etcd.io/etcd => ../../FORBIDDEN_DEPENDENCY
go.etcd.io/etcd/pkg/v3 v3.0.0 => ../../FORBIDDEN_DEPENDENCY
go.etcd.io/etcd/v3 v3.0.0 => ../../FORBIDDEN_DEPENDENCY
go.etcd.io/tests/v3 v3.0.0 => ../../FORBIDDEN_DEPENDENCY
)
6 changes: 3 additions & 3 deletions etcdctl/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ replace (
// Bad imports are sometimes causing attempts to pull that code.
// This makes the error more explicit.
replace (
go.etcd.io/etcd => ./FORBIDDEN_DEPENDENCY
go.etcd.io/etcd/v3 => ./FORBIDDEN_DEPENDENCY
go.etcd.io/tests/v3 => ./FORBIDDEN_DEPENDENCY
go.etcd.io/etcd => ../FORBIDDEN_DEPENDENCY
go.etcd.io/etcd/v3 v3.0.0 => ../FORBIDDEN_DEPENDENCY
go.etcd.io/tests/v3 v3.0.0 => ../FORBIDDEN_DEPENDENCY
)
6 changes: 3 additions & 3 deletions etcdutl/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ replace (
// Bad imports are sometimes causing attempts to pull that code.
// This makes the error more explicit.
replace (
go.etcd.io/etcd => ./FORBIDDEN_DEPENDENCY
go.etcd.io/etcd/v3 => ./FORBIDDEN_DEPENDENCY
go.etcd.io/tests/v3 => ./FORBIDDEN_DEPENDENCY
go.etcd.io/etcd => ../FORBIDDEN_DEPENDENCY
go.etcd.io/etcd/v3 v3.0.0 => ../FORBIDDEN_DEPENDENCY
go.etcd.io/tests/v3 v3.0.0 => ../FORBIDDEN_DEPENDENCY
)

require (
Expand Down
21 changes: 21 additions & 0 deletions go.work
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// This is a generated file. Do not edit directly.

go 1.23

toolchain go1.23.5

use (
.
./api
./client/internal/v2
./client/pkg
./client/v3
./etcdctl
./etcdutl
./pkg
./server
./tests
./tools/mod
./tools/rw-heatmaps
./tools/testgrid-analysis
)
Loading