Closed Bug 515436 Opened 11 years ago Closed 11 years ago

Electrolysis test runs need a special LD_LIBRARY_PATH

Categories

(Release Engineering :: General, defect)

x86
Windows NT
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: benjamin, Assigned: bhearsum)

References

Details

Attachments

(4 files, 5 obsolete files)

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.
Attachment #399528 - Flags: review?(bhearsum)
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
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
I'll see if I can sort that out.
Assignee: benjamin → ted.mielczarek
Blocks: electrolysis
Ok, actually got gcc 4.3.3 installed on my refplatform VM, will spin a build and try this out.
Status: NEW → ASSIGNED
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)
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.
Attachment #405290 - Flags: review?(benjamin) → review+
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+
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
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)
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.
(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.
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
Assignee: nobody → benjamin
Attachment #406062 - Flags: review?(bhearsum)
Attachment #406062 - Flags: review?(bhearsum) → review-
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?
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']?
(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.
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.
(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.
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)
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.
Blocks: 523094
Back to the releng pool for staging and deployment.
Assignee: benjamin → nobody
I'm going to give these patches a run in staging today.
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.
It turns out the Linux build box needs this change as well, now that we're calling `make check` on it.
Attachment #406220 - Attachment is obsolete: true
Attachment #408069 - Flags: review?(bhearsum)
Attachment #406220 - Flags: review?(bhearsum)
Attachment #406221 - Attachment is obsolete: true
Attachment #406221 - Flags: review?(bhearsum)
Assigning to bhearsum for review and landing
Assignee: nobody → bhearsum
I'm running the latest patches in staging today.
didn't get ac hance to finish testing this today. Will finish up tomorrow.
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+
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+
Going to land this Thursday morning.
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+
Comment on attachment 408069 [details] [diff] [review]
Use an optional 'unittest-env' key instead, rev. 2

changeset:   1693:73b33fb0c1b5
Attachment #408069 - Flags: checked-in+
Masters have been reconfiged and new builds kicked.
Oh, and I had to land a bustage fix for some silly syntactical stuff, http://hg.mozilla.org/build/buildbotcustom/rev/e0533ce4be71
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: 11 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.