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: offer a consistent "global install" command #30515

Open
mvdan opened this Issue Mar 1, 2019 · 31 comments

Comments

Projects
None yet
@mvdan
Copy link
Member

mvdan commented Mar 1, 2019

As of 1.12, one can run the following from outside a Go module:

GO111MODULE=on go get foo.com/cmd/bar

The same mechanism can be used to download a specific version instead of @latest, such as @v1.2.3.

We can even emulate very similar behavior in Go 1.11, with a one-liner like:

cd $(mktemp -d); go mod init tmp; go get foo.com/cmd/bar

1.13 will likely make GO111MODULE=on a default, so it's likely that project READMEs will be able to justs tell users to run go get foo.com/cmd/bar on Go 1.13 and later.

However, this has a problem - if the user runs the command from within a Go module, the command will add clutter to the module's go.mod and go.sum files. The binary will also be installed as usual, so the user might not even notice the unintended consequences until much later.

This is a fairly common point of confusion among Go developers, particularly those new to modules. Now that most Go projects are modules, the chances of one running cmd/go commands within a module are quite high.

What we need is a "global install" command, which will ignore the current module. For example, imagine a go get -global, to ensure backwards compatibility. Or perhaps even repurposing go install to always mean global installs, since go get can be used for non-global ones.

I think we should make a decision and implement it before the 1.13 release, so that READMEs can finally drop the problematic go get -u foo.com/cmd/bar line. We can almost do it now, minus this confusing edge case with $PWD.

/cc @bcmills @rsc @myitcv @rogpeppe @thepudds

@bcmills

This comment has been minimized.

Copy link
Member

bcmills commented Mar 1, 2019

@myitcv

This comment has been minimized.

Copy link
Member

myitcv commented Mar 1, 2019

cc @ianthehat given the recent golang-tools related conversations too.

@bcmills

This comment has been minimized.

Copy link
Member

bcmills commented Mar 1, 2019

See previously #27643, #28400, #28156.

@jayconrod

This comment has been minimized.

Copy link
Contributor

jayconrod commented Mar 1, 2019

I think we should repurpose go install for this when GO111MODULE=on. Since the build cache is required now, it doesn't seem like there's much use for go install anymore, especially when the target is a main package not in the build list.

@mvdan

This comment has been minimized.

Copy link
Member Author

mvdan commented Mar 1, 2019

I agree that repurposing go install for this would be ideal in the long run, as right now go get and go install seem to overlap a bit too much. But we'd need to document and announce the new behavior of go install well, as this would be a breaking change for some users.

@ianthehat

This comment has been minimized.

Copy link

ianthehat commented Mar 1, 2019

I think it is fairly easy to make a case for
go install module/binary@version
doing exactly what the user would expect (not modifying your local go.mod, and reliably building that specific version no matter what the local directory/state is)
I don't think it would even be a breaking change if that is all we did (at the moment specifying a version would not be valid)

The harder things are at the edges, things like are a way of doing that work and then running the resulting binary (had to do will with go run, and I am not sure we should even try), or wether the behaviour of go install when not given a version should be changed to avoid any confusion.

@thepudds

This comment has been minimized.

Copy link

thepudds commented Mar 1, 2019

From what I have observed as well as for my personal usage, this would be very useful regardless of how it is spelled (go install, go get -global, ...).

If it is implemented, I think it would be important to use the remote go.mod if one exists, including respecting any replace and exclude directives in that remote go.mod. In practice, authors will need to use replace or exclude in some cases, even though hopefully it will not be the common case. One of the main goals of modules is 100% reproducible builds, and that seems especially important when publishing a binary.

@thepudds

This comment has been minimized.

Copy link

thepudds commented Mar 1, 2019

At this point, personally I would vote for go install.

Regarding this:

I think it is fairly easy to make a case for
go install module/binary@version
...
The harder things are at the edges, things like ... whether the behaviour of go install when not given a version should be changed to avoid any confusion.

If go install module/binary@version takes on the behavior suggested by this proposal, then I think go install module/binary would need to be equivalent to go install module/binary@latest (including for consistency with go get foo vs. go get foo@latest behavior).

A related question -- should go get some/cmd and go get some/cmd@v1.2.3 stop installing binaries? That would be a more radical change and would reduce overlapping behavior. One could argue that might be desirable if starting from scratch, but given the number of install instructions out there that say go get some/cmd or go get -u some/cmd, I suspect it would be preferable to let go get keep installing binaries.

@peter-edge

This comment has been minimized.

Copy link

peter-edge commented Mar 1, 2019

Just passing by. +1 to having this. My current workaround as of 1.12:

