code review 93580043: cmd/dist, cmd/go, cmd/cgo: allow $CC_FOR_TARGET and $CX...

cmd/dist, cmd/go, cmd/cgo: allow $CC_FOR_TARGET and $CXX_FOR_TARGET to be differnet for each target

Currently, building Go with a value for $CC_FOR_TARGET and $CXX_FOR_TARGET will overwrite whatever value of defaultCC and defaultCXX was previously there, causing packages to fail when attempting to compile them for a target other than the last one built for.

This change changes defaultCC and defaultCXX to functions that look up a map[string]string. The keys are GOOS/GOARCH; the values are the expected value of $CC_FOR_TARGET/$CXX_FOR_TARGET.

The zdefaultcc.go file is generated with the help of two new z-files in src/cmd/go that contain the maps's values: defaultccs and defaultcxxs. Each of these files consists of lines of the form
        d[`GOOS/GOARCH`] = `command`
These are put into functions and new compiles add to the end of each file. The intention of this design is that if recompiling a target with a different $CC_FOR_TARGET, the new value will replace the old target without us having to go through the defaultccs file each time.

The two new z-files are also cleaned properly.

Tested on linux/amd64 with successful cross-compilation sets for linux/amd64 and windows/386.

[Pietro Gagliardi (andlabs), 2014-05-24 21:54:04]

Hello golang-codereviews@googlegroups.com,

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

[Pietro Gagliardi (andlabs), 2014-05-25 07:10:53]

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

Please take another look.

[Pietro Gagliardi (andlabs), 2014-05-25 07:11:56]

On 2014/05/25 07:10:53, Pietro Gagliardi (andlabs) wrote:
> Hello mailto:golang-codereviews@googlegroups.com (cc:
> mailto:golang-codereviews@googlegroups.com),
> 
> Please take another look.

This update is to fix a bug I didn't notice before: it was only using
runtime.GOOS/runtime.GOARCH at runtime, not the values of $GOOS/$GOARCH, as cgo
uses elsewhere.

[r, 2014-05-27 18:25:27]

This change is too invasive to happen so close to a release. Please ping us once
1.3 is out.

[gobot, 2014-06-16 20:10:27]

R=iant@golang.org (assigned by r@golang.org)

[Pietro Gagliardi (andlabs), 2014-06-19 13:19:33]

Hello golang-codereviews@googlegroups.com, gobot@golang.org, iant@golang.org
(cc: golang-codereviews@googlegroups.com),

Please take another look.

[iant, 2014-06-19 16:49:20]

There is a typo in the first line of the CL description.

[iant, 2014-06-19 16:57:04]

I'm not comfortable with the idea that the result of the build depends on
previous builds.  I think it's important that each build be easily repeatable. 
Permitting builds to depend on previous builds opens up the possibility of bugs
that are extremely hard to track down because they are impossible to replicate. 
I don't want to go there.

Let's figure out some other way to do this, though I admit I have no
suggestions.

https://codereview.appspot.com/93580043/diff/50001/src/cmd/dist/build.c
File src/cmd/dist/build.c (right):

https://codereview.appspot.com/93580043/diff/50001/src/cmd/dist/build.c#newco...
src/cmd/dist/build.c:1568: char *targetgoarch, *targetgoos;
Move this up to the list of variables at the top of the file.  Put them on
separate lines.

[minux, 2014-06-19 17:20:44]

On Thu, Jun 19, 2014 at 12:57 PM, <iant@golang.org> wrote:

> I'm not comfortable with the idea that the result of the build depends
> on previous builds.  I think it's important that each build be easily
> repeatable.  Permitting builds to depend on previous builds opens up the
> possibility of bugs that are extremely hard to track down because they
> are impossible to replicate.  I don't want to go there.
>
I think the previous build's only relevant when the user explicitly
requested by
make.bash --no-clean.
and all.bash will do dist bootstrap -a -v so the previous results should be
cleaned.

