You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2015/02/20 19:26:04 UTC
svn commit: r1661179 - in /subversion/trunk/subversion:
include/private/svn_wc_private.h libsvn_client/commit.c libsvn_wc/node.c
tests/cmdline/log_tests.py
Author: rhuijben
Date: Fri Feb 20 18:26:04 2015
New Revision: 1661179
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.
* 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/include/private/svn_wc_private.h
subversion/trunk/subversion/libsvn_client/commit.c
subversion/trunk/subversion/libsvn_wc/node.c
subversion/trunk/subversion/tests/cmdline/log_tests.py
Modified: subversion/trunk/subversion/include/private/svn_wc_private.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_wc_private.h?rev=1661179&r1=1661178&r2=1661179&view=diff
==============================================================================
--- subversion/trunk/subversion/include/private/svn_wc_private.h (original)
+++ subversion/trunk/subversion/include/private/svn_wc_private.h Fri Feb 20 18:26:04 2015
@@ -519,26 +519,6 @@ svn_wc__node_get_origin(svn_boolean_t *i
apr_pool_t *scratch_pool);
/**
- * Set @a *deleted_ancestor_abspath to the root of the delete operation
- * that deleted @a local_abspath. If @a local_abspath itself was deleted
- * and has no deleted ancestor, @a *deleted_ancestor_abspath will equal
- * @a local_abspath. If @a local_abspath was not deleted,
- * set @a *deleted_ancestor_abspath to @c NULL.
- *
- * A node is considered 'deleted' if it is deleted or moved-away, and is
- * not replaced.
- *
- * @a *deleted_ancestor_abspath is allocated in @a result_pool.
- * Use @a scratch_pool for all temporary allocations.
- */
-svn_error_t *
-svn_wc__node_get_deleted_ancestor(const char **deleted_ancestor_abspath,
- svn_wc_context_t *wc_ctx,
- const char *local_abspath,
- apr_pool_t *result_pool,
- apr_pool_t *scratch_pool);
-
-/**
* Set @a *not_present to TRUE when @a local_abspath has status
* svn_wc__db_status_not_present. Set @a *user_excluded to TRUE when
* @a local_abspath has status svn_wc__db_status_excluded. Set
Modified: subversion/trunk/subversion/libsvn_client/commit.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/commit.c?rev=1661179&r1=1661178&r2=1661179&view=diff
==============================================================================
--- 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/libsvn_wc/node.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/node.c?rev=1661179&r1=1661178&r2=1661179&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/node.c (original)
+++ subversion/trunk/subversion/libsvn_wc/node.c Fri Feb 20 18:26:04 2015
@@ -527,33 +527,6 @@ svn_wc__internal_walk_children(svn_wc__d
}
svn_error_t *
-svn_wc__node_get_deleted_ancestor(const char **deleted_ancestor_abspath,
- svn_wc_context_t *wc_ctx,
- const char *local_abspath,
- apr_pool_t *result_pool,
- apr_pool_t *scratch_pool)
-{
- svn_wc__db_status_t status;
-
- *deleted_ancestor_abspath = NULL;
-
- SVN_ERR(svn_wc__db_read_info(&status,
- NULL, NULL, NULL, NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL, NULL,
- wc_ctx->db, local_abspath,
- scratch_pool, scratch_pool));
-
- if (status == svn_wc__db_status_deleted)
- SVN_ERR(svn_wc__db_scan_deletion(deleted_ancestor_abspath, NULL, NULL,
- NULL, wc_ctx->db, local_abspath,
- result_pool, scratch_pool));
-
- return SVN_NO_ERROR;
-}
-
-svn_error_t *
svn_wc__node_is_not_present(svn_boolean_t *is_not_present,
svn_boolean_t *is_excluded,
svn_boolean_t *is_server_excluded,
Modified: subversion/trunk/subversion/tests/cmdline/log_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/log_tests.py?rev=1661179&r1=1661178&r2=1661179&view=diff
==============================================================================
--- 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
Re: r1661179 - avoiding revision gaps when committing simple moves
Posted by Julian Foad <ju...@btopenworld.com>.
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
>