Re: cvs2git: One possible bug, and two other issues

On Sun, Jan 9, 2011 at 9:02 AM, Michael Haggerty <[email protected]> wrote:
> On 01/03/2011 03:48 PM, Anders Pilegaard wrote:
>> Issue 3 - Allow selected tags to "expand" to cover more files
>> -------------------------------------------------------------
>> [...]
>> In our CVS repository we normally tag bugfixes with "before" and
>> "after" tags, to make it easier to apply them to another branch later
>> on.  Mostly we let those tags cover the entire repository.  But
>> sometimes - especially when the commit changes only a few files - we
>> just tag the affected files.
>> [...]
>> But this was not the intention of those tags - they were meant to
>> attach to specific commits.  And they do tag the files actually
>> changed in the commit - just not all the files in the repository.
>>
>> So I added some more code to allow certain tags to pass the
>> "is_simple_copy" test [...]
>
> I like this idea.  I guess that this practice is pretty common,
> especially considering how expensive it is to make a repository-wide tag
> in a big CVS repository.
>
> Just a not of caution: even in the case that a "before" and "after" tag
> pair can each be successfully expanded across the repository does not
> ensure that diff(before -> after) == diff(before_expanded ->
> after_expanded) because there could be differences in other files.
> Therefore tag expansion could result in permanently losing information
> about exactly what modifications were considered part of the bugfix.  I
> don't see a way to get around this problem short of teaching cvs2svn to
> treat tags in pairs.

I noticed this happening in our own tests - and it is a serious
problem for us since we reallyl want to preserve the "diff" property
of before/after tags.

Hmm - would it be possible with reasonable effort to detect if a tag
has a unique expansion?  For any given tag there is in general a set
of commits which could correspond to this tag.  If the tag covers all
files in the repository the set should only have one element (since
any commit would change *something*).  If the tag only covers a few
files there is a risk that multiple commits could match the tag - and
that is the situation where expansion causes the tag to become
ambiguous.  So AFAICT it should be safe to expand the tag if there is
exactly one commit matching the tag?

> - Does your patch also allow branches to be expanded, or only tags?  Is
> there a reason for the decision?

I only thought about tags when I made the change.  Thinking about it
now I guess it would be quite dangerous to expand branches also?
We've always made branches cover the entire repository, so it is no
big issue for us.  But if a branch tag is expanded, how will we then
handle the commits on the branch?  Which revision should non-branch
files have?

Hmm - in some ways expanding a branch corresponds to expanding an
entire series of before/after tags.  Either we have to somehow ensure
that later commits on the branch all use the same revision - or we
accept that the converted branch reports a bunch of changes happening
on other files.

I may have missed something, but for now I think that expanding
branches is way too dangerous.

> - I think that we should avoid using "lambdas" in the configuration
> file.  Our users often seem overwhelmed by having to configure their
> conversion using Python syntax, and lambdas are less obvious than most
> constructs.

Fine with me - I know Python well and have some background with
functional languages, so it came naturally to me - but the
configuration should of course be kept as simple as possible ... :-)

> - Why do you only list the file basenames in the "not a proper subset"
> message?  It could be ambiguous...

The only reason was to reduce the amount of information.  So I guess
it ties in with the other mail about what information should be in the
commit messages and what should be stored elsewhere.

Best regards,
  Anders Pilegaard

------------------------------------------------------
http://cvs2svn.tigris.org/ds/viewMessage.do?dsForumId=1667&dsMessageId=2699572

To unsubscribe from this discussion, e-mail: [[email protected]].