$(STATICCHECK):
$(eval STATICCHECK_TMP := $(shell mktemp -d))
@cd $(STATICCHECK_TMP); GOBIN=path/to/bin go get honnef.co/go/tools/cmd/staticcheck@$(STATICCHECK_VERSION)
@rm -rf $(STATICCHECK_TMP)

@pbx0

This comment has been minimized.

Copy link

pbx0 commented Mar 1, 2019

I also support go install being for global binary installs to gopath and go get only for use inside a module path.

Modules brings some breaking tooling changes and it seems like making some more here is the time to do it and worth the cost since this makes the tooling much easier to understand.

@rogpeppe

This comment has been minimized.

Copy link
Contributor

rogpeppe commented Mar 2, 2019

I support this suggestion.

One question: would this re-purposed go install command respect replace and exclude directives in the installed module? I've argued before that it should, and I still think that's the case.

To take one example, it wouldn't be possible to build a correctly working executable for github.com/juju/juju/cmd/juju without respecting replace directives.

@ianthehat

This comment has been minimized.

Copy link

ianthehat commented Mar 2, 2019

I think the results should be identical to as if you had checked out the module at the supplied version to a temporary directory, changed to that directory and done go install ., so yes, fully obeying the go.mod with replace and excludes applied, not treating it as a dependancy.

@mvdan

This comment has been minimized.

Copy link
Member Author

mvdan commented Mar 2, 2019

Something that we might want to be careful about is go install commands which are given local packages, and not remote ones. For example, I presume that go install . or go install ./cmd/foo should still use the current module.

@ianthehat

This comment has been minimized.

Copy link

ianthehat commented Mar 2, 2019

Yes, this is one of the reasons why I am dubious about making any changes to non versioned forms, and thus don't agree that an implied @latest is the right choice.
On the other hand, I am slightly worried that the radical change of behavior adding a version causes might be confusing, but I think it's probably ok.

@thepudds

This comment has been minimized.

Copy link

thepudds commented Mar 2, 2019

@mvdan I suspect I am missing something obvious… But could the rule be to use the go.mod file (including respecting replace and exclude) based on wherever the "package main" is coming from? In other words, use the remote go.mod file if installing a remote command, and use the local go.mod file of the module you are in currently if doing something like go install .?

@jayconrod

This comment has been minimized.

Copy link
Contributor

jayconrod commented Mar 4, 2019

Trying to distill the discussion so far into a proposal:

We would change the behavior of go install [modules]. [modules] is one or more module patterns, as currently interpreted by go get. go get itself would not change. Each module pattern has an optional version query (@latest if no query is specified). Each module pattern will be handled separately.

We change behavior if:

  • go install is run in module mode AND
  • The module pattern being installed is NOT any of the following:
    • Empty (equivalent to .)
    • A local path without a version (e.g., ./cmd/foo, ./...).
    • A metapackage (all, cmd, std).
    • Anything matching packages in the standard library.

If the conditions above are true, go install will:

  • Download the module at the specified version
    • Patterns like @latest or @v1 will be interpreted the same way that go get interprets them.
  • Build packages and executables matching the pattern, treating the downloaded module as the main module.
    • replace and exclude directives will be observed.
    • vendor directories will be used if they would be used normally, e.g., -mod=vendor is set. After #30240 is implemented, vendoring would be used by default (as if the main module were the installed module).
  • Copy packages and executables to their target locations, i.e., the path indicated by go list -f '{{.Target}}'
    • This will normally be $(go env GOPATH)/bin or $(go env GOPATH)/pkg.
  • go install will not modify the go.mod or go.sum files in the module where go install is invoked. Version constraints in the module where go install is invoked will be ignored.

Examples:

  • These commands will not change behavior:
    • Any variant of go get.
    • go install
    • go install cmd
    • go install ./cmd/foo
    • go install ./...
    • go install main.go
  • These commands will change when run in module mode:
    • go install golang.org/x/tools/packages
    • go install golang.org/x/tools/cmd/goimports
    • go install golang.org/x/tools/cmd/goimports@v1.2.3
    • go install golang.org/x/tools/cmd/...
    • go install golang.org/x/tools/cmd/...@v1.2.3
    • go install ./cmd/foo@latest
  • An error message would be printed if go install is run in GOPATH mode with an argument that includes an @ character, same as go get.
@jayconrod

This comment has been minimized.

Copy link
Contributor

jayconrod commented Mar 4, 2019

I think a key question is whether go install with a non-local path and no version should behave differently than a non-local path with an explicit version. For example, should go install golang.org/x/tools/cmd/goimports work differently than go install golang.org/x/tools/cmd/goimports@latest?

