Closed Bug 483232 Opened 15 years ago Closed 15 years ago

Create script to automate partner repacks

Categories

(Release Engineering :: General, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: coop, Assigned: coop)

References

Details

Attachments

(5 files, 1 obsolete file)

Kev is currently using some shell scripts to run the individual repacks. My first task is to write a repack script that can be run either by hand or by the release automation.

The script should allow for batch or individual repacking. It should also try to minimize disk usage if it's going to be run on the pool of slaves, although that's less critical on the Macs.
I've started on the script. Kev is getting me a repo containing the partner config data shortly (I'm using a stub for now).
Status: NEW → ASSIGNED
Priority: -- → P2
Blocks: 483233
Depends on: 483996
Attached file Script for performing partner repacks (obsolete) —
Here's the script I've written to encapsulate the partner repack logic I've gleaned from Kev.

The intention is to allow the script to be run easily by hand, or via the existing automation (Bootstrap).

The script current lives under a scripts/ dir in the partner-repacks repo on my user repo:

http://hg.mozilla.org/users/coop_mozilla.com/partner-repacks/file/tip

Once the script is reviewed, I plan to move the entire repository under http://hg.mozilla.org/build. 

The partners/ subdir contains the current partner config info I received from Kev.
First and foremost, this script adds a PartnerRepack step to Bootstrap. This step wraps the script posted as attachment 370053 [details].

I've added the ability to temporarily augment the path in RunShellCommand. This allows us to checkout tools outside the existing PATH and then use them in later steps.

I've also added the ability to make per-step additions to the CC list in SendAnnouncement(). This is useful for things like PartnerRepack for notifying people like Kev/Tomcat when repacks are done without having to bother them about the rest of the build process.
Here's log output from two recent runs:

http://staging-1.9-master.build.mozilla.org:8810/builders/partner%20repack/builds/5/steps/shell_7/logs/stdio

http://staging-1.9-master.build.mozilla.org:8810/builders/partner%20repack/builds/6/steps/shell_7/logs/stdio

Build #5 was done using the existing staging bootstrap config for 1.9. Build #7 had some local tweaks to the bootstrap config to generate 3.0.8 builds that Kev/Tomcat could compare with those made by hand.
Attachment #370053 - Flags: review?(nthomas)
Attachment #370055 - Flags: review?(nthomas)
Attachment #370057 - Flags: review?(nthomas)
Comment on attachment 370057 [details] [diff] [review]
Adding partner repack step to master.cfg (staging only)

>Index: master.cfg
>===================================================================
>RCS file: /cvsroot/mozilla/tools/buildbot-configs/automation/staging-1.9/master.cfg,v
>+partner_depscheduler = Dependent(
>+     name="partner_dep",
>+      upstream=stage_depscheduler,
>+      builderNames=["partner repack"],
>+)

Nit: Please use partner_repack to be consistent with other release builder names, here and elsewhere. r+ with that change.
Attachment #370057 - Flags: review?(nthomas) → review+
Attachment #370055 - Flags: review?(nthomas) → review+
Comment on attachment 370055 [details] [diff] [review]
Add partner repack logic to Bootstrap

>Index: Bootstrap/Step/PartnerRepack.pm
>===================================================================
>+sub Execute {
>+    my $product = $config->Get(var => 'product');

Nit: product is unused here. All four sub's have some vars which aren't used and can be removed.

>+    my $partnerRepackDir = $config->Get(var => 'partnerRepackDir');
....
>+    MkdirWithPath(dir => $partnerRepackDir)
>+      or die("Cannot mkdir $partnerRepackDir: $!");

Several other steps use a versioned dir to help prevent build2 stomping all over build1, and to give you a clean slate in that situation. Worth considering.

>+    $this->CvsCo(cvsroot => $mozillaCvsroot,
>+                 checkoutDir => 'package_tools',
>+                 modules => [CvsCatfile('mozilla', 'build',
>+                                        'package','mac_osx')],
>+                 logFile => $buildLog,
>+                 workDir => $partnerRepackDir
>+    );

Should do this on the release tag (productTag + '_RELEASE').

>+      appendToPath => "${partnerRepackDir}/package_tools",

You could use catfile here too, right ? Yearning for the shell ? :-)

