Closed Bug 1496293 Opened Last year Closed 11 months ago

[Tier2] Android nightlies bustages Execution failed for task ':app:syncPreprocessedResForOfficialWithoutGeckoBinariesNoMinApiPhotonDebug'

Categories

(Firefox Build System :: Android Studio and Gradle Integration, defect)

defect
Not set

Tracking

(firefox64 fixed)

VERIFIED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: CosminS, Assigned: nalexander)

References

Details

(Whiteboard: [stockwell disable-recommended])

Attachments

(1 file)

This looks like a random build failure, but it isn't -- it's an error in localization that we surface really poorly.  E.g.,

https://treeherder.mozilla.org/logviewer.html#?job_id=203248901&repo=mozilla-central&lineNumber=1449

[task 2018-10-03T23:55:27.094Z] 23:55:27     INFO -  :app:syncPreprocessedResForOfficialWithoutGeckoBinariesNoMinApiPhotonDebug[Fatal Error] :1052:75: The entity "fxaccount_status_choose_what" was referenced, but not declared.

or

https://treeherder.mozilla.org/logviewer.html#?job_id=203248909&repo=mozilla-central&lineNumber=1521

[task 2018-10-03T23:55:57.878Z] 23:55:57     INFO -  :app:syncPreprocessedResForOfficialWithoutGeckoBinariesNoMinApiPhotonDebug[Fatal Error] :1333:71: The entity "pref_media_autoplay_allow" was referenced, but not declared.

Bug 1478411 tracks surfacing the error in a way that's useful for sheriffs.  I'll push that forward right now, since it's roughly equivalent to fixing this issue.

In the meantime, Delphine, could you work your magic and try to figure out how to update locales that are busted for Fennec right now?  Many thanks.
Flags: needinfo?(nalexander) → needinfo?(lebedel.delphine)
I'm not sure what to do here, because the strings cited above have not been translated in the locales for which there is an error showing up.

