705403 - Sendchanges [on windows] from build steps are being done from old buildbot version

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

[Justin Wood (:Callek)]

While investigating more breakage from SeaMonkey new slaves I noticed that since my new slaves never installed buildbot 0.8.2 into mozilla-build we are failing the sendchange step on our debug builders (opt for us doesn't do that)

This is causing many tests not to be run for us. This has to do with the fact that we currently expect "buildbot" command to be in path when we get here, but since its in "buildbotve" but not the actual run environ of the slave its not.

This blocks Bug 594474 because when we install a new mozilla-build version we will blow away the buildbot 0.8.2 we currently have installed there, and would cause this to fail.

Possible Suggestions:
# Modify path so we can find the buildbot binary in buildbotve
# Modify the build step to point at the buildbot binary explicitly
# Modify the build step to have a cwd of the buildbotve when we execute this.
# Other?

SeaMonkey is going to work around this by appending the ve environ to our machine paths for now.

Comment 1

[Chris Cooper [:coop] (he/him)]

(In reply to Justin Wood (:Callek) from comment #0)
> While investigating more breakage from SeaMonkey new slaves I noticed that
> since my new slaves never installed buildbot 0.8.2 into mozilla-build we are
> failing the sendchange step on our debug builders (opt for us doesn't do
> that)

When you say "opt for us doesn't do that," is opt not trying to do a sendchange at all, or it is issuing a sendchange successfully?
 
> This is causing many tests not to be run for us. This has to do with the
> fact that we currently expect "buildbot" command to be in path when we get
> here, but since its in "buildbotve" but not the actual run environ of the
> slave its not.

Would installing 0.8.2 (or hard-/sym-linking the existing version) to where the command is expecting to find buildbot solve the problem?
 
> This blocks Bug 594474 because when we install a new mozilla-build version
> we will blow away the buildbot 0.8.2 we currently have installed there, and
> would cause this to fail.

Part of bug 594474 will be ensuring that any packages that need to be re-installed after deploying a new version of MozillaBuild are re-installed, and conversely making sure that any redundant packages are removed and ref platforms updated.

> Possible Suggestions:
> # Modify path so we can find the buildbot binary in buildbotve
> # Modify the build step to point at the buildbot binary explicitly
> # Modify the build step to have a cwd of the buildbotve when we execute this.
> # Other?
> 
> SeaMonkey is going to work around this by appending the ve environ to our
> machine paths for now.

Is there anything for releng to do here, or is this strictly a SeaMonkey issue?

Comment 2

[Justin Wood (:Callek)]

(In reply to Chris Cooper [:coop] from comment #1)
> (In reply to Justin Wood (:Callek) from comment #0)
> > While investigating more breakage from SeaMonkey new slaves I noticed that
> > since my new slaves never installed buildbot 0.8.2 into mozilla-build we are
> > failing the sendchange step on our debug builders (opt for us doesn't do
> > that)
> 
> When you say "opt for us doesn't do that," is opt not trying to do a
> sendchange at all, or it is issuing a sendchange successfully?

I mean that opt SeaMonkey doesn't do these sendchanges at all.

> > This is causing many tests not to be run for us. This has to do with the
> > fact that we currently expect "buildbot" command to be in path when we get
> > here, but since its in "buildbotve" but not the actual run environ of the
> > slave its not.
> 
> Would installing 0.8.2 (or hard-/sym-linking the existing version) to where
> the command is expecting to find buildbot solve the problem?

Installing 0.8.2 would allow us to find buildbot 0.8.2, but it would be overkill and a version mismatch from what the slave is actually using of a buildbot version and imo, a bad idea.

Doing a symlink to the 0.8.4-pre (or whatever) version dir would solve it as well, but windows doesn't do those [easily] and is easier to confuse everyone.

> > This blocks Bug 594474 because when we install a new mozilla-build version
> > we will blow away the buildbot 0.8.2 we currently have installed there, and
> > would cause this to fail.
> 
> Part of bug 594474 will be ensuring that any packages that need to be
> re-installed after deploying a new version of MozillaBuild are re-installed,
> and conversely making sure that any redundant packages are removed and ref
> platforms updated.

Of course, but this bug as it stands is part of that "We need to track"

> > Possible Suggestions:
> > # Modify path so we can find the buildbot binary in buildbotve
> > # Modify the build step to point at the buildbot binary explicitly
> > # Modify the build step to have a cwd of the buildbotve when we execute this.
> > # Other?
> > 
> > SeaMonkey is going to work around this by appending the ve environ to our
> > machine paths for now.
> 
> Is there anything for releng to do here, or is this strictly a SeaMonkey
> issue?

I would suggest that solution for RelEng as well, *or* we could specify in buildbot-configs where to find the buildbot binary/script and use it on the slaves for the SendChange.

Basically there are a few solutions here, but in the short term its not a problem for Releng, but once any windows machines no longer have buildbot 0.8.2 installed it will be unless we fix this bug via one of the many potential solutions.

Comment 3

[Chris Cooper [:coop] (he/him)]

(In reply to Justin Wood (:Callek) from comment #2) 
> Basically there are a few solutions here, but in the short term its not a
> problem for Releng, but once any windows machines no longer have buildbot
> 0.8.2 installed it will be unless we fix this bug via one of the many
> potential solutions.

So, to sum up: this is working for Firefox now, but essentially by accident.

We should be more deliberate about where we can find the buildbot binary, yes. I think we'll likely go with modifying the path to include buildbotve also.

Comment 4

[Justin Wood (:Callek)]

(In reply to Chris Cooper [:coop] from comment #3)
> (In reply to Justin Wood (:Callek) from comment #2) 
> > Basically there are a few solutions here, but in the short term its not a
> > problem for Releng, but once any windows machines no longer have buildbot
> > 0.8.2 installed it will be unless we fix this bug via one of the many
> > potential solutions.
> 
> So, to sum up: this is working for Firefox now, but essentially by accident.
> 

Correct.

And to just be clear, if Firefox has any new windows build slaves brought up from scratch (I forget where your refimage snapshot stands on this) you'll hit this as well.

Comment 5

[Chris Cooper [:coop] (he/him)]

Looks like this is causing some issues in bug 708288, so I might as well fix it here.

Comment 6

[Chris Cooper [:coop] (he/him)]

Attachment: Add buildbotve to PATH

Comment 7

[Chris Cooper [:coop] (he/him)]

Attachment: Add buildbotve to PATH (buildbot-configs)

And here's the companion patch that adds buildbotve to the PATH in the config file for builders.

Comment 8

[Justin Wood (:Callek)]

Comment on attachment 581802 [details] [diff] [review]
Add buildbotve to PATH (buildbot-configs)

Has this change been tested, it doesn't look like it would pass:
Justin@ORION /d/sources/comm-central
$ export PATH="MAYBE_FAIL;"

Justin@ORION /d/sources/comm-central
$ echo $PATH
MAYBE_FAIL;

Justin@ORION /d/sources/comm-central
$ /d/mozilla-build/python/python
Python 2.6.5 (r265:79096, Mar 19 2010, 21:48:26) [MSC v.1500 32 bit (Intel)] on
win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import os
>>> os.environ["PATH"]
'MAYBE_FAIL;'
>>> os.environ["PATH"] = "MAYBE_PASS;${PATH}"
>>> os.environ["PATH"]
'MAYBE_PASS;${PATH}'
>>> exit()

Comment 9

[Chris Cooper [:coop] (he/him)]

(In reply to Justin Wood (:Callek) from comment #8)
> Comment on attachment 581802 [details] [diff] [review]
> Add buildbotve to PATH (buildbot-configs)
> 
> Has this change been tested, it doesn't look like it would pass:

Yes, it has, and it does work. I wasn't sure about the syntax either, and I had to experiment to get the proper interpretation context.

From Win2k3 build slave compile step log (mw32-ix-slave01):

PATH=D:\mozilla-build\buildbotve\scripts;D:\mozilla-build\msys\local\bin;d:\mozilla-build\wget;d:\mozilla-build\7zip;d:\mozilla-build\blat261\full;d:\mozilla-build\python25;d:\mozilla-build\svn-win32-1.6.3\bin;d:\mozilla-build\upx203w;d:\mozilla-build\emacs-22.3\bin;d:\mozilla-build\info-zip;d:\mozilla-build\nsis-2.22;d:\mozilla-build\nsis-2.33u;d:\mozilla-build\hg;d:\mozilla-build\python25\Scripts;d:\mozilla-build\kdiff3;d:\mozilla-build\vim\vim72;d:\mozilla-build\yasm;.;D:\mozilla-build\msys\local\bin;D:\mozilla-build\msys\mingw\bin;D:\mozilla-build\msys\bin;d:\sdks\v7.0\bin;d:\msvs8\Common7\IDE;d:\msvs8\VC\BIN;d:\msvs8\Common7\Tools;d:\msvs8\Common7\Tools\bin;d:\msvs8\VC\PlatformSDK\bin;d:\msvs8\SDK\v2.0\bin;c:\WINDOWS\Microsoft.NET\Framework\v2.0.50727;d:\msvs8\VC\VCPackages;c:\WINDOWS\system32;c:\WINDOWS;c:\WINDOWS\System32\Wbem;c:\Program Files\Intel\DMIX;d:\mozilla-build\python25;c:\Program Files\Microsoft SQL Server\90\Tools\binn\;d:\sdks\tegra042\tools;d:\sdks\tegra042\platformlibs\bin\winxp\x86\release;d:\sdks\tegra042\3rdparty\bin\winxp\x86\release;d:\mozilla-build\python25\scripts;d:\mozilla-build\hg\bin;d:\mozilla-build\moztools\bin

http://dev-master01.build.scl1.mozilla.com:8044/builders/WINNT%205.2%20mozilla-central%20build/builds/6/steps/compile/logs/stdio

From XP test slave mochitest step log (talos-r3-xp-010):

PATH=C:\mozilla-build;C:\mozilla-build\msys\bin;C:\mozilla-build\msys\local\bin;C:\mozilla-build\buildbotve\scripts;C:\mozilla-build\Python25;C:\mozilla-build\Python25\Scripts;C:\mozilla-build\hg;C:\mozilla-build\7zip;C:\mozilla-build\upx203w;C:\WINDOWS\System32\Wbem;C:\WINDOWS\System32;C:\WINDOWS;

http://dev-master01.build.scl1.mozilla.com:8045/builders/Rev3%20WINNT%205.1%20mozilla-central%20opt%20test%20mochitests-1%2F5/builds/3/steps/mochitest-plain-1/logs/stdio

From Win7 test slave mochitest step log (talos-r3-w7-003):

PATH=C:\mozilla-build;C:\mozilla-build\msys\bin;C:\mozilla-build\msys\local\bin;C:\mozilla-build\buildbotve\scripts;C:\mozilla-build\Python25;C:\mozilla-build\Python25\Scripts;C:\mozilla-build\hg;C:\mozilla-build\7zip;C:\mozilla-build\upx203w;C:\WINDOWS\System32\Wbem;C:\WINDOWS\System32;C:\WINDOWS;

http://dev-master01.build.scl1.mozilla.com:8045/builders/Rev3%20WINNT%206.1%20mozilla-central%20opt%20test%20mochitests-2%2F5/builds/0/steps/mochitest-plain-2/logs/stdio

Comment 10

[Justin Wood (:Callek)]

(In reply to Chris Cooper [:coop] from comment #9)
> (In reply to Justin Wood (:Callek) from comment #8)
> > Comment on attachment 581802 [details] [diff] [review]
> > Add buildbotve to PATH (buildbot-configs)
> > 
> > Has this change been tested, it doesn't look like it would pass:
> 
> Yes, it has, and it does work. I wasn't sure about the syntax either, and I
> had to experiment to get the proper interpretation context.

Ah-Ha! Buildbot is doing this in a much different way than I had thought/expected:

found: http://mxr.mozilla.org/build/source/buildbot/master/NEWS#178 to backup your patch here. (sorry for the churn)

Comment 11

[Chris Cooper [:coop] (he/him)]

Comment on attachment 581328 [details] [diff] [review]
Add buildbotve to PATH

http://hg.mozilla.org/build/buildbotcustom/rev/a7bfa1cf6d53

Comment 12

[Chris Cooper [:coop] (he/him)]

Comment on attachment 581802 [details] [diff] [review]
Add buildbotve to PATH (buildbot-configs)

http://hg.mozilla.org/build/buildbot-configs/rev/1d21e762cca7

Comment 13

[Chris Cooper [:coop] (he/him)]

Made it to production today.

Comment 14

[Aki Sasaki (not active)]

Backed out due to graph server post + win32-mobile repack breakage.
http://hg.mozilla.org/build/buildbot-configs/rev/bb815b0660ca

Comment 15

[Rail Aliiev [:rail]]

I believe these patches also break desktop mobile l10n repacks as well.

Comment 16

[Aki Sasaki (not active)]

Also http://hg.mozilla.org/build/buildbotcustom/rev/4414dbcecb75 .

Comment 17

[Chris Cooper [:coop] (he/him)]

(In reply to Aki Sasaki [:aki] from comment #14)
> Backed out due to graph server post + win32-mobile repack breakage.

Can we please post log URLs when we see these things? /me goes to try and find logs.

Comment 18

[Rail Aliiev [:rail]]

One of them: http://ftp.mozilla.org/pub/mozilla.org/mobile/candidates/9.0-candidates/build1/logs/release-mozilla-release-win32-mobile_repack_1-build8.txt.gz

Others are http://ftp.mozilla.org/pub/mozilla.org/mobile/candidates/9.0-candidates/build1/logs/release-mozilla-release-win32-mobile_repack_*-build8.txt.gz

The problem is locale variable which is unicode, so every variable which incorporate locale is unicode as well. We pull locale names from a JSON file (l10 changesets).

Comment 19

[Chris Cooper [:coop] (he/him)]

(In reply to Rail Aliiev [:rail] from comment #18)
> The problem is locale variable which is unicode, so every variable which
> incorporate locale is unicode as well. We pull locale names from a JSON file
> (l10 changesets).

I think I found it. AB_CD is unicode as I expected, but then so is POST_UPLOAD_CMD. If I squash those to strings, I should be able to proceed here.

Comment 20

[Chris Cooper [:coop] (he/him)]

Attachment: Add buildbotve to PATH, also for graphserver

Pretty small change from last time, just adds the new PATH for graphserver calls as well.

Comment 21

[Chris Cooper [:coop] (he/him)]

Attachment: Add buildbotve to PATH (buildbot-configs)

Just un-bitrotting here. Carrying forward review.

Comment 22

[Chris Cooper [:coop] (he/him)]

Attachment: Cast unicode env vars as strings

As mentioned in comment 19, if we cast the postUploadCmd and the locale early enough, we don't have to worry about them again.

Tested successfully today in staging.

Comment 23

[Justin Wood (:Callek)]

Comment on attachment 584152 [details] [diff] [review]
Cast unicode env vars as strings

Nit: why don't we cast at the return of postUploadCmdPrefix?

Comment 24

[Chris Cooper [:coop] (he/him)]

Comment on attachment 584152 [details] [diff] [review]
Cast unicode env vars as strings

(In reply to Justin Wood (:Callek) from comment #23) 
> Nit: why don't we cast at the return of postUploadCmdPrefix?

Done.

https://hg.mozilla.org/build/tools/rev/3b9ae09e497b

Comment 25

[Chris Cooper [:coop] (he/him)]

Comment on attachment 584151 [details] [diff] [review]
Add buildbotve to PATH (buildbot-configs)

https://hg.mozilla.org/build/buildbot-configs/rev/005f21d86152

Comment 26

[Chris Cooper [:coop] (he/him)]

Comment on attachment 584150 [details] [diff] [review]
Add buildbotve to PATH, also for graphserver

https://hg.mozilla.org/build/buildbotcustom/rev/13e05e9626e7

Comment 27

[[e-mail] (:bhearsum)]

This got merged to production today.

Comment 28

[Chris Cooper [:coop] (he/him)]

Attachment: Add buildbotve to PATH for fuzzer

Comment 29

[Chris Cooper [:coop] (he/him)]

Comment on attachment 591105 [details] [diff] [review]
Add buildbotve to PATH for fuzzer

https://hg.mozilla.org/build/buildbotcustom/rev/f13cdb39f0ac

Comment 30

[Rail Aliiev [:rail]]

Looks like this time it broke mochitest, see bug 721779

Comment 31

[Rail Aliiev [:rail]]

This one, I believe: http://hg.mozilla.org/build/buildbotcustom/rev/f13cdb39f0ac

Comment 32

[[e-mail] (:bhearsum)]

Why do we modify the win32-unittest PATH? Test machines don't do sendchanges.

Comment 33

[Chris Cooper [:coop] (he/him)]

Attachment: More targeted PATH fix for fuzzer

Comment 34

[Chris Cooper [:coop] (he/him)]

Comment on attachment 592210 [details] [diff] [review]
More targeted PATH fix for fuzzer

https://hg.mozilla.org/build/buildbotcustom/rev/9f504a8aecd3

Comment 35

[Chris Cooper [:coop] (he/him)]

Attachment: Add buildbotve to PATH for try

try-debug jobs work fine already, but regular try jobs need this PATH augmentation to work. 

I've torn out a lot of hair over the fact that they're not the same this week.

Comment 36

[Armen [:armenzg]]

Comment on attachment 594126 [details] [diff] [review]
Add buildbotve to PATH for try

You better don't pull anymore hair or you will start drinking Guinness and wear plain boring Hawaiian shirts.

Comment 37

[Chris Cooper [:coop] (he/him)]

Comment on attachment 594126 [details] [diff] [review]
Add buildbotve to PATH for try

https://hg.mozilla.org/build/buildbot-configs/rev/37116bf09eae

Comment 38

[Rail Aliiev [:rail]]

Merged to production-0.8 and reconfigured the masters.