>+sub Verify {
>+    my $buildLog = catfile($logDir,
>+                           'repack_' . $version . '-build-partner-repack.log');

Nit: s/-build-/-verify-/

>+sub Announce {
>+    $remoteRepackDir =~ s|^/home/ftp||;

You can actually use /pub/mozilla.org/... on stage, and so avoid a difference between file and web paths.

>+    $this->SendAnnouncement(
>+      subject => "$product $version partner repack step finished",
>+      message => "$product $version partner repacks were copied to the staging directory:\n\n" .
>+                 "http://${stagingServer}${remoteRepackDir}/${version}/build${build}/",
>+      cc => $partnerRepackCC,
>+    );

Is there a plan for how the renaming of builds into the release-style is going to be done ? That's the only major thing that seems to be missing.

r+ with the requested fixes.
Comment on attachment 370053 [details]
Script for performing partner repacks

>#########################################################################
>def shellCommand(cmd):
>    p = Popen(cmd, shell=True)
>    return os.waitpid(p.pid, 0)

Looks like many of the calls to shellCommand should be fatal if they don't succeed, eg failing to mount the dmg, appending to tar or exe. Can you handle that here rather than return an unused exit code ? r- for this.
   
>#########################################################################
>def getFormattedPlatform(platform):

What's this function for ? Do some of the repack.cfg's use different strings to linux-i686, mac, and win32 and then need massaging ?

>#########################################################################
>def repackMac(build, partner_dir, build_dir, repack_dir):
...
>    shellCommand(eject_cmd)
>    os.remove("stage/ ")

This is removing the symlink to Applications ? Please add a comment to clarify because at first glance it looks deleting the dir just resync'd from the mounted dmg.

>#########################################################################
>def repackWin32(build, partner_dir, build_dir, repack_dir):
>    print "Repacking win32 build %s" % build
>
>    base_dir = os.getcwd();
>    full_build_path = "%s/%s/%s" % (base_dir, build_dir, build)
>    full_partner_path = "%s/%s" % (base_dir, partner_dir)
>    working_dir = "%s/working" % repack_dir
>    mkdir(working_dir)
>    os.chdir(working_dir)
>
>    copy(full_build_path, '.')

The three platform functions share a lot of code, so it would be nicer to subclass the unpack & repack operations. Fine as a followup if you prefer.

>    # Pre-flight checks
>    if len(args) < 0 or not options.version:
>        print "Error: you must specify a version number."
>        error = True

What does the first condition catch ? args seems to be unused, and ends up with value 0. 

>    if not options.verify_only:
>        if not os.path.exists(original_builds_dir):
>            mkdir(original_builds_dir)
>        if not os.path.exists(repacked_builds_dir):
>            mkdir(repacked_builds_dir)

Nit: mkdir is also testing for directory existence, so you could skip doing that here.

>    for partner_dir in os.listdir(options.partners_dir):

Comment: A future optimisation would be to use a manifest of active partners, so that we don't waste cycles building no longer active ones (eg mozilla07).

>        if not options.verify_only:
>            print "Starting repack process for partner: %s" % partner_dir

Looking at the sample logs you gave, there seems to be lots of repacking output in the log before the first of these messages. Perhaps a problem with stdout/stderr handling ? r- for this too, because it makes it hard to follow if there were any problems.

>                    else:
>                        # Download original build from stage
>                        print os.getcwd()

Nit: print just for debugging ?

>        # Remove our working dir so things are all cleaned up and ready for
>        # easy upload.
>        workingDir = "%s/working" % partner_repack_dir
>        rmdirRecursive(workingDir)

Shouldn't this be indented one more level, so that it's done per-platform too ?
Attachment #370053 - Flags: review?(nthomas) → review-
Identical patch to staging, but with the nit about the builder name fixed.

Need to get the main script right (attachment 370053 [details]) before I can land these anyway.
Attachment #370225 - Flags: review?(nthomas)
(In reply to comment #8)
> Looks like many of the calls to shellCommand should be fatal if they don't
> succeed, eg failing to mount the dmg, appending to tar or exe. Can you handle
> that here rather than return an unused exit code ? r- for this.

Yeah, I'll add some handling/cleanup code here.
 
> What's this function for ? Do some of the repack.cfg's use different strings to
> linux-i686, mac, and win32 and then need massaging ?

I wrote this function before I actually got the partner config data from Kev, so I was just being safe.
 
> This is removing the symlink to Applications ? Please add a comment to clarify
> because at first glance it looks deleting the dir just resync'd from the
> mounted dmg.

Good call.

> The three platform functions share a lot of code, so it would be nicer to
> subclass the unpack & repack operations. Fine as a followup if you prefer.

I'll see what I call roll up.

> What does the first condition catch ? args seems to be unused, and ends up with
> value 0. 

Hrmm, more cruft from before I had real config data to work with.
  
> Nit: mkdir is also testing for directory existence, so you could skip doing
> that here.

But mkdir also throws an error if the directory exists.
 
> Comment: A future optimisation would be to use a manifest of active partners,
> so that we don't waste cycles building no longer active ones (eg mozilla07).

Not sure about the best solution here. 

Ideally, the partner should not be in the hg repo if we're not repacking for them any longer (easiest from a scripting perspective). The partners/ subdir of the repo is meant to be repopulated from the config db via a admin push (according to Kev), so that dir should either disappear or the repack.cfg for that partner should contain an active=False flag.
 
> Looking at the sample logs you gave, there seems to be lots of repacking output
> in the log before the first of these messages. Perhaps a problem with
> stdout/stderr handling ? r- for this too, because it makes it hard to follow if
> there were any problems.

Agreed, and surprising because those logs have different ordering than local script-only tests. Let me play with it to make sure output is flushed properly as we go.
 
> Nit: print just for debugging ?

D'oh.
 
> Shouldn't this be indented one more level, so that it's done per-platform too ?

No, I don't think so. I try to only move the partner bundles once and can then copy the contents as many times as required for multiple locales and OSes.
(In reply to comment #7)
> Is there a plan for how the renaming of builds into the release-style is going
> to be done ? That's the only major thing that seems to be missing.

Kev or Tomcat: can you comment on your naming requirements? 

Current dir structure is as follows:

firefox/
  tinderbox-builds/
    latest-mozilla1.9.0-partner-repacks/
      $version/
        build#/
          partner_name/

The repacks currently keep the same filename as the original source build. I haven't tweaked the filenames at all because AIUI the repacks need to go through a manual QA process prior to any renaming or moving to a final drop point for the partners. 

We *can* name them any way we want, using settings in repack.cfg if need be.
(In reply to comment #11)
> firefox/
>   tinderbox-builds/

I think you mean nightly/ here.

>     latest-mozilla1.9.0-partner-repacks/
>       $version/
>         build#/
>           partner_name/
biggest question I have is whether or not the signing scripts require
particular filenames, or just a directory structure. if it's just a directory
structure, then I would love to have a unique identifier pre-pended to the
installer files as well.

could you let me know what the requirements for the signing piece are? with
that, I'll update a spec.
Attachment #370225 - Flags: review?(nthomas) → review+
(In reply to comment #12)
> I think you mean nightly/ here.

Don't care, really. I just want a place that is out of the mainstream where pre-QA repacks can live that won't end up on the mirror network. The nightly/ dir gets more traffic than tinderbox-builds/ is the only reason I stuck them in tinderbox-builds/.

Ideally (IMO) this wouldn't even be on stage, but there no sense replicating existing infrastructure.
(In reply to comment #13)
> biggest question I have is whether or not the signing scripts require
> particular filenames, or just a directory structure. if it's just a directory
> structure, then I would love to have a unique identifier pre-pended to the
> installer files as well.
> 
> could you let me know what the requirements for the signing piece are? with
> that, I'll update a spec.

cc-ing nthomas for input since he usually does the signing for these.
Windows installer signing currently expects to find things in the release naming style, in particular to be started in the win32/ directory and to find a bunch of locales dirs below that. It acts on *.exe within each locale dir. If we start signing with cwd of .../version/build#/ (of comment #11) it should work fine, as long as we don't mind signing everything at once.

PGP signing looks for files below the current dir, excluding non-release stuff, so it'll work regardless. I'd be surprised if partners are using the .asc files though.

Are we going to release the files to partners in a <uniqueID>-firefox-<version>-<locale>-<platform>-<type> style ? Not so end-user friendly, even if it makes life simpler for us. If not, we'll need to groom the files - I'd have to check if a leading <uniqueID> will requires changes - and we might as well do that before signing.
(In reply to comment #10)
> (In reply to comment #8)
> > Nit: mkdir is also testing for directory existence, so you could skip doing
> > that here.
> But mkdir also throws an error if the directory exists.

You sure ? Looks like to calls to os.path.exists()
 
>> > Shouldn't this be indented one more level, so that it's done per-platform too ?
> No, I don't think so. I try to only move the partner bundles once and can then
> copy the contents as many times as required for multiple locales and OSes.

Perhaps I missed something about the execution order. I'm mainly concerned about including components from linux builds in windows and vice versa. Perhaps that's safe because they unpack to different dirs, but cleaning each time seemed like a way to futureproof.
Comment on attachment 370053 [details]
Script for performing partner repacks

nthomas: I have a new version of the script for review in my user repo:

http://hg.mozilla.org/users/coop_mozilla.com/partner-repacks/file/e80af1808017/scripts/partner-repacks.py

Shall I post the file as a patch, or are you fine to review it in situ?

Here's the log of a new run featuring the updating script and Bootstrap code:

http://staging-1.9-master.build.mozilla.org:8810/builders/partner%20repack/builds/7/steps/shell_7/logs/stdio
Attachment #370053 - Attachment is obsolete: true
Rev e80af1808017 is looking good coop, especially like the logging. Only thing I'd call out is RepackMac.copyFiles() looks like it can be written
    def copyFiles(self):
      super(RepackMac, self).copyFiles('stage/Firefox.app/Contents/MacOS')
(In reply to comment #20)
> Rev e80af1808017 is looking good coop, especially like the logging. Only thing
> I'd call out is RepackMac.copyFiles() looks like it can be written
>     def copyFiles(self):
>       super(RepackMac, self).copyFiles('stage/Firefox.app/Contents/MacOS')

copytree does handle 'cp -r' situations, although I could do away with copytree altogether and just use 'cp -r' and then roll up those functions.
(In reply to comment #21) 
> copytree does handle 'cp -r' situations, although I could do away with copytree
> altogether and just use 'cp -r' and then roll up those functions.

Sadly this didn't work. Mac requires a subtly different directory structure from win32/lin.

Given comment 20, I'm going to close this out and file a new bug to get an hg repo created for partner-repacks under build/.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Forgot to mention that we'll deal with any build/dir naming changes in a separate bug.
Blocks: 486573
Comment on attachment 370055 [details] [diff] [review]
Add partner repack logic to Bootstrap

Bah, forgot to land these before vacation.
Attachment #370055 - Flags: checked‑in+ checked‑in+
Attachment #370057 - Flags: checked‑in+ checked‑in+
Attachment #370225 - Flags: checked‑in+ checked‑in+
When I tried to run the partner_repack builder for 3.0.10 it blew up on me because PartnerRepack.pm didn't exist. Turns out that that file just wasn't tagged. I tagged the latest revision with RELEASE_AUTOMATION_M13 and still got the same error. It looks like mozilla/tools/release/release needs to be aware of the PartnerRepack step, I've got a patch incoming...
Attachment #374480 - Flags: review?(catlee) → review+
Comment on attachment 374480 [details] [diff] [review]
attempt #1 to fix partner repacks in bootstrap

Checking in release;
/cvsroot/mozilla/tools/release/release,v  <--  release
new revision: 1.15; previous revision: 1.14
done
cvsbitters-2:release bhearsum$ cvs tag -F RELEASE_AUTOMATION_M13 release
T release
Attachment #374480 - Flags: checked‑in+ checked‑in+
I'm going to re-open this bug. There seems to be another problem or two the Bootstrap portion of this - the latest error message I got complains about pkg-dmg and 7za:
log:   arg4: 1
log: Starting time is 09:48:41 04/24/09
log: Logging output to /builds/logs/repack_3.0.10-build-partner-repack.log
log: Timeout: 3600
Error: couldn't find the 7za executable in PATH.
Error: couldn't find the pkg-dmg executable in PATH.
log: output: Error: couldn't find the 7za executable in PATH.
Error: couldn't find the pkg-dmg executable in PATH.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Ideally we would have created RELEASE_AUTOMATION_M14 from the tip before 3.0.10 started, but it probably got missed in the rush.
(In reply to comment #29)
> Error: couldn't find the 7za executable in PATH.
> Error: couldn't find the pkg-dmg executable in PATH.

So 7zip missing is pretty easy to fix, but Bootstrap is supposed to add pkg-dmg to the path after it gets checked out. I'll need to do some digging.
Status: REOPENED → ASSIGNED
(In reply to comment #30)
> Ideally we would have created RELEASE_AUTOMATION_M14 from the tip before 3.0.10
> started, but it probably got missed in the rush.

Yeah, that's all that's missing here. The files I updated (Mozbuild/Util.pm, Bootstrap/Step.pm) aren't tagged for release automation. Those files add the path manipulation stuff that allow us to checkout pkg-dmg and then add it to the path.
To getthe repacks for 3.0.10 out the door, I've temporarily moved pkg-dmg into the existing PATH and triggered the step again.
bhearsum has already tagged the new files/versions for RELEASE_AUTOMATION_M13.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago15 years ago
Resolution: --- → FIXED
This makes me very very happy :-)
Created RELEASE_AUTOMATION_M14 to pick up the MozBuild/Util.pm and Bootstrap/Step.pm part of this bug (and also bug 481079).
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: