Bug 287262 (locales-repack)

build packages for locales by starting with en-US complete packages

RESOLVED FIXED in Firefox1.5

Status

()

Firefox
Build Config
P1
normal
RESOLVED FIXED
13 years ago
6 years ago

People

(Reporter: Chase Phillips, Assigned: Benjamin Smedberg)

Tracking

Trunk
Firefox1.5
x86
All
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: still needs tinderbox work from Chase, see attachment 182704)

Attachments

(8 attachments, 6 obsolete attachments)

164.48 KB, patch
dveditz
: review+
Details | Diff | Splinter Review
27.74 KB, patch
gandalf
: review+
Details | Diff | Splinter Review
15.51 KB, patch
Chase Phillips
: review+
Details | Diff | Splinter Review
956 bytes, patch
Axel Hecht
: review+
Details | Diff | Splinter Review
1.32 KB, patch
Chase Phillips
: review+
Details | Diff | Splinter Review
6.17 KB, patch
Benjamin Smedberg
: review+
Details | Diff | Splinter Review
4.59 KB, patch
Details | Diff | Splinter Review
4.14 KB, patch
gandalf
: review+
Details | Diff | Splinter Review
(Reporter)

Description

13 years ago
The current build process for creating a copy of Firefox in any locale requires
building the source from scratch (or reusing a set of built files) and then
packaging those files with the locale files.  We should add a new process to our
build system that allows the build to start with a set of en-US package files
(maybe just the .zip or maybe the .zip and the .exe), unpackages them, then
repackages the files for a new locale.

I see the human process for managing the builds occurring something like this:

  * While the trunk/branch is in open development, the l10n build systems build
    as they do today by checking out the CVS code.

  * When we get code complete in en-US, we take the generated files from the
    en-US build systems and place them in a special location on the l10n build
    systems that the build process recognizes as holding finished products.

  * Upon seeing these files, the build process knows to skip checking out most
    of the source code (for example, maybe it only checks out the locale files)
    and instead unpackages the files.

  * The build process then picks up at the point where the locale files are
    packaged with the product files (from the en-US build), completes the
    build, and moves on to the next locale.

  * The resulting files that are uploaded are sent to the FTP site and are
    essentially the same product files as the en-US files, hence having the
    same Talkback ID and symbols as en-US users.
(Assignee)

Updated

13 years ago
Target Milestone: --- → Firefox1.1
Version: 1.0 Branch → Trunk
(Reporter)

Comment 1

13 years ago
This would alleviate any "flukes" for l10n that have a tendency of happening
during the build process (ie. skipping any problems we could run into during the
checkout and build phase, any possibility that the symbol upload failed or got
hung up, &c).

Also, bsmedberg points out that this would let localizers build versions of
their locale from CVS without needing things like VC6 (though they'd still need
other components, like the Cygwin tools).
(Assignee)

Updated

13 years ago
Depends on: 287273
(Reporter)

Comment 2

13 years ago
Ideally I'd like for as little of this code to be in the Tinderbox client
scripts as possible.  It's okay if it invokes a different client.mk target so
long as most of the work isn't happening in "separate" Tinderbox Perl code. 
People that want to use this should be able to without having to set up their
own Tinderbox clients.
No longer depends on: 287273
(Assignee)

Updated

13 years ago
Depends on: 287273
(Reporter)

Comment 3

13 years ago
One way this could work would be:

  1. Drop build file into main build directory.
  2. Run:
       * make -f client.mk unpackage

At this point dist/ would be recreated based on the contents of the
(exe|zip)/tar.gz/dmg and the other build targets that repackage files (like
running make in browser/installer) would work as normal.
(Reporter)

Comment 4

13 years ago
Setting blocking-aviary1.1? for triage.
Flags: blocking-aviary1.1?
(Assignee)

Updated

13 years ago
Assignee: nobody → benjamin
(Assignee)

Updated

13 years ago
Status: NEW → ASSIGNED
Priority: -- → P1
(Assignee)

Updated

13 years ago
Depends on: 289461
(Assignee)

Comment 5

13 years ago
Created attachment 180075 [details] [diff] [review]
Work in progress

This is just a snapshot of work in progress, it uses the new XPI_NAME facility
to leverage the standard libs:: target. This might actually be ready to land,
so that I can do the unpack-repack stages next.
Attachment #180075 - Flags: review?(chase)
(Assignee)

Updated

13 years ago
Alias: locales-repack

Comment 6

13 years ago
try for 1.1
Flags: blocking-aviary1.1? → blocking-aviary1.1+
(Assignee)

Comment 7

