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: go install <path> modifies current go.mod #26048

Closed
mwf opened this issue Jun 25, 2018 · 7 comments
Closed

cmd/go: go install <path> modifies current go.mod #26048

mwf opened this issue Jun 25, 2018 · 7 comments
Labels
FrozenDueToAge modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@mwf
Copy link

mwf commented Jun 25, 2018

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

go version go1.10.1 darwin/amd64 vgo:2018-02-20.1

Latest vgo:

commit 0f3e556044ecbd8a0805d098e52a743a1d0e3566 (HEAD -> master, origin/master, origin/HEAD)
Author: Tim Cooper <tim.cooper@layeh.com>
Date:   Fri Jun 22 17:51:20 2018 -0300

    cmd/go/internal/modcmd: fix sorting

What did you do?

Create empty go.mod (to be able to execute vgo install)

cd $(mktemp -d)
echo "module temp" > go.mod
vgo install github.com/mwf/goplay/vgo/hello_world
vgo install github.com/mwf/goplay/vgo/hello_world/v2

What did you expect to see?

I expect both v1 and v2 binaries to be installed at $GOPATH/bin

What did you see instead?

Second call to vgo install github.com/mwf/goplay/vgo/hello_world/v2 fails, because vgo install modifies go.mod in current directory.

$ cat go.mod
module temp

require github.com/mwf/goplay/vgo/hello_world v1.0.1
$ vgo install github.com/mwf/goplay/vgo/hello_world/v2
vgo: import "github.com/mwf/goplay/vgo/hello_world/v2" [/Users/ikorolev/.gvm/pkgsets/go1.10.1/global/src/mod/github.com/mwf/goplay/vgo/hello_world@v1.0.1/v2]: open /Users/ikorolev/.gvm/pkgsets/go1.10.1/global/src/mod/github.com/mwf/goplay/vgo/hello_world@v1.0.1/v2: no such file or directory

So vgo tries to find v2 under current v1 tree instead of resolving the version and obviously fails.

I don't remember the actual plans about vgo get and vgo install, because I read too many docs these days :) but it seems better to divide the usage: get for updating the current module, install for installing binaries from anywhere. Now you can't do it without being under a module root.

