[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] - Fix for issue #3792

From: Noorul Islam K M <noorul_at_collab.net>
Date: Sat, 05 Feb 2011 12:55:55 +0530

Daniel Shahaf <d.s_at_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.
-_at_XFail()
-_at_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));
Received on 2011-02-05 08:27:41 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.