Closed Bug 1616704 Opened 1 year ago Closed 1 year ago

Make `./mach build binaries && ./mach run` pick up binary changes to `libxul.so`

Categories

(Firefox Build System :: General, defect)

All
macOS
defect
Not set
normal

Tracking

(firefox-esr68 unaffected, firefox73 wontfix, firefox74 wontfix, firefox75 fixed)

RESOLVED FIXED
mozilla75
Tracking Status
firefox-esr68 --- unaffected
firefox73 --- wontfix
firefox74 --- wontfix
firefox75 --- fixed

People

(Reporter: mstange, Assigned: nalexander)

References

(Regression)

Details

(Keywords: in-triage, regression)

Attachments

(1 file)

Steps to reproduce:

  1. Build mozilla-central for Android, i.e. with ac_add_options --enable-application=mobile/android.
  2. Run GeckoView-example on a connected Android device via ./mach run.
  3. Make a change to a C++ file.
  4. Execute ./mach build binaries && ./mach run, and answer Y when it asks whether it should reinstall the app.

Expected results:
The C++ changes should have been picked up in the GeckoView-example that now runs on the device.

Actual results:
The changes didn't have any effect.

Anecdotally, running ./mach run once more (and saying Y when it asks whether it should reinstall) seems to work.

This is very reproducible. As a workaround, I am now running ./mach run twice after every code change.

I can reproduce this as well, though I don't understand the gradle integration enough to know how to fix it. I added a new symbol to a .cpp file, and this is what I see:

./mach build binaries

  • New symbol shows up in objdir/dist/bin/libxul.so

./mach run

  • New symbol shows up in objdir/dist/fennec/lib/armeabi-v7a/libxul.so

./mach run (2nd time)

  • New symbol now shows up in objdir/gradle/build/mobile/android/geckoview_example/intermediates/transforms/mergeJniLibs/withGeckoBinaries/debug/0/lib/armeabi-v7a/libxul.so, which I think is the actual file that gets used in the apk.

nalexander, do you have any ideas here?

Flags: needinfo?(nalexander)

nalexander, do you have any ideas here?

Well, sort of. First, since Bug 1550146, mach build includes mach package, which is what actually produces the APK that gets sent to the device. By only running mach build binaries the packaging step is skipped. There's no reason to expect that to work: for example, if you do mach build binaries, you don't get a fresh Windows installer or macOS DMG. But I understand that few folks have that detailed mental model about what's going on behind the scenes.

Now, I was initially surprised that mach build binaries && mach run doesn't pick up the changes, because mach run invokes a Gradle target that I expected would see the updated libxul.so. But your detailed steps remind me of what's happening: the first mach run must be copying the library into dist but not consuming it (somehow?), but then it's in the right place for second mach run. This is a bit of history, from when we supported Fennec, and might still be required for artifact builds. My guess is that there's some subtlety around the consumption piece that is being exercised differently now.

So: mach build binaries && mach package && mach run should definitely work (verify it?) but I'll take a time-boxed look at the given flow now.

Flags: needinfo?(nalexander)
Regressed by: 1550146
Summary: ./mach build binaries && ./mach run for GeckoView-example does not pick up binary changes → Make `./mach build binaries && ./mach run` pick up binary changes to `libxul.so`
Assignee: nobody → nalexander
Status: NEW → ASSIGNED

The Android-Gradle build plugin has evolved, as has GeckoView (it now
expects libraries in libs/ instead of assets/), so we need to
ensure the libraries are copied into place at the right time.

To test this, you can use an artifact build. Run ./mach build, and
then binary edit a string in $TOPOBJDIR/dist/bin/libxul.so (I used
Emacs and bbe to do that). Run ./mach android build-geckoview_example, and then verify the updated string has been
packaged, for example with something like:

unzip -c $TOPOBJDIR/gradle/build/mobile/android/geckoview_example/outputs/apk/withGeckoBinaries/debug/geckoview_example-withGeckoBinaries-debug.apk lib/x86_64/libxul.so | strings | grep NEW 'STRING'

Coincidentally, this is essentially the same issue as
https://github.com/mozilla/application-services/issues/2659.

mstange: please test.

Flags: needinfo?(mstange)
Pushed by nalexander@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0347e375abe3
Make `./mach build binaries && ./mach run` pick up binary changes to `libxul.so`. r=geckoview-reviewers,agi
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla75

It seems to be working! Thanks so much!

Flags: needinfo?(mstange)
You need to log in before you can comment on or make changes to this bug.