-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
cmd/go: add a workspace mode #45713
Comments
Thanks for working on this, Michael! It's clear that many people want a solution to multi-module development, given all the activity in various issues over the last couple of years. For those who just arrived here, note that this was discussed over the previous week on the golang-tools list, too. Some points raised there resulted in edits to the proposal draft. Overall, I really like the idea. Below are some thoughts. As a summary, I think the proposal is trying to fix a few too many issues at once.
I still wonder why we need If the answer is "it's too expensive to find all modules every time", I'd love to see some proof of that. I'd also like to see if caching could be a solution, similar to
We touched on this in the email list, and you added this section, so as a follow-up - I'm still not convinced that the first version of And if I want to build or test one of the modules, I'd want to
I don't think that would be a good idea, to be honest. I'm starting to believe that build configurations (e.g. what build tags to use in A "build config" also has a purpose, at least when proposed in #39005: for the original author to announce to the users what are the supported ways to build their packages. Without that information, one can only assume that all build tag and GOOS/GOARCH combinations are possible, which quickly explodes exponentially. Since most users would not publish All in all, I don't think this proposal should ever aim to solve the "build config" problem.
This one is an interesting thought. This only works if we declare that For example, if we're OK with monorepos publishing |
Note: comments are related to the proposal document. Please take all "must", "should", "*-not" placed below as prepended with a humble IMO.)
Workspace mode should NOT be turned on without a human demanding so. Proposed defaults with "auto" and go.work presence make for traps if there is go.work present in vcs but we want to build/test in module mode — we may forgot to add a flag "workspace=off" using tools by hand. Or there was an uncommited go.work we forgot to copy, or we checked out older revision, then we think we continue within workspace while we now do not. Former trap might bite the most when we'd set to work deep into the tree but go.work still hangs two floors above us: we might think we're testing this module in module mode (oh, no go.work here) while we are testing this module with dependencies replaced by an active go.work directive. Make intent explicit either by mandating option to always be present. or by environment.
The go.work file should not take absolute path in the directory directive — only paths relative to the place of the go.work file. Otherwise having go.work commited to the repository freezes filesystem layout at a go.work mandated state, as "no restriction on where the directory is located" is proposed too. ("No restriction, anywhere" in practice says that after a longer while of hiatus user might be missing pieces of her workspace; with no friendly
To my understanding, the ultimate goal of "workspace" is to produce versioned code for reproductible builds, hence at least the unversioned replacements should be disallowed in go.mods of modules we currently work on in the "workspace". Allowing for two or more sets of replacements overlayed would be a costly mistake. IOW: either we commit to progress in work with set of go.mod replacements (current state), or we commit to work using workspaces and put all our replacements in go.work. Tools should guard us from the mix. In my imagination, the "workspace" workflow (ie. workspace=on) should be a simple loop:
After 4 the CI pipeline should use the same code we tagged at 3! If code to be built with workspace "=on" and "=off" does not converge at 4, we've lost the track of what we're shipping. Tools should somehow prevent that. A bare minimum is to disallow unversioned replacements in go.mods of modules pulled into workspace. The real shield would be to move all replacements from |
@mvdan, Thanks for the feedback! A few replies:
I'm not convinced that the right behavior is to add all the modules under the directory, even if it were free: if we added all the modules under the directory, then it would be much harder (I think impossible?) to build a workspace that includes a module, but not any of the modules contained in subdirectories. So if we added all the modules in a directory we'd have to add another mechanism to remove modules from the workspace. And if we're going to do that it seems to me that the default should be to add a module at a time, and maybe consider if we want to do a directory tree in the future, if it turns out to be necessary. I think the name of the
I'm wondering what we'd get by disabling builds outside any of the modules? It's something that essentially comes for 'free' from the rest of the design because the
I think that the workspace file could be useful to allow the developer to specify configuration to tooling, because it is developer specific configuration, but this proposal does not intend to solve the build config problem. It's definitely out of scope for the design. If it's too distracting for the conversation I can remove the reference from the related issues section in the doc.
The doc does recommend that "most go.work files should exist outside of any repository" and I do think that If a monorepo publishes a |
@ohir, thanks for the comments!
I definitely understand the issue about users not realizing which workspace they're in and accidentally ending up with the wrong build. I think this is most likely to happen if the workspace file is checked into a repository. (Otherwise, the user created the workspace file themselves, and opted-in to workspace mode). Would you agree with that? I think in general checking in a workspace file to a repository should be rare: because it is really only useful for multi-module repositories which themselves are not the common case, and because it will affect the build configuration for every developer of a module, it's something that should only be done if it fits into the workflow of most developers.
Do you think this would be a problem for go.work files that are not checked into a repository? Of course, I agree that absolute paths should not show up in a go.work file that's checked into a repository (for that matter, there shouldn't be any relative paths that point outside the repository). If such a change was committed, I expect that it would be quickly reverted because it likely wouldn't work for any other developers than the one who originally committed it! Here is a case for absolute paths: some editors might want to make temporary workspace files because a user just opened two files in different modules. In that case the workspace file would be put in a temporary directory, and it would make more sense to add absolute paths rather than trying to construct a relative path from the temporary directory to the location of the module. And on Windows, I believe it's not possible to construct relative paths across volumes, so the editor would have to attempt to make a temporary directory on the same volume, something that might not be easy to do. I wonder if this is something an outside tool can catch, like a sort of workspace
Oh! I think the doc must have not been as clear: the ultimate goal is not to produced versioned code for reproducible builds, but to assist in workflows on multiple modules. To get the reproducible build you still need a single Because replacements are made for developing modules, I think users would be surprised to not have them in workspaces. I think it's a bit unfortunate that they need to be supported in workspaces, but it's hard for me to see a better solution.
I think having replaces in multiple places is a good idea, but (unless I'm misunderstanding) this seems to encourage users to check in |
I see. I can't say I can think of any scenario where I'd need this, but I've also avoided nested Go modules at any cost.
How about
I'm looking at this from the other side; why should we support this edge case right away if we don't have a strong use case for it? Doing builds inside the workspace but outside any module is something we could add at a later time if users really ask for it, but we wouldn't be able to take it back if we later find out it's confusing or unnecessary.
I'd remove that section, personally. Nowadays I tend to think that build config shouldn't need to be attached at the module level, and the same applies to the workspace level.
I think making that recommendation clearer is a good idea, as it's probably one of the first questions users will have.
Hmm, I find that recommendation for users to be a tad weird. We're introducing nested workspaces and builds outside any module, all for the sake of being able to satisfy the use case of local-only replace directives. It feels like a bit of a stretch :) |
Does this imply that every I’ve worked on systems with slow network filesystems where that would be expensive. |
I really don't think of this as an edge case, it's just a simple way to determine how to set the build list: "Prefer workspace mode (ie search for a go.work), if found use its build list. If not found, use module mode (ie search for a go.mod), and use its build list". I think we'd need to have a stronger reason to add the extra case of making an extra rule to disable it, for example cases that it could become confusing for.
Got it. I'll start by make it it much more clear that we have no intention of addressing that use case in this proposal or extending
Done, in the latest revision, and strengthened it to advocate against checking in
Thinking about this much more in light of your comments and @ohir's I think recommending |
Caveat: I've skimmed the proposal but haven't really tried to understand all the nuances here, so this may well be too simplistic. My suggestion would be to try really, really hard to do this without introducing the Here module workspace
go 1.18
require (
./baz foo.org/bar/baz
./tools golang.org/x/tools
) If the module github.com/relab/gorums
go 1.18
require (
./baz foo.org/bar/baz
./tools golang.org/x/tools
google.golang.org/grpc v1.36.1
google.golang.org/protobuf v1.26.0
) As I mentioned at the top, I don't know if this would work for all scenarios, but I would strongly encourage finding a simple design. Obviously, I understand adding another field to the |
I want to understand this a bit better: my assumption for these cases are that most builds (or lists other go command operations that need the info in a |
I've been on systems where stats on files under Perhaps this isn't a case that matters any more these days; multiuser systems are certainly out of vogue. |
@meling I'd like to understand better why you'd like to avoid introducing the I don't totally understand the second example: what does it mean when you have both types of requires in a 'regular' module? |
Intuitively, that's now how I thought about it. Should workspace mode really kick in if there's a If you think about go.work as a replacement to GOPATH, then it seems reasonable to want to use workspace mode outside any module. But if you think of it as a way to develop multiple modules at once, then it still doesn't make sense to me. From my point of view, the special case is to allow this extra mode of operation, not to disallow it :)
Just thinking outloud: this does make the story for monorepos worse, because in those cases it can be easier for developers to share a go.work file by committing it at the root of the VCS repository. It seems like there might be intended use cases here which are at odds with each other. |
Maybe there could be an env var that let's you specify a list of directories and the go command treats those directories as the root when performing searches up the directory tree for go.work or go.mod or anything else that comes up in the future. It seems like a general setting and might deserve its own thread, even if this proposal is what would cause it to be needed more than it is now. |
If there are no More generally: the rules are similar to what
It's true that multiple modules can still be developed at once even if we add the restriction, so that it doesn't solely address the problem of developing multiple modules at once. But it's a smaller set of rules for a user to remember (when I'm under this go.work file, rather than, when I'm under this go.work file, and contained in one if its modules). I think a lot of users would think of it as an arbitrary restriction if they got the error message saying that it's not supported outside of a module's directory. I'm interested in what cases this could lead to user confusion or other errors that we should be worried about.
One future option could be to make an external tool that produces a |
Yeah I think that would be a reasonable option if that ends up being a problem, though I hope that this proposal isn't the reason why we would need it. |
I'm somewhat confused 😄 . Because it was my understanding that the work flow of being able to check in a I might well be alone, but I've somewhat lost track of the original use cases driving this proposal, and therefore the extent to which this proposal satisfies those problems/concerns. In particular, the UX before and after, who is expected to create |
To confirm, this is is the workflow of someone wanting to work across modules in a multi module workspace, right? If so, the proposal definitely aims to address this proposal. We could create a tool to create go.work from the modules recursively contained in a directory. Like a more robust version of this: #!/bin/sh
find $1 | grep "\.mod$" | awk '
BEGIN { print "go 1.17\n\ndirectory (" }
!/testdata/ { printf "\t./%s\n",substr($1,0,length($1)-length("/go.mod")) }
END { print ")" }
' Then someone working in a module like tools would run |
Indeed, but that impacts the overall UX of the solution. I'm not arguing for/against such a tool, but to my mind the ultimate workflow needs to be clear to anyone reading this proposal. Not least because this is (I think?) the first mention of such a tool, and I assume it's beyond the scope of this proposal?
I think this is a question for those folks who have to deal with this workflow. Is the fact that every developer now has to create a |
I prefer fewer file types in my folders, and having to examine two different file types for dependencies and replace directives, etc seems unnecessary, when there is already a file type for the Go module system. As for the "magic" workspace module workspace
go 1.18
workspace (
./baz foo.org/bar/baz
./tools golang.org/x/tools
) Obviously, the In fact, an alternative format could reuse the module workspace
go 1.18
replace (
foo.org/bar/baz => ./baz
golang.org/x/tools => ./tools
) PS: I'm assuming that a
My idea was that entries with path names could serve the same role as |
@myitcv Got it- I've updated the doc to mention that an external tool can potentially help with the creation of go.work files for multi module repos. @meling A |
This proposal has been added to the active column of the proposals project |
@matloob thanks for fleshing out the proposal.
Wondering if this might be a typo, I didn't find clarification in the other comments -- did you mean "override any replaces in the main modules' |
@rhcarvalho Ooh yeah it's absolutely a typo! Edited the comment to fix! Thanks! |
Change https://golang.org/cl/379655 mentions this issue: |
@matloob This is in the 1.18 milestone. Is there anything else to do here for 1.18? Thanks. |
Change https://golang.org/cl/381779 mentions this issue: |
I guess Line 1483 in b37c6e1
Use rather than Directory ?
|
Yes. |
For #45713 For #47694 Change-Id: I6f615c07749fca49c19f2ae22f79971c29aa8183 Reviewed-on: https://go-review.googlesource.com/c/go/+/381779 Trust: Ian Lance Taylor <iant@golang.org> Reviewed-by: Michael Matloob <matloob@golang.org>
Change https://golang.org/cl/381874 mentions this issue: |
For #45713 Change-Id: Ia55a96702b99cccaf5d96c2125ee513700658444 Reviewed-on: https://go-review.googlesource.com/c/go/+/381874 Trust: Ian Lance Taylor <iant@golang.org> Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Matloob <matloob@golang.org> Trust: Michael Matloob <matloob@golang.org>
Add entries for go.work files and the new go work commands to golang.org/ref/mod. For golang/go#45713 Change-Id: I1ad3335f977fad864d75790e31e48ee34e531ff5 Reviewed-on: https://go-review.googlesource.com/c/website/+/379655 Reviewed-by: Bryan Mills <bcmills@google.com> Trust: Michael Matloob <michaelmatloob@gmail.com>
There are other related bugs, but all implementation for this issue has been done. I'm going to close this and let the other issues track the reported bugs. |
Change https://go.dev/cl/385995 mentions this issue: |
This change removes the -workfile flag and allows the go.work file path to be set using GOWORK (which was previously read-only). This removes the potential discrepancy and confusion between the flag and environment variable. GOWORK will still return the actual path of the go.work file found if it is set to '' or 'auto'. GOWORK will return 'off' if it is set to 'off'. For #45713 Fixes #51171 Change-Id: I72eed65d47c63c81433f2b54158d514daeaa1ab3 Reviewed-on: https://go-review.googlesource.com/c/go/+/385995 Trust: Michael Matloob <matloob@golang.org> Run-TryBot: Michael Matloob <matloob@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>
Change https://go.dev/cl/389297 mentions this issue: |
For #45713 Change-Id: Ia2901cbfc5deb52503e74fcf9dff26a56ec582c3 Reviewed-on: https://go-review.googlesource.com/c/go/+/389297 Trust: Michael Matloob <matloob@golang.org> Run-TryBot: Michael Matloob <matloob@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
Change https://go.dev/cl/389914 mentions this issue: |
I'm wondering if workflows will support getting a module from different branch? Currently, in a multi-module repo, I've to tag the version with a path to go.mod file (e.g. git tag client_libraries/go/eventbus/v1.0.0) |
… tutorial to go help work For #45713 Change-Id: Ia2901cbfc5deb52503e74fcf9dff26a56ec582c3 Reviewed-on: https://go-review.googlesource.com/c/go/+/389297 Trust: Michael Matloob <matloob@golang.org> Run-TryBot: Michael Matloob <matloob@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> (cherry picked from commit 1eb1f62) Reviewed-on: https://go-review.googlesource.com/c/go/+/389914 Trust: Dmitri Shuralyov <dmitshur@golang.org> Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Change https://go.dev/cl/393360 mentions this issue: |
The -workfile flag's function has been absorbed by the GOWORK environment variable. Update the workspaces reference to reflect that. Also fix an error in the workspaces tutorial. For golang/go#45713 Change-Id: Id8dc8bb8dc6458d03d7b947ec78d84263d9689e1 Reviewed-on: https://go-review.googlesource.com/c/website/+/393360 Trust: Michael Matloob <matloob@golang.org> Run-TryBot: Michael Matloob <matloob@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Beth Brown <ecbrown@google.com>
Add entries for go.work files and the new go work commands to golang.org/ref/mod. For golang/go#45713 Change-Id: I1ad3335f977fad864d75790e31e48ee34e531ff5 Reviewed-on: https://go-review.googlesource.com/c/website/+/379655 Reviewed-by: Bryan Mills <bcmills@google.com> Trust: Michael Matloob <michaelmatloob@gmail.com>
The -workfile flag's function has been absorbed by the GOWORK environment variable. Update the workspaces reference to reflect that. Also fix an error in the workspaces tutorial. For golang/go#45713 Change-Id: Id8dc8bb8dc6458d03d7b947ec78d84263d9689e1 Reviewed-on: https://go-review.googlesource.com/c/website/+/393360 Trust: Michael Matloob <matloob@golang.org> Run-TryBot: Michael Matloob <matloob@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Beth Brown <ecbrown@google.com>
Detailed Design Doc: Proposal: Multi-Module Workspaces in
cmd/go
Demo Video: Go Workspaces Proposal Demo
High level overview:
I propose adding a new workspace mode in the
go
command for editing multiple modules. The presence of ago.work
file in the working directory or a containing directory will put thego
command into workspace mode. Thego.work
file specifies a set of local modules that comprise a workspace. When invoked in workspace mode, thego
command will always select these modules and a consistent set of dependencies.This is intended to help with workflows making changes across multiple modules and with editor support for those workflows.
This is what an example
go.work
file would look like:This adds two modules to the workspace. If the user's current working directory is is under the directory containing this
go.work
, thego
command will be in workspace mode and use both the modules defined by./baz/go.mod
and./tools/go.mod
as main modules, regardless of which module the user is currently in (unless workspace mode is turned off or a different workspace is chosen with the proposed new-workfile
flag). Thereplace
would override any replacesin the main modules'
go.mod
files.Related issues
#32394 x/tools/gopls: support multi-module workspaces
Issue #32394 is about
gopls
' support for multi-module workspaces.gopls
currently allows users to provide a "workspace root" which is a directory it searches forgo.mod
files to build a supermodule from. Alternatively, users can create agopls.mod
file in their workspace root thatgopls
will use as its supermodule. This proposal creates a concept of a workspace that is similar to thatgopls
that is understood by thego
command so that users can have a consistent configuration across their editor and direct invocations of thego
command.#44347 proposal: cmd/go: support local experiments with interdependent modules; then retire GOPATH
Issue #44347 proposes adding a
GOTINKER
mode to thego
command. Under the proposal, ifGOTINKER
is set to a directory, thego
command will resolve import paths and dependencies in modules by looking first in aGOPATH
-structured tree under theGOTINKER
directory before looking at the module cache. This would allow users who want to have aGOPATH
like workflow to build aGOPATH
atGOTINKER
, but still resolve most of their dependencies (those not in theGOTINKER
tree) using the standard module resolution system. It also provides for a multi-module workflow for users who put their modules underGOTINKER
and work in those modules.This proposal also tries to provide some aspects of the
GOPATH
workflow and to help with multi-module workflows. A user could put the modules that they would put underGOTINKER
in that proposal into theirgo.work
files to get a similar experience to the one they'd get under theGOTINKER
proposal. A major difference between the proposals is that inGOTINKER
modules would be found by their paths under theGOTINKER
tree instead of being explicitly listed in thego.work
file. But both proposals provide for a set of replaced module directories that take precedence over the module versions that would normally be resolved by MVS, when working in any of those modules.#26640 cmd/go: allow go.mod.local to contain replace/exclude lines
The issue of maintaining user-specific replaces in
go.mod
files was brought up in #26640. It proposes an alternativego.mod.local
file so that local changes to the go.mod file could be made adding replaces without needing to risk local changes being committed ingo.mod
itself. Thego.work
file provides users a place to put many of the local changes that would be put in the proposedgo.mod.local
file.The text was updated successfully, but these errors were encountered: