371350 - Bootstrap regressions for 1.5.0.10/2.0.0.2

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[J. Paul Reed [:preed]]

Found some release automation regressions while staging 1.5.0.10/2.0.0.2:

Step Stage died: msgCannot rename /data/cltbld/firefox-2.0.0.2/batch1/stage/windows /data/cltbld/firefox-2.0.0.2/batch1/stage/win32: No such file or directory at Bootstrap/Step/Stage.pm line 130.

Comment 1

[J. Paul Reed [:preed]]

Also this:

Invalid (extra) locale present: en-US
Missing non-locale XPI: linux-i686/xpi/browser
Missing non-locale XPI: linux-i686/xpi/talkback
Missing non-locale XPI: linux-i686/xpi/xpcom
Missing non-locale XPI: linux-i686/xpi/adt
Invalid (extra) locale present: en-US
Missing non-locale XPI: win32/xpi/browser
Missing non-locale XPI: win32/xpi/talkback
Missing non-locale XPI: win32/xpi/xpcom
Missing non-locale XPI: win32/xpi/adt
Invalid (extra) locale present: en-US
Invalid (extra) locale for platform 'osx': gu-IN
Invalid (extra) locale XPI for platform 'osx': gu-IN
Missing non-locale XPI: mac/xpi/browser
Missing non-locale XPI: mac/xpi/talkback
Missing non-locale XPI: mac/xpi/xpcom
Missing non-locale XPI: mac/xpi/adt

The automation runs verify-locales.pl, but it doesn't print (or even check) the result.

We need to handle shipped-locales correctly at the prestage-trimmed step.

Comment 2

[J. Paul Reed [:preed]]

Also, for 2.0.0.x ops, we need to remove all directories that we don't recognize from the -candidates directory; we're getting things like unsigned/ and signed-test, etc.

These are important to keep in prestage, but need to be removed in prestage-trimmed.

Comment 3

[J. Paul Reed [:preed]]

Also, prestage-trimmed gets trimmed of all mars.

Comment 4

[J. Paul Reed [:preed]]

Attachment: Patch to take care of a few of teh stage step regressions

This fixes:

-- Rsync was copying the rcN directory, not its contents; this was (I think) because catfile wasn't keeping the trailing '/' (since it's technically not necessary if there's anything after it; of course, it matters for rsync syntax).

-- Fix inconsistent use of catfile(), which created a path like /foo/barbaz/foo instead of /foo/bar/baz/foo

-- Create code for handling the XPI directories; this was just removed in one revision, and never replaced (?)

Comment 5

[J. Paul Reed [:preed]]

Attachment: Part II

This takes of the other regressions listed, specifically:

In Step.pm:

-- RunShellCommand() handles chdir()ing into a directory to run a command, so use it instead (Shell() had a bug where it didn't restore the directory after it chdir()'d into it).

-- if ($timeout) happened to be 0 or undefined, then the command wouldn't get run at all (??) There will always be a default timeout, but if the timeout given is 0, it really should wait forever (RunShellCommand() looks to get this right).

-- Fix the signal name/number madness (fallout from the patch for bug 372583).

In Stage.pm:

-- Clean up/add a bunch of comments
-- Cleanup prestage-trimmed using shipped-locales as a basis for the cleanup
-- Run groom-files for mars afterwards
-- Create a stage-unsigned and stage-signed directory
-- Remove unknown directories (that we don't know how to handle) from prestage-trimmed
-- Make sure we chown() all deliverables to the right group

This was tested with firefox 2.0.0.2 deliverables; I need to test with Firefox 1.5.0.10 and Thunderbird 1.5.0.10, but I'll do that shortly.

Comment 6

[Robert Helmer [:rhelmer]]

Comment on attachment 257906 [details] [diff] [review]
Part II

Wow this is great, very thorough. 

It makes me sad that this step is so friggin complicated :( Probably a candidate for refactoring into substeps later, if it can't be simplified.

One nit - "bouncer platforms" are mentioned in a few places, but the bouncer platform for windows is "win" not "win32".

Comment 7

[J. Paul Reed [:preed]]

Attachment: Part II, Take 2

Interdiff will be your friend here.

After testing some more on Firefox 2.0.0.2, this fixes the following issues:

Step.pm

-- Re-ordered the use statement to use standard ordering (stdlib modules first, site-modules second)
-- add a "dumpLogs" config variable that will cause shell statements to be echo'd this is in prep for buildbot deployment of automation.

Config.pm

-- Make sure Get() and Exists() assert if var is not passed
-- Get() had a bug where it would die with no such config variables if the actual variable evaluated to false; that's not what was wanted. What was wanted was whether the variable exists, not whether it evaluates to false (dumpLogs, when set to 0, tickled this bug).

Stage:

-- Massage some of the error messages; some were missing spaces, some were just incorrect.
-- Point the Verify() function at the copy of shipped-locales that the step checked out; this makes it so the Stage step doesn't rely on the Source step anymore.
-- I realized that all of the ja/ja-JP-mac stuff would be handled by the shipped-locales logic, so it made sense to just delete the handling of that entirely and let the shipped-locales logic pick it up.
-- Had the wrong regex for the source tarball

I'm betting this still isn't 100% correct; 1.5.0.10 had a bunch of extra xpis for parts of the product (Thunderbird especially), and these are going to get deleted; I'll patch this in Part III, though.

Comment 8

[J. Paul Reed [:preed]]

Part II Landed:
 
Checking in Bootstrap/Config.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Config.pm,v  <--  Config.pm
new revision: 1.4; previous revision: 1.3
done
Checking in Bootstrap/Step.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step.pm,v  <--  Step.pm
new revision: 1.10; previous revision: 1.9
done
Checking in Bootstrap/Step/Stage.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/Stage.pm,v  <--  Stage.pm
new revision: 1.9; previous revision: 1.8
done

Comment 9

[J. Paul Reed [:preed]]

Attachment: Part III

Handle 1.5.0.x-branch non-locale .xpis correctly.