code review 128230046: cmd/dist: goc2c ignores GOROOT_FINAL

cmd/dist: goc2c ignores GOROOT_FINAL

When building golang, the environment variable GOROOT_FINAL can be set
to indicate a different installation location from the build
location. This works fine, except that the goc2c build step embeds
line numbers in the resulting c source files that refer to the build
location, no the install location.

This would not be a big deal, except that in turn the linker uses the
location of runtime/string.goc to embed the gdb script in the
resulting binary and as a net result, the debugger now complains that
the script is outside its load path (it has the install location
configured).

See https://code.google.com/p/go/issues/detail?id=8524 for the full
description.

Fixes issue 8524.

[hgschmie, 2014-08-14 17:38:53]

Hello golang-codereviews@googlegroups.com,

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

[iant, 2014-08-15 14:02:15]

https://codereview.appspot.com/128230046/diff/40001/src/cmd/dist/goc2c.c
File src/cmd/dist/goc2c.c (right):

https://codereview.appspot.com/128230046/diff/40001/src/cmd/dist/goc2c.c#newc...
src/cmd/dist/goc2c.c:783: file = goc_final;
This means that any errors during the build will be misattributed to
GOROOT_FINAL/path rather than the source path.  That will be confusing for Go
developers.  I think you need to separate the file used in error messages from
the file used in #line commands.

[hgschmie, 2014-08-15 18:49:02]

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

Please take another look.

[hgschmie, 2014-08-15 18:50:46]

https://codereview.appspot.com/128230046/diff/40001/src/cmd/dist/goc2c.c
File src/cmd/dist/goc2c.c (right):

https://codereview.appspot.com/128230046/diff/40001/src/cmd/dist/goc2c.c#newc...
src/cmd/dist/goc2c.c:783: file = goc_final;
Fixed that in #5.

[iant, 2014-08-15 19:03:08]

LGTM

[iant, 2014-08-15 19:03:54]

Have you signed the CLA as described at
http://golang.org/doc/contribute.html#copyright ?

[hgschmie, 2014-08-15 22:12:52]

On 2014/08/15 19:03:54, iant wrote:
> Have you signed the CLA as described at
> http://golang.org/doc/contribute.html#copyright ?

Yes, guess it is still processing. I did the "I AGREE" online form.

[iant, 2014-08-15 22:19:04]

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

cmd/dist: goc2c ignores GOROOT_FINAL

When building golang, the environment variable GOROOT_FINAL can be set
to indicate a different installation location from the build
location. This works fine, except that the goc2c build step embeds
line numbers in the resulting c source files that refer to the build
location, no the install location.

This would not be a big deal, except that in turn the linker uses the
location of runtime/string.goc to embed the gdb script in the
resulting binary and as a net result, the debugger now complains that
the script is outside its load path (it has the install location
configured).

See https://code.google.com/p/go/issues/detail?id=8524 for the full
description.

Fixes issue 8524.

LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://codereview.appspot.com/128230046

Committer: Ian Lance Taylor <iant@golang.org>

[gobot, 2014-08-15 23:38:32]

This CL appears to have broken the windows-386 builder.
See http://build.golang.org/log/836e83f493d3400c324d5102bbf45840a7c23136

[iant, 2014-08-16 00:43:00]

On Fri, Aug 15, 2014 at 4:38 PM,  <gobot@golang.org> wrote:
> This CL appears to have broken the windows-386 builder.
> See http://build.golang.org/log/836e83f493d3400c324d5102bbf45840a7c23136
>
> https://codereview.appspot.com/128230046/

Not real failures.  All timeouts.

Ian