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

Re: svn commit: r1455898 - in /subversion/trunk/subversion: libsvn_client/resolved.c libsvn_wc/wc_db_update_move.c tests/cmdline/stat_tests.py

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Wed, 13 Mar 2013 21:48:55 +0000 (GMT)

Philip Martin wrote:
> Julian Foad <julianfoad_at_btopenworld.com> writes:
>>> +
>>> +          if (merge_outcome == svn_wc_merge_conflict)
>>> +            {
>>> +              content_state = svn_wc_notify_state_conflicted;
>>> +            }
>>> +           else
>>> +            {
>>> +              SVN_ERR(svn_wc__internal_file_modified_p(&is_locally_modified,
>>> +                                                       db, local_abspath,
>>> +                                                       FALSE,
>>> +                                                       scratch_pool));
>>> +              if (is_locally_modified)
>>> +                content_state = svn_wc_notify_state_merged;
>>> +              else
>>> +                content_state = svn_wc_notify_state_changed;
>>> +            }
>>
>> I don't understand that last 'else' block.
>>
>> We're asking 'Is it locally modified?' but I don't think we have a
>> consistent notion of what 'it' is, since the above svn_wc__internal_merge()
>> might or might not have updated the WC file at local_abspath -- it might
>> instead have created a temp file and set up some work items which will
>> install that file later [1].
>>
>> Do we actually want to ask here, 'Was the file locally modified before we
>> started merging?'  If so, this whole block should be replaced with just
>> "content_state = svn_wc_notify_state_merged;".
>
> I'm a bit confused by that code as well but I didn't add that code in
> this commit I just changed the identation.  It dates back to r1406631.

You didn't add this code but you repeated the 'svn_wc__internal_file_modified_p' call earlier, and moved all the existing code inside the 'else' of that earlier test, thus changing the conditions under which this can be executed -- it can no longer be executed if the file wasn't already locally modified.

I think the intent is to print 'U' if we're updating the file content
when it was not already locally modified, or 'G' if we are merging into a
file that was already locally modified.  Or something close to that. 
I'm not convinced that we're very consistent about this aspect of
updates and merges.

- Julian
Received on 2013-03-13 22:49:29 CET

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