I'd argue the form with no version should be consistent with other module-related commands. For a new user, it would be strange to accidentally run the former command instead of the latter, then see it take your build constraints into account and possibly modify your go.mod file. This would be a change from current behavior. If we require explicit versions, we'd only be adding new behavior.

@mvdan

This comment has been minimized.

Copy link
Member Author

mvdan commented Mar 4, 2019

I think go install remote.org/pkg should behave just as if @latest had been given. That's more consistent, and like you say, less confusing to users.

@bcmills

This comment has been minimized.

Copy link
Member

bcmills commented Mar 5, 2019

@rogpeppe @ianthehat The trouble with applying replace directives is that it isn't feasible to apply them consistently. We could apply replacements that specify other modules, but not filesystem paths: the replacement module must have its own go.mod file, and as such will not be found within the same module source tree in the module cache.

That means, for example, that we wouldn't be able to fetch the module from a proxy.

The same consideration holds for vendor directories: if we implement #30240, then the go.mod files in the vendor directory will cause essentially all of the source files in that tree to be pruned out of the module cache.

We could, at least in theory, apply the subset of replacements that specify module paths rather than filesystem paths. However, that would produce a third variation on the build configuration: one that doesn't necessarily match the clean-checkout configuration (because it doesn't apply filesystem replacements), but doesn't necessarily match the clean-external-build configuration either (because it does apply non-filesystem replacements).

As another alternative, we could apply module replacements and emit an error of the module has any filesystem replacements. That would produce the same result as a build within the module, but at the cost of rejecting some otherwise-well-formed modules.

I don't think that the benefit of enabling temporary fixes and workarounds offsets the complexity of adding either of those modes of operation. Users have enough trouble understanding module-mode behavior as it is. It seems much simpler to require that modules build successfully using unmodified dependencies, even if that results in a bit of extra work for the maintainers of large modules to upstream their modifications or maintain a complete fork.

@ianthehat

This comment has been minimized.

Copy link

ianthehat commented Mar 5, 2019

As I said before, the rule should be the tool is built exactly as if you had fetched it from a proxy, extracted it to a temporary directory, changed to the directory, and typed go install. I think possibly we should also specify the readonly flag, but that's a separate discussion.
This will apply replace directories in a fully consistent way, with no special rules of any kind required.
More importantly, it will attempt to build exactly the same binary that you would build if you checked out the repository, which is a consistency that is far easier to explain.
It may well fail to work if someone has checked in a bad go.mod, but I am fine with that, people should not be checking in go.mod files with replace directories that are not self contained anyway, and if they do then go install module/binary@version will stop working for them.
I have also said that I strongly disagree with #30240, and I agree this is one more way it will cause problems if we do implement it, but it merely causes a case that does not currently work to still not work, it's not really an argument for not doing this.

@rsc

This comment has been minimized.

Copy link
Contributor

rsc commented Mar 5, 2019

"Repurposing" go install seems like a non-starter to me. It has a defined meaning, and that meaning is not "pretend we're not in the current module". We can't break that. Similarly, go get is by design the only command that accepts @version. If go install adds it, everything has to add it (go build, go test, go vet, etc), and that way lies incoherence.

@rsc

This comment has been minimized.

Copy link
Contributor

rsc commented Mar 5, 2019

Half-applying replace directives also seems like a non-starter to me. It would be a new half-way mode that has similar coherence problems.

@mvdan

This comment has been minimized.

Copy link
Member Author

mvdan commented Mar 5, 2019

"Repurposing" go install seems like a non-starter to me. It has a defined meaning

That's fair enough, but aren't go install and go get practically the same? Now that install also fetches source code, that is.

This is where the idea of slightly changing the meaning of one of them comes from, to keep both commands useful. We could always add a flag like go get -global, but I find that a bit clunky, since installing a Go program on the system is a fairly common need.

@rsc

This comment has been minimized.

Copy link
Contributor

rsc commented Mar 5, 2019

@bcmills and I talked a bit about various "auto-detect" kind of ways to shove this into the go command and didn't come up with anything palatable. I could see adding a short flag to go get to support this, like maybe go get -b for binary or bare, but the first question to answer is what "this" means.

  • Does it mean "run as if in a directory outside any module?"
  • Does it mean "run using the upgrades implied by the current module's go.mod but just don't write any changes back down to go.mod?"
  • Does it mean something else?
@rsc

This comment has been minimized.

Copy link
Contributor

rsc commented Mar 5, 2019

@mvdan Whether they are practically the same doesn't really matter. What matters is that they have established semantics that can't just be redefined.

@mvdan

This comment has been minimized.

Copy link
Member Author

mvdan commented Mar 5, 2019

Does it mean "run as if in a directory outside any module?"

