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: deprecate the -i flag #41696

Open
jayconrod opened this issue Sep 29, 2020 · 13 comments
Open

cmd/go: deprecate the -i flag #41696

jayconrod opened this issue Sep 29, 2020 · 13 comments

Comments

@jayconrod
Copy link
Contributor

@jayconrod jayconrod commented Sep 29, 2020

go build, go install, and go test accept the -i flag. From go help build:

The -i flag installs the packages that are dependencies of the target.

For example, if you run go build -i example.com/a, and example.com/x imports example.com/y, go build installs the file $GOPATH/pkg/$GOOS_$GOARCH/example.com/y.a.

This was useful for speeding up builds before Go 1.10, since previously installed packages didn't need to be recompiled. go build re-used packages installed in $GOPATH/pkg.

Go 1.10 introduced the build cache, so the $GOPATH/pkg directory is largely obsolete. Compiled packages are now stored in $GOCACHE. The -i flag merely copies files out of the build cache, so there's no longer any performance advantage.

The -i can cause errors when a target directory is not writable, as in #37962. In that issue, VSCode installed tools with -i, which caused errors when $GOROOT/pkg was not writable (common when Go is installed system-wide with an installer). Something caused runtime/cgo to be recompiled (perhaps a new clang version or flag), but it couldn't be written to $GOROOT/pkg/darwin_amd64/runtime/cgo.a.

Because of these problems, we should consider deprecating and eventually removing the -i flag.

@jayconrod jayconrod added this to the Proposal milestone Sep 29, 2020
@jayconrod
Copy link
Contributor Author

@jayconrod jayconrod commented Sep 29, 2020

@rsc rsc added this to Incoming in Proposals Oct 6, 2020
@rsc rsc moved this from Incoming to Active in Proposals Oct 7, 2020
@rsc
Copy link
Contributor

@rsc rsc commented Oct 14, 2020

Are there any objections to doing this? I see plenty of emoji in favor and nothing else.

@mvdan
Copy link
Member

@mvdan mvdan commented Oct 17, 2020

My only potential objection is breaking people who learned to use go build -i years ago, and perhaps introduced the flag in build or CI scripts. So the eventual removal of support for -i should come with a clear deprecation warning in the release changelog at least one or two releases before then.

@rsc
Copy link
Contributor

@rsc rsc commented Oct 20, 2020

@mvdan, yes, deprecate and remove means it will have a release of printing a warning that -i is going away followed by the actual removal in the next release.

@rsc
Copy link
Contributor

@rsc rsc commented Oct 21, 2020

Based on the discussion above, this seems like a likely accept.
To be clear, Go 1.16 would warn that the flag should not be used (but still complete successfully), and Go 1.17 would remove the flag.

@rsc rsc moved this from Active to Likely Accept in Proposals Oct 21, 2020
@rsc
Copy link
Contributor

@rsc rsc commented Oct 28, 2020

No change in consensus, so accepted.

@rsc rsc moved this from Likely Accept to Accepted in Proposals Oct 28, 2020
@rsc rsc changed the title proposal: cmd/go: deprecate the -i flag cmd/go: deprecate the -i flag Oct 28, 2020
@rsc rsc removed this from the Proposal milestone Oct 28, 2020
@rsc rsc added this to the Backlog milestone Oct 28, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Oct 29, 2020

Change https://golang.org/cl/266368 mentions this issue: cmd/go: print deprecation messages for -i

gopherbot pushed a commit that referenced this issue Nov 9, 2020
build, install, and test will now print deprecation messages when the
-i flag is used. clean will continue to support -i.

For #41696

Change-Id: I956c235c487a872c5e6c1395388b4d6cd5ef817a
Reviewed-on: https://go-review.googlesource.com/c/go/+/266368
Trust: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
@gopherbot
Copy link

@gopherbot gopherbot commented Nov 10, 2020

Change https://golang.org/cl/268581 mentions this issue: cmd/go: update test_race_install expected output for CL 266368

gopherbot pushed a commit that referenced this issue Nov 10, 2020
test_race_install checks that 'go test -i -race …' does not rebuild
already installed packages, by also passing '-v' and verifying that no
package names are printed to stderr.

CL 266368 added a deprecation message for the '-i' flag that caused
the stderr output to be non-empty, although it still does not print
any package names.

Updates #41696

Change-Id: I13e10e49b7c33139be9b13f24cb393c9f58fd85d
Reviewed-on: https://go-review.googlesource.com/c/go/+/268581
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
@mvdan
Copy link
Member

@mvdan mvdan commented Apr 20, 2021

It just occurred to me that sometimes I still used go test -i -v ./... to see what packages were being built in advance of running tests.

I guess I can move to go test -run=- -v ./... nowadays, but I thought I'd point that out as a reasonable use case.

@bcmills
Copy link
Member

@bcmills bcmills commented Apr 20, 2021

@mvdan, is there a way to use the -c flag to get the information you want?

(Perhaps go test -c -v -o /dev/null ./...?)

@mvdan
Copy link
Member

@mvdan mvdan commented Apr 20, 2021

Doesn't seem like I can use -c with many packages at once, which makes sense.

For clarity, what I liked about go test -i -v ./... is that I could see the stream of packages being built. This is useful if go test ./... suddenly seems to take a long time to start running tests, for example after someone adds a large cgo dependency like go-sqlite3. Seeing the stream of packages being built, one would see the progress stuck at that package for more than ten seconds, which is a pretty clear signal.

But maybe there are better ways to inspect what's being a bottleneck in a Go build :) There's go list -test -deps ./..., but that simply lists packages, it doesn't perform a build or tell me what packages are the slowest to build.

go list -test -export -v ./... is pretty close, though; -export means building the packages, -test pulls in test deps, and -v prints packages as they are being built.

@bcmills
Copy link
Member

@bcmills bcmills commented Apr 20, 2021

But maybe there are better ways to inspect what's being a bottleneck in a Go build

Maybe -debug-trace (#38714)?

@gopherbot
Copy link

@gopherbot gopherbot commented May 25, 2021

Change https://golang.org/cl/322629 mentions this issue: cmd/go,cmd/link: do not check for staleness in most tests

gopherbot pushed a commit that referenced this issue May 27, 2021
Instead, check that stale packages in the standard library
are not rebuilt when already present in the build cache,
and are not installed implicitly when rebuilt.

We retain the staleness checks for the runtime package in tests
involving '-i', because those are guaranteed to fail anyway if the
package is stale and the "stale" failure message is arguably clearer.
They can be removed if/when we remove the '-i' flag, but the runtime
package is less likely to become stale because it does not have cgo
dependencies.

Fixes #46347
Updates #33598
Updates #35459
Updates #41696

Change-Id: I7b0a808addd930f9f4911ff53ded62272af75a40
Reviewed-on: https://go-review.googlesource.com/c/go/+/322629
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Proposals
Accepted
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants