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

Re: Merge bug causes changesets to be applied although this should not be the case

From: Paul Burba <ptburba_at_gmail.com>
Date: Wed, 10 Apr 2013 11:35:35 -0400

On Wed, Apr 10, 2013 at 5:12 AM, Christoph Schulz <develop_at_kristov.de> wrote:
> Hello,
>
> in SVN 1.7.9 and earlier versions (als tested 1.7.7 and 1.7.8), it can
> happen that a merge succeeds although the patch definitely cannot be
> applied. This happens when the changeset to be applied deletes some lines at
> the very beginning of a file. Such a changeset is happily "applied" by "svn
> merge" although the target file does not contain the lines to be deleted.
>
> Please see the associated test case. The merge after r4 succeeds (without
> having any effects), although the corresponding "patch" command correctly
> fails. Basically, "svn merge" allows to apply files2.patch before
> files1.patch, which is clearly wrong.

>#!/bin/sh
>svnadmin create ~/mergetest-repo
>svn co file://$HOME/mergetest-repo mergetest
>cd mergetest/
>mkdir -p trunk branches/testing
>svn add *
>svn ci -m "directories created"
># r1 created
>cp ~/files.txt trunk/files.txt
>svn add trunk/files.txt
>svn ci -m "added files.txt"
># r2 created
>svn update
>patch trunk/files.txt ~/files1.patch
>svn ci -m "files.txt changed"
># r3 created
>svn update
>patch trunk/files.txt ~/files2.patch
>svn ci -m "files.txt corrected"
># r4 created
>svn update
>svn copy trunk/files.txt_at_2 branches/testing/
>svn ci -m "copied files.txt to testing branch"
>svn update
># this should _not_ work, as r4 contains a deletion
># which cannot be applied to files.txt_at_2!
># the "patch" utility correctly reports that the hunk could not be applied
>#svn diff -c4 trunk/files.txt | patch -p0 branches/testing/files.txt || exit 1
>svn merge -c4 trunk branches/testing # <-- merge succeeds!!! why?

Hi Christoph,

If the left side of the merge doesn't equal the right side, but the
right side of the merge is identical to the target, then we have long
treated this as a no-op. As to why, I'm not entirely sure, but this
behavior is intentional at least as far back as 1.5. There is a note
in the code noting that this should more correctly be a conflict:

From https://svn.apache.org/repos/asf/subversion/trunk/subversion/libsvn_wc/merge.c@1466488:

/* Attempt a trivial merge of LEFT_ABSPATH and RIGHT_ABSPATH to
 * the target file at TARGET_ABSPATH.
.
<SNIP>
.
 * This case is also treated as trivial:
 *
 * left != right, right == target => no-op
 *
 * ### Strictly, this case is a conflict, and the no-op outcome is only
 * one of the possible resolutions.
 *
 * TODO: Raise a conflict at this level and implement the 'no-op'
 * resolution of that conflict at a higher level, in preparation for
 * being able to support stricter conflict detection.
 .
<SNIP>
.
 */
static svn_error_t *
merge_file_trivial(svn_skel_t **work_items,
                   enum svn_wc_merge_outcome_t *merge_outcome,
                   const char *left_abspath,
                   const char *right_abspath,
                   const char *target_abspath,
                   const char *detranslated_target_abspath,
                   svn_boolean_t dry_run,
                   svn_wc__db_t *db,
                   svn_cancel_func_t cancel_func,
                   void *cancel_baton,
                   apr_pool_t *result_pool,
                   apr_pool_t *scratch_pool)
{
.
<SNIP>
.
  else
    {
      /* If the locally existing, changed file equals the incoming 'right'
       * file, there is no conflict. For binary files, we historically
       * conflicted them needlessly, while merge_text_file figured it out
       * eventually and returned svn_wc_merge_unchanged for them, which
       * is what we do here. */
      if (same_right_target)
        {
          *merge_outcome = svn_wc_merge_unchanged;
          return SVN_NO_ERROR;
        }
    }

  *merge_outcome = svn_wc_merge_no_merge;
  return SVN_NO_ERROR;
}

If you are inclined, and have the time, perhaps you could file an
issue in the issue tracker
(http://subversion.tigris.org/issue-tracker.html)? If you do, please
reference this thread.

>svn ci -m "r4 integrated from trunk"
># r5 created

Minor nit: That's actually r6

>svn update
>svn merge -c3 trunk branches/testing
>svn ci -m "r3 integrated from trunk"
># r6 created

...and that's r7. Doesn't really matter for your example of course.

>svn update
># files are different!

Yes. But given the current (intentional) behavior in the merge of r4
above, this is what is expected. Let's say that r4 did raise some
type of conflict. How would you resolve it? It seems quite likely
that the resolution would be the same as what automatically happens
today. And then the merge of r3 would work just like it does here.
And in the event that you manually resolved the change some other way,
then perhaps the merge of r3 would raise a conflict. Either way
though, I don't see any problem with the way the merge of r3 is
handled.

Is it that you want all three of these merges to result in the same WC state?

  svn merge -r2:4 ^trunk branches/testing

or

  svn merge -c3 ^trunk branches/testing
  svn merge -c4 ^trunk branches/testing

or

  svn merge -c4 ^trunk branches/testing
  svn merge -c3 ^trunk branches/testing

>cmp -s trunk/files.txt branches/testing/files.txt && echo "OK" || echo "FAILURE"
>cd ..

> Is this a known bug? Do any workarounds exist to prevent this bug from
> occurring?

Don't cherry pick the newer revisions before older revisions.
Obviously from your simple reproduction script[1] there appears little
need to do this, but I'm assuming you have some pressing need to do
this in some more complex real world example. If you do have such a
real world example, describing it would go a long way to prompting
someone to fix this, because right now it strikes me as more of an
academic problem.

--
Paul T. Burba
CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development
Skype: ptburba
[1] Thanks for that by the way -- It makes investigating bug reports
so much easier!
> Regards,
>
> Christoph Schulz
Received on 2013-04-10 17:36:13 CEST

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