golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.24k stars 17.57k forks source link

cmd/go: provide a convenient way to iterate all packages in go.work workspace #50745

Open hyangah opened 2 years ago

hyangah commented 2 years ago

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 in go.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 https://github.com/golang/go/issues/37227

katexochen commented 2 years ago

go test -v $(go list -f '{{.Dir}}/...' -m | xargs)

This is especially not possible in CI, as the go.work file shouldn't be checked in (according to the proposal). To have a consistent solution, it would be great to solve this problem for modules in general, not only for repositories with a go.work file.

seankhliao commented 2 years ago

how would you decide what to include if you don't have a go.work file?

katexochen commented 2 years ago

how would you decide what to include if you don't have a go.work file?

I just would like to iterate over all sub modules, e.g., run all unit test in the active module and all sub modules.

eighty4 commented 1 year ago

Having a go.work file for local workflow setup is a use case, but I am setting up a project that I definitely plan on adding my go.work file to my committed sources. There's not much value add, other than enabling a fork to engage in development without having to recreate an identical go.work file, but that's only because the workspace feature doesn't have workflows baked into the tooling. It seems like a huge miss to add workspaces and not have a way to run go test across all the workspace modules or to be able to run go get for a module of a workspace without using cd.

I'm not familiar with the internals of go's community. Are they accepting of proposals and PRs?

katexochen commented 1 year ago

@eighty4 If you have go.work checked in, you can easily list all modules with

go list -f '{{.Dir}}' -m | xargs

You could then do something like

mods=$(go list -f '{{.Dir}}' -m | xargs)
for mod in $mods; do
            (cd "$mod"; go test ./...)
done

Not perfect, but works. It's much more pain if you don't check in go.work (as recommended by the documentation) and want to do such thing...

eighty4 commented 1 year ago

Sorry you had to repeat the answer for me. Works pretty well:

go list -f '{{.Dir}}' -m | xargs go test

eighty4 commented 1 year ago

I don't see how I could achieve the same one liner for go mod tidy. Is it possible to achieve this with an xargs/pipe:

# 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
seankhliao commented 1 year ago

Reusing ./... for workspaces would clash if you had a module in the workspace root (use .).

Would ./.... (4 dots) be too subtle?

katexochen commented 1 year ago

Would ./.... (4 dots) be too subtle?

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 -m makes them module aware?

eighty4 commented 1 year ago

I don't know what the ./... and go test [packages] do or are for. I could only get pattern matching to work with go test such as go test my_test.go. Do you have to specify go test github.com/fullyqualified/modulepath to test a package? ./... seemed to have no effect.

eighty4 commented 1 year ago

What's an example of a module aware command with -m?

cyrusv commented 1 year ago

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?

katexochen commented 1 year ago

The ability to run all tests incl submodules, I think, should work whether or not an individual chooses to use workspaces feature

I agree with that. Notice that this is a general issue with go tooling, not only for running tests. I'd also like to go tidy all submodules etc.

katexochen commented 1 year ago

mods=$(go list -f '{{.Dir}}' -m | xargs) for mod in $mods; do (cd "$mod"; go test ./...) done

With Go 1.20, the new -C flag can be used to simplify this to

mods=$(go list -f '{{.Dir}}' -m)
for mod in $mods; do
            go test -C "$mod" ./...
done

This also works for go tidy @eighty4 :

mods=$(go list -f '{{.Dir}}' -m)
for mod in $mods; do
            go mod tidy -C "$mod"
done
cyrusv commented 1 year ago

@seankhliao, is there agreement from the maintainers that

If there is agreement, then what are next steps? Do we need a formal proposal for what the syntax should be for the fix, then a PR to implement? I'm happy to take action if the maintainers give some guidance -- I have not yet contributed to golang before!

cyrusv commented 1 year ago

bump @bcmills, any advice on how to advance a solution?

seankhliao commented 1 year ago

My opinion would be that no pattern should cross between unrelated module dependency graphs, which can produce confusing / conflicting information on dependency resolution.

hyangah commented 1 year ago

IMHO the key idea of go.work is to make the selected modules "related" and the go command runs the version selection algorithm on the combined modules and already builds a very different dependency graph. If users want to preserve the separation, they can do GOWORK=off go test ./... from their desired directory.

cyrusv commented 1 year ago

@hyangah, your proposal seems sensible and consistent to me. In that case, it sounds like your are proposing to evolve go.work to be strongly recommended tool for structuring a multi-module project, and checked into version control to provide consistency. Is that a fair understanding? This way, @seankhliao's preference to keep modules separate is held. @hyangah, how do you propose to meet this requirement from the Workspaces proposal?

go.work files should not be checked into version control repos containing modules so that the go.work file in a module does not end up overriding the configuration a user created themselves outside of the module. The go.work documentation should contain clear warnings about this.

However, if we are unwilling to formalize Workspaces to be strongly recommended for multi-module projects, then I'm not sure it makes sense.

eighty4 commented 1 year ago

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.

samherrmann commented 1 year ago

@katexochen, the following one-liners appear to work:

eighty4 commented 1 year ago

Should it be

go work sync
go list -f '{{.Dir}}' -m | xargs -L1 go mod tidy -C
go work sync
go list -f '{{.Dir}}' -m | xargs -L1 go mod tidy -C

or

go list -f '{{.Dir}}' -m | xargs -L1 go mod tidy -C
go work sync
go list -f '{{.Dir}}' -m | xargs -L1 go mod tidy -C

?

yuriy-yarosh commented 1 year ago

@eighty4 go mod tidy then go work sync so go work could pick up the respective changes from workspace module dependencies.

Here's my do.sh helper script.