You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Rall <dl...@collab.net> on 2006/04/06 20:08:02 UTC

Behavior of the new 'svnmerge.py integrated' command

On Wed, 05 Apr 2006, Giovanni Bajo wrote:

> Blair Zajac <bl...@orcaware.com> wrote:
> 
> > $ /usr/bin/time ~/bin/svnmerge -s -v -v integrated
> > svn propget --strict "svnmerge-integrated" "."
> > svnmerge: calculate head path for the branch
> > svn info "."
> > svn info "http://svn/svn/repos/spijava/branches/blair"
> > svnmerge: head is "http://svn/svn/repos/spijava/trunk"
> > svn propget --strict "svnmerge-integrated" "."
> > svn log -r 1:HEAD --limit=1 -q .
> > svnmerge: available revisions to be merged are:
> > 40413-106349
> 
> I'm confused as to why "svnmerge integrated" should ever run "svn log" (or even
> run any remote operation at all).

I don't think the code that's there ATM is quite right, because it's
looking for the oldest branch revision instead of the oldest source
(head) revision.  However, the 'svn log' may still be needed for
correctness and/or consistency with 'svnmerge.py avail', because the
"svnmerge-integrated" property records more revisions in its revision
ranges than have actually been merged into a branch...

For example, consider a vanilla test case
(post-TestCase_TestRepo.setUp()) in svnmerge_test.py:

A propget of "svnmerge-integrated" on "test-branch/" has a value of
"trunk:/1-6" before any changes have merged from trunk into the branch
(but post-'init').  Considering that trunk wasn't even born until r3,
why should the 'integrated' command claim that revisions 1-6 had been
merged when in fact only 3-6 -- the birth of trunk through the
revision the branch was copied from trunk -- have actually been
(implicitly) "merged" into the branch?


Perhaps the storage format of the revision ranged used in the
"svnmerge-integrated" property is a simplifying assumption used by the
implementation 'svnmerge.py avail' to calculate the set of revisions
available (in this case, 9-10)?  During some initial testing of the
patch for the in-the-works 'rollback' command, David James and I found
that we did want to ignore this set of "too old" changes during the
rollback process.  However, perhaps this is not appropriate behavior
for 'integrated'...thoughts?
-- 

Daniel Rall

Re: [Svnmerge] Behavior of the new 'svnmerge.py integrated' command

Posted by Daniel Rall <dl...@collab.net>.
On Thu, 06 Apr 2006, Daniel Rall wrote:

> On Wed, 05 Apr 2006, Giovanni Bajo wrote:
...
> > I'm confused as to why "svnmerge integrated" should ever run "svn log" (or even
> > run any remote operation at all).
> 
> I don't think the code that's there ATM is quite right, because it's
> looking for the oldest branch revision instead of the oldest source
> (head) revision.

Back from a brief vacation, I had time to glance over this today with
David James.  This was indeed a bug in 'integrated', fixed in r19337
of Subversion's trunk.

> However, the 'svn log' may still be needed for
> correctness and/or consistency with 'svnmerge.py avail', because the
> "svnmerge-integrated" property records more revisions in its revision
> ranges than have actually been merged into a branch...
> 
> For example, consider a vanilla test case
> (post-TestCase_TestRepo.setUp()) in svnmerge_test.py:
> 
> A propget of "svnmerge-integrated" on "test-branch/" has a value of
> "trunk:/1-6" before any changes have merged from trunk into the branch
> (but post-'init').  Considering that trunk wasn't even born until r3,
> why should the 'integrated' command claim that revisions 1-6 had been
> merged when in fact only 3-6 -- the birth of trunk through the
> revision the branch was copied from trunk -- have actually been
> (implicitly) "merged" into the branch?
> 
> 
> Perhaps the storage format of the revision ranged used in the
> "svnmerge-integrated" property is a simplifying assumption used by the
> implementation 'svnmerge.py avail' to calculate the set of revisions
> available (in this case, 9-10)?  During some initial testing of the
> patch for the in-the-works 'rollback' command, David James and I found
> that we did want to ignore this set of "too old" changes during the
> rollback process.  However, perhaps this is not appropriate behavior
> for 'integrated'...thoughts?

I've seen no comments on this, so I left the implementation as-is,
*not* claiming that revisions which predate the source URL have been
integrated into the destination WC.

While passing the output of 'integrated' to 'svnmerge.py rollback' or
'svn' probably would work fine, I dislike the tool claiming that it's
merged revisions which it couldn't possibly have merged.  The cost of
one 'svn log' operation seems like a reasonable trade-off here.


> Dan wrote:
> 
> > Blair wrote:
...
> > 2) It took a lot of CPU time and clock time (over 3 minutes).  After it 
> > runs the
> > 
> > svn log -r 1:HEAD --limit=1 -q .
> > 
> > there's no visible indication of what the script is doing.  I assumed that 
> > it was stuck in the response from the svn log, but it turns out that that 
> > returns in a couple of second, so that's not the cause.
> > 
> > It would be good to have an additional message stating what its doing then.
> 
> I suspect that the extra time may be due to subtracting the
> revisions which predate the branch (svnmerge.py at 19195, line
> 1011), ...

This turned out to be the cause of the slowness, which was caused by
passing a list to RevisionSet.__sub__().  Changing this to pass a
revision set -- also in r19337 -- improved performance by operating on
a dict instead of a list.
-- 

Daniel Rall