Last Comment Bug 702302 - Create a single-locale repack magic
: Create a single-locale repack magic
Status: RESOLVED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: ARM Android
: P1 normal (vote)
: ---
Assigned To: Axel Hecht [:Pike]
:
Mentors:
Depends on: 701833
Blocks: 700289 659544 698425 701068 705137 709233
  Show dependency treegraph
 
Reported: 2011-11-14 09:14 PST by Axel Hecht [:Pike]
Modified: 2012-01-09 11:37 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
11+


Attachments
do multi- and single-locale builds (23.57 KB, patch)
2011-12-01 09:47 PST, Axel Hecht [:Pike]
wjohnston2000: review+
blassey.bugs: review+
Details | Diff | Splinter Review

Description Axel Hecht [:Pike] 2011-11-14 09:14:47 PST
We need build-system support to create single-locale repacks.

Really repackaging an apk sounds too tricky and brittle, so the idea is to do something like we're doing for tests:

Add a target to bundle files from the en-US build, and get that uploaded.
Use that bundle (and possibly the existing apk?) to do single locale packages.

The strings for that should go into res/values/strings.xml, too, so that we don't have to rely on locale setting to get them picked up from res/values-foo.

Depends on figuring out where the files actually are, see bug 701833, and blocks the release automation of this, bug 698425
Comment 1 Axel Hecht [:Pike] 2011-11-24 07:29:48 PST
I'm trying to read mobile/android/installer/Makefile.in right now, I wonder:

Is it a fair first step to remove all the deb stuff in there? Like, the android app will never ever do any of that, right?
Comment 2 Axel Hecht [:Pike] 2011-11-24 07:30:35 PST
PS: same for xulrunner?
Comment 3 Doug Turner (:dougt) 2011-11-25 00:29:59 PST
pike - it can be removed but it orthogonal to fixing this bug.
Comment 4 Axel Hecht [:Pike] 2011-11-30 08:48:57 PST
I'm making some progress here, but right now, I'm still ending up with a browser that has a broken active tab due to browser.properties not loading right in browser.js. Native UI and CSS errors work.

The good news is, no additional package required so far.
Comment 5 Axel Hecht [:Pike] 2011-11-30 16:24:49 PST
OK, found the problem, I got a working single-locale build, and the multis should be fine, too. Next up, clean up the patch.
Comment 6 Axel Hecht [:Pike] 2011-12-01 09:47:24 PST
Created attachment 578289 [details] [diff] [review]
do multi- and single-locale builds

So, this patch does a few things:

