Closed Bug 1521996 Opened 8 months ago Closed 4 months ago

Separate Desktop and Android `mach run` and `mach install` implementations

Categories

(Firefox Build System :: Mach Core, enhancement)

enhancement
Not set

Tracking

(firefox68 fixed)

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: bdekoz, Assigned: nalexander)

References

(Blocks 2 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

Summary: mach [--profile, --settings, --new-window] not working on android targets → mach run [--profile, --settings, --new-window] not working on android targets

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.

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.

(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.

In the same vein: can we make it easier to debug GV-based things with this? See Bug 1522318 for a use case.

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

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"

It looks like there's just one hyphen in -profile - NOT --profile!

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....

Any of the Android 4.3 test logs from treeherder should do. Like:

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=223901610&repo=mozilla-central&lineNumber=1462

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/

profile dir must be on sdcard0 on fireOS, apparently. external sd cards (sdcard1) do not have permissions.

/sdcard/Download/profile-dir

seems to work

See Also: → 1522318

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.

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

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

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 handle moz_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

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

See Also: → 1527097
Attachment #9040592 - Attachment is obsolete: true
Pushed by nalexander@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e3a0fedff65e
Part 1: Add `conditional_name` to mach @Command definition. r=ahal
https://hg.mozilla.org/integration/autoland/rev/376efab28772
Part 2: Specialize `mach {run,install}` for Android. r=ahal,gbrown
https://hg.mozilla.org/integration/autoland/rev/c0df3469a4cb
Part 3: Make `mach run` for Android bring Fennec forward by default. r=gbrown
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Assignee: nobody → nalexander
Blocks: 1556793
Blocks: 1557805
Summary: mach run [--profile, --settings, --new-window] not working on android targets → Separate Desktop and Android `mach run` and `mach install` implementations
Blocks: 1580995
You need to log in before you can comment on or make changes to this bug.