Closed
Bug 375219
Opened 17 years ago
Closed 17 years ago
egg.build.m.o making spurious configure checkins
Categories
(Release Engineering :: General, defect)
Release Engineering
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: nthomas, Assigned: rhelmer)
References
()
Details
Attachments
(4 files, 1 obsolete file)
123 bytes,
text/plain
|
Details | |
1.07 KB,
text/plain
|
Details | |
739 bytes,
patch
|
preed
:
review+
|
Details | Diff | Splinter Review |
2.09 KB,
patch
|
dbaron
:
review+
|
Details | Diff | Splinter Review |
The automatic configure.in --> configure job that runs on egg is making checkins in the absence of any change to configure.in. See the URL for full details, but basically there are two checkins. Overall they make no change but the first checkin leaves a lot of information out. Trunk machines were busted just now when they tried to build with the incomplete file from the first checkin, same on Mozilla1.8.0 yesterday.
Comment 1•17 years ago
|
||
Now happening on Mozilla1.8, too.
Assignee | ||
Updated•17 years ago
|
Assignee: build → rhelmer
Assignee | ||
Updated•17 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•17 years ago
|
||
Assignee | ||
Comment 3•17 years ago
|
||
Updated•17 years ago
|
Attachment #259535 -
Attachment mime type: application/octet-stream → text/plain
Updated•17 years ago
|
Attachment #259536 -
Attachment mime type: application/octet-stream → text/plain
Assignee | ||
Comment 4•17 years ago
|
||
Not sure what happened here, but it looks like we had two checkins to each of trunk and MOZILLA_1_8_BRANCH, I think that the first one was in error and the second corrected it (need more time to verify this). As you can see from attachment 259536 [details] the configure script, which runs every 10 minutes:
* creates a temporary working area
* checks out appropriate files
* runs autoconf
* does "cvs diff configure"
If the exit code from step #4 is not 0, then check in with the message "Automated update from host $HOST".
* remove working area
Something could go wrong with the autoconf run (like a crash) and we'd still get the checkin.
Assignee | ||
Comment 5•17 years ago
|
||
I checked that the current configure is the same as revision 1.1788 Best guess so far is that something went wrong (maybe in cvs checkout or while autoconf was running, like autoconf crashing) which resulted in configure being generated incorrectly and 1.1789 was checked in. The next run went ok and 1.1790 was checked in (identical to 1.1788). At the very least let's get this script in CVS and put some more error checking into it. For instance, we should only check in if "cvs diff" is error code 1 (not just non-0) and exit on any other error. Logs about errors should go somewhere. Longer-term, this does not need to be on a dedicated host. This could be done with a buildbot scheduler that knows to look for changes to the relevant files and only does it as-needed, using most any available build machine.
Assignee | ||
Comment 6•17 years ago
|
||
Also include obsolete sync-configure (we use sync-configure-better, which is apparently *better*). There's already a mozilla/tools/build with old stuff in it, I went ahead and added a description of what these are to the existing README file.
Attachment #259630 -
Flags: review?(preed)
Comment 7•17 years ago
|
||
Comment on attachment 259630 [details] [diff] [review] add sync-configure scripts to CVS Looks good, but let's check in sync-configure as sync-configure, and then check in sync-configure-better over the top of sync-configure, so we can get meaningful diffs. might have to change sync-all-configures to point to sync-configure, not sync-configure-better, too. Also, after checking in, we should announce some downtime/possible wonkiness before switching over to the copy from CVS. We could announce it today and do it on Monday.
Attachment #259630 -
Flags: review?(preed) → review+
Assignee | ||
Comment 8•17 years ago
|
||
(In reply to comment #7) > (From update of attachment 259630 [details] [diff] [review]) > Looks good, but let's check in sync-configure as sync-configure, and then check > in sync-configure-better over the top of sync-configure, so we can get > meaningful diffs. might have to change sync-all-configures to point to > sync-configure, not sync-configure-better, too. Ok that's all done, I went ahead and made the cronjob script point to the new version in the second checkin as well, so it should work out-of-the-box now. > Also, after checking in, we should announce some downtime/possible wonkiness > before switching over to the copy from CVS. We could announce it today and do > it on Monday. How should we do this? Add CVS checkout capability to sync-all-configures, or have a "cvs co whatever && sync-all-configures" cron line? The former doesn't look as ugly but the latter lets us adjust branches and such through CVS. Unless someone has a better suggestion I am leaning toward the latter.
Reporter | ||
Comment 10•17 years ago
|
||
egg did it's trick on the 1.8.0 branch again today.
Comment 11•17 years ago
|
||
egg attacked trunk about 40ish minutes ago; I hope I've undone the damage it caused now by backing out its checkins.
Comment 12•17 years ago
|
||
Note that egg made two checkins in the incident from comment 11. The first screwed up everything; the second un-screwed up a lot of things, but it didn't un-screw up the testing tinderboxen. I'm very tempted to raise the bug status to blocker.
Assignee | ||
Comment 13•17 years ago
|
||
configure auto-generation is disabled until we figure this out. I'll be looking at it today.
Comment 14•17 years ago
|
||
Would be nice if someone would add a note on the Firefox tinderbox page that configure needs to be checked in manually until this is fixed...
Assignee | ||
Comment 15•17 years ago
|
||
(In reply to comment #14) > Would be nice if someone would add a note on the Firefox tinderbox page that > configure needs to be checked in manually until this is fixed... Good idea, done.
Assignee | ||
Comment 16•17 years ago
|
||
I've re-enabled configure autogeneration. All the scripts are checked into CVS under mozilla/tools/build, and egg's crontab has this entry: 1,11,21,31,41,51 * * * * date >> /tmp/sync-all-configures.log && cd /home/cltbld/bin && cvs up && ./sync-all-configures >> /tmp/sync-all-configures.log 2>&1 This is just a slight modification to what was there before; it won't wipe the log out every run now (which I think was the original intent, seeing as how date is appended to the log before the run). Hopefully this will give us a clue as to what's going on. My guess is that autoconf is crashing, I'll work up a patch to make the error checking better.
Assignee | ||
Comment 17•17 years ago
|
||
Any non-zero exit code from autoconf or cvs checkout is bad, and only an exit code of 1 or 0 are ok from cvs diff.
Attachment #260768 -
Flags: review?
Assignee | ||
Updated•17 years ago
|
Attachment #260768 -
Flags: review? → review?(dbaron)
This has the problem that it will accumulate files in /tmp on any error. The simplest fix might be to put the stuff after the creation of the temporary directory in a function, e.g., use_tmpdir() { ... then echo "ERROR: ..." return 1 fi return 0 } use_tmpdir result=$? cd /tmp rm -rf $TDIR exit $result (It also might be nice to use mktemp -d.)
Comment 19•17 years ago
|
||
(In reply to comment #18) > This has the problem that it will accumulate files in /tmp on any error. The > simplest fix might be to put the stuff after the creation of the temporary > directory in a function, e.g., Well, it might also be that in the failure case, the files left in /tmp might aid in ascertaining the cause. Perhaps leaving this as is and adding another job that runs out of cron (or just adding a section to this one) to delete the files this leavens in /tmp that are over 7 days old or something similar to that would be better?
Comment 20•17 years ago
|
||
According to the tinderbox page, configure.in auto-generation is back on. But bsmedberg checked in a change to configure.in at 2007-04-10 08:37, and configure hasn't been regenerated yet.
Assignee | ||
Comment 21•17 years ago
|
||
(In reply to comment #20) > According to the tinderbox page, configure.in auto-generation is back on. But > bsmedberg checked in a change to configure.in at 2007-04-10 08:37, and > configure hasn't been regenerated yet. Works when I ran it by hand; probably a problem with the crontab. I just adjusted it a bit, I'll keep an eye on it and post a corrected attachment once it works as expected.
Assignee | ||
Comment 22•17 years ago
|
||
Attachment #260768 -
Attachment is obsolete: true
Attachment #261151 -
Flags: review?(dbaron)
Attachment #260768 -
Flags: review?(dbaron)
Comment on attachment 261151 [details] [diff] [review] move most work into use_tmpdir(), clean up tmpdir after runs >+ elif [ $DIFF_STATUS == 0 ] >+ then >+ rm -rf /tmp/${TDIR} >+ return 0 You don't need this rm -rf, since there's another one below. Other than that, looks fine. Sorry for the delay. r=dbaron
Attachment #261151 -
Flags: review?(dbaron) → review+
Assignee | ||
Comment 24•17 years ago
|
||
Landed, I'll keep an eye on egg to make sure it's running ok: Checking in sync-configure; /cvsroot/mozilla/tools/build/sync-configure,v <-- sync-configure new revision: 1.3; previous revision: 1.2 done
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Product: mozilla.org → Release Engineering
You need to log in
before you can comment on or make changes to this bug.
Description
•