Closed Bug 979650 Opened 6 years ago Closed 6 years ago
_MARIONETTE defaults to off in self-built builds
This means if someone developing lower level code inadvertently causes a Marionette test on Tbpl to break, if they want to try and fix it, the first thing they're likely to see is: $ ./mach marionette-test The marionette-test command requires a Marionette-enabled build. Add 'ENABLE_MARIONETTE=1' to your mozconfig file and re-build the application. Your currently active mozconfig is /Users/dmose/r/abr-gecko-dev/.mozconfig. My guess is that this the result of some tests like: http://dxr.mozilla.org/mozilla-central/source/browser/app/moz.build#15 Which require marionette to be explicitly enabled, rather than explicitly disabled.
Just FYI, here was the mozconfig I was using on Mac that caused this to be hit: export PATH="`brew --prefix ccache`/libexec:$PATH" # Import the stock config for building the browser (Firefox) . $topsrcdir/browser/config/mozconfig . $topsrcdir/build/macosx/mozconfig.common # Define where build files should go. This places them in the directory # "obj-ff-dbg" under the current source directory #mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/../objdir/mac-debug # -s makes builds quieter by default # -j4 allows 4 tasks to run in parallel. Set the number to be the amount of # cores in your machine. 4 is a good number. mk_add_options MOZ_MAKE_FLAGS="-s -j7" # Enable debug builds ac_add_options --enable-debug ac_add_options --enable-optimize
Interestingly, adding various different things at the top of the file, including ENABLE_MARIONETTE=1 export ENABLE_MARIONETTE=1 and mk_add_options ENABLE_MARIONETTE=1 followed by "./mach build" do not fix the problem. Perhaps a clobber build is required...
Yeah, a clobber is required. Since ENABLE_MARIONETTE=1 is now the default for official builds, I agree it makes sense to always enable it for desktop Firefox, rather than requiring something in mozconfig. Adding Ted in case he has a different opinion.
The defaults should always be as close as possible to what we ship. I don't see any reason for this to not be on by default.
Looks like we are already doing the right things for desktop Firefox builds (but not B2G desktop builds); we just need to update the mach targets to realize this.
Running this on try: https://tbpl.mozilla.org/?tree=Try&rev=f6cd599854f0
(In reply to Jonathan Griffin (:jgriffin) from comment #6) > Created attachment 8389399 [details] [diff] [review] > Always build with Marionette for Firefox, b2g desktop, > > Running this on try: https://tbpl.mozilla.org/?tree=Try&rev=f6cd599854f0 Looks like this is a fail for B2G desktop builds.
new try run: https://tbpl.mozilla.org/?tree=Try&rev=96a4e9ff8200
Finally got a green try run! https://tbpl.mozilla.org/?tree=Try&rev=b0b5c48cbfeb. The intent here is to primarily to make it so that B2G desktop builds always get built with Marionette, and also to remove the ENABLE_MARIONETTE checking for desktop Firefox mach commands (since it isn't relevant there).
Attachment #8390722 - Flags: review?(ted)
Attachment #8389399 - Attachment is obsolete: true
Comment on attachment 8390722 [details] [diff] [review] Always build with Marionette for Firefox, b2g desktop, Review of attachment 8390722 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/app/moz.build @@ +12,4 @@ > 'nsBrowserApp.cpp', > ] > > +DEFINES['ENABLE_MARIONETTE'] = 1 Can you just rip out all the "DEFINES['ENABLE_MARIONETTE'] = 1" bits and move that up to configure.in instead? if test -n "$ENABLE_MARIONETTE"; then AC_DEFINE(ENABLE_MARIONETTE) fi
Attachment #8390722 - Flags: review?(ted) → review+
Move the define() stuff to configure.in: https://tbpl.mozilla.org/?tree=Try&rev=3093ab7a0d1f https://tbpl.mozilla.org/?tree=Try&rev=ecdd77a6bcf7
Attachment #8390722 - Attachment is obsolete: true
Comment on attachment 8393746 [details] [diff] [review] Always build with Marionette for Firefox, b2g desktop, carry r+ forward
Attachment #8393746 - Flags: review+
try runs were green. https://hg.mozilla.org/integration/mozilla-inbound/rev/f36d896befb4
Target Milestone: --- → mozilla31
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.