[Svnmerge] svnmerge and externals

[William Baum]

On Sat, Jan 7, 2012 at 9:20 PM, Dustin J. Mitchell <dustin at v.igoro.us> wrote:
> First, svnmerge is barely maintained anymore, since its functionality
> has largely been superseded since Subversion 1.5, so I can't guarantee
> patches will get merged -- I certainly don't feel qualified to give a
> thumbs-up here.

I realize practically nobody uses svnmerge anymore, but it does
provide some functionality that is not at all easy to replace.  In my
case, I have a number of processes built around svnmerge that do not
work with subversion 1.7 and the new working copy format.  These
issues prevented us from moving to subversion 1.7 until the
compatibility issues with svnmerge were addressed.

>> The first section deals with externals breaking check_dir_clean in a
>> simpler manner than Lars' change to XML.  I also added a report of the
>> detail of offending files to the verbose output.
>
> This looks OK, although I've lost the context for "Lars' change".  In
> general, I think that parsing XML output from svn commands is more
> stable and backward-compatible -- important for a script that probably
> sees much more use in version-constrained subversion environments.

Lars' proposed change:
http://old.nabble.com/forum/Permalink.jtp?root=32795044&post=32811091&page=y
(All the orcaware.com links seem to be broken at the moment..)

I did attempt to implement Lars' check_dir_clean function and was
unsuccessful.  It seems to be more of a suggested proof of concept
than working code, as it doesn't work with the existing launchsvn
function, and Lars' launchsvn stub is not compatible with the other
uses of launchsvn which expect a list rather than a string.

As I began to convert the output from launchsvn from a list into a
string, it occurred to me that this was getting carried away.  The
existing code is clean, simple, and has worked well for a long time.
The problem that needed to be solved was simply that svn 1.7 included
externals in the output of "status -q" where svn 1.6 and prior did
not.  So if one merely stripped the lines beginning with 'X'
(externals) from the output, the behavior would be compatible with
prior versions.  Discrete, simple, testable, done..

>> The second section forces the local working copy check to return true.
>>  One could implement a different check if there were a compelling
>> reason, but the risk of working on a non-working directory seems
>> minimal.
>
> I think this needs to be justified -- why remove the check?  And a
> patch to remove it should actually remove it, not just hack it (and,
> worse, comment out code -- commented out code should never be checked
> into version control).

The check needs to be removed because it doesn't work, and I'm not
aware of any suitable replacement mechanism.  With the new working
copy format, there is no way to determine from the local directory
structure itself whether a directory is part of a working copy or not.
 With the old working copy format, it made sense to check for the .svn
directory before attempting to execute svn status, or propget, or
whatever, because the check was extremely efficient and inexpensive.
With the 1.7 format, one would have to go looking for an .svn
directory in every parent directory or execute svn itself to
interrogate the working copy status of a directory.  Executing svn to
see if it's safe to execute svn seems less than productive.

svnmerge is just a wrapper for the svn executable itself. It doesn't
actually do anything to the local file structure or repository except
through svn commands, and all of the commands it passes to svn (log,
status, propget, proplist, propdel, info, merge) require a working
copy, and fail gracefully if the directory is not a working copy.

In my testing, the only difference between checking and not checking
the status of the local directory is whether you get an error message
from svnmerge or from svn.  I have not come up with a scenario in
which executing svnmerge in a non-working copy directory will actually
do anything.

> And a
> patch to remove it should actually remove it, not just hack it (and,
> worse, comment out code -- commented out code should never be checked
> into version control).

Fair enough.  And yes, it's a hack. Removing the other references to
is_wc() is a more significant change that's harder to undo if I turn
out to be wrong about the need for checking the working copy status or
somebody comes up with an elegant means to do so.. I left the old code
commented out so it was obvious what it used to do to someone who
wanted to restore the old functionality for svn 1.6.  My apologies --
old habits die hard.

svnmerge as it exists in the trunk is incompatible with the current
subversion.  I was able to restore functionality with a couple of
minor tweaks that may be useful to others interested in using svnmerge
with subversion 1.7.

--Bill