code review 5661054: build: keep environment variable changes local during W...

build: keep environment variable changes local during Windows build

[rsc, 2012-02-14 04:37:37]

Hello golang-dev@googlegroups.com,

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

[rsc, 2012-02-14 04:38:03]

Completely untested.  Am I misreading the docs?

[brainman, 2012-02-14 04:42:42]

On 2012/02/14 04:38:03, rsc wrote:
> Completely untested.  Am I misreading the docs?

I will play with it here. I never use batch files, so it is all new for me, but
it sounds like what we need. Feel free to submit.

Alex

[rsc, 2012-02-14 04:44:00]

On Mon, Feb 13, 2012 at 23:42,  <alex.brainman@gmail.com> wrote:
> I will play with it here. I never use batch files, so it is all new for

Can you hg clpatch it in and play with it?  If it looks good, submit it.
I've restarted both builders with cleaner environments; maybe that
will help.

I am a little skeptical, because it looks like the builder sets up
a pretty clean environment to begin with.

Russ

[brainman, 2012-02-14 04:58:31]

It builds, but testing fails:

runtime/cgo
testing
testing/iotest
testing/quick
.
# Testing packages.
'go' is not recognized as an internal or external command,
operable program or batch file.

go is, probably, not in the PATH anymore.

Alex

[brainman, 2012-02-14 04:59:48]

Mind you, setlocal did work, my caller cmd.exe environment didn't change.

Alex

[rsc, 2012-02-14 05:06:35]

On Mon, Feb 13, 2012 at 23:58,  <alex.brainman@gmail.com> wrote:
> It builds, but testing fails:
>
> runtime/cgo
> testing
> testing/iotest
> testing/quick
> .
> # Testing packages.
> 'go' is not recognized as an internal or external command,
> operable program or batch file.
>
> go is, probably, not in the PATH anymore.

Indeed.  The --no-local argument in all.bat
is supposed to tell make.bat not to use
setlocal/endlocal, so that the vars will still
be there for run.bat to see, but maybe I got
that check wrong.

[brainman, 2012-02-14 05:08:11]

I am more and more confused :-).

You have

eval $(go tool dist env)

in run.bash, but how does builder knows where to find go executable.

Alex

[brainman, 2012-02-14 05:11:00]

And I still do not understand how is it possible for amd64 to work now, but 386
builder still fails with

# runtime/cgo
C:\Users\ADMINI~1\AppData\Local\Temp\2\go-build197556978\runtime\cgo\_obj\_cgo_gotypes.go:7:
import C:\go\pkg\windows_386/syscall.a: object is [windows 386 weekly.2012-02-07
+9d7addec2635 X:none] expected [windows 386 weekly.2012-02-07 +1cd9ea014a9e
X:none]

Alex

[rsc, 2012-02-14 05:11:46]

On Tue, Feb 14, 2012 at 00:08,  <alex.brainman@gmail.com> wrote:
> I am more and more confused :-).
>
> You have
>
> eval $(go tool dist env)
>
> in run.bash, but how does builder knows where to find go executable.

all.bash runs make.bash using . ./make.bash
meaning that it runs as part of all.bash's shell.
So the environment variables set in make.bash
propagate up to all.bash and then back down
into run.bash.

I'm trying to do the same thing on Windows.

Russ

[rsc, 2012-02-14 05:14:22]

On Tue, Feb 14, 2012 at 00:11,  <alex.brainman@gmail.com> wrote:
> And I still do not understand how is it possible for amd64 to work now,
> but 386 builder still fails with
>
> # runtime/cgo
>
C:\Users\ADMINI~1\AppData\Local\Temp\2\go-build197556978\runtime\cgo\_obj\_cgo_gotypes.go:7:
> import C:\go\pkg\windows_386/syscall.a: object is [windows 386
> weekly.2012-02-07 +9d7addec2635 X:none] expected [windows 386
> weekly.2012-02-07 +1cd9ea014a9e X:none]

I'm pretty sure this is the 386 builder being run with stale
environment variables left over from the build of the builder.
I tried clearing a few but it obviously didn't help.
It did help on amd64 though.  I just restarted the 386 builder
in a brand new cmd.exe window.

Russ

[brainman, 2012-02-14 05:16:29]

Oh. That should do it. I think you are pretty close. Maybe leave it for another
day. We have one builder going. I will have a go too.

Alex

[brainman, 2012-02-14 05:24:29]

On 2012/02/14 05:11:46, rsc wrote:
> 
> I'm trying to do the same thing on Windows.
> 

It looks like you won't be able to use setlocal to achive that. From
http://www.computerhope.com/setlocal.htm:

"... When the end of a batch script is reached, an implied ENDLOCAL is executed
for any outstanding SETLOCAL commands issued by that batch script. ..."

Alex

[rsc, 2012-02-14 05:36:03]

On Tue, Feb 14, 2012 at 00:24,  <alex.brainman@gmail.com> wrote:
> "... When the end of a batch script is reached, an implied ENDLOCAL is
> executed for any outstanding SETLOCAL commands issued by that batch
> script. ..."

That should be okay, since I am not issuing setlocal in make.bat
(at least I am trying to goto around it).  But it's nice to know I don't
have to call endlocal explicitly.

[brainman, 2012-02-15 01:59:29]

http://codereview.appspot.com/5661054/diff/2004/src/make.bat
File src/make.bat (right):

http://codereview.appspot.com/5661054/diff/2004/src/make.bat#newcode15
src/make.bat:15: if x%1==--no-local goto nolocal
s/==-/==x-/
this is your problem.

But I made more small changes and simplifications:
http://codereview.appspot.com/5673048/

I propose you drop your CL and we use mine.