Closed Bug 464154 Opened 11 years ago Closed 11 years ago

L10n repackages for hg release automation

Categories

(Release Engineering :: General, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: armenzg, Assigned: bhearsum)

References

Details

Attachments

(4 files, 3 obsolete files)

This bug can track all other bugs needed to be completed to have l10n repackages in parallel on release automation
Depends on: 464035
Depends on: 464156
Depends on: 464157
Depends on: 464161
Blocks: 464165
Depends on: 453840
Summary: L10n repackages on hg-automation → L10n repackages for hg release automation
Blocks: 433930
Assignee: nobody → bhearsum
Status: NEW → ASSIGNED
Priority: -- → P2
Depends on: 464244
Duplicate of this bug: 464160
Duplicate of this bug: 464157
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.
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.
Attachment #353868 - Flags: review?(ccooper)
Attachment #353868 - Flags: review?(armenzg)
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.
Attachment #353871 - Flags: review?(nthomas)
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).
Attachment #353871 - Flags: review?(nthomas) → review+
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
Attachment #353868 - Flags: review?(armenzg) → review-
(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.
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".
(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
(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.
Attachment #353868 - Flags: review?(ccooper)
Comment on attachment 353868 [details] [diff] [review]
refactor RepackFactory to support nightly and release builds

cancelling review, there will be a new patch coming
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.
Attachment #354317 - Flags: review?(ted.mielczarek)
Attachment #354317 - Flags: review?(l10n)
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)
Attachment #353868 - Attachment is obsolete: true
Attachment #354318 - Flags: review?(ccooper)
(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 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.
Attachment #354317 - Flags: review?(l10n) → review+
PS: sh if clauses in the commands should be indented with '\t  ' etc, for future reference.
Same thing as before, better erroring out in l10n-upload-% (thanks Pike).
Attachment #354317 - Attachment is obsolete: true
Attachment #354322 - Flags: review?(ted.mielczarek)
Attachment #354317 - Flags: review?(ted.mielczarek)
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.
Attachment #354318 - Attachment is obsolete: true
Attachment #354323 - Flags: review?(ccooper)
Attachment #354318 - Flags: review?(ccooper)
Attachment #354323 - Flags: review?(armenzg)
Attachment #354323 - Flags: review?(ccooper) → review+
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.
(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 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
Attachment #354323 - Flags: review?(armenzg) → review+
(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?
(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.
Yeah, that was meant to be more like a "hopefully we can make wget-en-US do the job in Jan"
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?
Attachment #354322 - Flags: review?(ted.mielczarek) → review+
(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.
Totally forgot about this yesterday =\
Attachment #354393 - Flags: review?(ccooper)
Attachment #354393 - Flags: review?(ccooper) → review+
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
Attachment #354322 - Flags: checked‑in+
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
Attachment #354323 - Flags: checked‑in+
Comment on attachment 354393 [details] [diff] [review]
master-side patch for production

changeset:   616:48f1625f3452
Attachment #354393 - Flags: checked‑in+
Comment on attachment 353871 [details] [diff] [review]
master side patch

changeset:   616:48f1625f3452
Attachment #353871 - Flags: checked‑in+
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.
(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.
Looks like this stuff worked fine over the weekend.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Duplicate of this bug: 464244
I filed bug 474805 for some improvements that should be made based on what this bug has introduced.
Blocks: 475120
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.