https://hg.mozilla.org/build/mozharness/file/d416937ec90a/scripts/spidermonkey/build.browser defines a mozconfig file. I thought we put automation's mozconfig files in the tree so people could adjust things during try pushes among other things.

The reason this is blocking 961258 is because build/mozconfig.common is not sourced by this job. This is the only job on try not sourcing that file. That's a bug.

I'm not entirely sure where to put it. This patch puts it in browser/config/mozconfigs/rootanalysis. Note that we only run the rootanalysis build on linux64, so I could put it in the linux64 directory. But there's not that much of a reason for that restriction. Alternatively, it could live in js/src/devtools/rootAnalysis like the rest of the rootanalysis stuff does. I dunno.

Terrence: there's nothing here that really needs any special review. Basic sanity checking is fine.

Comment on attachment 8363295 [details] [diff] [review] Switch to in-tree mozconfig and new path for JS binary Review of attachment 8363295 [details] [diff] [review]: ----------------------------------------------------------------- Sure! r=me

Comment on attachment 8363289 [details] [diff] [review] Add the rootanalysis mozconfig to the tree Review of attachment 8363289 [details] [diff] [review]: ----------------------------------------------------------------- A short comment at the top of the file might prevent future head scratching. ::: browser/config/mozconfigs/rootanalysis @@ +1,5 @@ > +ac_add_options --enable-debug > +ac_add_options --enable-tests > +ac_add_options --enable-optimize > +ac_add_options --disable-elf-hack > +. $topsrcdir/browser/config/mozconfig Might want to source the file first. @@ +9,5 @@ > +mk_add_options MOZ_OBJDIR=$ANALYZED_OBJDIR > + > +export CFLAGS=-Wno-attributes > +export CPPFLAGS=-Wno-attributes > +export CXXFLAGS=-Wno-attributes You shouldn't need export here. We should append to these variables, not overwrite them, right? e.g. CFLAGS="$CFLAGS -Wno-attributes"

Comment on attachment 8363289 [details] [diff] [review] Add the rootanalysis mozconfig to the tree Review of attachment 8363289 [details] [diff] [review]: ----------------------------------------------------------------- You need to source at least $topsrcdir/build/unix/mozconfig.linux and $topsrcdir/build/mozconfig.common.override. ::: browser/config/mozconfigs/rootanalysis @@ +1,4 @@ > +ac_add_options --enable-debug > +ac_add_options --enable-tests > +ac_add_options --enable-optimize > +ac_add_options --disable-elf-hack Can you add a comment as to why elfhack needs to be disabled?

Also, something is setting PATH to contain /tools/gcc-4.7.2-0moz1/bin. That should be removed.

