Closed
Bug 515436
Opened 15 years ago
Closed 15 years ago
Electrolysis test runs need a special LD_LIBRARY_PATH
Categories
(Release Engineering :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: benjamin, Assigned: bhearsum)
References
Details
Attachments
(4 files, 5 obsolete files)
5.17 KB,
patch
|
benjamin
:
review+
ted
:
checked-in+
|
Details | Diff | Splinter Review |
1.74 KB,
patch
|
bhearsum
:
checked-in+
|
Details | Diff | Splinter Review |
2.29 KB,
patch
|
bhearsum
:
review+
bhearsum
:
checked-in+
|
Details | Diff | Splinter Review |
7.98 KB,
patch
|
bhearsum
:
review+
bhearsum
:
checked-in+
|
Details | Diff | Splinter Review |
Electrolysis with the new compiler uses some newer symbols from libstdc++ which aren't in the old version that comes on the build slaves.
I think that this can be fixed by exporting LD_LIBRARY_PATH=CC=/tools/gcc-4.3.3/installed/lib into the environment on all the buildsteps. I'm going to try and implement this and ask if I did it right.
Reporter | ||
Comment 1•15 years ago
|
||
Attachment #399528 -
Flags: review?(bhearsum)
Assignee | ||
Comment 2•15 years ago
|
||
This patch didn't work:
python leaktest.py -- -P default
in dir /builds/slave/electrolysis-linux-debug/build/obj-firefox/_leaktest (timeout 1200 secs)
watching logfiles {}
argv: ['python', 'leaktest.py', '--', '-P', 'default']
<snip>
LD_LIBRARY_PATH=obj-firefox/dist/bin:/tools-gcc-4.3.3/installed/lib
INFO | automation.py | Application pid: 3924
/builds/slave/electrolysis-linux-debug/build/obj-firefox/dist/bin/firefox-bin: /usr/lib/libstdc++.so.6: version `GLIBCXX_3.4.10' not found (required by /builds/slave/electrolysis-linux-debug/build/obj-firefox/dist/bin/libxul.so)
TEST-UNEXPECTED-FAIL | automation.py | Exited with code 1 during test run
INFO | automation.py | Application ran for: 0:00:00.060409
Comment 3•15 years ago
|
||
automation.py prepends our dist/bin to LD_LIBRARY_PATH, but it preserves any existing setting:
http://mxr.mozilla.org/mozilla-central/source/build/automation.py.in#407
Reporter | ||
Updated•15 years ago
|
Blocks: electrolysis
Comment 5•15 years ago
|
||
Ok, actually got gcc 4.3.3 installed on my refplatform VM, will spin a build and try this out.
Status: NEW → ASSIGNED
Comment 6•15 years ago
|
||
We need this to make the other patch have any effect. Turns out all our scripts that use automation.py were clobbering LD_LIBRARY_PATH.
Attachment #405290 -
Flags: review?(benjamin)
Comment 7•15 years ago
|
||
I was able to run mochitest, reftest, leaktest, and the pgo profileserver script on my gcc 4.3 e10s build with this patch. I also verified that I could run mochitest and reftest from a packaged build properly.
Reporter | ||
Updated•15 years ago
|
Attachment #405290 -
Flags: review?(benjamin) → review+
Comment 8•15 years ago
|
||
Comment on attachment 405290 [details] [diff] [review]
fix automation.py and various consumers to not overwrite LD_LIBRARY_PATH
Pushed to m-c:
http://hg.mozilla.org/mozilla-central/rev/b65bcf3fa240
Merging this to e10s and landing the other patch should fix this.
Attachment #405290 -
Flags: checked-in+
Comment 9•15 years ago
|
||
Oh, also, I think we need that env var set on the unittest runs too, not just leaktest. Anyway, passing back over to bhearsum to fix that part.
Assignee: ted.mielczarek → bhearsum
Assignee | ||
Comment 10•15 years ago
|
||
Just landed this and kicked a build.
On a side note, I'm a little surprised we're not having the same issues with unittest builds on E10s.
Attachment #399528 -
Attachment is obsolete: true
Attachment #406045 -
Flags: checked-in+
Attachment #399528 -
Flags: review?(bhearsum)
Reporter | ||
Comment 11•15 years ago
|
||
I thought they were using these same configs... but I get a little lost in all the different settings. Anyway, we do have this problem with the unittest machines.
Assignee | ||
Comment 12•15 years ago
|
||
(In reply to comment #11)
> I thought they were using these same configs... but I get a little lost in all
> the different settings. Anyway, we do have this problem with the unittest
> machines.
They're the same machines, the environments are different though. The unittest machines use:
http://hg.mozilla.org/build/buildbotcustom/file/tip/env.py#l72
by default, but it looks like we can override it in the factory constructor for Packaged unittests. If we need LD_LIBRARY_PATH set for 'make check' we'll have to add env overriding to UnittestBuildFactory.
Assignee | ||
Comment 13•15 years ago
|
||
The leak test builds are fixed now. Still need to fix the unittest ones, I don't have time to look at them right now though.
Assignee: bhearsum → nobody
Reporter | ||
Updated•15 years ago
|
Assignee: nobody → benjamin
Reporter | ||
Comment 14•15 years ago
|
||
Attachment #406061 -
Flags: review?(bhearsum)
Reporter | ||
Comment 15•15 years ago
|
||
Attachment #406062 -
Flags: review?(bhearsum)
Assignee | ||
Updated•15 years ago
|
Attachment #406062 -
Flags: review?(bhearsum) → review-
Assignee | ||
Comment 16•15 years ago
|
||
Comment on attachment 406062 [details] [diff] [review]
Modify unittest environment for e10s, rev. 1
Unfortunately, unittests are setup a little differently than other builds. I think the "right" way to go on this is to set this in the regular 'linux', 'macosx', and 'win32' envs and pass that along to UnittestBuildFactory instead. Does that make sense?
Reporter | ||
Comment 17•15 years ago
|
||
You mean combining
MozillaEnvironments['linux-unittest'] with
BRANCHES['electrolysis']['platforms']['linux']['env']? Why is that better than what I'm doing now, which combines MozillaEnvironments['linux-unittest'] with BRANCHES['electrolysis']['platforms']['linux-unittest']['env']?
Assignee | ||
Comment 18•15 years ago
|
||
(In reply to comment #17)
> You mean combining
> MozillaEnvironments['linux-unittest'] with
> BRANCHES['electrolysis']['platforms']['linux']['env']? Why is that better than
> what I'm doing now, which combines MozillaEnvironments['linux-unittest'] with
> BRANCHES['electrolysis']['platforms']['linux-unittest']['env']?
We don't have a 'linux-unittest' platform, though, and I'd rather not create it. There's already a bunch of stuff in the 'if config['enable_unittests']' block that uses things 'pf[whatever]', which ends up translating to BRANCHES['electrolysis']['platforms']['linux'] in the case we care about.
Reporter | ||
Comment 19•15 years ago
|
||
There are potentially conflicting entries, though:
e.g. ['linux']['env'] {
'MOZ_OBJDIR': OBJDIR, (which is obj-firefox)
While the unittest configs set 'MOZ_OBJDIR' to 'objdir'. Looking through the env, there are extra settings in the standard 'env' table, but none of them look destructive. It just seems unexpected that the 'linux' env would end up being used for unittests. Note that with this patch you only have to create the 'linux-unittest' platform if you want to have a non-default config: it has a default empty dict, so there's not a lot of empty stuff hanging around.
Assignee | ||
Comment 20•15 years ago
|
||
(In reply to comment #19)
> There are potentially conflicting entries, though:
> e.g. ['linux']['env'] {
> 'MOZ_OBJDIR': OBJDIR, (which is obj-firefox)
>
> While the unittest configs set 'MOZ_OBJDIR' to 'objdir'. Looking through the
> env, there are extra settings in the standard 'env' table, but none of them
> look destructive. It just seems unexpected that the 'linux' env would end up
> being used for unittests. Note that with this patch you only have to create the
> 'linux-unittest' platform if you want to have a non-default config: it has a
> default empty dict, so there's not a lot of empty stuff hanging around.
OK, you've sold me. You need to initialize the 'linux-unittest' platform along with the others though And please add a comment that makes it clear that it's only used for this one purpose.
Reporter | ||
Comment 21•15 years ago
|
||
Turns out adding something to 'platforms' doesn't work out because generateBranchObjects tries to turn it into something. So instead, how about just adding an optional [platform]['unittest-env'] key?
Attachment #406061 -
Attachment is obsolete: true
Attachment #406062 -
Attachment is obsolete: true
Attachment #406220 -
Flags: review?(bhearsum)
Attachment #406061 -
Flags: review?(bhearsum)
Reporter | ||
Comment 22•15 years ago
|
||
Attachment #406221 -
Flags: review?(bhearsum)
Assignee | ||
Comment 23•15 years ago
|
||
These latest patches look like they'll work. It needs to be tested in staging though, I probably won't get to that this week.
Reporter | ||
Comment 24•15 years ago
|
||
Back to the releng pool for staging and deployment.
Assignee: benjamin → nobody
Assignee | ||
Comment 25•15 years ago
|
||
I'm going to give these patches a run in staging today.
Assignee | ||
Comment 26•15 years ago
|
||
Benjamin, your patches don't work for packaged unittests, unfortunately, because those are run through UnittestPackagedBuildFactory. That one already has a concept of an env being passed in, but only for the purposes of overriding the default. That can be changed to work the same way as UnittestBuildFactory works.
Very sorry I didn't notice this earlier.
Reporter | ||
Comment 27•15 years ago
|
||
It turns out the Linux build box needs this change as well, now that we're calling `make check` on it.
Reporter | ||
Comment 28•15 years ago
|
||
Attachment #406220 -
Attachment is obsolete: true
Attachment #408069 -
Flags: review?(bhearsum)
Attachment #406220 -
Flags: review?(bhearsum)
Reporter | ||
Comment 29•15 years ago
|
||
Attachment #408070 -
Flags: review?(bhearsum)
Reporter | ||
Updated•15 years ago
|
Attachment #406221 -
Attachment is obsolete: true
Attachment #406221 -
Flags: review?(bhearsum)
Assignee | ||
Comment 31•15 years ago
|
||
I'm running the latest patches in staging today.
Assignee | ||
Comment 32•15 years ago
|
||
didn't get ac hance to finish testing this today. Will finish up tomorrow.
Assignee | ||
Comment 33•15 years ago
|
||
Comment on attachment 408070 [details] [diff] [review]
Specialize the UnittestBuildFactory and UnittestPackagedBuildFactory with the optional unittest-env key, rev. 2
I ran this patch through staging. I'm happy with it, and it fixes 'make check' on the opt and debug builds. We'll organize a time to land it tomorrow or Thursday morning.
Attachment #408070 -
Flags: review?(bhearsum) → review+
Assignee | ||
Comment 34•15 years ago
|
||
Comment on attachment 408069 [details] [diff] [review]
Use an optional 'unittest-env' key instead, rev. 2
I ran this patch through staging. I'm happy with it, and it fixes 'make check' on the opt and debug builds. We'll organize a time to land it tomorrow or Thursday morning.
Attachment #408069 -
Flags: review?(bhearsum) → review+
Assignee | ||
Comment 35•15 years ago
|
||
Going to land this Thursday morning.
Assignee | ||
Comment 36•15 years ago
|
||
Comment on attachment 408070 [details] [diff] [review]
Specialize the UnittestBuildFactory and UnittestPackagedBuildFactory with the optional unittest-env key, rev. 2
changeset: 474:13e2462b66cb
Attachment #408070 -
Flags: checked-in+
Assignee | ||
Comment 37•15 years ago
|
||
Comment on attachment 408069 [details] [diff] [review]
Use an optional 'unittest-env' key instead, rev. 2
changeset: 1693:73b33fb0c1b5
Attachment #408069 -
Flags: checked-in+
Assignee | ||
Comment 38•15 years ago
|
||
Masters have been reconfiged and new builds kicked.
Assignee | ||
Comment 39•15 years ago
|
||
Oh, and I had to land a bustage fix for some silly syntactical stuff, http://hg.mozilla.org/build/buildbotcustom/rev/e0533ce4be71
Assignee | ||
Comment 40•15 years ago
|
||
opt and refcnt builds both run 'make check' fine after this was landed. There's still a lingering issue with opt builds, where they end up failing with:
../../../dist/bin/run-mozilla.sh ../../../dist/bin/xpcshell /builds/slave/electrolysis-linux/build/toolkit/components/feeds/test/shell.js /builds/slave/electrolysis-linux/build/toolkit/components/feeds/test
(<unknown>:16544): Gtk-WARNING **: cannot open display:
But that is a separate issue from this bug.
Thanks for writing the patches for this, Benjamin, and sorry it took so long to get this fixed.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Product: mozilla.org → Release Engineering
You need to log in
before you can comment on or make changes to this bug.
Description
•