Hi Brane,
I don’t think the BASE optimization into svn_wc__db_read_children_info() is
required to obtain good performance. In most working copies (I would hope :))
most nodes are status normal so the normal call will obtain the information you
need. And as we don’t report symlink vs file in the status output (and no
last_changed_* for deleted) there would just be an additional
svn_wc__db_base_get_info() on replacements.
(Which the ‘svn’ client already has… but that is a different story)
Otherwise +1 on using a different query with ‘AND op_depth=0’.
(Adding a Boolean as flag to the query breaks the sqlite optimizer for the
query as it only performs optimizing once, before it knows the actual values
passed)
The only reason we obtain the special flag is to show if a file is obstructing
a symlink or vice versa, and in your case we don’t want to report that, so
setting special to FALSE when handling things from status should ‘just work’.
(If you implement this in wc_db.c I would say that we should obtain the proper
value)
The remaining interesting case would be when we see obstructing working copies…
as in that case we don’t support reading the nodes (and their children) by
their abspaths.
Personally I wouldn’t be surprised if you could obtain the required performance
for this feature by just skipping all file comparisions (and the individual
filestats where necessary), instead of implementing the whole ignore everything
local work.
I don’t think reading the directory entries per directory is that expensive…
Subversion <= 1.6 read them per node during status processing… That was really
expensive on Windows. But it could be that reading all the directory details is
expensive on !Windows now, but perhaps it might help to pass TRUE for
only_check_type on svn_io_get_dirents3().
(On Windows that doesn’t change the performance, but I think it does on other
platforms)
Bert
From: Branko Čibej [mailto:br...@wandisco.com]
Sent: dinsdag 1 april 2014 13:10
To: dev@subversion.apache.org
Subject: Re: svn commit: r1583599 - in
/subversion/branches/remote-only-status/subversion: include/svn_client.h
libsvn_wc/status.c tests/libsvn_client/client-test.c
On 01.04.2014 12:54, Bert Huijben wrote:
-----Original Message-----
From: br...@apache.org <mailto:br...@apache.org> [mailto:br...@apache.org]
Sent: dinsdag 1 april 2014 12:41
To: comm...@subversion.apache.org <mailto:comm...@subversion.apache.org>
Subject: svn commit: r1583599 - in /subversion/branches/remote-only-
status/subversion: include/svn_client.h libsvn_wc/status.c
tests/libsvn_client/client-test.c
Author: brane
Date: Tue Apr 1 10:41:29 2014
New Revision: 1583599
URL: http://svn.apache.org/r1583599
Log:
On the remote-only-status branch: Do not report local replacements.
* subversion/include/svn_client.h (svn_client_status6): Update docstring.
* subversion/libsvn_wc/status.c (assemble_status): Ignore local adds.
Report local replacements as deletions of the working node.
(get_dir_status): Remove redundant (and incorrect) filter for additions.
* subversion/tests/libsvn_client/client-test.c (test_remote_only_status):
Extend test case with an example of a local replacement.
Modified:
subversion/branches/remote-only-status/subversion/include/svn_client.h
subversion/branches/remote-only-status/subversion/libsvn_wc/status.c
subversion/branches/remote-only-
status/subversion/tests/libsvn_client/client-test.c
Modified: subversion/branches/remote-only-
status/subversion/include/svn_client.h
URL: http://svn.apache.org/viewvc/subversion/branches/remote-only-
status/subversion/include/svn_client.h?rev=1583599&r1=1583598&r2=15835
99&view=diff
==========================================================
====================
--- subversion/branches/remote-only-
status/subversion/include/svn_client.h (original)
+++ subversion/branches/remote-only-
status/subversion/include/svn_client.h Tue Apr 1 10:41:29 2014
@@ -2512,6 +2512,13 @@ typedef svn_error_t *(*svn_client_status
* - If @a check_working_copy is not set, do not scan the working
* copy for locally modified and missing files. This parameter
* will be ignored unless @a check_out_of_date is set.
+ * When set, the status report will be different in the following
+ * details:
+ *
+ * -- Local modifications, missing nodes and locally added nodes
+ * will not be reported.
+ * -- Locally replaced nodes will be reported as deletions of
+ * the original node instead of as replacements.
*
* If @a no_ignore is @c FALSE, don't report any file or directory (or
* recurse into any directory) that is found by recursion (as opposed to
Modified: subversion/branches/remote-only-
status/subversion/libsvn_wc/status.c
URL: http://svn.apache.org/viewvc/subversion/branches/remote-only-
status/subversion/libsvn_wc/status.c?rev=1583599&r1=1583598&r2=158359
9&view=diff
==========================================================
====================
--- subversion/branches/remote-only-status/subversion/libsvn_wc/status.c
(original)
+++ subversion/branches/remote-only-
status/subversion/libsvn_wc/status.c Tue Apr 1 10:41:29 2014
@@ -573,7 +573,42 @@ assemble_status(svn_wc_status3_t **statu
if (below_working != svn_wc__db_status_not_present
&& below_working != svn_wc__db_status_deleted)
{
- node_status = svn_wc_status_replaced;
+ if (check_working_copy)
+ node_status = svn_wc_status_replaced;
+ else
+ {
+ /* This is a remote-only walk; report the
+ base node info instead of the replacement. */
+ const char *target;
+ const svn_checksum_t *checksum;
+ struct svn_wc__db_info_t *base_info =
+ apr_palloc(scratch_pool, sizeof(*base_info));
+ memcpy(base_info, info, sizeof(*base_info));
+ SVN_ERR(svn_wc__db_read_pristine_info(
+ &base_info->status,
+ &base_info->kind,
+ &base_info->changed_rev,
+ &base_info->changed_date,
+ &base_info->changed_author,
+ &base_info->depth,
+ &checksum, &target,
+ &base_info->had_props, NULL,
+ db, local_abspath,
+ scratch_pool, scratch_pool));
+ SVN_ERR(svn_wc__db_base_get_info(
+ NULL, NULL, &base_info->revnum,
+ NULL, NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ db, local_abspath,
+ scratch_pool, scratch_pool));
If you really want the repository information you should read all the values
using svn_wc__db_base_get_info as the changed* values read by
svn_wc__db_read_pristine_info are those of the highest layer...
Only in case of a BASE-delete (not in case of a working delete, or a
replacement!) do they represent the information you want.
+ base_info->has_checksum = (checksum != NULL);
+#ifdef HAVE_SYMLINK
+ base_info->special = (target != NULL);
Target is not used (yet)... you must read the properties to determine if a node
is a symlink or not. I think you can get the property values from
svn_wc__db_base_get_info() now.
+#endif
Bert
+ node_status = svn_wc_status_deleted;
+ info = base_info;
+ }
}
else
node_status = svn_wc_status_added;
@@ -610,6 +645,16 @@ assemble_status(svn_wc_status3_t **statu
&& prop_status != svn_wc_status_none)
node_status = prop_status;
+
+ /* Ignore local additions in remote-only mode */
+ if (!check_working_copy
+ && node_status == svn_wc_status_added
+ && !moved_from_abspath)
+ {
+ *status = NULL;
+ return SVN_NO_ERROR;
+ }
I don't think this really checks what you want to check here... I would guess
you want to check the have_base value (too?), as replaced nodes will also have
an added status.
(And even with that you might still miss a few edge cases in case parent
directories are replaced with files, or the other way around)
Bert
Thanks for the review, again!
I'm actually thinking that this is really a hack, and I should just modify
svn_wc__db_read_single_info and svn_wc__db_read_children_info to be aware of
the remote-only flag. That's where the BASE->WORKING->ACTUAL overlay happens,
and what I'm doing here is basically just trying to reproduce part of the
result, which seems like a waste of code. IIUC, if I get the modifications just
right, the additions and replacements won't show up in the results anyway, and
I'll be able to revert this latest commit, and not have a higher-level filter
for added nodes.
-- Brane
--
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. br...@wandisco.com <mailto:br...@wandisco.com>
