For those who have difficulties seeing the attachment, let me repost it inline. You can also get it at http://kleinekatze.de/get_revision_number.20100331-1154
~Neels
[[[
Remove entry_t usage from svn_client__get_revision_number().
* subversion/include/private/svn_wc_private.h
(svn_wc__node_get_base_rev): Properly document this function.
(svn_wc__node_get_commit_base_rev): New function, declaration.
* subversion/libsvn_client/revisions.c
(svn_client__get_revision_number): Remove entry_t use. Behaviour changes:
Added nodes return SVN_INVALID_REVNUM instead of 0. Otherwise return
the commit-base revision (the copy-from revision if applicable, otherwise
the NG-BASE node's revision).
* subversion/libsvn_wc/node.c
(svn_wc__node_get_commit_base_rev): New function, implementation.
* subversion/tests/cmdline/diff_tests.py
(diff_non_version_controlled_file): Adapt expected error message string.
--This line, and those below, will be ignored--
Index: subversion/include/private/svn_wc_private.h
===================================================================
--- subversion/include/private/svn_wc_private.h (revision 929082)
+++ subversion/include/private/svn_wc_private.h (working copy)
@@ -477,6 +477,16 @@ svn_wc__node_is_status_added(svn_boolean
* Get the base revision of @a local_abspath using @a wc_ctx. If
* @a local_abspath is not in the working copy, return
* @c SVN_ERR_WC_PATH_NOT_FOUND.
+ *
+ * Return the revision of the revert-base, i.e. the revision that this node
+ * was checked out at or last updated/switched to regardless of any
+ * uncommitted changes (delete, replace and/or copy-here/move-here). The
+ * returned revision does not reflect any uncommitted changes on the node.
+ *
+ * For a locally added node that is not part of a replace, this
+ * returns @c SVN_INVALID_REVNUM.
+ *
+ * See also svn_wc__node_get_commit_base_rev().
*/
svn_error_t *
svn_wc__node_get_base_rev(svn_revnum_t *base_revision,
@@ -485,6 +495,31 @@ svn_wc__node_get_base_rev(svn_revnum_t *
apr_pool_t *scratch_pool);
/**
+ * Get the base revision of @a local_abspath using @a wc_ctx. If
+ * @a local_abspath is not in the working copy, return
+ * @c SVN_ERR_WC_PATH_NOT_FOUND.
+ *
+ * Return the revision number of the base for this node's next commit, which
+ * reflects the uncommitted changes on this node.
+ *
+ * If this node has no uncommitted changes, return the same as
+ * svn_wc__node_get_base_rev().
+ *
+ * If this node is moved-here or copied-here (possibly as part of a replace),
+ * return the revision of the copy/move source.
+ *
+ * If this node is locally added or replaced and is not moved-here or
+ * copied-here, return the (revert-base) revision of the parent node under
+ * which this node was added (more precisely, the revision of the parent of
+ * this local add's op-root).
+ */
+svn_error_t *
+svn_wc__node_get_commit_base_rev(svn_revnum_t *base_revision,
+ svn_wc_context_t *wc_ctx,
+ const char *local_abspath,
+ apr_pool_t *scratch_pool);
+
+/**
* Get the lock token of @a local_abspath using @a wc_ctx or NULL
* if there is no lock. If @a local_abspath is not in the working
* copy, return @c SVN_ERR_WC_PATH_NOT_FOUND.
Index: subversion/libsvn_client/revisions.c
===================================================================
--- subversion/libsvn_client/revisions.c (revision 929082)
+++ subversion/libsvn_client/revisions.c (working copy)
@@ -79,8 +79,6 @@ svn_client__get_revision_number(svn_revn
case svn_opt_revision_working:
case svn_opt_revision_base:
{
- const svn_wc_entry_t *ent;
-
/* Sanity check. */
if (local_abspath == NULL)
return svn_error_create(SVN_ERR_CLIENT_VERSIONED_PATH_REQUIRED,
@@ -91,19 +89,15 @@ svn_client__get_revision_number(svn_revn
if (svn_path_is_url(local_abspath))
goto invalid_rev_arg;
- SVN_ERR(svn_wc__get_entry_versioned(&ent, wc_ctx, local_abspath,
- svn_node_unknown, FALSE, FALSE,
- scratch_pool, scratch_pool));
-
- *revnum = ent->revision;
+ SVN_ERR(svn_wc__node_get_commit_base_rev(revnum, wc_ctx,
+ local_abspath,
+ scratch_pool));
}
break;
case svn_opt_revision_committed:
case svn_opt_revision_previous:
{
- const svn_wc_entry_t *ent;
-
/* Sanity check. */
if (local_abspath == NULL)
return svn_error_create(SVN_ERR_CLIENT_VERSIONED_PATH_REQUIRED,
@@ -114,15 +108,14 @@ svn_client__get_revision_number(svn_revn
if (svn_path_is_url(local_abspath))
goto invalid_rev_arg;
- SVN_ERR(svn_wc__get_entry_versioned(&ent, wc_ctx, local_abspath,
- svn_node_unknown, FALSE, FALSE,
- scratch_pool, scratch_pool));
-
- if (! SVN_IS_VALID_REVNUM(ent->cmt_rev))
+ SVN_ERR(svn_wc__node_get_changed_info(revnum, NULL, NULL,
+ wc_ctx, local_abspath,
+ scratch_pool, scratch_pool));
+ if (! SVN_IS_VALID_REVNUM(*revnum))
return svn_error_createf(SVN_ERR_CLIENT_BAD_REVISION, NULL,
_("Path '%s' has no committed "
"revision"), local_abspath);
- *revnum = ent->cmt_rev;
+
if (revision->kind == svn_opt_revision_previous)
(*revnum)--;
}
Index: subversion/libsvn_wc/node.c
===================================================================
--- subversion/libsvn_wc/node.c (revision 929082)
+++ subversion/libsvn_wc/node.c (working copy)
@@ -626,6 +626,41 @@ 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));
+
+ /* 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));
+ }
+
+ return SVN_NO_ERROR;
+}
+
+svn_error_t *
svn_wc__node_get_lock_token(const char **lock_token,
svn_wc_context_t *wc_ctx,
const char *local_abspath,
Index: subversion/tests/cmdline/diff_tests.py
===================================================================
--- subversion/tests/cmdline/diff_tests.py (revision 929082)
+++ subversion/tests/cmdline/diff_tests.py (working copy)
@@ -568,7 +568,7 @@ def diff_non_version_controlled_file(sbo
# there was a way to figure out if svn crashed, but all run_svn gives us is
# the output, so here we are...
for line in err_output:
- if re.search("foo' is not under version control$", line):
+ if re.search("foo' was not found.$", line):
break
else:
raise svntest.Failure
]]]
neels wrote:
> Hi all,
>
> I have a patch that passes 'make check', but changes the behaviour of
> svn_client__get_revision_number() in ways that surpass my current
> understanding. Previously, this function would follow the
> overloadedness of entry->revision, in ways not entirely clear to me.
>
> read_entries_new() does these things WRT entry->revision:
> - get the revision from __read_info()
> - if status_added, pick up the parent node's revision instead
> - but if base_shadowed, use the NG-BASE node's revision.
> - if not_present, use NG-BASE node's revision
> - if plain add, set revision = 0
> - if obstructed_add, set revision = SVN_INVALID_REVNUM
> - if copied, set revision to scan_addition's ORIGINAL_REVISION
>
> When svn_client__get_revision_number() was called with
> svn_opt_revision_base or svn_opt_revision_working, it just returned
> the entry->revision. Frankly, it's a bloody nightmare to figure out
> what happened when, exactly.
>
> Instead, I intend to implement svn_client__get_revision_number() with
> svn_opt_revision_base | svn_opt_revision_working like this:
> return the commit-base (not the revert-base) revision. i.e., return
> the NG-BASE node's revision, but:
> if the node is copied here, return the copy_from revision;
> if the node is plainly added, even if the node is replaced by a
> plain add, return SVN_INVALID_REVNUM.
>
> Attached patch implements that AFAICT, but I note some behaviour
> changes (I did a dumb printf() of every revision returned and compared
> trunk's make check output to my patch)
> - Added nodes now return revision number -1 instead of 0 (expected)
> - Copied-here nodes seem to have sometimes returned an earlier
> revision than the copy_from revision that this patch returns
> (unexpected to me)
>
> Note once more that the patch passes 'make check'. But I am unsure
> whether I really understand what I'm doing. Comments welcome.
>
> Less importantly, another change is in the returned error message. If
> the function was called on a node not under version control, it would
> issue an SVN_ERR_WC_PATH_NOT_FOUND with error string "'foo' is not
> under version control". With this patch, svn still returns the same
> error code, but an error message of "'foo' was not found." (see the
> change of diff_tests.py in attached patch) -- I'm wondering if I
> should mangle the error message to be identical to previous behaviour.
> The old message makes slightly more sense to humans in this case,
> right?
>
> Thanks,
> ~Neels
signature.asc
Description: OpenPGP digital signature