This is what I'd imagine.

What matters is that they have established semantics that can't just be redefined.

Wouldn't the same have applied to go install or go build suddenly fetching code in module mode? That was hidden behind GO111MODULE=on, but still, it seems to me like it changed the established semantics somewhat. Similar to what's been suggested here, in my mind.

@thepudds

This comment has been minimized.

Copy link

thepudds commented Mar 5, 2019

Regarding the concern about filesystem paths in replace directives in a remote go.mod, I think it could be reasonable to reject those with an error if someone does go get -b (or perhaps alternatively, reject any filesystem-based replace directives in a remote go.mod that go outside the module).

If the result is an error, does that avoid the concern with a new half-way mode?

Some alternatives for what go get -b could mean:

  1. go get -b could mean "run as if you cloned the repo and ran go install from within the repo."

  2. Alternatively, go get itself when run outside of a module could be redefined in 1.13 to respect replace and exclude directives in a remote go.mod if one exists. This was considered for 1.12 (e.g., one variation suggested by @bcmills in #24250 (comment)) but was not implemented for 1.12. If that was implemented for 1.13, then go get -b could mean "run as if in a directory outside any module" (which implies respecting replace and exclude directives in a remote go.mod given that is what go get would do when outside of a module).

I wouldn't be surprised if one or both of those formations are not precise enough, but wanted to send some grist towards the mill.

@mvdan

This comment has been minimized.

Copy link
Member Author

mvdan commented Mar 5, 2019

Something also worth mentioning is that the proposal was never about obeying replace directives. I think it would be easier for everyone to focus on the issue that is the current directory where one runs go install.

Once we have some form of a "global install" command, we can open a separate issue about what to do with replace directives.

@jayconrod

This comment has been minimized.

Copy link
Contributor

jayconrod commented Mar 6, 2019

After speaking with @bcmills yesterday and reading through the comments here, I think we should ignore replace directives entirely. To recap, file system replace directives will almost always point outside the module, probably somewhere within the repository. GOPROXY will be the common case in the future, so we won't actually download anything outside a module most of the time. Consequently, file system replace directives usually won't work. If we only observed module replace directives (ignoring or rejecting file system directives with an error), this would introduce a new build mode. Builds could fail in this mode but not when replace directives are completely ignored or observed. We shouldn't ask authors to test in this additional mode.

Also, I agree with @rsc's point about go install semantics. This would be too big of a change.

So maybe we can reach consensus on the following:

  • The new behavior would be with go get -b, only with modules enabled. go get -b would print an error in GOPATH mode.
  • go get -b <pkg>@<version> would behave as if you ran go get <pkg>@<version> from an empty directory with no go.mod or other root in any parent directory. It would ignore requirements, excludes, and replacements from the current module (if there is a current module).
@jimmyfrasche

This comment has been minimized.

Copy link
Member

jimmyfrasche commented Mar 6, 2019

Semantics aside, go get -b doesn't seem right. It's cryptic without being mnemonic.

It's not obvious what "b" stands for. That makes it hard to intuit what it means the first time you see it. That, in turn, makes it hard to remember later on—especially if it's been a while since you've used it.

Of course, if you use it enough, you'll memorize it, but not everyone is going to need to use this often enough to memorize it. And, if you have to consult the docs, a clearer name is going to stand out, so you can jump directly to the relevant section instead of having to scan until you find it.

While this may not be an uncommon thing to do, I don't believe it is something that is so common that eliminating a few keystrokes will add up to any meaningful savings for the average user.

Something like go get -global seems preferable, given that repurposing go install is out.

@ianthehat

This comment has been minimized.

Copy link

ianthehat commented Mar 6, 2019

So the command we want is one that given a versioned import path, installs the binary built exactly at that version.
The primary use case is installing tools used during development, which is why it is important that it works even within a module and does not modify that module in any way.
Assuming we agree on that part, looking at the help of the two commands:

usage: go install [-i] [build flags] [packages]

Install compiles and installs the packages named by the import paths.
usage: go get [-d] [-m] [-u] [-v] [-insecure] [build flags] [packages]

Get resolves and adds dependencies to the current development module
and then builds and installs them.

I think it is very very clear that it matches exactly the help of go install and totally does not match the help of go get, so I would argue that it does not repurpose go install. This is specifically why I am arguing we should not change any existing working go install command, no inferring of latest or anything, we just add some way to go install to allow versioned imports. I would be fine with adding a special flag to go install if we need to make the use case clearer than just adding a version to the import path, but I really think it fits naturally within the scope of go install.

I think the first thing to do though is to read agreement about the operations we need and what their semantics should be, once we are sure of that we can discuss the command line that implements those.

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.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.