[Svnmerge] [PATCH] preserving mods to renamed files/directories

lsuvkne at onemodel.org lsuvkne at onemodel.org
Mon Jul 9 20:57:58 PDT 2007


This is the patch I'm most interested in discussing, by far.

This relates to preserving file and  revision log comments changes
across a merge that includes renames.  This one is harder to break up
into smaller units, I think.

I have applied almost all of the suggested changes that Dustin kindly
provided in an earlier review; those not already discussed are,
below.

The heart of the changes (and the likely easiest place to start reading
this, IMO) is in action_merge(), where just before the script calls
this:
    svn_command("merge --force -r
..I added a call to check_for_changes_to_preserve_across_merge(). This
function, and the functions it depends on, determine from "merge
--dry-run" output whether there are any matching "add/delete" pairs of
statements (representing renames) about to be applied, and for each of
those renames, determines from logs what file edits (diffs) and comments
would be lost due to the merge's deleting that file and replacing it. It
saves the diffs to temporary patch files, accumulates the comments, and
returns  the information back to action_merge().  Later in
action_merge(), for each file where  edits were lost by the merge, it
applies the proper patch, and appends the otherwise "lost" comments to
the file svnmerge-commit-message.txt.

It's not necessarily pretty but seems to reduce the risk for our most
common scenarios.  There are some scenarios that I know it doesn't
support, best practices, and tips from our internal wiki that I can
share later if there's interest.

Daniel Rall suggested in an earlier message about this that it might be 
worth putting efforts into addressing svn bug #2685 
(http://subversion.tigris.org/issues/show_bug.cgi?id=2685), instead of
adding this new complexity to svnmerge. I am hoping to learn more about
the scope of that bug, and the  code involved, to see  if it's something
I can do myself in the time I can commit, but either way it seems like
our organization needs something in an earlier timeframe than that. Even
with 1.5.0 merge tracking coming out, this seems useful because merge
tracking might eliminate the need for much of what svnmerge.py used to
do, but the "rename problem" will remain. (See
http://svnbook.red-bean.com/nightly/en/svn.branchmerge.copychanges.html#svn.branchmerge.copychanges.bestprac.moves
, though there are many other links I can provide for more information
around that issue.) 

Here are replies to Dustin's earlier suggestions:

>> +        self.verify_strings_in_file("svnmerge?  commit?  message.txt", \
>> +                                    ["Comments from pre?  existing copy", \
>> +                                    "In r15 renamed", "r14", "modify 
> test1"])
> 
> It's a little visually unclear what the svnmerge commit message should
> look like ??   perhaps a loose regex over the whole message would work
> better?  Some extra comments here might help, too, giving context.

Done--with a detailed regex.
 
>> +    def testMergeDirectoryRenameOnlyOntoModifiedFile(self):
> ...
>> +        self.multilaunch("""
>> +            svn add 'dir1/dir2/test6'
>> +            svn ci ?  m 'adding directories and file test6'
>> +            svn cp dir1 ../test?  branch
>> +        """)
> 
> There are 13 commits in the base repository, so this is r14
> 
>> +        # Initialize svnmerge
>> +        #todo: (see cmts at same plc in 
> testMergeFileRenameOntoModifiedFile())
>> +        self.svnmerge2(["init", self.test_path + "/test?  branch", "?  r 1?  16"])
> 
> Yet you're initializing through r16?

Good point. I've tried to clarify this part by rearranging a little and
commenting a little, and in a couple of other places you pointed out.

>> +def log_to_file(msg, filename=None):
> 
> I don't see log_to_file() used anywhere..

Removed now.  I used it for some ad-hoc debugging but it didn't
make sense to leave it there.

>>  def target_to_url(dir):
>>      """Convert working copy path or repos URL to a repos URL."""
>> ?      if is_wc(dir):
>> +    # allow dir to be either a directory, a file in a directory, or a file 
> in 
>> +    # the current directory, respectively:
> 
> This is a change ??   typically a single file cannot be considered a
> working copy.  What's the justficiation for this?  There should be a
> unit test for the new functionality, too.

I wrote a test specifically for it now; good point.  I had changed it
because I needed what it said it did ("Convert working copy path or
repos URL to a repos URL"), rather than what it actually did.  I needed
to get the URL equivalent of a file in a working copy (as opposed to
only directories).  The previous method didn't make sense to me--if you
passed in a parameter that didn't pass the test "is_url(dir)", it simply
returned the same value back, i.e., not turning it into a URL as the
function name and comment suggest. The test
TestCase_TestRepo.testMergeFileRenameOntoModifiedFile also fails without
it.

>>      # Try using "svn info URL". This works only on SVN clients >= 1.2
>> ?      try:
>> ?          info = get_svninfo(url)
>> ?          return info["Repository Root"]
>> ?      except LaunchError:
>> ?          pass
>> +    #try:
>> +    info = get_svninfo(url)
>> +    return info["Repository Root"]
>> +    #except LaunchError:  
>> +        #pass
>> +    '''todo: I had a bug which was obscured by this original 'pass' line.  
>> +    Would it be better to raise the error?  The bug was caused when passing 
> 
>> +    'file:///tmp/__svnmerge_test/repo/branches/test?  branch/dir1/dir2' 
>> +    into target_to_repos_relative_path(), which works if we append "@16" 
>> +    (ultimately this data came from the function 
>> +    testMergeDirectoryAndFileRenamesOntoModifiedFile() in 
>> +    svnmerge_test.py) when it does 'self.svnmerge("merge ?  r 17")'), but 
>> +    it took a while to find the bug and its fix, because this routine 
>> +    happily returned (incorrect) data instead of throwing the error.  
>> +        If we do permanently this 'except LaunchError' line, then this is
>> +    the same logic as at the top of the method??  so some of it could
>> +    be removed.  Does that make sense or am I missing the 
>> +    original thinking behind it?
> 
> It's not quite the same, because the second version is operating on the
> URL, not the working copy ??   from the description, it sounds like that
> matters for svn clients in the 1.2 series (I don't have such a beastie
> to test with).
> 
> As for ignoring errors ??   it might be better to find a way to log the
> errors in verbose mode, but still ignore them.  The idea is that things
> will fail depending on the version of the client.

It looks to me like the logic is written to handle different svn
clients, but it doesn't look to me like the different svn clients
require this behavior. I don't like hiding the exception because it made
it harder for me to find a bug.  Also demonstrating that it probably
doesn't matter for the old svn clients, I built and tested this with svn
1.1.4 and 1.2.3, and 7 and 1 original tests fail with those svn versions
respectively (testing with unmodified svnmerge and test code from
trunk), directly due to svn being the older version.  So it looks like
svnmerge.py really doesn't support those svn versions anyway.  On the
other hand, if we do support those older versions, that makes me wonder
if we should test with them each time, and fix whatever was going on in
the past.  My versions of svnmerge.py and the tests get 26 and 8 errors,
respectively, with those older svn versions, but given all factors, it
doesn't seem worth fixing unless we fix everything for those old
versions.

I guess the short version is: it seems to add no harm, and does some 
good.  :)

In case it's of interest, the errors running tests with svn version
1.2.3 included this:
ERROR: Make sure you can intermix command name, arguments and
----------------------------------------------------------------------
Traceback (most recent call last):
  File "./svnmerge_test.py", line 280, in testOptionOrder
    match=r"no integration info")  # accepted
  File "./svnmerge_test.py", line 180, in svnmerge
    return self.svnmerge2(cmds.split(), *args, **kwargs)
  File "./svnmerge_test.py", line 191, in svnmerge2
    ret = svnmerge.main(args)
  File "/home/callla/dnload/subversion/trunk/contrib/client-side/svnmerge/svnmerge.py", line 1895, in main
    branch_props = get_merge_props(branch_dir)
  File "/home/callla/dnload/subversion/trunk/contrib/client-side/svnmerge/svnmerge.py", line 619, in get_merge_props
    return get_revlist_prop(dir, opts["prop"])
  File "/home/callla/dnload/subversion/trunk/contrib/client-side/svnmerge/svnmerge.py", line 613, in get_revlist_prop
    out = launchsvn('propget %s' % args, split_lines=False)
  File "/home/callla/dnload/subversion/trunk/contrib/client-side/svnmerge/svnmerge.py", line 250, in launchsvn
    return launch(cmd, **kwargs)
  File "/home/callla/dnload/subversion/trunk/contrib/client-side/svnmerge/svnmerge.py", line 231, in launch
    raise LaunchError(ret, cmd, out)
LaunchError: (1, 'svn propget --strict "svnmerge-integrated" "."', "svn: This client is too old to work with working copy '.'; please get a newer Subversion client\n")


The errors with svn version 1.1.4 included that previous error, plus
this one and others (like 1.2.3, done with the original
svnmerge.py/svnmerge_test.py):
FAIL: Init svnmerge, block, modify head, merge, rollback.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "./svnmerge_test.py", line 1013, in testBlockMergeAndRollback
    error = False)
  File "./svnmerge_test.py", line 454, in launch
    return TestCase_SvnMerge.launch(self, cmd, *args, **kwargs)
  File "./svnmerge_test.py", line 241, in launch
    return self._parseoutput(ret, out, **kwargs)
  File "./svnmerge_test.py", line 225, in _parseoutput
    "svnmerge failed, with this output:\n%s" % out)
AssertionError: svnmerge failed, with this output:
svn: Path '..' ends in '..', which is unsupported for this operation


>> ?  def get_copyfrom(dir):
>> +def get_copyfrom(dir, liberal_comparison=False):
>>      """Get copyfrom info for a given target (it represents the directory 
> from
>>      where it was branched). NOTE: repos root has no copyfrom info. In this 
> case
>>      None is returned."""
> 
> Is liberal comparison required for this patch, or is it a related but
> functionally separate change?  Either way, it needs to be described in
> the docstring and tested for.  Similarly, the new return signature of
> this function should be described in the docstring.

The liberal comparison is required for my changes, and is (implicitly)
tested, as the tests I added fail without it.  I put in the optional
parameter to make sure not to change prior behavior.  I've also now
added explicit tests: for the old and new behavior, both embedded in an
existing test so as not to take the performance hit of running
setUp/tearDown again. 
 
>> +            except AttributeError:
>> +                if not liberal_comparison:
>> +                    raise AttributeError # todo: how re?  raise same one as we 
> caught?
> 
> use a bare 'raise'

Done; thanks.

> 
>> +def get_commit_logs_for_rev_range(start, end):
>> +    logOutput = launchsvn("log ?  v ?  r %d:%d %s" % \
>> +                          (start, end, opts["source?  url"]))
>> +    return logOutput
>> +
> 
> It would probably be better to roll this into get_commit_log(), with a
> different signature (perhaps passing a tuple as "revision" can trigger
> this behavior?)

I made them separate because they have completely different parameters
and internals.   I have made them adjacent now, so that in seeing one,
one sees the other for possible use.  Let me know if I'm just missing
the point.

>> +def get_adds_and_deletes_and_modifications_from_log(logText):
>> +    """ Using the add/delete statements in the log for what is being 
> merged, 
>> +    compare them and see which ones can be matched thus are "renames"
>> +    For more rationale, see comments in 
>> +    check_for_changes_to_preserve_across_merge(start, end).
>> +    """
>> +    linePattern = re.compile("^\?\?\?  ")
> 
> '?  ' isn't a special character in a regex, so no need to backslashify it.
> You may want to use the string * int trick to get the full width:
>     linePattern = re.compile("^" + '?  ' * 72 + '$')

Thanks. I used it to check for 7 hyphens. Is the count of 72 contractual
in some sense? If it later changed to 70 or something, I'd hate to be
dependent on an exact number. (I guess it may be almost as likely that
other aspects of the text could change, oh well.)
 
>> +    revisionNumberPattern = re.compile("^r(\d+)")
>> +    addFromPattern = re.compile("\s*A (.+) \(from (.+)\d+.+$")
>> +    deletedPattern = re.compile("\s*D (.+)$")
> 
> The 're' module isn't seeing your backslashes ??   prefix the string with
> 'r' to prevent Python from interpreting the backslashes:
>     deletedPattern = re.compile(r"\s*D (.+)$")
> 
>> +                        # on windows, python's regexp didn't allow(?) 
> removal 
>> +                        # of the version # (":17") off the end with the 
> regular 
>> +                        # expression, so do it here.
>> +                        addedFromPath = 
> addedFromPath[:addedFromPath.find(':')]  
> 
> This should be relatively straightforward with 're' ??   maybe it's a
> backslash problem?

Could be.  Would it be all right to have made that a "todo" item in the
code? Here's the text of it:
     # Todo: see if it would be
     # fixed by prefixing regex strings with 'r', such as:
     #     addFromPattern = re.compile(r"   A (.+) \(from (.+)\:\d+\)$")
     # ...and then we could eliminate this condition.

 
>> +def 
> get_revision_if_earlier_copied_from_merge_source(fileBeingDeletedWorkingCopyN
> ame):
>> +    """This file may have had changes merged into it from the merge source
>> +    previously. If so, find out in which revision that happened, so that
>> +    we don't try to get local 
>> +    history from before that point??  it isn't there, it's on the merge
>> +    source, and it isn't the part that is being lost.
>> +    """
> 
> So let's say that foo.c had this revision history
>  r300 ?   local modification 3
>  r280 ?   local modification 2
>  r270 ?   merge of r255 from source
>  r260 ?   local modification 1
>  r250 ?   merge of r245 from source
>  ...
> This function would find revision 270, which means that it would miss
> the log messages (and diffs?) from r250.

Good question. This function is called by
get_diff_file_name_and_append_to_deletedfiles_comments(), which itself
gets the log messages and diffs from earlier versions, and it calls this
function to find out which ones it does NOT want to check for diffs and
log messages.  In this example, assuming that all the changes are coming
from the same source, and that where the example says "merge from
source" it actually means "file copied from source because of a merge
containing a rename (aka copy + delete)", then if it picked up the log
messages and diffs from r250 or r270, it would be putting in extra
comments that are redundant because they duplicate what is already on
the same merge source that is again about to be copied in.  On the other
hand, we *do* want to pick up the changes that happened since r270, and
get_diff_file_name_and_append_to_deletedfiles_comments() will do that.

Maybe you can help me say it more consisely and clearly in the comment,
or maybe I'm not understanding something.


>> +def get_nearest_commonancestor_revision(fileBeingDeletedWorkingCopyName,\
>> +                                        fileAdded, 
> revisionNumberOfThisRename):
>> +    # Find the common ancestor of the two files (one to be deleted and
>> +    # one to be added, as the rename does), to see what changed on the
>> +    # line that is about to be deleted (since they diverged).
>> +    
>> +    # We can't (at least not here) use logic like what is at the start
>> +    # of the function action_init(), because it wouldn't apply in a case
>> +    # where the branch we're working with was not made by copying directly 
>> +    # from the 
>> +    # current line.  I.e., they both could have been made by copying from
>> +    # some other line. We do try to use it in cases when it would work, 
> before
>> +    # this current function is called, however.
>> +    
>> +    # todo: this algorithm IS A KLUDGE (i.e., for determining what revision 
> 
>> +    # range we should use on the 
>> +    # file to be deleted to create the diff, i.e., what changed
>> +    # on this branch in this file, since it and the renamed
>> +    # file diverged).  It assumes that any changes on the two branches were 
> never
>> +    # checked in during the same commit??  in this author's environment that 
>> +    # seems like
>> +    # a safe assumption, and overall much better than what we had
>> +    # before (the destructive renames).  The algorithm assumes that when we 
> 
>> +    # find a 
>> +    # revision where they were modified on the same commit, they must
>> +    # have been the same file at that point in history, therefore we can 
> use 
>> +    # that revision # as a starting point.
>> +    # The assumption is bad because someone could conceivably have a 
> working copy
>> +    # containing both branches, and cd to a directory that contains
>> +    # them both and check in changes to both using a single commit
>> +    # command).
>
> There was some discussion on the list recently about changes resulting
> from conflict?  resolution in a merge; that is, r270 above may not be
> precise copy of r255.  Will that cause a problem here, too?

I am not seeing the connection....  This method doesn't compare the
contents anyway, in case that helps.

 
>> +def write_patch_file(diffText, fileAddedStringToEmbed):
>> +    fileBeingAddedWorkingCopyNameWithReplacedSlashes = \
>> +                                    fileAddedStringToEmbed.replace('/','?  ')
> will this work on windows?

Good question. I don't know if the parameter value at that point has
slashes or backslashes, but the tests require patch files to be written
and read, and they all pass on windows and linux.  I'm sure enough that
they execute this line, because they change a file nested in a
subdirectory which should do it.

Thanks again for all your reviews so far.  It's good and we're making 
progress.

-Luke


(Log comment coming on the next round of email for this....)
-------------- next part --------------
--- /home/callla/dnload/subversion/trunk/contrib/client-side/svnmerge/svnmerge.py	2007-07-09 16:13:15.000000000 -0600
+++ svnmerge.py	2007-07-09 11:57:47.000000000 -0600
@@ -300,7 +329,7 @@
     else:
         password = ""
     cmd = opts["svn"] + username + password + " " + s
-    if show or opts["verbose"] >= 2:
+    if show or (opts.get("verbose") and opts["verbose"] >= 2):
         print cmd
     if pretend:
         return None
@@ -761,15 +790,27 @@
             continue
         key, value = L.split(": ", 1)
         info[key] = value.strip()
-    _cache_svninfo[path] = info
+    # Don't cache ".", because its meaning changes depending on 
+    # what directory we are in.  Exposed by test testTargetToUrl(),
+    # but only when it is run with all tests.
+    if not path == '.':
+        _cache_svninfo[path] = info
     return info
 
 def target_to_url(dir):
     """Convert working copy path or repos URL to a repos URL."""
-    if is_wc(dir):
+    # allow dir to be either a directory, a file in a directory, or a file in 
+    # the current directory, respectively:
+    
+    if is_url(dir):
+        return dir
+    if is_wc(dir) or (os.path.isfile(dir) and is_wc(os.path.dirname(dir))) \
+        or (os.path.isfile(dir) and len(os.path.dirname(dir) == 0) \
+        and is_wc(".")):
+        
         info = get_svninfo(dir)
         return info["URL"]
-    return dir
+    raise ValueError, dir + ' is not a working copy'
 
 def get_repo_root(dir):
     """Compute the root repos URL given a working-copy path, or a URL."""
@@ -786,11 +827,8 @@
         url = dir
 
     # Try using "svn info URL". This works only on SVN clients >= 1.2
-    try:
-        info = get_svninfo(url)
-        return info["Repository Root"]
-    except LaunchError:
-        pass
+    info = get_svninfo(url)
+    return info["Repository Root"]
 
     # Constrained to older svn clients, we are stuck with this ugly
     # trial-and-error implementation. It could be made faster with a
@@ -820,7 +858,7 @@
     # (Not sure why the
     # non-greedy quantifiers in a regexp "logEntryContainingFoundAddActionPattern"
     # didn't seem to work for  this--i.e., putting in '.*?' just before and 
-    # after the mention of "actionAddPathPattern" in the section of
+    # after the mention of "actionAddPatternWithPath" in the section of
     # get_copyfrom() right after this is called, caused it to get the 
     # whole regex which contains many log entries--not just the single 
     # log entry I want. (todo: use an xml parser instead? no matter for now)
@@ -843,10 +881,14 @@
                          + logText + "'"
     return logText[start:end]
 
-def get_copyfrom(dir):
+def get_copyfrom(dir, liberal_comparison=False):
     """Get copyfrom info for a given target (it represents the directory from
     where it was branched). NOTE: repos root has no copyfrom info. In this case
-    None is returned.  
+    None is returned.  If liberal_comparison is False (default), and there is
+    not an exact match with the "dir" parameter, it will return None values, 
+    whereas if liberal_comparison is True it will return values for the
+    deepest matching parent directory (because the "dir" parameter
+    was in fact copied, by being contained in the copied directory above it).
     
     Returns the:
         - source file or directory from which the copy was made
@@ -858,18 +900,35 @@
     logText = launchsvn('log -v --xml --stop-on-copy "%s"' % dir,
                     split_lines=False)
     logText = logText.replace("\n", " ")
-
+    
     # first get the source; abort if unavailable (not a copy)
     try:
-        actionAddPatternWithPath = actionAddPattern % re.escape(repos_path)
-        addPathPatternMatch = re.search(actionAddPatternWithPath, logText)
-        source = re.search(r'copyfrom-path="([^"]*)"', \
-                           addPathPatternMatch.group(1)).group(1)
+        while True: # loop end logic is internal
+            try:
+                actionAddPatternWithPath = actionAddPattern % re.escape(repos_path)
+                addPathPatternMatch = re.search(actionAddPatternWithPath, logText)
+                source = re.search(r'copyfrom-path="([^"]*)"', \
+                                   addPathPatternMatch.group(1)).group(1)
+                break
+            except AttributeError:
+                if not liberal_comparison:
+                    raise
+                else:
+                    ''' If repos_path wasn't found as being copied, maybe a 
+                    subset of it was (which would have the same effect--the 
+                    value in 'dir' came from copying another directory, though
+                    it was a containing one, not this exact-named one):'''
+                    lastSlashPosition = repos_path.rfind('/')
+                    if lastSlashPosition == -1:
+                        '''' break from the loop--we really did fail as the 
+                        originally raised AttributeError suggested '''
+                        raise
+                    repos_path = repos_path[:lastSlashPosition]
+                    continue
     except AttributeError:
         return None,None,None
     
-    # get the revision from which source was copied (if the above re.search
-    # succeeded, yet this throws an exception, we probably want to know).
+    # get the revision from which source was copied
     rev = re.search(r'copyfrom-rev="([^"]*)"', 
         addPathPatternMatch.group(1)).group(1)
     
@@ -924,9 +983,14 @@
 def get_commit_log(url, revnum):
     """Return the log message for a specific integer revision
     number."""
-    out = launchsvn("log --incremental -r%d %s" % (revnum, url))
+    out = launchsvn("log -v --incremental -r%d %s" % (revnum, url))
     return "".join(out[1:])
 
+def get_commit_logs_for_rev_range(start, end):
+    logOutput = launchsvn("log -v -r %d:%d %s" % \
+                          (start, end, opts["source-url"]))
+    return logOutput
+
 def construct_merged_log_message(url, revnums):
     """Return a commit log message containing all the commit messages
     in the specified revisions at the given URL.  The separator used
@@ -1098,6 +1162,338 @@
     ret.reverse()
     return ret
 
+def get_adds_and_deletes_and_modifications_from_log(logText):
+    """ Using the add/delete statements in the log for what is being merged, 
+    compare them and see which ones can be matched thus are "renames"
+    For more rationale, see comments in 
+    check_for_changes_to_preserve_across_merge(start, end).
+    """
+    linePattern = re.compile("^" + '-' * 7)
+    revisionNumberPattern = re.compile("^r(\d+)")
+    addFromPattern = re.compile("\s*A (.+) \(from (.+)\d+.+$")
+    deletedPattern = re.compile("\s*D (.+)$")
+    modifiedNodes = []
+    ''' The key of "nodesAddedAndFromWhere" is the path+name of the file 
+    added; the value is a tuple where [0] is the path+name file from which 
+    it was copied (created), and [1] is the revision # in which this was done.
+    '''
+    nodesAddedAndFromWhere = {}
+    # The key of "nodesToBeDeleted" is the path+name of a file that is going to be
+    # deleted by the rename (=add+del); the value is whether it had changes that
+    # should be preserved across the rename.
+    nodesToBeDeleted = {}
+    
+    # find all the nodes (files or directories) that were added/deleted by rename.
+    i=0
+    while i < len(logText):
+        if linePattern.match(logText[i]):
+            # iterate across the log output for each revision--delimited by 
+            # lines of hyphens ('---'...)
+            i = i + 1
+            
+            while (i < len(logText)) and not (linePattern.match(logText[i])):
+                matchObject = revisionNumberPattern.match(logText[i])
+                if matchObject != None:
+                    revisionNumber = matchObject.group(1)
+                
+                matchObject = addFromPattern.match(logText[i])
+                if matchObject != None:
+                    fileAdded = matchObject.group(1)
+                    addedFromPath = matchObject.group(2)
+                    if addedFromPath.find(':'):
+                        # on windows, python's regexp didn't allow(?) removal 
+                        # of the version # (":17") off the end with the regular 
+                        # expression, so do it here. 
+                        # Todo: see if it would be
+                        # fixed by prefixing regex strings with 'r', such as:
+                        # addFromPattern = re.compile(r"   A (.+) \(from (.+)\:\d+\)$")
+                        # ...and then we could eliminate this condition.
+                        addedFromPath = addedFromPath[:addedFromPath.find(':')]  
+                    nodesAddedAndFromWhere[fileAdded] = addedFromPath, \
+                                                        revisionNumber
+                
+                matchObject = deletedPattern.match(logText[i])
+                if matchObject != None:
+                    nodesToBeDeleted[matchObject.group(1).strip()] = 0
+                
+                i = i + 1
+    
+    return nodesAddedAndFromWhere, nodesToBeDeleted, modifiedNodes
+
+def get_revision_if_earlier_copied_from_merge_source(fileBeingDeletedWorkingCopyName):
+    """This file may have had changes merged into it from the merge source
+    previously. If so, find out in which revision that happened, so that
+    we don't try to get local 
+    history from before that point--it isn't there, it's on the merge
+    source, and it isn't the part that is being lost.
+    """
+    lastSlashPosition = fileBeingDeletedWorkingCopyName.rfind('/')
+    if lastSlashPosition == -1:
+        # it's a file in the current directory
+        nameToCheckIfWasCopied = fileBeingDeletedWorkingCopyName 
+    else:
+        # check just the directory, not the file under it
+        nameToCheckIfWasCopied = fileBeingDeletedWorkingCopyName[:lastSlashPosition]  
+
+    cf_source_path, cf_rev, copy_committed_in_rev = \
+        get_copyfrom(nameToCheckIfWasCopied, True)
+    
+    if cf_rev == None:  
+        # Didn't find a match--directory in question wasn't copied from the 
+        # merge source (or anywhere).  Todo: think through: what if it was
+        # copied from somewhere else?L
+        return None
+    else:
+        return copy_committed_in_rev
+
+def get_nearest_commonancestor_revision(fileBeingDeletedWorkingCopyName,\
+                                        fileAdded, revisionNumberOfThisRename):
+    # Find the common ancestor of the two files (one to be deleted and
+    # one to be added, as the rename does), to see what changed on the
+    # line that is about to be deleted (since they diverged).
+    
+    # We can't (at least not here) use logic like what is at the start
+    # of the function action_init(), because it wouldn't apply in a case
+    # where the branch we're working with was not made by copying directly 
+    # from the 
+    # current line.  I.e., they both could have been made by copying from
+    # some other line. We do try to use it in cases when it would work, before
+    # this current function is called, however.
+    
+    # todo: this algorithm IS A KLUDGE (i.e., for determining what revision 
+    # range we should use on the 
+    # file to be deleted to create the diff, i.e., what changed
+    # on this branch in this file, since it and the renamed
+    # file diverged).  It assumes that any changes on the two branches were never
+    # checked in during the same commit--in this author's environment that 
+    # seems like
+    # a safe assumption, and overall much better than what we had
+    # before (the destructive renames).  The algorithm assumes that when we 
+    # find a 
+    # revision where they were modified on the same commit, they must
+    # have been the same file at that point in history, therefore we can use 
+    # that revision # as a starting point.
+    # The assumption is bad because someone could conceivably have a working copy
+    # containing both branches, and cd to a directory that contains
+    # them both and check in changes to both using a single commit
+    # command).
+    #
+    # One better way might be to consider using the node revision id (aka 
+    # "entity id"??)
+    # as described here http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_fs_base/notes/structure?view=markup
+    # starting at "Within the database".  Maybe we could go back in
+    # history through the revisions for each file, and find the historical
+    # revision for which they have the same node id.  This however might mean 
+    # that
+    # this script would only work with an svn installation that was compiled 
+    # with python 
+    # bindings support.
+    # 
+    # Or maybe a simpler way would be to find the historical revision
+    # for which they are simply the same path, by parsing logs, which
+    # wouldn't require accessing the svn api from this python script
+    # which currently tends to just invoke the command-line.
+    # 
+    # BUT for now I'm taking the cheap route to prove the overall
+    # concept, and possibly get feedback, and then can reconsider
+    # doing this step in a better way.  Discussion is welcome.
+    
+    logsForFileBeingDeleted = str(launchsvn("log " \
+                                  + fileBeingDeletedWorkingCopyName))
+    fileBeingAddedFullUrl = get_repo_root(".") + fileAdded
+    logsForFileBeingAdded = str(launchsvn("log " + fileBeingAddedFullUrl + \
+                                "@" + revisionNumberOfThisRename))
+    
+    fileBeingDeletedRevisionNums = re.findall('\s?r(\d+) \| ', \
+                                              logsForFileBeingDeleted)
+    fileBeingAddedRevisionNums   = re.findall('\s?r(\d+) \| ', \
+                                              logsForFileBeingAdded)
+    fileBeingDeletedRevisionNums.sort()
+    fileBeingAddedRevisionNums.sort()
+    
+    # start with highest rev number and find the first one that they
+    # have in common (could be optimized...)
+    for revnum in reversed(fileBeingDeletedRevisionNums):  
+        if revnum in fileBeingAddedRevisionNums:
+            latestCommonAncestorRevision = revnum
+            break
+    else:
+        # Shouldn't happen.  (If we renamed
+        # something, the old/new files had to come from a common ancestry, 
+        # right?--and if they have no common ancestor w/ the file into
+        # which we are merging that rename, it doesn't make sense.) If
+        # someone really needed to for some reason they could use the
+        # 'svn merge' built-in command for it.
+        raise Exception, "Code reached an unexpected point that shouldn't "+\
+                          "ever happen; see comments in code for details."
+    
+    return latestCommonAncestorRevision
+
+def write_patch_file(diffText, fileAddedStringToEmbed):
+    fileBeingAddedWorkingCopyNameWithReplacedSlashes = \
+                                    fileAddedStringToEmbed.replace('/','-')
+    diffFileName = "svnmerge-preserveRenamedFileDataFor-" + \
+                fileBeingAddedWorkingCopyNameWithReplacedSlashes + ".diff"
+    lostInfoDiffFile = open(diffFileName,'w',0)
+    for line in diffText:
+        lostInfoDiffFile.write(line)
+    lostInfoDiffFile.close()
+    return diffFileName
+
+def get_diff_file_name_and_append_to_deletedfiles_comments(
+                                            fileBeingDeletedWorkingCopyName, \
+                                            fileAdded, addedFromPath, \
+                                            revisionNumberOfThisRename, \
+                                            allDeletedFilesComments):
+    # Having found a renamed file (or file contained in a renamed directory; 
+    # therefore old local copy is going to be deleted), see if it has changes 
+    # that should be preserved across the rename.
+    revisionContainingTheAddedFromPath = str(int(revisionNumberOfThisRename) - 1)
+    preserveChangesAfterRevision = get_revision_if_earlier_copied_from_merge_source(fileBeingDeletedWorkingCopyName)
+    if preserveChangesAfterRevision == None:
+        nearest_common_ancestor_rev = get_nearest_commonancestor_revision(
+                                        fileBeingDeletedWorkingCopyName, \
+                                        fileAdded, revisionNumberOfThisRename)
+        preserveChangesAfterRevision = str(int(nearest_common_ancestor_rev))
+    
+    # is the --notice-ancestry really relevant in this situation?
+    diffCmd = "diff --notice-ancestry --force -r " + \
+               preserveChangesAfterRevision + ":HEAD" + " " + \
+               fileBeingDeletedWorkingCopyName
+    diffOutput = launchsvn(diffCmd)
+    
+    diffFileName = None
+    fileBeingAddedWorkingCopyName = fileAdded[len(opts['source-path']) + 1: ]
+    
+    if (len(diffOutput) > 0):
+        diffFileName = write_patch_file(diffOutput, fileBeingAddedWorkingCopyName)
+        
+    # get comments to preserve even if no code changes to preserve.
+    # todo: could use a more efficient way to calc the exact revnums, 
+    # like skipping those phantom revisions or something?
+    revnums = {}
+    for i in range(int(preserveChangesAfterRevision), int(get_latest_rev(fileBeingDeletedWorkingCopyName)) + 1):
+        revnums[i] = None # value doesn't matter, we just need keys for RevisionSet creation.
+    deletedCommentsRevisionSet = RevisionSet(revnums)
+    
+    # get whatever comments will be lost, for appending to checkin comments 
+    # later.
+    deletedFilesComments = construct_merged_log_message(\
+                                            fileBeingDeletedWorkingCopyName, \
+                                            deletedCommentsRevisionSet)
+    
+    # avoid repeating same thing several times
+    commentsAlreadyFoundAtStringLocation = allDeletedFilesComments.find(deletedFilesComments)
+    if commentsAlreadyFoundAtStringLocation >= 0:
+        revisionDescrsPrecedingThisComment = re.findall("In r\d+ renamed ", \
+                                                    allDeletedFilesComments)
+        revisionDesc = revisionDescrsPrecedingThisComment[len(revisionDescrsPrecedingThisComment) - 1]
+        deletedFilesComments = "[comments same as above for '" + \
+                                revisionDesc[0:len(revisionDesc)-2] + "...']"
+    
+    allDeletedFilesComments = allDeletedFilesComments + \
+                           "In r" + revisionNumberOfThisRename + \
+                           " renamed " + addedFromPath + " to " + \
+                           fileAdded + \
+                           os.linesep + "  " + deletedFilesComments
+        
+    return diffFileName, fileBeingAddedWorkingCopyName, allDeletedFilesComments
+
+def check_for_changes_to_preserve_across_merge(start, end):
+    """Because as of this writing (svn 1.4.3 and prior), svn rename is implemented 
+    as add+delete, this check is needed to prevent loss of data in case the 
+    file to be deleted by this rename has been modified: (Until this bug: 
+       http://subversion.tigris.org/issues/show_bug.cgi?id=898 
+    ...is fixed.  See 
+       http://svnbook.red-bean.com/nightly/en/svn.branchmerge.copychanges.html#svn.branchmerge.copychanges.bestprac.moves
+    ...for one data loss scenario at least in svn 1.4, and the threads 
+    surrounding these messages here:
+       http://subversion.tigris.org/servlets/SearchList?list=users&searchText=yet+better+rename+approach&defaultField=body&Search=Search
+    ...for more scenarios.)
+    
+    The parameters "start" and "end" are revision numbers for "minimal merge 
+    intervals", determined from the revision numbers being merged in.
+    """
+    filesToPatchAndPatchNames = {}
+    allDeletedFilesComments = ""
+    
+    logText = get_commit_logs_for_rev_range(start, end)
+    nodesAddedAndFromWhere, nodesToBeDeleted, modifiedNodes = \
+                    get_adds_and_deletes_and_modifications_from_log(logText)
+    
+    # for the nodes added/deleted by rename, get any prior changes & comments 
+    # that we don't want to lose.
+    for nodeAdded in nodesAddedAndFromWhere.keys():
+        addedFromPath = nodesAddedAndFromWhere[nodeAdded][0]
+        revisionNumberOfThisRename = nodesAddedAndFromWhere[nodeAdded][1]
+        if nodesToBeDeleted.has_key(addedFromPath):
+            nodeBeingDeletedWorkingCopyName = addedFromPath[\
+                                            len(opts['source-path']) + 1: ]
+            
+            # if it's a directory that was renamed, we need to get the diff 
+            # and comments info for potentially any or all of the
+            # files in the merge target which could be overwritten by files 
+            # under renamed directories in the merge source, otherwise
+            # those local edits could also be lost.
+            if get_svninfo(nodeBeingDeletedWorkingCopyName)["Node Kind"] == \
+                                                            "directory":
+                nodesUnderRenamedMergeSourceDirectory = \
+                    launchsvn('list -R ' + get_repo_root('.') + nodeAdded + \
+                                            "@" + revisionNumberOfThisRename)
+                nodesUnderRenamedMergeTargetDirectory = \
+                    launchsvn('list -R ' + nodeBeingDeletedWorkingCopyName)
+                
+                # get only the files within the renamed dir (don't get 
+                # directories, indicated by trailing slashes), then w/ 
+                # those files get diffs
+                filesUnderRenamedMergeSourceDirectory = []
+                filesUnderRenamedMergeTargetDirectory = []
+                filesInCommon = []
+                for node in nodesUnderRenamedMergeSourceDirectory:
+                    node = node.strip()
+                    # windows svn 1.4.2 uses slashes here, but play it safe 
+                    # (since i thought on 2 occasions I saw it use backslashes,
+                    # this will work either way.)
+                    if not (node[-1] == '/' or node[-1] == os.sep): 
+                        filesUnderRenamedMergeSourceDirectory.append(node)
+                for node in nodesUnderRenamedMergeTargetDirectory:
+                    node = node.strip()
+                    # windows svn 1.4.2 uses slashes here, but play it safe
+                    # (since i thought on 2 occasions I saw it use backslashes,
+                    # this will work either way.)
+                    if not (node[-1] == '/' or node[-1] == os.sep): 
+                        filesUnderRenamedMergeTargetDirectory.append(node)
+                
+                for file in filesUnderRenamedMergeSourceDirectory:
+                    if file in filesUnderRenamedMergeTargetDirectory:
+                        filesInCommon.append(file)
+                for file in filesInCommon:
+                    # should the 1st slash in these parameters be os.sep instead? Would be good to clarify (and test/verify in each case) why we're using '/' sometimes and os.sep other times. Checked it enough to make the tests pass, but other than that it could still be verified in specific cases.  svn on windows sometimes(?) returns strings with a '\', other things seem to require a '/' on both platforms.
+                    diffFileName, fileBeingAddedWorkingCopyName, \
+                    allDeletedFilesComments = \
+                        get_diff_file_name_and_append_to_deletedfiles_comments(\
+                            nodeBeingDeletedWorkingCopyName + '/' + file, \
+                            nodeAdded + '/' + file, opts['source-path'] + \
+                            '/' + nodeBeingDeletedWorkingCopyName + '/' + \
+                            file, revisionNumberOfThisRename, \
+                            allDeletedFilesComments)  
+                    
+                    if diffFileName != None:
+                        filesToPatchAndPatchNames[fileBeingAddedWorkingCopyName] \
+                                                            = diffFileName
+            else:
+                diffFileName, fileBeingAddedWorkingCopyName, \
+                allDeletedFilesComments = \
+                    get_diff_file_name_and_append_to_deletedfiles_comments(\
+                        nodeBeingDeletedWorkingCopyName, nodeAdded, \
+                        addedFromPath, revisionNumberOfThisRename, \
+                        allDeletedFilesComments)
+                if diffFileName != None:
+                    filesToPatchAndPatchNames[fileBeingAddedWorkingCopyName] \
+                        = diffFileName
+                                                     
+    return filesToPatchAndPatchNames, allDeletedFilesComments
+
 def fix_line_terminators(filename):
     ''' Avoid the error on windows when the commit failed with:  "svn: 
     Inconsistent line ending style" (both w/ tests and manual runs).
@@ -1315,16 +1712,58 @@
     old_merge_props = branch_props
     old_block_props = get_block_props(branch_dir)
     merge_metadata = logs[opts["source-url"]].merge_metadata()
+    
+    allDeletedFilesComments = ""
     for start,end in minimal_merge_intervals(revs, phantom_revs):
         if not record_only:
             # Clear merge/blocked properties to avoid spurious property conflicts
             set_merge_props(branch_dir, {})
             set_block_props(branch_dir, {})
+
+            filesToPatchAndPatchNames, deletedFilesComments = \
+                check_for_changes_to_preserve_across_merge(start, end);
+            allDeletedFilesComments = allDeletedFilesComments + \
+                                                        deletedFilesComments
+            
             # Do the merge
             svn_command("merge --force -r %d:%d %s %s" % \
                         (start - 1, end, opts["source-url"], branch_dir))
             # TODO: to support graph merging, add logic to merge the property meta-data manually
 
+            for fileToPatch in filesToPatchAndPatchNames.keys():
+                patchcmd = "patch --force " + fileToPatch + " " + \
+                                    filesToPatchAndPatchNames[fileToPatch]
+                if not opts["dry-run"]:
+                    try:
+                        launch(patchcmd)
+                        os.remove(filesToPatchAndPatchNames[fileToPatch])
+                    except LaunchError, inst:
+                        returncode, cmd, stdoutAndErr = inst
+                        if stdoutAndErr.find("'patch' is not recognized as " + \
+                                    "an internal or external command") >= 0:
+                            raise LaunchError(returncode, cmd, \
+                                stdoutAndErr + "If you're on Windows, " + \
+                                "you need to install " + \
+                                "cygwin and make sure the 'patch' command is " + \
+                                "available in the path (or at a minimum, the " + \
+                                "two files patch.exe and cygwin1.dll, as of " + \
+                                "6/2007); then do 'svn revert -R .' and " + \
+                                "re-run svnmerge, or apply the patch " + \
+                                "file individually with:  \'" + \
+                                patchcmd + "\'")
+                        else:
+                            print "Patch command had errors:"
+                            print "        " + cmd + ":   " + str(returncode)\
+                                                        + ": " + stdoutAndErr
+                            print "You can probably resolve these manually " + \
+                                "prior to checkin, without needing to re-run " + \
+                                "svnmerge.py"
+                else:
+                    print "Patch files not auto-deleted, for observation \
+                            after --dry-run"
+                    print patchcmd
+            
+
     # Update the set of merged revisions.
     merged_revs = merged_revs | revs | reflected_revs | phantom_revs
     branch_props[opts["source-path"]] = str(merged_revs)
@@ -1345,6 +1784,11 @@
         if opts["commit-verbose"]:
             print >>f
             print >>f, construct_merged_log_message(opts["source-url"], revs),
+            if len(allDeletedFilesComments) > 0:
+                print >>f
+                print >>f, "Comments from pre-existing copy (if any) of " + \
+                            "renamed/deleted files-----------:"
+                print >>f, allDeletedFilesComments
 
         f.close()
         
@@ -2008,12 +2452,14 @@
     ]),
 }
 
-
-def main(args):
-    global opts
-
+def init_global_opts():
     # Initialize default options
+    global opts
     opts = default_opts.copy()
+
+def main(args):
+    init_global_opts()
+    
     logs.clear()
 
     optsparser = CommandOpts(global_opts, common_opts, command_table,
-------------- next part --------------
--- /home/callla/dnload/subversion/trunk/contrib/client-side/svnmerge/svnmerge_test.py	2007-07-09 16:13:15.000000000 -0600
+++ svnmerge_test.py	2007-07-09 12:45:01.000000000 -0600
@@ -449,6 +470,25 @@
                     TEST_REPO_PATH=self.test_repo_path,
                     TEST_REPO_URL=self.test_repo_url)
 
+    def verify_strings_in_file(self, filename, stringsToVerify):
+        f = open(filename)
+        try:
+            contents = f.read()
+            for s in stringsToVerify:
+                self.assertTrue(contents.find(s) >= 0)
+        finally:
+            f.close()
+        
+    
+    def verify_regex_in_file(self, filename, regex):
+        f = open(filename)
+        try:
+            contents = f.read()
+            self.assertTrue(re.search(regex, contents) != None)
+        finally:
+            f.close()
+        
+    
     def launch(self, cmd, *args, **kwargs):
         cmd = cmd % self.command_dict()
         return TestCase_SvnMerge.launch(self, cmd, *args, **kwargs)
@@ -476,6 +516,22 @@
         else:
             return out[0].strip()
 
+    def testTargetToUrl(self):
+        os.chdir(self.test_path)
+        os.chdir("trunk")
+        self.multilaunch("""
+            svn mkdir dir1
+            svn cp test1 dir1
+            svn ci -m "add directory and a file"
+        """)
+        
+        svnmerge.init_global_opts()
+        self.assert_(re.search("file:///.+/__svnmerge_test/repo/trunk", svnmerge.target_to_url(".")))
+        self.assert_(re.search("file:///.+/__svnmerge_test/repo/trunk/dir1", svnmerge.target_to_url("dir1")))
+        self.assert_(re.search("file:///.+/__svnmerge_test/repo/trunk/dir1/test1", svnmerge.target_to_url("dir1/test1")))
+        self.assert_(re.search("file:///.+/__svnmerge_test/repo/trunk/test1", svnmerge.target_to_url("test1")))
+        self.assertRaises(ValueError, svnmerge.target_to_url, "thisIsNotAFileNorDirectoryName")
+
     def testNoWc(self):
         os.mkdir("foo")
         os.chdir("foo")
@@ -1135,6 +1191,464 @@
                         match=r"Committed revision 18")
         except AssertionError:
             self.assertTrue(os.path.isfile("dir_conflicts.prej"))
