366850 - fix bootstrap bugs found in 1.5.0.9 release cycle

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Robert Helmer [:rhelmer]]

Attachment: fixes for stage step

preed found a couple bugs in bootstrap that had to be worked around for the latest 1.5 release, this bug is for fixing those.

Comment 1

[Robert Helmer [:rhelmer]]

Attachment: fixes for l10n step

* use l10n-specific pull date
* proper parenthesis around force/branch logic

Comment 3

[Robert Helmer [:rhelmer]]

Comment on attachment 251302 [details] [diff] [review]
fixes for stage step

preed already checked some of this in, need to merge first.

Comment 4

[Robert Helmer [:rhelmer]]

Looks like everything for Stage.pm already went in with bug 364088, nevermind that one.

Comment 5

[Robert Helmer [:rhelmer]]

Attachment: better logging, handle key copy, handle contrib-localized

Comment 6

[J. Paul Reed [:preed]]

Comment on attachment 251306 [details] [diff] [review]
better logging, handle key copy, handle contrib-localized


>+    for my $dir ('contrib', 'contrib-localized') {
>+        if (not -d "$skelDir/$dir") {
>+            MkdirWithPath('dir' => "$skelDir/$dir") 
>+              or die "Could not mkdir $skelDir/$dir : $!";
>+            $this->Log('msg' => "Created directory $skelDir/$dir");
>+        }
>+
>+        chmod(oct(2775), "$skelDir/$dir")
>+          or die "Cannot change mode on $skelDir/$dir to 2775: $!";
>+        $this->Log('msg' => "Changed mode of $dir to 2775");
>+        chown(-1, $gid, "$skelDir/$dir")
>+          or die "Cannot chgrp $skelDir/$dir to $product: $!";
>+        $this->Log('msg' => "Changed group of $skelDir/$dir to $product");

This is a minor thing, but in cases, like this, I tend to do something like:
    foreach my $dirPart ('contrib', 'control-localized') {
       my $fullDir = "$skelDir/$dirPart";
       ... then use $fullDir throughout instead of constructing it from pieces for every call ...
    }

The benefit is that if you (or someone else) in the future needs to change the definition of $fullDir, they won't accidentally forget to change it throughout the entire block; they just have to change it in one place.

I would consider fixing that; but other than that, looks good.

Comment 7

[J. Paul Reed [:preed]]

Comment on attachment 251303 [details] [diff] [review]
fixes for l10n step

>     # only force or branch specific files, not the whole tree
>-    if ($force and scalar(@{$files}) <= 0 ) {
>+    if ($force and scalar(@{$files} <= 0 )) {
>         die("ASSERT: Cannot specify force without files");
>-    } elsif ($branch and scalar(@{$files}) <= 0) {
>+    } elsif ($branch and scalar(@{$files} <= 0)) {

Regression.

Comment 8

[Robert Helmer [:rhelmer]]

Attachment: fixes for l10n tagging

Sorry for the regression! I am working to everything landed CVS so there isn't so much back/forth merging.

Comment 9

[J. Paul Reed [:preed]]

Comment on attachment 251989 [details] [diff] [review]
fixes for l10n tagging 

r=preed.

Comment 10

[Robert Helmer [:rhelmer]]

Landed, thanks:

Checking in Bootstrap/Step/Tag.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/Tag.pm,v  <--  Tag.pm
new revision: 1.4; previous revision: 1.3
done