Hi!
Here's my new patch.
On Tue, Mar 09, 2010 at 12:52:23AM +0100, Bert Huijben wrote:
> > Several points:
> > * use svn_wc__internal_text_modified_p(); then you won't need wc_ctx
> > (just the db)
Done!
> > * read_info can return NULL for the repos_* values
I'm checking that the values are null before setting using them for
svn_uri_join().
> > * typo in analyze_status docstring (the typename)
Fixed!
> > * status_deleted nodes have no revision info; you may simply want to
> > test for SVN_INVALID_REVNUM, since read_info() will return that when
> > it is "not interesting". IOW, it does the proper work for you
I'm checking that revision is not SVN_INVALID_REVNUM. As I've understood
it, that happens when we have something in WORKING, it has been deleted
or added and in that case there's no revision info for us to use. Right?
> > * do you need wc_root? how about just using
> > svn_wc__internal_path_switched() ?
I first tried to use the child_disjoint.*() func of libsvn_wc/lock.c but
it felt strange to use that code when we already have
svn_wc__check_wc_root() that checks if a node is switched. I can
separate the switch part into a separate func in a follow up and
replace the call to svn_wc__check_wc_root() with that one then. But I'm
so terribly slow at getting my patches finished that I thought it was
best to use _check_wc_root() as is for now.
> * You could optimize the checking for modifications to stop checking when
> you find the first change. (You are not reporting specific changes)
Done that!
I've removed the comments in the end about sparse_checkout not detecting
all cases. The walker reaches all nodes beneath local_abspath and if we
check for absent too I can't come up with any more cases where we won't
detect sparse_checkouts. Is that assumption correct?
I'm catching the _WC_PATH_NOT_FOUND error. Before my changes the code
didn't return that error. BUT, is that the _right_ thing to do? Are we
counting on the svn_wc_.* functions to always return _WC_PATH_NOT_FOUND
error for unversioned resources and such? Since it's a new revision I
could just as well return it, right?
[[[
As part of WC-NG, remove some uses of svn_wc_entry_t.
* subversion/libsvn_wc/revision_status.c
(status_baton): Rename this...
(walk_baton): .. to this.
(analyze_status): Change this to implement svn_wc__node_func_t and use
WC-NG funcs instead of information in svn_wc_entry_t.
(svn_wc_revision_status2): Use svn_wc__node_walk_children() instead of
svn_wc_walk_status() to spare us from the overhead of invoking an
editor.
]]]
Daniel
Index: subversion/libsvn_wc/revision_status.c
===================================================================
--- subversion/libsvn_wc/revision_status.c (revision 921239)
+++ subversion/libsvn_wc/revision_status.c (arbetskopia)
@@ -23,65 +23,109 @@
#include "svn_wc.h"
#include "svn_dirent_uri.h"
+#include "wc_db.h"
+#include "wc.h"
+#include "props.h"
#include "private/svn_wc_private.h"
#include "svn_private_config.h"
/* A baton for analyze_status(). */
-struct status_baton
+struct walk_baton
{
svn_wc_revision_status_t *result; /* where to put the result */
svn_boolean_t committed; /* examine last committed revisions */
const char *local_abspath; /* path whose URL we're looking for */
const char *wc_url; /* URL for the path whose URL we're looking for */
+ svn_wc__db_t *db;
apr_pool_t *pool; /* pool in which to store alloc-needy things */
};
-/* An svn_wc_status_func4_t callback function for analyzing status
- structures. */
+/* An svn_wc__node_found_func_t callback function for analyzing the status
+ * of nodes */
static svn_error_t *
-analyze_status(void *baton,
- const char *local_abspath,
- const svn_wc_status2_t *status,
- apr_pool_t *pool)
+analyze_status(const char *local_abspath,
+ void *baton,
+ apr_pool_t *scratch_pool)
{
- struct status_baton *sb = baton;
+ struct walk_baton *wb = baton;
+ svn_revnum_t changed_rev;
+ svn_revnum_t revision;
+ svn_depth_t depth;
+ svn_wc__db_status_t status;
+ const char *repos_root;
+ const char *repos_relpath;
+ svn_boolean_t wc_root;
+ svn_boolean_t switched;
- if (! status->entry)
+ SVN_ERR(svn_wc__db_read_info(&status, NULL, &revision, &repos_relpath,
+ &repos_root, NULL, &changed_rev,
+ NULL, NULL, NULL, &depth, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL, wb->db,
+ local_abspath, scratch_pool, scratch_pool));
+
+ /* We need the excluded and absent paths when checking for sparse
+ * checkouts. But only for that. To collect those we're walking all the
+ * hidden nodes. */
+ if (status == svn_wc__db_status_excluded
+ || status == svn_wc__db_status_absent)
+ {
+ wb->result->sparse_checkout |= TRUE;
+ return SVN_NO_ERROR;
+ }
+
+ if (status == svn_wc__db_status_not_present)
return SVN_NO_ERROR;
+ SVN_ERR(svn_wc__check_wc_root(&wc_root, NULL, &switched, wb->db,
+ local_abspath, scratch_pool));
+
+ wb->result->switched |= switched;
+
/* Added files have a revision of no interest */
- if (status->text_status != svn_wc_status_added)
+ if (revision != SVN_INVALID_REVNUM)
{
- svn_revnum_t item_rev = (sb->committed
- ? status->entry->cmt_rev
- : status->entry->revision);
+ svn_revnum_t item_rev = (wb->committed
+ ? changed_rev
+ : revision);
- if (sb->result->min_rev == SVN_INVALID_REVNUM
- || item_rev < sb->result->min_rev)
- sb->result->min_rev = item_rev;
+ if (wb->result->min_rev == SVN_INVALID_REVNUM
+ || item_rev < wb->result->min_rev)
+ wb->result->min_rev = item_rev;
- if (sb->result->max_rev == SVN_INVALID_REVNUM
- || item_rev > sb->result->max_rev)
- sb->result->max_rev = item_rev;
+ if (wb->result->max_rev == SVN_INVALID_REVNUM
+ || item_rev > wb->result->max_rev)
+ wb->result->max_rev = item_rev;
}
- if (status->entry->depth != svn_depth_exclude)
+ if (! wb->result->modified)
{
- sb->result->switched |= status->switched;
- sb->result->modified |= (status->text_status != svn_wc_status_normal);
- sb->result->modified |= (status->prop_status != svn_wc_status_normal
- && status->prop_status != svn_wc_status_none);
+ svn_boolean_t text_mod;
+ svn_boolean_t props_mod;
+
+ SVN_ERR(svn_wc__props_modified(&props_mod, wb->db, local_abspath,
+ scratch_pool));
+
+ SVN_ERR(svn_wc__internal_text_modified_p(&text_mod, wb->db,
+ local_abspath,
+ FALSE,
+ TRUE,
+ scratch_pool));
+ wb->result->modified |= (text_mod || props_mod);
}
- sb->result->sparse_checkout |= (status->entry->depth != svn_depth_infinity);
- if (sb->local_abspath
- && (! sb->wc_url)
- && (strcmp(local_abspath, sb->local_abspath) == 0)
- && (status->entry))
- sb->wc_url = apr_pstrdup(sb->pool, status->entry->url);
+ wb->result->sparse_checkout |= ((depth != svn_depth_infinity
+ && depth != svn_depth_unknown));
+ if (wb->local_abspath
+ && repos_root
+ && repos_relpath
+ && (! wb->wc_url)
+ && (strcmp(local_abspath, wb->local_abspath) == 0))
+ wb->wc_url = svn_uri_join(repos_root, repos_relpath, wb->pool);
+
return SVN_NO_ERROR;
}
@@ -96,7 +140,9 @@
apr_pool_t *result_pool,
apr_pool_t *scratch_pool)
{
- struct status_baton sb;
+ struct walk_baton wb;
+ svn_wc__db_kind_t kind;
+ svn_error_t *err;
svn_wc_revision_status_t *result = apr_palloc(result_pool, sizeof(*result));
*result_p = result;
@@ -109,24 +155,36 @@
result->modified = FALSE;
result->sparse_checkout = FALSE;
+ /* Rule out the possibility of local_abspath not beeing a versioned path
+ */
+ err = svn_wc__db_read_kind(&kind, wc_ctx->db, local_abspath, FALSE,
+ scratch_pool);
+
+ if (err)
+ {
+ if (err->apr_err == SVN_ERR_WC_PATH_NOT_FOUND)
+ {
+ svn_error_clear(err);
+ return SVN_NO_ERROR;
+ }
+ else
+ SVN_ERR(err);
+
/* initialize walking baton */
- sb.result = result;
- sb.committed = committed;
- sb.local_abspath = local_abspath;
- sb.wc_url = NULL;
- sb.pool = scratch_pool;
+ wb.result = result;
+ wb.committed = committed;
+ wb.local_abspath = local_abspath;
+ wb.wc_url = NULL;
+ wb.db = wc_ctx->db;
+ wb.pool = scratch_pool;
- SVN_ERR(svn_wc_walk_status(wc_ctx,
- local_abspath,
- svn_depth_infinity,
- TRUE /* get_all */,
- FALSE /* no_ignore */,
- TRUE, /* get_excluded */
- NULL /* ignore_patterns */,
- analyze_status, &sb,
- NULL, NULL,
- cancel_func, cancel_baton,
- scratch_pool));
+ SVN_ERR(svn_wc__node_walk_children(wc_ctx,
+ local_abspath,
+ TRUE /* show_hidden */,
+ analyze_status, &wb,
+ svn_depth_infinity,
+ cancel_func, cancel_baton,
+ scratch_pool));
if ((! result->switched) && (trail_url != NULL))
@@ -134,26 +192,18 @@
/* If the trailing part of the URL of the working copy directory
does not match the given trailing URL then the whole working
copy is switched. */
- if (! sb.wc_url)
+ if (! wb.wc_url)
{
result->switched = TRUE;
}
else
{
apr_size_t len1 = strlen(trail_url);
- apr_size_t len2 = strlen(sb.wc_url);
- if ((len1 > len2) || strcmp(sb.wc_url + len2 - len1, trail_url))
+ apr_size_t len2 = strlen(wb.wc_url);
+ if ((len1 > len2) || strcmp(wb.wc_url + len2 - len1, trail_url))
result->switched = TRUE;
}
}
- /* ### 1.6+: If result->sparse_checkout is FALSE the answer is not final
- We can still have excluded nodes!
-
- ### TODO: Check every node below local_abspath for excluded
-
- ### BH: What about absent? You don't know if you have a complete tree
- without checking for absent too */
-
return SVN_NO_ERROR;
}
