-
Notifications
You must be signed in to change notification settings - Fork 17.6k
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: allow replacement version to be omitted if the target module has a required version #28176
Comments
I'm pretty sure that won't work at all right now: as a consequence of the fix for #26607, you cannot both |
This is certainly worth considering when we allow overlapping replacements, though. (That's #26904.) |
Just expanding on the use case briefly: Currently it is an error to not have a right-hand version in a If the right-hand version in a
And then have the actual versions used could be automatically be resolved "normally" via MVS and the normal version picking rules, but hitting That would be less fragile than something like a
I suspect that would help make modules a better upgrade for users that previously relied on glide Perhaps the ultimate answer might end up being "use Athens", or "roll your own GOPROXY", but seems at least worth considering including this in the core go tool. |
@bcmills not the most urgent question, but could you expand slightly on what you meant here:
In particular, what do you mean by a "target" there -- the left side module in the replace, or the right side module? Going back to the first example from @dmitris above, I think this is what he was trying to say as a theoretical
Which piece of that would violate the "forbid use of one module with two different paths" restriction from the fix for #26607? (Of course, that is not a valid |
The right side module.
I'm not sure. I think I misunderstood the original report! |
@bcmills - to make sure we are on the same page, do you think some variation of the following change as clarified by @thepudds on Oct. 12 would be possible:
(instead of having to specify the replacement as currently: It you try that now (with go1.12.1), you get an error: A coworker just asked me today about the best way to update "in one shot" both the I understand that there is like a problem with |
I have a use case that I think fits this, but might not? I often find myself forking open source modules, and wanting to build. The relative file path works fine for my local checkout, but fails horrible for CI or inside-docker builds. I want to say something simple, like |
A
See #26964. |
If I have a fork, than it's not going to be in the same place. (Unless I'm using submodules.... that's an interesting thought) |
@bcmills - would like to bump it up one more time.
It is fairly labor-intensive, painful and error prone to do by hand - very common case is when the Much better would be able to do:
(without specifying any version, and where the version would be taken from the Do you think it is possible in principle? Do you have any questions or objections regarding the need and use case? |
FWIW, I think this makes sense in principle, seems aligned with the general modules philosophy, and helps In addition to the corporate mirror use case, it would also help when you have to do something like:
This can be needed because currently you must use Using a In that example, really all you want to do is:
|
Just lurking here, I may not have understood correctly, but how do you ensure compatibility (including tags) of the replaced path? consider what you propose:
What ensures that the destination Unless you have setup automated tooling for keeping an exact copy of the upstream, there might be a skew in what you host internally and what is available upstream. For keeping copies, or mirrors, there are solutions like project Athens. |
@davidovich In our case, there is a periodic (nightly or on demand) mirroring of all the commits for all the mirrored repositories (except deletions - to protect against the
Athens is an exciting project and useful in many cases - but as https://docs.gomods.io/ says, it is Go specific: "Athens is a Server for Your Go Packages". One advantage of a "general" internal opensource mirror is that it can serve a variety of corporate use cases and development teams - besides Go repos, you might need to mirror https://android.googlesource.com/ for mobile devs, https://git.netfilter.org/ for packet hackers, C++ / Python libraries for those still awaiting the Go enlightenment, and so on - all using the same mirror and update mechanism. And if you already have such a mirror in-house, you might as well use it for Go - instead of having to set up and maintain another piece of infrastructure like an Athens server. |
Thanks for the response @dmitris. So this feature would only apply to exact mirrors. I understand the usefulness. I also see that a user could misuse (replacement on a stale repo that does not have a tag) and I don't know what/how confusing the failure mode would be in that case. |
the suggestion is to have an optional omission of the version number in replace (defaulting to the version from the corresponding |
We have no reason in general to believe that a replacement for a given module has all of the same tags (even into the future!) as the module it replaces. There are cases where it does hold, temporarily, but those In contrast, having an omitted version mean “the MVS-selected version of the module” (as I suspect we'll want for #26904) is always well-defined if any version of the replacement module has ever existed. That seems like a much more universal behavior, and more in line with the large semantic space occupied by “a module path without a version”. (That's not to say that we can't also consider some signal for “replace X with Y at the same tag as X”; I just don't think that signal should be “omitting the version in the |
Perhaps I am missing a nuance, but I think the intent from @dmitris is to use whatever version MVS would select, or at least, that seems to be what makes sense to me. For example, if a
That would mean use whatever MVS selects for foo. For example:
That eliminates the manual work of someone like @dmitris currently needing to manually update the version on the right-hand side of If
But the underlying details of |
I mean the right-hand side. (The selected version of the module whose version was omitted.) |
The wrinkle with using the MVS-selected version from the left side is that it may be ambiguous. Consider: require (
spoon v1.1.0
fork v1.2.0
)
replace (
spoon => spork
fork => spork
) Now which version do we use for |
In contrast, the following seems unambiguous: we should use exactly the version of require (
spoon v1.1.0
fork v1.2.0
spork v1.0.0
)
replace (
spoon => spork
fork => spork
) |
One way to handle that could be to disallow that. In other words, if you alias > 1 left-hand side module (spoon and fork) to a single right-hand side module (spork), you must have a version supplied on the right-hand side (or slightly more liberal is to allow it if there is agreement on versions for For the last example you gave:
An alternate way to specify the same input information could be putting the version for spork in the
The drawback of course is in that case, the In part it comes down to whether you want MVS to be computing a version based on That might depend on the use case? If for example On the other hand, I'm less sure that using the right-hand side of the
(where the import statements must be or for:
(where the import statements and hence |
Maybe there could be a general rule about a For example, when a
My prior comment was typed a bit quickly, as was this one, so sorry in advance if I am butchering left vs. right or missing some obvious things here. |
FWIW, the terms I've been using are “replaced module” (for the left-hand side of the rule) and “target module” (for the right-hand side). I would prefer to have a much simpler rule. The one I had in mind is:
To address the use-case for this proposal, I would add an explicit symbol (perhaps That gives something like:
|
@thepudds #28176 (comment) accurately describes our need / request ( 1) the main thing is to avoid the fragility of inadvertently having only the @bcmills - I guess
(compare:
) |
added a comment #32721 (comment) describing our "stop gap" solution/tool for the replacement versions getting out of sync with |
@dmitris, here's an interesting issue (from #32721 (comment)): what happens if your module is a fork of, say, upstream The next available release version is Given that versions of Go modules are immutable and that programmers are fallible, I don't see how the assumption that the two modules are versioned in lock-step can scale. |
@bcmills most of our |
Another solid use case for this, from my own (apparently duplicate) ticket #35382, is to effectively "alias" imports. The primary use case for this is vanity URLs and URL redirects. For example, the package path
The package's go.mod and import comments enforce it being imported as
Unfortunately, this means we're specifying the version for that library twice, once in the This type of path aliasing was (is still, I guess) a feature on the vendoring tools commonly used prior to the module implementation, and it seems rational as a result to support it on the module implementation as well. |
For reference - currently it seems to be impossible to have a Edit: actually the extra "custom mirroring" is not necessary - it is sufficient to add a |
^^ (gopkg.in/yaml.v2 problem) is fixed in #34254 |
We also have this issue. We use internal mirrors for many of our dependencies, which means we have to use a Ideally, we would be able to specify the |
Currently you must add a version when entering a non-filesystem (remote) replacement for a module. A foolish attempt to put
replace github.com/fsnotify/fsnotify => gitmirror.corp.xyz.com/fsnotify/fsnotify
provokes a rebuke fromgo mod verify
:go.mod:42: replacement module without version must be directory path (rooted or starting with ./ or ../)
replace github.com/fsnotify/fsnotify => gitmirror.corp.xyz.com/fsnotify/fsnotify v1.4.7
.However, there is already a version specification for that package in
go.mod
therequire
statement, for example:require github.com/fsnotify/fsnotify v1.4.7
This seems to be the reasonable default value for the missing version in the
replace
directive for the same package - "if no replacement version is given, use the same as in therequire
directive for that specific package`.What would be especially nice is when upgrading a package such as github.com/fsnotify/fsnotify to the future v1..4.8 version, one would not need to first run
go get -u github.com/fsnotify/fsnotify
and then have to look up the new version and manually update the old version to the new one in thereplace
section (or worse, forgetting to do it and ending up with the unintended replacement with the old version).@thepudds said on Slack that he wanted to suggest this as well. @bcmills @rsc - does it seem reasonable to you?
The text was updated successfully, but these errors were encountered: