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

RE: Possible Merge Tracking TODO: No notification of merges on children with mergeinfo unaffected by the merge

From: Paul Burba <pburba_at_collab.net>
Date: 2007-03-28 18:44:34 CEST

> -----Original Message-----
> From: Daniel Rall [mailto:dlr@collab.net]
> Sent: Tuesday, March 27, 2007 2:55 PM
> To: Paul Burba
> Cc: Subversion Development; Kamesh Jayachandran; Peter N. Lundblad
> Subject: Re: Possible Merge Tracking TODO: No notification of
> merges on children with mergeinfo unaffected by the merge
>
> On Tue, 27 Mar 2007, Daniel Rall wrote:
>
> > On Tue, 27 Mar 2007, Paul Burba wrote:

<snip>

> > > 7) Now repeat the first merge we performed,
> > > the mergeinfo is set correctly, but we get
> > > a notification that rho is mer'G'ed which
> > > isn't true since r3 doesn't affect rho.
> > > ------------------------------------------
> > > >svn merge %URL%/A/D/G merge_tests-1\A\D_COPY\G -c3
> > > G merge_tests-1\A\D_COPY\G\rho
> > >
> > > >svn pl -vR merge_tests-1
> > > Properties on 'merge_tests-1\A\D_COPY':
> > > svn:mergeinfo : /A/D:1
> > > Properties on 'merge_tests-1\A\D_COPY\G':
> > > svn:mergeinfo : /A/D/G:1,3
> > > Properties on 'merge_tests-1\A\D_COPY\G\rho':
> > > svn:mergeinfo : /A/D/G/rho:1,3-4
> > >
> > > Note: If the target child is a directory the 'G' notification
> > > doesn't occur.
> > >
> > > So, should the notification in 7) not occur? If so I'll
> add this to
> > > the "Handle notifications resulting from a merge." section in
> > > http://svn.collab.net/repos/svn/trunk/notes/merge-tracking.txt
> > > and/or look into a fix.
> >
> > Paul, the notification from step #7 shouldn't occur. I'd hazard a
> > guess that the notification is related to the way handling for
> > sub-tree merge info performs a separate editor drive for
> children of
> > the target of a merge which have their own merge info, likely in
> > conjunction with the fact that this is a single file.
>
> From reading the code, this seems to be it.
>
> > Usage of the "text_state" local variable in
> > libsvn_client/merge.c:do_single_file_merge(), where we explicitly
> > invoke notification_receiver(), seems interesting.
> ...
>
> do_single_file_merge() calls merge_file_changed(), which sets
> text_state. merge_file_changed() in turn calls
> svn_wc_text_modified_p() to detect whether the target WC path
> has any local mods. The local mods from the merge in step #6
> are detected, and the notification is set accordingly:
>
> else if (has_local_mods)
> *content_state = svn_wc_notify_state_merged;

Looking at merge_file_changed() I wonder about Philip's old (from
r12025) comment at the end of the callback:

      /* Philip asks "Why?" Why does the notification depend on whether
the
         file had modifications before the merge? If the merge didn't
change
         the file because the local mods already included the change why
does
         that result it "merged" notification? That's information
available
         through the status command, while the fact that the merge
didn't
         change the file is lost :-( */

      if (content_state)
        {
          if (merge_outcome == svn_wc_merge_conflict)
            *content_state = svn_wc_notify_state_conflicted;
          else if (has_local_mods)
            *content_state = svn_wc_notify_state_merged;
          else if (merge_outcome == svn_wc_merge_merged)
            *content_state = svn_wc_notify_state_changed;
          else if (merge_outcome == svn_wc_merge_no_merge)
            *content_state = svn_wc_notify_state_missing;
          else /* merge_outcome == svn_wc_merge_unchanged */
            *content_state = svn_wc_notify_state_unchanged;
        }

Any ideas on why we do this? I couldn't see why except the obvious:
that we want to report paths as merged even if the local changes and the
merge diff are identical. With merge-tracking there seems even less
reason to do this since the svn:mergeinfo on 'rho' shows that r3 was
merged into it.

Simply changing...

- else if (has_local_mods)
+ else if (has_local_mods
+ && merge_outcome != svn_wc_merge_unchanged)

...would solve the problem (though it results in a spurious failure in
merge_test-1 and maybe elsewhere, I haven't checked the other tests
yet).

> Since no merge actually occurs to "rho" during step #7,

Not exactly true, since svn_wc_merge2() in merge_file_changed() really
goes through the motions of merging, it's just that there is ultimately
no effect and svn_wc_merge2() reports the merge outcome as
svn_wc_merge_unchanged.

> I still think we need a way to silence this notification for
> this particular case.
> If everyone agrees with this, we can special-case the way
> discover_and_merge_children() calls do_single_file_merge()
> (e.g. drill another flag into do_single_file_merge() for
> whether or not to send notifications for this case).

Assuming for a moment my idea about merge_file_changed() is bogus, any
suggestions how to make discover_and_merge_children() know that r3
doesn't affect 'rho'? svn_ra_get_file_revs()? Rev 3 might very well
affect 'rho' and then we'd want to report the merge.

Paul B.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Mar 28 18:45:11 2007

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