My build setup is special, my build scripts define MOZ_OBJDIR before I execute anything. Mach ignores that and so when it doesn't find it in mozconfig it makes up its own.

Comment on attachment 703697 [details] [diff] [review] Default to values from the environment Review of attachment 703697 [details] [diff] [review]: ----------------------------------------------------------------- This will need a test if it is to be committed. See test_mozconfig.py. Easiest way to run tests is |make -C python check|. That being said, I need to think about this some more. Let me think out loud for a moment. When it comes to configuring the build environment, I typically hold the opinion that we should explicitly limit the ways in which things can be configured. At the end of the day, that yields a simpler system with less variations. Simpler systems are easier to maintain and understand. I don't think people fully grok how mozconfigs work today, so I'm particularly sensitive to keeping it simple. What this patch does is provide a new way to define the object directory. On one hand, yes, this is how it can work with client.mk. But, I'm pretty sure the only reason it works with client.mk is because of the craptastic behavior of make conflating local make variables with environment variables. Essentially, the old behavior relied on environment variables leaking into client.mk variable space as variables. (If I were inventing make today I would require make files to explicitly pull in whitelisted environment variables.) One scenario pulling MOZ_OBJDIR in from an environment variable affords you is the ability to have multiple objdirs per mozconfig file. This enables power user scenarios such as utilizing the power of shell scripts in mozconfigs to e.g. power all your trees from one mozconfig, keying off MOZ_OBJDIR to choose which options to load. That's a convenient feature to have! This is probably reason enough to accept this patch. Although the same thing could be done by setting any arbitrary environment variable and using that to influence mozconfig execution. The mozconfig in this case would just call mk_add_options MOZ_OBJDIR with a different value. I do wonder how this will interact with the yet-to-be-designed-and-implemented desire to add "activation" scripts to the tree. See bug 794506 comment #1. I also have a desire to kill mk_add_options, especially mk_add_options MOZ_OBJDIR. The glaring long-term flaw with mk_add_options is it is associated with make. If we no longer use make to build the tree, mk_add_options no longer has any role (although we probably still want to supporting defining MOZ_OBJDIR with it for backwards compat - but only MOZ_OBJDIR). Anyway, the "activate" script would likely take an objdir as an optional argument. You would say "start a Mozilla dev environment and put output in this directory." That would be the only way to define the objdir (unless we decided to code in an environment variable backdoor). Who knows. ... OK. I think I've covered all the angles. I think I like this patch. r+ once there is a test. ::: python/mozbuild/mozbuild/mozconfig.py @@ +177,5 @@ > > env = dict(os.environ) > > + if 'MOZ_OBJDIR' in env: > + result['topobjdir'] = env['MOZ_OBJDIR'] More concisely written as: result['topobjdir'] = env.get('MOZ_OBJDIR', None)

I'd be interested to see your build scripts; it seems to me it would be preferable if you didn't need to build another layer on top of mach.

