464154 - L10n repackages for hg release automation

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

[Armen [:armenzg]]

This bug can track all other bugs needed to be completed to have l10n repackages in parallel on release automation

Comment 3

[[e-mail] (:bhearsum)]

I'm making good progress on this. I'm currently working on bug 455578 to get uploading working. I'll be posting patches for this bug once upload support is done.

Comment 4

[[e-mail] (:bhearsum)]

Attachment: refactor RepackFactory to support nightly and release builds

I've given this a good go on staging, I'm pretty confident it's ok. Mostly this is a refactor with little functionality change to nightly repacks. I've improved the source checkout a little bit, and removed the requirement to run configure twice (by running autoconf directly instead of having configure do it).

The release stuff is very much based on the repack script I used for 3.1b1 and 3.1b2 (https://bugzilla.mozilla.org/attachment.cgi?id=341938). The upload target has not landed yet, but will, before this patch does.

Comment 5

[[e-mail] (:bhearsum)]

Attachment: master side patch

This patch switches the Nightly builds over to the NightlyRepackFactory and adds l10n support to the release builds. Those work a little differently than their Bootstrap counterparts, as they live in separate factories. I think this is a healthier way to do it, anyways.

Comment 6

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 353871 [details] [diff] [review]
master side patch

>diff -r 15d3ac74f64d mozilla2-staging/release_master.py
>--- a/mozilla2-staging/release_master.py	Thu Dec 18 09:23:07 2008 -0500
>+    repack_scheduler = DependentL10n(
>+        name='%s_repack' % platform,
>+        upstream=build_scheduler,
>+        builderNames=['%s_repack' % platform],
>+        repoType='hg',
>+        repoPath='mozilla-central',

This should be controlled by a variable in release_config.py, right ? Some part of l10nCentral I think.

>+    repack_factory = ReleaseRepackFactory(
>+        sourceRepo=nightly_config.HGURL,
>+        branch='mozilla-central',
>+        project=productName,
>+        repoPath='mozilla-central',
>+        l10nRepoPath='l10n-central',

Similar stuff here, so looks like we need a bug to remove mozilla-central hard wiring from the release stuff.

r+ on the assumption that you follow up on that (and I didn't grievously misunderstand how this works).

Comment 7

[Armen [:armenzg]]

Comment on attachment 353868 [details] [diff] [review]
refactor RepackFactory to support nightly and release builds

>         self.addStep(ShellCommand,
>          command=['sh', '-c',
>           WithProperties('if [ -d %(locale)s ]; then ' +
>                          'hg -R %(locale)s pull -r tip ; ' +
>                          'else ' +
>                          'hg clone ' +
>-                         'http://hg.mozilla.org/'+l10nRepoPath+'/%(locale)s/ ; ' +
>-                         'fi ' +
>-                         '&& hg -R %(locale)s update')],
>+                         sourceRepo+'/'+l10nRepoPath+'/%(locale)s/ ; ' +
>+                         'fi')],
Can we keep on updating the locale:
&& hg -R %(locale)s update')],
on the NightlyFactory updateSource's?

>+        self.addStep(ShellCommand,
>+         command=['bash', '-c', 'autoconf-2.13'],
This used to fail for me on the macs that were on staging since they did not have the symlink to autoconf213 and that is why I used "make -f client.mk configure" since it would auto-determine the right one for me. Is this still a problem?


>         self.addStep(ShellCommand,
>-         command=['make', '-f', 'client.mk', 'configure'],
>-         env={'CONFIGURE_ARGS': '--enable-application=browser'},
>+         command=['bash', '-c', 'autoconf-2.13'],
>          haltOnFailure=True,
>-         descriptionDone='autoconf',
>-         workdir='build/'+branch
>+         descriptionDone=['autoconf js'],
>+         workdir='build/'+branch+'/js/src'
>         )
What is this second autoconf step for? would the nightly be running it as well? what is the js/src thing?


>         self.addStep(Compile,
>          command=['sh', '--',
>                   './configure', '--enable-application=browser',
>-                  '--disable-compile-environment',
I need at some point this but I believe it might have only been for my local machine since I lacked the environment to do a build. I guess it is OK to remove for release machines

>-                  '--with-l10n-base=../l10n-central'],
>+                  '--with-l10n-base=../%s' % l10nRepoPath],
Thanks for fixing this


>+
>+        self.downloadBuilds()
>+        self.doRepack()
>+
>+        uploadEnv = {
>+            'MOZ_PKG_PRETTYNAMES': '1',
Is the same naming as releases going to be used for nightly repackages? (I probably don't understand properly the meaning of this variable)

>+            'UPLOAD_TO_TMP': '1',


>+        self.addStep(ShellCommand,
>+         command=['make', WithProperties('l10n-upload-%(locale)s')],
Do we have this make target?

The rest of the patch seems good to me.

The split of the Factory looks good but r- because of the first of my comments

Comment 8

[[e-mail] (:bhearsum)]

(In reply to comment #7)
> Can we keep on updating the locale:
> && hg -R %(locale)s update')],
> on the NightlyFactory updateSource's?
> 

Yeah, good point.

> This used to fail for me on the macs that were on staging since they did not
> have the symlink to autoconf213 and that is why I used "make -f client.mk
> configure" since it would auto-determine the right one for me. Is this still a
> problem?
> 

I bit the bullet here and created autoconf-2.13 symlinks on our Macs. This will be fine now.

> What is this second autoconf step for? would the nightly be running it as well?
> what is the js/src thing?
> 

Spidermonkey (js) recently move to an autoconf build system. We need to generate 'configure' for it as well.

> 
> >         self.addStep(Compile,
> >          command=['sh', '--',
> >                   './configure', '--enable-application=browser',
> >-                  '--disable-compile-environment',
> I need at some point this but I believe it might have only been for my local
> machine since I lacked the environment to do a build. I guess it is OK to
> remove for release machines
> 

Yeah, it's totally fine on build machines.

> >+        uploadEnv = {
> >+            'MOZ_PKG_PRETTYNAMES': '1',
> Is the same naming as releases going to be used for nightly repackages? (I
> probably don't understand properly the meaning of this variable)
> 

Good point about this. We certainly don't want to use MOZ_PKG_PRETTYNAMES for nightlies.

> >+            'UPLOAD_TO_TMP': '1',
> 
> 
> >+        self.addStep(ShellCommand,
> >+         command=['make', WithProperties('l10n-upload-%(locale)s')],
> Do we have this make target?
> 

I'm about to post the patch for this - it will land before this one does.

Comment 9

[Axel Hecht [:Pike]]

The js configure shouldn't be required for l10n repacks.

We only need the the configure with compile enabled to create the update tools, js on the other hand should really only be required for the actual en-US binaries.

I don't really understand the configure vs make -f client.mk configure piece. To me the distinction is mostly on whether we have a mozconfig or not. Is that not so? Or could this be the configure target being confused by having a new .mozconfig compared to .mozconfig.mk or so due the mozconfig setup? I'm wondering what the bigger picture is here, and about which is the "most commonly used code path".

Comment 10

[Armen [:armenzg]]

(In reply to comment #9)
> I don't really understand the configure vs make -f client.mk configure piece.
> To me the distinction is mostly on whether we have a mozconfig or not. Is that
> not so? Or could this be the configure target being confused by having a new
> .mozconfig compared to .mozconfig.mk or so due the mozconfig setup? I'm
> wondering what the bigger picture is here, and about which is the "most
> commonly used code path".

I used "make configure" because the mac slaves on staging when I was doing my testing did not had the symlink to autoconf213 and the step would fail in them. I only used "make configure" as a workaround but it is not needed if the symlink is all the mac slaves.
Using autoconf-2.13 is the way to go if all slaves have the symlink

Comment 11

[[e-mail] (:bhearsum)]

(In reply to comment #9)
> The js configure shouldn't be required for l10n repacks.
> 

Yeah, you're probably right. I'm going to be double checking this today.

> We only need the the configure with compile enabled to create the update tools,
> js on the other hand should really only be required for the actual en-US
> binaries.

Yes, that's true. I don't see any reason to have nightlies not run configure the same way, though.

> 
> I don't really understand the configure vs make -f client.mk configure piece.
> To me the distinction is mostly on whether we have a mozconfig or not. Is that
> not so? Or could this be the configure target being confused by having a new
> .mozconfig compared to .mozconfig.mk or so due the mozconfig setup? I'm
> wondering what the bigger picture is here, and about which is the "most
> commonly used code path".

Configure is run manually here because we share a mozconfig with the en-US release builds, in the release case. The gets complicated with 'client.mk configure' because of universal builds on Mac. Running configure manually gets around those checks. I still think this is a better thing to do than have yet-another-mozconfig, though.

Comment 12

[[e-mail] (:bhearsum)]

Comment on attachment 353868 [details] [diff] [review]
refactor RepackFactory to support nightly and release builds

cancelling review, there will be a new patch coming

Comment 13

[[e-mail] (:bhearsum)]

Attachment: more locales/Makefile.in fixes, l10n-upload-% target.

There's a few small fixes to some Makefiles here that were either missed in previous patches or re-broken by MOZ_PKG_DIR:
* properly set defaults for ZIP_IN and WIN32_INSTALLER_IN. Note that I have removed the repackage-zip-%: ZIP_IN=$(_ABS_DIST)/$(PACKAGE) and equivalent installer rule. With these in it appears to be impossible to override them.
* Fix STAGEDIST to use MOZ_PKG_DIR instead of MOZ_PKG_APPNAME
* Move MOZ_PKG_DIR to package-name.mk so update-packaging can use it easily
* Add shortcuts for MARs and langpacks (COMPLETE_MAR = $path/$basename.complete.mar, etc.)
* Put langpacks in the correct directory
* Fix PACKAGE_DIR in update-packaging to use MOZ_PKG_DIR

The other thing here is obviously the l10n-upload-% target. I pretty much just copied the packager.mk one. I'm not entirely happy with calling it 4 times but after 6 hours of trying to make ifneq ($(wildcard ...)) work I had to give up and do it this way.

Comment 14

[[e-mail] (:bhearsum)]

Attachment: enable release repacks, l10n-upload-% target

Alright, here's an updated version of this patch. Changes since the previous patch:
* Update l10n repositories to 'default' for nightly repacks.
* Typo fixes

Note that I've kept the autoconf-2.13 in js/src - when running ./configure by hand it is definitely required - configure errors out otherwise.

I ran extensive tests on this. Note that I used mozilla-1.9.1 because it has a larger number of locales which actually build right now:
* Nightly builds on all platforms based off of mozilla-1.9.1 w/ the patch in attachment 354317 [details] [diff] [review]. Results are here:
http://staging-master.build.mozilla.org:8010/waterfall?builder=Linux mozilla-1.9.1 l10n nightly&builder=OS X 10.5.2 mozilla-1.9.1 l10n nightly&builder=WINNT 5.2 mozilla-1.9.1 l10n nightly
Builds are here:
http://staging-stage.build.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-1.9.1-l10n/
* A run of release builds based on 3.1b2. Results are here:
http://staging-master.build.mozilla.org:8010/waterfall?builder=linux_repack&builder=macosx_repack&builder=win32_repack
Builds are here:
http://staging-stage.build.mozilla.org/pub/mozilla.org/firefox/nightly/3.1b2-candidates/build1/unsigned/win32/

The long and short is that many nightly locales built fine - including all of the ones currently listed as green on the l10n dashboard. For the release builds, everything built fine except pt-PT. release/l10n-mozilla-1.9.1/pt-PT doesn't appear to have FIREFOX_3_1b2_RELEASE tags in it - so it's definitely not something related to this patch causing it. (win32 builds are still finishing up, but not having any issues with them)

Comment 15

[[e-mail] (:bhearsum)]

(In reply to comment #6)
> Similar stuff here, so looks like we need a bug to remove mozilla-central hard
> wiring from the release stuff.
>

Just filed https://bugzilla.mozilla.org/show_bug.cgi?id=470966 on this. I'll pick it up after the break.

Comment 16

[Axel Hecht [:Pike]]

Comment on attachment 354317 [details] [diff] [review]
more locales/Makefile.in fixes, l10n-upload-% target.

>diff -r 7127f726f661 browser/locales/Makefile.in
<...>
> 
>+l10n-upload-%: AB_CD=$*
>+l10n-upload-%:
>+	$(PYTHON) $(topsrcdir)/build/upload.py --base-path $(DIST) "$(DIST)/$(PACKAGE)"
>+	if test -f $(DIST)/$(COMPLETE_MAR); then \
>+	$(PYTHON) $(topsrcdir)/build/upload.py --base-path $(DIST) $(DIST)/$(COMPLETE_MAR); \
>+	fi
>+	if test -f $(DIST)/$(LANGPACK); then \
>+	$(PYTHON) $(topsrcdir)/build/upload.py --base-path $(DIST) $(DIST)/$(LANGPACK); \
>+	fi
>+ifeq (WINNT, $(OS_ARCH))
>+	$(PYTHON) $(topsrcdir)/build/upload.py --base-path $(DIST) "$(INSTALLER_PACKAGE)"
>+endif

r=me, with nits on the actual l10n-upload rule:

I'd prefer to be safe here rather than sorry, and actually error on missing files. That is, don't if test -f for both the mar and the langpack, and protect the COMPLETE_MAR by 
ifdef MOZ_MAKE_COMPLETE_MAR
as in the repackage target.

That might require that we expose that variable in another step in the factory, but this way, we're working on a consistent set of deliverables.

Comment 17

[Axel Hecht [:Pike]]

PS: sh if clauses in the commands should be indented with '\t  ' etc, for future reference.

Comment 18

[[e-mail] (:bhearsum)]

Attachment: l10n-upload-% target, nits fixed

Same thing as before, better erroring out in l10n-upload-% (thanks Pike).

Comment 19

[[e-mail] (:bhearsum)]

Attachment: ReleaseRepacks - minor change for l10n-upload-% nits

This is the same as before, except we'll be passing MOZ_MAKE_COMPLETE_MAR to l10n-upload-% as per Pike's review on that patch.

Comment 20

[Chris Cooper [:coop] (he/him)]

Comment on attachment 354323 [details] [diff] [review]
ReleaseRepacks - minor change for l10n-upload-% nits

>+    def downloadBuilds(self):
>+        # OMG CLEAN THIS SHIT UP
>+        # We need to know the absolute path to the input builds when we repack,
>+        # so we need retrieve at run-time as a build property

Perhaps not surprisingly, this is the only section I have issue with. Is there a cleaner way to get the abs path, in which case, why don't we just do that? The comment seems over-the-top if it's just something we need to live with.

r+ otherwise.

Comment 21

[[e-mail] (:bhearsum)]

(In reply to comment #20)
> (From update of attachment 354323 [details] [diff] [review])
> >+    def downloadBuilds(self):
> >+        # OMG CLEAN THIS SHIT UP
> >+        # We need to know the absolute path to the input builds when we repack,
> >+        # so we need retrieve at run-time as a build property
> 
> Perhaps not surprisingly, this is the only section I have issue with. Is there
> a cleaner way to get the abs path, in which case, why don't we just do that?
> The comment seems over-the-top if it's just something we need to live with.
> 
> r+ otherwise.

I can't believe I left this in. I spent a few hours last week trying to make this better and I don't have an immediate solution. I'll replace this comment with something less harsh in hopes I'll get to it in January.

Comment 22

[Armen [:armenzg]]

Comment on attachment 354323 [details] [diff] [review]
ReleaseRepacks - minor change for l10n-upload-% nits

>+        for dir in ('nsprpub', 'config'):
>+            self.addStep(ShellCommand,
>+             command=['make'],
>+             workdir='build/'+self.branch+'/'+dir,
>+             description=['make ' + dir],
>+             haltOnFailure=True
>+            )
As I mentioned to you in IRC, can we leave a comment regarding this?
>+        self.downloadBuilds()
>+        self.doRepack()
I believe you are "making" nsprpub and config again in the Release's doRepack method

r+ with fixing the last comment

Comment 23

[Axel Hecht [:Pike]]

(In reply to comment #21)
> 
> I can't believe I left this in. I spent a few hours last week trying to make
> this better and I don't have an immediate solution. I'll replace this comment
> with something less harsh in hopes I'll get to it in January.

Can we re-use the wget-en-US target here?

Comment 24

[[e-mail] (:bhearsum)]

(In reply to comment #23)
> (In reply to comment #21)
> > 
> > I can't believe I left this in. I spent a few hours last week trying to make
> > this better and I don't have an immediate solution. I'll replace this comment
> > with something less harsh in hopes I'll get to it in January.
> 
> Can we re-use the wget-en-US target here?

Yeah, I suppose that's a possibility. In the interest of making this work I don't want to make that switch right now. I'll make a point to look into it in January, though.

Comment 25

[Axel Hecht [:Pike]]

Yeah, that was meant to be more like a "hopefully we can make wget-en-US do the job in Jan"

Comment 26

[(not currently active) Ted Mielczarek]

Comment on attachment 354322 [details] [diff] [review]
l10n-upload-% target, nits fixed

+	$(PYTHON) $(topsrcdir)/build/upload.py --base-path $(DIST) $(DIST)/$(COMPLETE_MAR); \

What's up with the semicolon and line continuation there?

Comment 27

[[e-mail] (:bhearsum)]

(In reply to comment #26)
> (From update of attachment 354322 [details] [diff] [review])
> +    $(PYTHON) $(topsrcdir)/build/upload.py --base-path $(DIST)
> $(DIST)/$(COMPLETE_MAR); \
> 
> What's up with the semicolon and line continuation there?

AFAICT it's required:
bitters-2:tmp bhearsum$ make
if test -d /Users; then \
	  echo "/Users exists" \
	fi
/bin/sh: -c: line 1: syntax error: unexpected end of file
make: *** [a] Error 2
bitters-2:tmp bhearsum$ vim Makefile 
bitters-2:tmp bhearsum$ make
if test -d /Users; then \
	  echo "/Users exists"; \
	fi
/Users exists

Admittedly, I'm no Makefile pro.

Comment 28

[[e-mail] (:bhearsum)]

Attachment: master-side patch for production

Totally forgot about this yesterday =\

Comment 29

[[e-mail] (:bhearsum)]

Comment on attachment 354322 [details] [diff] [review]
l10n-upload-% target, nits fixed

Pushed to mozilla-central & mozilla-1.9.1 (tracemonkey isn't doing l10n, so we don't need it there yet - hopefully they'll pick it up before we do).

m-c: changeset:   22997:42d7cb939283
m-1.9.1: changeset:   22472:25c62f20137f

Comment 30

[[e-mail] (:bhearsum)]

Comment on attachment 354323 [details] [diff] [review]
ReleaseRepacks - minor change for l10n-upload-% nits

Checking in process/factory.py;
/cvsroot/mozilla/tools/buildbotcustom/process/factory.py,v  <--  factory.py
new revision: 1.65; previous revision: 1.64
done

Comment 31

[[e-mail] (:bhearsum)]

Comment on attachment 354393 [details] [diff] [review]
master-side patch for production

changeset:   616:48f1625f3452

Comment 32

[[e-mail] (:bhearsum)]

Comment on attachment 353871 [details] [diff] [review]
master side patch

changeset:   616:48f1625f3452

Comment 33

[[e-mail] (:bhearsum)]

This stuff is all checked in now, but I haven't been able to get a run of it on production because the Dependent schedulers are currently busted.

I'm very confident it will be fine but I'll leave this open until we get a run of it.

Comment 34

[[e-mail] (:bhearsum)]

(In reply to comment #26)
> (From update of attachment 354322 [details] [diff] [review])
> +    $(PYTHON) $(topsrcdir)/build/upload.py --base-path $(DIST)
> $(DIST)/$(COMPLETE_MAR); \
> 
> What's up with the semicolon and line continuation there?

Of course, when I actually checked this in I realized what you meant. Had to checkin a bustage fix removing it.

Comment 35

[[e-mail] (:bhearsum)]

Looks like this stuff worked fine over the weekend.

Comment 37

[Robert Kaiser]

I filed bug 474805 for some improvements that should be made based on what this bug has introduced.