We preprocess AndroidManifest.xml.in and some sundry .java.in in mobile/android/base/Makefile.in. This ticket tracks moving those to moz.build GENERATED_FILES in some way.

Review commit: https://reviewboard.mozilla.org/r/39325/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/39325/

This has the slight cost of not displaying all of the -D preprocessor options in the logs. Review commit: https://reviewboard.mozilla.org/r/39327/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/39327/

Comment on attachment 8729270 [details] MozReview Request: Bug 1254355 - Part 1: Make AndroidManifest.xml and fennec_ids.txt GENERATED_FILES. r?glandium https://reviewboard.mozilla.org/r/39325/#review36105 ::: python/mozbuild/mozbuild/action/generate_android.py:24 (Diff revision 1) > +def _local_defines(): Ugh, this is the worst of both worlds. I'd rather keep these things in Makefile.in than do this. Plus, you're repeating what's already in moz.build here. You can be sure this would get out of sync very quickly. This ought to be a generic way to replace PP_TARGETS, using the local defines from moz.build. See what we do with generate_symbols_file.py to add the local defines. Now, the obvious problem with this "plan" is that the Makefile contains DEFINES. But let's see what is there: - ANDROID_VERSION_CODE which is either coming from MOZ_APP_ANDROID_VERSION_CODE or from executing a script. - MOZ_ANDROID_SHARED_ID which, it turns out, could just be in moz.build already - MOZ_BUILDID, which is only used in AppConstants.java.in so, you don't have to care about it for this bug, except it's also used for the alternative form of ANDROID_VERSION_CODE. Now the question is, why does ANDROID_VERSION_CODE have to have that fallback in Makefile.in?

Comment on attachment 8729271 [details] MozReview Request: Bug 1254355 - Part 2: Make *Constants.java GENERATED_FILES. r?glandium https://reviewboard.mozilla.org/r/39327/#review36107 ::: mobile/android/base/moz.build (Diff revision 1) > -for var in ('MOZ_ANDROID_ANR_REPORTER', 'MOZ_LINKER_EXTRACT', 'MOZ_DEBUG', Err, one step forward, three steps backwards.

https://reviewboard.mozilla.org/r/39325/#review36167 ::: python/mozbuild/mozbuild/action/generate_android.py:24 (Diff revision 1) > +from mozbuild.android_version_code import android_version_code > + > +import os > + > + > +def _local_defines(): I think your "worst of both worlds" is addressed by the second commit, which just moves the `DEFINES` stuff out of `moz.build` and into the script. We have talked many times about a generic `moz.build` `PP_TARGETS`; do we really want this? It's more generic than `GENERATED_FILES` with a per-use script. Looking at https://dxr.mozilla.org/mozilla-central/search?q=PP_TARGETS+-path%3Aobj*&redirect=true&case=false I see a few things that can be `FINAL_TARGET_PP_FILES` and then a few things that mostly want `topsrcdir`. I thought the vision was that such things would get their own per-use script. I'll remake this patch to add `PREPROCESSED_FILES` to `moz.build` if that's desired. As for the version code -- most of the time we really need to generate it. We could make it work just like buildid.h, but it really seems like it *should* be in a `GENERATED_FILES` script -- like this patch does.

glandium: I thought we were explicitly trying to avoid that generic primitive, and to use per-instance `GENERATED_FILES` scripts. If you feel this should be a generic `PREPROCESSED_FILES` in `moz.build`, I'll do that. We can make the version code computation just like buildid.h if that's the most sensible. And I just realized this should depend on buildid.h in any case, which we'd get "for free" if we made this a generic preprocessed declaration and just included "android_version_code.h" in that file.

You can also have a generic preprocessor GENERATED_FILES script... you just need a way to pass it the local define flags.

Comment on attachment 8858928 [details] Bug 1254355 - Part 1: Prepare to generalize generate_build_config. https://reviewboard.mozilla.org/r/130920/#review133524 This seems like a useful refactor. I think I see where you're going with this...

Comment on attachment 8858929 [details] Bug 1254355 - Part 2: Generate AndroidManifest.xml using GENERATED_FILES. https://reviewboard.mozilla.org/r/130922/#review133526 Nice cleanup! Amazing how much gunk you were able to delete by cutting out make.

I fixed a small mistake (removed a DEFINES used by PP_TARGET_FILES) and manually tested the compiled AndroidManifest.xml from a green try build and a central build. Landing!

Pushed by nalexander@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1228043614d0 Part 1: Prepare to generalize generate_build_config. r=gps https://hg.mozilla.org/integration/autoland/rev/a5af7f7132f6 Part 2: Generate AndroidManifest.xml using GENERATED_FILES. r=gps

Backed out for failing android lint job (unused file crash_reporter.png): https://hg.mozilla.org/integration/autoland/rev/944f3ae4b8ef0faac12f29211ea66f837619f7dd Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=a5af7f7132f69a44e723012f9091b137e3a0a09f&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=success&filter-resultStatus=pending&filter-resultStatus=running&filter-searchStr=Android+4.0+API15%2B+opt+Executed+by+TaskCluster+android-lint+tc(lint) Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=92496769&repo=autoland ../../../../../../../mobile/android/base/crashreporter/res/drawable-mdpi/crash_reporter.png: The resource R.drawable.crash_reporter appears to be unused

Comment on attachment 8859649 [details] Bug 1254355 - Pre: Use #ifdef, not #if, for MOZ_CRASHREPORTER. https://reviewboard.mozilla.org/r/131664/#review134428 ::: mobile/android/base/AndroidManifest.xml.in:267 (Diff revision 1) > #include ../services/manifests/FxAccountAndroidManifest_activities.xml.in > #ifdef MOZ_ANDROID_SEARCH_ACTIVITY > #include ../search/manifests/SearchAndroidManifest_activities.xml.in > #endif > > -#if MOZ_CRASHREPORTER > +#ifdef MOZ_CRASHREPORTER gps: I made an error hand comparing the manifests, and there was an issue due to the "special" handling of defines; they can be "defined()", or "0", or "1" -- and `MOZ_CRASHREPORTER` is handled differently throughout the tree. This makes it be "defined()", which agrees with most places in the tree.

Comment on attachment 8859649 [details] Bug 1254355 - Pre: Use #ifdef, not #if, for MOZ_CRASHREPORTER. https://reviewboard.mozilla.org/r/131664/#review134970

Comment on attachment 8859650 [details] Bug 1254355 - Post: Make android-* jobs depend on more of the build. https://reviewboard.mozilla.org/r/131666/#review134972

We're sorry - something has gone wrong while rewriting or rebasing your commits. The commits being pushed no longer match what was requested. Please file a bug.

Only part 2 was backed out, making for a very confusing landing order. But hopefully we're okay now!

Pushed by nalexander@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/41d155de1019 Pre: Use #ifdef, not #if, for MOZ_CRASHREPORTER. r=gps https://hg.mozilla.org/integration/autoland/rev/6a0e9a3f2673 Part 2: Generate AndroidManifest.xml using GENERATED_FILES. r=gps https://hg.mozilla.org/integration/autoland/rev/5cc1ea46d811 Post: Make android-* jobs depend on more of the build. r=gps

https://hg.mozilla.org/mozilla-central/rev/41d155de1019 https://hg.mozilla.org/mozilla-central/rev/6a0e9a3f2673 https://hg.mozilla.org/mozilla-central/rev/5cc1ea46d811

Sigh, I busted l10n repacks. Try push that should fix the issue above. Will try to get a build peer review for the (trivial) fix and reland without backing out.

Comment on attachment 8860533 [details] Bug 1254355 - Follow-up: Use full paths to match GENERATED_FILES targets. https://reviewboard.mozilla.org/r/132550/#review135426 What was actually breaking without the abspath?