13 years ago
Created attachment 180701 [details] [diff] [review]
Move all localized strings from config.ini to install.ini, rev. 1

Dan, this patch is a lot less complex than it appears. It basically creates a
wrapper function around GetPrivateProfileString and replaces {VAR} from
config.ini with the appropriate message from install.ini.

GetConfigIniProfileString requires careful review, but I've run it through a
little testsuite and the installer also appears fine. The rest is just renaming
and build-config support.
Attachment #180701 - Flags: review?(dveditz)
(Assignee)

Comment 8

13 years ago
The reason why this is required, by the way, is that I don't have a good way to
re-create config.ini after I unpack it from the installer, because it contains
dynamically-generated XPI size information; recreating install.ini is easy,
because it is just run through the preprocessor.
(Assignee)

Comment 9

13 years ago
Created attachment 180717 [details] [diff] [review]
Move all localized strings from config.ini to install.ini, rev. 1.1

Further running shows a problem when there are two {} substitutions in one
line, and I forgot -N for the browser install.ini file.
Attachment #180701 - Attachment is obsolete: true
Attachment #180717 - Flags: review?(dveditz)
(Assignee)

Updated

13 years ago
Attachment #180701 - Flags: review?(dveditz)
Comment on attachment 180717 [details] [diff] [review]
Move all localized strings from config.ini to install.ini, rev. 1.1

looks good except for a readability nit about GetConfigIniProfileString (via
IRC). r- waiting for that update.
Attachment #180717 - Flags: review?(dveditz) → review-
(Assignee)

Comment 11

13 years ago
Created attachment 180741 [details] [diff] [review]
Move all localized strings from config.ini to install.ini, rev. 1.2 [checked in]

Your comments provoked some happy changes:
1) use of _alloca instead of the fixed-length buffer solves the problem of
aMaxLen being longer than MAX_BUF in the future
2) I can cut off the length of the replacement using GetPrivateProfileString
directly, instead of munging it afterwards, which makes the code a lot easier
to read.
Attachment #180717 - Attachment is obsolete: true
Attachment #180741 - Flags: review?(dveditz)
Comment on attachment 180741 [details] [diff] [review]
Move all localized strings from config.ini to install.ini, rev. 1.2 [checked in]

r=dveditz
Attachment #180741 - Flags: review?(dveditz) → review+
(Assignee)

Updated

13 years ago
Attachment #180741 - Flags: approval-aviary1.1a?

Comment 13

13 years ago
Comment on attachment 180741 [details] [diff] [review]
Move all localized strings from config.ini to install.ini, rev. 1.2 [checked in]

a=asa
Attachment #180741 - Flags: approval-aviary1.1a? → approval-aviary1.1a+
(Assignee)

Updated

13 years ago
Attachment #180741 - Attachment description: Move all localized strings from config.ini to install.ini, rev. 1.2 → Move all localized strings from config.ini to install.ini, rev. 1.2 [checked in]
(Assignee)

Comment 14

13 years ago
Created attachment 180897 [details] [diff] [review]
Make en-US.manifest contain all the locale registration, rev. 1 [checked in]

This moves the locale registration for all of en-US.jar into en-US.manifest,
from installed-chrome.txt. Thunderbird doesn't need any changes because of the
work already completed in bug 289448. The autoconfig changes remove an
extraneous $(REGCHROME) from the makefile which was totally unnecessary since
we now regchrome from the jar.mn manifests.
Attachment #180897 - Flags: review?(gandalf)
(Assignee)

Comment 15

13 years ago
Created attachment 181046 [details] [diff] [review]
Repackage win32 7zip installers, rev. 1

This patch depends on the previous one to work correctly.
Attachment #181046 - Flags: review?(gandalf)
Attachment #180897 - Flags: review?(gandalf) → review+
Attachment #181046 - Flags: review?(gandalf) → review+
(Assignee)

Updated

13 years ago
Attachment #180897 - Flags: approval-aviary1.1a?
(Assignee)

Comment 16

13 years ago
Comment on attachment 181046 [details] [diff] [review]
Repackage win32 7zip installers, rev. 1

Chase, the tinderbox script would have to run "make -C browser/locales
repackage-win32-installer-%locale%"

And if we wanted to repackage a particular build (e.g. a release) it would call

"make -C browser/locales repackage-win32-installer-%locale%
WIN32_INSTALLER_IN=/cygdrive/c/path/to/win32.installer.exe
WIN32_INSTALLER_OUT=/cygdrive/c/path/to/distribute/to/shipname.exe"
Attachment #181046 - Flags: superreview?(chase)
Attachment #181046 - Flags: approval-aviary1.1a?

Comment 17

13 years ago
Comment on attachment 180897 [details] [diff] [review]
Make en-US.manifest contain all the locale registration, rev. 1 [checked in]

a=asa
Attachment #180897 - Flags: approval-aviary1.1a? → approval-aviary1.1a+
(Assignee)

Comment 18

13 years ago
Comment on attachment 180897 [details] [diff] [review]
Make en-US.manifest contain all the locale registration, rev. 1 [checked in]

en-US.manifest changes committed, with an additional fix for
extensions/reporter.
Attachment #180897 - Attachment description: Make en-US.manifest contain all the locale registration, rev. 1 → Make en-US.manifest contain all the locale registration, rev. 1 [checked in]
(Reporter)

Comment 19

13 years ago
Comment on attachment 180075 [details] [diff] [review]
Work in progress

Obsoleted by later patch.
Attachment #180075 - Attachment is obsolete: true
Attachment #180075 - Flags: review?(chase)
(Assignee)

Comment 20

13 years ago
Created attachment 181356 [details] [diff] [review]
Repackage win32 7zip installers, rev. 2

This does win32 installers and ZIP builds.

It *should* do unix tarballs, but I haven't tested it.

It will produce errors for mac, until I can transfer it to my mac machine and
do some hdiutil-loving.

It should not have any affect on the default build (it's
localization-repackaging only).
Attachment #181046 - Attachment is obsolete: true
Attachment #181356 - Flags: review?(chase)
(Reporter)

Comment 21

13 years ago
Comment on attachment 181356 [details] [diff] [review]
Repackage win32 7zip installers, rev. 2

r=chase with the caveat that I want us to be in a place where the same l10n
modifications we've been using in post-mozilla-rel.pl on the aviary branch will
work for us on the trunk.
Attachment #181356 - Flags: review?(chase) → review+
(Assignee)

Comment 22

13 years ago
Created attachment 181403 [details] [diff] [review]
Update client.mk with additional locale directories, rev. 1 [checked in]
(Assignee)

Updated

13 years ago
Attachment #181046 - Flags: superreview?(chase)
Attachment #181046 - Flags: approval-aviary1.1a?

Comment 23

13 years ago
Fixed

Updated

13 years ago
Attachment #181403 - Flags: review+
(Assignee)

Updated

13 years ago
Attachment #181403 - Flags: approval-aviary1.1a?
Comment on attachment 181403 [details] [diff] [review]
Update client.mk with additional locale directories, rev. 1 [checked in]

a=shaver
Attachment #181403 - Flags: approval-aviary1.1a? → approval-aviary1.1a+
(Assignee)

Updated

13 years ago
Attachment #181403 - Attachment description: Update client.mk with additional locale directories, rev. 1 → Update client.mk with additional locale directories, rev. 1 [checked in]
Please add nl-NL to the all-locales list.

Thanks
Please add ru to the all-locales list.
(In reply to comment #25)
> Please add nl-NL to the all-locales list.
> 
> Thanks

i copied all to "nl"
(Assignee)

Comment 28

13 years ago
Created attachment 181458 [details] [diff] [review]
Ported from bug 279768

This is an old patch from bug 279768 that I never approved because it didn't
have testing on cygwin and AS perl. I have been told that both perls work, so
it can be committed now.
Attachment #181458 - Flags: review?(chase)
Attachment #181458 - Flags: approval-aviary1.1a?
(Reporter)

Updated

13 years ago
Attachment #181458 - Flags: review?(chase) → review+
Attachment #181458 - Flags: approval-aviary1.1a? → approval-aviary1.1a+
(Assignee)

Comment 29

13 years ago
Created attachment 181552 [details] [diff] [review]
Tbox support for configurable comparison dirs, rev. 1

Chase, this one's for you to test and commit: it allows you to configure the
list of directories that should be compared using compare-locales.pl, instead
of hardcoding the list. This means that you can use the same tinderbox script
for the aviary branch
@CompareLocaleDirs = ("toolkit", "browser");
and for trunk Firefox
@CompareLocaleDirs = ("netwerk", "dom", "toolkit", "security/manager",
"browser", "other-licenses/branding/firefox", "extensions/reporter");
Attachment #181552 - Flags: review?(chase)
(In reply to comment #27)
> (In reply to comment #25)
> > Please add nl-NL to the all-locales list.
> > 
> > Thanks
> 
> i copied all to "nl"

Thanks for adding it but how do i get a tinderbox page for it? when it was nl-NL
it was added to the same page als the branch builds. 

http://tinderbox.mozilla.org/showbuilds.cgi?tree=Mozilla-l10n-nl does not excist :-(

MM

Comment 31

13 years ago
I tried to create the nl tinderbox page, but it still gives me a server error.
Either I did it wrong, or it needs a cycle to come up.
(In reply to comment #31)
> I tried to create the nl tinderbox page, but it still gives me a server error.
> Either I did it wrong, or it needs a cycle to come up.

thanks for trying but it still don't work, is there sombody who can fix it?
(Reporter)

Comment 33

13 years ago
(In reply to comment #32)
> (In reply to comment #31)
> > I tried to create the nl tinderbox page, but it still gives me a server error.
> > Either I did it wrong, or it needs a cycle to come up.
> 
> thanks for trying but it still don't work, is there sombody who can fix it?

There was an error in the Mozilla-l10n-nl tree's configuration.  I recreated it
based on the Mozilla-l10n-ru tree and it's accessible now.
(Assignee)

Updated

13 years ago
Whiteboard: still needs tinderbox work from Chase, see attachment 181552
(Assignee)

Comment 34

13 years ago
In addition, the "chroma" trunk tboxen are red because my scripts are looking for

dist/install/sea/firefox-1.0+.en-US.win32.installer.exe

And the tinderbox build system is apparently creating

dist/install/sea/SetupGeneric.exe

I'm not sure why.
(In reply to comment #33)
> (In reply to comment #32)
> > (In reply to comment #31)
> > > I tried to create the nl tinderbox page, but it still gives me a server 
error.
> > > Either I did it wrong, or it needs a cycle to come up.
> > 
> > thanks for trying but it still don't work, is there sombody who can fix it?
> There was an error in the Mozilla-l10n-nl tree's configuration.  I recreated 
it
> based on the Mozilla-l10n-ru tree and it's accessible now.

there is still a error, it looks like it is not refreshing the files and still 
use the extra-jar.mn from a week ago :-(. see bug 291862

Comment 36

13 years ago
Filed Bug 292719 on that issue.
(Assignee)

Comment 37

13 years ago
Created attachment 182611 [details] [diff] [review]
Repackage mac disk images, rev. 1
Attachment #182611 - Flags: review?(chase)
(Reporter)

Comment 38

13 years ago
Comment on attachment 182611 [details] [diff] [review]
Repackage mac disk images, rev. 1

>Index: build/package/mac_osx/make-diskimage

> # mount it
> echo "mounting disk image"
>-# `hdid -nomount rw.$DMG_NAME | grep "^/dev/disk.s2" | sed -e "s?^/dev/??" -e "s/[^0-9a-z].*//"`
>-DEV_NAME=`hdid $DMG_TEMP_NAME | sed 1q | awk '{print $1}'`
>-MOUNT_DIR=`hdid $DMG_TEMP_NAME | grep Apple_HFS | awk '{print $3}'`
>+mkdir -p dmg-temp
>+DEV_NAME=`hdiutil attach -readwrite -private -noautoopen -mountpoint dmg-temp $DMG_TEMP_NAME | egrep '^/dev/' | sed 1q | awk '{print $1}'`
>+MOUNT_DIR=dmg-temp

Good move on rearranging this and making the mount point local to the build. 
Let's do this by setting MOUNT_DIR first, then using that variable in mkdir and
hdiutil calls, too.

> # copy content via ditto
> #
> # ditto fails when target has no space left or is read-only.  Let's capture
>@@ -111,9 +114,9 @@
> fi
> 
> # make sure it's not world writeable
> echo "fixing permissions"
>-chmod -R go-w ${MOUNT_DIR}
>+chmod -R -f go-w ${MOUNT_DIR} || true

Why add -f to chmod?  Let's live with the .Trashes error message for now with
the potential benefit of catching odd chmod errors that we don't expect to see.

With those fixed, r=chase.
(Reporter)

Updated

13 years ago
Attachment #182611 - Flags: review?(chase) → review-
(Assignee)

Comment 39

13 years ago
Created attachment 182671 [details] [diff] [review]
Repackage mac disk images, rev. 1.1

Final patch updated with nits from Chase.
Attachment #182611 - Attachment is obsolete: true
(Assignee)

Updated

13 years ago
Attachment #182671 - Flags: review+
(Assignee)

Comment 40

13 years ago
Created attachment 182704 [details] [diff] [review]
Tbox support for configurable comparison dirs, rev. 1.1 [checked in]

This is the final (checked in) version of attachment 181552 [details] [diff] [review].
Attachment #181552 - Attachment is obsolete: true
(Assignee)

Updated

13 years ago
Attachment #181552 - Flags: review?(chase)

Comment 41

13 years ago
https://bugzilla.mozilla.org/show_bug.cgi?id=293129#c4 says that we don't set
startpage and general.useragent.locale.

Startpage is set in 
mozilla/other-licenses/branding/firefox/content/browserconfig.properties
which should move over to locale, as we actually build that one for each locale.

g.u.l is set in
http://lxr.mozilla.org/seamonkey/source/browser/locales/en-US/firefox-l10n.js#39
http://lxr.mozilla.org/seamonkey/source/browser/app/profile/firefox.js#137
http://lxr.mozilla.org/seamonkey/source/toolkit/locales/en-US/chrome/global/intl.properties#8

We need to confirm that the -l10n.js version successfully overrides the one in
firefox.js.

Comment 42

13 years ago
I have tried the sv-SE Linux build from latest-trunk-l10n/, and about:config and
the About dialog correctly reports the "general.useragent.locale" as "sv-SE".

The start page still needs to be fixed though.
(Assignee)

Comment 43

13 years ago
Created attachment 183034 [details] [diff] [review]
Fix the start page (browserconfig.properties) [checked in]
Attachment #183034 - Flags: review?(gandalf)
Attachment #183034 - Flags: review?(gandalf) → review+
(Assignee)

Updated

13 years ago
Attachment #183034 - Flags: approval-aviary1.1a1?

Comment 44

13 years ago
Comment on attachment 183034 [details] [diff] [review]
Fix the start page (browserconfig.properties) [checked in]

a=asa
Attachment #183034 - Flags: approval-aviary1.1a1? → approval-aviary1.1a1+
(Assignee)

Updated

13 years ago
Attachment #183034 - Attachment description: Fix the start page (browserconfig.properties) → Fix the start page (browserconfig.properties) [checked in]
(Reporter)

Comment 45

13 years ago
Comment on attachment 182671 [details] [diff] [review]
Repackage mac disk images, rev. 1.1

Ugh.  I suspect the make-diskimage changes in this patch aren't working
correctly on barcelona as there have been no new suite builds since May 4.
(Reporter)

Comment 46

13 years ago
Bug 294409 tracks the lack of Mac Suite nightlies since after 2005/05/04.
Blocks: 294409
(Reporter)

Comment 47

13 years ago
(In reply to comment #45)
> (From update of attachment 182671 [details] [diff] [review] [edit])
> Ugh.  I suspect the make-diskimage changes in this patch aren't working
> correctly on barcelona as there have been no new suite builds since May 4.

Suspicions confirmed.  Revision 1.12 of make-diskimage works fine on barcelona,
1.13 doesn't.  I'd prefer we back out the make-diskimage change so that we can
get Suite Mac nightlies building again.  Any objections?

FYI, barcelona is running Mac OS X 10.2.8.
(Reporter)

Comment 48

13 years ago
bsmedberg backed out his make-diskimage change so now bug 294409 is fixed. 
Those changes are not critical for work in this bug to continue.  As such,
removing bug 294409 from the list of bugs that are blocked by this bug.
No longer blocks: 294409
(Reporter)

Updated

13 years ago
Whiteboard: still needs tinderbox work from Chase, see attachment 181552 → still needs tinderbox work from Chase, see attachment 182704
(Assignee)

Updated

13 years ago
Depends on: 297566

Comment 49

13 years ago
In the sv-SE Windows installer, the install script for browser.xpi sets Windows
registry keys with a "(en-US)" locale suffix.

I don't know the significance of this identifier, and it does not seem to have a
negative effect on a localized Firefox, but it is nevertheless a false name.

Comment 50

13 years ago
I filed bug 297813 on removing the locale name from the registry keys.

Updated

13 years ago
Flags: blocking-aviary1.1+
<bsmedberg> gandalf: it's mostly done, right? maybe we should just mark it FIXED
and file followup bugs as necessary?
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Sorry, I didn't read the 50 comments, but it looks, according to the summary,
that this bug should cover installer/tarball creation of the successfully
localized builds, so I don't think I have to search for another (open) bug. For
those who for any strange reason out there still have no notice, there aren't
any linux tarballs of successful thunderbird builds, or those builds aren't
visible from ftp, but the result is that there aren't any l10n linux builds for
thunderbird in ftp, unlike thunderbird win32 and mac builds.
> Sorry, I didn't read the 50 comments

Your lack of will to read the bug is not a excuse of ignorance. We require some
good manners and your post is an example of annoying trolling.
If it's not that clear to you, I can explain that you can't use any random bug
you choose to write your demands (because this is the way you use to "ask" for
help) just because you're to lazy to report a bug or find a proper one.
You need to log in before you can comment on or make changes to this bug.