On Fri, Apr 16, 2010 at 11:09:06PM -0000, ne...@apache.org wrote:
[...]
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_wc/node.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/node.c Fri Apr 16 23:09:06 2010
> @@ -626,6 +626,75 @@ svn_wc__node_get_base_rev(svn_revnum_t *
> }
>
> svn_error_t *
> +svn_wc__node_get_commit_base_rev(svn_revnum_t *commit_base_revision,
> + svn_wc_context_t *wc_ctx,
> + const char *local_abspath,
> + apr_pool_t *scratch_pool)
> +{
> + svn_wc__db_status_t status;
> +
> + SVN_ERR(svn_wc__db_read_info(&status, NULL,
> + commit_base_revision,
> + 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));
What about if a path has been replaced? svn_wc__node_get_commit_base_rev
is used when diffing wc to wc. Consider this:
[[[
$ svn rm A/D/G/pi
$ svn di
Index: A/D/G/pi
===================================================================
--- A/D/G/pi (revision 1)
+++ A/D/G/pi (working copy)
@@ -1 +0,0 @@
-This is the file 'pi'.
]]]
[[[
$ svn rm A/D/G/pi
$ touch A/D/G/pi
$ svn add A/D/G/pi
Index: A/D/G/pi
===================================================================
--- A/D/G/pi (working copy)
+++ A/D/G/pi (working copy)
@@ -1 +0,0 @@
-This is the file 'pi'.
]]]
When we have a replaced node we get an incorrect base revision!
> +
> + /* If this returned a valid revnum, there is no WORKING node. The node is
> + cleanly checked out, no modifications, copies or replaces. */
> + if (SVN_IS_VALID_REVNUM(*commit_base_revision))
> + return SVN_NO_ERROR;
> +
> + if (status == svn_wc__db_status_added)
> + {
> + /* If the node was copied/moved-here, return the copy/move source
> + revision (not this node's base revision). If it's just added,
> + return SVN_INVALID_REVNUM. */
> + SVN_ERR(svn_wc__db_scan_addition(NULL, NULL, NULL, NULL, NULL, NULL,
> + NULL, NULL, commit_base_revision,
> + wc_ctx->db, local_abspath,
> + scratch_pool, scratch_pool));
> + }
I assume you're doing this to not change the behaviour of the callers.
But that we in the future, always should set the revision to -1 for
add, copy-here and move-here.
> + else if (status == svn_wc__db_status_deleted)
> + {
> + /* This node is deleted, but we didn't get a revnum above. So this must
> + be a delete inside a locally copied tree. Return the revision number
> + of the parent of the delete, presumably the copy-from revision.
> + ### This is legacy behaviour, and it sucks. */
I thought that we would only get a rev from read_info() when we have a
delete if the op_root was the path itself, e.g. that we would need to
scan upwards if we had deleted A/D and was checking the rev of A/D/G/pi.
> + const char *work_del_abspath;
> + const char *parent_abspath;
> + svn_wc__db_status_t parent_status;
> +
> + SVN_ERR(svn_wc__db_scan_deletion(NULL, NULL, NULL,
> + &work_del_abspath,
> + wc_ctx->db, local_abspath,
> + scratch_pool, scratch_pool));
> + SVN_ERR_ASSERT(work_del_abspath != NULL);
> + parent_abspath = svn_dirent_dirname(work_del_abspath, scratch_pool);
> +
> + SVN_ERR(svn_wc__db_read_info(&parent_status,
> + 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, parent_abspath,
> + scratch_pool, scratch_pool));
> +
> + SVN_ERR_ASSERT(parent_status == svn_wc__db_status_added
> + || parent_status == svn_wc__db_status_obstructed_add);
> +
> + SVN_ERR(svn_wc__db_scan_addition(NULL, NULL, NULL, NULL, NULL, NULL,
> + NULL, NULL,
> + commit_base_revision,
> + wc_ctx->db, parent_abspath,
> + scratch_pool, scratch_pool));
> + }
> +
> + return SVN_NO_ERROR;
> +}
In a couple of places we're going to have similar behaviour as this one.
Consider libsvn_wc/adm_crawler.c:find_base_rev() and my WIP for
libsvn_wc/status.c:assemble_status(). Maybe some parts could be put
together to avoid code duplication.
> +
> +svn_error_t *
> svn_wc__node_get_lock_token(const char **lock_token,
> svn_wc_context_t *wc_ctx,
> const char *local_abspath,
