1868342 - Perma Android bustages ERROR: The following duplicated files are not allowed: defaults/pref/mobile-l10n.js when Gecko 122 merges to Beta on 2023-12-18

Status: VERIFIED FIXED

(Core :: DOM: Security, defect, P4)

Description

[Cosmin Sabou [:CosminS]]

Central as beta-sim.

How to run these simulations.

[task 2023-12-05T09:58:24.821Z] 09:58:24     INFO -  Duplicates 33739 bytes (9423 compressed) (2 times):
[task 2023-12-05T09:58:24.821Z] 09:58:24     INFO -    res/locale/dom/dom.properties
[task 2023-12-05T09:58:24.822Z] 09:58:24     INFO -    chrome/en-US/locale/en-US/browser/overrides/dom/dom.properties
[task 2023-12-05T09:58:24.822Z] 09:58:24     INFO -    chrome/en-US/locale/en-US/global/dom/dom.properties
[task 2023-12-05T09:58:24.822Z] 09:58:24     INFO -  WARNING: Found 16 duplicated files taking 78400 bytes (22621 compressed)
[task 2023-12-05T09:58:24.822Z] 09:58:24     INFO -  ERROR: The following duplicated files are not allowed:
[task 2023-12-05T09:58:24.822Z] 09:58:24     INFO -  defaults/pref/mobile-l10n.js
[task 2023-12-05T09:58:24.822Z] 09:58:24    ERROR -  gmake[4]: *** [/builds/worker/checkouts/gecko/toolkit/mozapps/installer/packager.mk:26: stage-package] Error 1
[task 2023-12-05T09:58:24.822Z] 09:58:24     INFO -  gmake[4]: Leaving directory '/builds/worker/workspace/obj-build/mobile/android/installer'
[task 2023-12-05T09:58:24.823Z] 09:58:24    ERROR -  gmake[3]: *** [/builds/worker/checkouts/gecko/mobile/android/build.mk:14: stage-package] Error 2
[task 2023-12-05T09:58:24.823Z] 09:58:24     INFO -  gmake[3]: Target 'recurse_android-stage-package' not remade because of errors.
[task 2023-12-05T09:58:24.823Z] 09:58:24    ERROR -  gmake[2]: *** [/builds/worker/checkouts/gecko/config/recurse.mk:34: android-stage-package] Error 2
[task 2023-12-05T09:58:24.823Z] 09:58:24    ERROR -  gmake[1]: *** [/builds/worker/checkouts/gecko/config/rules.mk:361: default] Error 2

Comment 1

[Frederik Braun [:freddy]]

Doesn't affect users. Not assigning a high priority/severity. But ideally, should be fixed. Regressing bug points at Eemeli for his localization fix (thank you!), let's see when he finds the time to take a look.

Comment 2

[Cosmin Sabou [:CosminS]]

Just an fyi, this blocks tests being run on those busted Android platforms, comparison with bustages vs without bustages.

Comment 3

[Eemeli Aro [:eemeli]]

Looking at the live_backing.log from one of the busted builds has this a little earlier than the snippet posted above:

[task 2023-12-05T09:58:24.815Z] 09:58:24     INFO -  Duplicates 0 bytes:
[task 2023-12-05T09:58:24.815Z] 09:58:24     INFO -    defaults/pref/mobile-l10n.js
[task 2023-12-05T09:58:24.815Z] 09:58:24     INFO -    chrome/en-US/locale/en-US/browser/browser.properties

The bug that caused this removed the last actual content from the mobile browser.properties, so it currently contains only a comment. Are the comments ignored in these duplication comparisons? If so, the "Duplicates 0 bytes" line would suggest that both of these files end up empty after processing, and therefore they match each other.

I'm currently working on bug 1866297 that'll remove the effectively-empty mobile browser.properties, but as it's also mentioned in the toolkit's jar.mn I'd rather resolve bug 1868154 first, so that I can remove all non-browser usage of that file. I think that should sort this out?

How can I run this build/test script locally, so that I could recreate the problem for myself? The logs don't really make it clear what's the command that sets up whatever preconditions this failure has.

Comment 4

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

1. Configure the repository for a central-as-early-beta simulation.
2. The find-dupes.py command checked for the duplicate files (command with all parameters).

A check of older builds shows the comments get stripped out in the build.

Comment 5

[Eemeli Aro [:eemeli]]

I wasn't able to simulate beta locally (on a M1 MacBook Pro) due to configure error:

 0:19.79 checking for target linker...
 0:19.80 DEBUG: Executing: `/usr/bin/clang -std=gnu99 --target=aarch64-linux-android21 -fuse-ld=lld -Wl,--version --sysroot=/Users/eemeli/.mozbuild/android-ndk-r25c/toolchains/llvm/prebuilt/darwin-x86_64/sysroot --gcc-toolchain=/Users/eemeli/.mozbuild/android-ndk-r25c/toolchains/llvm/prebuilt/darwin-x86_64`
 0:19.80 DEBUG: Executing: `/usr/bin/clang -std=gnu99 --target=aarch64-linux-android21 -Wl,--version --sysroot=/Users/eemeli/.mozbuild/android-ndk-r25c/toolchains/llvm/prebuilt/darwin-x86_64/sysroot --gcc-toolchain=/Users/eemeli/.mozbuild/android-ndk-r25c/toolchains/llvm/prebuilt/darwin-x86_64`
 0:19.80 ERROR: Failed to find an adequate linker

However, I was able to recreate the error by manually emptying the mobile-l10n.js and browser.properties files and then running the find-dupes.py command after a normal local build.

Overall, this bug seems to duplicate the experience of bug 1426943, as suggested on the mobile-l10n.js comment. The fix I'm working on (removing browser.properties) should remove the current error as it'll remove this second empty file from the repo.

But is the // comment no longer left in by the tooling, as it itself suggests to be the case? And should this bug be considered fully resolved by me removing browser.properties, or is there a different proper-er fix that ought to be applied as well?

Comment 6

[Frederik Braun [:freddy]]

Most, if not all of this is a bit new to me.
I don't know about the tooling and how it deals with comments. I think removing browser.properties is preferred than relying on a hack - right?

Comment 7

[Eemeli Aro [:eemeli]]

Removing the browser.properties file is indeed appropriate, and that's happening in bug 1866297. My question was whether something ought to also be done about find-dupes.py, which is currently happy with up to one empty file in the build, but complains about a second one with the not-really-clear error experienced here.

Based on the commit history, perhaps this is a question for :glandium?

Comment 8

[Mike Hommey [:glandium]]

Yeah, it would make sense to make find-dupes not look at empty files.

Comment 9

[Serban Stanca [:SerbanS]]

Verified fixed in today’s beta simulation push.