Closed Bug 1607193 Opened 2 years ago Closed 2 years ago

Drop l10n-check from automation and build


(Release Engineering :: Release Automation: L10N, task)

Not set


(Not tracked)



(Reporter: Pike, Assigned: tomprince)


(Blocks 1 open bug)



(3 files)

Spin-off from bug 1036563.

Callek, now that we have l10n builds on try, I wonder if the l10n-check step is actually providing significant value.

IIRC, I added it to ensure that repacks would work in their most basic form. They should find the right packages, and do at least some modifications. I guess it also defines what the minimal set of a buildable localization should be, but then again, it's also not really all that interesting. And hopefully, at some point that minimal file for the build config will go away, too, as we move everything to Fluent.

What's your take?

If we want to keep it, we should probably enable it via a white-list instead of the current black-list, as that seems to be overdoing it. Is there a good existing configuration to hook in to?

Flags: needinfo?(bugspam.Callek)

I'd be happy to remove l10n-check at this point. I don't recall the last time l10n-check failed in a way that impacts l10n nightlies/releases without also affecting l10n nightlies/releases. We can manually test the real jobs on try, and we can test a different l10n job on try as well.

If you do think there is value here we have: that can be edited to help switch from blacklist to whitelist. I'm not sure what configs we definitely want this on for atm though.

Flags: needinfo?(bugspam.Callek)

Ok, let's drop l10n-check. I think this is mostly ripping it out of automation, so I'll keep the bug here. I'd be happy to see the actual make target die as a ride-along ;-)

Summary: Keep l10n-check or at least ensure it's running on a whitelist → Drop l10n-check from automation and build

Justin, could you please take of this? We are spending $$$ on this for no good reason if I understood correctly

Flags: needinfo?(bugspam.Callek)

This disables the l10n-check as part of the build, now that we have
on-push L10n jobs.

Keywords: leave-open
Assignee: nobody → mozilla
Pushed by
Disable l10n-check by default; r=Callek,firefox-build-system-reviewers,mshal

The irony is that 24 hours after this landed, l10n nightlies are busted, and l10n-check would likely have caught the problem earlier.

Let's see. I've triggered the L10n builds on, right before the backout.

If they're broken, we should make sure they get run on changes like the ones you talked about.

I pushed the broken tree with a backout of this bug on try and it didn't break l10n-check, so... I guess l10n-check was not testing the full pipeline anyways... but we do need /something/ to be able to backfill, because that doesn't exist at the moment. And the best sheriffs can do is guesswork.

It's not covering the full pipeline, yeah, but in this case, it doesn't test the MOZ_LANGPACK_CONTRIBUTORS code path in There's hope for the L10n builds to actually break for you on this ;-)

Dang, while there's a failed build in every N, none of the locales in browser/locales/all-onchange-changesets.json have no contributors set, and thus didn't trigger the bug.

(In reply to Axel Hecht [:Pike] from comment #11)

Dang, while there's a failed build in every N, none of the locales in browser/locales/all-onchange-changesets.json have no contributors set, and thus didn't trigger the bug.

We should probably write tests (in a new bug) that account for this, possibly as separate than the L10n/Nightly-L10n builds.

Redirecting my n-i to mshal, since I think we want to remove the in-tree code for this, but I'm not certain that it will end up forefront on my plate anytime soon.

Flags: needinfo?(bugspam.Callek) → needinfo?(mshal)
Flags: needinfo?(mshal)
Pushed by
Remove MOZ_AUTOMATION_L10N_CHECK; r=firefox-build-system-reviewers,rstewart
Remove l10n-check; r=firefox-build-system-reviewers,rstewart
Closed: 2 years ago
Keywords: leave-open
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.