code review 151730045: cmd/go: make build -a skip standard packages in Go releases

cmd/go: make build -a skip standard packages in Go releases

Today, 'go build -a my/pkg' and 'go install -a my/pkg'
recompile not just my/pkg and all its dependencies that
you wrote but also the standard library packages.
Recompiling the standard library is problematic on
some systems because the installed copy is not writable.

The -a behavior means that you can't use 'go install -a all'
or 'go install -a my/...' to rebuild everything after a Go
release - the rebuild stops early when it cannot overwrite
the installed standard library.

During development work, however, you do want install -a
to rebuild everything, because anything might have changed.

Resolve the conflict by making the behavior of -a depend
on whether we are using a released copy of Go or a devel copy.
In the release copies, -a no longer applies to the standard library.
In the devel copies, it still does.

This is the latest in a long line of refinements to the
"do I build this or not" logic. It is surely not the last.

Fixes issue 8290.

[rsc, 2014-09-26 18:19:47]

Hello golang-codereviews@googlegroups.com (cc: adg, iant, r),

I'd like you to review this change to
https://code.google.com/p/go/

[r, 2014-09-26 18:30:27]

LGTM to me, which is to say, "bleah".

[btracey, 2014-09-26 18:31:30]

This also fixes issue 6424



On Friday, September 26, 2014 11:19:48 AM UTC-7, russ cox wrote:
> Reviewers: golang-codereviews,
> 
> 
> 
> Message:
> 
> Hello golang-codereviews@googlegroups.com (cc: adg, iant, r),
> 
> 
> 
> I'd like you to review this change to
> 
> https://code.google.com/p/go/
> 
> 
> 
> 
> 
> Description:
> 
> cmd/go: make build -a skip standard packages in Go releases
> 
> 
> 
> Today, 'go build -a my/pkg' and 'go install -a my/pkg'
> 
> recompile not just my/pkg and all its dependencies that
> 
> you wrote but also the standard library packages.
> 
> Recompiling the standard library is problematic on
> 
> some systems because the installed copy is not writable.
> 
> 
> 
> The -a behavior means that you can't use 'go install -a all'
> 
> or 'go install -a my/...' to rebuild everything after a Go
> 
> release - the rebuild stops early when it cannot overwrite
> 
> the installed standard library.
> 
> 
> 
> During development work, however, you do want install -a
> 
> to rebuild everything, because anything might have changed.
> 
> 
> 
> Resolve the conflict by making the behavior of -a depend
> 
> on whether we are using a released copy of Go or a devel copy.
> 
> In the release copies, -a no longer applies to the standard library.
> 
> In the devel copies, it still does.
> 
> 
> 
> This is the latest in a long line of refinements to the
> 
> "do I build this or not" logic. It is surely not the last.
> 
> 
> 
> Fixes issue 8290.
> 
> 
> 
> Please review this at https://codereview.appspot.com/151730045/
> 
> 
> 
> Affected files (+67, -2 lines):
> 
>    M src/cmd/go/build.go
> 
>    M src/cmd/go/doc.go
> 
>    M src/cmd/go/pkg.go
> 
>    M src/cmd/go/test.bash
> 
>    A src/cmd/go/testgo.go
> 
> 
> 
> 
> 
> Index: src/cmd/go/build.go
> 
> ===================================================================
> 
> --- a/src/cmd/go/build.go
> 
> +++ b/src/cmd/go/build.go
> 
> @@ -57,6 +57,7 @@
> 
> 
> 
>   	-a
> 
>   		force rebuilding of packages that are already up-to-date.
> 
> +		In Go releases, does not apply to the standard library.
> 
>   	-n
> 
>   		print the commands but do not run them.
> 
>   	-p n
> 
> Index: src/cmd/go/doc.go
> 
> ===================================================================
> 
> --- a/src/cmd/go/doc.go
> 
> +++ b/src/cmd/go/doc.go
> 
> @@ -76,6 +76,7 @@
> 
> 
> 
>   	-a
> 
>   		force rebuilding of packages that are already up-to-date.
> 
> +		In Go releases, does not apply to the standard library.
> 
>   	-n
> 
>   		print the commands but do not run them.
> 
>   	-p n
> 
> Index: src/cmd/go/pkg.go
> 
> ===================================================================
> 
> --- a/src/cmd/go/pkg.go
> 
> +++ b/src/cmd/go/pkg.go
> 
> @@ -14,6 +14,7 @@
> 
>   	"os"
> 
>   	pathpkg "path"
> 
>   	"path/filepath"
> 
> +	"runtime"
> 
>   	"sort"
> 
>   	"strings"
> 
>   	"time"
> 
> @@ -685,6 +686,12 @@
> 
>   	}
> 
>   }
> 
> 
> 
> +// The runtime version string takes one of two forms:
> 
> +// "go1.X[.Y]" for Go releases, and "devel +hash" at tip.
> 
> +// Determine whether we are in a released copy by
> 
> +// inspecting the version.
> 
> +var isGoRelease = !strings.HasPrefix(runtime.Version(), "go1")
> 
> +
> 
>   // isStale reports whether package p needs to be rebuilt.
> 
>   func isStale(p *Package, topRoot map[string]bool) bool {
> 
>   	if p.Standard && (p.ImportPath == "unsafe" || buildContext.Compiler  
> 
> == "gccgo") {
> 
> @@ -705,7 +712,16 @@
> 
>   		return false
> 
>   	}
> 
> 
> 
> -	if buildA || p.target == "" || p.Stale {
> 
> +	// If we are running a release copy of Go, do not rebuild the standard  
> 
> packages.
> 
> +	// They may not be writable anyway, but they are certainly not changing.
> 
> +	// This makes 'go build -a' skip the standard packages when using an  
> 
> official release.
> 
> +	// See issue 4106 and issue 8290.
> 
> +	pkgBuildA := buildA
> 
> +	if p.Standard && isGoRelease {
> 
> +		pkgBuildA = false
> 
> +	}
> 
> +
> 
> +	if pkgBuildA || p.target == "" || p.Stale {
> 
>   		return true
> 
>   	}
> 
> 
> 
> Index: src/cmd/go/test.bash
> 
> ===================================================================
> 
> --- a/src/cmd/go/test.bash
> 
> +++ b/src/cmd/go/test.bash
> 
> @@ -4,7 +4,7 @@
> 
>   # license that can be found in the LICENSE file.
> 
> 
> 
>   set -e
> 
> -go build -o testgo
> 
> +go build -tags testgo -o testgo
> 
>   go() {
> 
>   	echo TEST ERROR: ran go, not testgo: go "$@" >&2
> 
>   	exit 2
> 
> @@ -71,6 +71,32 @@
> 
>   fi
> 
>   rm -r $d
> 
> 
> 
> +TEST 'go build -a in dev branch'
> 
> +./testgo install math || ok=false # should be up to date already but just  
> 
> in case
> 
> +d=$(TMPDIR=/var/tmp mktemp -d -t testgoXXX)
> 
> +if ! TESTGO_IS_GO_RELEASE=0 ./testgo build -v -a math 2>$d/err.out; then
> 
> +	cat $d/err.out
> 
> +	ok=false
> 
> +elif ! grep -q runtime $d/err.out; then
> 
> +	echo "testgo build -a math in dev branch DID NOT build runtime, but  
> 
> should have"
> 
> +	cat $d/err.out
> 
> +	ok=false
> 
> +fi
> 
> +rm -r $d
> 
> +
> 
> +TEST 'go build -a in release branch'
> 
> +./testgo install math || ok=false # should be up to date already but just  
> 
> in case
> 
> +d=$(TMPDIR=/var/tmp mktemp -d -t testgoXXX)
> 
> +if ! TESTGO_IS_GO_RELEASE=1 ./testgo build -v -a math 2>$d/err.out; then
> 
> +	cat $d/err.out
> 
> +	ok=false
> 
> +elif grep -q runtime $d/err.out; then
> 
> +	echo "testgo build -a math in dev branch DID build runtime, but should  
> 
> NOT have"
> 
> +	cat $d/err.out
> 
> +	ok=false
> 
> +fi
> 
> +rm -r $d
> 
> +
> 
>   # Test local (./) imports.
> 
>   testlocal() {
> 
>   	local="$1"
> 
> Index: src/cmd/go/testgo.go
> 
> ===================================================================
> 
> new file mode 100644
> 
> --- /dev/null
> 
> +++ b/src/cmd/go/testgo.go
> 
> @@ -0,0 +1,21 @@
> 
> +// Copyright 2014 The Go Authors.  All rights reserved.
> 
> +// Use of this source code is governed by a BSD-style
> 
> +// license that can be found in the LICENSE file.
> 
> +
> 
> +// This file contains extra hooks for testing the go command.
> 
> +// It is compiled into the Go binary only when building the
> 
> +// test copy; it does not get compiled into the standard go
> 
> +// command, so these testing hooks are not present in the
> 
> +// go command that everyone uses.
> 
> +
> 
> +// +build testgo
> 
> +
> 
> +package main
> 
> +
> 
> +import "os"
> 
> +
> 
> +func init() {
> 
> +	if v := os.Getenv("TESTGO_IS_GO_RELEASE"); v != "" {
> 
> +		isGoRelease = v == "1"
> 
> +	}
> 
> +}

[rsc, 2014-09-26 18:41:32]

*** Submitted as https://code.google.com/p/go/source/detail?r=6364b071649c ***

cmd/go: make build -a skip standard packages in Go releases

Today, 'go build -a my/pkg' and 'go install -a my/pkg'
recompile not just my/pkg and all its dependencies that
you wrote but also the standard library packages.
Recompiling the standard library is problematic on
some systems because the installed copy is not writable.

The -a behavior means that you can't use 'go install -a all'
or 'go install -a my/...' to rebuild everything after a Go
release - the rebuild stops early when it cannot overwrite
the installed standard library.

During development work, however, you do want install -a
to rebuild everything, because anything might have changed.

Resolve the conflict by making the behavior of -a depend
on whether we are using a released copy of Go or a devel copy.
In the release copies, -a no longer applies to the standard library.
In the devel copies, it still does.

This is the latest in a long line of refinements to the
"do I build this or not" logic. It is surely not the last.

Fixes issue 8290.

LGTM=r
R=golang-codereviews, r, tracey.brendan
CC=adg, golang-codereviews, iant
https://codereview.appspot.com/151730045

[minux, 2014-09-27 03:36:58]

this seems too magic to me.

consider what if we ask a issue reporter to test a patch? we can't just say
use "go build -a" to rebuild everything.

how about continue on write failures but only when it's writing to GOROOT.
(or compare and fail only when the file actually needs to be updated)? and
display a message at end saying std package installation failed.

the comparing approach depends on we have prefect build reproduciblity,
which is also a good thing.

[gobot, 2014-09-29 12:50:01]

This changed caused perf changes on linux-amd64-perf:


build-1                   old          new      delta
build-cputime     10040626000   1636101000     -83.71
build-time        10359951345   1637070238     -84.20

build-2                   old          new      delta
build-cputime     10068629000   1636102000     -83.75
build-time         7386702699   1622866753     -78.03

build-4                   old          new      delta
build-cputime     10176635000   1656103000     -83.73
build-time         6209324305   1627858363     -73.78

build-8                   old          new      delta
build-cputime     10260640000   1676104000     -83.66
build-time         6095606940   1632273208     -73.22

build-16                  old          new      delta
build-cputime     10552659000   1704106000     -83.85
build-time         6129019418   1633034416     -73.36

http://build.golang.org/perfdetail?commit=6364b071649cb3ce63cbd1df3e60237d0de...



—gobot

[rsc, 2014-09-29 13:43:33]

On Fri, Sep 26, 2014 at 11:36 PM, minux <minux@golang.org> wrote:

> this seems too magic to me.
>
> consider what if we ask a issue reporter to test a patch? we can't just
> say use "go build -a" to rebuild everything.
>
> how about continue on write failures but only when it's writing to GOROOT.
> (or compare and fail only when the file actually needs to be updated)? and
> display a message at end saying std package installation failed.
>
> the comparing approach depends on we have prefect build reproduciblity,
> which is also a good thing.
>
that's even more magical. fwiw, i can't remember ever asking someone to
apply a patch to the release branch to test.

[minux, 2014-09-29 20:40:09]

On Sep 29, 2014 9:43 AM, "Russ Cox" <rsc@golang.org> wrote:
>
> On Fri, Sep 26, 2014 at 11:36 PM, minux <minux@golang.org> wrote:
>>
>> this seems too magic to me.
>>
>> consider what if we ask a issue reporter to test a patch? we can't just
say use "go build -a" to rebuild everything.
>>
>> how about continue on write failures but only when it's writing to
GOROOT.
>> (or compare and fail only when the file actually needs to be updated)?
and display a message at end saying std package installation failed.
>>
>> the comparing approach depends on we have prefect build reproduciblity,
which is also a good thing.
>
> that's even more magical. fwiw, i can't remember ever asking someone to
apply a patch to the release branch to test.
i still think making the release branch lool as similar as possible to the
development branch is very desirable. And with my proposed behavior you
will at least get a warning if the go tool is not doing what it will
normally do.

the current behavior is silent so the user has to remember more.

[costinc, 2014-09-30 10:39:52]

Won't this affect cross compiling? Until now I've used
"go install -a" to rebuild std for target goos/goarch,
with release versions of go installed in my home dir.

[tianon, 2014-12-02 09:27:00]

On 2014/09/26 18:41:32, rsc wrote:
> *** Submitted as https://code.google.com/p/go/source/detail?r=6364b071649c ***
> 
> cmd/go: make build -a skip standard packages in Go releases
> 
> Today, 'go build -a my/pkg' and 'go install -a my/pkg'
> recompile not just my/pkg and all its dependencies that
> you wrote but also the standard library packages.
> Recompiling the standard library is problematic on
> some systems because the installed copy is not writable.
> 
> The -a behavior means that you can't use 'go install -a all'
> or 'go install -a my/...' to rebuild everything after a Go
> release - the rebuild stops early when it cannot overwrite
> the installed standard library.
> 
> During development work, however, you do want install -a
> to rebuild everything, because anything might have changed.
> 
> Resolve the conflict by making the behavior of -a depend
> on whether we are using a released copy of Go or a devel copy.
> In the release copies, -a no longer applies to the standard library.
> In the devel copies, it still does.
> 
> This is the latest in a long line of refinements to the
> "do I build this or not" logic. It is surely not the last.
> 
> Fixes issue 8290.
> 
> LGTM=r
> R=golang-codereviews, r, tracey.brendan
> CC=adg, golang-codereviews, iant
> https://codereview.appspot.com/151730045

This breaks the ability to use "netgo" with an official source release, as I've
outlined at https://github.com/docker/docker/issues/9449 (in testing building
Docker against the 1.4rc2 release).  Is there an official method we should be
using instead of "go build -a -tags netgo" in order to get netgo?  The only
workaround I can figure is to patch "make.bash" to include "-tags netgo" when
compiling the standard library, but that seems really extreme, especially since
that's not something we could include in an image for general consumption very
easily (so everyone who needs "netgo" will have to patch and recompile, or we'll
have to force everyone to use "netgo", which is also definitely undesirable).

[minux, 2014-12-02 10:10:17]

On Tue, Dec 2, 2014 at 4:27 AM, <admwiggin@gmail.com> wrote:
>
> This breaks the ability to use "netgo" with an official source release,
> as I've outlined at https://github.com/docker/docker/issues/9449 (in
> testing building Docker against the 1.4rc2 release).  Is there an
> official method we should be using instead of "go build -a -tags netgo"
> in order to get netgo?  The only workaround I can figure is to patch
> "make.bash" to include "-tags netgo" when compiling the standard
> library, but that seems really extreme, especially since that's not
> something we could include in an image for general consumption very
> easily (so everyone who needs "netgo" will have to patch and recompile,
> or we'll have to force everyone to use "netgo", which is also definitely
> undesirable).
>

Have you tried the following 2-step process?
go install -a -tags netgo net # just reinstall net
go install -v std # no -a

[tianon, 2014-12-02 16:31:52]

On 2 December 2014 at 03:09, minux <minux@golang.org> wrote:
> Have you tried the following 2-step process?
> go install -a -tags netgo net # just reinstall net
> go install -v std # no -a

Gave that a shot, and still no dice.  I added "-v" to the first
command so that I could see what packages it actually compiles, and I
don't get any output from either command on 1.4rc2 (and I still get
the same error with linking in net).

♥,
- Tianon
  4096R / B42F 6819 007F 00F8 8E36  4FD4 036A 9C25 BF35 7DD4

[minux, 2014-12-02 23:00:25]

On Tue, Dec 2, 2014 at 11:31 AM, Tianon Gravi <admwiggin@gmail.com> wrote:

> On 2 December 2014 at 03:09, minux <minux@golang.org> wrote:
> > Have you tried the following 2-step process?
> > go install -a -tags netgo net # just reinstall net
> > go install -v std # no -a
>
> Gave that a shot, and still no dice.  I added "-v" to the first
> command so that I could see what packages it actually compiles, and I
> don't get any output from either command on 1.4rc2 (and I still get
> the same error with linking in net).
>
OK. Leave for rsc.