Re: [PATCH v1] Remove no-op revisions

Jon Foster wrote:
> Oswald Buddenhagen wrote:
>> i don't remember whether you already said that ... why else
>> would somebody have used commit -f and why would you want
>> to get rid of these commits?
> 
> Having looked at the troublesome files some more (and having
> learned about another way to abuse CVS), I think they were
> actually created with "cvs import" with a branch number of 1.1.2
> specified explicitly.  (Normally the vendor branch is 1.1.1).

Is there a way to specify the branch number that CVS should use for an
import?  Maybe this an be done by choosing an existing branch name then
using "cvs import -b BRANCH ..."?

1.1.2 is usually a branch number that would be used for a normal
(non-vendor) branch that sprouts from revision 1.1.  What is the
evidence that it came from an import?  I guess there is:

* The incriminating 'Initial revision\n' log message on revision 1.1
(though that doesn't indicate *which* branch was the vendor branch)

* The empty delta relative to 1.1 (though this could come from a "commit
-f")

* Are the timestamps on 1.1 and 1.1.2.1 the same?

Could it be that there was earlier a vendor branch 1.1.1, but it was
later "obsoleted"?

All in all, this is very mysterious and quite specialized, so we should
be careful about what we build into cvs2svn to handle it...

> They've then had further commits, so the fact that 1.1.2 was a
> vendor branch has been lost.
> 
> The branch layout looks like this:
> 
>   1.1 -------------- 1.2 --- TRUNK (1)
>     \
>      \--- 1.1.2.1 ---------- FOO-BR (1.1.2) (old vendor br)
>              \
>               \------------- IMPORTANT-BR (1.1.2.1.1)
> 
> (Lets see if ASCII art survives the corporate mail system...)
> 
> The branch name FOO-BR is used elsewhere as a real branch.

This suggests that FOO-BR started its life as a non-vendor branch...

> And for non-technical reasons too boring to go into here,
> I don't want to convert FOO-BR from CVS to SVN.  I do need
> to convert IMPORTANT-BR.  But cvs2svn sees that IMPORTANT-BR
> depends on FOO-BR, so I have to convert both or neither.
> 
> When CVS does an import, it creates rev 1.1 with the file contents
> and an "Initial import" log message, then it creates the branch
> revision with the real log message and an empty diff.  My patch
> detects that 1.1.2.1 has an empty diff, and correctly fixes the
> branch layout:
> 
>      /---------------------- IMPORTANT-BR (1.1.2.1.1)
>     /
>   1.1 -------- 1.2 --------- TRUNK (1)
>     \
>      \---------------------- FOO-BR (1.1.2)
> 
> IMPORTANT-BR was actually created by branching trunk, so this
> matches what we expect.  And we can exclude FOO-BR and keep
> IMPORTANT-BR.
> 
> 
> So perhaps a better approach would be to detect specifically
> the import situation?  I.e. if the log message of rev 1.1 is
> "Initial Import", rev 1.2 (or later trunk rev) exists, and
> there's a rev 1.1.X.1 which has an empty diff from rev 1.1.
> In this case, perhaps we can guess that 1.1.X was a vendor
> branch with a non-standard branch number.  If we did that,
> then cvs2svn's normal vendor branch handling would give us:
> 
>      /---------------------- IMPORTANT-BR (1.1.2.1.1)
>     /
>   1.1.2.1(ntdb) ------------ FOO-BR (1.1.2) (vendor br)
>     \
>      \-------- 1.2 --------- TRUNK (1)
> 
> and thanks to the magic of vendor branches, I can exclude
> the FOO-BR vendor branch whilst keeping IMPORTANT-BR.
> 
> Would this be preferable?

I wouldn't want standard cvs2svn with the default configuration to
consider anything but the 1.1.1 branch to be a historical NTDB, because
the evidence remaining in CVS for such an interpretation is too
unreliable.  But for your particular purpose, you could certainly
consider patching the version cvs2svn that you use for your conversion.
 One obvious place that would need patching is the line

        vendor_branch_data = self.sdc.branches_data.get('1.1.1')

in _FileDataCollector._process_ntdbrs().

However, I am skeptical whether this would achieve your desired result,
since you say that FOO-BR is used as a normal branch in some other
files.  In that case, I think cvs2svn would refuse to treat it as a
vendor branch that can be deleted.  But maybe your vendor branch
exclusion code is not so squeamish?


As for generically allowing revisions with empty deltatexts to be
omitted, I can imagine that there are some unusual situations that might
benefit from such an option.  But ossi's reminder that such revisions
have separate log messages (and indeed might have been created *because*
of the log message) means that the feature would be somewhat dangerous.

The idea to concatenate the log messages is a good one, but would only
work if each and every file containing one of the revisions also
contains the other; otherwise, one would end up with two or three
separate commits with log messages like "msg1", "msg2", and "msg1 msg2"
that cannot be joined together.

All that being said, I can imagine accepting a patch to add an option to
omit zero-delta revisions, even though IMO the cost-benefit ratio is
pretty close to 1.


IMHO, the real problem that underlies this whole discussion is our
mishandling of NTDBRs when branches or tags sprout from them.  Ideally
(assuming that FOO-BR were recognized as a vendor branch), cvs2svn would
realize that your IMPORTANT-BR might have sprouted from FOO-BR *or*
trunk.  Then in most circumstances it should choose trunk as the
parent-branch and graft it to trunk, leaving FOO-BR with no blockers and
able to be deleted.  Fixing this problem would allow cvs2svn to create
much simpler branches, which is particularly beneficial for DVCSs.

Michael

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

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