- remove old l10n-*.inis that aren't used no more, xul and android
- fix regressions in l10n.ini and filter.py from the migration, xul and android
- make single and multi locale packages work, just android
- strip some deb package code from android
- fix the values-* directories for he and id (bug 700289, but why not here)
- get rid of intermediate stuff for android_strings.dtd
- make l10n-merge work for android_strings.dtd (didn't work before, AFAICT)

For the single-locale builds, the regular repack magic will work,
make wget-en-US
make merge-* LOCALE_MERGEDIR=$PWD/merge
make installers-* LOCALE_MERGEDIR=$PWD/merge

I tested the android multi-locale builds, matchOS works on both native UI and chrome. I also tested the single-locale builds on different OS selection, i.e., the German build works on a polish android setting.

There's a caveat or two:
Right now, I need to destroy the stage in each repack. That's due to the stage post-processing in INNER_MAKE_PACKAGE, in particular the file moves.
That means that the setup cost per repack is a good deal higher, but still far from doing a build.
Also, properties aren't stripped in repacks, because the POST_STAGE isn't called in INNER_MAKE_PACKAGE.
We should fix that in a follow-up, IMHO. Move the post-stage to post-stage, and move the properties-strip to INNER_MAKE_PACKAGE.

This patch has no impact on being able to localize both native and xul.
Comment 7 Brad Lassey [:blassey] (use needinfo?) 2011-12-02 12:04:58 PST
Comment on attachment 578289 [details] [diff] [review]
do multi- and single-locale builds

Review of attachment 578289 [details] [diff] [review]:
-----------------------------------------------------------------

basically r+, but I'd like to get answers to my two questions before marking r+

::: mobile/android/locales/Makefile.in
@@ +101,4 @@
>  clobber-zip:
>  	$(RM) $(STAGEDIST)/chrome/$(AB_CD).jar \
>  	  $(STAGEDIST)/chrome/$(AB_CD).manifest \
> +	  $(STAGEDIST)/defaults/pref/mobile-l10n.js

why this rename?

::: toolkit/mozapps/installer/packager.mk
@@ +325,2 @@
>  else
> +GECKO_APP_AP_PATH = $(_ABS_DIST)/../mobile/android/base

_ABS_DIST isn't the right thing here. $(DEPTH)/embedding/android would be right. To get a absolute path from that you can do $(shell cd $(DEPTH)/embedding/android; pwd)

@@ +351,5 @@
>    cd $(MOZ_PKG_DIR) && \
>    $(UNZIP) $(UNPACKAGE) && \
> +  mv lib/$(ABI_DIR)/libmozutils.so . && \
> +  mv lib/$(ABI_DIR)/*plugin-container* $(MOZ_CHILD_PROCESS_NAME) && \
> +  rm -rf lib/$(ABI_DIR)

why this change?
Comment 8 Axel Hecht [:Pike] 2011-12-02 13:07:40 PST
(In reply to Brad Lassey [:blassey] from comment #7)
> Comment on attachment 578289 [details] [diff] [review] [diff] [details] [review]
> do multi- and single-locale builds
> 
> Review of attachment 578289 [details] [diff] [review] [diff] [details] [review]:
> -----------------------------------------------------------------
> 
> basically r+, but I'd like to get answers to my two questions before marking
> r+
> 
> ::: mobile/android/locales/Makefile.in
> @@ +101,4 @@
> >  clobber-zip:
> >  	$(RM) $(STAGEDIST)/chrome/$(AB_CD).jar \
> >  	  $(STAGEDIST)/chrome/$(AB_CD).manifest \
> > +	  $(STAGEDIST)/defaults/pref/mobile-l10n.js
> 
> why this rename?

Not really a rename, but an addition. mobile-l10n.js contains per-locale information, and thus it's a good idea to remove it as part of the clobber-zip step, instead of relying just to have it overwritten.

> ::: toolkit/mozapps/installer/packager.mk
> @@ +325,2 @@
> >  else
> > +GECKO_APP_AP_PATH = $(_ABS_DIST)/../mobile/android/base
> 
> _ABS_DIST isn't the right thing here. $(DEPTH)/embedding/android would be
> right. To get a absolute path from that you can do $(shell cd
> $(DEPTH)/embedding/android; pwd)

I'm pretty sure this is not embedding/android for the native UI case, but mobile/android/base. I'm happy to convert to
$(call core_abspath,$(DEPTH)/...)
in both cases, though.

> @@ +351,5 @@
> >    cd $(MOZ_PKG_DIR) && \
> >    $(UNZIP) $(UNPACKAGE) && \
> > +  mv lib/$(ABI_DIR)/libmozutils.so . && \
> > +  mv lib/$(ABI_DIR)/*plugin-container* $(MOZ_CHILD_PROCESS_NAME) && \
> > +  rm -rf lib/$(ABI_DIR)
> 
> why this change?

In INNER_MAKE_PACKAGE, there's

    mkdir -p lib/$(ABI_DIR) && \
    mv libmozutils.so $(MOZ_CHILD_PROCESS_NAME) lib/$(ABI_DIR) && \

which is unbalanced in INNER_UNMAKE_PACKAGE right now. The funky think is that $(MOZ_CHILD_PROCESS_NAME) actually has a directory in there, too, thus it's a bit more tricky to unmove. This would go away, in both MAKE and UNMAKE, when we do the follow up, stacking the rename into POST_STAGE.
Comment 9 Wesley Johnston (:wesj) 2011-12-05 01:23:32 PST
Comment on attachment 578289 [details] [diff] [review]
do multi- and single-locale builds

Review of attachment 578289 [details] [diff] [review]:
-----------------------------------------------------------------

Digging through the bits I've dealt with before, this looks fine to me.
Comment 10 Brad Lassey [:blassey] (use needinfo?) 2011-12-05 10:10:17 PST
Comment on attachment 578289 [details] [diff] [review]
do multi- and single-locale builds

clobber-zip step, instead of relying just to have it overwritten.
> 
> > ::: toolkit/mozapps/installer/packager.mk
> > @@ +325,2 @@
> > >  else
> > > +GECKO_APP_AP_PATH = $(_ABS_DIST)/../mobile/android/base
> > 
> > _ABS_DIST isn't the right thing here. $(DEPTH)/embedding/android would be
> > right. To get a absolute path from that you can do $(shell cd
> > $(DEPTH)/embedding/android; pwd)
> 
> I'm pretty sure this is not embedding/android for the native UI case, but
> mobile/android/base. I'm happy to convert to
> $(call core_abspath,$(DEPTH)/...)
> in both cases, though.
Right, for native its mobile/android/base, I'm just not a fan fo the use of _ABS_DIST here, so please convert both of them

> 
> > @@ +351,5 @@
> > >    cd $(MOZ_PKG_DIR) && \
> > >    $(UNZIP) $(UNPACKAGE) && \
> > > +  mv lib/$(ABI_DIR)/libmozutils.so . && \
> > > +  mv lib/$(ABI_DIR)/*plugin-container* $(MOZ_CHILD_PROCESS_NAME) && \
> > > +  rm -rf lib/$(ABI_DIR)
> > 
> > why this change?
> 
> In INNER_MAKE_PACKAGE, there's
> 
>     mkdir -p lib/$(ABI_DIR) && \
>     mv libmozutils.so $(MOZ_CHILD_PROCESS_NAME) lib/$(ABI_DIR) && \
> 
> which is unbalanced in INNER_UNMAKE_PACKAGE right now. The funky think is
> that $(MOZ_CHILD_PROCESS_NAME) actually has a directory in there, too, thus
> it's a bit more tricky to unmove. This would go away, in both MAKE and
> UNMAKE, when we do the follow up, stacking the rename into POST_STAGE.
ok, can't say I follow completely but I'll trust you here.
Comment 11 Axel Hecht [:Pike] 2011-12-06 11:28:07 PST
https://hg.mozilla.org/projects/birch/rev/5d205868839c, marking FIXED.

I had to do one more slight fix to AB_rCD in mobile/android/base/locales before landing, that just moved the ifdef into a $(if), which fixed an issue with the chrome-% target and the he/id edgecases. Just tested those now and they didn't work as expected.
Comment 12 Aki Sasaki [:aki] 2011-12-07 23:46:58 PST
Did this get merged to mozilla-central along with the rest of Birch?
Birch is no longer in use.
Comment 13 Axel Hecht [:Pike] 2011-12-08 01:47:18 PST
http://hg.mozilla.org/mozilla-central/rev/f8c62105b2f9 on central. Seems like birch didn't merge, but rebase.

Note You need to log in before you can comment on or make changes to this bug.