there have been numerous reports recently (though no issues filed that i've seen) of commit messages getting confused and attributed to the wrong users/projects. at first, i thought it might be a lack of table locking and transactions in the xcvs-* scripts. however, it appears we're correctly locking things.
however, there's something that could potentially be the problem: xcvs-commitinfo.php and xcvs-loginfo.php rely on files written to /tmp using the posix_getpgrp() (process group) of the xcvs-* scripts. xcvs-commitinfo.php builds up a file of commit messages (since it gets invoked separately for each file), and then xcvs-loginfo.php combines everything into a single commit. i'm not positive, but it seems possible that this could be getting confused in some cases, and commits could be written into a /tmp file that should really be associated with a different user.
i've never bothered to fully understand all this "log_combine" code, assuming it all must just work. ;) however, i've now got my doubts... if it's corrupting commit info, it's really a critical bug. given a quick look, this seems to be about the only thing i can think of that might be causing this.
one possible solution: use both getpgrp() *and* the uid or username of the committer. that should help ensure it's unique, and yet you still get the same answer in both places for the same commit.
btw, there are 18 /tmp/xcvs-lastlog.* files on cvs.d.o right now, the most recent dating from mar 8th. so, it could also be a problem with stale files that are then re-used when the OS cycles through the pid space again and starts reusing process groups...
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | be-nice-to-sequences_0.patch | 2.08 KB | bdragon |
| #1 | be-nice-to-sequences.patch | 2.02 KB | bdragon |
Comments
Comment #1
bdragon commentedPatch as per IRC discussion, it appears it was a database locking issue big enough to drive a truck through.
Comment #2
bdragon commentedAnd I'm too used to having db_query. *sigh*
/me takes a few to rewrite xcvs_next_id PROPERLY...
Comment #3
bdragon commentedOK, this one actually got TESTED... grumble grumble editing the wrong file grumble
Comment #4
dwwreviewed (looks perfect), tested on s.d.o (works perfect).
committed to HEAD, DRUPAL-4-7--2, and DRUPAL-4-7.
installed on cvs.d.o for both live and contrib repos.
/me feels stupid for overlooking the {sequences} table when considering the DB locking...
THANKS BDRAGON!
Comment #5
(not verified) commented