Separate Desktop and Android `mach run` and `mach install` implementations
Categories
(Firefox Build System :: Mach Core, enhancement)
Tracking
(firefox68 fixed)
Tracking | Status | |
---|---|---|
firefox68 | --- | fixed |
People
(Reporter: bdekoz, Assigned: nalexander)
References
(Blocks 3 open bugs)
Details
Attachments
(3 files, 1 obsolete file)
I'm building gecko trunk for --target-i686-linux-android on a linux host (f29) and am set up via a USB link to an android target device.
ADB setup, build, package, install, no-argument run etc are fine. However, when I go to test the new build with a specific profile directory (and user.js file), I run into problems.
mkdir android-profile
cp user.js ./android-profile
adb push android-profile /data/local/tmp/
./mach -v run --profile /data/local/tmp/android-profile
No joy. I also tried other arguments, seemingly without use-value:
./mach -v run --setings FILE
./mach -v run --new-window URL
What does work:
./mach run
Reporter | ||
Updated•6 years ago
|
Comment 1•6 years ago
|
||
As far as I know, we have only ever attempted to support basic 'mach run' on android: I expect all arguments are ignored on android.
Comment 2•6 years ago
|
||
There is some old documentation (may not be entirely accurate) at
https://wiki.mozilla.org/Mobile/Fennec/Android/AdvancedTopics#Arguments_and_Environment_Variables
that may help with some of this.
Assignee | ||
Comment 3•6 years ago
|
||
(In reply to Geoff Brown [:gbrown] from comment #1)
As far as I know, we have only ever attempted to support basic 'mach run' on android: I expect all arguments are ignored on android.
It's a little more complicated. Arguments are getting passed through, but they're not getting interpreted by mach run
itself. So the help string is completely wrong and you need to have knowledge of how Gecko itself parses arguments, etc. We can do a little better here, although (IIRC) making mach have totally different command implementations with different argument sets per-platform is hard to achieve.
Assignee | ||
Comment 4•6 years ago
|
||
In the same vein: can we make it easier to debug GV-based things with this? See Bug 1522318 for a use case.
Reporter | ||
Comment 5•6 years ago
|
||
Hey thanks for the hint Geoff.
I can get at the -d URL issue and other tweaks as per help from Nick yesterday, so I'm further along. So close!
I read the doc you pointed at
https://wiki.mozilla.org/Mobile/Fennec/Android/AdvancedTopics#Arguments_and_Environment_Variables
And tried to run the --profile example (with a profile I created with 'adb shell' in the form of /mnt/sdcard2/moz-perf-profile) under ADB with limited results:
%adb shell am start -a android.intent.action.MAIN -c android.intent.category.LAUNCHER -n org.mozilla.fennec_aurora/org.mozilla.gecko.BrowserApp --es args '--profile /mnt/sdcard2/moz-perf-profile'
Starting: Intent { act=android.intent.action.MAIN cat=[android.intent.category.LAUNCHER] cmp=/mnt/sdcard2/moz-perf-profile (has extras) }
Error type 3
Error: Activity class {/mnt/sdcard2/moz-perf-profile} does not exist.
This cmp=XXX part appears to be wrong, as
cmp=/mnt/sdcard2/moz-perf-profile
should be
cmp=org.mozilla.fennec_aurora/org.mozilla.gecko.BrowserApp
Comment 6•6 years ago
|
||
Many automated tests launch fennec with a custom profile. Mochitest test logs show how that's done. Here's an example:
[task 2019-01-24T11:17:29.218Z] 11:17:29 INFO - adb launch_application: am start -W -n org.mozilla.fennec_aurora/org.mozilla.gecko.BrowserApp -a android.intent.action.VIEW --es env9 MOZ_UPLOAD_DIR=/sdcard/tests/mozlog --es env8 R_LOG_DESTINATION=stderr --es args "-no-remote -profile /sdcard/tests/profile//" --es env3 DISABLE_UNSAFE_CPOW_WARNINGS=1 --es env2 R_LOG_VERBOSE=1 --es env1 XPCOM_DEBUG_BREAK=stack --es env0 MOZ_CRASHREPORTER=1 --es env7 MOZ_LOG_FILE=/sdcard/tests/mozlog/moz.log --es env6 MOZ_CRASHREPORTER_SHUTDOWN=1 --es env5 MOZ_IN_AUTOMATION=1 --es env4 MOZ_DISABLE_NONLOCAL_CONNECTIONS=1 --es env12 MOZ_HIDE_RESULTS_TABLE=1 --es env11 R_LOG_LEVEL=6 --es env10 MOZ_CRASHREPORTER_NO_REPORT=1 -d "http://mochi.test:8888/tests?autorun=1&closeWhenDone=1&logFile=%2Fsdcard%2Ftests%2Flogs%2Fmochitest.log&fileLevel=INFO&consoleLevel=INFO&hideResultsTable=1&manifestFile=tests.json&dumpOutputDirectory=%2Fsdcard%2Ftests"
Comment 7•6 years ago
|
||
It looks like there's just one hyphen in -profile - NOT --profile!
Reporter | ||
Comment 8•6 years ago
|
||
Thanks Geoff. Any chance you point at a URL for a log I can scope please?
Here's what I am doing ATM
adb push moz-perf-profile /mnt/sdcard2/
adb shell am start -a android.intent.action.MAIN -c android.intent.category.LAUNCHER -n org.mozilla.fennec_aurora/org.mozilla.gecko.BrowserApp -d about:blank
adb shell
am start -W -n org.mozilla.fennec_aurora/org.mozilla.gecko.BrowserApp -a android.intent.action.VIEW --es args "-no-remote -profile /mnt/sdcard2/moz-perf-profile/" -d https://www.instagram.com/justintimberlake/
which seems to work on the device, but then when I go inspect the profile directory, it only contains my copied-in user.js file, and nothing else, which indicates to me that this is not working....
Comment 9•6 years ago
|
||
Any of the Android 4.3 test logs from treeherder should do. Like:
Our test jobs start from a freshly booted Android image, install Firefox for Android, copy the profile to the device, then launch Firefox for Android. From comment 8, it looks like Firefox is already started with a default profile, then you direct it to the new profile...but I bet that "second" Firefox is the same process as the first one...I wonder what happens to the profiles in that case. I'd try:
adb shell am force-stop org.mozilla.fennec_aurora
adb push moz-perf-profile /mnt/sdcard2/
adb shell am start -W -n org.mozilla.fennec_aurora/org.mozilla.gecko.BrowserApp -a android.intent.action.VIEW --es args "-no-remote -profile /mnt/sdcard2/moz-perf-profile/" -d https://www.instagram.com/justintimberlake/
Reporter | ||
Comment 10•6 years ago
|
||
profile dir must be on sdcard0 on fireOS, apparently. external sd cards (sdcard1) do not have permissions.
/sdcard/Download/profile-dir
seems to work
Updated•6 years ago
|
Assignee | ||
Comment 11•6 years ago
|
||
We want mach run
for Android to be wildly different than mach run
for Desktop. But right now, mach really doesn't support two different
implementations of the same underlying named command. The avenues
that might support different implementations, mostly run through
conditions
.
conditions
were added to mach commands in Bug 901972, and never
really anticipated this use case: commands are keyed by name condition
evaluation is delayed until dispatch-time. In order to have different
commands with the same name, and have full support for --help
,
command matching, suggestions, etc, we really need conditions to
evaluate at parse-time. Indeed, since Bug 901972 landed, we've moved
context creation earlier in the dispatch flow and hacked in things
that look like parse-time conditions (see Bug 1291335 and Bug
1305695).
This approach is not the prettiest, but it handles this narrow
use-case -- making mach run
and mach install
different on Android
-- without much code churn.
Assignee | ||
Comment 12•6 years ago
|
||
This just separates out the Android definitions into
mobile/android/mach_commands.py. There was vestigial support for
running on Android with debuggers, but it was for wiring up JimDB,
which is no longer supported and in fact hasn't worked on actual
devices for a very long time. (The new flow for running on Android
under a debugger goes through the Android Studio hybrid debugger.)
Depends on D18290
Assignee | ||
Comment 13•6 years ago
|
||
mach run
as it is doesn't really parallel mach run
on Desktop;
this makes it a little closer more fully featured. The underlying
functionality is all there in layers of mozharness; let's make it
easier to get to.
Depends on D18291
Assignee | ||
Comment 14•6 years ago
|
||
This turned out to be finicky, because mozharness has layers of
helpers that don't bottom out in the layer we actually want, which is
a GeckoView-specific layer. Specifically:
launch_application
: does not handlemoz_env
.launch_fennec
: hard-codes "org.mozilla.gecko.BrowserApp".launch_activity
: hard-codes "{app}.Activity" (i.e., does not accept a fully qualified class name).
The e10s
does appear to be required, so we handle it specially.
I expected this to work for
org.mozilla.geckoview.test/org.mozilla.geckoview.test.TestRunnerActivity
but it appears not to, so I've removed it. I think that's missing
functionality in TestRunnerActivity, not an error in the intent
dispatch encoded here.
Depends on D18292
Assignee | ||
Comment 15•6 years ago
|
||
The patch series that I posted doesn't handle the things bdekoz actually wants (prefs, settings, profile manipulation) but it sets things up: it stops mach run
from lying about what's supported (!) and shows how to expose existing functionality from mozharness and the existing layers to consumers.
I have limited time to push this, so I wanted to get feedback (in the form of first review) before trying to do any more. Try build is percolating at
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3373a1aad58cabd4c004dad8f6b69be7bccc677f
Updated•6 years ago
|
Updated•5 years ago
|
Comment 16•5 years ago
|
||
Comment 17•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e3a0fedff65e
https://hg.mozilla.org/mozilla-central/rev/376efab28772
https://hg.mozilla.org/mozilla-central/rev/c0df3469a4cb
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Comment 18•5 years ago
|
||
I know I reviewed this and should have thought of this back then, but I was doing some debugging in bug 1580280 and realized this causes us to invoke configure before mach
even parses the command line. So e.g, every mach invocation ever is going into configure.
So I just wanted to point out that mach supports a callable parser, so it would be totally possible to have different command line arguments for Android vs Desktop in a single mach implementation (as well as branching in the implementation itself). In fact, mochitest's ArgumentParser has this exact condition already:
https://searchfox.org/mozilla-central/rev/f1e99da78fe6c3c68696358dac06aed90f8112d3/testing/mochitest/mach_commands.py#212
In other words, we could scrap conditional_names
entirely and re-work the install
and run
commands to have different arguments and branches depending on whether we have an Android build or not. Wish I thought of this during review.
Anyway, it's not a super high priority or anything.. but I think it would be good to avoid using more conditional_names
in the future.
Assignee | ||
Comment 19•5 years ago
|
||
(In reply to Andrew Halberstadt [:ahal] from comment #18)
I know I reviewed this and should have thought of this back then, but I was doing some debugging in bug 1580280 and realized this causes us to invoke configure before
mach
even parses the command line. So e.g, every mach invocation ever is going into configure.So I just wanted to point out that mach supports a callable parser, so it would be totally possible to have different command line arguments for Android vs Desktop in a single mach implementation (as well as branching in the implementation itself). In fact, mochitest's ArgumentParser has this exact condition already:
https://searchfox.org/mozilla-central/rev/f1e99da78fe6c3c68696358dac06aed90f8112d3/testing/mochitest/mach_commands.py#212In other words, we could scrap
conditional_names
entirely and re-work theinstall
andrun
commands to have different arguments and branches depending on whether we have an Android build or not. Wish I thought of this during review.Anyway, it's not a super high priority or anything.. but I think it would be good to avoid using more
conditional_names
in the future.
This is helpful to know, but could you also file a ticket tracking this?
Regardless of what we do with conditional names, there's still an issue, 'cuz just asking for the build configuration shouldn't trigger configure
. I think this is something new with the transition to Python 3. Definitely I've tested running mach lint --linter android-lint
with a Desktop mozconfig
and with no mozconfig
at all and I didn't see mach configure
running. Do you see this when running with Python 3 as well?
Comment 20•5 years ago
|
||
Filed bug 1584567 to track removing conditional_names
. I actually have patches that remove them already because I thought it might fix my android-lints
issue (still pending results).
But yeah, I agree there's something else going on here, this definitely only happens with Python 3. Personally my goal is to avoid having to debug environment variable setting in configure.. I'm the wrong person for that task. But someone will have to figure it out sooner or later (even if we punt on it for now).
Fwiw, I've actually seen the MOZ_AUTOMATION issue before:
https://searchfox.org/mozilla-central/rev/23f836a71cfe961373c8bd0d0219ec60a64b3c8f/mach#154
So reproducing it won't be a problem.
Edit: Let's move discussion back to bug 1580280.
Description
•