Skip to content
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

Open
dmitris opened this issue Oct 12, 2018 · 25 comments

Comments

Projects
None yet
6 participants
@dmitris
Copy link
Contributor

commented Oct 12, 2018

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 from go mod verify:
go.mod:42: replacement module without version must be directory path (rooted or starting with ./ or ../)

  • so you have to fix it to be: 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 the require 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 the require 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 the replace 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?

@bcmills

This comment has been minimized.

Copy link
Member

commented Oct 12, 2018

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.

@bcmills

This comment has been minimized.

Copy link
Member

commented Oct 12, 2018

This is certainly worth considering when we allow overlapping replacements, though. (That's #26904.)

@bcmills bcmills changed the title modules - make the trailing version optional in the replace statement in go.mod cmd/go: allow replacement version to be omitted if the target module has a required version Oct 12, 2018

@bcmills bcmills added this to the Go1.12 milestone Oct 12, 2018

@thepudds

This comment has been minimized.

Copy link

commented Oct 12, 2018

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.

@thepudds

This comment has been minimized.

Copy link

commented Oct 12, 2018

@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).

@bcmills bcmills modified the milestones: Go1.12, Go1.13 Oct 25, 2018

@bcmills

This comment has been minimized.

Copy link
Member

commented Nov 15, 2018

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!

@dmitris

This comment has been minimized.

Copy link
Contributor Author

commented Mar 25, 2019

@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).

@directionless

This comment has been minimized.

Copy link

commented Mar 26, 2019

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

@bcmills

This comment has been minimized.

Copy link
Member

commented Mar 27, 2019

@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.

@directionless

This comment has been minimized.

Copy link

commented Apr 3, 2019

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)

@dmitris

This comment has been minimized.

Copy link
Contributor Author

commented Apr 24, 2019

@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 required 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?

@thepudds

This comment has been minimized.

Copy link

commented Apr 24, 2019

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
@davidovich

This comment has been minimized.

Copy link

commented Apr 24, 2019

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.

@dmitris

This comment has been minimized.

Copy link
Contributor Author

commented Apr 24, 2019

@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.

@davidovich

This comment has been minimized.

Copy link

commented Apr 24, 2019

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.

@dmitris

This comment has been minimized.

Copy link
Contributor Author

commented Apr 24, 2019

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 replaces). 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.

@bcmills

This comment has been minimized.

Copy link
Member

commented Apr 30, 2019

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 requires 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”.)

@thepudds

This comment has been minimized.

Copy link

commented May 7, 2019

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:

  • If this is the only require foo across the build list, then it would be v1.2.3.
  • If this is the only 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.
  • If there are multiple 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.

@thepudds

This comment has been minimized.

Copy link

commented May 7, 2019

@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.

@bcmills

This comment has been minimized.

Copy link
Member

commented May 7, 2019

I mean the right-hand side. (The selected version of the module whose version was omitted.)

@bcmills

This comment has been minimized.

Copy link
Member

commented May 7, 2019

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?

@bcmills

This comment has been minimized.

Copy link
Member

commented May 7, 2019

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
)
@thepudds

This comment has been minimized.

Copy link

commented May 7, 2019

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).

@thepudds

This comment has been minimized.

Copy link

commented May 7, 2019

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 #28176 (comment))

  • 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.

@bcmills

This comment has been minimized.

Copy link
Member

commented May 7, 2019

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:

  1. If the 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.)
  2. If the 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 required 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
)
@dmitris

This comment has been minimized.

Copy link
Contributor Author

commented May 7, 2019

@thepudds #28176 (comment) 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

)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.