(In reply to :Ms2ger from comment #2) > I'd be interested to see your build scripts; it seems to me it would be > preferable if you didn't need to build another layer on top of mach. Sure: http://svn.oxymoronical.com/viewvc/mozilla/MozillaBuild/trunk/ Beware, they were originally created 6 years ago and have been being tweaked ever since, so are pretty crusty around the edges. Until mach does everything I want I'm going to continue using my build scripts. I managed to work around this issue by sticking this in my mozconfig: mk_add_options MOZ_OBJDIR="$MOZ_OBJDIR"

I don't have the time or inclination to put in any more effort here now I have a workaround and the issue doesn't affect me. Someone else should feel free to pick it up or just close the bug.

This bug also affected my local setup when I switched to mach from make. It would be good to either fix the bug or deprecate the environment variable and mention this in the docs.

A few patches touching mozconfig loading have landed recently, including just a few minutes ago on inbound. Please try to repro against inbound.

Same as before, but now with tests. We need this for bug 978211 since automation uses MOZ_OBJDIR to set the object directory. Otherwise, mach assumes a different objdir from what make uses, and dies as a result.

Comment on attachment 8424069 [details] [diff] [review] 0001-Bug-832112-add-mach-support-for-MOZ_OBJDIR.patch Review of attachment 8424069 [details] [diff] [review]: ----------------------------------------------------------------- ::: python/mozbuild/mozbuild/mozconfig.py @@ +209,5 @@ > > env = dict(os.environ) > > + if 'MOZ_OBJDIR' in env: > + result['topobjdir'] = env['MOZ_OBJDIR'] You could just do 'topobjdir': os.environ.get('MOZ_OBJDIR'), in the result = {} block.

Now with os.environ.get() - r+ carried forward.

Unfortunately making mach respect MOZ_OBJDIR breaks valgrind builds because the valgrind.sh tools script uses a hard-coded objdir that doesn't match MOZ_OBJDIR. njn, does this patch seem reasonable? It should work both before and after the other patch lands, though note it does mean it will be using "obj-firefox" in automation instead of "objdir" since that's what MOZ_OBJDIR is set to.

Comment on attachment 8425873 [details] [diff] [review] valgrind-objdir Review of attachment 8425873 [details] [diff] [review]: ----------------------------------------------------------------- Seems fine. AFAIK that hard-coded name has no particular significance.

tools patch: https://hg.mozilla.org/build/tools/rev/ba4e4a66729f

The previous valgrind.sh patch fixed the issue when running 'mach', but I didn't realize that supporting MOZ_OBJDIR in mach also broke the build step. The problem is that when an relative objdir is specified, it is assumed to be relative to topsrcdir. If it isn't, mozbuild.action.webidl breaks since it ends up looking in a different place for the objdir than where valgrind.sh puts it. This changes valgrind.sh slightly to make its objdir relative to srcdir if it's a relative path, so it matches what mach/mozbuild are doing. In automation, since MOZ_OBJDIR=obj-firefox, this means the objdir is .../src/obj-firefox

Comment on attachment 8427248 [details] [diff] [review] valgrind-objdir2 Review of attachment 8427248 [details] [diff] [review]: ----------------------------------------------------------------- ::: scripts/valgrind/valgrind.sh @@ +59,3 @@ > objdir=${MOZ_OBJDIR-objdir} > + > +# If the objdir is a relative path, it is relative to the srcdir Nit: period at end of the sentence, please.

tools patch 2 (with review feedback): https://hg.mozilla.org/build/tools/rev/0fe254e5e200

try: https://tbpl.mozilla.org/?tree=Try&rev=b322c473aa59 inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/b83296ec51b3

Thunderbird is failing in configure: OSError: [Errno 2] No such file or directory: '/builds/slave/tb-c-cen-lx-000000000000000000/build/mozilla/objdir-tb' configure: error: /builds/slave/tb-c-cen-lx-000000000000000000/build/mozilla/configure failed for mozilla *** Fix above errors and then restart with "make -f client.mk build" make[2]: *** [configure] Error 1 make[2]: Leaving directory `/builds/slave/tb-c-cen-lx-000000000000000000/build' make[1]: *** [objdir-tb/Makefile] Error 2 make[1]: Leaving directory `/builds/slave/tb-c-cen-lx-000000000000000000/build' make: *** [build] Error 2

Just to note that the problem seems to be only the Mac and Linux builds. Windows seem fine https://tbpl.mozilla.org/?tree=Thunderbird-Trunk

Backing out due to bug 1017775 after way too much discussion: https://hg.mozilla.org/integration/mozilla-inbound/rev/eb3703f6d892

So, here's the interesting thing. When looking at the full picture, I spotted a bug in python/mozbuild/mozbuild/base.py, fixed it, tried running MOZ_OBJDIR=obj-foo ./mach build, and it worked. Then to double check, I unapplied my patch, and rerun MOZ_OBJDIR=obj-foo ./mach build. Guess what? It worked too. And yes, I did check I had changeset eb3703f6d892 in my checkout.

MOZ_OBJDIR=obj-foo ./mach python doesn't do the right thing, though. MOZ_OBJDIR=obj-foo ./mach build also creates an obj-`config.guess` directory.

(In reply to Mike Hommey [:glandium] from comment #21) > So, here's the interesting thing. When looking at the full picture, I > spotted a bug in python/mozbuild/mozbuild/base.py, fixed it, tried running > MOZ_OBJDIR=obj-foo ./mach build, and it worked. Then to double check, I > unapplied my patch, and rerun MOZ_OBJDIR=obj-foo ./mach build. Guess what? > It worked too. And yes, I did check I had changeset eb3703f6d892 in my > checkout. Did you have a obj-`config.guess` fully populated when you tested without the patch? Running 'MOZ_OBJDIR=obj-foo ./mach build' should fail after all build steps have completed with: Error running mach: ['build'] The error occurred in code that was called by the mach command. This is either a bug in the called code itself or in the way that mach is calling it. You should consider filing a bug for this issue. If filing a bug, please include the full output of mach, including this error message. The details of the failure are as follows: Exception: config.status not available. Run configure. File "/home/marf/mozilla-central-git/python/mozbuild/mozbuild/mach_commands.py", line 420, in build if self.substs['MOZ_BUILD_APP'] != 'mobile/android': File "/home/marf/mozilla-central-git/python/mozbuild/mozbuild/base.py", line 267, in substs return self.config_environment.substs File "/home/marf/mozilla-central-git/python/mozbuild/mozbuild/base.py", line 254, in config_environment raise Exception('config.status not available. Run configure.') This happens because base.py is trying to open topobjdir/config.status, where topobjdir=obj-`config.guess` instead of topobjdir=obj-foo. If you have an already-built obj-`config.guess` with config.status and such in it, mach will happily use files from that objdir instead.

Fixed by bug 762358.