I don't understand why this is an error. CC'ing Pike in case he can make sense of this.
Flags: needinfo?(lebedel.delphine) → needinfo?(l10n)
(In reply to Delphine Lebédel [:delphine - use Need Info] from comment #3)
> I'm not sure what to do here, because the strings cited above have not been
> translated in the locales for which there is an error showing up.

This is far more interesting, then.  It's my understanding that untranslated strings are intended to "fall back" to the en-US strings.  This error suggests that is not happening.

Drilling into the various failures, I can confirm that https://hg.mozilla.org/l10n-central/zam/file/b254873449242129b9bb7391ca602fc2562d7cfd does not, in fact, define `fxaccount_status_choose_what`.

Now, why that is a problem is unclear.  My guess is that we're not invoking the relevant GENERATED_FILES scripts with the correct (merged) DTD files, and indeed, I see this in the logs.  In my local $OBJDIR/mobile/android/base/backend.mk, I have:

libs:: $(MDDEPDIR)/res/values/strings.xml.stub
res/values/strings.xml: $(MDDEPDIR)/res/values/strings.xml.stub ;
GARBAGE += res/values/strings.xml
GARBAGE += $(MDDEPDIR)/res/values/strings.xml.stub
EXTRA_MDDEPEND_FILES += res/values/strings.xml.pp
$(MDDEPDIR)/res/values/strings.xml.stub: /Users/nalexander/Mozilla/gecko/python/mozbuild/mozbuild/action/generate_strings_xml.py $(srcdir)/strings.xml.in $(call MERGE_RELATIVE_FILE,android_strings.dtd,mobile/android/base/locales) $(call MERGE_RELATIVE_FILE,sync_strings.dtd,mobile/android/base/locales) $(if $(IS_LANGUAGE_REPACK),FORCE)
	$(REPORT_BUILD)
	$(call py_action,file_generate,--locale=$(AB_CD) /Users/nalexander/Mozilla/gecko/python/mozbuild/mozbuild/action/generate_strings_xml.py main res/values/strings.xml $(MDDEPDIR)/res/values/strings.xml.pp $(MDDEPDIR)/res/values/strings.xml.stub $(srcdir)/strings.xml.in $(call MERGE_RELATIVE_FILE,android_strings.dtd,mobile/android/base/locales) $(call MERGE_RELATIVE_FILE,sync_strings.dtd,mobile/android/base/locales))
	@$(TOUCH) $@

Note all the MERGE_RELATIVE_FILE invocations.  However, in the logs (say https://treeherder.mozilla.org/logviewer.html#?job_id=203248898&repo=mozilla-central&lineNumber=9865, which has a single failing locale), I see:

[task 2018-10-03T23:55:24.442Z] 23:55:24     INFO -  /builds/worker/workspace/build/src/obj-firefox/_virtualenvs/init/bin/python -m mozbuild.action.file_generate --locale=ta /builds/worker/workspace/build/src/python/mozbuild/mozbuild/action/generate_strings_xml.py main res/values/strings.xml .deps/res/values/strings.xml.pp .deps/res/values/strings.xml.stub /builds/worker/workspace/build/src/mobile/android/base/strings.xml.in /builds/worker/workspace/build/l10n-central/ta/mobile/android/base/android_strings.dtd /builds/worker/workspace/build/l10n-central/ta/mobile/android/base/sync_strings.dtd

Those inputs are _not_ the merged files.  That leads us to https://searchfox.org/mozilla-central/source/config/config.mk#392-411, which is possibly my least favourite block of code in the tree.

So I conclude that yes, this is build system error.  I cannot investigate further at this time.  It's my understanding that we're going to turn off these single-locale repacks entirely (Bug 1468368), and I'd really rather do that (and expunge this whole class of problem) rather than wade back in to the MERGE*FILE macros again.

Axel has, I believe, changed the semantics of merging locales such that we no longer need to fallback to en-US for incomplete locales.  If that's changed, maybe we can simplify this stuff and patch this over?
The last green N1 was https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&resultStatus=success,busted&searchStr=android,4.0,api16%2B,opt,localised,repacks,nightly-l10n-android-api-16-nightly-1%2Fopt,l10n(n1)&selectedJob=203065553&tochange=317b91c1fbd51e48238d05bb3e90663245e751f3&fromchange=d770ea2a1b257febbbbe07a38163d66bdc47e1fd.  So there's a much wider window of changesets to consider.  I will try to look at this over the weekend, but I think it _extremely_ unlikely that the android-common.mk changes are responsible.  It's much more likely that we touched something while tweaking GENERATED_FILES or tup that is relevant.
Keeping the needinfo for now. I did a preliminary investigation, and we're calling l10n-merge, but it's failing and not showing the expected outcome. I'll need to dig into a number of other issues before I can turn my local clones to a fennec build config to investigate. If someone wants to beat me to it, the `make merge-ach` should show the same output as it does in the multi-locale build, where things just seem to work.

        $(if $(filter en-US,$(AB_CD)),, $(MAKE) merge-$*)

works for chrome-%, but not for installers-% -> libs-%.
(In reply to Axel Hecht [:Pike] from comment #8)
> Keeping the needinfo for now. I did a preliminary investigation, and we're
> calling l10n-merge, but it's failing and not showing the expected outcome.
> I'll need to dig into a number of other issues before I can turn my local
> clones to a fennec build config to investigate. If someone wants to beat me
> to it, the `make merge-ach` should show the same output as it does in the
> multi-locale build, where things just seem to work.
> 
>         $(if $(filter en-US,$(AB_CD)),, $(MAKE) merge-$*)
> 
> works for chrome-%, but not for installers-% -> libs-%.

Axel, thanks for digging into this.  Indeed, I see

[task 2018-10-03T10:47:00.709Z] 10:47:00     INFO -  /builds/worker/workspace/build/src/obj-firefox/_virtualenvs/init/bin/python -m mozbuild.action.file_generate --locale=ach /builds/worker/workspace/build/src/python/mozbuild/mozbuild/action/generate_strings_xml.py main res/values/strings.xml .deps/res/values/strings.xml.pp .deps/res/values/strings.xml.stub /builds/worker/workspace/build/src/mobile/android/base/strings.xml.in /builds/worker/workspace/build/src/obj-firefox/mobile/android/locales/merge-dir/ach/mobile/android/base/android_strings.dtd /builds/worker/workspace/build/src/obj-firefox/mobile/android/locales/merge-dir/ach/mobile/android/base/sync_strings.dtd

Which _is_ the correct location (note the `merge-dir`) in the last green N1 from

https://treeherder.mozilla.org/logviewer.html#?job_id=203065553&repo=mozilla-central

So somehow something changed in that timeframe.  What on earth does this mean?

[task 2018-10-03T23:52:11.434Z] 23:52:11     INFO -  /builds/worker/workspace/build/src/mach compare-locales  --merge /builds/worker/workspace/build/src/obj-firefox/mobile/android/locales/merge-dir /builds/worker/workspace/build/src/mobile/android/locales/l10n.toml /builds/worker/workspace/build/l10n-central sq
[task 2018-10-03T23:52:11.645Z] 23:52:11     INFO -  Tests have been disabled by mozconfig with the flag"ac_add_options --disable-tests".
[task 2018-10-03T23:52:11.645Z] 23:52:11     INFO -  Remove the flag, and re-compile to enable tests.
[task 2018-10-03T23:52:11.657Z] 23:52:11     INFO -  /builds/worker/workspace/build/src/toolkit/locales/l10n.mk:193: recipe for target 'merge-sq' failed

Ah, I think I see now.  I think `mach compare-locales` is mis-classified as `testing`: https://searchfox.org/mozilla-central/rev/80ac71c1c54af788b32e851192dfd2de2ec18e18/tools/compare-locales/mach_commands.py#22.  And Bug 1291335 changed the handling of the `testing` category to error out when `--disable-tests` is set in the mozconfig.  Indeed, I see https://hg.mozilla.org/mozilla-central/rev/b58cfd180ba7 in our pushlog range.

I think the simplest solution is to audit commands in the `testing` category and to reclassify `compare-locales`.
Blocks: 1291335
(In reply to Nick Alexander :nalexander [he/him] from comment #11)
> I think the simplest solution is to audit commands in the `testing` category
> and to reclassify `compare-locales`.

That would be great. If that doesn't work out, we could certainly backout 1291335 and reconsider the criteria for requiring --enable-tests.
(In reply to Geoff Brown [:gbrown] from comment #12)
> (In reply to Nick Alexander :nalexander [he/him] from comment #11)
> > I think the simplest solution is to audit commands in the `testing` category
> > and to reclassify `compare-locales`.
> 
> That would be great. If that doesn't work out, we could certainly backout
> 1291335 and reconsider the criteria for requiring --enable-tests.

I think we should make `compare-locales` part `build`, since it's used by `mach build`.  Axel, can you do this, and can you make |make merge-%| failures actually break the build?  I don't know why they don't right now but that doesn't seem to be the right choice.  I don't know off-hand how to achieve this, and maybe you have more context on why this is so.  Historically merging could fail but building could succeed?
There are different use-cases for compare-locales, and some want to use it as linter.

So in that case, it makes sense to fail on errors in the data.

In the case of l10n-merge, those data errors shouldn't fail the build.

The data errors are frequent, and I haven't seen any processing errors in a long time, so I'm optimizing for the linter case, and return error codes on data errors. Which in return means that automation not interested in data errors should ignore the return value of the c-l process.

I don't have a lot of open cycles right now myself, sorry. I'd be happy if someone could take this bug.
Flags: needinfo?(l10n)
This is follow-up to Bug 1291335, which inadvertently prevented Fennec
single-locale repacks from running |mach compare-locales|, since the
command was incorrectly considered part of the `testing` category.
Pushed by nalexander@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/84e2040e592a
Let Fennec single-locale repacks run |mach compare-locales|. r=gbrown
https://hg.mozilla.org/mozilla-central/rev/84e2040e592a
Status: NEW → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
Assignee: nobody → nalexander
Latest android single-locale nightlies are full of green.

There's one failure for cak, but that should be unrelated. Looks like a premature end of a json file, no idea. I'd let that go fo a nightly or two to see if it's reproducible.
Status: RESOLVED → VERIFIED
(In reply to Axel Hecht [:Pike] from comment #21)
> Latest android single-locale nightlies are full of green.
> 
> There's one failure for cak, but that should be unrelated. Looks like a
> premature end of a json file, no idea. I'd let that go fo a nightly or two
> to see if it's reproducible.

Didn't happen again in the past few nightlies, so that's nothing l10n-related.
Product: Firefox for Android → Firefox Build System
Target Milestone: Firefox 64 → mozilla64
You need to log in before you can comment on or make changes to this bug.