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: satisfy import by adding require of available replacement #26241

Closed
rsc opened this issue Jul 6, 2018 · 17 comments
Closed

cmd/go: satisfy import by adding require of available replacement #26241

rsc opened this issue Jul 6, 2018 · 17 comments
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@rsc
Copy link
Contributor

rsc commented Jul 6, 2018

#25053 and at least one other issue I've lost got tripped up by having to write both a replace and a require statement to get at local code. I wonder if we could avoid that confusion by having the import satisfier check for a relevant replacement in go.mod and if one exists, try to use it directly instead of doing the network lookup at all. If the replacement is version-less then we could make up a version like v0.0.0-0.

@rsc rsc added this to the vgo milestone Jul 6, 2018
@rsc rsc modified the milestones: vgo, Go1.11 Jul 12, 2018
@rsc rsc added the modules label Jul 12, 2018
@rsc rsc changed the title x/vgo: satisfy import by adding require of available replacement cmd/go: satisfy import by adding require of available replacement Jul 12, 2018
@rogpeppe
Copy link
Contributor

rogpeppe commented Aug 2, 2018

I just came across a specific use case for this: when developing against submodules within the same repository. It's useful to be able to modify and use code within the submodule (and from other external modules) before pushing and tagging the changes.

In fact, it's important for any development process when developing across modules and without a network.

@jrick
Copy link
Contributor

jrick commented Aug 6, 2018

I feel that the require with a real version requirement is still neccessary if both modules may be required by a third. Otherwise, the solved version may be too low because it wasn't expressed by the go.mod.

My team has been using replaces with relative paths to great effect on repos which contain multiple modules to allow unpublished changes to be immediately used elsewhere by the repo. However, it does become a pain point as each individual module being included (even transitive modules which are not in the requires) need such a statement.

One idea I am thinking of is to allow "wildcard replaces" to avoid dozens of replace statements, rather, this would only need a single "replace github.com/my/proj/* => ./" (or any other relative path back to the repo root) in each module's go.mod. Then, github.com/my/proj/a and github.com/my/proj/b would be both be replaced by a relative path, along with any future required dependencies with the shared package prefix.

@myitcv
Copy link
Member

myitcv commented Aug 6, 2018

I feel that the require with a real version requirement is still neccessary if both modules may be required by a third. Otherwise, the solved version may be too low because it wasn't expressed by the go.mod.

Are you referring to different major version requirements here?

@jrick
Copy link
Contributor

jrick commented Aug 6, 2018

No, the minor or patch.

@myitcv
Copy link
Member

myitcv commented Aug 8, 2018

No, the minor or patch.

Then I don't follow, because without the replace statement they would end up with the same version in any case, by definition.

The one case I think we would need to be distinguish is different major versions. Maybe this points to us being able to specify replace statements with only the major version, e.g.:

mvdan.cc/sh v2 => /path/to/mvdan

which would leave a require for v1.x.x untouched.

@jrick
Copy link
Contributor

jrick commented Aug 9, 2018

When the tool only sees and can update to publicized versions of a module, and when you have multiple modules in the same repo which reference each other as dependencies, it is not possible to update one module in a backwards-compatible way and use the new features or bug fixes in another module of the same repo without pushing and tagging the module. The replace works around this, however you must be careful that you still publish and bump the version of the dependency, and the required version of the dependency used in other modules of the repo, before publishing other versions (otherwise, to your importers, the replacement doesn't happen and the required version would be too low).

On a large repo with many modules which all depend on each other in complex ways, this makes the module versioning workflow untenable without replacements, even without ever making a single breaking change.

I haven't yet hit any situations where I needed or wanted to replace an older major with newer major version, and I don't think that would be something worth pursuing. Replacements only apply locally to the module you build directly from, which would cause my package imports to refer to older and incorrect packages, and would not match the import that consumers of my modules should use.

@myitcv
Copy link
Member

myitcv commented Aug 14, 2018

@rsc the one case that might make this more important to include for Go 1.11 is where the dependency is against a major version number >= 2. Because then the import path changes... at which point it's almost certainly the case that the original module's VCS will not know anything about the new major version. And hence any check against that original module's VCS for the new major version import path will fail.

e.g.

module example.com/hello

require github.com/gopherjs/gopherjs/v11 latest

replace (
	// the work to move to v11 was done in a fork
	github.com/gopherjs/gopherjs => github.com/myitcv/gopherjs introduce_v11
)

@thepudds
Copy link
Contributor

Just a quick comment that people continue to run into this (in Slack, mailing list, etc.), so seems worthy to at least consider doing something here...

@bcmills
Copy link
Contributor

bcmills commented Sep 6, 2018

@thepudds Yes, I think this is one of the two most duplicated issues with modules. It seems pretty clear that we need to fix it by 1.12. 🙂

@mediocregopher
Copy link

I think I may have run into a manifestation of this problem while trying to solve a predicament I'm in, which I wrote out here.

Let's say I have three files (Sorry I couldn't put this in a gist, it didn't like having a directory in the name of one of the files):

./main.go:

package main

import "abc.xyz/bar/example/inner"

func main() {
    inner.Success()
}

./inner/inner.go:

package inner

import "fmt"

func Success() {
    fmt.Println("success!")
}

./go.mod:

module abc.xyz/foo/example

replace abc.xyz/bar/example => ./

Theoretically this setup could work, the dependency solver could see that abc.xyz/bar/example is accounted for and doesn't need to be fetched, but it tries to fetch it anyway and fails because that's not a real package url.

In that reddit post I linked I described in greater detail how this would help. The TLDR is that anyone who has a package like github.com/user/pkg.v2, where that package uses sub-packages of itself, and they want to also support having github.com/user/pkg/v2, using go mod, can't right now (unless I'm missing something). The reason is that the import paths to the sub-packages will be wrong in one of the versions of the package. Being able to essentially tell a package "you're name is also X" would solve this, I think.

