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

RE: [PATCH]: Stop mer'G'e notification for files with local mods that are unchanged by merge

From: Paul Burba <pburba_at_collab.net>
Date: 2007-04-06 20:50:08 CEST

> -----Original Message-----
> From: Daniel Rall [mailto:dlr@collab.net]
> Sent: Thursday, April 05, 2007 7:16 PM
> To: Paul Burba
> Cc: Subversion Development; Kamesh Jayachandran; Karl Fogel;
> philip@codematters.co.uk
> Subject: Re: [PATCH]: Stop mer'G'e notification for files
> with local mods that are unchanged by merge
>
> On Wed, 04 Apr 2007, Paul Burba wrote:
>
> > When merging into a file with preexisting local modifications, our
> > code in merge.c:merge_file_changed() always reports a mer'G'e has
> > occurred, even if the merge made no change to the file (i.e.
> > svn_wc_merge2() returns a merge outcome of svn_wc_merge_unchanged).
> >
> > This results in a lot of incorrect notifications with
> merge-tracking -
> > see http://svn.haxx.se/dev/archive-2007-03/1157.shtml But
> I'd argue
> > it's incorrect regardless of merge-tracking and would like to apply
> > the attached patch. I see no problems with this, but
> Philip Martin's
> > question in merge_file_changed()...
> >
> > /* 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;
> >
> > ...has been there a long time and *something* gave him pause or I
> > suspect he would have fixed this way back in the day...
> >
> > Anyone know the history of this and/or have any objections?
>
> Paul, I don't have first-hand knowledge of the history behind
> this code. Looking back through libsvn_client/diff.c's VC
> history, this code was introduced in r2305 to fix issue #662,
> "making merge callbacks use the new notification system."
> Philip added his comment
> 5 months later in r3640. At face value, it doesn't look to
> me like the code was ever correct.
>
> +1 on committing your patch, and nominating this bug fix for backport
> to the 1.4.x and 1.3.x branches.

Thanks for taking a look Dan. Committed r24483 and nominated for
backport (I added you as a +1 for both - assuming it was implied).

Paul B.
 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Apr 6 20:50:38 2007

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.