Closed Bug 366850 Opened 13 years ago Closed 13 years ago

fix bootstrap bugs found in 1.5.0.9 release cycle

Categories

(Release Engineering :: General, defect, blocker)

defect
Not set
blocker

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: rhelmer, Assigned: rhelmer)

References

Details

Attachments

(2 files, 2 obsolete files)

Attached patch fixes for stage step (obsolete) — Splinter Review
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.
Attachment #251302 - Flags: review?(preed)
Depends on: end2end-bld
Blocks: end2end-bld
No longer depends on: end2end-bld
Attached patch fixes for l10n step (obsolete) — Splinter Review
* use l10n-specific pull date
* proper parenthesis around force/branch logic
Assignee: build → rhelmer
Status: NEW → ASSIGNED
Attachment #251303 - Flags: review?(preed)
Comment on attachment 251302 [details] [diff] [review]
fixes for stage step

preed already checked some of this in, need to merge first.
Attachment #251302 - Attachment is obsolete: true
Attachment #251302 - Flags: review?(preed)
Looks like everything for Stage.pm already went in with bug 364088, nevermind that one.
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.
Attachment #251306 - Flags: review?(preed) → review+
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.
Attachment #251303 - Flags: review?(preed) → review-
Sorry for the regression! I am working to everything landed CVS so there isn't so much back/forth merging.
Attachment #251303 - Attachment is obsolete: true
Attachment #251989 - Flags: review?(preed)
Comment on attachment 251989 [details] [diff] [review]
fixes for l10n tagging 

r=preed.
Attachment #251989 - Flags: review?(preed) → review+
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
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.