702302 - Create a single-locale repack magic

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Axel Hecht [:Pike]]

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]]

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]]

PS: same for xulrunner?

Comment 3

[Doug Turner (:dougt)]

pike - it can be removed but it orthogonal to fixing this bug.

Comment 4

[Axel Hecht [:Pike]]

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]]

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]]

Attachment: 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?)]

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]]

(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)]

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?)]

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]]

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 (not active)]

Did this get merged to mozilla-central along with the rest of Birch?
Birch is no longer in use.

Comment 13

[Axel Hecht [:Pike]]

http://hg.mozilla.org/mozilla-central/rev/f8c62105b2f9 on central. Seems like birch didn't merge, but rebase.