Open dmitris opened 5 years ago
there is already a version specification for that package in
go.mod
[in a]require
statement
I'm pretty sure that won't work at all right now: as a consequence of the fix for #26607, you cannot both require
a module and use it as the target of a replace
.
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 replace
directive if the replacement is a remote repo.
If the right-hand version in a replace
was allowed to be optional when the replacement is a remote repo, it could be natural then to tell the go tooling how to obtain a copy from a mirror, for example:
replace github.com/foo/bar => gitmirror.corp.com/foo/bar
And then have the actual versions used could be automatically be resolved "normally" via MVS and the normal version picking rules, but hitting gitmirror.corp.com
rather than github.com
to get the VCS metadata (and actual source code, etc.). In other words, the replacement would apply to the repo, not the version.
That would be less fragile than something like a go.mod
file with 15 replace
lines that effectively hard code the versions (with sample here taken from a real-world example):
replace (
github.com/ardielle/ardielle-go => mirror.foobar.com/ardielle/ardielle-go.git v1.5.1
github.com/aws/aws-sdk-go => mirror.foobar.com/aws/aws-sdk-go.git v1.15.49 // indirect
github.com/coreos/go-oidc => mirror.foobar.com/coreos/go-oidc.git v2.0.0+incompatible
github.com/golang/protobuf => mirror.foobar.com/golang/protobuf.git v1.2.0
...
I suspect that would help make modules a better upgrade for users that previously relied on glide repo
directives or dep source
and similar.
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:
I'm pretty sure that won't work at all right now: as a consequence of the fix for #26607, you cannot both require a module and use it as the target of a replace.
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 go.mod
if this issue/suggestion here was implemented:
require github.com/fsnotify/fsnotify v1.4.7
replace github.com/fsnotify/fsnotify => gitmirror.corp.xyz.com/fsnotify/fsnotify
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 go.mod
today because of there is no version on the right-hand side of the replace, but that's the topic of this issue/suggestion here).
In particular, what do you mean by a "target" there -- the left side module in the replace, or the right side module?
The right side module.
Which piece of that would violate the "forbid use of one module with two different paths" restriction from the fix for #26607?
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:
require github.com/fsnotify/fsnotify v1.4.7
replace github.com/fsnotify/fsnotify => gitmirror.corp.xyz.com/fsnotify/fsnotify
(instead of having to specify the replacement as currently: replace github.com/fsnotify/fsnotify => gitmirror.corp.xyz.com/fsnotify/fsnotify v1.4.7
)?
It you try that now (with go1.12.1), you get an error: go.mod:29: replacement module without version must be directory path (rooted or starting with ./ or ../)
.
A coworker just asked me today about the best way to update "in one shot" both the require
and replace
sections of go.mod
to a more recent version of some dependencies - the best I could think of was "use go get
to update the require
section, then a text editor to manually modify all the corresponding lines in the replace
section." This is not only lots of manual labor but also very error-prone - a common error I see is forgetting to change some replace lines consistently and therefore inadvertently using the old version (and making go.mod
a confused mess). Yes, one could write a linter for go.mod
to flag the differences in versions between require
and replace
- and then hope everyone remembers to use it every time changing go.mod - but much better and easier would be to have the go tool helping here.
I understand that there is like a problem with replace github.com/fsnotify/fsnotify => gitmirror.corp.xyz.com/fsnotify/fsnotify
syntax since go
consideres gitmirror.corp.xyz.com/fsnotify/fsnotify
the file path to the replacement module, therefore the error "must be directory path...".
Maybe we could add some "syntax sugar" like replace github.com/fsnotify/fsnotify => gitmirror.corp.xyz.com/fsnotify/fsnotify v*
meaning use the same as version as in the corresponding require
line (I believe "*" is never a valid version spec otherwise).
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 replace github.com/fsnotify/fsnotify => replace github.com/directionless/fsnotify@branchname
but instead I'm not even sure what I do. Mostly I get frustrated
@directionless
The relative file path works fine for my local checkout, but fails horrible for CI or inside-docker builds.
A replace
directive with a relative path that points inside the same repository should work fine within CI and Docker builds, provided that they are building from a checkout of that repository (as opposed to, say, an expanded copy of the module found within the Go module cache).
I want to say something simple, like replace github.com/fsnotify/fsnotify => replace github.com/directionless/fsnotify@branchname
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. In order to use our corporate git mirror, currently we have to manually keep updating version numbers in like this:
replace github.com/user/xyz => git.mycompany.com/ghmirror/user--xyz v1.2.3
It is fairly labor-intensive, painful and error prone to do by hand - very common case is when the require
d package version is upgraded with go get
, but the corresponding replace
one is not 😭
Much better would be able to do:
replace github.com/user/xyz => git.mycompany.com/ghmirror/user--xyz
(without specifying any version, and where the version would be taken from the require
). This would greatly help to keep the require
and replace
statements in sync without too much extra yak shaving 😄
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 replace
more directly replace the functionality of the source
directive in dep
, the repo
directive in glide
and similar functionality in other dependency managers.
In addition to the corporate mirror use case, it would also help when you have to do something like:
replace github.com/docker/docker => github.com/docker/engine v18.09.5
This can be needed because currently you must use import "github.com/docker/docker"
as the import path, but that docker/docker
repo does not get version tags anymore (seemingly as part of the docker commercialization effort, the split to moby, etc., and not a simple reason like "whoops, they forgot").
Using a replace
like that with the version specified is a bit painful for many of the reasons @dmitris touched on above (including gets out of sync with the version in a require
based on an explicit go get
, or based on the natural version MVS would select based on all version constraints across the entire build).
In that example, really all you want to do is:
replace github.com/docker/docker => github.com/docker/engine
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:
require github.com/user/xyz v1.2.3
replace github.com/user/xyz => git.mycompany.com/ghmirror/user--xyz
What ensures that the destination ghmirror
has even the tag v1.2.3? Or that the replacement is changed in an incompatible manner later, and needs to be pinned.
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 left-pad
-like incidents), so it is more or less "keeping an exact copy of the upstream". I don't encounter or hear of many cases of the versions being available in upstream but not in the internal mirror, so don't think it's a real problem (at least in our case). Also if you do have a version that's not available internally, you usually discover it very quickly with go mod verify
or from the CI build failure, and can adjust accordingly (ex. pin to the previous version or the latest available in the mirror or kick the mirror update for that repo).
For keeping copies, or mirrors, there are solutions like project Athens.
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 require
) - so for those for whom such an omission is not desirable or detrimental, I think nothing would change compared to the status quo (just continue to put version in the replace
s). You would still be able to state the version in the replace
lines - which would also be useful if you want to override package github.com/foo/bar v1.2.3
with git.mymirror.com/foo/bar v0.9.0
etc. But it would make the devel life easier for those for whom the current "double version specification" is an onerous chore and a maintenance / correctness issue.
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 replace
directives can become nonsensical as soon as the original module publishes a new version and someone require
s it: now you have an unreplicated tag, and what do you do with it?
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 replace
directive”.)
In contrast, having an omitted version mean “the MVS-selected version of the module” (as I suspect we'll want for #26904)
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 go.mod
read:
require foo v1.2.3
replace foo => foo.copy // note: no version on right-hand side
That would mean use whatever MVS selects for foo. For example:
require foo
across the build list, then it would be v1.2.3
.require foo
across the build list, and someone does go get foo@v1.4.5
, then the go.mod
would be updated to read require foo v1.4.5
, and then v1.4.5
would be used.require foo
across the build list, then it would be the semantically highest version of foo required.That eliminates the manual work of someone like @dmitris currently needing to manually update the version on the right-hand side of replace
after doing something like a go get
, and more generally allows someone to say "please find the actual code here" (akin to source
directive in dep
, etc.).
If foo.copy
does not have a tag v1.2.3
but v1.2.3
is selected as the version by MVS, then that would be an error, just as it is currently an unknown revision
error to do something like:
require bar some-tag-that-does-not-exist
But the underlying details of replace
semantics are tricky, so maybe these comments don't make sense.
@bcmills
In contrast, having an omitted version mean “the MVS-selected version of the module” (as I suspect we'll want for #26904)
When you say "the module" there, I am not sure if you mean the left-hand side module or right-hand side module of the replace
statement.
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 spork
, assuming that these replace
directives specify aliases of a single module (per #26904) rather than replacing the source code for modules fork
and spoon
individually?
In contrast, the following seems unambiguous: we should use exactly the version of fork
that we specified.
require (
spoon v1.1.0
fork v1.2.0
spork v1.0.0
)
replace (
spoon => spork
fork => spork
)
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 )
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 spoon
and fork
).
For the last example you gave:
In contrast, the following seems unambiguous: we should use exactly the version of fork that we specified.
require ( spoon v1.1.0 fork v1.2.0 spork v1.0.0 ) replace ( spoon => spork fork => spork )
An alternate way to specify the same input information could be putting the version for spork in the replace
itself:
require (
spoon v1.1.0
fork v1.2.0
)
replace (
spoon => spork v1.0.0
fork => spork v1.0.0
)
The drawback of course is in that case, the spork
version would be coming from the replace
, and not be influenced by other require spork
elsewhere.
In part it comes down to whether you want MVS to be computing a version based on spork
.
That might depend on the use case?
If for example spork
is some forked module that has capabilities of spoon
and fork
(which is suggested by the names you picked), and perhaps you have more than one require spork
in your build list, then it could make sense to have MVS pick the version of spork
.
On the other hand, I'm less sure that using the right-hand side of the replace
to pick the version makes sense for things like:
replace github.com/docker/docker => github.com/docker/engine
(where the import statements must be import "github.com/docker/docker"
, and hence you would often have multiple require github.com/docker/docker vX.Y.Z
across the build list).
or for:
replace github.com/user/xyz => mirror.mycompany.com/ghmirror/user-xyz
(where the import statements and hence require
statements across the various go.mod
files would use github.com/user/xyz
).
Maybe there could be a general rule about a require foo
implying that foo
should participate in
MVS for version selection?
For example, when a replace
statement has no versions in it:
If only the right-hand side appears in the require list of that go.mod
, then the right-hand side participates in MVS.
If only the left-hand side appears in the require list of that go.mod
, then the left-hand side participates in MVS (but perhaps it is an error if multiple replace
statements collude to make this ambiguous, such as in https://github.com/golang/go/issues/28176#issuecomment-490170934)
If both the right-hand and the left-hand appear in the require list of that go.mod
, then it is an error (or, if needed based on use case analysis, maybe break the tie by favoring one over the other in this case).
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:
replace
directive specifies a target version, then exactly that version replaces the source code of the replaced module, and no module aliasing occurs. (This is the behavior of replace
today, except that we would allow the same module to replace the source code of multiple others.)replace
directive does not specify a target version, then the replaced module becomes an alias for the target module, and the target module is resolved via MVS as usual (using the require
d version, resolving it to the latest
version if not already present).To address the use-case for this proposal, I would add an explicit symbol (perhaps @
, =
, or @same
?) to the first form, meaning “use the source code from the target path at the version specified by the replaced module”.
That gives something like:
require (
spoon v1.1.0
fork v1.2.0
)
replace (
spoon => spork @same # v1.1.0
fork => spork @same # v1.2.0
)
@thepudds https://github.com/golang/go/issues/28176#issuecomment-490162178 accurately describes our need / request ( 1) the main thing is to avoid the fragility of inadvertently having only the require
and not replace
part updated 2) avoid the "double work" of having to manually update the version in two places, or have require
upgraded by go get
but replace
- through manual copy-and-paste).
@bcmills - I guess @same
would do the job but it would look a bit visually noisy IMO (possibly bikeshedding) to have a page of require
lines all with that marker. Maybe *
could also work in that function?
replace (
github.com/spf13/pflag => git.companyxyz.com/ghmirror/spf13--pflag * // indirect
golang.org/x/net => git.companyxyz.com/ghmirror/golang--net *
golang.org/x/oauth2 => git.companyxyz.com/ghmirror/golang--oauth2 * // indirect
golang.org/x/sync => git.companyxyz.com/ghmirror/golang--sync * // indirect
golang.org/x/sys => git.companyxyz.com/ghmirror/golang--sys * // indirect
)
(compare:
github.com/spf13/pflag => git.companyxyz.com/ghmirror/spf13--pflag @same // indirect
github.com/yahoo/athenz => git.companyxyz.com/ghmirror/yahoo--athenz @same
golang.org/x/net => git.companyxyz.com/ghmirror/golang--net @same
golang.org/x/oauth2 => git.companyxyz.com/ghmirror/golang--oauth2 @same // indirect
golang.org/x/sync => git.companyxyz.com/ghmirror/golang--sync @same // indirect
golang.org/x/sys => git.companyxyz.com/ghmirror/golang--sys @same // indirect
)
added a comment https://github.com/golang/go/issues/32721#issuecomment-530514187 describing our "stop gap" solution/tool for the replacement versions getting out of sync with require
ones on updates
@dmitris, here's an interesting issue (from https://github.com/golang/go/issues/32721#issuecomment-531764896): what happens if your module is a fork of, say, upstream v1.2.3
, and you discover a bug within your patch that needs to be fixed?
The next available release version is v1.2.4
, but if you're assuming that the two modules are versioned in lock-step, that would collide with the upstream v1.2.4
if and when they make their own patch release.
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 replace
s are for the corporate internal mirroring. In cases where we modify / fix the external code, I would expect folks to fork the repo into somewhere outside of the mirror-github
org, and I'd modify modfix
to leave the replace
s as is if the target doesn't start with the git.ourcompany.com/mirror-github
prefix. I could also imagine having a "magic" comment like // sic!
meaning "I really mean that as written and it's OK if the version is different from the one in the require
stanza - modfix
, leave this alone!". For those few special cases, it would be onto the developers to update the replace
when needed.
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 go.opencensus.io
redirects to github.com/census-instrumentation/opencensus-go
using the go-import
metatag:
<meta name="go-import" content="go.opencensus.io git https://github.com/census-instrumentation/opencensus-go">
The package's go.mod and import comments enforce it being imported as go.opencensus.io
. However, my corporate firewall has timeout issues when dealing with certain redirect endpoints like this (the opencensus one being a very significant one for us, because the cloud.google.com/go/datastore
library has it as a several-layers-deep transitive dependency). To solve this, we added the following to our project's go.mod:
go.opencensus.io => github.com/census-instrumentation/opencensus-go v0.22.0
Unfortunately, this means we're specifying the version for that library twice, once in the require
block and once in the replace
block, leading to the error-prone manual update need highlighted earlier in this thread. In this case, go.opencensus.io
is a strict redirect of the underlying github.com/census-instrumentation/opencensus-go
repository. It doesn't have a separate versioning system, so any version referenced on go.opencensus.io
is only valid if and only if it exists on the underlying github repo anyway, so there's never a problem with a new tag being present on the github repo and not on the replaced path, or vice versa.
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 replace
of gopkg.in/yaml.v2
without breaking some go
commands (like go mod download
as well as the new module-happy godoc
) see https://github.com/go-yaml/yaml/issues/546#issuecomment-552884813 as well as an "ugly hack" with mirroring just one branch in a new repo: https://github.com/dmitris/go-yaml-v2/blob/master/README.copy.md#problem.
Edit: actually the extra "custom mirroring" is not necessary - it is sufficient to add a replace
with a pseudo-version like
gopkg.in/yaml.v2 => git.xyzcompany.com/mirror-github/go-yaml--yaml v0.0.0-20191104180343-f90ceb4f4090
- than all go commands including go mod download
and godoc
work. (The question is just how to calculate such a pseudo-version with minimal efforts 😄 )
^^ (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 replace
directive and define a hard-coded version separate from the require
.
Ideally, we would be able to specify the replace
without a version, and the automated version solver would find a working version from our mirror.
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 thereplace
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?