Closed Bug 1349773 Opened 4 years ago Closed 4 years ago

Thunderbird 53.0b1 Linux repacks fail


(Thunderbird :: Build Config, defect)

53 Branch
Not set


(Not tracked)



(Reporter: wsmwk, Assigned: Fallen)



(1 file, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #1349771 +++

All Linux repacks fail
command: output:
Deleting /builds/slave/tb-rel-c-beta-lx_rpk_1-0000000/comm-beta/obj-l10n/mail/locales/merged
command: START
command: python compare-locales/scripts/compare-locales -m /builds/slave/tb-rel-c-beta-lx_rpk_1-0000000/comm-beta/obj-l10n/mail/locales/merged comm-beta/mail/locales/l10n.ini l10n br
command: cwd: /builds/slave/tb-rel-c-beta-lx_rpk_1-0000000
command: env: {'PYTHONPATH': 'compare-locales/lib'}
command: output:
adding to /builds/slave/tb-rel-c-beta-lx_rpk_1-0000000/comm-beta/obj-l10n/mail/locales/merged/dom/chrome/netErrorApp.dtd
adding to /builds/slave/tb-rel-c-beta-lx_rpk_1-0000000/comm-beta/obj-l10n/mail/locales/merged/editor/ui/chrome/dialogs/EditorReplace.dtd
adding to /builds/slave/tb-rel-c-beta-lx_rpk_1-0000000/comm-beta/obj-l10n/mail/locales/merged/mail/chrome/messenger/
adding to /builds/slave/tb-rel-c-beta-lx_rpk_1-0000000/comm-beta/obj-l10n/mail/locales/merged/mail/chrome/messenger/removeAccount.dtd
adding to /builds/slave/tb-rel-c-beta-lx_rpk_1-0000000/comm-beta/obj-l10n/mail/locales/merged/mail/chrome/messenger/addressbook/abCardOverlay.dtd
adding to /builds/slave/tb-rel-c-beta-lx_rpk_1-0000000/comm-beta/obj-l10n/mail/locales/merged/mail/chrome/messenger/addressbook/abMailListDialog.dtd
adding to /builds/slave/tb-rel-c-beta-lx_rpk_1-0000000/comm-beta/obj-l10n/mail/locales/merged/mail/chrome/messenger/addressbook/abMainWindow.dtd
adding to /builds/slave/tb-rel-c-beta-lx_rpk_1-0000000/comm-beta/obj-l10n/mail/locales/merged/mail/chrome/messenger/addressbook/
adding to /builds/slave/tb-rel-c-beta-lx_rpk_1-0000000/comm-beta/obj-l10n/mail/locales/merged/mail/chrome/messenger/preferences/compose.dtd
adding to /builds/slave/tb-rel-c-beta-lx_rpk_1-0000000/comm-beta/obj-l10n/mail/locales/merged/toolkit/chrome/passwordmgr/
      ERROR: Unparsed content "

<!-- This file exists to allow applications to override one or more messages
     from netError.dtd; Applications which want to do this should override
     this file with their own version of netErrorApp.dtd -->

<!-- An example (from Firefox):" at 211-458
      ERROR: Unparsed content "
" at 1161-1166
Attached patch buildbotcustom Patch - v1 (obsolete) β€” β€” Splinter Review
The problem here is we are using an old version of compare-locales. We should switch to running mach compare-locales instead. With this patch, we will be running the following, tested using a simple runner that mocks WithProperties and by copying out a few class variables:

['basedir/objdir-tb/_virtualenv/bin/python', 'basedir/mozilla/mach', 'compare-locales', '--merge-dir', 'basedir/work/merged', 'br']

It leaves out setting l10n.ini and the l10n base dir, ideally these are taken from defaults provided by logic and mozconfig from mach.

It is not quite clear to me what special case should be used on windows, 5 months ago bug 1310229 used the virtualenv python, while 10 months ago bug 1262760 uses the hard-coded mozilla-build python. I went with the more recent version.

As a side note, I noticed that mach compare-locales always clobbers the merge directory. This should be causing trouble at where lightning expects to just clobber the calendar subdirectory. I might have to file a bug (with patch) to add a --no-clobber option to mach compare-locales and make use of that in Lightning. I'm surprised this isn't a (notable) issue for us.
Assignee: nobody → philipp
Attachment #8854627 - Flags: review?(rail)
(In reply to Philipp Kewisch [:Fallen] from comment #1)
> As a side note, I noticed that mach compare-locales always clobbers the
> merge directory.

Ok, it seems this isn't quite true, I had gone just by the description. It clobbers subdirectories if they already exist, which is fine. I'm not quite sure if we could run into a situation where subdirectories from an old run still exist. We might want to revert the removal of the rm step.

I should also probably add --l10n-ini and maybe --l10n-base explicitly, in investigating the --no-clobber suggestion I had a situation where it used the wrong l10n.ini path due to the usual m-c/c-c split issues. Too tired to do this today though.
Comment on attachment 8854627 [details] [diff] [review]
buildbotcustom Patch - v1

Review of attachment 8854627 [details] [diff] [review]:

DIAF! :)
I think the only consumer of those calls is Thunderbird automation.
Attachment #8854627 - Flags: review?(rail) → review+
Attached patch Fix - v2 β€” β€” Splinter Review
Here is the patch with the l10n options passed as announced. The right directories are making my head spin, but I think with the convoluted code it will be hard to verify manually. I did the usual testing in a sandbox with hopefully correct variable names.
Attachment #8854627 - Attachment is obsolete: true
Attachment #8854872 - Flags: review+

rail, I'd appreciate if you could take care of the reconfig etc.
Flags: needinfo?(rail)
Flags: needinfo?(rail)
Closed: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.