+        
+    def testMergeFileRenameOntoModifiedFile(self):
+        """Init svnmerge, edit file in merge target, rename file in merge 
+        source, merge, verify edit was preserved across the rename, and that 
+        commit message additions look right.
+        Related to the "rename problem": for more info see comments in the 
+        modified svnmerge.py file's 
+        "check_for_changes_to_preserve_across_merge".
+        """
+
+        os.chdir(self.test_path)
+        os.chdir("trunk")
+
+        # Initialize svnmerge
+        """todo: "svnmerge init" requires revision range parameters in this 
+        case because of r22788 which changed
+        behavior to be unlike that described in "svnmerge help init", at least
+        in the case where the merge source was created by copying from the 
+        merge target. Try it without the  "-r 1-13" parameter to see the  
+        errors.  Similarly, if you just do "svnmerge init" then 
+        "svnmerge block 12", it fails because it gets the logs to only process
+        revision #'s that actually show up in the logs as changed for that
+        particular file (to omit things changed on other urls, I think), and 
+        12 isn't one of those. """
+        self.svnmerge2(["init", self.test_path + "/test-branch", "-r 1-13"])
+        
+        # edit a local file; rename same file in merge source dir
+        addedText = 'firstmod'
+        editfile = open("test1", "a")
+        try:
+            editfile.write("\n" + addedText)
+        finally:
+            editfile.close()
+        self.multilaunch("""
+            svn up
+            svn ci -m "modify test1"
+        """)
+        os.chdir("../test-branch")
+        self.multilaunch("""
+            svn mv test1 test1renamed
+            svn ci -m "rename test1"
+            svn up
+        """)
+        
+        # Svnmerge merge
+        os.chdir("../trunk")
+        self.svnmerge("merge")
+        
+        # for reading consistency and to make these run whether on *nix or 
+        # windows, using "\s{1,2}" to represent (most) line breaks
+        commitMessageRegex = "Comments from pre-existing copy \(if any\) " + \
+                "of renamed/deleted files-----------:\s{1,2}" + \
+                "In r15 renamed /branches/test-branch/test1 to " + \
+                "/branches/test-branch/test1renamed\s{1,2}" + \
+                "  ........\s{1,2}" + \
+                "  r4 \| \S* \| \d+-\d+-\d+ \d+:\d+:\d+ -\d+ .+ 1 line\s{1,2}" + \
+                "  Changed paths:\s{1,2}" + \
+                "     A /trunk/test1\s{1,2}" + \
+                "\s*" + \
+                "  add test1\s{1,2}" + \
+                "........\s{1,2}" + \
+                "  r14 \| \S* \| \d+-\d+-\d+ \d+:\d+:\d+ -\d+ .+ \| 1 line\s{1,2}" + \
+                "  Changed paths:\s{1,2}" + \
+                "     M /trunk\s{1,2}" + \
+                "     M /trunk/test1\s{1,2}" + \
+                "\s*" + \
+                "  modify test1\s{1,2}" + \
+                "........\s{1,2}"
+        self.verify_regex_in_file("svnmerge-commit-message.txt", \
+                                  commitMessageRegex)
+        self.launch("svn commit -F svnmerge-commit-message.txt",
+                    match=r"Committed revision")
+        
+        # verify that new filename exists and contains "firstmod"
+        self.assertTrue(os.path.isfile("test1renamed"))
+        self.verify_strings_in_file("test1renamed",[addedText])
+
+    def testMergeDirectoryRenameOnlyOntoModifiedFile(self):
+        """Init svnmerge, edit file in merge target, rename directory in merge
+        source, merge, verify edit was preserved across the rename, 
+        and that commit message additions look right.  This is driven by a 
+        failure seen in early manual testing.
+        Related to the "rename problem": for more info see comments in the 
+        modified svnmerge.py file's "check_for_changes_to_preserve_across_merge".
+        """
+
+        os.chdir(self.test_path)
+        os.chdir("trunk")
+        self.multilaunch("""
+            svn mkdir "dir1"
+            svn mkdir "dir1/dir2"
+        """)
+        test6InitialContents = "test 6"
+        open("dir1/dir2/test6", "w").write(test6InitialContents)
+        
+        # we need to create test-branch at this point before we 
+        # edit the file on trunk (see below for more explanation)
+        self.multilaunch('''
+            svn add "dir1/dir2/test6"
+            svn ci -m "adding directories and file test6"
+            svn cp dir1 ../test-branch
+        ''')                                     # creates r14
+
+        # edit a file in merge target (trunk), after it has already been
+        # copied over to the merge source (test-branch), so there will be
+        # a difference between the two directories and thus cause
+        # svnmerge to generate a patch to preserve that change across
+        # the merge of test-branch into trunk:
+        addedText = 'firstmod'
+        editfile = open("dir1/dir2/test6", "a")
+        try:
+            editfile.write("\n" + addedText)
+        finally:
+            editfile.close()
+        r15Comment = "modify test6 file in merge target"
+        self.launch("svn up")
+        self.launch('svn ci -m "'+r15Comment+'"')  # creates r15
+        
+        # set up initial state on test-branch
+        os.chdir("../test-branch")
+        self.launch('svn ci -m "creating dir1, dir2, and file test6"')  # created r16
+        
+        # Initialize svnmerge
+        #todo: (see cmts at same plc in testMergeFileRenameOntoModifiedFile())
+        os.chdir("../trunk")
+        self.svnmerge2(["init", self.test_path + "/test-branch", "-r 1-16"])
+        self.multilaunch('''
+            svn up
+            svn ci -m "initializing trunk"
+        ''')                                      # creates r17
+        os.chdir("../test-branch")
+        
+        # set up changes on test-branch
+        self.launch("svn mv dir1 dir1renamed")
+        r18Comment = "ren dir1 in merge source"
+        self.launch('svn ci -m "'+r18Comment+'"')  # creates r18
+        
+        # Svnmerge merge
+        os.chdir("../trunk")
+        self.svnmerge("merge -r 18")
+        
+        # verification
+        self.verify_strings_in_file("dir1renamed/dir2/test6", [test6InitialContents, addedText])
+        # shouldn't happen this time; does on next test method though:
+        self.assertFalse(os.path.exists("dir1renamed/dir2/test6.rej"))
+
+        # for reading consistency and to make these run whether on *nix or 
+        # windows, using "\s{1,2}" to represent (most) line breaks
+        commitMessageRegex = "Merged revision 18 via svnmerge from\s{1,2}" + \
+            "file://.+repo/branches/test-branch\s{1,2}" + \
+            "\s{1,2}" + \
+            "........\s{1,2}" + \
+            "  r18 \| \S* \| \d+-\d+-\d+ \d+:\d+:\d+ -\d+ .+ 1 line\s{1,2}" + \
+            "  Changed paths:\s{1,2}" + \
+            "     D /branches/test-branch/dir1\s{1,2}" + \
+            "     A /branches/test-branch/dir1renamed \(from " + \
+            "/branches/test-branch/dir1:16\)\s{1,2}" + \
+            "\s{1,2}" + \
+            "  " + r18Comment + "\s{1,2}" + \
+            "........\s{1,2}" + \
+            "\s{1,2}" + \
+            "Comments from pre-existing copy \(if any\) of " + \
+            "renamed/deleted | files-----------:\s{1,2}" + \
+            "In r18 renamed /branches/test-branch/dir1/dir2/test6 " + \
+            "to /branches/test-branch/dir1renamed/dir2/test6\s{1,2}" + \
+            "  ........\s{1,2}" + \
+            "  r14 \| \S* \| \d+-\d+-\d+ \d+:\d+:\d+ -\d+ .+ 1 line\s{1,2}" + \
+            "  Changed paths:\s{1,2}" + \
+            "     A /trunk/dir1\s{1,2}" + \
+            "     A /trunk/dir1/dir2\s{1,2}" + \
+            "     A /trunk/dir1/dir2/test6\s{1,2}" + \
+            "\s{1,2}" + \
+            "  adding directories and file test6\s{1,2}" + \
+            "........\s{1,2}" + \
+            "  r15 \| \S* \| \d+-\d+-\d+ \d+:\d+:\d+ -\d+ .+ 1 line\s{1,2}" + \
+            "  Changed paths:\s{1,2}" + \
+            "     M /trunk/dir1/dir2/test6\s{1,2}" + \
+            "\s{1,2}" + \
+            "  " + r15Comment + "\s{1,2}.+"
+        self.verify_regex_in_file("svnmerge-commit-message.txt", \
+                                  commitMessageRegex)
+        
+    def testMergeDirectoryAndFileRenamesOntoModifiedFile(self):
+        """Create 2 nested directories in data already created by setUp, 
+        create file in them, copy that tree, init svnmerge, rename both dirs, 
+        edit copy of file in merge target, merge, verify edit was preserved 
+        across the rename, that dirs look right, and that commit message 
+        additions look right.
+        Related to the "rename problem": for more info see comments in the 
+        modified svnmerge.py file's "check_for_changes_to_preserve_across_merge".
+        """
+
+        os.chdir(self.test_path)
+        os.chdir("trunk")
+        self.multilaunch("""
+            svn mkdir "dir1"
+            svn mkdir "dir1/dir2"
+        """)
+        test6InitialContents = "test 6"
+        open("dir1/dir2/test6", "w").write(test6InitialContents)
+
+        # we need to create test-branch at this point before we 
+        # edit the file on trunk (see below for more explanation)
+        self.multilaunch("""
+            svn add "dir1/dir2/test6"
+            svn ci -m "adding directories and file test6"
+            svn cp dir1 ../test-branch
+        """)                                        # creates r14
+
+        # edit a file in merge target (trunk), after it has already been
+        # copied over to the merge source (test-branch), so there will be
+        # a difference between the two directories and thus cause
+        # svnmerge to generate a patch to preserve that change across
+        # the merge of test-branch into trunk:
+        addedText = 'firstmod'
+        editfile = open("dir1/dir2/test6", "a")
+        try:
+            editfile.write("\n" + addedText)
+        finally:
+            editfile.close()
+        r15Comment = "modify test6 file in merge target"
+        self.launch("svn up")
+        self.launch('svn ci -m "'+r15Comment+'"')  # creates r15
+        
+        # check in initial state on test-branch (with the pre-modification
+        # copy of the file dir1/dir2/test6)
+        os.chdir("../test-branch")
+        self.launch('svn ci -m "creating dir1, dir2, and file test6"') # created r16
+        
+        # Initialize svnmerge
+        #todo: (see comments at same location in 
+        #testMergeFileRenameOntoModifiedFile())
+        os.chdir("../trunk")
+        self.svnmerge2(["init", self.test_path + "/test-branch", "-r 1-16"])
+        self.multilaunch("""
+            svn up
+            svn ci -m "initializing trunk"
+        """)                                               # created r17
+        os.chdir("../test-branch")
+        
+        # set up changes on test-branch
+        self.launch("svn mv dir1 dir1renamed")
+        editfile = open("dir1renamed/dir2/test6", "a")
+        mergeSourceFileAddedText = "\na file edit on merge source"
+        try:
+            editfile.write(mergeSourceFileAddedText)
+        finally:
+            editfile.close()
+        r18Comment = "ren dir1, and modify test6 file in merge source"
+        self.launch('svn ci -m "'+r18Comment+'"')  # creates r18
+        self.multilaunch("""
+            svn mv dir1renamed/dir2/test6 dir1renamed/dir2/test6renamed
+            svn up
+        """)
+        r19Comment = "ren test6"
+        self.launch('svn ci -m "'+r19Comment+'"')  # creates r19   
+        self.multilaunch("""
+            svn mv dir1renamed/dir2/test6renamed dir1renamed
+            svn mv dir1renamed/dir2 dir1renamed/dir2renamed
+            svn up
+            svn ci -m "ren dir2 and move test6renamed to new location"
+        """)                       # creates r20
+        
+        os.chdir("../trunk")
+        source, rev, copy_committed_in_rev = svnmerge.get_copyfrom('dir1/dir2', True)
+        self.assert_(source == None and rev == None and copy_committed_in_rev == None)
+        source, rev, copy_committed_in_rev = svnmerge.get_copyfrom('dir1/dir2', False)
+        self.assert_(source == None and rev == None and copy_committed_in_rev == None)
+        
+        # Svnmerge merge
+
+        # Merging/testing in steps as user would have to, forced by errors if 
+        # you try to do all at once (like "not a working copy" or "can't open 
+        # file"...).
+        self.svnmerge("merge -r 18 -n")
+        self.assertTrue(os.path.exists('svnmerge-' + \
+                                       "preserveRenamedFileDataFor-" + \
+                                       "dir1renamed-dir2-test6.diff"))
+        self.svnmerge("merge -r 18")
+        file6 = open("dir1renamed/dir2/test6")
+        try:
+            contents = file6.read()
+            self.assertTrue(contents.find(mergeSourceFileAddedText) >= 0)
+            self.assertTrue(contents.find(test6InitialContents) >= 0)
+            
+            ''' Expecting patch failure, so new text not patched in, user must 
+            resolve. If there is a way, I'd like to pass a switch to patch so 
+            it behaves like 'svn merge' in the case of conflicts: put the 
+            data into the file anyway (and, for the real icing on the cake, 
+            don't allow checkin until svn resolved is run). Then we could 
+            reverse this assertFalse to say assertTrue.'''
+            self.assertFalse(contents.find(addedText) >= 0)
+            
+            ''' make sure patch failure files are created--we do want to make 
+            sure patch is running and does the right thing, to some extent at 
+            least (tempering the previous comment).'''
+            self.assertTrue(os.path.exists("dir1renamed/dir2/test6.rej")) 
+        finally:
+            file6.close()
+        # user would have resolved manually to get right file contents; 
+        # not worrying about it for test:
+        self.launch("svn resolved dir1/dir2/test6") 
+        self.launch("svn up")
+        self.launch("svn ci -F svnmerge-commit-message.txt",
+                    match=r"Committed revision") # creates r21
+        
+        # for reading consistency and to make these run whether on *nix or 
+        # windows, using "\s{1,2}" to represent (most) line breaks
+        commitMessageRegex = "Merged revisions 18 via svnmerge from\s{1,2}" + \
+                "file://.+repo/branches/test-branch\s{1,2}" + \
+                "\s{1,2}" + \
+                "........\s{1,2}" + \
+                "  r18 \| \S* \| \d+-\d+-\d+ \d+:\d+:\d+ -\d+ .+ 1 line\s{1,2}" + \
+                "  Changed paths:\s{1,2}" + \
+                "     D /branches/test-branch/dir1\s{1,2}" + \
+                "     A /branches/test-branch/dir1renamed \(from " + \
+                "/branches/test-branch/dir1:16\)\s{1,2}" + \
+                "     M /branches/test-branch/dir1renamed/dir2/test6\s{1,2}" + \
+                "\s+" + \
+                "  " + r18Comment + "\s{1,2}" + \
+                "........\s{1,2}" + \
+                "\s+" + \
+                "Comments from pre-existing copy \(if any\) of " + \
+                "renamed/deleted files-----------:\s{1,2}" + \
+                "In r18 renamed /branches/test-branch/dir1/dir2/test6 " + \
+                "to /branches/test-branch/dir1renamed/dir2/test6\s{1,2}" + \
+                "  ........\s{1,2}" + \
+                "  r14 \| \S* \| \d+-\d+-\d+ \d+:\d+:\d+ -\d+ .+ 1 line\s{1,2}" + \
+                "  Changed paths:\s{1,2}" + \
+                "     A /trunk/dir1\s{1,2}" + \
+                "     A /trunk/dir1/dir2\s{1,2}" + \
+                "     A /trunk/dir1/dir2/test6\s{1,2}" + \
+                "\s+" + \
+                "  adding directories and file test6\s{1,2}" + \
+                "........\s{1,2}" + \
+                "  r15 \| \S* \| \d+-\d+-\d+ \d+:\d+:\d+ -\d+ .+ 1 line\s{1,2}" + \
+                "  Changed paths:\s{1,2}" + \
+                "     M /trunk/dir1/dir2/test6\s{1,2}" + \
+                "\s+" + \
+                "  " + r15Comment + "\s{1,2}.+"
+        self.verify_regex_in_file("svnmerge-commit-message.txt", \
+                                   commitMessageRegex)
+        
+        source, rev, copy_committed_in_rev = svnmerge.get_copyfrom('dir1renamed/dir2', False)
+        self.assert_(source == None and rev == None and copy_committed_in_rev == None)
+        source, rev, copy_committed_in_rev = svnmerge.get_copyfrom('dir1renamed/dir2', True)
+        self.assert_(source == '/branches/test-branch/dir1renamed' and rev == '18' and copy_committed_in_rev == '21')
+        source, rev, copy_committed_in_rev = svnmerge.get_copyfrom('dir1renamed', False)
+        self.assert_(source == '/branches/test-branch/dir1renamed' and rev == '18' and copy_committed_in_rev == '21')
+        source, rev, copy_committed_in_rev = svnmerge.get_copyfrom('dir1renamed', True)
+        self.assert_(source == '/branches/test-branch/dir1renamed' and rev == '18' and copy_committed_in_rev == '21')
+
+        # do/verify/commit another step
+        self.svnmerge("merge -r 19")
+        
+        # for reading consistency and to make these run whether on *nix or 
+        # windows, using "\s{1,2}" to represent (most) line breaks
+        commitMessageRegex = "Merged revisions 19 via svnmerge from\s{1,2}" + \
+                "file://.+repo/branches/test-branch\s{1,2}" + \
+                "\s{1,2}" + \
+                "........\s{1,2}" + \
+                "  r19 \| \S* \| \d+-\d+-\d+ \d+:\d+:\d+ -\d+ .+ 1 line\s{1,2}" + \
+                "  Changed paths:\s{1,2}" + \
+                "     D /branches/test-branch/dir1renamed/dir2/test6\s{1,2}" + \
+                "     A /branches/test-branch/dir1renamed/dir2/test6renamed " + \
+                "\(from /branches/test-branch/dir1renamed/dir2/test6:18\)\s{1,2}" + \
+                "\s+" + \
+                "  " + r19Comment + "\s{1,2}" + \
+                "........\s{1,2}" + \
+                "\s*" + \
+                "Comments from pre-existing copy \(if any\) of " + \
+                "renamed/deleted files-----------:\s{1,2}" + \
+                "In r19 renamed /branches/test-branch/dir1renamed/dir2/test6 " + \
+                "to /branches/test-branch/dir1renamed/dir2/test6renamed\s{1,2}" + \
+                "  ................\s{1,2}" + \
+                "  r21 \| \S* \| \d+-\d+-\d+ \d+:\d+:\d+ -\d+ .+ 32 lines\s{1,2}" + \
+                "  Changed paths:\s{1,2}" + \
+                "     M /trunk\s{1,2}" + \
+                "     D /trunk/dir1\s{1,2}" + \
+                "     A /trunk/dir1renamed \(from " + \
+                "/branches/test-branch/dir1renamed:18\)\s{1,2}" 
+        
+        #commitMessageRegex = commitMessageRegex + \
+                # Curiously, the next line (omitted from the regexp)
+                # only shows up on windows. Seems like 
+                # different svn behavior across platforms? You can see it if on
+                # each platform you set a breakpoint at the line just above,
+                # where we do a commit that "creates r21". Just before
+                # the line executes, if you then go to a command window
+                # and cd to the temporary directory containing the "trunk"
+                # portion of the repository, and run the command "svn st -v"
+                # you'll see that windows and linux show the same thing--some
+                # adds and deletes. Then execute the commit line. Immediately
+                # after that line executes, go back to the command window
+                # and run "svn log -v" and you'll see in the initial output
+                # the difference between windows and linux svn (or something?):
+                # only the windows side shows the following line in the output
+                # for what changed on r21:
+            #"\s+M /trunk/dir1renamed/dir2/test6{1,2}" 
+                # I haven't filed a bug, don't know if that is one...?
+                # So instead, I'll add filler on the next line, so it works
+                # either way.
+        commitMessageRegex = commitMessageRegex + ".+"
+        
+        # Then continue w/ what we normally expected, which is the
+        # same on both platforms I tested:
+        commitMessageRegex = commitMessageRegex + \
+                "\s*" + \
+                "  Merged revisions 18 via svnmerge from\s{1,2}" + \
+                "\s*file:///.+repo/branches/test-branch\s{1,2}" + \
+                "\s*" + \
+                "  ........\s{1,2}" + \
+                "    r18 \| \S* \| \d+-\d+-\d+ \d+:\d+:\d+ -\d+ .+ 1 line\s{1,2}" + \
+                "    Changed paths:\s{1,2}" + \
+                "       D /branches/test-branch/dir1\s{1,2}" + \
+                "       A /branches/test-branch/dir1renamed \(from " + \
+                "/branches/test-branch/dir1:16\)\s{1,2}" + \
+                "       M /branches/test-branch/dir1renamed/dir2/test6\s{1,2}" + \
+                "\s*" + \
+                "    " + r18Comment + "\s{1,2}" + \
+                "  ........\s{1,2}" + \
+                "\s*" + \
+                "  Comments from pre-existing copy \(if any\) of " + \
+                "renamed/deleted files-----------:\s{1,2}" + \
+                "  In r18 renamed /branches/test-branch/dir1/dir2/test6 " + \
+                "to /branches/test-branch/dir1renamed/dir2/test6\s{1,2}" + \
+                "    ........\s{1,2}" + \
+                "    r14 \| \S* \| \d+-\d+-\d+ \d+:\d+:\d+ -\d+ .+ 1 line\s{1,2}" + \
+                "    Changed paths:\s{1,2}" + \
+                "       A /trunk/dir1\s{1,2}" + \
+                "       A /trunk/dir1/dir2\s{1,2}" + \
+                "       A /trunk/dir1/dir2/test6\s{1,2}" + \
+                "\s*" + \
+                "    adding directories and file test6\s{1,2}" + \
+                "  ........\s{1,2}" + \
+                "    r15 \| \S* \| \d+-\d+-\d+ \d+:\d+:\d+ -\d+ .+ 1 line\s{1,2}" + \
+                "    Changed paths:\s{1,2}" + \
+                "       M /trunk/dir1/dir2/test6\s{1,2}\s*" + \
+                "\s*" + \
+                "    " + r15Comment + "\s{1,2}.+"
+        self.verify_regex_in_file("svnmerge-commit-message.txt", \
+                                   commitMessageRegex)
+        self.assertTrue(os.path.exists("dir1renamed/dir2/test6renamed"))
+        self.launch("svn up")
+        self.launch("svn commit -F svnmerge-commit-message.txt",
+                    match=r"Committed revision")  #creates r22
+        
+        # do/verify/commit another step
+        moreAddedText = "anothermod"
+        editfile = open("dir1renamed/dir2/test6renamed", "a")
+        try:
+            editfile.write("\n" + moreAddedText)
+        finally:
+            editfile.close()
+        self.launch("svn up")                                                                                                                         
+        self.launch('svn ci -m "modify test6renamed file in merge target"') # creates r23
+        self.svnmerge("merge -r 20")
+        # todo: write more verifications from here
 
 if __name__ == "__main__":
     # If an existing template repository and working copy for testing


More information about the Svnmerge mailing list