You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "C. Michael Pilato" <cm...@collab.net> on 2012/10/05 03:46:16 UTC
Re: svn commit: r1394332 - /subversion/trunk/subversion/include/svn_client.h
On 10/04/2012 07:57 PM, breser@apache.org wrote:
> Author: breser
> Date: Thu Oct 4 23:57:26 2012
> New Revision: 1394332
[...]
> Modified: subversion/trunk/subversion/include/svn_client.h
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_client.h?rev=1394332&r1=1394331&r2=1394332&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/include/svn_client.h (original)
> +++ subversion/trunk/subversion/include/svn_client.h Thu Oct 4 23:57:26 2012
> @@ -1469,10 +1469,9 @@ svn_client_switch(svn_revnum_t *result_r
> *
> * If @a force is not set and @a path is already under version
> * control, return the error #SVN_ERR_ENTRY_EXISTS. If @a force is
> - * set, do not error on already-versioned items. When used on a
> - * directory in conjunction with the @a recursive flag, this has the
> - * effect of scheduling for addition unversioned files and directories
> - * scattered deep within a versioned tree.
> + * set, do not error on already-versioned items. When used with @a depth
> + * set to #svn_depth_infinity it will enter versioned directories; scheduling
> + * versioned children.
"scheduling versioned children" doesn't really parse for me in this context
semantically. Also, the semicolon that precedes the phrase is not the
proper punctuation for offsetting such a thing.
Perhaps you meant something like:
"... it will enter versioned directories, scheduling any unversioned
children thereof for addition."
But why only #svn_depth_infinity? Will it not do the same (to different
depths, of course) for #svn_depth_files and #svn_depth_immediates?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: svn commit: r1394332 -
/subversion/trunk/subversion/include/svn_client.h
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Ben Reser wrote on Thu, Oct 04, 2012 at 19:06:31 -0700:
> On Thu, Oct 4, 2012 at 6:48 PM, C. Michael Pilato <cm...@collab.net> wrote:
> > On 10/04/2012 09:46 PM, C. Michael Pilato wrote:
> >> Perhaps you meant something like:
> >>
> >> "... it will enter versioned directories, scheduling any unversioned
> >> children thereof for addition."
> >
> > Sorry -- I just saw that you fixed the *unversioned* bit. My additional
> > questions remain:
> >
> >> But why only #svn_depth_infinity? Will it not do the same (to different
> >> depths, of course) for #svn_depth_files and #svn_depth_immediates?
>
No, because neither of those depths includes children of children. (If
you do an add with depth immediates, it recurses into child directories
of the target with depth empty.)
I haven't tested this, but that's how I expect it to work.
> How about:
>
> [[[
> When used with @a depth it will enter versioned directories (per the
> rules of the argument), and schedule unversioned children.
> ]]]
Re: svn commit: r1394332 - /subversion/trunk/subversion/include/svn_client.h
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 10/05/2012 09:29 AM, Bert Huijben wrote:
> Currently this function fails with an error when called on the working
> copy root, with and without force set to TRUE. (Probably because we
> originally handled force to suppress some light errors, while we can't
> add a working copy root to its parent)
>
> Maybe we should fix this as well if we are touching this code anyway?
FIXED. See http://subversion.tigris.org/issues/show_bug.cgi?id=4241
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: svn commit: r1394332 - /subversion/trunk/subversion/include/svn_client.h
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 10/05/2012 01:58 PM, C. Michael Pilato wrote:
> On 10/05/2012 12:10 PM, C. Michael Pilato wrote:
>> On 10/05/2012 09:29 AM, Bert Huijben wrote:
>>> Currently this function fails with an error when called on the working copy root, with and without force set to TRUE.
>>> (Probably because we originally handled force to suppress some light errors, while we can't add a working copy root to its parent)
>>>
>>> Maybe we should fix this as well if we are touching this code anyway?
>>
>> Gonna kick off some tests of the attached patch while I run to lunch. Care
>> to review in the meantime?
>>
>
> Well, that seems to have only broken 249 tests. :-P
Okay. It wasn't quite that bad. It broke a few tests which were running in
a ramdisk and exhausted the allocated space for the remaining tests. But
regardless, I'm testing a revised patch which should fare better.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: svn commit: r1394332 - /subversion/trunk/subversion/include/svn_client.h
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 10/05/2012 12:10 PM, C. Michael Pilato wrote:
> On 10/05/2012 09:29 AM, Bert Huijben wrote:
>> Currently this function fails with an error when called on the working copy root, with and without force set to TRUE.
>> (Probably because we originally handled force to suppress some light errors, while we can't add a working copy root to its parent)
>>
>> Maybe we should fix this as well if we are touching this code anyway?
>
> Gonna kick off some tests of the attached patch while I run to lunch. Care
> to review in the meantime?
>
Well, that seems to have only broken 249 tests. :-P
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: svn commit: r1394332 - /subversion/trunk/subversion/include/svn_client.h
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 10/05/2012 09:29 AM, Bert Huijben wrote:
> Currently this function fails with an error when called on the working copy root, with and without force set to TRUE.
> (Probably because we originally handled force to suppress some light errors, while we can't add a working copy root to its parent)
>
> Maybe we should fix this as well if we are touching this code anyway?
Gonna kick off some tests of the attached patch while I run to lunch. Care
to review in the meantime?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
RE: svn commit: r1394332 - /subversion/trunk/subversion/include/svn_client.h
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Bert Huijben [mailto:bert@qqmail.nl]
> Sent: vrijdag 5 oktober 2012 15:30
> To: Michael Pilato; 'Ben Reser'
> Cc: dev@subversion.apache.org
> Subject: RE: svn commit: r1394332 -
> /subversion/trunk/subversion/include/svn_client.h
>
>
>
> > -----Original Message-----
> > From: C. Michael Pilato [mailto:cmpilato@collab.net]
> > Sent: vrijdag 5 oktober 2012 14:49
> > To: Ben Reser
> > Cc: dev@subversion.apache.org
> > Subject: Re: svn commit: r1394332 -
> > /subversion/trunk/subversion/include/svn_client.h
> >
> > On 10/04/2012 10:06 PM, Ben Reser wrote:
> > > On Thu, Oct 4, 2012 at 6:48 PM, C. Michael Pilato <cm...@collab.net>
> > wrote:
> > >> On 10/04/2012 09:46 PM, C. Michael Pilato wrote:
> > >>> Perhaps you meant something like:
> > >>>
> > >>> "... it will enter versioned directories, scheduling any unversioned
> > >>> children thereof for addition."
> > >>
> > >> Sorry -- I just saw that you fixed the *unversioned* bit. My additional
> > >> questions remain:
> > >>
> > >>> But why only #svn_depth_infinity? Will it not do the same (to
> different
> > >>> depths, of course) for #svn_depth_files and
> #svn_depth_immediates?
> > >
> > > How about:
> > >
> > > [[[
> > > When used with @a depth it will enter versioned directories (per the
> > > rules of the argument), and schedule unversioned children.
> > > ]]]
> > >
> >
> > Honestly, the original phrasing of the docstring remains a better starting
> > point, in my opinion. Your changes lose the context that all this
> > discussion about depth and unversioned items in a versioned tree are still
> > tried primarily to the use of the force flag. So if it were up to me, I
> > would restore that paragraph to the state it was in and make only minor
> > changes:
> >
> > * If @a force is not set and @a path is already under version
> > * control, return the error #SVN_ERR_ENTRY_EXISTS. If @a force is
> > * set, do not error on already-versioned items. When used on a
> > * directory in conjunction with a @a depth value greater than
> > * #svn_depth_empty, this has the effect of scheduling for addition
> > * any unversioned files and directories scattered within even a
> > * versioned tree (up to @a depth).
>
> Currently this function fails with an error when called on the working copy
> root, with and without force set to TRUE.
> (Probably because we originally handled force to suppress some light errors,
> while we can't add a working copy root to its parent)
>
> Maybe we should fix this as well if we are touching this code anyway?
http://stackoverflow.com/questions/11659867/is-there-a-directory-bug-in-svnclient-add
made me remember this problem
(Reported against SharpSvn, but the error originates in libsvn_client/libsvn_wc)
>
> Bert
RE: svn commit: r1394332 - /subversion/trunk/subversion/include/svn_client.h
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: C. Michael Pilato [mailto:cmpilato@collab.net]
> Sent: vrijdag 5 oktober 2012 14:49
> To: Ben Reser
> Cc: dev@subversion.apache.org
> Subject: Re: svn commit: r1394332 -
> /subversion/trunk/subversion/include/svn_client.h
>
> On 10/04/2012 10:06 PM, Ben Reser wrote:
> > On Thu, Oct 4, 2012 at 6:48 PM, C. Michael Pilato <cm...@collab.net>
> wrote:
> >> On 10/04/2012 09:46 PM, C. Michael Pilato wrote:
> >>> Perhaps you meant something like:
> >>>
> >>> "... it will enter versioned directories, scheduling any unversioned
> >>> children thereof for addition."
> >>
> >> Sorry -- I just saw that you fixed the *unversioned* bit. My additional
> >> questions remain:
> >>
> >>> But why only #svn_depth_infinity? Will it not do the same (to different
> >>> depths, of course) for #svn_depth_files and #svn_depth_immediates?
> >
> > How about:
> >
> > [[[
> > When used with @a depth it will enter versioned directories (per the
> > rules of the argument), and schedule unversioned children.
> > ]]]
> >
>
> Honestly, the original phrasing of the docstring remains a better starting
> point, in my opinion. Your changes lose the context that all this
> discussion about depth and unversioned items in a versioned tree are still
> tried primarily to the use of the force flag. So if it were up to me, I
> would restore that paragraph to the state it was in and make only minor
> changes:
>
> * If @a force is not set and @a path is already under version
> * control, return the error #SVN_ERR_ENTRY_EXISTS. If @a force is
> * set, do not error on already-versioned items. When used on a
> * directory in conjunction with a @a depth value greater than
> * #svn_depth_empty, this has the effect of scheduling for addition
> * any unversioned files and directories scattered within even a
> * versioned tree (up to @a depth).
Currently this function fails with an error when called on the working copy root, with and without force set to TRUE.
(Probably because we originally handled force to suppress some light errors, while we can't add a working copy root to its parent)
Maybe we should fix this as well if we are touching this code anyway?
Bert
Re: svn commit: r1394332 - /subversion/trunk/subversion/include/svn_client.h
Posted by Ben Reser <br...@apache.org>.
On Fri, Oct 5, 2012 at 6:00 PM, Greg Stein <gs...@gmail.com> wrote:
> Of course, it would make more sense to *rename* the parameter to something
> more appropriate. "skip_versioned_nodes" might work.
It'd have to be no_skip_versioned_nodes.
Re: svn commit: r1394332 - /subversion/trunk/subversion/include/svn_client.h
Posted by Greg Stein <gs...@gmail.com>.
Of course, it would make more sense to *rename* the parameter to something
more appropriate. "skip_versioned_nodes" might work.
On Oct 5, 2012 5:19 PM, "Ben Reser" <br...@apache.org> wrote:
> On Fri, Oct 5, 2012 at 5:49 AM, C. Michael Pilato <cm...@collab.net>
> wrote:
> > Honestly, the original phrasing of the docstring remains a better
> starting
> > point, in my opinion. Your changes lose the context that all this
> > discussion about depth and unversioned items in a versioned tree are
> still
> > tried primarily to the use of the force flag. So if it were up to me, I
> > would restore that paragraph to the state it was in and make only minor
> changes:
> >
> > * If @a force is not set and @a path is already under version
> > * control, return the error #SVN_ERR_ENTRY_EXISTS. If @a force is
> > * set, do not error on already-versioned items. When used on a
> > * directory in conjunction with a @a depth value greater than
> > * #svn_depth_empty, this has the effect of scheduling for addition
> > * any unversioned files and directories scattered within even a
> > * versioned tree (up to @a depth).
>
> Committed this unchanged.
>
Re: svn commit: r1394332 - /subversion/trunk/subversion/include/svn_client.h
Posted by Ben Reser <br...@apache.org>.
On Fri, Oct 5, 2012 at 5:49 AM, C. Michael Pilato <cm...@collab.net> wrote:
> Honestly, the original phrasing of the docstring remains a better starting
> point, in my opinion. Your changes lose the context that all this
> discussion about depth and unversioned items in a versioned tree are still
> tried primarily to the use of the force flag. So if it were up to me, I
> would restore that paragraph to the state it was in and make only minor changes:
>
> * If @a force is not set and @a path is already under version
> * control, return the error #SVN_ERR_ENTRY_EXISTS. If @a force is
> * set, do not error on already-versioned items. When used on a
> * directory in conjunction with a @a depth value greater than
> * #svn_depth_empty, this has the effect of scheduling for addition
> * any unversioned files and directories scattered within even a
> * versioned tree (up to @a depth).
Committed this unchanged.
Re: svn commit: r1394332 - /subversion/trunk/subversion/include/svn_client.h
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 10/04/2012 10:06 PM, Ben Reser wrote:
> On Thu, Oct 4, 2012 at 6:48 PM, C. Michael Pilato <cm...@collab.net> wrote:
>> On 10/04/2012 09:46 PM, C. Michael Pilato wrote:
>>> Perhaps you meant something like:
>>>
>>> "... it will enter versioned directories, scheduling any unversioned
>>> children thereof for addition."
>>
>> Sorry -- I just saw that you fixed the *unversioned* bit. My additional
>> questions remain:
>>
>>> But why only #svn_depth_infinity? Will it not do the same (to different
>>> depths, of course) for #svn_depth_files and #svn_depth_immediates?
>
> How about:
>
> [[[
> When used with @a depth it will enter versioned directories (per the
> rules of the argument), and schedule unversioned children.
> ]]]
>
Honestly, the original phrasing of the docstring remains a better starting
point, in my opinion. Your changes lose the context that all this
discussion about depth and unversioned items in a versioned tree are still
tried primarily to the use of the force flag. So if it were up to me, I
would restore that paragraph to the state it was in and make only minor changes:
* If @a force is not set and @a path is already under version
* control, return the error #SVN_ERR_ENTRY_EXISTS. If @a force is
* set, do not error on already-versioned items. When used on a
* directory in conjunction with a @a depth value greater than
* #svn_depth_empty, this has the effect of scheduling for addition
* any unversioned files and directories scattered within even a
* versioned tree (up to @a depth).
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: svn commit: r1394332 - /subversion/trunk/subversion/include/svn_client.h
Posted by Ben Reser <br...@apache.org>.
On Thu, Oct 4, 2012 at 6:48 PM, C. Michael Pilato <cm...@collab.net> wrote:
> On 10/04/2012 09:46 PM, C. Michael Pilato wrote:
>> Perhaps you meant something like:
>>
>> "... it will enter versioned directories, scheduling any unversioned
>> children thereof for addition."
>
> Sorry -- I just saw that you fixed the *unversioned* bit. My additional
> questions remain:
>
>> But why only #svn_depth_infinity? Will it not do the same (to different
>> depths, of course) for #svn_depth_files and #svn_depth_immediates?
How about:
[[[
When used with @a depth it will enter versioned directories (per the
rules of the argument), and schedule unversioned children.
]]]
Re: svn commit: r1394332 - /subversion/trunk/subversion/include/svn_client.h
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 10/04/2012 09:46 PM, C. Michael Pilato wrote:
> Perhaps you meant something like:
>
> "... it will enter versioned directories, scheduling any unversioned
> children thereof for addition."
Sorry -- I just saw that you fixed the *unversioned* bit. My additional
questions remain:
> But why only #svn_depth_infinity? Will it not do the same (to different
> depths, of course) for #svn_depth_files and #svn_depth_immediates?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development