Closed Bug 979650 Opened 10 years ago Closed 10 years ago

ENABLE_MARIONETTE defaults to off in self-built builds

Categories

(Remote Protocol :: Marionette, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla31

People

(Reporter: dmosedale, Assigned: jgriffin)

Details

Attachments

(1 file, 2 obsolete files)

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.
Assignee: nobody → jgriffin
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.
(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.
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+
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
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.