On Wed, Oct 23, 2002 at 07:08:47AM +0200, Marko Macek wrote:
> Greg Stein wrote:
> > On Tue, Oct 22, 2002 at 10:20:27PM +0200, Marko Macek wrote:
>...
> > Also, note that c.has_file(fname) is not necessarily a proper condition for
> > processing a commit either. In the above example, it is possible that id-A
> > changes a file in the first two lines, and then id-B deletes the file (but
> > id-A is not complete).
>
> Hmm. It seems unlikely that two "overlapping" commits could work on the
> same file (except maybe on different branches). I believe CVS will not
> allow a commit if the file is not up-to-date.
While the first commit is processing, a second user could do an update, make
a change, and commit it back. Quite reasonable and possible.
Imagine a case where the first committer is touching every file (say, to
update the copyright years in some source code); in addition, she is
operating on a slow link, so this commit is taking a long time. The second
committer comes along, tries to commit, gets a conflict errors, updates, and
then commits again. He is on a fast link, so his commit is completed
*before* the first committer's massive update is completed.
[ and given that CVS will send the fulltext of the modified file to the
server, a codebase-wide change can cause a *lot* of data to be uploaded;
thus, I don't find this scenario all that unlikely; and even if it *were*
unlikely, cvs2svn must account for these edge cases; either to detect them
and provide a warning, or to actually handle them ]
>...
> Yes. The way we currently lose revisions during conversion is bad (plus
> it breaks my tagging/branching code).
I don't doubt it :-) In any case, that problem has been fixed. A stupid
error on my part a while back.
> > even though that may not reflect what *really* happened. I would state that
> > we should issue a warning so that a human can investigate. Ideally, we could
> > provide controls so that a human could separate an interleaved commit from
> > the same user/message.
>
> For now, I will remove the last condition in this part of the patch:
>
> >> # scan for commits to process
> >> process = [ ]
> >> - for id, c in commits.items():
> >> - if c.t_max + COMMIT_THRESHOLD < timestamp:
> >> + for xid, c in commits.items():
> >> + if c.t_max + COMMIT_THRESHOLD < timestamp or \
> >> + c.has_file(fname) or \
> >> + xid != id:
Euh... didn't we just get done stating that c.has_file() is (also) not an
appropriate condition for this test?
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Oct 24 04:05:59 2002