Closed Bug 371325 Opened 13 years ago Closed 13 years ago

Automate dependent tool config file generation

Categories

(Release Engineering :: General, defect, critical)

defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: morgamic, Unassigned)

References

Details

Attachments

(6 files, 14 obsolete files)

6.46 KB, patch
preed
: review+
Details | Diff | Splinter Review
1.69 KB, text/plain
preed
: review+
nthomas
: review+
Details
3.10 KB, patch
Details | Diff | Splinter Review
3.56 KB, patch
preed
: review+
Details | Diff | Splinter Review
3.24 KB, patch
Details | Diff | Splinter Review
12.57 KB, patch
nthomas
: review+
preed
: review+
Details | Diff | Splinter Review
Items in our central config found at /tools/release/bootstrap.cfg should be automatically propagated to dependent tool configs.

As a part of the process, care should be taken to check these new configs into some sort of source control to allow us to manage them systematically and see changesets.

Tools that would need to be configured as a part of release automation based on boostrap.cfg are:
patcher
tinderbox
bouncer
addons.mozilla.org
patcher

Others... ?
OS: BeOS → All
Hardware: PocketPC → All
You can see the list of configs we want to automate the generation for by looking through the release config bugs (bug 369669 for 1.5.0.10 and bug 369670 for 2.0.0.3; the list is mostly the same, though.)

General requirements: the configs should be checked out/in and tagged as a human would normally do it, so that pulling the FIREFOX_2_0_0_2_RELEASE tag in the appropriate mozilla/tools/tinderbox-configs directory gets you the right thing.

The configs that we're interested in are:

tinder-config.pl en-US bumps (https://bugzilla.mozilla.org/attachment.cgi?id=254373)

tinder-config.pl l10n bumps (which will need to be rebumped for respin support)

patcher2.cfg bumps: https://bugzilla.mozilla.org/attachment.cgi?id=255602

update verification bumps (which rhelmer mentions above):
https://bugzilla.mozilla.org/attachment.cgi?id=255620

Because of the checkin/checkout requirements, I suggested doing something along the lines of adding placeholders to the tinder-configs for things that should change (it's like the same four lines over and over again), and then the line to change below, so, for a (en-US release) tinder-config, something like:

 [snip]
 #- change to the tree you're actually building
 $BuildTree  = 'Mozilla1.8.0';
 
 #$BuildName = '';
 
 # CONFIG: $BuildTag = '%productTag%_RELEASE';
 $BuildTag = 'FIREFOX_1_5_0_9_RELEASE';
 [etc.]

This allows us to generate configs, but also allows us to record what the automation did (because it will check the config back in.

patcher2.cfg might be more annoying, because you'll have to do things like check out/reference the shipped-locales file (http://lxr.mozilla.org/mozilla1.8/source/browser/locales/shipped-locales), but it should be doable.

Let me know if that doesn't make sense, or I've had my head in the trenches too long and seems weird.
Reassigning to rhelmer.  Morgamic will focus on Bouncer stuff and revisit this if nobody's gotten to it since this requires intimate knowledge of build internals.
Assignee: morgamic → rhelmer
Status: NEW → ASSIGNED
Depends on: 375714
Depends on: 372746
Depends on: 375780
Depends on: 375782
Duplicate of this bug: 375780
Thinking about this, seems like we would want:

1) a tool in MozBuild to do the variable substitution as per preed's comment #2 
2) a new step, maybe Step::Config, after Step::Build, that uses this tool

I believe that we could generate all config files immediately following the build step, because the build ID is the only piece of data we are missing (we already have the shipped-locales file and all needed version info from the bootstrap.cfg, am I missing anything?).

The only things I am unsure about are: on which machine a Config step would run, and how does it get the build ID?. The simplest way I can think of is to run this step on the build machines, and read the build ID right out of the build log.

This isn't ideal IMHO, since I don't think that the build machines should require read/write access to the repository. Also, while this is fine for the tinderbox configs which are separated out into OS-specific files, we would have to take care that the patcher config is edited properly.

A few alternatives that come to mind:

* Buildbot, which is driving Bootstrap, could pick up the build ID from the output of the Build step, and pass it to the Config step
* Build machines could check in or upload a file containing the build ID to some well-known location, which the Config step will be able to find

Any ideas?
Caller would look something like:

MozBuild::Util::ConfigBump(
  configFile => 'blah',
  vars => {'productTag' => 'woosa'},
);

Trying to keep this as simple and clear as possible; the config file must be set up like:

# CONFIG: $BuildTag = '%productTag%_RELEASE';
$BuildTag = 'WHATEVER_RELEASE';

That is, there must be a line starting with "# CONFIG:" exactly, and the line immediately following it will be overwritten.

Would it be useful to make sure the line we're going to replace looks like the line described in the comment except for the variable being substituted? I'm not really sure that it is, let me know what you think though.
Attachment #260859 - Flags: review?(preed)
(In reply to comment #5)
> I believe that we could generate all config files immediately following the
> build step, because the build ID is the only piece of data we are missing (we
> already have the shipped-locales file and all needed version info from the
> bootstrap.cfg, am I missing anything?).

Actually that's not true, we need at least two configure steps. Logically it looks like this, I think:

* before Build (tinder-config.pl)
* before Repack (mozconfig/tinder-config.pl)
* before Update (patcher.cfg/verify.cfg/AUS)
* after Stage (bouncer)

The pre-Build bump depends on info from bootstrap.cfg, all of the others depend upon output from one or more of the previous steps (build ID for update, and the equivalent of the checksum files for bouncer).

Currently I am targetting the tinder-config.pl and mozconfig changes. For now I will assume that Build and Repack is happening on the same machine, and that the logs are available. It probably makes sense to either add a new preConfig() method or a substep (preed suggested these before). 

I am thinking a substep is better, since we're only talking about 2 classes (and only one we need a postConfig()). If anyone has thoughts on this please comment.
Comment on attachment 260859 [details] [diff] [review]
first stab at ConfigBump function

>Index: MozBuild/Util.pm
>===================================================================
>RCS file: /cvsroot/mozilla/tools/release/MozBuild/Util.pm,v
>retrieving revision 1.16
>diff -u -r1.16 Util.pm
>--- MozBuild/Util.pm	21 Mar 2007 22:35:47 -0000	1.16
>+++ MozBuild/Util.pm	6 Apr 2007 22:04:52 -0000
>@@ -11,7 +11,8 @@
> 
> use base qw(Exporter);
> 
>-our @EXPORT_OK = qw(RunShellCommand MkdirWithPath HashFile DownloadFile Email);
>+our @EXPORT_OK = qw(RunShellCommand MkdirWithPath HashFile DownloadFile Email
>+                    ConfigBump);

I could go either way on this being part of MozBuild::Util.

MozBuild::Util is supposed to contain functions that would be useful to all of the release infrastructure (Bootstrap, patcher2, tinderbox, etc.) and ConfigBump generally isn't useful to anything but Bootstrap.

I would maybe put this in the step it will be used with.

>+sub ConfigBump {
>+    my %args = @_;
>+
>+    my $configFile = $args{'configFile'};
>+    if (! defined($configFile)) {
>+        die('MozBuild::ConfigBump - configFile is a required argument');
>+    }

I've been marking assertions with 'ASSERT:', mostly so I can grep for ASSERTions in our code.

>+    my $vars = $args{'vars'};
>+    if (! $vars) {
>+        die('MozBuild::ConfigBump - vars is a required argument');

Same.

Also, below, you're assuming that $vars is a hash ref; you should either assert that OR if you move this code back into bootstrap, then just use $config directly.

>+    my $nextLine = undef;
>+    foreach my $line (@lines) {
>+        if (defined($nextLine)) {
>+            $line = $nextLine;
>+            $nextLine = undef;
>+            next;
>+        }
>+        foreach my $key (keys(%{$vars})) {
>+            if ($line =~ '^# CONFIG:') {
>+                if ($line =~ '%' . $key . '%') {
>+                    my $value = $vars->{$key};
>+                    $nextLine = $line;
>+                    $nextLine =~ s/# CONFIG: //;
>+                    $nextLine =~ s/\%$key\%/$value/;
>+                    last;
>+                }
>+            }
>+        }
>+    }

A couple of comments:

I would not read the entire file into memory, only to write it out again; it's probably negligible, but I would open the input file and open an ouput file, and modify the output as you go.

Also, you loop over all the keys in $vars for no reason 95% of the time. That is, the CONFIG lines are not the common case, so that would be more effectively written:

for all the input lines {
   if (line matches CONFIG) {
       interporlatedLine = line;
       if (there's a key that maches) {
          replace it
          print line;
          print interpolatedLine;
       }
   }
}

The looping over all the config keys can be avoided entirely by grep()ing the CONFIG line for %string%, and then taking the matches, looping over them, and replacing those in the (copied) string.
Attachment #260859 - Flags: review?(preed) → review-
Great suggestions, thanks!

This version optimizes the loop in the ways preed suggested, and seems to work properly for multiple lines that include one or more variables.
Attachment #260859 - Attachment is obsolete: true
Attachment #261069 - Flags: review?(preed)
Comment on attachment 261069 [details] [diff] [review]
move to Bootstrap::Config::Bump(), optimize

>+    my $vars = $args{'vars'};
>+    if (! defined($vars)) {
>+        die('ASSERT: Bootstrap::Config::Bump - vars is a required argument');
>+    }
>+    die('ASSERT: Bootstrap::Config::Bump: vars is not a hash ref.')
>+     if (defined($vars) && ref($vars) ne 'HASH');

You can collapse these checks into one assertion: if (!defined($vars) || ref($vars) ne 'HASH').

>+    open(INFILE, "< $configFile") 
>+     or die ("ASSERT: Could not open $configFile for reading: $!");
>+    open(OUTFILE, "> $tmpFile") 
>+     or die ("ASSERT: Bootstrap::Config::Bump - Could not open $tmpFile for writing: $!");

For consistency, open() failing is not really an ASSERTion condition throughout the rest of the application. The way I typically make the distinction is something you'd use assert()/NS_ASSERT() for in something like C, you use a die('ASSERT: ') for in perl.

Same thing with the close()s below.

>+    my $nextLine = undef;
>+    foreach my $line (<INFILE>) {
>+        if (defined($nextLine)) {
>+            $line = $nextLine;
>+            print OUTFILE $line;
>+            $nextLine = undef;
>+            next;
>+        } else {
>+            print OUTFILE $line;
>+        }
>+        if ($line =~ '^# CONFIG:') {
>+            foreach my $key (grep($line, keys(%{$vars}))) {
>+                if ($line =~ '%' . $key . '%') {
>+                    my $value = $vars->{$key};
>+                    $line = $line;
>+                    $line =~ s/# CONFIG: //;
>+                    $line =~ s/\%$key\%/$value/;
>+                    $nextLine = $line;
>+                }
>+            }
>+        }
>+    }

You can implement this without the nextLine logic, which is basically a (overly complicating) state machine.

foreach my $line (<INFILE>) {
    print OUTFILE $line;

    if ($line matches config) {
       ... your loop logic ...
       print OUTFILE $modifiedLine;
    }
}

Also, the foreach() logic for the key can be even more simplified/readable; this is totally off the top of my head, so I'm sure it's not 100% right:

foreach my $interpolate (grep($line, /%(\w+)%/)) {
    my $value = $vars->{$key};
    my $line =~ s/\%$key\%/$value/g;
}

You can also pull the s/# CONFIG: / (which should probably be /^#\s+CONFIG:\s+/) out of the loop; you only want that done once.
Attachment #261069 - Flags: review?(preed) → review-
(In reply to comment #10)
> >+    my $nextLine = undef;
> >+    foreach my $line (<INFILE>) {
> >+        if (defined($nextLine)) {
> >+            $line = $nextLine;
> >+            print OUTFILE $line;
> >+            $nextLine = undef;
> >+            next;
> >+        } else {
> >+            print OUTFILE $line;
> >+        }
> >+        if ($line =~ '^# CONFIG:') {
> >+            foreach my $key (grep($line, keys(%{$vars}))) {
> >+                if ($line =~ '%' . $key . '%') {
> >+                    my $value = $vars->{$key};
> >+                    $line = $line;
> >+                    $line =~ s/# CONFIG: //;
> >+                    $line =~ s/\%$key\%/$value/;
> >+                    $nextLine = $line;
> >+                }
> >+            }
> >+        }
> >+    }
> 
> You can implement this without the nextLine logic, which is basically a (overly
> complicating) state machine.
> 
> foreach my $line (<INFILE>) {
>     print OUTFILE $line;
> 
>     if ($line matches config) {
>        ... your loop logic ...
>        print OUTFILE $modifiedLine;
>     }
> }
> 
> Also, the foreach() logic for the key can be even more simplified/readable;
> this is totally off the top of my head, so I'm sure it's not 100% right:
> 
> foreach my $interpolate (grep($line, /%(\w+)%/)) {
>     my $value = $vars->{$key};
>     my $line =~ s/\%$key\%/$value/g;
> }


Just to be clear on the intent, are you suggesting iterating over all matches found on each line, and trying to substitute them that way? It's a good idea, I don't think grep can do that though? Not sure how to do that without another loop in perl, looking into it though.


> You can also pull the s/# CONFIG: / (which should probably be
> /^#\s+CONFIG:\s+/) out of the loop; you only want that done once.


The one in the "if" is a match, I suppose we could make it a substitute. I generally try to modify things in place on their own line, for readability. 
(In reply to comment #11)

> Just to be clear on the intent, are you suggesting iterating over all matches
> found on each line, and trying to substitute them that way? It's a good idea, I
> don't think grep can do that though?

Here's the code; it's not really tested beyond a perl -c:

    open(INFILE, "< $configFile") 
     or die ("Could not open $configFile for reading: $!");
    open(OUTFILE, "> $tmpFile") 
     or die ("Bootstrap::Config::Bump - Could not open $tmpFile for writing: $!");

    my $overwriteNextLine = 0;

    foreach my $line (<INFILE>) {
        if ($overwriteNextLine) {
            $overwriteNextLine = 0;
            next;
        }

        print OUTFILE $line;

        if ($line =~ /^#\s+CONFIG:/) {
            $overwriteNextLine = 1;
            my $interpLine = $line;
            $interpLine =~ s/^#\s+CONFIG:\s+//;

            foreach my $variable (grep(/%\w+%/, split(/\s+/, $line))) {
                my $key = $variable;
                $key =~ s/%//g;

                if (! exists($vars->{$key})) {
                    die("ASSERT: ...");
                }

                my $value = $vars->{$key};
                $interpLine =~ s/$variable/$value/g;
            }

            print OUTFILE $interpLine;
        }
    }
(In reply to comment #12)
> (In reply to comment #11)
> 
> > Just to be clear on the intent, are you suggesting iterating over all matches
> > found on each line, and trying to substitute them that way? It's a good idea, I
> > don't think grep can do that though?
> 
>             foreach my $variable (grep(/%\w+%/, split(/\s+/, $line))) {

Ah, yeah that'd do it. I'll incorporate this into the patch. Thanks!
Attached patch optimization suggestions (obsolete) — Splinter Review
Implements suggested optimizations (e.g. loop over found vars instead of all available keys).

Not reading the whole file into memory as we discussed outside of the bug; there's a skipNextLine boolean used instead.

I noticed in your example you used overwriteNextLine but I think skipNextLine makes more sense for what it's being used for (also makes reading the truth check make sense).

There's a simple test case in there too; I'll check in a simplified tinder-config.pl for it's use separately.
Attachment #261069 - Attachment is obsolete: true
Attachment #261736 - Flags: review?(preed)
Comment on attachment 261736 [details] [diff] [review]
optimization suggestions

This looks fine; a few suggestions:

-- What's the call to this function look like?

Do you have to build up a hash of values that you want interpolated? It would be nice if you could just pass in a Bootstrap::Config object, and it would do the right thing. Doing it this way  requires you to break abstractions. Or was there a reason for doing it this way? I know we talked about this briefly, but I don't remember all the details of that convo.

-- The code inside of the for loop has basically three instances: handling a config line, handling a non-config line, and handling the line after a config line; these are expressed in the way the if-loop is structured, so that's good; could you add a comment about which branch is for each case?

+            foreach my $variable (grep(/%\w+%/, split(/\s+/, $line))) {

That might not be quite right; we've got some variables with -'s and _'s in them, so you might want to match /%[\w_-]+%/. I suggested the regex, so that's my bad.
Attachment #261736 - Flags: review?(preed) → review+
(In reply to comment #15)
> (From update of attachment 261736 [details] [diff] [review])
> This looks fine; a few suggestions:
> 
> -- What's the call to this function look like?

Here's what the BuildConfig step I'm working on uses:

    $config->Bump(
      configFile => catfile($configBumpDir, $configFile),
      vars => { 
        BUILD_TREE => $tboxBuildTree,
        CVSROOT => $mozillaCvsroot,
        RELEASE_TAG => $releaseTag,
        VERSION => $version,
      },
    );

Since the method is being called from an instance of Config, we /could/ just access the config variables and substitute any we find, but my first inclination was to be explicit about what we want substituted.

> Do you have to build up a hash of values that you want interpolated? It would
> be nice if you could just pass in a Bootstrap::Config object, and it would do
> the right thing. Doing it this way  requires you to break abstractions. Or was
> there a reason for doing it this way? I know we talked about this briefly, but
> I don't remember all the details of that convo.

I could go either way, but my main concern is that we'd replace something we didn't intend to. I actually don't think that would happen though, I'll see what it looks like to just use the private %config hash in Bootstrap::Config. I imagine that it's just a matter of upper-casing and using %config instead of having to pass in a ref to %vars.

> -- The code inside of the for loop has basically three instances: handling a
> config line, handling a non-config line, and handling the line after a config
> line; these are expressed in the way the if-loop is structured, so that's good;
> could you add a comment about which branch is for each case?
> 
> +            foreach my $variable (grep(/%\w+%/, split(/\s+/, $line))) {
> 
> That might not be quite right; we've got some variables with -'s and _'s in
> them, so you might want to match /%[\w_-]+%/. I suggested the regex, so that's
> my bad.

\w+ does catch underscores, but not dashes. We get to choose what these are in the comments, I think being compliant to whatever a regex "word" is would be an easy enough rule to remember. Do you have a concrete example where dash would be useful?

I was just testing this on a real tinder-config and found a more serious issue; the above regex on encountering this:
# CONFIG: my $cvsroot = '%CVSROOT%';

Will actually think the key is:
'%CVSROOT%';

Because it is splitting on \s+ above. I have a patch coming up which does a followup:
                my $key = undef;
                if ($variable =~ /(%\w+%)/) {
                    $key = $1;
                    $key =~ s/%//g;
                } else {
                    die("Unable to parse $variable");
                }

Then for the replacement, because we want to preserve whatever was around the variable:

               my $value = $vars->{$key};
               $interpLine =~ s/\%$key\%/$value/g;

Unless you can think of a nicer way to do this? I think it should work in all cases that we care about, and it works in the staging environment (which has a real annotated tinder-config.pl). I'll add something that exercises this to the test case, as well.
Addresses the problem with the key in my previous comment, and uses the private %config hash in Bootstrap::Config.

This means that the caller simply does:

$config->Bump($tinderConfigFile);

The $tinderConfigFile would be expected to use the same exact variable names as the bootstrap.cfg, such as:

# CONFIG: my $cvsroot = '%mozillaCvsroot%';
Attachment #261736 - Attachment is obsolete: true
Attachment #261751 - Flags: review?(preed)
Comment on attachment 261751 [details] [diff] [review]
handle keys properly, and use private %config

>Index: Bootstrap/Config.pm
>===================================================================
>RCS file: /cvsroot/mozilla/tools/release/Bootstrap/Config.pm,v
>retrieving revision 1.5
>diff -u -r1.5 Config.pm
>--- Bootstrap/Config.pm	25 Mar 2007 23:54:25 -0000	1.5
>+++ Bootstrap/Config.pm	17 Apr 2007 03:56:47 -0000
>@@ -6,6 +6,7 @@
> 
> use strict;
> use POSIX "uname";
>+use File::Copy qw(move);
> 
> # shared static config
> my %config;
>@@ -30,7 +31,7 @@
> sub Parse {
>     my $this = shift;
>     
>-    open (CONFIG, "< bootstrap.cfg") 
>+    open(CONFIG, "< bootstrap.cfg") 
>       || die("Can't open config file bootstrap.cfg");
> 
>     while (<CONFIG>) {
>@@ -42,7 +43,7 @@
>         my ($var, $value) = split(/\s*=\s*/, $_, 2);
>         $config{$var} = $value;
>     }
>-    close CONFIG;
>+    close(CONFIG);
> }
> 
> ##
>@@ -162,4 +163,73 @@
>     }
> }
> 
>+##
>+# Bump - modifies config files
>+#
>+# Searches and replaces lines of the form:
>+#   # CONFIG: $BuildTag = '%productTag%_RELEASE';
>+#   $BuildTag = 'FIREFOX_1_5_0_9_RELEASE';
>+#
>+# The comment containing "CONFIG:" is parsed, and the value in between %%
>+# is treated as the key. The next line will be overwritten by the value
>+# matching this key in the private %config hash.
>+#
>+# If any of the requested keys are not found, this function calls die().
>+##
>+
>+sub Bump {
>+    my $this = shift;
>+    my %args = @_;
>+
>+    my $configFile = $args{'configFile'};
>+    if (! defined($configFile)) {
>+        die('ASSERT: Bootstrap::Config::Bump - configFile is a required argument');
>+    }
>+
>+    my $tmpFile = $configFile . '.tmp';
>+
>+    open(INFILE, "< $configFile") 
>+     or die ("Bootstrap::Config::Bump - Could not open $configFile for reading: $!");
>+    open(OUTFILE, "> $tmpFile") 
>+     or die ("Bootstrap::Config::Bump - Could not open $tmpFile for writing: $!");
>+
>+    my $skipNextLine = 0;
>+    foreach my $line (<INFILE>) {
>+        if ($skipNextLine) {
>+            $skipNextLine = 0;
>+            next;
>+        } elsif ($line =~ /^# CONFIG:\s+/) {
>+            print OUTFILE $line;
>+            $skipNextLine = 1;
>+            my $interpLine = $line;
>+            $interpLine =~ s/^#\s+CONFIG:\s+//;
>+            foreach my $variable (grep(/%\w+%/, split(/\s+/, $line))) {
>+                my $key = undef;
>+                if ($variable =~ /(%\w+%)/) {
>+                    $key = $1;
>+                    $key =~ s/%//g;
>+                } else {
>+                    die("Unable to parse $variable");
>+                }
>+
>+                if (! exists($config{$key})) {
>+                    die("ASSERT: no replacement found for $key");
>+                }
>+                my $value = $config{$key};

This is also breaking the abstraction.

You want to do a my $config = new Bootstrap::Config(); in the function, and then use $config->Get()

As for the other stuff, I think I get what you're trying to do, and there is a bug there, but I think you only need to modify two lines from the original patch:

+                $key =~ s/%//g;
becomes
+                $key =~ s/.*%[\w-]%.*/$1/g;

and

+                $interpLine =~ s/$variable/$value/g;
becomes
+                $interpLine =~ s/%$variable%/$value/g;

Also, the regex should probably minimally be [\w-] (\w includes _), since prohibiting -'s in the config file seems kinda weird.

I do like that there's an assertion about being unable to parse the variable.
Attachment #261751 - Flags: review?(preed) → review-
Attachment #261751 - Attachment is obsolete: true
Attachment #261849 - Flags: review?(preed)
Comment on attachment 261849 [details] [diff] [review]
look for dashes, simplify replace


>+            foreach my $variable (grep(/%\w+\-*%/, split(/\s+/, $line))) {

You want this regex to be /%[\w\-]+%/

>+                if (! ($key =~ s/.*%(\w+\-*)%.*/$1/g)) {

And this to be s/.*%([\w\-]+)%.*/$1/;

I *don't* think you want the g on there, because in the (admittedly pathological) case where you had something like:

%foo%%bar%, it would replace everything with the variable of $foo. Except, I think that would break anyway.

>+                if (! $config->Exists(var => $key)) {
>+                    die("ASSERT: no replacement found for $key");
>+                }
>+                my $value = $config->Get(var => $key);

Do you want to use sysvar here, so we get the appropriate OS substitution?

Other than that, looks good; r=preed with those fixes.
Attachment #261849 - Flags: review?(preed) → review+
Attached patch patch as landed (obsolete) — Splinter Review
Landed (also put an example tinder-config.pl in t/tinder-config.pl for the testcase):

Checking in Bootstrap/Config.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Config.pm,v  <--  Config.pm
new revision: 1.6; previous revision: 1.5
done
Checking in t/test.pl;
/cvsroot/mozilla/tools/release/t/test.pl,v  <--  test.pl
new revision: 1.5; previous revision: 1.4
done
Attachment #261849 - Attachment is obsolete: true
Attached patch sample BuildConfig class (obsolete) — Splinter Review
So to finish this bug off, we need two more Step classes:
BuildConfig
RepackConfig

These will run before the Build and Repack steps, respectively.

Preed, what do you think of the attached BuildConfig class? It's pretty simple, the only thing I am unhappy with is the Verify() method, because it insists that there has been a checkin, which means that verify will not succeed if we need to rerun the class.

However, the Tag::Bump class currently does this, so maybe this is ok? Basically, you will not need to ever rerun Build::Config unless you need to reconfigure the tinder-config.pl, so maybe this is a moot point.
Attachment #261879 - Flags: review?(preed)
Oh, the other thing is - do we really need a RepackConfig? We have all the info we need to generate the _l10n_release as well as _release tinder-config.pl/mozconfig, perhaps we should just have something like:

1. Tag
2. TinderConfig
3. Build
4. Repack
5. ...

TinderConfig step would bump the mozconfig/tinder-config.pl for both the _release and _l10n_release tinderbox config files.
Comment on attachment 261879 [details] [diff] [review]
sample BuildConfig class

This looks fine; I'd add to the checkin message "Automated checkin for..."

I'd actually like to make that change for all the checkin messages, so maybe that can be a separate bug.
Attachment #261879 - Flags: review?(preed) → review+
Attached patch TinderConfig step (obsolete) — Splinter Review
As discussed above.
Attachment #261879 - Attachment is obsolete: true
Attachment #262195 - Flags: review?(preed)
I took the opportunity to update the Makefile and bootstrap.cfg.example to test as if it were Firefox 2.0.0.4, and to put more comments into bootstrap.cfg.example as well as all the needed new variables.
Attachment #262196 - Flags: review?(preed)
Attachment #262198 - Flags: review?(preed)
Attachment #262198 - Flags: review?(nrthomas)
Attachment #262199 - Flags: review?(nrthomas)
Attached patch make logs more consistent (obsolete) — Splinter Review
Same as before, but put the logfile names together correctly.
Attachment #262195 - Attachment is obsolete: true
Attachment #262201 - Flags: review?(preed)
Attachment #262195 - Flags: review?(preed)
(In reply to comment #21)
> Created an attachment (id=261863) [details]
> patch as landed

Rob, I was just going through this bug before reviewing and noticed that you missed Paul's review notes in comment #20. As committed the the regex's will match foo_bar, foo_bar---- etc but not foo_bar-baz; see also global replace and var/sysvar.
Comment on attachment 262198 [details]
Config::Bump annotations for MOZILLA_1_8_BRANCH_release linux configs

>+# CONFIG: $milestone     = 'firefox%version%';
> $milestone     = 'firefox2.0.0.3';

I guess this could be 
  # CONFIG: $milestone     = '%product%%version%';
although it seems unlikely the hardcode would bite us. Otherwise looks good, r=cf
Attachment #262198 - Flags: review?(nrthomas) → review+
Comment on attachment 262199 [details]
Config::Bump annotations for MOZILLA_1_8_BRANCH_l10n_release linux configs

>Index: tinder-config.pl
...
> %WGetFiles = (
>+# CONFIG: 	      "http://stage.mozilla.org/pub/mozilla.org/firefox/nightly/2.0.0.3-candidates/rc%rc%/firefox-%version%.en-US.linux-i686.tar.gz" => 
> 	      "http://stage.mozilla.org/pub/mozilla.org/firefox/nightly/2.0.0.3-candidates/rc1/firefox-2.0.0.3.en-US.linux-i686.tar.gz" => 
> 	      "/builds/tinderbox/Fx-Mozilla1.8-l10n-release/Linux_2.4.20-28.8_Depend/firefox.tar.gz"
> 	      );

Use %version% for 2.0.0.3 here. r=cf with that.
Attachment #262199 - Flags: review?(nrthomas) → review+
It occurs to me that there is an undocumented assumption with the Bump logic - that a # CONFIG line is followed by the old default. So you could imagine, after sufficient time had elapsed, that someone did this

 # CONFIG: $my_var = %some_key%narfnarfnarf;
 $some_other_var = 'foo';

and $some_other_var would be lost. Maybe we can extract the first word from $interpLine (after the s///) and use it to regexp like 
  if (skipNextLine and (line =~ /$firstword/))

I guess that wouldn't help for repeated mk_add_options in a mozconfig.
(In reply to comment #32)
> (From update of attachment 262199 [details])
> >Index: tinder-config.pl
> ...
> > %WGetFiles = (
> >+# CONFIG: 	      "http://stage.mozilla.org/pub/mozilla.org/firefox/nightly/2.0.0.3-candidates/rc%rc%/firefox-%version%.en-US.linux-i686.tar.gz" => 
> > 	      "http://stage.mozilla.org/pub/mozilla.org/firefox/nightly/2.0.0.3-candidates/rc1/firefox-2.0.0.3.en-US.linux-i686.tar.gz" => 
> > 	      "/builds/tinderbox/Fx-Mozilla1.8-l10n-release/Linux_2.4.20-28.8_Depend/firefox.tar.gz"
> > 	      );
> 
> Use %version% for 2.0.0.3 here. r=cf with that.
> 

Right, plus I missed rc%rc% also :) Thanks
(In reply to comment #33)
> It occurs to me that there is an undocumented assumption with the Bump logic -
> that a # CONFIG line is followed by the old default. So you could imagine,
> after sufficient time had elapsed, that someone did this
> 
>  # CONFIG: $my_var = %some_key%narfnarfnarf;
>  $some_other_var = 'foo';
> 
> and $some_other_var would be lost. Maybe we can extract the first word from
> $interpLine (after the s///) and use it to regexp like 
>   if (skipNextLine and (line =~ /$firstword/))
> 
> I guess that wouldn't help for repeated mk_add_options in a mozconfig.
> 

I considered doing something like this.. I don't think we should, as we're checking out of and back into CVS, so clobbered changes won't be lost. Also it'd be annoying to have this die and the release process stop because the next line is a little different (e.g. typo in the checked-in version or something like that).

I've been thinking of these "# CONFIG" annotations as declaring how you want the next line to look. If you put it at the end of a file or put something important on the line after, Config::Bump doesn't try to support that.
Attached patch patch as landedSplinter Review
Here's the end result, thanks for catching that Nick!
Attachment #261863 - Attachment is obsolete: true
Attachment #262287 - Attachment is obsolete: true
Comment on attachment 262196 [details] [diff] [review]
release, bootstrap.cfg.example, Makefile changes

When the TinderConfig patch lands, r=preed.
Attachment #262196 - Flags: review?(preed) → review+
Attachment #262198 - Flags: review?(preed) → review+
Comment on attachment 262199 [details]
Config::Bump annotations for MOZILLA_1_8_BRANCH_l10n_release linux configs


> 	     
"/builds/tinderbox/Fx-Mozilla1.8-l10n-release/Linux_2.4.20-28.8_Depend/firefox.tar.gz"

Shouldn't this line be: # CONFIG: /builds/tinderbox/Fx-Mozilla1.8-l10n-release/%linux_l10n_buildPlatform%/firefox.tar.gz
 	      );

If so, r=preed if you change that; if not, is there a reason?
Attachment #262199 - Flags: review?(preed) → review-
Comment on attachment 262201 [details] [diff] [review]
make logs more consistent

It looks like you attached too copies of the same file? Or are they different?

Also, this isn't in patch/diff format? (which I only ask because it confuses bugzilla's diff tool and doesn't show me where this is going to be checked in).

Anyway, this should also tag the configs with the appropriate _RELEASE and _RELEASE_l10n tags after checking in them in.

>          cmd => 'cvs',
>          cmdArgs => ['-d', $mozillaCvsroot, 
>                      'co', '-d', 'tinderbox-configs',
>                      '-r', $branch,
>                      'mozilla/tools/tinderbox-configs/' . 
>                      $product . '/' . $osname],

Would a catfile() call make this more readable? At first glance, I thought it was wrong; it's not, but that's because it's hard to parse ',' vs. '.' when glancing quickly.

>              cmdArgs => ['-d', $mozillaCvsroot, 
>                          'ci', '-m', 
>                          '"bumping config for release for ' . $version . '"', 
>                          'tinderbox-configs/' . $configFile],

Were we going to standardize on a log message format, i.e. "Automated configuration bump: $filename, etc., etc., etc.? I'm mostly interested in a header that I can easily grep through cvs log for to find all configs that were bumped by Bootstrap.
Attachment #262201 - Flags: review?(preed) → review-
(In reply to comment #39)
> (From update of attachment 262199 [details])
> 
> > 	     
> "/builds/tinderbox/Fx-Mozilla1.8-l10n-release/Linux_2.4.20-28.8_Depend/firefox.tar.gz"
> 
> Shouldn't this line be: # CONFIG:
> /builds/tinderbox/Fx-Mozilla1.8-l10n-release/%linux_l10n_buildPlatform%/firefox.tar.gz
>               );

Actually, I probably meant l10n_buildPlatform, since the variable expansion uses sysvar, not var.
(In reply to comment #40)
> (From update of attachment 262201 [details] [diff] [review])
> It looks like you attached too copies of the same file? Or are they different?
> 
> Also, this isn't in patch/diff format? (which I only ask because it confuses
> bugzilla's diff tool and doesn't show me where this is going to be checked in).


No, it's just the whole file, I'll do up a patch next time.

 
> Anyway, this should also tag the configs with the appropriate _RELEASE and
> _RELEASE_l10n tags after checking in them in.
> 
> >          cmd => 'cvs',
> >          cmdArgs => ['-d', $mozillaCvsroot, 
> >                      'co', '-d', 'tinderbox-configs',
> >                      '-r', $branch,
> >                      'mozilla/tools/tinderbox-configs/' . 
> >                      $product . '/' . $osname],
> 
> Would a catfile() call make this more readable? At first glance, I thought it
> was wrong; it's not, but that's because it's hard to parse ',' vs. '.' when
> glancing quickly.


No, because this is the CVS checkout path (it's always "mozilla/.../.../" regardless of what OS the client is on :) ).


> >              cmdArgs => ['-d', $mozillaCvsroot, 
> >                          'ci', '-m', 
> >                          '"bumping config for release for ' . $version . '"', 
> >                          'tinderbox-configs/' . $configFile],
> 
> Were we going to standardize on a log message format, i.e. "Automated
> configuration bump: $filename, etc., etc., etc.? I'm mostly interested in a
> header that I can easily grep through cvs log for to find all configs that were
> bumped by Bootstrap.
 

Yes we should do that, I'll change this.
(In reply to comment #42)

> > Would a catfile() call make this more readable? At first glance, I thought it
> > was wrong; it's not, but that's because it's hard to parse ',' vs. '.' when
> > glancing quickly.
> 
> No, because this is the CVS checkout path (it's always "mozilla/.../.../"
> regardless of what OS the client is on :) ).

Yes, but you converted cvs calls in the Stage, Repack, and Updates steps to use catfile() in the CVS paths. Have these not failed because they haven't been run on Windows, or?

I think catfile() is more readable, but it obviously won't work if it does the wrong thing on Win32. Does it? Or does it figure out it's running in Cygwin?

Anyway, we should pick one way of doing it, so let's change this one, or fix up all the others.
(In reply to comment #43)
> (In reply to comment #42)
> 
> > > Would a catfile() call make this more readable? At first glance, I thought it
> > > was wrong; it's not, but that's because it's hard to parse ',' vs. '.' when
> > > glancing quickly.
> > 
> > No, because this is the CVS checkout path (it's always "mozilla/.../.../"
> > regardless of what OS the client is on :) ).
> 
> Yes, but you converted cvs calls in the Stage, Repack, and Updates steps to use
> catfile() in the CVS paths. Have these not failed because they haven't been run
> on Windows, or?

> I think catfile() is more readable, but it obviously won't work if it does the
> wrong thing on Win32. Does it? Or does it figure out it's running in Cygwin?


It'll work in cygwin because the path seperator will be "/" not "\", but using catfile here is an error as there is never a case where the CVS module/file separator will be anything except "/".


> Anyway, we should pick one way of doing it, so let's change this one, or fix up
> all the others.


I've got a patch together for this, I'll test it and file a separate bug. 
(In reply to comment #39)
> (From update of attachment 262199 [details])
> 
> > 	     
> "/builds/tinderbox/Fx-Mozilla1.8-l10n-release/Linux_2.4.20-28.8_Depend/firefox.tar.gz"
> 
> Shouldn't this line be: # CONFIG:
> /builds/tinderbox/Fx-Mozilla1.8-l10n-release/%linux_l10n_buildPlatform%/firefox.tar.gz
>               );
> 
> If so, r=preed if you change that; if not, is there a reason?


Hmm ... yeah we should do that, I'll add it.
Attachment #262201 - Attachment is obsolete: true
Attachment #262517 - Flags: review?(preed)
Comment on attachment 262517 [details] [diff] [review]
standardize log message, use diff format


>+    foreach my $branch ($branchTag . '_release', $branchTag . '_l10n_release') {
>+        $this->Shell(
>+          cmd => 'cvs',
>+          cmdArgs => ['-d', $mozillaCvsroot, 
>+                      'co', '-d', 'tinderbox-configs',
>+                      '-r', $branch,
>+                      'mozilla/tools/tinderbox-configs/' . 
>+                      $product . '/' . $osname],

Are we constructing cvs paths like this, or using catfile()?

If we're doing this, then please convert the rest of the uses of catfile() and CVS paths in Bootstrap (they currently outnumber this usage). Or convert this one to use catfile().

I find catfile() more readable, but as you point out, it may not work on Win32 (has that ever been tested? I wonder if it works in Cygwin...)

I care more about consistent usage within Bootstrap than catfile() vs. constructing them manually.
Attachment #262517 - Flags: review?(preed) → review-
(In reply to comment #47)

> I find catfile() more readable, but as you point out, it may not work on Win32
> (has that ever been tested? I wonder if it works in Cygwin...)
> 
> I care more about consistent usage within Bootstrap than catfile() vs.
> constructing them manually.

Oops. I didn't read the previous comments before reviewing. I'll reset it to r? and look at the patches together when you post the other one to convert all the other calls.

A suggestion: you may consider creating your own cvsCatfile(), for the readability issue.

Attachment #262517 - Flags: review- → review?
Testing of this patch is in-progress, let me know if you see anything wrong with it.
Attachment #262526 - Flags: review?(preed)
(In reply to comment #49)
> Created an attachment (id=262526) [details]
> do not use catfile for CVS module/path
> 
> Testing of this patch is in-progress, let me know if you see anything wrong
> with it.


Ok, tested, looks good. Preed, I like the cvsCatfile() suggestion, but really I'd rather come back later in a separate bug and do something more complete (like either wrap CVS commands in methods like we have with CvsTag() or look into a third-party module, maybe VCS:CVS?)

I think the way it is done in this patch is pretty unreadable as the module path is on it's own line, and most of the paths don't require many substitutions (a couple are just hardcoded strings e.g. 'mozilla/tools/patcher')
(In reply to comment #51)
> Created an attachment (id=262602) [details]
> l10n conf - substitute version and l10n_buildPlatform

Landed this one: 

Checking in mozconfig;
/cvsroot/mozilla/tools/tinderbox-configs/firefox/linux/mozconfig,v  <--  mozconfig
new revision: 1.1.14.7; previous revision: 1.1.14.6
done
Checking in tinder-config.pl;
/cvsroot/mozilla/tools/tinderbox-configs/firefox/linux/tinder-config.pl,v  <--  tinder-config.pl
new revision: 1.1.18.9; previous revision: 1.1.18.8
done
(In reply to comment #27)
> Created an attachment (id=262198) [details]
> Config::Bump annotations for MOZILLA_1_8_BRANCH_release linux configs

Landed:

Checking in tinder-config.pl;
/cvsroot/mozilla/tools/tinderbox-configs/firefox/linux/tinder-config.pl,v  <--  tinder-config.pl
new revision: 1.1.14.5; previous revision: 1.1.14.4
done
(In reply to comment #50)
> I think the way it is done in this patch is pretty unreadable as the module
> path is on it's own line, and most of the paths don't require many
> substitutions (a couple are just hardcoded strings e.g.
> 'mozilla/tools/patcher')

Er, well I meant to say ".. is pretty readable", Freudian slip? :)

Anyway let me know what you think, I think the cvsCatfile is probably overkill just for these, but I'd like to make improvements later to the way we call CVS that will incorporate that style.
Assignee: rhelmer → build
Status: ASSIGNED → NEW
Assignee: build → rhelmer
Status: NEW → ASSIGNED
Comment on attachment 262526 [details] [diff] [review]
do not use catfile for CVS module/path

I guess I'm ok with this; I really do like the readability of catfile, though; you might consider replacing the catfile() calls with a CvsCatfile() that you can stick in MozBuild::Util; this should work:

sub CvsCatfile {
   return join('/', @_);
}
Attachment #262526 - Flags: review?(preed) → review+
Comment on attachment 262517 [details] [diff] [review]
standardize log message, use diff format

Looks good; only thing I might check is that there actually were diffs; mozconfig, for instance, might not change for enUS builds, so the cvs ci there is a no-op.
Attachment #262517 - Flags: review? → review+
(In reply to comment #56)
> (From update of attachment 262517 [details] [diff] [review])
> Looks good; only thing I might check is that there actually were diffs;
> mozconfig, for instance, might not change for enUS builds, so the cvs ci there
> is a no-op.

Yeah, I figured that "cvs commit" probably checks for changes before doing anything, I know that it's negligible at least.
Landed:

Checking in Makefile;
/cvsroot/mozilla/tools/release/Makefile,v  <--  Makefile
new revision: 1.5; previous revision: 1.4
done
Checking in bootstrap.cfg.example;
/cvsroot/mozilla/tools/release/bootstrap.cfg.example,v  <--  bootstrap.cfg.example
new revision: 1.3; previous revision: 1.2
done
Checking in release;
/cvsroot/mozilla/tools/release/release,v  <--  release
new revision: 1.10; previous revision: 1.9
done
RCS file: /cvsroot/mozilla/tools/release/Bootstrap/Step/TinderConfig.pm,v
done
Checking in Bootstrap/Step/TinderConfig.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/TinderConfig.pm,v  <--  TinderConfig.pm
initial revision: 1.1
done
One last thing on this bug - I am going to go ahead and create a Bootstrap::Config class and implement CvsCatfile(). I'll put a patch up here when it's been fully tested (tomorrow most likely).
This creates a Bootstrap/Config.pm and adds a simple CvsCatfile method.
The rest of the patch implements this, and also some other cleanup/consistency:

* make tag substeps run verify right after execute, don't wait until the end (find failures faster)

* use oldVersion in the right place in Updates (Verify not Execute)

* standardize on "Automated version bump for ..." in Bootstrap/Step/Tag/Bump.pm
Attachment #262526 - Attachment is obsolete: true
Attachment #262914 - Flags: review?(nrthomas)
Sorry, same as last time, but I remembered to include the Bootstrap/Util.pm and don't revert sysvar to var for verifyConfig.
Attachment #262914 - Attachment is obsolete: true
Attachment #262916 - Flags: review?(nrthomas)
Attachment #262914 - Flags: review?(nrthomas)
Comment on attachment 262916 [details] [diff] [review]
include Bootstrap/Util.pm, use sysvar for Update's verifyConfig 

r=cf providing
* in Bootstrap/Util.pm have just the one copy of the file :-)
* we use catfile at Bootstrap/Step/TinderboxConfig.m line 55, currently it's
    'tinderbox-configs' . $configFile

Some more general comments
* in Tag/Bump.pm: $moduleVer, $versionTxt, and $milestoneTxt are created with catfile() and then used both on the local machine and in cvs pulls (via @bumpfiles). This may not be portable.
* some calls to Shell() for CVS use several lines to separate arguments and some are compact. Can we pick one ? I'd vote for the former.
Attachment #262916 - Flags: review?(nrthomas) → review+
Also, did you think about automating tagging of the config files ? 
Attached patch catfile cleanupSplinter Review
Addressed all of cf's comments except the last one (format the CVS lines for better readability) as I think that should be a separate patch. 

r?'ing preed also since he's back now :)
Attachment #262916 - Attachment is obsolete: true
Attachment #263273 - Flags: review?(preed)
Attachment #263273 - Flags: review?(nrthomas)
Attachment #263273 - Flags: review?(nrthomas) → review+
Comment on attachment 263273 [details] [diff] [review]
catfile cleanup

>Index: Bootstrap/Util.pm
>===================================================================
>RCS file: Bootstrap/Util.pm
>diff -N Bootstrap/Util.pm
>--- /dev/null	1 Jan 1970 00:00:00 -0000
>+++ Bootstrap/Util.pm	30 Apr 2007 17:56:54 -0000
>@@ -0,0 +1,21 @@
>+#
>+# Bootstrap utility functions.
>+# 
>+
>+package Bootstrap::Util;
>+
>+use base qw(Exporter);
>+
>+our @EXPORT_OK = qw(CvsCatfile);
>+
>+use strict;
>+
>+##
>+# Turn an array of directory/filenames into a CVS module path.
>+# ( name comes from File::Spec::catfile() )
>+##
>+sub CvsCatfile {
>+   return join('/', @_);
>+}

This will conflict with my patch in bug 375782, but if you get this landed this afternoon, I'll re-merge my patch.

This is fine, except it does break our %args convention; it makes sense to do so here, but I thought I'd note it. May be worth a comment about why.

>@@ -106,22 +108,6 @@
>       log => catfile($logDir, 'tag_mozilla-checkout.log'),
>       checkFor => '^U',
>     );
>-
>-    # Call substeps
>-    my $numSteps = scalar(@subSteps);
>-    my $currentStep = 0;
>-    while ($currentStep < $numSteps) {
>-        my $stepName = $subSteps[$currentStep];
>-        eval {
>-            $this->Log(msg => 'Tag running substep ' . $stepName);
>-            my $step = "Bootstrap::Step::Tag::$stepName"->new();
>-            $step->Verify();
>-        };
>-        if ($@) {
>-            die("Tag substep $stepName Verify died: $@");
>-        }
>-        $currentStep += 1;
>-    }

Why is this getting removed?
Attachment #263273 - Flags: review?(preed) → review+
(In reply to comment #65)
> (From update of attachment 263273 [details] [diff] [review])
> >Index: Bootstrap/Util.pm

Will do.

> This will conflict with my patch in bug 375782, but if you get this landed this
> afternoon, I'll re-merge my patch.
> 
> This is fine, except it does break our %args convention; it makes sense to do
> so here, but I thought I'd note it. May be worth a comment about why.

> >@@ -106,22 +108,6 @@
> >       log => catfile($logDir, 'tag_mozilla-checkout.log'),
> >       checkFor => '^U',
> >     );
> >-
> >-    # Call substeps
> >-    my $numSteps = scalar(@subSteps);
> >-    my $currentStep = 0;
> >-    while ($currentStep < $numSteps) {
> >-        my $stepName = $subSteps[$currentStep];
> >-        eval {
> >-            $this->Log(msg => 'Tag running substep ' . $stepName);
> >-            my $step = "Bootstrap::Step::Tag::$stepName"->new();
> >-            $step->Verify();
> >-        };
> >-        if ($@) {
> >-            die("Tag substep $stepName Verify died: $@");
> >-        }
> >-        $currentStep += 1;
> >-    }
> 
> Why is this getting removed?
> 

See comment #60. I think that verify should be called right after execute on the substeps, we don't want to wait until all executes are done before verifying anything as it takes way longer for a failure to show up. This came up during testing, I know it's not totally related to this patch.
Added the following comment above Bootstrap::Util::CvsCatfile
##
# Turn an array of directory/filenames into a CVS module path.
# ( name comes from File::Spec::catfile() )
#
# Note that this function does not take any arguments, to make the usage
# more like File::Spec::catfile()
##

and landed:

RCS file: /cvsroot/mozilla/tools/release/Bootstrap/Util.pm,v
done
Checking in Bootstrap/Util.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Util.pm,v  <--  Util.pm
initial revision: 1.1
done
Checking in Bootstrap/Step/Repack.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/Repack.pm,v  <--  Repack.pm
new revision: 1.12; previous revision: 1.11
done
Checking in Bootstrap/Step/Stage.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/Stage.pm,v  <--  Stage.pm
new revision: 1.12; previous revision: 1.11
done
Checking in Bootstrap/Step/Tag.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/Tag.pm,v  <--  Tag.pm
new revision: 1.9; previous revision: 1.8
done
Checking in Bootstrap/Step/TinderConfig.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/TinderConfig.pm,v  <--  TinderConfig.pm
new revision: 1.2; previous revision: 1.1
done
Checking in Bootstrap/Step/Updates.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/Updates.pm,v  <--  Updates.pm
new revision: 1.11; previous revision: 1.10
done
Checking in Bootstrap/Step/Tag/Bump.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/Tag/Bump.pm,v  <--  Bump.pm
new revision: 1.7; previous revision: 1.6
done
Checking in Bootstrap/Step/Tag/Talkback.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/Tag/Talkback.pm,v  <--  Talkback.pm
new revision: 1.3; previous revision: 1.2
done
Checking in Bootstrap/Step/Tag/l10n.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/Tag/l10n.pm,v  <--  l10n.pm
new revision: 1.3; previous revision: 1.2
done

Hopefully done here :) Leaving the bug open until the dependencies land, but reassigning to build@mozilla-org.bugs
Assignee: rhelmer → build
Status: ASSIGNED → NEW
Depends on: 379380
needed for "release automation", hence marking as critical.
Severity: normal → critical
Status: NEW → 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.