(In reply to Michael Shal [:mshal] from comment #36) > Comment on attachment 8860533 [details] > Bug 1254355 - Follow-up: Use full paths to match GENERATED_FILES targets. > > https://reviewboard.mozilla.org/r/132550/#review135426 > > What was actually breaking without the abspath? l10n repacks. The gecko-nodeps.ap_ depends on AndroidManifest.xml, which couldn't be found due to the path. (It would have been a problem for everything depending on AndroidManifest.xml.) It's just hard to test the l10n stuff locally, and I forgot about the absolute paths/virtual dirs thing in Make. It's bitten me a lot of times; it's part of the motivation for GENERATED_FILES!

Pushed by nalexander@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/33d4c8885613 Follow-up: Use full paths to match GENERATED_FILES targets. r=mshal

Android L10n Nightlies were still broken yesterday, so I backed this out: https://hg.mozilla.org/mozilla-central/rev/070fc3c2f8400c09f1037f5444c3d33b499e7128 https://hg.mozilla.org/mozilla-central/rev/08c15ccd9e5f2bc4a7a81bfe027b85788b67a6f1 https://hg.mozilla.org/mozilla-central/rev/402b6ada60ae759de1c2f9e26b99880952c5311d https://hg.mozilla.org/mozilla-central/rev/a9992206ae89d8bed1d1f427ea91e82d8633622e Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=93528314&repo=mozilla-central

The backout fixed the issue.

Callek: I was able to run an Android Nightly (N) build in try but I never got a Nightly Single-locale Repack (N1 .. N5) build in try. Some of the following produced builds, and some failed in the decision task, but none got my single-locale repack: hg push-to-try -m "try: -b o -p android-api-15-nightly,android-api-15 -u none -t none --include-nightly" hg push-to-try -m "try: -b o -p android-api-15-nightly,android-api-15,android -u none -t none --include-nightly" hg push-to-try -m "try: -b o -p android-api-15 --include-nightly" hg push-to-try -m "try: -b o -p android-api-15,android-api-15-gradle-dependencies,android-api-15-frontend,android-x86,android-lint --include-nightly" Can you suggest the correct incantation for N1 builds? Thanks!

(In reply to Nick Alexander :nalexander from comment #42) > Callek: I was able to run an Android Nightly (N) build in try but I never > got a Nightly Single-locale Repack (N1 .. N5) build in try. That's bug 1358865. The failing decision tasks were an infra issue on the weekend.

No, pretty sure this wasn't the bug Aryx mentioned... There may be another way, but this is what I found today. (I'm sad that stuff changed enough here, but I want to make this work for you today, rather than fixing it systemically first.) 'try: -b o -p linux,linux-nightly,linux64,linux64-nightly,android-api-15,android-api-15-nightly -u none -t none --include-nightly -j android-api-15-nightly' ^ Should do it, IF you apply the following patch as well: diff --git a/taskcluster/taskgraph/try_option_syntax.py b/taskcluster/taskgraph/try_option_syntax.py --- a/taskcluster/taskgraph/try_option_syntax.py +++ b/taskcluster/taskgraph/try_option_syntax.py @@ -29,16 +29,17 @@ BUILD_KINDS = set([ 'l10n', 'valgrind', 'static-analysis', 'spidermonkey', ]) # anything in this list is governed by -j, matching against the `build_platform` attribute JOB_KINDS_MATCHING_BUILD_PLATFORM = set([ + 'nightly-l10n', 'toolchain', 'android-stuff', ]) # mapping from shortcut name (usable with -u) to a boolean function identifying # matching test names def alias_prefix(prefix):

Callek: so, progress! Thanks for all your help so far. Any idea how I can make the N1 job in https://treeherder.mozilla.org/#/jobs?repo=try&revision=ef230b595751f0ff326b51aa9a534ad69f51723d&selectedJob=93897108 download fennec-...apk rather than target.apk? This seems like a bug when working on try but perhaps it's not.

(In reply to Nick Alexander :nalexander from comment #46) > Callek: so, progress! Thanks for all your help so far. Any idea how I can > make the N1 job in > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=ef230b595751f0ff326b51aa9a534ad69f51723d&selectedJob=9 > 3897108 download fennec-...apk rather than target.apk? This seems like a > bug when working on try but perhaps it's not. Bah! That's a legit bug in my configs. This isn't actually a problem with file name, but is a problem with the configs. Fixable with the following patch too: diff --git a/testing/mozharness/configs/single_locale/try_android-api-15.py b/testing/mozharness/configs/single_locale/try_android-api-15.py --- a/testing/mozharness/configs/single_locale/try_android-api-15.py +++ b/testing/mozharness/configs/single_locale/try_android-api-15.py @@ -1,25 +1,27 @@ +import os + BRANCH = "try" MOZILLA_DIR = BRANCH EN_US_BINARY_URL = "http://archive.mozilla.org/pub/" \ "mobile/nightly/latest-mozilla-central-android-api-15/en-US" config = { "branch": "try", "log_name": "single_locale", "objdir": "obj-l10n", "is_automation": True, "buildbot_json_path": "buildprops.json", "force_clobber": True, "clobberer_url": "https://api.pub.build.mozilla.org/clobberer/lastclobber", "locales_file": "%s/mobile/locales/l10n-changesets.json" % MOZILLA_DIR, "locales_dir": "mobile/android/locales", "ignore_locales": ["en-US"], - "nightly_build": False, + "nightly_build": True, 'balrog_credentials_file': 'oauth.txt', "tools_repo": "https://hg.mozilla.org/build/tools", "tooltool_config": { "manifest": "mobile/android/config/tooltool-manifests/android/releng.manifest", "output_dir": "%(abs_work_dir)s/" + MOZILLA_DIR, }, "exes": { 'tooltool.py': '/builds/tooltool.py', @@ -43,17 +45,17 @@ config = { "hg_l10n_tag": "default", 'vcs_share_base': "/builds/hg-shared", "l10n_dir": "l10n-central", "repack_env": { # so ugly, bug 951238 "LD_LIBRARY_PATH": "/lib:/tools/gcc-4.7.2-0moz1/lib:/tools/gcc-4.7.2-0moz1/lib64", "MOZ_OBJDIR": "obj-l10n", - "EN_US_BINARY_URL": EN_US_BINARY_URL, + "EN_US_BINARY_URL": os.environ.get("EN_US_BINARY_URL", EN_US_BINARY_URL), "LOCALE_MERGEDIR": "%(abs_merge_dir)s/", "MOZ_UPDATE_CHANNEL": "try", # XXX Invalid }, "upload_branch": "%s-android-api-15" % BRANCH, "ssh_key_dir": "~/.ssh", "merge_locales": True, "mozilla_dir": MOZILLA_DIR, "mozconfig": "%s/mobile/android/config/mozconfigs/android-api-15/l10n-nightly" % MOZILLA_DIR,

I filed that as bug 1355846 last time I pushed to try (for the compare-locales update)

With Callek's help, I've got things green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7c30c47a80eac920f2575f1c29f85bb29af8aa23 My memory just failed me, and testing this is subtle: I didn't want abspath, I wanted vdir-relative paths. Patch inbound.

Comment on attachment 8861638 [details] Bug 1254355 - Follow-up: Use VPATH relative paths to match GENERATED_FILES targets. https://reviewboard.mozilla.org/r/133630/#review136518

Comment on attachment 8861683 [details] Bug 1254355 - Part 2: Generate AndroidManifest.xml using GENERATED_FILES. gps already r+ed this; Mozreview is just terrible at re-landing after backout.

Comment on attachment 8861684 [details] Bug 1254355 - Post: Make android-* jobs depend on more of the build. gps already r+ed this; Mozreview is just terrible at re-landing after backout.

mcote: I am trying to autoland commits that have been backed out. I have grafted the backed out commits back (twice!) and pushed them back for review. Apparently I didn't push enough for review the second time (I pushed just the commit that changed) and mozreview has forgotten that I have r+ from gps on the two earlier commits. I set them to r+ in Bugzilla, but mozreview doesn't care. How do I reland (using autoland, since we're not supposed to push to inbound these days) without wasting the time of my reviewer, who are probably already justifiably annoyed with this ticket? This process is extremely frustrating!

https://hg.mozilla.org/integration/mozilla-inbound/rev/c556baa9feb8a9beae76a667458dc32043c3a4b7 Bug 1254355 - Part 2: Generate AndroidManifest.xml using GENERATED_FILES. r=gps,mshal https://hg.mozilla.org/integration/mozilla-inbound/rev/0c3b3c4e0ba6efdb3be17c10319e9967e076d46f Bug 1254355 - Post: Make android-* jobs depend on more of the build. r=gps

https://hg.mozilla.org/integration/mozilla-inbound/rev/b1bd89334ee5a2f92aa087e2c1f28f19ded0d399 Bug 1254355 - Follow-up: Use #ifdef, not #if, for MOZ_CRASHREPORTER. r=gps

https://hg.mozilla.org/mozilla-central/rev/c556baa9feb8 https://hg.mozilla.org/mozilla-central/rev/0c3b3c4e0ba6 https://hg.mozilla.org/mozilla-central/rev/b1bd89334ee5

Chatted with nalexander on IRC. Gist is that, once landed, MozReview commit series can't really be reused to handle backouts; a new series should be created, either in a new bug or with a new review ID (see http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview/commits.html#bug-references-review-identifiers-and-grouping-commits).