Bert proposed this for backport to 1.9.0.
> URL: http://svn.apache.org/r1661179
> Log:
> Start using our move information in our commit processing to avoid
> unneeded revision gaps when committing simple moves.
>
> This handles the simple case where (during commit) we know that a
> node didn't change in a specific revision range. As this is a
> client side fixup this can't resolve the hard cases, but at
> least it is safe and helps removing many gaps.
I'd love us to be better at committing moves without a gap, but I'd like to get
a bit more visibility and thought about how we do it. It just feels to me that
this may not be as obvious as it at first looks.
- Julian
> * subversion/include/private/svn_wc_private.h
> (svn_wc__node_get_deleted_ancestor): Remove unneeded function.
>
> * subversion/libsvn_client/commit.c
> (svn_client_commit6): Attempt to fix-up the copy-from revision of
> moved nodes, by fixing up the known safe cases. Trust that the
> delete_op_root is really the delete op_root.
>
> * subversion/libsvn_wc/node.c
> (svn_wc__node_get_deleted_ancestor): Remove function.
>
> * subversion/tests/cmdline/log_tests.py
> (log_revision_move_copy): Update test expectations. The client
> side commit is now smart enough to avoid the revision gap, for
> direct moves from BASE.
> Modified: subversion/trunk/subversion/libsvn_client/commit.c
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/commit.c (original)
> +++ subversion/trunk/subversion/libsvn_client/commit.c Fri Feb 20 18:26:04
> 2015
> @@ -537,6 +537,7 @@ svn_client_commit6(const apr_array_heade
> const char *current_abspath;
> const char *notify_prefix;
> int depth_empty_after = -1;
> + apr_hash_t *move_youngest = NULL;
> int i;
>
> SVN_ERR_ASSERT(depth != svn_depth_unknown && depth !=
> svn_depth_exclude);
> @@ -707,62 +708,12 @@ svn_client_commit6(const apr_array_heade
> if (cmt_err)
> goto cleanup;
>
> - if (moved_from_abspath && delete_op_root_abspath &&
> - strcmp(moved_from_abspath, delete_op_root_abspath) == 0)
> -
> + if (moved_from_abspath && delete_op_root_abspath)
> {
> - svn_boolean_t found_delete_half =
> - (svn_hash_gets(committables->by_path,
> delete_op_root_abspath)
> - != NULL);
> + svn_client_commit_item3_t *delete_half =
> + svn_hash_gets(committables->by_path,
> delete_op_root_abspath);
>
> - if (!found_delete_half)
> - {
> - const char *delete_half_parent_abspath;
> -
> - /* The delete-half isn't in the commit target list.
> - * However, it might itself be the child of a deleted node,
> - * either because of another move or a deletion.
> - *
> - * For example, consider: mv A/B B; mv B/C C; commit;
> - * C's moved-from A/B/C is a child of the deleted A/B.
> - * A/B/C does not appear in the commit target list, but
> - * A/B does appear.
> - * (Note that moved-from information is always stored
> - * relative to the BASE tree, so we have 'C moved-from
> - * A/B/C', not 'C moved-from B/C'.)
> - *
> - * An example involving a move and a delete would be:
> - * mv A/B C; rm A; commit;
> - * Now C is moved-from A/B which does not appear in the
> - * commit target list, but A does appear.
> - */
> -
> - /* Scan upwards for a deletion op-root from the
> - * delete-half's parent directory. */
> - delete_half_parent_abspath =
> - svn_dirent_dirname(delete_op_root_abspath, iterpool);
> - if (strcmp(delete_op_root_abspath,
> - delete_half_parent_abspath) != 0)
> - {
> - const char *parent_delete_op_root_abspath;
> -
> - cmt_err = svn_error_trace(
> - svn_wc__node_get_deleted_ancestor(
> - &parent_delete_op_root_abspath,
> - ctx->wc_ctx, delete_half_parent_abspath,
> - iterpool, iterpool));
> - if (cmt_err)
> - goto cleanup;
> -
> - if (parent_delete_op_root_abspath)
> - found_delete_half =
> - (svn_hash_gets(committables->by_path,
> - parent_delete_op_root_abspath)
> - != NULL);
> - }
> - }
> -
> - if (!found_delete_half)
> + if (!delete_half)
> {
> cmt_err = svn_error_createf(
> SVN_ERR_ILLEGAL_TARGET, NULL,
> @@ -787,6 +738,17 @@ svn_client_commit6(const apr_array_heade
>
> goto cleanup;
> }
> + else if (delete_half->revision == item->copyfrom_rev)
> + {
> + /* Ok, now we know that we perform an out-of-date check
> + on the copyfrom location. Remember this for a fixup
> + round right before committing. */
> +
> + if (!move_youngest)
> + move_youngest = apr_hash_make(pool);
> +
> + svn_hash_sets(move_youngest, item->path, item);
> + }
> }
> }
>
> @@ -885,6 +847,37 @@ svn_client_commit6(const apr_array_heade
> if (cmt_err)
> goto cleanup;
>
> + if (move_youngest != NULL)
> + {
> + apr_hash_index_t *hi;
> + svn_revnum_t youngest;
> +
> + SVN_ERR(svn_ra_get_latest_revnum(ra_session, &youngest, pool));
> +
> + for (hi = apr_hash_first(iterpool, move_youngest);
> + hi;
> + hi = apr_hash_next(hi))
> + {
> + svn_client_commit_item3_t *item = apr_hash_this_val(hi);
> +
> + /* We delete the original side with its original revision and will
> + receive an out-of-date error if that node changed since that
> + revision.
> +
> + The copy is of that same revision and we know that this revision
> + didn't change between this revision and youngest. So we can
> just
> + as well commit a copy from youngest.
> +
> + Note that it is still possible to see gaps between the delete and
> + copy revisions as the repository might handle multiple commits
> + at the same time (or when an out of date proxy is involved), but
> + in general it should decrease the number of gaps. */
> +
> + if (item->copyfrom_rev < youngest)
> + item->copyfrom_rev = youngest;
> + }
> + }
> +
> cmt_err = svn_error_trace(
> get_ra_editor(&editor, &edit_baton, ra_session, ctx,
> log_msg, commit_items, revprop_table,
>
> Modified: subversion/trunk/subversion/tests/cmdline/log_tests.py
> ==============================================================================
> --- subversion/trunk/subversion/tests/cmdline/log_tests.py (original)
> +++ subversion/trunk/subversion/tests/cmdline/log_tests.py Fri Feb 20
> 18:26:04
> 2015
> @@ -2627,7 +2627,10 @@ def log_revision_move_copy(sbox):
> sbox.simple_append('iotb', 'new line\n')
>
> sbox.simple_copy('A/mu', 'mutb')
> - sbox.simple_append('iotb', 'mutb\n')
> + sbox.simple_append('mutb', 'mutb\n')
> +
> + sbox.simple_move('A/B/E', 'E')
> + sbox.simple_move('E/alpha', 'alpha')
>
> #r2
> svntest.actions.run_and_verify_svn(None, [],
> @@ -2639,10 +2642,12 @@ def log_revision_move_copy(sbox):
> # of these nodes behaves in r2.
>
> # This one might change behavior once we improve move handling
> - expected_output = []
> - expected_err = '.*E195012: Unable to find repository location.*'
> + expected_output = [
> +
> '------------------------------------------------------------------------\n'
> + ]
> + expected_err = []
> svntest.actions.run_and_verify_svn(expected_output, expected_err,
> - 'log', '-v',
> sbox.ospath('iotb'),
> + 'log',
> '-v',sbox.ospath('iotb'),
> '-r2')
>
> # While this one
> @@ -2653,8 +2658,10 @@ def log_revision_move_copy(sbox):
> '-r2')
>
> # And just for fun, do the same thing for blame
> - expected_output = None
> - expected_err = '.*E195012: Unable to find repository location.*'
> + expected_output = [
> + ' 1 jrandom This is the file
> \'iota\'.\n'
> + ]
> + expected_err = []
> svntest.actions.run_and_verify_svn(expected_output, expected_err,
> 'blame',
> sbox.ospath('iotb'),
> '-r2')
> @@ -2662,9 +2669,26 @@ def log_revision_move_copy(sbox):
> expected_output = None
> expected_err = '.*E195012: Unable to find repository location.*'
> svntest.actions.run_and_verify_svn(expected_output, expected_err,
> - 'log', '-v',
> sbox.ospath('mutb'),
> + 'blame',
> sbox.ospath('mutb'),
> '-r2')
>
> + expected_output = svntest.verify.RegexListOutput([
> + '-+\\n',
> + 'r3\ .*\n',
> + re.escape('Changed paths:\n'),
> + re.escape(' D /A/B/E\n'),
> + re.escape(' A /E (from /A/B/E:2)\n'), # Patched - Direct move
> + re.escape(' D /E/alpha\n'),
> + re.escape(' A /alpha (from /A/B/E/alpha:1)\n'), # Indirect
> move - Not patched
> + re.escape(' D /iota\n'),
> + re.escape(' A /iotb (from /iota:2)\n'), # Patched - Direct
> move
> + re.escape(' A /mutb (from /A/mu:1)\n'), # Copy (always r1)
> + '-+\\n'
> + ])
> + svntest.actions.run_and_verify_svn(expected_output, [],
> + 'log', '-v', '-q',
> sbox.wc_dir,
> + '-c3')
> +
>
> ########################################################################
> # Run the tests
>
