I talked this over on IRC with Ben Sussman, and he recommended
avoiding creation of an empty file for a non-existent resource which
is part of a 3-way merge (possibly only for the "involves binary
files" special case we're dealing with for this issue), then the
refactoring the libsvn_client/diff.c to deal with that as necessary.
I tend to favor this strategy too.
An alternate strategy would be to modify merge_file_changed() to
accept some additional context (e.g. a new parameter) indicating
whether the files involved in the merge actually exist.
On Wed, 10 May 2006, Daniel Rall wrote:
> 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 23:52:23 2006