Closed Bug 375219 Opened 17 years ago Closed 17 years ago

egg.build.m.o making spurious configure checkins

Categories

(Release Engineering :: General, defect)

defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: nthomas, Assigned: rhelmer)

References

()

Details

Attachments

(4 files, 1 obsolete file)

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.
Now happening on Mozilla1.8, too.
Assignee: build → rhelmer
Status: NEW → ASSIGNED
Attached file cron script
Attached file configure script
Attachment #259535 - Attachment mime type: application/octet-stream → text/plain
Attachment #259536 - Attachment mime type: application/octet-stream → text/plain
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. 
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.
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 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+
(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.
egg did it's trick on the 1.8.0 branch again today.
egg attacked trunk about 40ish minutes ago; I hope I've undone the damage it caused now by backing out its checkins.
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.
configure auto-generation is disabled until we figure this out. I'll be looking at it today.
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...
(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.
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.
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?
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.)
(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?
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.
(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.
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+
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
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.