Benjamin Smedberg Reporter
	Description • 15 years ago

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.

	bhearsum@mozilla.com (: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

	(not currently active) Ted Mielczarek
	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

	(not currently active) Ted Mielczarek
	Comment 4 • 15 years ago

I'll see if I can sort that out.

	(not currently active) Ted Mielczarek
	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.

	(not currently active) Ted Mielczarek
	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.

	(not currently active) Ted Mielczarek
	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.

	(not currently active) Ted Mielczarek
	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.

	(not currently active) Ted Mielczarek
	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.

	bhearsum@mozilla.com (: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.

	Benjamin Smedberg 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.

	bhearsum@mozilla.com (:bhearsum) 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.

	bhearsum@mozilla.com (:bhearsum) 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.

	bhearsum@mozilla.com (:bhearsum) 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?

	Benjamin Smedberg 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']?

	bhearsum@mozilla.com (:bhearsum) 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.

	Benjamin Smedberg 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.

	bhearsum@mozilla.com (:bhearsum) 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.

	Benjamin Smedberg 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?

	bhearsum@mozilla.com (: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.

	Benjamin Smedberg Reporter
	Comment 24 • 15 years ago

Back to the releng pool for staging and deployment.

	bhearsum@mozilla.com (:bhearsum) Assignee
	Comment 25 • 15 years ago

I'm going to give these patches a run in staging today.

	bhearsum@mozilla.com (:bhearsum) 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.

	Benjamin Smedberg 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.

	John Ford [:jhford] CET/CEST Berlin Time
	Comment 30 • 15 years ago

Assigning to bhearsum for review and landing

	bhearsum@mozilla.com (:bhearsum) Assignee
	Comment 31 • 15 years ago

I'm running the latest patches in staging today.

	bhearsum@mozilla.com (:bhearsum) Assignee
	Comment 32 • 15 years ago

didn't get ac hance to finish testing this today. Will finish up tomorrow.

	bhearsum@mozilla.com (:bhearsum) 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.

	bhearsum@mozilla.com (:bhearsum) 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.

	bhearsum@mozilla.com (:bhearsum) Assignee
	Comment 35 • 15 years ago

Going to land this Thursday morning.

	bhearsum@mozilla.com (:bhearsum) 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

	bhearsum@mozilla.com (:bhearsum) 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

	bhearsum@mozilla.com (:bhearsum) Assignee
	Comment 38 • 15 years ago

Masters have been reconfiged and new builds kicked.

	bhearsum@mozilla.com (:bhearsum) 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

	bhearsum@mozilla.com (:bhearsum) 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.