code review 5504062: build: fixes for Windows

build: fixes for Windows

* work around a linker/cgo bug
* do not run deps.bash on Windows unless we need it
  (cuts a full minute off the build time)
* add windows to the list of cgo-enabled targets

The gopack problem is issue 2601.

[rsc, 2011-12-21 20:46:12]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/

[r, 2011-12-21 20:48:38]

LGTM

http://codereview.appspot.com/5504062/diff/2001/src/cmd/go/build.go
File src/cmd/go/build.go (right):

http://codereview.appspot.com/5504062/diff/2001/src/cmd/go/build.go#newcode583
src/cmd/go/build.go:583: // not sure I want to know. 
http://golang.org/issue/2601
take out the editorializing by s/,.*know/./

[bradfitz, 2011-12-21 20:51:22]

Remove the println debugging?

On Wed, Dec 21, 2011 at 12:46 PM, <rsc@golang.org> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> build: fixes for Windows
>
> * work around a gopack bug
> * do not run deps.bash on Windows unless we need it
>  (cuts a full minute off the build time)
> * add windows to the list of cgo-enabled targets
>
> The gopack problem is issue 2601.
>
> Please review this at
http://codereview.appspot.com/**5504062/<http://codereview.appspot.com/5504062/>
>
> Affected files:
>  M src/cmd/go/build.go
>  M src/make.bash
>  M src/pkg/go/build/dir.go
>
>
> Index: src/cmd/go/build.go
> ==============================**==============================**=======
> --- a/src/cmd/go/build.go
> +++ b/src/cmd/go/build.go
> @@ -452,7 +452,7 @@
>                return err
>        }
>
> -       var gofiles, cfiles, sfiles, objects []string
> +       var gofiles, cfiles, sfiles, objects, cgoObjects []string
>        gofiles = append(gofiles, a.p.GoFiles...)
>        cfiles = append(cfiles, a.p.CFiles...)
>        sfiles = append(sfiles, a.p.SFiles...)
> @@ -464,6 +464,7 @@
>                // cgo and non-cgo worlds, so it necessarily has files in
> both.
>                // In that case gcc only gets the gcc_* files.
>                var gccfiles []string
> +               println("std?", a.p.Standard, a.p.ImportPath)
>                if a.p.Standard && a.p.ImportPath == "runtime/cgo" {
>                        filter := func(files, nongcc, gcc []string)
> ([]string, []string) {
>                                for _, f := range files {
> @@ -487,7 +488,7 @@
>                if err != nil {
>                        return err
>                }
> -               objects = append(objects, outObj...)
> +               cgoObjects = append(cgoObjects, outObj...)
>                gofiles = append(gofiles, outGo...)
>        }
>
> @@ -576,6 +577,12 @@
>                objects = append(objects, out)
>        }
>
> +       // NOTE(rsc): On Windows, it is critically important that the
> +       // gcc-compiled objects (cgoObjects) be listed after the ordinary
> +       // objects in the archive.  I do not know why this is, and I am
> +       // not sure I want to know.  http://golang.org/issue/2601
> +       objects = append(objects, cgoObjects...)
> +
>        // pack into archive in obj directory
>        if err := b.gopack(a.p, obj, a.objpkg, objects); err != nil {
>                return err
> @@ -917,6 +924,8 @@
>                return nil, nil, errors.New("cannot use cgo when compiling
> for a different operating system")
>        }
>
> +       outObj = append(outObj, "") // for importObj, at end of function
> +
>        // cgo
>        // TODO: CGOPKGPATH, CGO_FLAGS?
>        gofiles := []string{obj + "_cgo_gotypes.go"}
> @@ -983,7 +992,11 @@
>        if err := b.cc(p, obj, importObj, importC); err != nil {
>                return nil, nil, err
>        }
> -       outObj = append(outObj, importObj)
> +
> +       // NOTE(rsc): The importObj is a 5c/6c/8c object and on Windows
> +       // must be processed before the gcc-generated objects.
> +       // Put it first.  We left room above.
> http://golang.org/issue/2601
> +       outObj[0] = importObj
>
>        return outGo, outObj, nil
>  }
> Index: src/make.bash
> ==============================**==============================**=======
> --- a/src/make.bash
> +++ b/src/make.bash
> @@ -71,6 +71,7 @@
>        fi
>  done
>
> +$USE_GO_TOOL ||
>  (
>        cd "$GOROOT"/src/pkg;
>        bash deps.bash  # do this here so clean.bash will work in the pkg
> directory
> Index: src/pkg/go/build/dir.go
> ==============================**==============================**=======
> --- a/src/pkg/go/build/dir.go
> +++ b/src/pkg/go/build/dir.go
> @@ -84,6 +84,8 @@
>        "linux/amd64":   true,
>        "freebsd/386":   true,
>        "freebsd/amd64": true,
> +       "windows/386":   true,
> +       "windows/amd64": true,
>  }
>
>  func defaultContext() Context {
>
>
>

[rsc, 2011-12-21 20:52:01]

On Wed, Dec 21, 2011 at 15:51, Brad Fitzpatrick <bradfitz@golang.org> wrote:
> Remove the println debugging?

just did that

[rsc, 2011-12-21 20:57:52]

*** Submitted as http://code.google.com/p/go/source/detail?r=f31b4b5afe43 ***

build: fixes for Windows

* work around a linker/cgo bug
* do not run deps.bash on Windows unless we need it
  (cuts a full minute off the build time)
* add windows to the list of cgo-enabled targets

The gopack problem is issue 2601.

R=golang-dev, r, bradfitz
CC=golang-dev
http://codereview.appspot.com/5504062