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

x/vgo: mod -sync does not list direct dependency of dependency but lists dependency of the dependency of the direct dependency #26215

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

Comments

@dlsniper
Copy link
Contributor

dlsniper commented Jul 4, 2018

What version of Go are you using (go version)?

vgo @ 6cd5a417451d8ee907692eded07ef1b6b53825b1

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

Windows 10/amd64

What did you do?

What did you expect to see?

The following contents in go.mod

module vgoawesomeProject

require (
	github.com/dlsniper/vgodemo v1.0.0
	github.com/gorilla/mux v1.6.2
)

or at least, if I understand the description of mod -sync correctly.

module vgoawesomeProject

require (
	github.com/dlsniper/vgodemo v1.0.0
)

What did you see instead?

module vgoawesomeProject

require (
	github.com/dlsniper/vgodemo v1.0.0
	github.com/gorilla/context v1.1.1
)

I don't understand how come context is listed there but not mux itself?
vgo build seems to at least list just vgodemo as dependency and not context which is more in line with what I would expect given our previous conversation in #25971.

Thank you.

@gopherbot gopherbot added this to the vgo milestone Jul 4, 2018
@bcmills
Copy link
Contributor

bcmills commented Jul 4, 2018

mux is implied by the go.mod for the vgodemo module, while context is not.

This should get much less confusing after https://golang.org/cl/121304 lands.

@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Jul 4, 2018
@dlsniper
Copy link
Contributor Author

dlsniper commented Jul 4, 2018

It's not just about being confusing. The problem is that tools need to be able to retrieve the list of all modules/packages needed in order to create the complete environment for users.

As an example of where this would be needed, here's how the GoLand integration for vgo works, and what users expect that the IDE will display for them: https://youtrack.jetbrains.com/issue/GO-5865#focus=streamItem-27-2935718-0-0 Should we use vgo list -m -json all in this case? But if I understand the CL you linked above, this too would not be enough then.

@bcmills
Copy link
Contributor

bcmills commented Jul 4, 2018

Yes, tools should use vgo list -m -json. By design, the go.mod file only describes the requirements imposed by that module itself. (Indirect dependencies should only appear if they are at a higher version than what is implied by the other transitive requirements.)

@dlsniper
Copy link
Contributor Author

dlsniper commented Jul 4, 2018

Here's another case then, at least in the current implementation.

With the following content in go.mod:

module vgoawesomeProject

require github.com/dlsniper/vgodemo v1.0.0

replace github.com/dlsniper/vgodemo v1.0.0 => github.com/dlsniper/vgodemo v0.0.0-20180704102233-4356961a96dc

Running vgo list -m -json gives:

{
        "Path": "vgoawesomeProject",
        "Main": true
}

Running vgo list -m -json all gives:

{
        "Path": "vgoawesomeProject",
        "Main": true
}
{
        "Path": "github.com/dlsniper/vgodemo",
        "Version": "v1.0.0",
        "Replace": {
                "Path": "github.com/dlsniper/vgodemo",
                "Version": "v0.0.0-20180704102233-4356961a96dc",
                "Time": "2018-07-04T10:22:33Z",
                "Dir": "D:\\go\\src\\mod\\github.com\\dlsniper\\vgodemo@v0.0.0-20180704102233-4356961a96dc"
        },
        "Time": "2018-07-04T10:22:33Z",
        "Dir": "D:\\go\\src\\mod\\github.com\\dlsniper\\vgodemo@v0.0.0-20180704102233-4356961a96dc"
}
{
        "Path": "github.com/gorilla/mux",
        "Version": "v1.6.2",
        "Time": "2018-05-13T03:22:33Z",
        "Dir": "D:\\go\\src\\mod\\github.com\\gorilla\\mux@v1.6.2"
}

However, in this output there is no github.com/gorilla/context present, but it should be, as it's a direct dependency of github.com/gorilla/mux. As such, if a user would want to navigate to anything that depends on context then this wouldn't be possible. As a side not this is a bit contrieved since context is used only in Go < 1.7 but I can come up with a more concrete example if needed.

@bcmills
Copy link
Contributor

bcmills commented Jul 4, 2018

in this output there is no github.com/gorilla/context present, but it should be, as it's a direct dependency of github.com/gorilla/mux

There is no go.mod in github.com/gorilla/mux v1.6.2, so (as of CL 120999) it has no dependencies. (If you run go mod -sync in vgoawesomeProject you should find that it adds an indirect requirement, but that's an addition made by examining the package import graph, not something that was already present in the module graph.)

There is a go.mod for gorilla/mux at head, but it doesn't specify github.com/gorilla/context either:
https://github.com/gorilla/mux/blob/cb4698366aa625048f3b815af6a0dea8aef9280a/go.mod#L1

@rsc
Copy link
Contributor

rsc commented Jul 6, 2018

Yes, the confusion is because gorilla/mux v1.6.2 is not providing its own dependencies, so your go.mod file is doing it instead. It looks like the next gorilla/mux will have a go.mod and hopefully one with context listed as a dep. And then your project won't have to supply it instead.

As Bryan noted, soon that line will say // indirect on it (follow CL 121304).

As Bryan also noted, unless you're writing a go.mod editor, you should use "go list -json -m all" instead of "go mod -json".

@rsc rsc closed this as completed Jul 6, 2018
@golang golang locked and limited conversation to collaborators Jul 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants