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

Closed
jayconrod opened this issue Sep 29, 2020 · 15 comments
Closed

cmd/go: deprecate the -i flag #41696

jayconrod opened this issue Sep 29, 2020 · 15 comments

Comments

@jayconrod
Copy link
Contributor

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 the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Sep 29, 2020
@jayconrod jayconrod added this to the Proposal milestone Sep 29, 2020
@gopherbot gopherbot added Proposal and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Sep 29, 2020
@jayconrod
Copy link
Contributor Author

cc @bcmills @matloob @rsc

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

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 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 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 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 (old) Oct 21, 2020
@rsc
Copy link
Contributor

rsc commented Oct 28, 2020

No change in consensus, so accepted.

@rsc rsc moved this from Likely Accept to Accepted in Proposals (old) 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 modified the milestones: Proposal, Backlog Oct 28, 2020
@gopherbot
Copy link

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

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 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
Contributor

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 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
Contributor

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

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>
andyrzhao pushed a commit to google/oauth2l that referenced this issue Dec 8, 2021
@gopherbot
Copy link

Change https://go.dev/cl/416094 mentions this issue: cmd/go: remove -i build flag

@gopherbot
Copy link

Change https://go.dev/cl/449075 mentions this issue: doc/go1.20: add release notes for cmd/go changes

gopherbot pushed a commit that referenced this issue Nov 14, 2022
Updates #41696.
Updates #50332.
Updates #41583.

Change-Id: I99e96a2996f14da262570a5cb5273dcdce45df2b
Reviewed-on: https://go-review.googlesource.com/c/go/+/449075
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
justxuewei added a commit to justxuewei/dubbo-go-samples that referenced this issue Apr 5, 2023
`-i` flag was removed from the Go 1.17, see
golang/go#41696. Besides, this PR fixes
a jsonrpc issue that has three same interface names.

Signed-off-by: Xuewei Niu <justxuewei@apache.org>
justxuewei added a commit to justxuewei/dubbo-go-samples that referenced this issue Apr 5, 2023
`-i` flag was removed from the Go 1.17, see
golang/go#41696. Besides, this PR fixes a jsonrpc
issue that has three same interface names.

`DOCKER_HOST_IP` is not used. Obtaining it depends on ifconfig, which is not
built-in on Ubuntu. This could lead to the command not found issue.

Signed-off-by: Xuewei Niu <justxuewei@apache.org>
@golang golang locked and limited conversation to collaborators Nov 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

No branches or pull requests

5 participants