[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 20:56:30 +0000 (GMT)

> Author: philip

> Date: Wed Mar 13 12:03:41 2013
> New Revision: 1455898
>
> URL: http://svn.apache.org/r1455898
> Log:
> Detect unmodified files being updated during move-update.
>
> * subversion/libsvn_client/resolved.c
>   (svn_client_resolve): Add sleep for timestamps.
>
> * subversion/libsvn_wc/wc_db_update_move.c
>   (update_working_file): Detect unmodified local file and install
>   pristine rather than merging.
[...]

> Modified: subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c
> +          /*
> +          * Run a 3-way merge to update the file, using the pre-update
> +          * pristine text as the merge base, the post-update pristine
> +          * text as the merge-left version, and the current content of the
> +          * moved-here working file as the merge-right version.
> +          */
> +          SVN_ERR(svn_wc__db_pristine_get_path(&old_pristine_abspath,
> +                                              db, wcroot->abspath,
> +                                              old_version->checksum,
> +                                              scratch_pool, scratch_pool));
> +          SVN_ERR(svn_wc__db_pristine_get_path(&new_pristine_abspath,
> +                                              db, wcroot->abspath,
> +                                              new_version->checksum,
> +                                              scratch_pool, scratch_pool));
> +          SVN_ERR(svn_wc__internal_merge(&work_item, &conflict_skel,
> +                                        &merge_outcome, db,
> +                                        old_pristine_abspath,
> +                                        new_pristine_abspath,
> +                                        local_abspath,
> +                                        local_abspath,
> +                                        NULL, NULL, NULL, /* diff labels */
> +                                        actual_props,
> +                                        FALSE, /* dry-run */
> +                                        NULL, /* diff3-cmd */
> +                                        NULL, /* merge options */
> +                                        propchanges,
> +                                        NULL, NULL, /* cancel_func + baton */
> +                                        scratch_pool, scratch_pool));
> +
> +          work_items = svn_wc__wq_merge(work_items, work_item, scratch_pool);
> +
> +          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 /* exact_comparison */,
> +                                                      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;".

>         }
>     }
>   else

[1] Actually in
r1432216 I edited the doc string of svn_wc__internal_merge() to say it
does not update the WC file.  It looks like I was over-zealous there and should change it as follows:

[[[
Index: subversion/libsvn_wc/wc.h
===================================================================
--- subversion/libsvn_wc/wc.h    (revision 1456105)
+++ subversion/libsvn_wc/wc.h    (working copy)
@@ -391,6 +391,7 @@
 /* Prepare to merge a file content change into the working copy.  This
    does not merge properties; see svn_wc__merge_props() for that.  This
-   does not change the working file on disk; it returns work items that
-   will replace the working file on disk when they are run.
+   does not necessarily change the file TARGET_ABSPATH on disk; it may
+   instead return work items that will replace the file on disk when
+   they are run.
 
    Merge the difference between LEFT_ABSPATH and RIGHT_ABSPATH into
]]]

- Julian
Received on 2013-03-13 21:57:05 CET

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