[iant, 2014-06-19 17:28:02]

On Thu, Jun 19, 2014 at 10:20 AM, minux <minux@golang.org> wrote:
>
> On Thu, Jun 19, 2014 at 12:57 PM, <iant@golang.org> wrote:
>>
>> I'm not comfortable with the idea that the result of the build depends
>> on previous builds.  I think it's important that each build be easily
>> repeatable.  Permitting builds to depend on previous builds opens up the
>> possibility of bugs that are extremely hard to track down because they
>> are impossible to replicate.  I don't want to go there.
>
> I think the previous build's only relevant when the user explicitly
> requested by
> make.bash --no-clean.
> and all.bash will do dist bootstrap -a -v so the previous results should be
> cleaned.

Even with --no-clean, do we have any other case where a previous build
affects the current build?  Not just in terms of what is installed,
but in terms of what is actually compiled?

Ian

[Pietro Gagliardi (andlabs), 2014-06-19 17:37:24]

On 2014/06/19 17:28:02, iant wrote:
> On Thu, Jun 19, 2014 at 10:20 AM, minux <mailto:minux@golang.org> wrote:
> >
> > On Thu, Jun 19, 2014 at 12:57 PM, <mailto:iant@golang.org> wrote:
> >>
> >> I'm not comfortable with the idea that the result of the build depends
> >> on previous builds.  I think it's important that each build be easily
> >> repeatable.  Permitting builds to depend on previous builds opens up the
> >> possibility of bugs that are extremely hard to track down because they
> >> are impossible to replicate.  I don't want to go there.
> >
> > I think the previous build's only relevant when the user explicitly
> > requested by
> > make.bash --no-clean.
> > and all.bash will do dist bootstrap -a -v so the previous results should be
> > cleaned.
> 
> Even with --no-clean, do we have any other case where a previous build
> affects the current build?  Not just in terms of what is installed,
> but in terms of what is actually compiled?
> 
> Ian

The only other thing I can think of is to
1) make defaultCC/defaultCXX the host $CC/$CXX only, only dropped into
go/zdefaults.go when building the host pkg/runtime
2) replace $CC_FOR_TARGET/$CXX_FOR_TARGET with just $CC/$CX and honor $CC and
$CXX at both Go compile-time and program compile-time
3) (maybe?) introduce $HOSTCC and $HOSTCXX for to override the host $CC/$CXX
stored in defaultCC and defaultCXX (but only if this is really necessary...)
This would violate 1.3 compatibility nowadays, but it seems like a better
solution (and it mimics the behavior of the $GOOS family of variables).

[iant, 2014-06-19 18:02:25]

On Thu, Jun 19, 2014 at 10:37 AM,  <pietro10@mac.com> wrote:
> On 2014/06/19 17:28:02, iant wrote:
>>
>> On Thu, Jun 19, 2014 at 10:20 AM, minux <mailto:minux@golang.org>
>
> wrote:
>
>> >
>> > On Thu, Jun 19, 2014 at 12:57 PM, <mailto:iant@golang.org> wrote:
>> >>
>> >> I'm not comfortable with the idea that the result of the build
>
> depends
>>
>> >> on previous builds.  I think it's important that each build be
>
> easily
>>
>> >> repeatable.  Permitting builds to depend on previous builds opens
>
> up the
>>
>> >> possibility of bugs that are extremely hard to track down because
>
> they
>>
>> >> are impossible to replicate.  I don't want to go there.
>> >
>> > I think the previous build's only relevant when the user explicitly
>> > requested by
>> > make.bash --no-clean.
>> > and all.bash will do dist bootstrap -a -v so the previous results
>
> should be
>>
>> > cleaned.
>
>
>> Even with --no-clean, do we have any other case where a previous build
>> affects the current build?  Not just in terms of what is installed,
>> but in terms of what is actually compiled?
>
>
>> Ian
>
>
> The only other thing I can think of is to
> 1) make defaultCC/defaultCXX the host $CC/$CXX only, only dropped into
> go/zdefaults.go when building the host pkg/runtime
> 2) replace $CC_FOR_TARGET/$CXX_FOR_TARGET with just $CC/$CX and honor
> $CC and $CXX at both Go compile-time and program compile-time
> 3) (maybe?) introduce $HOSTCC and $HOSTCXX for to override the host
> $CC/$CXX stored in defaultCC and defaultCXX (but only if this is really
> necessary...)
> This would violate 1.3 compatibility nowadays, but it seems like a
> better solution (and it mimics the behavior of the $GOOS family of
> variables).

