[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

#2403: Incorrect merge of binary file when already existing

From: Daniel Rall <dlr_at_collab.net>
Date: 2006-05-11 02:29:11 CEST

http://subversion.tigris.org/issues/show_bug.cgi?id=2403
http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=104672

Hypothesis: When doing a 3-way merge of a binary file where the left
(older) side of the merge doesn't exist, and the right (yours) side of
the merge is the same as the file in the WC (mine), we're attempting
to compare the left side of the merge to the working copy, rather than
detecting that the left side doesn't exist (as opposed to it just
being an empty file), and comparing the right side to the WC instead.

Example of what's effectively a no-op:

  svn merge file:///repos/trunk file:///repos/branches/B

     older, r0 -----------> yours, r4
  (does not exist) calc diff "New file"

                                           |
                                           | apply
                                           | diff
                                           V

                                       mine, r4+
                                       "New file"

I think we need something like the patch below (which passes
merge_tests.py). This implementation has the problem that you can't
detect the difference between an empty right (older) side of the merge
except for revision 0 (an empty repository). Assuming that this
approach seems correct to everyone else, the client code could be
changed to either a) not write an empty file for files which don't
exist in the repos, or b) pass some flag indicating that an empty file
also signifies that a file doesn't exist in the repository.

Thoughts?

Index: subversion/libsvn_client/diff.c
===================================================================
--- subversion/libsvn_client/diff.c (revision 19605)
+++ subversion/libsvn_client/diff.c (working copy)
@@ -828,22 +828,35 @@
       /* Special case: if a binary file isn't locally modified, and is
          exactly identical to the 'left' side of the merge, then don't
          allow svn_wc_merge to produce a conflict. Instead, just
- overwrite the working file with the 'right' side of the merge. */
+ overwrite the working file with the 'right' side of the merge.
+
+ Alternately, if the 'left' side of the merge doesn't exist in
+ the repository, and the 'right' side of the merge is
+ identical to the WC, pretend we did the merge. */
       if ((! has_local_mods)
           && ((mimetype1 && svn_mime_type_is_binary(mimetype1))
               || (mimetype2 && svn_mime_type_is_binary(mimetype2))))
         {
+ /* ### FIXME: What about nodes deleted after rev 0? Handle
+ ### detection of non-existent files higher up the stack
+ ### in libsvn_client, and don't write an empty file for
+ ### that case. Assure that any code which was dependent
+ ### upon the empty file still works. */
+ svn_boolean_t older_revision_exists = (older_rev > 0);
           svn_boolean_t same_contents;
+ const char *comparison_path = older_revision_exists ? older : yours;
+
           SVN_ERR(svn_io_files_contents_same_p(&same_contents,
- older, mine, subpool));
+ comparison_path,
+ mine, subpool));
           if (same_contents)
             {
- if (! merge_b->dry_run)
- SVN_ERR(svn_io_file_rename(yours, mine, subpool));
+ if (older_revision_exists && !merge_b->dry_run)
+ SVN_ERR(svn_io_file_rename(yours, mine, subpool));
               merge_outcome = svn_wc_merge_merged;
               merge_required = FALSE;
             }
- }
+ }
 
       if (merge_required)
         {

  • application/pgp-signature attachment: stored
Received on Thu May 11 02:29:47 2006

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.