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: -pkgdir doesn't respect -installsuffix or its variants #20137

Closed
tamird opened this issue Apr 26, 2017 · 5 comments
Closed

cmd/go: -pkgdir doesn't respect -installsuffix or its variants #20137

tamird opened this issue Apr 26, 2017 · 5 comments
Milestone

Comments

@tamird
Copy link
Contributor

@tamird tamird commented Apr 26, 2017

Please answer these questions before submitting your issue. Thanks!

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

go version go1.8.1 darwin/amd64

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/tamird/src/go"
GORACE=""
GOROOT="/Users/tamird/src/go1.8"
GOTOOLDIR="/Users/tamird/src/go1.8/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/sw/d6165ghx7jx3cdl7cggxzbx00000gn/T/go-build026752780=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"

What did you do?

main.go:

package main

func main() {
}
$ go install -pkgdir ~/foobarpkgdir
$ ls ~/foobar*
total 3264
drwxr-xr-x   4 tamird  staff   136B Apr 26 13:28 .
drwxr-xr-x+ 33 tamird  staff   1.1K Apr 26 13:28 ..
drwxr-xr-x   3 tamird  staff   102B Apr 26 13:28 runtime
-rw-r--r--   1 tamird  staff   1.6M Apr 26 13:28 runtime.a
$ go install -race -pkgdir ~/foobarpkgdir
$ ls ~/foobar*
total 3344
drwxr-xr-x   4 tamird  staff   136B Apr 26 13:29 .
drwxr-xr-x+ 33 tamird  staff   1.1K Apr 26 13:29 ..
drwxr-xr-x   5 tamird  staff   170B Apr 26 13:29 runtime
-rw-r--r--   1 tamird  staff   1.6M Apr 26 13:29 runtime.a
$ go install -installsuffix baz -pkgdir ~/foobarpkgdir
$ ls ~/foobar*
total 3264
drwxr-xr-x   4 tamird  staff   136B Apr 26 13:31 .
drwxr-xr-x+ 33 tamird  staff   1.1K Apr 26 13:31 ..
drwxr-xr-x   5 tamird  staff   170B Apr 26 13:29 runtime
-rw-r--r--   1 tamird  staff   1.6M Apr 26 13:31 runtime.a

What did you expect to see?

I expected -pkgdir to specify the "base" package directory, akin to $GOPATH/pkg, so that race builds and builds specifying an install suffix are separately cached.

What did you see instead?

-pkgdir caused -race and -installsuffix to not affect the package cache directory.

-pkgdir was implemented this way in #10210. Unfortunately, behaviour makes it extremely cumbersome to implement cross compilation toolchains that use cgo since handling of -race and -installsuffix must be reimplemented outside of the go tool.

If it is not feasible to change the behaviour of -pkgdir, perhaps a new flag -rootpkgdir could be added that would place nice with installsuffixes.

@rsc

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Jun 5, 2017

For Go 1.9 I'm inclined to leave this alone; Go 1.10 will probably make it unnecessary to exercise this kind of control.

@tamird

This comment has been minimized.

Copy link
Contributor Author

@tamird tamird commented Jun 5, 2017

@benesch

This comment has been minimized.

Copy link
Contributor

@benesch benesch commented Jul 6, 2017

@rsc seems to be planning an overhaul of build caching (source 1, source 2, source 3) for Go 1.10, though I can't seem to turn up more information about the project. I too would be quite interested to hear more about what's in store! Build/test caching is one of our bigger pain points over at CockroachDB.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Nov 2, 2017

Change https://golang.org/cl/75473 mentions this issue: cmd/go: cache built packages

gopherbot pushed a commit that referenced this issue Nov 3, 2017
This CL adds caching of built package files in $GOCACHE, so that
a second build with a particular configuration will be able to reuse
the work done in the first build of that configuration, even if the
first build was only "go build" and not "go install", or even if there
was an intervening "go install" that wiped out the installed copy of
the first build.

The benchjuju benchmark runs go build on a specific revision of jujud 10 times.

Before this CL:

	102.72u 15.29s 21.98r 	 go build -o /tmp/jujud github.com/juju/juju/cmd/jujud ...
	105.99u 15.55s 22.71r 	 go build -o /tmp/jujud github.com/juju/juju/cmd/jujud ...
	106.49u 15.70s 22.82r 	 go build -o /tmp/jujud github.com/juju/juju/cmd/jujud ...
	107.09u 15.72s 22.94r 	 go build -o /tmp/jujud github.com/juju/juju/cmd/jujud ...
	108.19u 15.85s 22.78r 	 go build -o /tmp/jujud github.com/juju/juju/cmd/jujud ...
	108.92u 16.00s 23.02r 	 go build -o /tmp/jujud github.com/juju/juju/cmd/jujud ...
	109.25u 15.82s 23.05r 	 go build -o /tmp/jujud github.com/juju/juju/cmd/jujud ...
	109.57u 15.96s 23.11r 	 go build -o /tmp/jujud github.com/juju/juju/cmd/jujud ...
	109.86u 15.97s 23.17r 	 go build -o /tmp/jujud github.com/juju/juju/cmd/jujud ...
	110.50u 16.05s 23.37r 	 go build -o /tmp/jujud github.com/juju/juju/cmd/jujud ...

After this CL:

	113.66u 17.00s 24.17r 	 go build -o /tmp/jujud github.com/juju/juju/cmd/jujud ...
	3.85u 0.68s 3.49r 	 go build -o /tmp/jujud github.com/juju/juju/cmd/jujud ...
	3.98u 0.72s 3.63r 	 go build -o /tmp/jujud github.com/juju/juju/cmd/jujud ...
	4.07u 0.72s 3.57r 	 go build -o /tmp/jujud github.com/juju/juju/cmd/jujud ...
	3.98u 0.70s 3.43r 	 go build -o /tmp/jujud github.com/juju/juju/cmd/jujud ...
	4.58u 0.70s 3.58r 	 go build -o /tmp/jujud github.com/juju/juju/cmd/jujud ...
	3.90u 0.70s 3.46r 	 go build -o /tmp/jujud github.com/juju/juju/cmd/jujud ...
	3.85u 0.71s 3.52r 	 go build -o /tmp/jujud github.com/juju/juju/cmd/jujud ...
	3.70u 0.69s 3.64r 	 go build -o /tmp/jujud github.com/juju/juju/cmd/jujud ...
	3.79u 0.68s 3.41r 	 go build -o /tmp/jujud github.com/juju/juju/cmd/jujud ...

This CL reduces the overall all.bash time from 4m22s to 4m17s on my laptop.
Not much faster, but also not slower.

See also #4719, #20137, #20372.

Change-Id: I101d5363f8c55bf4825167a5f6954862739bf000
Reviewed-on: https://go-review.googlesource.com/75473
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Dec 1, 2017

Go 1.10 has in fact made it unnecessary to exercise this kind of control. I can't quite tell from the original report why you thought -pkgdir was needed, but it is not needed anymore.

@rsc rsc closed this Dec 1, 2017
@golang golang locked and limited conversation to collaborators Dec 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants
You can’t perform that action at this time.