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)
Remote Protocol
Marionette
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla31
People
(Reporter: dmosedale, Assigned: jgriffin)
Details
Attachments
(1 file, 2 obsolete files)
7.14 KB,
patch
|
jgriffin
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•10 years ago
|
||
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
Reporter | ||
Comment 2•10 years ago
|
||
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...
Assignee | ||
Comment 3•10 years ago
|
||
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.
Comment 4•10 years ago
|
||
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 | ||
Updated•10 years ago
|
Assignee: nobody → jgriffin
Assignee | ||
Comment 5•10 years ago
|
||
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.
Assignee | ||
Comment 6•10 years ago
|
||
Running this on try: https://tbpl.mozilla.org/?tree=Try&rev=f6cd599854f0
Assignee | ||
Comment 7•10 years ago
|
||
(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.
Assignee | ||
Comment 8•10 years ago
|
||
new try run: https://tbpl.mozilla.org/?tree=Try&rev=96a4e9ff8200
Assignee | ||
Comment 9•10 years ago
|
||
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)
Assignee | ||
Updated•10 years ago
|
Attachment #8389399 -
Attachment is obsolete: true
Comment 10•10 years ago
|
||
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+
Assignee | ||
Comment 11•10 years ago
|
||
Move the define() stuff to configure.in: https://tbpl.mozilla.org/?tree=Try&rev=3093ab7a0d1f https://tbpl.mozilla.org/?tree=Try&rev=ecdd77a6bcf7
Assignee | ||
Updated•10 years ago
|
Attachment #8390722 -
Attachment is obsolete: true
Assignee | ||
Comment 12•10 years ago
|
||
Comment on attachment 8393746 [details] [diff] [review] Always build with Marionette for Firefox, b2g desktop, carry r+ forward
Attachment #8393746 -
Flags: review+
Assignee | ||
Comment 13•10 years ago
|
||
try runs were green. https://hg.mozilla.org/integration/mozilla-inbound/rev/f36d896befb4
Target Milestone: --- → mozilla31
https://hg.mozilla.org/mozilla-central/rev/f36d896befb4
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Updated•1 year ago
|
Product: Testing → Remote Protocol
You need to log in
before you can comment on or make changes to this bug.
Description
•