It seems to me that for each GOHOST/GOOS combo we have a different set
of tools--that is a different Xg, Xl, etc.  Perhaps we could store the
default non-Go compilers to use in some file under $GOROOT/pkg/tool?

Ian

[Pietro Gagliardi (andlabs), 2014-07-05 14:03:42]

On 2014/06/19 18:02:25, iant wrote:
> On Thu, Jun 19, 2014 at 10:37 AM,  <mailto:pietro10@mac.com> wrote:
> > On 2014/06/19 17:28:02, iant wrote:
> >>
> >> On Thu, Jun 19, 2014 at 10:20 AM, minux <mailto:minux@golang.org>
> >
> > wrote:
> >
> >> >
> >> > On Thu, Jun 19, 2014 at 12:57 PM, <mailto:iant@golang.org> wrote:
> >> >>
> >> >> I'm not comfortable with the idea that the result of the build
> >
> > depends
> >>
> >> >> on previous builds.  I think it's important that each build be
> >
> > easily
> >>
> >> >> repeatable.  Permitting builds to depend on previous builds opens
> >
> > up the
> >>
> >> >> possibility of bugs that are extremely hard to track down because
> >
> > they
> >>
> >> >> are impossible to replicate.  I don't want to go there.
> >> >
> >> > I think the previous build's only relevant when the user explicitly
> >> > requested by
> >> > make.bash --no-clean.
> >> > and all.bash will do dist bootstrap -a -v so the previous results
> >
> > should be
> >>
> >> > cleaned.
> >
> >
> >> Even with --no-clean, do we have any other case where a previous build
> >> affects the current build?  Not just in terms of what is installed,
> >> but in terms of what is actually compiled?
> >
> >
> >> Ian
> >
> >
> > The only other thing I can think of is to
> > 1) make defaultCC/defaultCXX the host $CC/$CXX only, only dropped into
> > go/zdefaults.go when building the host pkg/runtime
> > 2) replace $CC_FOR_TARGET/$CXX_FOR_TARGET with just $CC/$CX and honor
> > $CC and $CXX at both Go compile-time and program compile-time
> > 3) (maybe?) introduce $HOSTCC and $HOSTCXX for to override the host
> > $CC/$CXX stored in defaultCC and defaultCXX (but only if this is really
> > necessary...)
> > This would violate 1.3 compatibility nowadays, but it seems like a
> > better solution (and it mimics the behavior of the $GOOS family of
> > variables).
> 
> It seems to me that for each GOHOST/GOOS combo we have a different set
> of tools--that is a different Xg, Xl, etc.  Perhaps we could store the
> default non-Go compilers to use in some file under $GOROOT/pkg/tool?
> 
> Ian

That works too.

[Pietro Gagliardi (andlabs), 2014-07-11 01:01:53]

I have this new setup implemented, this time on a clean go tip, so I have
submitted it as CL 111060046 to keep things sane.

[gobot, 2014-12-19 05:19:11]

R=close

To the author of this CL:

The Go project has moved to Gerrit Code Review.

If this CL should be continued, please see the latest version of
https://golang.org/doc/contribute.html for instructions on
how to set up Git and the Go project's Gerrit codereview plugin,
and then create a new change with your current code.

If there has been discussion on this CL, please give a link to it
(golang.org/cl/93580043 is best) in the description in your
new CL.

Thanks very much.