Closed Bug 411928 Opened 12 years ago Closed 12 years ago

Bootstrap::Step::TinderConfig should support Nightly mode

Categories

(Release Engineering :: General, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: bhearsum, Assigned: bhearsum)

References

Details

Attachments

(6 files, 14 obsolete files)

1.36 KB, patch
Details | Diff | Splinter Review
2.43 KB, patch
rhelmer
: review+
Details | Diff | Splinter Review
3.83 KB, patch
Details | Diff | Splinter Review
572 bytes, patch
rhelmer
: review+
Details | Diff | Splinter Review
7.93 KB, patch
Details | Diff | Splinter Review
6.47 KB, patch
Details | Diff | Splinter Review
This would save us from modifying the tinder-config files each time we sync the cvs mirror. I tried running it once, but it failed in Verify(). We may have to adjust Verify() or just skip it all together.
Assignee: nobody → bhearsum
Priority: P3 → P2
As far as I can tell the only thing that will break nightly mode is the tagging of the tinder-config/mozconfig files. This patch is untested as of yet.
Attachment #297565 - Flags: review?(rhelmer)
Status: NEW → ASSIGNED
Comment on attachment 297565 [details] [diff] [review]
add nightly support to TinderConfig

>Index: Bootstrap/Step/TinderConfig.pm
>===================================================================
>RCS file: /cvsroot/mozilla/tools/release/Bootstrap/Step/TinderConfig.pm,v
>retrieving revision 1.6
>diff -u -r1.6 TinderConfig.pm
>--- Bootstrap/Step/TinderConfig.pm	8 Jan 2008 17:39:11 -0000	1.6
>+++ Bootstrap/Step/TinderConfig.pm	17 Jan 2008 17:35:55 -0000
>@@ -81,6 +81,11 @@
>             );
>         }
> 
>+        if ($version eq 'nightly') {
>+            $this->Log(msg => 'Skip TinderConfig tagging for nightly mode');
>+            return;
>+        }
>+
>         my @tagNames = ($productTag . '_RELEASE',
>                         $productTag . '_RC' . $rc);
> 
>@@ -148,10 +153,13 @@
>             notAllowed => 'aborted',
>         );
> 
>-        $this->CheckLog(
>-            log => catfile($logDir, 'build_config-tag-' . $branch . '.log'),
>-            checkFor => '^T',
>-        );
>+        # In nightly mode we don't do any tagging, so there's nothing to verify
>+        if ($version = 'nightly') {

Do you mean "if ($version != 'nightly')"?
Attachment #297565 - Flags: review?(rhelmer) → review-
> Do you mean "if ($version != 'nightly')"? 
> 

Yeah, I do. Thanks for catching that...this is what happens when I try to write code on a bus, heh.
Attached patch fix silly error (obsolete) — Splinter Review
Attachment #297565 - Attachment is obsolete: true
Attachment #297581 - Flags: review?(rhelmer)
Comment on attachment 297581 [details] [diff] [review]
fix silly error

Looks good, but can you add a Log msg in the "skipping verify" case too?
Attachment #297581 - Flags: review?(rhelmer) → review+
Checking in Bootstrap/Step/TinderConfig.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/TinderConfig.pm,v  <--  TinderConfig.pm
new revision: 1.7; previous revision: 1.6
done
Attachment #297581 - Attachment is obsolete: true
I'm going to put these changes in place on staging 1.9 and see how it goes.
Attachment #297589 - Flags: review?(rhelmer)
Attachment #297589 - Flags: review?(rhelmer) → review+
Attachment #297585 - Attachment description: patch as landed → [checked in] patch as landed
I didn't add a variable for BuildTag, because it's commented out (unused?) nor for milestone, since it is static as 'trunk'.
Attachment #297611 - Flags: review?(rhelmer)
Attachment #297611 - Flags: review?(rhelmer) → review+
Note that tests have been disabled because they were crashing on OS X. We can re-enable when necessary pretty easily.
Attachment #297611 - Attachment is obsolete: true
Attachment #297796 - Flags: review?(rhelmer)
Attachment #297797 - Flags: review?(rhelmer)
Attachment #297589 - Attachment is obsolete: true
Attachment #297798 - Flags: review?(rhelmer)
Attachment #297796 - Flags: review?(rhelmer) → review+
Comment on attachment 297797 [details] [diff] [review]
pull from the right branches for nightly mode

won't work for nightly/branch - needs to be ($branchTag, $branchTag+'_l10n'), I think.
Attachment #297797 - Flags: review?(rhelmer) → review-
Attachment #297798 - Flags: review?(rhelmer) → review+
I factored this logic out to a helper function since it was getting a bit large, and used in two places.
The latest osx dep build on staging-1.9-master was running this code.
Attachment #297797 - Attachment is obsolete: true
Attachment #297816 - Flags: review?(rhelmer)
Comment on attachment 297816 [details] [diff] [review]
properly determine nightly branches

>Index: Bootstrap/Step/TinderConfig.pm
>===================================================================
>RCS file: /cvsroot/mozilla/tools/release/Bootstrap/Step/TinderConfig.pm,v
>retrieving revision 1.7
>diff -u -r1.7 TinderConfig.pm
>--- Bootstrap/Step/TinderConfig.pm	17 Jan 2008 19:40:20 -0000	1.7
>+++ Bootstrap/Step/TinderConfig.pm	18 Jan 2008 18:29:20 -0000
>@@ -40,16 +40,7 @@
>     MkdirWithPath(dir => $productConfigBumpDir)
>       or die("Cannot mkdir $productConfigBumpDir: $!");
> 
>-    my @branches = ();
>-    # tinderbox-configs tags are different on branches vs trunk
>-    # Do the right thing in both cases
>-    if ($branchTag eq 'HEAD') {
>-        push(@branches, ('release', 'l10n_release'));
>-    }
>-    else {
>-        push(@branches, $branchTag . '_release');
>-        push(@branches, $branchTag . '_l10n_release');
>-    }
>+    my @branches = @{DetermineBranches()};
> 
>     foreach my $branch (@branches) {
>         $this->CvsCo(cvsroot => $mozillaCvsroot,
>@@ -82,8 +73,8 @@
>         }
> 
>         if ($version eq 'nightly') {
>-            $this->Log(msg => 'Skip TinderConfig tagging for nightly mode');
>-            return;
>+            $this->Log(msg => 'Skipping TinderConfig tagging for nightly mode');
>+            next;
>         }
> 
>         my @tagNames = ($productTag . '_RELEASE',
>@@ -128,17 +119,9 @@
>     my $config = new Bootstrap::Config();
>     my $branchTag = $config->Get(var => 'branchTag');
>     my $logDir = $config->Get(sysvar => 'logDir');
>+    my $version = $config->Get(var => 'version');
>     
>-    my @branches = ();
>-    # tinderbox-configs tags are different on branches vs trunk
>-    # Do the right thing in both cases
>-    if ($branchTag eq 'HEAD') {
>-        push(@branches, ('release', 'l10n_release'));
>-    }
>-    else {
>-        push(@branches, $branchTag . '_release');
>-        push(@branches, $branchTag . '_l10n_release');
>-    }
>+    my @branches = @{DetermineBranches()};


assert that you got something back here. r+ with that change.
Attachment #297816 - Flags: review?(rhelmer) → review+
Feel free to check these patches in today, Rob. I'm heading out soon and want to be around to watch for possible fallout if I check them in :).
Attachment #297816 - Attachment is obsolete: true
Comment on attachment 297839 [details] [diff] [review]
[checked in] add asserts

Checking in Bootstrap/Step/TinderConfig.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Step/TinderConfig.pm,v  <--  TinderConfig.pm
new revision: 1.8; previous revision: 1.7
done
Checking in configs/fx-moz19-nightly-bootstrap.cfg;
/cvsroot/mozilla/tools/release/configs/fx-moz19-nightly-bootstrap.cfg,v  <--  fx-moz19-nightly-bootstrap.cfg
new revision: 1.4; previous revision: 1.3
done
Attachment #297839 - Attachment description: add asserts → [checked in] add asserts
Comment on attachment 297839 [details] [diff] [review]
[checked in] add asserts

>? bootstrap.cfg
>? t/tinder-config.pl
>Index: Bootstrap/Step/TinderConfig.pm
>===================================================================
>RCS file: /cvsroot/mozilla/tools/release/Bootstrap/Step/TinderConfig.pm,v
>retrieving revision 1.7
>diff -u -r1.7 TinderConfig.pm
>--- Bootstrap/Step/TinderConfig.pm	17 Jan 2008 19:40:20 -0000	1.7
>+++ Bootstrap/Step/TinderConfig.pm	18 Jan 2008 20:05:58 -0000
>@@ -177,3 +172,37 @@
>+        if ($version eq 'nightly') {
>+            push(@branches, $branchTag);
>+            push(@branches, $branchTag . 'l10n');

Fixed typo here as followup patch; should be '_l10n' not 'l10n'
Comment on attachment 297861 [details] [diff] [review]
do not make sure this dir does not exist if in nightly mode

Actually, I didn't notice that Ben cleaned this first as a buildbot step; that's actually better. Sorry for the noise.
Attachment #297861 - Attachment is obsolete: true
Attachment #297861 - Flags: review?
Attached patch explicitly set update_aus_host (obsolete) — Splinter Review
This works, but we need to explicitly set to aus servername so we have the chance to override it.
Attachment #297796 - Attachment is obsolete: true
Attachment #297912 - Flags: review?(bhearsum)
Attached patch explicitly set update_aus_host (obsolete) — Splinter Review
Attachment #297912 - Attachment is obsolete: true
Attachment #297912 - Flags: review?(bhearsum)
Attached patch explicitly set update_aus_host (obsolete) — Splinter Review
don't override pre-set CVSROOT, this is needed for production!
Attachment #297921 - Attachment is obsolete: true
Attachment #297922 - Flags: review?(nrthomas)
Attachment #298034 - Flags: review?(nrthomas)
get update_filehost too
Attachment #297922 - Attachment is obsolete: true
Attachment #298035 - Flags: review?(nrthomas)
Attachment #297922 - Flags: review?(nrthomas)
bump mofo cvsroot for 1.8 only
Attachment #298034 - Attachment is obsolete: true
Attachment #298036 - Flags: review?(nrthomas)
Attachment #298034 - Flags: review?(nrthomas)
I think there are a few other things we should be able to bump too:

* TestsPhoneHome (whether or not tests report results)
* pageload server
* graph server

We should run staging versions of the latter two, so I'm not as worried about those. TestsPhoneHome we'll need relatively soon.
Comment on attachment 298035 [details] [diff] [review]
enable config bump for 1.9 branch nightlies

r+. 

These lines can be removed as it's now the default key, and might make things a bit easier for you on the staging machines:
 $ssh_key       = "'$ENV{HOME}/.ssh/ffxbld_dsa'";

We should convert the production boxes to ffxbld at some point, but will need a r/w key for CVS to commit the config changes etc etc.
Attachment #298035 - Flags: review?(nrthomas) → review+
Comment on attachment 298036 [details] [diff] [review]
enable config bump for 1.8 branch nightlies

r+
Attachment #298036 - Flags: review?(nrthomas) → review+
Attachment #298263 - Flags: review?(rhelmer) → review+
Comment on attachment 297798 [details] [diff] [review]
[checked in] clean up config area before running tinder-config

Checking in staging/master.cfg;
/cvsroot/mozilla/tools/buildbot-configs/automation/staging/master.cfg,v  <--  master.cfg
new revision: 1.18; previous revision: 1.17
done
Checking in staging-1.9/master.cfg;
/cvsroot/mozilla/tools/buildbot-configs/automation/staging-1.9/master.cfg,v  <--  master.cfg
new revision: 1.15; previous revision: 1.14
done
Attachment #297798 - Attachment description: clean up config area before running tinder-config → [checked in] clean up config area before running tinder-config
Comment on attachment 298263 [details] [diff] [review]
[checked in] add turnOffTests to 1.8 nightly bootstrap.cfg

Checking in configs/fx-moz18-nightly-bootstrap.cfg;
/cvsroot/mozilla/tools/release/configs/fx-moz18-nightly-bootstrap.cfg,v  <--  fx-moz18-nightly-bootstrap.cfg
new revision: 1.3; previous revision: 1.2
done
Attachment #298263 - Attachment description: add turnOffTests to 1.8 nightly bootstrap.cfg → [checked in] add turnOffTests to 1.8 nightly bootstrap.cfg
One thing I just thought of:
TinderConfig checks in new configs after bumping them. I'm not sure we want to do this in production. Will other machines be using these configs?
(In reply to comment #34)
> One thing I just thought of:
> TinderConfig checks in new configs after bumping them. I'm not sure we want to
> do this in production. Will other machines be using these configs?

I don't think it will do any harm. It will prevent anyone from being able to e.g. change the BuildTree if bootstrap disagrees with what it should be, but that is ok with me.

BTW your patches looks good, are you going to land those?
Comment on attachment 298735 [details] [diff] [review]
[checked in] fix runMozillaTests config line

Checking in linux/tinder-config.pl;
/cvsroot/mozilla/tools/tinderbox-configs/firefox/linux/tinder-config.pl,v  <--  tinder-config.pl
new revision: 1.21; previous revision: 1.20
done
Checking in macosx/tinder-config.pl;
/cvsroot/mozilla/tools/tinderbox-configs/firefox/macosx/tinder-config.pl,v  <--  tinder-config.pl
new revision: 1.39; previous revision: 1.38
done
Checking in win32/tinder-config.pl;
/cvsroot/mozilla/tools/tinderbox-configs/firefox/win32/tinder-config.pl,v  <--  tinder-config.pl
new revision: 1.27; previous revision: 1.26
done
Attachment #298735 - Attachment description: fix runMozillaTests config line → [checked in] fix runMozillaTests config line
Comment on attachment 298736 [details] [diff] [review]
[checked in] fix runMozillaTests config line

Checking in linux/tinder-config.pl;
/cvsroot/mozilla/tools/tinderbox-configs/firefox/linux/tinder-config.pl,v  <--  tinder-config.pl
new revision: 1.1.12.13; previous revision: 1.1.12.12
done
Checking in macosx/tinder-config.pl;
/cvsroot/mozilla/tools/tinderbox-configs/firefox/macosx/tinder-config.pl,v  <--  tinder-config.pl
new revision: 1.8.4.11; previous revision: 1.8.4.10
done
Checking in win32/tinder-config.pl;
/cvsroot/mozilla/tools/tinderbox-configs/firefox/win32/tinder-config.pl,v  <--  tinder-config.pl
new revision: 1.2.10.12; previous revision: 1.2.10.11
done
Attachment #298736 - Attachment description: fix runMozillaTests config line → [checked in] fix runMozillaTests config line
All the patches for this have been checked in now. I want a run on the staging environments to make sure update_verify works before declaring this fixed.
(In reply to comment #38)
> All the patches for this have been checked in now. I want a run on the staging
> environments to make sure update_verify works before declaring this fixed.
> 

Oops. This comment was supposed to go in bug 398494.
This seems to be working fine in the staging environments.
Status: ASSIGNED → RESOLVED
Closed: 12 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.