You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Noorul Islam K M <no...@collab.net> on 2011/04/07 06:47:49 UTC
Re: [PATCH] Fix for issue 3787
Stefan Sperling <st...@elego.de> writes:
> On Thu, Mar 10, 2011 at 04:49:32PM +0530, Noorul Islam K M wrote:
>
>>
>> >From issue tracker
>> (http://subversion.tigris.org/issues/show_bug.cgi?id=3787)
>>
>> It would be ever-so-helpful to folks looking to capture and replicate a working
>> copy sparse checkouts configuration if 'svn info -R' would show exclude items,
>> if only optionally, and even then if only just with minimal info:
>>
>> Path: some/excluded-path
>> Depth: exclude
>>
>> Today you cannot look at 'svn info -R' output and get a complete picture of what
>> is and isn't in the sparse configuration.
>> --------------------------------------------------------------------------------
>>
>> I added new option "--show-exclude" to 'svn info' subcommand. When this
>> option is passed to info command along with '-R', it will include nodes
>> with depth 'exclude' also for displaying information. This issue is
>> related to 3792 which is already fixed.
>
> I don't think the new --show-exclude option is necessary.
> We should always show information about excluded items in the output
> svn info -R.
>
> Can you rework the patch accordingly? That will also make it smaller.
>
Please find updated patch. All tests pass using 'make check'.
Log
[[[
Fix for issue 3787. Make 'svn info -R' display minimal information about
nodes with depth exclude in the tree.
* subversion/libsvn_client/info.c
(crawl_entries): If depth is infinity or files then pass TRUE to
svn_wc__node_walk_children as show_hidden parameter value.
* subversion/tests/cmdline/depth_tests.py
(info_show_exclude): New test.
(test_list): Run it.
Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
]]]
Re: [PATCH] Fix for issue 3787
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Noorul Islam K M wrote on Thu, Apr 07, 2011 at 10:17:49 +0530:
> Index: subversion/tests/cmdline/depth_tests.py
> ===================================================================
> --- subversion/tests/cmdline/depth_tests.py (revision 1089373)
> +++ subversion/tests/cmdline/depth_tests.py (working copy)
> @@ -2815,7 +2815,38 @@
> None, None, None, None, None, False,
> '--parents', omega_path)
>
> +@Issue(3787)
> +def info_show_exclude(sbox):
> + "tests 'info -R' on excluded directory"
>
Nit: usually we use the base form of the verb, i.e., s/tests/test/.
> + sbox.build()
> + wc_dir = sbox.wc_dir
> +
You can pass READ_ONLY to sbox.build().
> + A_path = os.path.join(wc_dir, 'A')
> + svntest.main.run_svn(None, 'up', '--set-depth', 'exclude', A_path)
> +
> + expected_info = [{
> + 'Path' : '.',
> + 'Repository Root' : sbox.repo_url,
> + 'Repository UUID' : svntest.actions.get_wc_uuid(wc_dir),
> + }]
> +
get_wc_uuid() is expensive, please call it just once, it will return the
same value each time...
> + expected_info.append({
> + 'Path' : 'A',
> + 'Repository Root' : sbox.repo_url,
> + 'Repository UUID' : svntest.actions.get_wc_uuid(wc_dir),
> + 'Depth' : 'exclude',
> + })
> +
> + expected_info.append({
> + 'Path' : re.escape("iota"),
> + 'Repository Root' : sbox.repo_url,
> + 'Repository UUID' : svntest.actions.get_wc_uuid(wc_dir),
> + })
> +
> + os.chdir(wc_dir)
> + svntest.actions.run_and_verify_info(expected_info, '-R')
> +
> #----------------------------------------------------------------------
> # list all tests here, starting with None:
> test_list = [ None,
> @@ -2862,6 +2893,7 @@
> update_excluded_path_sticky_depths,
> update_depth_empty_root_of_infinite_children,
> sparse_update_with_dash_dash_parents,
> + info_show_exclude,
> ]
Re: [PATCH] Fix for issue 3787
Posted by Noorul Islam K M <no...@collab.net>.
Noorul Islam K M <no...@collab.net> writes:
> "Bert Huijben" <be...@qqmail.nl> writes:
>
>>> -----Original Message-----
>>> From: Noorul Islam K M [mailto:noorul@collab.net]
>>> Sent: maandag 4 juli 2011 18:05
>>> To: Daniel Shahaf
>>> Cc: Subversion
>>> Subject: Re: [PATCH] Fix for issue 3787
>>
>>> [[[
>>>
>>> Fix for issue 3787. Make 'svn info --depth' variants display minimal
>>> information about nodes with depth exclude in the tree.
>>>
>>> * subversion/libsvn_client/info.c
>>> (crawl_entries): Pass TRUE to svn_wc__node_walk_children() as
>>> show_hidden parameter value.
>>>
>>> * subversion/tests/cmdline/depth_tests.py
>>> (info_show_exclude): New test.
>>> (test_list): Run it.
>>>
>>> Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
>>>
>>> ]]]
>>
>> (Please look in your irc log. We talked about this on IRC before)
>>
>> This simple fix makes that the code also gets not-present and server
>> excluded nodes. But the info api (and the output) don't handle these cases.
>>
>> Switching the 'show hidden' flag requires more work in the callback to hide
>> all unexpected hidden kinds.
>>
>> Bert
>
> In those cases build_info_for_entry() is throwing error. Is that not
> good enough?
>
> <code>
> else if (status == svn_wc__db_status_not_present
> || status == svn_wc__db_status_server_excluded)
> {
> return svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,
> _("The node '%s' was not found."),
> svn_dirent_local_style(local_abspath,
> scratch_pool));
> }
> </code>
>
I updated the patch to filter out nodes with status
svn_wc__db_status_not_present and svn_wc__db_status_server_excluded in
the callback function itself.
Log
[[[
Fix for issue 3787. Make 'svn info --depth' variants display minimal
information about nodes with depth exclude in the tree.
* subversion/libsvn_client/info.c
(crawl_entries): Pass TRUE to svn_wc__node_walk_children() as
show_hidden parameter value.
(info_found_node_callback): Filter out nodes with status
svn_wc__db_status_not_present and svn_wc__db_status_server_excluded.
* subversion/tests/cmdline/depth_tests.py
(info_show_exclude): New test.
(test_list): Run it.
Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
]]]
Thanks and Regards
Noorul
Re: [PATCH] Fix for issue 3787
Posted by Noorul Islam K M <no...@collab.net>.
"Bert Huijben" <be...@qqmail.nl> writes:
>> -----Original Message-----
>> From: Noorul Islam K M [mailto:noorul@collab.net]
>> Sent: maandag 4 juli 2011 18:05
>> To: Daniel Shahaf
>> Cc: Subversion
>> Subject: Re: [PATCH] Fix for issue 3787
>
>> [[[
>>
>> Fix for issue 3787. Make 'svn info --depth' variants display minimal
>> information about nodes with depth exclude in the tree.
>>
>> * subversion/libsvn_client/info.c
>> (crawl_entries): Pass TRUE to svn_wc__node_walk_children() as
>> show_hidden parameter value.
>>
>> * subversion/tests/cmdline/depth_tests.py
>> (info_show_exclude): New test.
>> (test_list): Run it.
>>
>> Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
>>
>> ]]]
>
> (Please look in your irc log. We talked about this on IRC before)
>
> This simple fix makes that the code also gets not-present and server
> excluded nodes. But the info api (and the output) don't handle these cases.
>
> Switching the 'show hidden' flag requires more work in the callback to hide
> all unexpected hidden kinds.
>
> Bert
In those cases build_info_for_entry() is throwing error. Is that not
good enough?
<code>
else if (status == svn_wc__db_status_not_present
|| status == svn_wc__db_status_server_excluded)
{
return svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,
_("The node '%s' was not found."),
svn_dirent_local_style(local_abspath,
scratch_pool));
}
</code>
Thanks and Regards
Noorul
RE: [PATCH] Fix for issue 3787
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Noorul Islam K M [mailto:noorul@collab.net]
> Sent: maandag 4 juli 2011 18:05
> To: Daniel Shahaf
> Cc: Subversion
> Subject: Re: [PATCH] Fix for issue 3787
> [[[
>
> Fix for issue 3787. Make 'svn info --depth' variants display minimal
> information about nodes with depth exclude in the tree.
>
> * subversion/libsvn_client/info.c
> (crawl_entries): Pass TRUE to svn_wc__node_walk_children() as
> show_hidden parameter value.
>
> * subversion/tests/cmdline/depth_tests.py
> (info_show_exclude): New test.
> (test_list): Run it.
>
> Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
>
> ]]]
(Please look in your irc log. We talked about this on IRC before)
This simple fix makes that the code also gets not-present and server
excluded nodes. But the info api (and the output) don't handle these cases.
Switching the 'show hidden' flag requires more work in the callback to hide
all unexpected hidden kinds.
Bert
Re: [PATCH] Fix for issue 3787
Posted by Noorul Islam K M <no...@collab.net>.
Stefan Sperling <st...@elego.de> writes:
> On Thu, Apr 07, 2011 at 02:46:45PM +0530, Noorul Islam K M wrote:
>
>> Daniel Shahaf <d....@daniel.shahaf.name> writes:
>>
>> > Noorul Islam K M wrote on Thu, Apr 07, 2011 at 10:17:49 +0530:
>> >
>> >> Index: subversion/libsvn_client/info.c
>> >> ===================================================================
>> >> --- subversion/libsvn_client/info.c (revision 1089373)
>> >> +++ subversion/libsvn_client/info.c (working copy)
>> >> @@ -408,13 +408,17 @@
>> >> {
>> >> struct found_entry_baton fe_baton;
>> >> svn_error_t *err;
>> >> + svn_boolean_t show_exclude = FALSE;
>> >>
>> >> fe_baton.changelist_hash = changelist_hash;
>> >> fe_baton.receiver = receiver;
>> >> fe_baton.receiver_baton = receiver_baton;
>> >> fe_baton.wc_ctx = ctx->wc_ctx;
>> >>
>> >> - err = svn_wc__node_walk_children(ctx->wc_ctx, local_abspath, FALSE,
>> >> + if (depth == SVN_DEPTH_INFINITY_OR_FILES(TRUE))
>> >> + show_exclude = TRUE;
>> >> +
>> >> + err = svn_wc__node_walk_children(ctx->wc_ctx, local_abspath, show_exclude,
>> >> info_found_node_callback, &fe_baton, depth,
>> >> ctx->cancel_func, ctx->cancel_baton, pool);
>> >>
>> >
>> > Suppose that A/ and 'iota' are excluded. 'svn info --depth=immediates
>> > wc_dir' should also show information about both A/ and 'iota', but with
>> > your patch applied it doesn't show information for A/.
>> >
>> > (I didn't test 'iota' and testing the equivalent --depth=files recipe.)
>>
>> The requirement is to show info for excludes if '-R' is passed. Doesn't
>> that mean you have to use --depth=infinity?
>
> -R implies --depth=infinity.
> It means "unlimited recursion". It is the old way of saying "show me
> everything beneath this directory". before --depth was introduced.
>
> But with --depth, it's possible to specify how deep the recursion should be,
> to a limited extent. Say I want to see status output only for the current
> directory and its direct children. I can use --depth=immediates for that.
>
> I think --depth was initially conceived for checkout and update to control
> sparse working copies. Then it ended up being used also to replace the -R
> and -N switches of other subcommands (-N being --depth=empty).
>
> So, yes, please show exluded status also for other depth values.
> E.g. if I ask for the status of an exluded item with --depth=empty,
> I'd expect to be told that this item is excluded.
>
> Some examples:
>
> No matter whether ./foo is a file or directory, after this command...
>
> $ svn up --set-depth=exclude foo
>
> ... these commands should produce equal output:
> $ svn st foo
> $ svn st --depth=empty foo
> $ svn st --depth=immediates foo
> $ svn st --depth=infinity foo
> $ svn st --depth=files foo
>
> And these should show the same information for foo (among information
> about other items in the current directory):
>
> $ svn st
> $ svn st --depth=immediates
>
> and this also, but only if foo is a file:
>
> $ svn st --depth=files foo
>
> While this should show no information about foo:
>
> $ svn st --depth=empty
Stefan,
Please find attached the updated patch. I covered all the cases above in
the test.
Log
[[[
Fix for issue 3787. Make 'svn info --depth' variants display minimal
information about nodes with depth exclude in the tree.
* subversion/libsvn_client/info.c
(crawl_entries): Pass TRUE to svn_wc__node_walk_children() as
show_hidden parameter value.
* subversion/tests/cmdline/depth_tests.py
(info_show_exclude): New test.
(test_list): Run it.
Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
]]]
Thanks and Regards
Noorul
Re: [PATCH] Fix for issue 3787
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan Sperling wrote on Thu, Apr 07, 2011 at 11:38:43 +0200:
> I think --depth was initially conceived for checkout and update to control
> sparse working copies. Then it ended up being used also to replace the -R
> and -N switches of other subcommands (-N being --depth=empty).
Actually -N meant (and means) different things in different subcommands
--- that's why we have the SVN_DEPTH_INFINITY_OR_*() macros.
Re: [PATCH] Fix for issue 3787
Posted by Stefan Sperling <st...@elego.de>.
On Thu, Apr 07, 2011 at 02:42:40PM +0300, Daniel Shahaf wrote:
> Stefan Sperling wrote on Thu, Apr 07, 2011 at 11:38:43 +0200:
> > While this should show no information about foo:
> >
> > $ svn st --depth=empty
>
> And then there is
>
> % svn st --depth=empty foo
>
> which, I believe, should show information about 'foo' even if it's a file.
Yes, I listed that, too, at the very top.
Re: [PATCH] Fix for issue 3787
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan Sperling wrote on Thu, Apr 07, 2011 at 11:38:43 +0200:
> While this should show no information about foo:
>
> $ svn st --depth=empty
And then there is
% svn st --depth=empty foo
which, I believe, should show information about 'foo' even if it's a file.
Re: [PATCH] Fix for issue 3787
Posted by Stefan Sperling <st...@elego.de>.
On Thu, Apr 07, 2011 at 02:46:45PM +0530, Noorul Islam K M wrote:
> Daniel Shahaf <d....@daniel.shahaf.name> writes:
>
> > Noorul Islam K M wrote on Thu, Apr 07, 2011 at 10:17:49 +0530:
> >
> >> Index: subversion/libsvn_client/info.c
> >> ===================================================================
> >> --- subversion/libsvn_client/info.c (revision 1089373)
> >> +++ subversion/libsvn_client/info.c (working copy)
> >> @@ -408,13 +408,17 @@
> >> {
> >> struct found_entry_baton fe_baton;
> >> svn_error_t *err;
> >> + svn_boolean_t show_exclude = FALSE;
> >>
> >> fe_baton.changelist_hash = changelist_hash;
> >> fe_baton.receiver = receiver;
> >> fe_baton.receiver_baton = receiver_baton;
> >> fe_baton.wc_ctx = ctx->wc_ctx;
> >>
> >> - err = svn_wc__node_walk_children(ctx->wc_ctx, local_abspath, FALSE,
> >> + if (depth == SVN_DEPTH_INFINITY_OR_FILES(TRUE))
> >> + show_exclude = TRUE;
> >> +
> >> + err = svn_wc__node_walk_children(ctx->wc_ctx, local_abspath, show_exclude,
> >> info_found_node_callback, &fe_baton, depth,
> >> ctx->cancel_func, ctx->cancel_baton, pool);
> >>
> >
> > Suppose that A/ and 'iota' are excluded. 'svn info --depth=immediates
> > wc_dir' should also show information about both A/ and 'iota', but with
> > your patch applied it doesn't show information for A/.
> >
> > (I didn't test 'iota' and testing the equivalent --depth=files recipe.)
>
> The requirement is to show info for excludes if '-R' is passed. Doesn't
> that mean you have to use --depth=infinity?
-R implies --depth=infinity.
It means "unlimited recursion". It is the old way of saying "show me
everything beneath this directory". before --depth was introduced.
But with --depth, it's possible to specify how deep the recursion should be,
to a limited extent. Say I want to see status output only for the current
directory and its direct children. I can use --depth=immediates for that.
I think --depth was initially conceived for checkout and update to control
sparse working copies. Then it ended up being used also to replace the -R
and -N switches of other subcommands (-N being --depth=empty).
So, yes, please show exluded status also for other depth values.
E.g. if I ask for the status of an exluded item with --depth=empty,
I'd expect to be told that this item is excluded.
Some examples:
No matter whether ./foo is a file or directory, after this command...
$ svn up --set-depth=exclude foo
... these commands should produce equal output:
$ svn st foo
$ svn st --depth=empty foo
$ svn st --depth=immediates foo
$ svn st --depth=infinity foo
$ svn st --depth=files foo
And these should show the same information for foo (among information
about other items in the current directory):
$ svn st
$ svn st --depth=immediates
and this also, but only if foo is a file:
$ svn st --depth=files foo
While this should show no information about foo:
$ svn st --depth=empty
Re: [PATCH] Fix for issue 3787
Posted by Noorul Islam K M <no...@collab.net>.
Daniel Shahaf <d....@daniel.shahaf.name> writes:
> Noorul Islam K M wrote on Thu, Apr 07, 2011 at 10:17:49 +0530:
>
>> Index: subversion/libsvn_client/info.c
>> ===================================================================
>> --- subversion/libsvn_client/info.c (revision 1089373)
>> +++ subversion/libsvn_client/info.c (working copy)
>> @@ -408,13 +408,17 @@
>> {
>> struct found_entry_baton fe_baton;
>> svn_error_t *err;
>> + svn_boolean_t show_exclude = FALSE;
>>
>> fe_baton.changelist_hash = changelist_hash;
>> fe_baton.receiver = receiver;
>> fe_baton.receiver_baton = receiver_baton;
>> fe_baton.wc_ctx = ctx->wc_ctx;
>>
>> - err = svn_wc__node_walk_children(ctx->wc_ctx, local_abspath, FALSE,
>> + if (depth == SVN_DEPTH_INFINITY_OR_FILES(TRUE))
>> + show_exclude = TRUE;
>> +
>> + err = svn_wc__node_walk_children(ctx->wc_ctx, local_abspath, show_exclude,
>> info_found_node_callback, &fe_baton, depth,
>> ctx->cancel_func, ctx->cancel_baton, pool);
>>
>
> Suppose that A/ and 'iota' are excluded. 'svn info --depth=immediates
> wc_dir' should also show information about both A/ and 'iota', but with
> your patch applied it doesn't show information for A/.
>
> (I didn't test 'iota' and testing the equivalent --depth=files recipe.)
The requirement is to show info for excludes if '-R' is passed. Doesn't
that mean you have to use --depth=infinity?
Thanks and Regards
Noorul
Re: [PATCH] Fix for issue 3787
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Noorul Islam K M wrote on Thu, Apr 07, 2011 at 10:17:49 +0530:
> Index: subversion/libsvn_client/info.c
> ===================================================================
> --- subversion/libsvn_client/info.c (revision 1089373)
> +++ subversion/libsvn_client/info.c (working copy)
> @@ -408,13 +408,17 @@
> {
> struct found_entry_baton fe_baton;
> svn_error_t *err;
> + svn_boolean_t show_exclude = FALSE;
>
> fe_baton.changelist_hash = changelist_hash;
> fe_baton.receiver = receiver;
> fe_baton.receiver_baton = receiver_baton;
> fe_baton.wc_ctx = ctx->wc_ctx;
>
> - err = svn_wc__node_walk_children(ctx->wc_ctx, local_abspath, FALSE,
> + if (depth == SVN_DEPTH_INFINITY_OR_FILES(TRUE))
> + show_exclude = TRUE;
> +
> + err = svn_wc__node_walk_children(ctx->wc_ctx, local_abspath, show_exclude,
> info_found_node_callback, &fe_baton, depth,
> ctx->cancel_func, ctx->cancel_baton, pool);
>
Suppose that A/ and 'iota' are excluded. 'svn info --depth=immediates
wc_dir' should also show information about both A/ and 'iota', but with
your patch applied it doesn't show information for A/.
(I didn't test 'iota' and testing the equivalent --depth=files recipe.)