(In reply to Mike Hommey [:glandium] from comment #7) > Comment on attachment 8363289 [details] [diff] [review] > Add the rootanalysis mozconfig to the tree > > Review of attachment 8363289 [details] [diff] [review]: > ----------------------------------------------------------------- > > You need to source at least $topsrcdir/build/unix/mozconfig.linux and > $topsrcdir/build/mozconfig.common.override. > > ::: browser/config/mozconfigs/rootanalysis > @@ +1,4 @@ > > +ac_add_options --disable-elf-hack > > Can you add a comment as to why elfhack needs to be disabled? I think it failed, so I eliminated it immediately since it's irrelevant here. (If I had a build target that skipped linking entirely, I would use it.) But here's a push with it included: 12:59:09 INFO - 21:27.01 elfhack 12:59:10 INFO - 21:27.76 === 12:59:10 INFO - 21:27.76 === If you get failures below, please file a bug describing the error 12:59:10 INFO - 21:27.76 === and your environment (compiler and linker versions), and use 12:59:10 INFO - 21:27.77 === --disable-elf-hack until this is fixed. 12:59:10 INFO - 21:27.77 === 12:59:10 INFO - 21:27.82 === 12:59:10 INFO - 21:27.82 === If you get failures below, please file a bug describing the error 12:59:10 INFO - 21:27.82 === and your environment (compiler and linker versions), and use 12:59:10 INFO - 21:27.82 === --disable-elf-hack until this is fixed. 12:59:10 INFO - 21:27.82 === 12:59:10 INFO - 21:27.83 0x0000000000000019 (INIT_ARRAY) 0x2073e0 12:59:10 INFO - 21:27.83 0x000000000000000c (INIT) 0x3fe8 12:59:10 INFO - 21:27.83 /builds/slave/l64-br-haz_try_dep-00000000000/build/obj-analyzed/build/unix/elfhack/elfhack: /usr/lib64/libstdc++.so.6: version `GLIBCXX_3.4.15' not found (required by /builds/slave/l64-br-haz_try_dep-00000000000/build/obj-analyzed/build/unix/elfhack/elfhack) 12:59:10 INFO - 21:27.83 /builds/slave/l64-br-haz_try_dep-00000000000/build/obj-analyzed/build/unix/elfhack/elfhack: /usr/lib64/libstdc++.so.6: version `GLIBCXX_3.4.15' not found (required by /builds/slave/l64-br-haz_try_dep-00000000000/build/obj-analyzed/build/unix/elfhack/elfhack)

(In reply to Mike Hommey [:glandium] from comment #8) > Also, something is setting PATH to contain /tools/gcc-4.7.2-0moz1/bin. That > should be removed. That will be a problem. gcc 4.7.3 has a bug that causes the static analysis to fail (a type lookup from the plugin code throws an error that kills the compile.) gcc 4.7.2 for mysterious reasons does not hit the problem (the code is the same; I'm not sure why it doesn't fail.) gcc 4.8+ appear to have fixed the bug, fwiw. But if the mozconfig gcc is 4.7.3, that's going to break the analysis build. From bug 965148, it looks like gcc 4.7.2 might now break the build?

This is a mozconfig that works for me. Note that with the other mozconfigs included, I no longer need to disable elfhack. But I still need to use GCC 4.7.2.

(In reply to Steve Fink [:sfink] from comment #10) > From bug 965148, it looks like gcc 4.7.2 might now break the build? The problem is not gcc 4.7.2, it's binutils. And the tooltool gcc comes with a newer binutils. (In reply to Steve Fink [:sfink] from comment #11) > Created attachment 8369618 [details] [diff] [review] > Add the rootanalysis mozconfig to the tree > > This is a mozconfig that works for me. Note that with the other mozconfigs > included, I no longer need to disable elfhack. That's because of --enable-stdcxx-compat > But I still need to use GCC 4.7.2. Maybe you could get a gcc 4.7.2+newer binutils build in tooltool, then?

(In reply to Mike Hommey [:glandium] from comment #12) > (In reply to Steve Fink [:sfink] from comment #10) > > From bug 965148, it looks like gcc 4.7.2 might now break the build? > > The problem is not gcc 4.7.2, it's binutils. And the tooltool gcc comes with > a newer binutils. What breaks? Is it anything I need to care about for the analysis builds? (eg if it's pgo-only, I don't care.) > (In reply to Steve Fink [:sfink] from comment #11) > > But I still need to use GCC 4.7.2. > > Maybe you could get a gcc 4.7.2+newer binutils build in tooltool, then? Ok, I have one in bug 967383. But can I understand what this is for? Why is switching to tooltool a good thing? Does it give me something different from building an RPM and installing it in my mock environment? I still don't really know how to use it, either. Should I change my build scripts to fetch it via tooltool now? Where should I put my analysis-specific gcc tooltool manifest file? Somewhere in js/src/devtools/rootAnalysis with the rest of the analysis code, or in browser/config/tooltool, or ? If I were to make an osx version of the analysis build, where does the logic to select the right tooltool manifest go? /me is missing the big picture here.

(In reply to Steve Fink [:sfink] from comment #13) > (In reply to Mike Hommey [:glandium] from comment #12) > > (In reply to Steve Fink [:sfink] from comment #10) > > > From bug 965148, it looks like gcc 4.7.2 might now break the build? > > > > The problem is not gcc 4.7.2, it's binutils. And the tooltool gcc comes with > > a newer binutils. > > What breaks? Is it anything I need to care about for the analysis builds? > (eg if it's pgo-only, I don't care.) New webrtc code uses avx assembly that the old binutils doesn't know. > > (In reply to Steve Fink [:sfink] from comment #11) > > > But I still need to use GCC 4.7.2. > > > > Maybe you could get a gcc 4.7.2+newer binutils build in tooltool, then? > > Ok, I have one in bug 967383. But can I understand what this is for? Why is > switching to tooltool a good thing? Does it give me something different from > building an RPM and installing it in my mock environment? I still don't > really know how to use it, either. Should I change my build scripts to fetch > it via tooltool now? Where should I put my analysis-specific gcc tooltool > manifest file? Somewhere in js/src/devtools/rootAnalysis with the rest of > the analysis code, or in browser/config/tooltool, or ? If I were to make an > osx version of the analysis build, where does the logic to select the right > tooltool manifest go? > > /me is missing the big picture here. I'll leave this to some releng people to answer because i really don't know how root analysis is hooked with buildbot.

Note an obvious advantage of tooltool over anything else is that as long as you have packages pushed to the tooltool server, you can test them on try without requiring changes to anything else than the tree you push to try.

(In reply to Mike Hommey [:glandium] from comment #15) > Note an obvious advantage of tooltool over anything else is that as long as > you have packages pushed to the tooltool server, you can test them on try > without requiring changes to anything else than the tree you push to try. That's a wash. With tooltool, I have to get stuff pushed to the tooltool server before I can test with it. With rpm/mock, I have to get stuff pushed to the RPM server before I can test with it. Both kinda suck when you're not sure whether your package is final. I usually test with my own slave first, and I know how to point to an additional yum repo where I can put trial packages. I haven't figured out how to add an additional tooltool server, though I guess I can probably just replace the one and only server with my own since I don't need very much from it. (Or I could do a server-side redirect for unknown hashes.)

(In reply to Steve Fink [:sfink] from comment #16) > (In reply to Mike Hommey [:glandium] from comment #15) > > Note an obvious advantage of tooltool over anything else is that as long as > > you have packages pushed to the tooltool server, you can test them on try > > without requiring changes to anything else than the tree you push to try. > > That's a wash. With tooltool, I have to get stuff pushed to the tooltool > server before I can test with it. With rpm/mock, I have to get stuff pushed > to the RPM server before I can test with it. Both kinda suck when you're not > sure whether your package is final. > > I usually test with my own slave first, and I know how to point to an > additional yum repo where I can put trial packages. I haven't figured out > how to add an additional tooltool server, though I guess I can probably just > replace the one and only server with my own since I don't need very much > from it. (Or I could do a server-side redirect for unknown hashes.) That sounds like a developer productivity issue. It sounds like we need to lower the barrier to testing custom packages in automation. CC Taras.

Hoo boy. Switching over my mozharness script to use tooltool turned out to require some changes. I tried not to make too much of a mess, but... well, it's still a mess. This still feels like it's missing a level of indirection. My tooltool-ized sixgill links against an older libgmp than I have on my laptop, so I really need some sort of manifest selection or something. And I may have to make yet another version for the b2g analysis, since it needs to work with a prebuild Android NDK gcc.

Current tooltool manifests that seem to work with my build slave.

Rebased past 28 changesets.

Comment on attachment 8371993 [details] [diff] [review] Switch to using tooltool for gcc and sixgill I'm kind of wary of adding an unused 'privileged' kwarg to run_command(). Does it work if we add **kwargs for ignored kwargs?

Yeah, that's probably better. I was tempted to prefix the non-mock command with 'sudo'. :-)

The buildbot configs seem to be the current canonical source for the tooltool server URL. It would be nice to split out "slave (build network) environment" stuff from the rest of the buildbot configuration, since it's really not tied to buildbot specifically and yet it's what I keep ending up having to thread through misc.py. But I guess that's out of scope for me right now.

You were right. The ANALYZED_OBJDIR was totally unnecessary. I removed it and everything's fine -- it builds into the default objdir but still deposits its output where I expect to find it. Down to a very simple mozconfig now. I'm switching to using tooltool via a (large) set of other patches in this bug.

Comment on attachment 8372068 [details] [diff] [review] Pass through tooltool_url_list to mozharness Sure. Looks like it's the same everywhere, so we're not gaining much.

in production

Comment on attachment 8372069 [details] [diff] [review] Add the rootanalysis mozconfig to the tree Review of attachment 8372069 [details] [diff] [review]: ----------------------------------------------------------------- I think this file should be in the linux64 directory. ::: browser/config/mozconfigs/hazards @@ +6,5 @@ > + > +# The hazard analysis build breaks with a plugin bug in gcc 4.7.3. Use $PATH to > +# pass through the appropriate compiler. > +unset CC > +unset CXX If you're using tooltool, your gcc 4.7.2 is going to have the same path as gcc 4.7.3 used on other linux builds. So keeping that CC/CXX is going to work. @@ +8,5 @@ > +# pass through the appropriate compiler. > +unset CC > +unset CXX > + > +. "$topsrcdir/browser/config/mozconfig" Not really needed (none of the other ones include this)

https://hg.mozilla.org/integration/mozilla-inbound/rev/0d48fce3ffa2 https://hg.mozilla.org/integration/mozilla-inbound/rev/2603ed1bbfa1

http://hg.mozilla.org/build/mozharness/rev/a17a46cd5d2e http://hg.mozilla.org/build/mozharness/rev/f1062df29526

https://hg.mozilla.org/mozilla-central/rev/0d48fce3ffa2 https://hg.mozilla.org/mozilla-central/rev/2603ed1bbfa1

Mozharness change live in production.

Backing out this whole mess for now. http://hg.mozilla.org/build/mozharness/rev/0cf80abef167 - backed out f1062df29526 http://hg.mozilla.org/build/mozharness/rev/8183d22eee5a - backed out a17a46cd5d2e

I made some last-minute changes to the mozconfig to incorporate review comments, and really *thought* I'd tested them on my slave, but whatever I tested was not what landed. Here's a followup patch that avoids specifying the compiler and hopefully documents why I'm being so nonconformist. I've also switched my mozharness to do things more like the in-tree mozconfigs expect (eg putting gcc in $topsrcdir/gcc instead of /tools/gcc), even though it'd probably work either way now with this mozconfig change. (Took me a while to figure out what was going wrong.)

Ok, this one passes a full end-to-end test. Note that this leaves in the 'privileged' boolean kwarg, but no longer uses it. I'm not sure if it's generally useful. Let me know if you'd rather I ripped it out; it's easy to do.

Comment on attachment 8375838 [details] [diff] [review] Switch to in-tree mozconfig and new path for JS binary* * * >- "exes": { 'hgtool.py': 'tools/buildfarm/utils/hgtool.py' }, >+ "exes": { 'hgtool.py': 'tools/buildfarm/utils/hgtool.py', >+ 'tooltool.py': '/tools/tooltool.py', >+ }, Nit: we'll get PEP 8 warnings if there's a space after the initial {; if you can remove that space (and a corresponding space in each of the two following lines) that'll avoid the warning. I think leaving 'privileged' is fine.

Comment on attachment 8375772 [details] [diff] [review] Make hazard mozconfig avoid the stuff that breaks the analysis Review of attachment 8375772 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/config/mozconfigs/linux64/hazards @@ +9,5 @@ > + > +. "$topsrcdir/build/mozconfig.common" > +ac_add_options --enable-elf-hack > +ac_add_options --enable-stdcxx-compat > + Doesn't it work to augment CC and CXX instead of using a wrapper script?

(In reply to Mike Hommey [:glandium] from comment #37) > Comment on attachment 8375772 [details] [diff] [review] > Make hazard mozconfig avoid the stuff that breaks the analysis > > Review of attachment 8375772 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: browser/config/mozconfigs/linux64/hazards > @@ +9,5 @@ > > + > > +. "$topsrcdir/build/mozconfig.common" > > +ac_add_options --enable-elf-hack > > +ac_add_options --enable-stdcxx-compat > > + > > Doesn't it work to augment CC and CXX instead of using a wrapper script? Yes, totally. But it would mean moving some of the wrapping logic into the build system instead of keeping it separate. Right now, it's set up to work with any build system with only PATH modification. And the logic seems to need to be adapted fairly frequently, so it's kind of nice to keep it as upstream as possible. There's probably a happier middle road. CC="/abs/wrapper/script $CC" or something, as you suggested. It could probably live in parallel with the other wrapping mechanism. For now, though, this is tested and works. I'm willing to change it again in the future, but I've already spent a bunch of time adapting this to a changing environment already and need to get to some other stuff. I'll file a bug for it.

Finally got around to landing this again. Let me see if I can get all the patch flags fixed up... https://hg.mozilla.org/build/mozharness/rev/7abfed3a7c28