Daniel Shahaf <d...@daniel.shahaf.name> writes:
> Noorul Islam K M wrote on Thu, Feb 03, 2011 at 14:15:48 +0530:
>
>>
>> Attached is the patch to fix issue #3792. Please review and let me know
>> your comments.
>>
>> Log
>> [[[
>>
>> Fix for issue #3792. Make svn info to display information for
>> excluded items.
>>
>
> s/to//
>
Thank you Daniel for review comments.
Incorporated
>> * subversion/include/private/svn_wc_private.h,
>> subversion/libsvn_wc/node.c
>> (svn_wc__node_depth_is_exclude): New helper function to find
>> whether the node is set with depth 'exclude'.
>>
>> * subversion/libsvn_client/info.c
>> (crawl_entries): Build minimal information if the node has
>> depth set as 'exclude'.
>>
>> * subversion/svn/info-cmd.c
>> (print_info): Print depth as 'exclude' for nodes having depth
>> exclude. Hide node kind while printing exclude infomation.
>>
>
> It's okay to print the node kind if it's known. (and it probably is)
>
Incorporated.
>> * subversion/tests/cmdline/depth_tests.py
>> (test_list): Remove XFail marker from info_excluded test.
>>
>> Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
>> ]]]
>>
>
>> Index: subversion/tests/cmdline/depth_tests.py
>> ===================================================================
>> --- subversion/tests/cmdline/depth_tests.py (revision 1066732)
>> +++ subversion/tests/cmdline/depth_tests.py (working copy)
>> @@ -2841,7 +2841,7 @@
>> excluded_path_misc_operation,
>> excluded_receive_remote_removal,
>> exclude_keeps_hidden_entries,
>> - XFail(info_excluded, issues=3792),
>> + info_excluded,
>> tree_conflicts_resolved_depth_empty,
>> tree_conflicts_resolved_depth_files,
>> tree_conflicts_resolved_depth_immediates,
>> Index: subversion/svn/info-cmd.c
>> ===================================================================
>> --- subversion/svn/info-cmd.c (revision 1066732)
>> +++ subversion/svn/info-cmd.c (working copy)
>> @@ -291,7 +291,8 @@
>> break;
>>
>> case svn_node_none:
>> - SVN_ERR(svn_cmdline_printf(pool, _("Node Kind: none\n")));
>> + if (info->depth != svn_depth_exclude)
>> + SVN_ERR(svn_cmdline_printf(pool, _("Node Kind: none\n")));
>
> That doesn't look right. Does the DB know the node kind of an excluded
> node? If so, it should print it; if not, it should return "unknown".
>
>> break;
>>
>> case svn_node_unknown:
>> @@ -363,6 +364,9 @@
>> SVN_ERR(svn_cmdline_printf(pool, _("Copied From Rev: %ld\n"),
>> info->copyfrom_rev));
>> }
>> +
>> + if (info->depth == svn_depth_exclude)
>> + SVN_ERR(svn_cmdline_printf(pool, _("Depth: exclude\n")));
>>
>
> I know that there is already some other place in the code that prints
> the Depth: line, why didn't you reuse that place?
>
It is inside the if block 'if (info->has_wc_info)'. In our case flow
will not reach there so I had to put it here.
>> if (info->last_changed_author)
>> SVN_ERR(svn_cmdline_printf(pool, _("Last Changed Author: %s\n"),
>> Index: subversion/include/private/svn_wc_private.h
>> ===================================================================
>> --- subversion/include/private/svn_wc_private.h (revision 1066732)
>> +++ subversion/include/private/svn_wc_private.h (working copy)
>> @@ -759,6 +759,15 @@
>> apr_pool_t *result_pool,
>> apr_pool_t *scratch_pool);
>>
>> +/**
>> + * Find whether @a local_abspath is set with depth exclude using @a wc_ctx.
>> + */
>> +svn_error_t *
>> +svn_wc__node_depth_is_exclude(svn_boolean_t *exclude,
>> + svn_wc_context_t *wc_ctx,
>> + const char *local_abspath,
>> + apr_pool_t *scratch_pool);
>> +
>> #ifdef __cplusplus
>> }
>> #endif /* __cplusplus */
>> Index: subversion/libsvn_wc/node.c
>> ===================================================================
>> --- subversion/libsvn_wc/node.c (revision 1066732)
>> +++ subversion/libsvn_wc/node.c (working copy)
>> @@ -247,6 +247,30 @@
>> }
>>
>> svn_error_t *
>> +svn_wc__node_depth_is_exclude(svn_boolean_t *exclude,
>> + svn_wc_context_t *wc_ctx,
>> + const char *local_abspath,
>> + apr_pool_t *scratch_pool)
>> +{
>> + svn_wc__db_status_t status;
>> + svn_error_t *err;
>> +
>> + *exclude = FALSE;
>> +
>> + 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,
>> + wc_ctx->db, local_abspath, scratch_pool,
>> + scratch_pool);
>> +
>> + if ((! err) && (status == svn_wc__db_status_excluded))
>> + *exclude = TRUE;
>> +
>> + return svn_error_return(err);
>
> Haven't reviewed this part, but I'll have to do so before committing.
>
Ok
>> +}
>> +
>> +svn_error_t *
>> svn_wc__node_get_depth(svn_depth_t *depth,
>> svn_wc_context_t *wc_ctx,
>> const char *local_abspath,
>> Index: subversion/libsvn_client/info.c
>> ===================================================================
>> --- subversion/libsvn_client/info.c (revision 1066732)
>> +++ subversion/libsvn_client/info.c (working copy)
>> @@ -405,20 +405,31 @@
>> /* Check for a tree conflict on the root node of the info, and if
>> there
>> is one, send a minimal info struct. */
>> const svn_wc_conflict_description2_t *tree_conflict;
>> + svn_boolean_t exclude = FALSE;
>>
>
> You don't need to (and shouldn't) initialize this variable.
>
My bad.
>> SVN_ERR(svn_wc__get_tree_conflict(&tree_conflict, ctx->wc_ctx,
>> local_abspath, pool, pool));
>>
>> - if (tree_conflict)
>> + /* Check whether depth of the node is set as exclude. */
>> + if (! tree_conflict)
>> + SVN_ERR(svn_wc__node_depth_is_exclude(&exclude, ctx->wc_ctx,
>> + local_abspath, pool));
>> +
>> + if (tree_conflict || exclude)
>> {
>> svn_info_t *info;
>> svn_error_clear(err);
>>
>> SVN_ERR(build_info_for_unversioned(&info, pool));
>> - info->tree_conflict = svn_wc__cd2_to_cd(tree_conflict, pool);
>>
>> + if (tree_conflict)
>> + info->tree_conflict = svn_wc__cd2_to_cd(tree_conflict, pool);
>> + else
>> + info->depth = svn_depth_exclude;
>
> s/else/if (exclude)/, please.
>
There is actually an outer if condition which makes this not useful. But
anyhow I changed it.
>> +
>> SVN_ERR(svn_wc__node_get_repos_info(&(info->repos_root_URL),
>> - NULL,
>> + exclude ?
>> + &(info->repos_UUID) : NULL,
>
> Why?
>
I thought I should not make changes to existing behaviour. I think it is
safe to just pass &(info->repos_UUID) in both cases.
>> ctx->wc_ctx,
>> local_abspath, FALSE, FALSE,
>> pool, pool));
Here is the updated log and patch. All tests pass.
Log
[[[
Fix for issue #3792. Make svn info display information for
excluded items.
* subversion/include/private/svn_wc_private.h,
subversion/libsvn_wc/node.c
(svn_wc__node_depth_is_exclude): New helper function to find
whether the node is set with depth 'exclude'.
* subversion/libsvn_client/info.c
(crawl_entries): Build minimal information if the node has
depth set as 'exclude'.
* subversion/svn/info-cmd.c
(print_info): Print depth as 'exclude' for nodes having depth
exclude.
* subversion/tests/cmdline/depth_tests.py
Remove XFail decorator from info_excluded test.
Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
]]]
Index: subversion/tests/cmdline/depth_tests.py
===================================================================
--- subversion/tests/cmdline/depth_tests.py (revision 1067395)
+++ subversion/tests/cmdline/depth_tests.py (working copy)
@@ -2349,8 +2349,6 @@
# Issue 3792.
-@XFail()
-@Issue(3792)
def info_excluded(sbox):
"'info' should treat excluded item as versioned"
Index: subversion/svn/info-cmd.c
===================================================================
--- subversion/svn/info-cmd.c (revision 1067395)
+++ subversion/svn/info-cmd.c (working copy)
@@ -363,6 +363,9 @@
SVN_ERR(svn_cmdline_printf(pool, _("Copied From Rev: %ld\n"),
info->copyfrom_rev));
}
+
+ if (info->depth == svn_depth_exclude)
+ SVN_ERR(svn_cmdline_printf(pool, _("Depth: exclude\n")));
if (info->last_changed_author)
SVN_ERR(svn_cmdline_printf(pool, _("Last Changed Author: %s\n"),
Index: subversion/include/private/svn_wc_private.h
===================================================================
--- subversion/include/private/svn_wc_private.h (revision 1067395)
+++ subversion/include/private/svn_wc_private.h (working copy)
@@ -759,6 +759,15 @@
apr_pool_t *result_pool,
apr_pool_t *scratch_pool);
+/**
+ * Find whether @a local_abspath is set with depth exclude using @a wc_ctx.
+ */
+svn_error_t *
+svn_wc__node_depth_is_exclude(svn_boolean_t *exclude,
+ svn_wc_context_t *wc_ctx,
+ const char *local_abspath,
+ apr_pool_t *scratch_pool);
+
#ifdef __cplusplus
}
#endif /* __cplusplus */
Index: subversion/libsvn_wc/node.c
===================================================================
--- subversion/libsvn_wc/node.c (revision 1067395)
+++ subversion/libsvn_wc/node.c (working copy)
@@ -247,6 +247,30 @@
}
svn_error_t *
+svn_wc__node_depth_is_exclude(svn_boolean_t *exclude,
+ svn_wc_context_t *wc_ctx,
+ const char *local_abspath,
+ apr_pool_t *scratch_pool)
+{
+ svn_wc__db_status_t status;
+ svn_error_t *err;
+
+ *exclude = FALSE;
+
+ 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,
+ wc_ctx->db, local_abspath, scratch_pool,
+ scratch_pool);
+
+ if ((! err) && (status == svn_wc__db_status_excluded))
+ *exclude = TRUE;
+
+ return svn_error_return(err);
+}
+
+svn_error_t *
svn_wc__node_get_depth(svn_depth_t *depth,
svn_wc_context_t *wc_ctx,
const char *local_abspath,
Index: subversion/libsvn_client/info.c
===================================================================
--- subversion/libsvn_client/info.c (revision 1067395)
+++ subversion/libsvn_client/info.c (working copy)
@@ -405,20 +405,30 @@
/* Check for a tree conflict on the root node of the info, and if there
is one, send a minimal info struct. */
const svn_wc_conflict_description2_t *tree_conflict;
+ svn_boolean_t exclude;
SVN_ERR(svn_wc__get_tree_conflict(&tree_conflict, ctx->wc_ctx,
local_abspath, pool, pool));
- if (tree_conflict)
+ /* Check whether depth of the node is set as exclude. */
+ if (! tree_conflict)
+ SVN_ERR(svn_wc__node_depth_is_exclude(&exclude, ctx->wc_ctx,
+ local_abspath, pool));
+
+ if (tree_conflict || exclude)
{
svn_info_t *info;
svn_error_clear(err);
SVN_ERR(build_info_for_unversioned(&info, pool));
- info->tree_conflict = svn_wc__cd2_to_cd(tree_conflict, pool);
+ if (tree_conflict)
+ info->tree_conflict = svn_wc__cd2_to_cd(tree_conflict, pool);
+ else if (exclude)
+ info->depth = svn_depth_exclude;
+
SVN_ERR(svn_wc__node_get_repos_info(&(info->repos_root_URL),
- NULL,
+ &(info->repos_UUID),
ctx->wc_ctx,
local_abspath, FALSE, FALSE,
pool, pool));