And finally the last concern - any plans to support versioned install? Like vgo install github.com/mwf/goplay/vgo/hello_world@v1.0.0? It also fails now :(

@gopherbot gopherbot added this to the vgo milestone Jun 25, 2018
@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 26, 2018
@myitcv
Copy link
Member

myitcv commented Jul 2, 2018

This is related to #24749 which was marked as a dup of #24605. The latter was closed by https://go-review.googlesource.com/c/vgo/+/117257.

But I think my original issue remains, and it's similar to the issue described here. For example I would expect vgo test ./... not to modify the root go.mod.

@rsc
Copy link
Contributor

rsc commented Jul 10, 2018

Modifying go.mod is OK. What's not OK is that the go.mod line for vgo/hello_world is blocking the addition of vgo/hello_world/v2. I have a CL coming soon to fix this.

@myitcv, sorry about the closed duplicate. #24749 and #24605 really are the same kind of issue but I inadvertently only fixed the latter (foo/...) and not the former (./...). Pending CL 122397 fixes the #24749 case.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/123095 mentions this issue: cmd/go/internal/modload: finish Import implementation

@mwf
Copy link
Author

mwf commented Jul 17, 2018

Hi @rsc, @myitcv !

I just decided to reconsider this issue about modifying go.mod again, because the current behaviour doesn't seem logical for me.

The issue with failing installation is fixed, so after running

cd $(mktemp -d)
echo "module temp" > go.mod
vgo install github.com/mwf/goplay/vgo/hello_world
vgo install github.com/mwf/goplay/vgo/hello_world/v2
cat go.mod

we get this:

module temp

require (
	github.com/mwf/goplay/vgo/hello_world v1.0.1 // indirect
	github.com/mwf/goplay/vgo/hello_world/v2 v2.0.0 // indirect
)

But what you do want from go install is to install a binary to $GOBIN, in most cases you want have direct dependency from this modules.
So running vgo mod -sync correctly eliminates this dependencies and go.mod becomes just

module temp

So what's the point to modify the go.mod at all if it will be purged anyway?

And the next step in the logic chain, why do we need go.mod at all while running vgo install?

I'm just a little bit confused with it, because I can understand why go get needs go.mod (because there won't be GOPATH, so actually you can't clone the project with it anymore), but go install could be very helpful when you just want to install some binary to your system and run it.

@bcmills bcmills modified the milestones: vgo, Go1.11 Jul 20, 2018
@bcmills bcmills changed the title x/vgo: vgo install <path> modifies current go.mod cmd/go: go install <path> modifies current go.mod Jul 20, 2018
@bcmills
Copy link
Contributor

bcmills commented Jul 20, 2018

So what's the point to modify the go.mod at all if it will be purged anyway?

One use-case: if you're trying to debug a user-reported crash in some binary that uses your module, you might go install the binary from within that module — and then you really want your IDE and/or debugger to be able to find the correct definitions for all of the packages involved, which in turn depends on having accurate module definitions.

And the next step in the logic chain, why do we need go.mod at all while running vgo install?

To know what versions of modules to use when building the binary, and to know what versions were used when building the binary. (But see #24250 for the request to be able to install binaries at specific versions outside the context of a current module.)

@mwf
Copy link
Author

mwf commented Jul 20, 2018

One use-case: if you're trying to debug a user-reported crash in some binary that uses your module, you might go install the binary from within that module — and then you really want your IDE and/or debugger to be able to find the correct definitions for all of the packages involved, which in turn depends on having accurate module definitions.

You could use go get for the same use-case. And it would be more convenient, because you could use go get <the binary>@version and you can't do it with go install - it doesn't work with @version syntax.
And if I were debugging a binary using my module I would clone it to a tmp dir and add a replace directive to my project. And the debug process would be add change to module - switch to the binary project && recompile. Having the binary installed inside you project won't help you to debug, I believe.

And the next step in the logic chain, why do we need go.mod at all while running vgo install?

To know what versions of modules to use when building the binary, and to know what versions were used when building the binary. (But see #24250 for the request to be able to install binaries at specific versions outside the context of a current module.)

No, I didn't mean the go.mod inside the project you're installing. But go.mod in the directory where you run go install.
EDITED: I understood your answer in a wrong way, sorry. You meant the case when the binary you install don't have a go.mod itself, and you'd like to fixate a dependency list somewhere. But we won't need this behaviour in future when most of the projects already contain go.mod inside.
/end

I just want to install a binary, that's it. And now I can't just run

$ cd `mktemp -d`
$ export GOBIN=$PWD
$ GO111MODULE=on go install golang.org/x/tools/cmd/goimports
go: cannot find main module root; see 'go help modules'

In order to install a binary to GOBIN I have to initialize a bogus go.mod, which breaks the current flow of installing go binary from anywhere you want.

And the error go: cannot find main module root; see 'go help modules' is confusing a bit. I've played with vgo for a while and I know the problem.
But someone not familiar with vgo would think "WAT?! A module root? It seems there is an error in the project I'm trying to install" :))

So what I'm trying to say, we have 2 types of project - library and binary.
We redefined go get saying it works with modules now and modifies your current go.mod - use it to get and update your dependencies (which are libraries in most cases). Fine :)

It would be great to say then - "Use go install to install a binary".
So that the GO111MODULE=on go install golang.org/x/tools/cmd/goimports won't fail just because you don't have root go.mod

We've all got used to run go get golang.org/x/tools/cmd/goimports and expect goimports to be compiled and installed.
Let's have the same simple flow but with go install?

@bcmills if you wish you could convert #24250 into a proposal and I could describe my thoughts there in a more elegant way. Because I understand a former bug issue is not a right place to do it :)

P.S. I'm in love with new go module system guys and I really appreciate all the amazing work you've done ❤️
So please don't take me wrong, I'm just trying to help you with a vision from the outside.

And I'm issuing the questions, which could help me understand the reasoning under some decisions, so that I could explain it to the others and help to adopt new dependency management in my company and in the community.

Thanks!

@bcmills
Copy link
Contributor

bcmills commented Jul 20, 2018

We don't have a solid enough design for a concrete proposal yet (see #24250 (comment)).

If you want to sketch out the interactions and work out the details, please do make one: even if there's something missing, having a concrete proposal would give us a baseline to build on. (Even though #24250 and this issue exist, it's probably clearer to reset the discussion for a concrete proposal, and we can always mark things as duplicates.)

@golang golang locked and limited conversation to collaborators Jul 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

5 participants