@myitcv
Copy link
Member

myitcv commented Nov 12, 2018

@mediocregopher if you'll excuse the bullet point form of response:

  • renaming your repo will be fine just so long as you don't have people already using github.com/mediocregopher/radix.v3 as a module. If you do, then renaming the repo renames the module (because you're using github.com there is no layer of indirection between the module/import path and the location of the VCS) and that will break for them. It looks you released v3.0.0 about 11 hours ago, so perhaps this isn't so much of a concern
  • older versions of Go will also be fine with the /v3 module, but only to a point. As the wiki outlines, Go 1.9.7+, 1.10.3+ and Go 1.11+ (in GOPATH-mode) all understand how to deal with versioned import paths when building/testing packages. So if you rename your repo, make the project a /v3 module, then:
    • Go 1.11+ will work in module mode with an import path of github.com/mediocregopher/radix/v3
    • Go 1.9.7+, 1.10.3+ and Go 1.11+ (in GOPATH-mode) will work with import paths of either github.com/mediocregopher/radix.v3 or github.com/mediocregopher/radix
    • Any older versions of Go will not work
  • The theoretical replace you have above will not work, because the target directory contains the module abc.xyz/foo/example and not abc.xyz/bar/example
  • Regarding:

Being able to essentially tell a package "you're name is also X" would solve this, I think.

replace directives act on modules, not packages. A module is a collection of related packages and is the unit of versioning.

@mediocregopher
Copy link

@myitcv no problem! Thanks for responding

renaming your repo will be fine just so long as you don't have people already using github.com/mediocregopher/radix.v3 as a module.

I don't believe anyone is, I did the v3.0.0 tag just to test something out, I should probably delete it though so no one makes that mistake though.

The theoretical replace you have above will not work, because the target directory contains the module abc.xyz/foo/example and not abc.xyz/bar/example

That was supposed to be a simplified use-case of the actual one I'm running into. I didn't want to drive the discussion too far off-course, but maybe it just added confusion.

My actual problem lies in that github.com/mediocregopher/radix.v3 imports github.com/mediocregopher/radix.v3/resp. If I leave all of those import paths as-is in radix.v3's code then they will only work for someone using the radix.v3 instance of the package (via go get). If I want to use radix/v3, then I have to switch all the imports of radix.v3/resp to radix/v3/resp, which would then break anyone using go 1.10 or less.

So I can only do one or the other right now, and I'll have to just leave it radix.v3. Unless it was possible to use a replace to alias a package. If it was, and combined with github's automatic redirecting of renamed packages, both cases could be supported. I would leave the import paths as radix.v3/resp in the code and:

  • anyone doing go get github.com/mediocregopher/radix.v3 would get transparently redirected to github.com/mediocregopher/radix, but the import paths in the code would still be radix.v3 and it would wok.

  • anyone using go mod to import github.com/mediocregopher/radix/v3 would grab it, and the replace directive would tell it, theoretically, "don't download ..radix.v3 when you see it in the code, you can find that package/module here" and those import paths would also still work.

Hopefully that clears it up a bit, and sorry if this is driving the topic too far off course.

@myitcv
Copy link
Member

myitcv commented Nov 12, 2018

@mediocregopher

which would then break anyone using go 1.10 or less.

This is not completely accurate. It will break for people using 1.10.x < 1.10.3 and 1.9.x < 1.9.7. I don't know how many people you think fall into that category.

You could always do the rename, leave current master as is then make the breaking change in v4 on a master.v4 branch.

Let's continue the conversation in https://github.com/mediocregopher/radix.v3 however to avoid sidetracking this issue on the specifics of your case.

@bcmills
Copy link
Contributor

bcmills commented Nov 14, 2018

I'm not sure whether this will make 1.12 — I didn't get to a fix before the freeze, and it's not obvious to me whether the fix will be small enough to make the cut after.

@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Nov 14, 2018
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/152739 mentions this issue: cmd/go/internal/modload: use replacements to resolve missing imports

@bcmills
Copy link
Contributor

bcmills commented Dec 6, 2018

it's not obvious to me whether the fix will be small enough to make the cut after.

As it turns out, the fix was small enough. 🙂

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/153157 mentions this issue: doc: mention the use of replacements to resolve imports for 1.12

gopherbot pushed a commit that referenced this issue Dec 7, 2018
Updates #26241

Change-Id: I8ffac13d9cc1ee4d4de8fcd2042a7fa60fca567b
Reviewed-on: https://go-review.googlesource.com/c/153157
Reviewed-by: Ian Lance Taylor <iant@golang.org>
lcarva added a commit to containerbuildsystem/cachito that referenced this issue Jul 18, 2019
Fedora 30 ships golang 1.12 which has an important fix for handling
non-VCS based go modules:

    golang/go#26241

Projects like Kubernetes specify "sub-modules" such as k8s.io/api, which
point to a path within the repo. In previous version, golang tries to
fetch those from the Internet causing unnecessary delays.

Signed-off-by: Luiz Carvalho <lucarval@redhat.com>
@golang golang locked and limited conversation to collaborators Dec 7, 2019
@rsc rsc unassigned bcmills Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

8 participants