-
Notifications
You must be signed in to change notification settings - Fork 18k
cmd/go: provide a convenient way to iterate all packages in go.work workspace #50745
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
Comments
This is especially not possible in CI, as the |
how would you decide what to include if you don't have a |
I just would like to iterate over all sub modules, e.g., run all unit test in the active module and all sub modules. |
The Go project recommends single-module repositories over multi-module wherever possible [1]; multi-module setups seem to be most useful when they represent separately-versioned and released projects living within the same repository (further analysis in [2]). Additionally, there is currently no convenient way to test all modules in a 'go.work' file (see [3]). Due to the factors listed, convert repository from multi- to single-module. [1] https://github.com/golang/go/wiki/Modules#should-i-have-multiple-modules-in-a-single-repository [2] https://medium.com/compass-true-north/catching-up-with-the-world-go-modules-in-a-monorepo-c3d1393d6024 [3] golang/go#50745 Signed-off-by: Victoria Dye <vdye@github.com>
The Go project recommends single-module repositories over multi-module wherever possible [1]; multi-module setups seem to be most useful when they represent separately-versioned and released projects living within the same repository (further analysis in [2]). Additionally, there is currently no convenient way to test all modules in a 'go.work' file (see [3]). Due to the factors listed, convert repository from multi- to single-module. [1] https://github.com/golang/go/wiki/Modules#should-i-have-multiple-modules-in-a-single-repository [2] https://medium.com/compass-true-north/catching-up-with-the-world-go-modules-in-a-monorepo-c3d1393d6024 [3] golang/go#50745 Signed-off-by: Victoria Dye <vdye@github.com>
The Go project recommends single-module repositories over multi-module wherever possible [1]; multi-module setups seem to be most useful when they represent separately-versioned and released projects living within the same repository (further analysis in [2]). Additionally, there is currently no convenient way to test all modules in a 'go.work' file (see [3]). Due to the factors listed, convert repository from multi- to single-module. [1] https://github.com/golang/go/wiki/Modules#should-i-have-multiple-modules-in-a-single-repository [2] https://medium.com/compass-true-north/catching-up-with-the-world-go-modules-in-a-monorepo-c3d1393d6024 [3] golang/go#50745 Signed-off-by: Victoria Dye <vdye@github.com>
The Go project recommends single-module repositories over multi-module wherever possible [1]; multi-module setups seem to be most useful when they represent separately-versioned and released projects living within the same repository (further analysis in [2]). Additionally, there is currently no convenient way to test all modules in a 'go.work' file (see [3]). Due to the factors listed, convert repository from multi- to single-module. [1] https://github.com/golang/go/wiki/Modules#should-i-have-multiple-modules-in-a-single-repository [2] https://medium.com/compass-true-north/catching-up-with-the-world-go-modules-in-a-monorepo-c3d1393d6024 [3] golang/go#50745 Signed-off-by: Victoria Dye <vdye@github.com>
Having a I'm not familiar with the internals of go's community. Are they accepting of proposals and PRs? |
@eighty4 If you have go list -f '{{.Dir}}' -m | xargs You could then do something like
Not perfect, but works. It's much more pain if you don't check in |
Sorry you had to repeat the answer for me. Works pretty well:
|
I don't see how I could achieve the same one liner for # workspace dir
go mod tidy
# cd for each module
cd composable
go mod tidy
cd ../git
go mod tidy
cd ../testutil
go mod tidy
cd ../util
go mod tidy |
Reusing Would |
Yes, I think this would be too subtle. :( But I also lack a better idea. Maybe a flag, to make commands workspace aware, in the same way |
I don't know what the |
What's an example of a module aware command with |
4 dots is interesting idea--maybe a little more obvious would be An issue with these shell scripts is that you don't get an aggregated coverage report. I am curious about the framing of the issue here: Is this a gap in workspace, or a gap in go test? My understanding is that workspace is an optional, local convenience. The ability to run all tests incl submodules, I think, should work whether or not an individual chooses to use workspaces feature. Is that a fair understanding? |
I agree with that. Notice that this is a general issue with go tooling, not only for running tests. I'd also like to |
With Go 1.20, the new
This also works for
|
@seankhliao, is there agreement from the maintainers that
If there is agreement, then what are next steps? |
bump @bcmills, any advice on how to advance a solution? |
I think it works great this way. Workspaces should evolve to simplify the go mod tidy upkeep across all modules of a workspace with a command that performs mod tidy in each workspace module, with each tidy being its own independent operation. The statement proposal's wording feels like a hang up. |
@katexochen, the following one-liners appear to work:
|
Should it be
or
? |
@eighty4 Here's my do.sh helper script. |
**What problem does this PR solve?**: When GOWORK is enabled, "go list -m" always lists all modules in the workspace. To list the module for the current working directory, we disable GOWORK. This is sound whether or not you use Go workspaces. **Before** ```shell > pwd /capi-runtime-extensions/api > go env GOWORK /capi-runtime-extensions/go.work > go list -m github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/docs github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/external/caaph github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/external/capa github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/external/capx ``` **After** ```shell > pwd /capi-runtime-extensions/api > GOWORK=off go list -m github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api ``` **Which issue(s) this PR fixes**: Fixes # **How Has This Been Tested?**: <!-- Please describe the tests that you ran to verify your changes. Provide output from the tests and any manual steps needed to replicate the tests. --> **Special notes for your reviewer**: <!-- Use this to provide any additional information to the reviewers. This may include: - Best way to review the PR. - Where the author wants the most review attention on. - etc. --> It seems like `go list` is working as designed. Here's a vscode-go maintainer [recommending](golang/go#50745 (comment)) `GOWORK=off` when "module separation" is needed.
I think what we'd want here is not an exactt equivalent to We use It's not clear what to call such a pattern though. |
I think we should call the pattern |
Filed #71294 |
Given the change in #71294 is now implemented, we can't release Go 1.25 without having it also be documented and mentioned in release notes. Since that issue is already closed but this one's still open, marking it as a release blocker. |
Change https://go.dev/cl/658495 mentions this issue: |
Hi @matloob Thanks for working on this. I found this issues a few week ago,because I had a need for something like this. I'm happy this issue is now addressed with the PR you made. I tried to follow the implementation, and also looked at the changes that was made to the documentation. The Go 1.25 is not yet release, but I'm curious. I might have missed something, so please be lenient if I misunderstood something. So let me sum up:
So the idea was to bring a way to test (build, mod clean... whatsoever...) all modules from the go.work file I hope it's pretty close to the idea that was debated here What is unclear to me is about the new way to work with the changes that were introduced By reading some messages, we may understand that launching "go test ./.." will magically launch the tests for all submodule listed in the go.work file. I think it might be great, but then it would comes with changes that could have consequences. Someone trying to test a root modules might test all submodule, but some submodule may require different environment like a docker instance. So it would be a possible breaking changes. That's why I looked at the code. From what I saw (especially in the unit tests), I would say that what was added is about having a new argument value for go list So now, unless I'm wrong But the whole point was not to list them, so here I assume the code changes are about adding something equivalent to Could you confirm ? TL;DR; how to use the changes that were added to test all submodules ? Thanks |
The There is no one single command to test all submodules of a workspace. |
Thank for your reply.
Now, it's clearer. Thanks |
go.work
makes it easier to work with multiple modules.Often users want to run tests or analysis for all packages in their workspace. For a single module case, this can be easily achieved by using
./...
pattern. Currently, however, there is no equivalent of./...
in workspace mode. So users need to run go commands from inside of each individual module, or supply the paths for each module directories listed ingo.work
. (e.g.go test -v $(go list -f '{{.Dir}}/...' -m | xargs)
).We need to figure out how to offer
./...
-equivalent behavior when working with `go.work.My personal preference is to reuse
./...
(VSCode Go uses./...
pattern to implement workspace-wide test, lint, coverage computation, etc).p.s. I noticed the issue about clarifying the meaning of
...
in module mode is still open #37227The text was updated successfully, but these errors were encountered: