You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by kf...@collab.net on 2004/03/08 15:45:09 UTC
The new svn_wc_adm_*_depth functions.
Is it our intention that these new functions
svn_wc_adm_open_depth
svn_wc_adm_probe_try_depth
svn_wc_adm_probe_open_depth
supersede their non-depth predecessors, and that (eventually) all
callers of the predecessors be converted to use the depth versions? I
would think so, but just want to make sure here first.
If so, it's time to mark the predecessors
svn_wc_adm_open
svn_wc_adm_probe_try
svn_wc_adm_probe_open
as 'deprecated'. This should be pretty simple: just make a special
'deprecated' marker that's easy to grep for, and open a single new
issue in the 2.0 milestone:
"Remove deprecated 1.x interfaces."
That issue will give instructions to grep for the special marker. It
doesn't really matter whether we convert all the predecessors now, or
just make a note in the deprecated function's doc string that it needs
to be done before removing the function. (I'd prefer to do it now, if
it's not a big task, though, to avoid having bad examples in our
code.)
I can take care of all this; I just want to make sure it's what we
intend.
-Karl
djh@tigris.org writes:
> Log:
> Fix issue #1680: Don't lock entire tree in svn status --non-recursive
>
> Patch by Jon Foster, approved by Philip Martin.
>
> * svn_wc.h
> (svn_wc_adm_open_depth, svn_wc_adm_probe_open_depth,
> svn_wc_adm_probe_try_depth): New functions.
> (svn_wc_adm_open, svn_wc_adm_probe_open, svn_wc_adm_probe_try):
> Changed comments.
>
> * libsvn_wc/lock.c
> (do_open): Replace tree_lock parameter with depth, and use it.
> (svn_wc__adm_pre_open): Fixup call to do_open.
> (svn_wc_adm_open_depth): New function.
> (svn_wc_adm_open): Reimplement as trivial wrapper.
> (svn_wc_adm_probe_open_depth): New function.
> (svn_wc_adm_probe_open): Reimplement as trivial wrapper.
> (svn_wc_adm_probe_try_depth): New function.
> (svn_wc_adm_probe_try): Reimplement as trivial wrapper.
>
> * libsvn_client/status.c
> (svn_client_status): Use svn_wc_adm_probe_open_depth to avoid excessive
> recursion if --non-recursive.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: The new svn_wc_adm_*_depth functions.
Posted by kf...@collab.net.
Philip Martin <ph...@codematters.co.uk> writes:
> I think we are talking at cross purposes, when I say "Subversion is
> still using the non-depth interface" I mean it is explicitly using the
> functions you are proposing to deprecate. It is straightforward to
> convert the code to use the new depth interface, but it hasn't been
> done.
Ah. I mentioned that as one of the deprecation tasks in my original
post, but I might not have been clear:
> That issue will give instructions to grep for the special marker.
> It doesn't really matter whether we convert all the predecessors
> now, or just make a note in the deprecated function's doc string
> that it needs to be done before removing the function. (I'd
> prefer to do it now, if it's not a big task, though, to avoid
> having bad examples in our code.)
"predecessors" should have been "callers of the predecessors", which
is what I meant.
Anyway, yes, converting the callers is part of deprecating.
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: The new svn_wc_adm_*_depth functions.
Posted by Philip Martin <ph...@codematters.co.uk>.
kfogel@collab.net writes:
> Philip Martin <ph...@codematters.co.uk> writes:
>> > This seems to be saying you agree with my proposal to deprecate, then.
>>
>> It might be a little premature, Subversion itself is still using the
>> non-depth interface.
>
> The 'depth' parameter introduced in svn_wc_adm_*_depth doesn't really
> correspond to the '--depth' option we've talked about in the past.
I think we are talking at cross purposes, when I say "Subversion is
still using the non-depth interface" I mean it is explicitly using the
functions you are proposing to deprecate. It is straightforward to
convert the code to use the new depth interface, but it hasn't been
done. This is separate from the problem of providing directory only,
directory and children, and complete tree operations in the client.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: The new svn_wc_adm_*_depth functions.
Posted by Benjamin Pflugmann <be...@pflugmann.de>.
On Fri 2004-03-12 at 10:35:18 -0600, kfogel@collab.net wrote:
> Benjamin Pflugmann <be...@pflugmann.de> writes:
> > When reading source code which you are unfamiliar with, functions
> > named dothis(), dothis2(), dothis3() are one of the worst things,
> > IMHO.
>
> I think this "2" (or "N") suffix is actually something of a
> convention. I've seen it before in more than one place, and never
> been confused about what it meant: an API needed variants. Consider
> popen(), popen2(), popen3().
Bad example, I don't know Python :)
Well, I looked it up (that's why I know it's propably Python) and
popen2() and popen3() seem like an exception to me, because there is
some easy "memory hook" indeed (the number is the amount of pipes
returned), not simply an meaningless enumeration. Well, at least for
those two... popen4() seems be one of those abominations that I fear,
and I found no popen() in these Python docs so I don't comment on
that.
> It's hard to say in absolute terms "It's confusing" or "It's not
> confusing". All I can tell you is that I've not been confused by it,
> and other APIs have chosen a similar course.
If you have no problems with enumerations, and it's only me, fine.
When there's no problem, don't fix it. :-)
> I can put a note in HACKING about what '@deprecated' means -- i.e,
> that it's the *API* that's deprecated, not necessarily the name. We
> should simply assume an arbitrary amount of API change when 2.0 comes
> out. It's not like it'll be that hard for anyone to fix their apps.
> You try to compile your code, it doesn't work, you go do the obvious
> thing (removing the "N" suffixes from functions, basically).
Jupp, that wasn't my concern.
HAND,
Benjamin.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: The new svn_wc_adm_*_depth functions.
Posted by kf...@collab.net.
Benjamin Pflugmann <be...@pflugmann.de> writes:
> When reading source code which you are unfamiliar with, functions
> named dothis(), dothis2(), dothis3() are one of the worst things,
> IMHO.
I think this "2" (or "N") suffix is actually something of a
convention. I've seen it before in more than one place, and never
been confused about what it meant: an API needed variants. Consider
popen(), popen2(), popen3().
It's hard to say in absolute terms "It's confusing" or "It's not
confusing". All I can tell you is that I've not been confused by it,
and other APIs have chosen a similar course.
> There is no indication of what the "2" means except that is some kind
> of variation of the theme of dothis(). What then happens is that
> everytime one sees dothis() or dothis2(), one has to get back to the
> doc/definition to see which of them did what, because there is usually
> no easy way to build a "memory hook" out of an enumeration.
>
> I'd prefer any of
> - foo_pool()
> - foo_recommended()
> - foo_for_api_3_0()
>
> ...or whatever makes more sense to you. But please don't simply
> enumerate them. Pretty please?
I can put a note in HACKING about what '@deprecated' means -- i.e,
that it's the *API* that's deprecated, not necessarily the name. We
should simply assume an arbitrary amount of API change when 2.0 comes
out. It's not like it'll be that hard for anyone to fix their apps.
You try to compile your code, it doesn't work, you go do the obvious
thing (removing the "N" suffixes from functions, basically).
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: The new svn_wc_adm_*_depth functions.
Posted by Benjamin Pflugmann <be...@pflugmann.de>.
On Thu 2004-03-11 at 22:42:51 -0700, D.J. Heap wrote:
> kfogel@collab.net wrote:
> [snip]
> >
> >If we're going to fall back to the old names, then the "2" variants
> >might make the most sense.
> >
> >IMHO, we should do this. By resetting our APIs back to the
> >names-of-first-choice, we avoid names growing arbitrarily long (think
> >3.0).
> >
> >Thoughts?
>
> It looks like an agreement is forming on the '2' variants (it sounds
> reasonable to me) -- if there are no objections then I will wait another
> day or so and update the patch to rename the functions with that scheme
> and submit it for review?
May I humbly ask that you reconsider giving the function get an at
least somewhat meaningful name?
When reading source code which you are unfamiliar with, functions
named dothis(), dothis2(), dothis3() are one of the worst things,
IMHO.
There is no indication of what the "2" means except that is some kind
of variation of the theme of dothis(). What then happens is that
everytime one sees dothis() or dothis2(), one has to get back to the
doc/definition to see which of them did what, because there is usually
no easy way to build a "memory hook" out of an enumeration.
I'd prefer any of
- foo_pool()
- foo_recommended()
- foo_for_api_3_0()
...or whatever makes more sense to you. But please don't simply
enumerate them. Pretty please?
Bye,
Benjamin.
PS: If these go into 1.1.0 or the like, which is my current
understanding, we have to live with the name until 2.0, correct?
If not, ignore my plea. But if so, what if we need another change
after 1.1.0 is out with dothis2()? We get a dothis3() in 1.3.0 and
a dothis4() in 1.9.0? Ok, ok, I exagerate, but you get the idea.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Naming of function versions
Posted by Travis P <sv...@castle.fastmail.fm>.
On Mar 18, 2004, at 7:45 AM, D.J. Heap wrote:
> Nuutti Kotivuori wrote:
> [snip]
>> No, I don't think you have. I just haven't seen any project where
>> deprecated functions would have not disappeared in a following major
>> release, but instead become the supported functions again. Also, I
>> don't believe I have seen the something2 naming for new functions
>> anywhere. So I would not call those methods conventional. But that may
>> ofcourse be just me.
>> -- Naked
>
> FWIW, the something2 naming is all over the place in MS API's --
> especially COM interfaces. They also used to use somethingEx, but
> mostly stopped doing that as there is nowhere to go after
> that...somethingExEx?
>
> Hardly makes it a standard, of course, but at least it's not
> unprecedented.
>
> DJ
It's certainly not unprecedented. Using Apple's Toolbox library on
GS/OS (Apple IIgs) circa 1987, NewWindow2 was introduced with much more
functionality than NewWindow. I would not be surprised if the
Macintosh Toolbox had the same type of thing though I wasn't
programming Macs at that time but the GS/OS Toolbox was clearly highly
related to the Mac version. Both continued to be supported.
-Travis
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Naming of function versions
Posted by "D.J. Heap" <dj...@shadyvale.net>.
Nuutti Kotivuori wrote:
[snip]
>
> No, I don't think you have. I just haven't seen any project where
> deprecated functions would have not disappeared in a following major
> release, but instead become the supported functions again. Also, I
> don't believe I have seen the something2 naming for new functions
> anywhere. So I would not call those methods conventional. But that may
> ofcourse be just me.
>
> -- Naked
>
FWIW, the something2 naming is all over the place in MS API's --
especially COM interfaces. They also used to use somethingEx, but
mostly stopped doing that as there is nowhere to go after
that...somethingExEx?
Hardly makes it a standard, of course, but at least it's not unprecedented.
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Naming of function versions
Posted by Nuutti Kotivuori <na...@iki.fi>.
Greg Hudson wrote:
> It's not really fair to call our approach "unconventional" when
> there is no convention to follow. (Certainly there's none that I'm
> aware of.)
Ah, a mere linguistic mistake on my part I guess. I just meant that it
was not conventional, not that it would be going against a convention.
And you are right, there probably isn't a convention on how such
things are done with programs abiding to the APR versioning document,
since it is stricter than many others.
-- Naked
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Naming of function versions
Posted by Greg Hudson <gh...@MIT.EDU>.
On Thu, 2004-03-18 at 08:16, Nuutti Kotivuori wrote:
> No, I don't think you have. I just haven't seen any project where
> deprecated functions would have not disappeared in a following major
> release, but instead become the supported functions again. Also, I
> don't believe I have seen the something2 naming for new functions
> anywhere. So I would not call those methods conventional. But that may
> ofcourse be just me.
It's not really fair to call our approach "unconventional" when there is
no convention to follow. (Certainly there's none that I'm aware of.)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Naming of function versions
Posted by Nuutti Kotivuori <na...@iki.fi>.
Greg Stein wrote:
> It isn't "simply too great". It just isn't possible. If you compile
> against 1.1.x, then your application *requires* 1.1.x. There isn't
> anything that you can do unless you lift the hood and dig very very
> hard to ensure that your app can bind/work against 1.0.x.
>
> The simplest example is a #define constant changing from 1.0.x to
> 1.1.x. That changing value can then make a particular function
> alter its behavior. It would know whether a 1.0.x or a 1.1.x user is
> calling it. This kind of change in the constant is acceptable under
> the guidelines.
Um, the APR versioning text seems to explicitly disallow changes to
constants. It also says "Further, if an application is designed to
work with an earlier minor version, then we don't want to suddenly
change the requirements for that application. This means that the
headers cannot silently map an old function into a newer function, as
that would turn an application, say, based on 1.2 into an application
requiring the 1.4 or later release." And even more "It is possible to
design a set of headers where a macro will always refer to the
'latest' function available. Of course, if an application chooses to
use this macro, then the resulting compiled-binary will be dependent
upon whatever version it was compiled against. This strategy adds the
new functionality for applications, yet retains the necessary source
and binary compatibility for applications designed or built against
previous minor releases."
I don't know, maybe I am just reading the guidelines through my own
set of tinted glass, but I read the minor version constraints as such:
- Application coded originally against 1.0 headers is source and
binary compatible against later minor versions, but also that
application compiled against 1.1 headers is still binary compatible
with 1.0 libraries, if there are no library versioning issues on the
OS level.
- Application coded against 1.1 headers, but without using any 1.1
specific features, is still source and binary compatible against 1.0,
regardless whether it has been compiled against the 1.1 headers or
1.0 headers.
If this is *not* so, then obviously all that I have been talking here
is about princesses and toads.
> Something built against 1.0.x must continue to function with the
> 1.1.x libraries (and the guidelines are defined to enable this), but
> the reverse does not hold true. If you install a 1.1.x application,
> then the libraries must be upgraded to 1.1.x. The theory here is
> that an administrator can feel safe doing the upgrade because it
> will not break any existing 1.0.x applications.
The upgrade mentality is sound, yes. But I always saw that applying to
only applications that actually use the 1.1 specific features, and not
just being compiled or coded against 1.1.
>> Well, I still consider the approach here unconventional, but by no
>> means unsound. So as long as it is documented, it's all rose petals
>> and vanilla ice-cream.
>
> Unconventional? What aspect? I must have missed an earlier post of
> yours :-)
No, I don't think you have. I just haven't seen any project where
deprecated functions would have not disappeared in a following major
release, but instead become the supported functions again. Also, I
don't believe I have seen the something2 naming for new functions
anywhere. So I would not call those methods conventional. But that may
ofcourse be just me.
-- Naked
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Naming of function versions
Posted by Greg Stein <gs...@lyra.org>.
On Wed, Mar 17, 2004 at 06:31:55AM +0200, Nuutti Kotivuori wrote:
> kfogel@collab.net wrote:
> > Nuutti Kotivuori <na...@iki.fi> writes:
>
> [...]
>
[ in reference to "avoid 1.1 functions so your code works against 1.0" ]
> > I think if we use your interpretation, the burden on development is
> > simply too great. We've been very clear about what's guaranteed and
> > what's not; let's not impose more burden on ourselves for the sake
> > of an entirely avoidable use case.
It isn't "simply too great". It just isn't possible. If you compile
against 1.1.x, then your application *requires* 1.1.x. There isn't
anything that you can do unless you lift the hood and dig very very hard
to ensure that your app can bind/work against 1.0.x.
The simplest example is a #define constant changing from 1.0.x to 1.1.x.
That changing value can then make a particular function alter its
behavior. It would know whether a 1.0.x or a 1.1.x user is calling it.
This kind of change in the constant is acceptable under the guidelines.
Something built against 1.0.x must continue to function with the 1.1.x
libraries (and the guidelines are defined to enable this), but the reverse
does not hold true. If you install a 1.1.x application, then the libraries
must be upgraded to 1.1.x. The theory here is that an administrator can
feel safe doing the upgrade because it will not break any existing 1.0.x
applications.
>...
> > Sure! We should continue using "@deprecated", because it gives us a
> > well-defined string to search for. But there's no reason the
> > additional documentation can't be present. It can only help. That
Agreed.
> > way if someone *wants* to write compatibly with the 1.0.x API, they
> > can do so, even if only 1.1.x is present on their system.
With the caveats mentioned above. Whatever you compile against will
determine what you must link/run against. If you code to the 1.0.x API and
build against it, then you can run on a system which has 1.x.y libraries
installed.
> On that note, we should probably start using "@since 1.1" (or 1.1.0,
> if that is preferred) on every new function (or thing) added to the
> interface - and probably nuke all those tags at 2.0, since it won't
> matter then anymore and it is just clutter.
Absolutely agreed. ++1.
>...
> > P.S. I've held off putting anything in HACKING about this until the
> > thread reaches some sort of obvious consensus :-).
>
> Well, I still consider the approach here unconventional, but by no
> means unsound. So as long as it is documented, it's all rose petals
> and vanilla ice-cream.
Unconventional? What aspect? I must have missed an earlier post of yours :-)
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: The new svn_wc_adm_*_depth functions.
Posted by kf...@collab.net.
"Sander Striker" <st...@apache.org> writes:
> I would think so. It would make much sense to have a foo2() at some
> point, but not a foo(). The whole point of versioning is to not
> have versions in your function names I would think ;).
I assume you meant "would not make much sense".
> > IMHO, we should do this. By resetting our APIs back to the
> > names-of-first-choice, we avoid names growing arbitrarily long (think
> > 3.0).
>
> +1.
+1 to your +1 :-).
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: [PATCH] Re: The new svn_wc_adm_*_depth functions.
Posted by Sander Striker <st...@apache.org>.
> From: kfogel@newton.ch.collab.net [mailto:kfogel@newton.ch.collab.net]On
> Behalf Of kfogel@collab.net
> Greg Hudson <gh...@MIT.EDU> writes:
> > With one set of functions, we're adding foo_depth() variants which
> > include a depth argument.
> >
> > With another set of functions, we didn't want to add foo_pool() variants
> > because that looks dumb, so we're adding foo2() variants instead.
> >
> > I would like us to be consistent.
>
> Good point. A question that impinges on this:
>
> Are we planning to keep these suffixes in 2.0? Or will we change the
> new names to the old names (just keep the new APIs) when we cross the
> 2.0 line?
I would think so. It would make much sense to have a foo2() at some
point, but not a foo(). The whole point of versioning is to not
have versions in your function names I would think ;).
> If we're going to fall back to the old names, then the "2" variants
> might make the most sense.
>
> IMHO, we should do this. By resetting our APIs back to the
> names-of-first-choice, we avoid names growing arbitrarily long (think
> 3.0).
+1.
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Naming of function versions
Posted by Nuutti Kotivuori <na...@iki.fi>.
kfogel@collab.net wrote:
> Nuutti Kotivuori <na...@iki.fi> writes:
[...]
> I think if we use your interpretation, the burden on development is
> simply too great. We've been very clear about what's guaranteed and
> what's not; let's not impose more burden on ourselves for the sake
> of an entirely avoidable use case.
"Hmm... Okay, I can relate to that." :-)
>> Simply explaining the situation in the documentation will give the
>> users the best grounds on making the decision over using the newer
>> function and forfeiting older minor version compatibility or just
>> sticking with the old - and also giving them an idea what is going
>> to happen in the future. As an example, in the WC administrative
>> directory lock depth case, the documentation string could simply
>> point the user to a respective svn_wc_adm_foo2 function, point out
>> that it was added since 1.1.0 (or development leading to 1.1.0) and
>> that 2.0 will most likely have the svn_wc_adm_foo function with the
>> parameters of svn_wc_adm_foo2.
>
> Sure! We should continue using "@deprecated", because it gives us a
> well-defined string to search for. But there's no reason the
> additional documentation can't be present. It can only help. That
> way if someone *wants* to write compatibly with the 1.0.x API, they
> can do so, even if only 1.1.x is present on their system.
On that note, we should probably start using "@since 1.1" (or 1.1.0,
if that is preferred) on every new function (or thing) added to the
interface - and probably nuke all those tags at 2.0, since it won't
matter then anymore and it is just clutter.
> Using "2" makes sense when it's a straightforward updating of an old
> API whose name was fine. But when the function's meaning is
> changing significantly, or when the old name is agreed to be
> unsatisfactory, then we can simply choose a better name, with no "2"
> suffix.
>
> Either way, the old, deprecated function can refer to the new one.
Agreed.
> P.S. I've held off putting anything in HACKING about this until the
> thread reaches some sort of obvious consensus :-).
Well, I still consider the approach here unconventional, but by no
means unsound. So as long as it is documented, it's all rose petals
and vanilla ice-cream.
-- Naked
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Naming of function versions (Was: Re: [PATCH] Re: The new svn_wc_adm_*_depth functions.)
Posted by kf...@collab.net.
Nuutti Kotivuori <na...@iki.fi> writes:
> Now, I am not saying that deprecating the non-'2' variants of the
> functions would be strictly wrong. There are many ways to look at the
> matter and many ways to view deprecation - but these are my views on
> the matter.
>
> To me, deprecation an awfully strong tag when placed on a function. It
> is something that should chime a warning immediately. It is like
> saying that "You should not be using this function."
That's the idea. We want people writing new code to use the new API,
unless they're explicitly trying to be compatible with older
Subversions.
> But there is a reason to keep on using that function. Considering the
> APR versioning guidelines, it is quite okay to write code against 1.1
> headers that works okay on 1.0 if you don't use any of the new
> features provided by the 1.1 API. Hence everything the 1.1 API
> provides should be just that - additional functions that you can use
> if you want to take advantage of the *new* features, knowing that you
> are then dependant on that minor version.
Anyone who wants/expects their code to compile against 1.0.x needs to
test it with 1.0.x. The whole reason we have these guidelines is so
people will know what's promised and what's not. We didn't promise
that writing against the 1.1 API will automatically work with 1.0.
I think if we use your interpretation, the burden on development is
simply too great. We've been very clear about what's guaranteed and
what's not; let's not impose more burden on ourselves for the sake of
an entirely avoidable use case.
The problem you describe can be solved by simply documenting the facts
(as you recommend yourself, a bit farther on in your post...).
> And like already mentioned, deprecation usually means that function
> will be removed in the near future. Specifying that this only means
> the current form of the function, and not that the function name is
> going away, is confusing to people who use the library.
I grant that it's confusing, until one understands that "2.0" means
"full reset". When that happens, we can easily publish a table
showing how to adjust calling code.
> As a solution to the problem, I propose just being a bit more verbose
> and not using the deprecated tag.
>
> Simply explaining the situation in the documentation will give the
> users the best grounds on making the decision over using the newer
> function and forfeiting older minor version compatibility or just
> sticking with the old - and also giving them an idea what is going to
> happen in the future. As an example, in the WC administrative
> directory lock depth case, the documentation string could simply point
> the user to a respective svn_wc_adm_foo2 function, point out that it
> was added since 1.1.0 (or development leading to 1.1.0) and that 2.0
> will most likely have the svn_wc_adm_foo function with the parameters
> of svn_wc_adm_foo2.
Sure! We should continue using "@deprecated", because it gives us a
well-defined string to search for. But there's no reason the
additional documentation can't be present. It can only help. That
way if someone *wants* to write compatibly with the 1.0.x API, they
can do so, even if only 1.1.x is present on their system.
> As for the actual naming issue when a function interface is "improved"
> - it can be argued both ways. Descriptive names such as *_depth point
> out exactly what can be expected of the new functions, which is an
> important point in my opinion. Coming up with descriptive suffixes can
> be hard, though. Then again, naming them *2 underlines the fact that
> they are just newer revisions of the same functions, and there is
> never any question which alternative is the latest and greatest. But
> it is not descriptive at all and people might have a harder time
> deciding if they want to use the older or newer function.
We should decide on a case-by-case basis. (See my comments in issue
#1552, for example.)
Using "2" makes sense when it's a straightforward updating of an old
API whose name was fine. But when the function's meaning is changing
significantly, or when the old name is agreed to be unsatisfactory,
then we can simply choose a better name, with no "2" suffix.
Either way, the old, deprecated function can refer to the new one.
-Karl
P.S. I've held off putting anything in HACKING about this until the
thread reaches some sort of obvious consensus :-).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Naming of function versions (Was: Re: [PATCH] Re: The new
svn_wc_adm_*_depth functions.)
Posted by Nuutti Kotivuori <na...@iki.fi>.
D. J. Heap wrote:
> After thinking about it some more, I still like the '2' variant idea
> and then resetting them back to the normal name in a major version
> change -- but as a client it does seem a bit confusing. That is,
> the functions that are marked deprecated are not actually going to
> be removed. In fact, they will become the preferred function again
> with a slightly different parameter list and the '2' variants (which
> were not deprecated) will be removed.
>
> I don't have a better idea, but it does seem a little odd to me.
> Maybe just add a more detailed note about this to the doc comments
> or something?
Well, I personally don't like naming functions with a '2' suffix too
much, but regardless of that, I think the deprecation needs a bit more
thought.
I will quote a few paragraphs from the APR versioning document for
reference:
,----[ Replacing functions ]
| This gets a bit trickier. The original function must remain
| available at the link-level so that an application compiled
| against a minor version will continue to work with later minor
| versions. Further, if an application is designed to work with an
| earlier minor version, then we don't want to suddenly change the
| requirements for that application. This means that the headers
| cannot silently map an old function into a newer function, as that
| would turn an application, say, based on 1.2 into an application
| requiring the 1.4 or later release.
|
| This means that functions cannot truly be replaced. The new,
| alternate function can be made available in the header and
| applications can choose to use it (and become dependent upon the
| minor release where the function appears).
|
| It is possible to design a set of headers where a macro will
| always refer to the "latest" function available. Of course, if an
| application chooses to use this macro, then the resulting
| compiled-binary will be dependent upon whatever version it was
| compiled against. This strategy adds the new functionality for
| applications, yet retains the necessary source and binary
| compatibility for applications designed or built against previous
| minor releases.
`----
,----[ Deprecating functions ]
| ...
|
| Note that all deprecated functions will be removed at the next major
| version bump.
`----
Now, I am not saying that deprecating the non-'2' variants of the
functions would be strictly wrong. There are many ways to look at the
matter and many ways to view deprecation - but these are my views on
the matter.
To me, deprecation an awfully strong tag when placed on a function. It
is something that should chime a warning immediately. It is like
saying that "You should not be using this function."
But there is a reason to keep on using that function. Considering the
APR versioning guidelines, it is quite okay to write code against 1.1
headers that works okay on 1.0 if you don't use any of the new
features provided by the 1.1 API. Hence everything the 1.1 API
provides should be just that - additional functions that you can use
if you want to take advantage of the *new* features, knowing that you
are then dependant on that minor version.
And like already mentioned, deprecation usually means that function
will be removed in the near future. Specifying that this only means
the current form of the function, and not that the function name is
going away, is confusing to people who use the library.
I would like to use deprecation only for functions that have a strong
point against using them - something like that they will not work as
expected, or have security implications or whatever. Functions that if
Subversion library users find themselves using, they should consider
carefully the alternatives, even if it might mean that they will
require a newer minor version to work after that.
As a solution to the problem, I propose just being a bit more verbose
and not using the deprecated tag.
Simply explaining the situation in the documentation will give the
users the best grounds on making the decision over using the newer
function and forfeiting older minor version compatibility or just
sticking with the old - and also giving them an idea what is going to
happen in the future. As an example, in the WC administrative
directory lock depth case, the documentation string could simply point
the user to a respective svn_wc_adm_foo2 function, point out that it
was added since 1.1.0 (or development leading to 1.1.0) and that 2.0
will most likely have the svn_wc_adm_foo function with the parameters
of svn_wc_adm_foo2.
But most importantly, it still drives the point home and does not make
anyone jumpy over the deprecation issue.
As for the actual naming issue when a function interface is "improved"
- it can be argued both ways. Descriptive names such as *_depth point
out exactly what can be expected of the new functions, which is an
important point in my opinion. Coming up with descriptive suffixes can
be hard, though. Then again, naming them *2 underlines the fact that
they are just newer revisions of the same functions, and there is
never any question which alternative is the latest and greatest. But
it is not descriptive at all and people might have a harder time
deciding if they want to use the older or newer function.
-- Naked
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: The new svn_wc_adm_*_depth functions.
Posted by "D.J. Heap" <dj...@shadyvale.net>.
kfogel@collab.net wrote:
> "D.J. Heap" <dj...@shadyvale.net> writes:
>
>>It looks like an agreement is forming on the '2' variants (it sounds
>>reasonable to me) -- if there are no objections then I will wait
>>another day or so and update the patch to rename the functions with
>>that scheme and submit it for review?
>
>
> Sounds good. Thanks!
After thinking about it some more, I still like the '2' variant idea and
then resetting them back to the normal name in a major version change --
but as a client it does seem a bit confusing. That is, the functions
that are marked deprecated are not actually going to be removed. In
fact, they will become the preferred function again with a slightly
different parameter list and the '2' variants (which were not
deprecated) will be removed.
I don't have a better idea, but it does seem a little odd to me. Maybe
just add a more detailed note about this to the doc comments or something?
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: The new svn_wc_adm_*_depth functions.
Posted by kf...@collab.net.
"D.J. Heap" <dj...@shadyvale.net> writes:
> It looks like an agreement is forming on the '2' variants (it sounds
> reasonable to me) -- if there are no objections then I will wait
> another day or so and update the patch to rename the functions with
> that scheme and submit it for review?
Sounds good. Thanks!
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: The new svn_wc_adm_*_depth functions.
Posted by "D.J. Heap" <dj...@shadyvale.net>.
kfogel@collab.net wrote:
[snip]
>
> If we're going to fall back to the old names, then the "2" variants
> might make the most sense.
>
> IMHO, we should do this. By resetting our APIs back to the
> names-of-first-choice, we avoid names growing arbitrarily long (think
> 3.0).
>
> Thoughts?
It looks like an agreement is forming on the '2' variants (it sounds
reasonable to me) -- if there are no objections then I will wait another
day or so and update the patch to rename the functions with that scheme
and submit it for review?
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: The new svn_wc_adm_*_depth functions.
Posted by kf...@collab.net.
Greg Hudson <gh...@MIT.EDU> writes:
> With one set of functions, we're adding foo_depth() variants which
> include a depth argument.
>
> With another set of functions, we didn't want to add foo_pool() variants
> because that looks dumb, so we're adding foo2() variants instead.
>
> I would like us to be consistent.
Good point. A question that impinges on this:
Are we planning to keep these suffixes in 2.0? Or will we change the
new names to the old names (just keep the new APIs) when we cross the
2.0 line?
If we're going to fall back to the old names, then the "2" variants
might make the most sense.
IMHO, we should do this. By resetting our APIs back to the
names-of-first-choice, we avoid names growing arbitrarily long (think
3.0).
Thoughts?
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: The new svn_wc_adm_*_depth functions.
Posted by John Peacock <jp...@rowman.com>.
Greg Hudson wrote:
> With one set of functions, we're adding foo_depth() variants which
> include a depth argument.
>
> With another set of functions, we didn't want to add foo_pool() variants
> because that looks dumb, so we're adding foo2() variants instead.
>
> I would like us to be consistent.
>
Actually, in my case I would like to make both instances semi-public, with the
wrapper remaining public, which is a slightly different case. I'll go along
with whatever scheme reaches consensus first. ;~)
John
--
John Peacock
Director of Information Research and Technology
Rowman & Littlefield Publishing Group
4501 Forbes Boulevard
Suite H
Lanham, MD 20706
301-459-3366 x.5010
fax 301-429-5748
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: The new svn_wc_adm_*_depth functions.
Posted by Greg Hudson <gh...@MIT.EDU>.
Hi. I'm sorry to step in at this late date with a dumb cosmetic issue,
but I'm watching two list conversations about API enhancements which
seem to be going in different naming directions.
With one set of functions, we're adding foo_depth() variants which
include a depth argument.
With another set of functions, we didn't want to add foo_pool() variants
because that looks dumb, so we're adding foo2() variants instead.
I would like us to be consistent.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: The new svn_wc_adm_*2 functions.
Posted by "D.J. Heap" <dj...@shadyvale.net>.
kfogel@collab.net wrote:
> "D.J. Heap" <dj...@shadyvale.net> writes:
>
>>New patch using the '2' style for the new functions.
>
>
> I haven't reviewed the whole thing, but I assume it's just the obvious
> transformation to your original patch, so +1.
Yep.
>
> Enough with the posting and the patches and the hey hey hey, though :-).
>>From r9031, you are a full committer (as per the secret discussions
> among the other full committers, in the smoke-filled room in the
> Undisclosed Location, you know the place).
Thanks!
>
> Obviously, feel free to post anything for review before committing
> anytime you want -- but it's your judgement call now.
>
> -K
Yes, I'm most comfortable doing that except for very trivial changes,
perhaps.
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: The new svn_wc_adm_*2 functions.
Posted by kf...@collab.net.
"D.J. Heap" <dj...@shadyvale.net> writes:
> New patch using the '2' style for the new functions.
I haven't reviewed the whole thing, but I assume it's just the obvious
transformation to your original patch, so +1.
Enough with the posting and the patches and the hey hey hey, though :-).
[PATCH] Re: The new svn_wc_adm_*2 functions.
Posted by "D.J. Heap" <dj...@shadyvale.net>.
New patch using the '2' style for the new functions.
Log:
Rename the new *_depth functions, update call sites (and some
comments), and mark the old functions as deprecated.
* subversion\include\svn_wc.h
Rename new svn_wc_adm_*_depth functions to svn_wc_adm_*2 and
mark the old functions as deprecated.
* subversion\libsvn_wc\wc.h
* subversion\libsvn_wc\copy.c
* subversion\libsvn_wc\log.c
* subversion\libsvn_wc\adm_ops.c
* subversion\libsvn_wc\status.c
* subversion\libsvn_wc\adm_files.c
* subversion\libsvn_wc\lock.c
* subversion\libsvn_wc\update_editor.c
* subversion\libsvn_client\relocate.c
* subversion\libsvn_client\switch.c
* subversion\libsvn_client\delete.c
* subversion\libsvn_client\externals.c
* subversion\libsvn_client\export.c
* subversion\libsvn_client\revisions.c
* subversion\libsvn_client\status.c
* subversion\libsvn_client\prop_commands.c
* subversion\libsvn_client\url.c
* subversion\libsvn_client\checkout.c
* subversion\libsvn_client\revert.c
* subversion\libsvn_client\diff.c
* subversion\libsvn_client\copy.c
* subversion\libsvn_client\log.c
* subversion\libsvn_client\update.c
* subversion\libsvn_client\resolved.c
* subversion\libsvn_client\commit_util.c
* subversion\libsvn_client\add.c
* subversion\libsvn_client\commit.c
* subversion\clients\cmdline\switch-cmd.c
* subversion\clients\cmdline\main.c
* subversion\clients\cmdline\propedit-cmd.c
* subversion\clients\cmdline\info-cmd.c
Update svn_wc_adm_* calls and comments to the new depth aware
versions of those functions.
Re: [PATCH] Re: The new svn_wc_adm_*_depth functions.
Posted by kf...@collab.net.
"D.J. Heap" <dj...@shadyvale.net> writes:
> The latest iteration...
>
> Change calls from the old svn_wc_adm_* functions to the new
> svn_wc_adm_*_depth functions and mark the old functions as deprecated.
+1 to commit. I reviewed every flag change; if there's anything wrong
here, we both missed it :-).
Thanks for doing this cleanup! I've filed issue #1783 as a reminder
to removed deprecated APIs for 2.0. (You don't need to add anything
to the issue, I'm just mentioning it so you know it's done.)
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
[PATCH] Re: The new svn_wc_adm_*_depth functions.
Posted by "D.J. Heap" <dj...@shadyvale.net>.
The latest iteration...
Log:
Change calls from the old svn_wc_adm_* functions to the new
svn_wc_adm_*_depth functions and mark the old functions as deprecated.
* subversion/include/svn_wc.h
(svn_wc_adm_open, svn_wc_adm_probe_open, svn_wc_adm_probe_try)
Mark as deprecated.
* subversion/libsvn_wc/wc.h
Tweak comment for svn_wc__adm_pre_open to refer to new function.
* subversion/libsvn_wc/copy.c
* subversion/libsvn_wc/log.c
* subversion/libsvn_wc/adm_ops.c
* subversion/libsvn_wc/status.c
* subversion/libsvn_wc/adm_files.c
* subversion/libsvn_wc/update_editor.c
* subversion/libsvn_wc/lock.c
* subversion/libsvn_client/switch.c
* subversion/libsvn_client/delete.c
* subversion/libsvn_client/externals.c
* subversion/libsvn_client/export.c
* subversion/libsvn_client/revisions.c
* subversion/libsvn_client/status.c
* subversion/libsvn_client/prop_commands.c
* subversion/libsvn_client/checkout.c
* subversion/libsvn_client/revert.c
* subversion/libsvn_client/diff.c
* subversion/libsvn_client/copy.c
* subversion/libsvn_client/log.c
* subversion/libsvn_client/update.c
* subversion/libsvn_client/resolved.c
* subversion/libsvn_client/commit_util.c
* subversion/libsvn_client/add.c
* subversion/libsvn_client/commit.c
* subversion/libsvn_client/relocate.c
* subversion/libsvn_client/url.c
* subversion/clients/cmdline/switch-cmd.c
* subversion/clients/cmdline/main.c
* subversion/clients/cmdline/propedit-cmd.c
* subversion/clients/cmdline/info-cmd.c
Call the new depth aware svn_wc_adm_* functions.
Re: The new svn_wc_adm_*_depth functions.
Posted by "D.J. Heap" <dj...@shadyvale.net>.
Philip Martin wrote:
> Jon Foster <jo...@jon-foster.co.uk> writes:
>
>
>>>Index: subversion/libsvn_client/status.c
>>>===================================================================
>>>--- subversion/libsvn_client/status.c (revision 8957)
>>>+++ subversion/libsvn_client/status.c (working copy)
>>>@@ -155,8 +155,8 @@
>>> /* Using pool cleanup to close it. This needs to be recursive so that
>>> auth data can be stored. */
>>> if (strlen (anchor) != strlen (path))
>>>- SVN_ERR (svn_wc_adm_open (&anchor_access, NULL, anchor,
>>>FALSE, - TRUE, pool));
>>>+ SVN_ERR (svn_wc_adm_open_depth (&anchor_access, NULL,
>>>anchor, FALSE, + -1, pool));
>>
>>Does this *really* need to be recursive? The comment says so; I don't
>>know enough about the auth mechanism to be sure.
>
>
> The comment is out of date, it refers to an era when auth data was
> stored in the working copy. There may be other reasons why the
> recursive lock is required.
>
I also noticed a number of places that looked like they might not really
need a recursive lock. Once this patch (or some semblance of it) is
applied, I have a list I planned on looking at more closely in the near
future -- in particular the property stuff since it's performance is
another major annoyance on our large working copies where we use some
custom properties.
For this future optimizing, does someone know of a list somewhere of the
reasons why a full recursive lock is needed for operations that only
really use the immediate children of a directory? Such as the way the
auth info used to need it or other things like that? I haven't noticed
any such documentation, but I could have missed it.
I'm updating this patch (for Julian's comments) and will repost it later
tonight with a better subject.
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: The new svn_wc_adm_*_depth functions.
Posted by Philip Martin <ph...@codematters.co.uk>.
Jon Foster <jo...@jon-foster.co.uk> writes:
>> Index: subversion/libsvn_client/status.c
>> ===================================================================
>> --- subversion/libsvn_client/status.c (revision 8957)
>> +++ subversion/libsvn_client/status.c (working copy)
>> @@ -155,8 +155,8 @@
>> /* Using pool cleanup to close it. This needs to be recursive so that
>> auth data can be stored. */
>> if (strlen (anchor) != strlen (path))
>> - SVN_ERR (svn_wc_adm_open (&anchor_access, NULL, anchor,
>> FALSE, - TRUE, pool));
>> + SVN_ERR (svn_wc_adm_open_depth (&anchor_access, NULL,
>> anchor, FALSE, + -1, pool));
>
> Does this *really* need to be recursive? The comment says so; I don't
> know enough about the auth mechanism to be sure.
The comment is out of date, it refers to an era when auth data was
stored in the working copy. There may be other reasons why the
recursive lock is required.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: The new svn_wc_adm_*_depth functions.
Posted by Jon Foster <jo...@jon-foster.co.uk>.
Hi,
D.J. Heap wrote:
> Change calls from the old svn_wc_adm_* functions to the new
> svn_wc_adm_*_depth functions and mark the old functions as deprecated.
Thanks for doing this. One of the things I've been meaning to do is
audit all uses of these functions to check if we're locking too much
anywhere else. Your patch made this easy.
Your patch looks fine (it's effectively a no-op), my comments below
are suggesting changes that can be done later in different patches.
> Index: subversion/libsvn_client/status.c
> ===================================================================
> --- subversion/libsvn_client/status.c (revision 8957)
> +++ subversion/libsvn_client/status.c (working copy)
> @@ -155,8 +155,8 @@
> /* Using pool cleanup to close it. This needs to be recursive so that
> auth data can be stored. */
> if (strlen (anchor) != strlen (path))
> - SVN_ERR (svn_wc_adm_open (&anchor_access, NULL, anchor, FALSE,
> - TRUE, pool));
> + SVN_ERR (svn_wc_adm_open_depth (&anchor_access, NULL, anchor, FALSE,
> + -1, pool));
Does this *really* need to be recursive? The comment says so; I don't
know enough about the auth mechanism to be sure.
If it does need to be recursive, did my previous patch introduce a
subtle bug when anchor==path and update=TRUE?
> Index: subversion/libsvn_client/prop_commands.c
> ===================================================================
> @@ -156,7 +156,8 @@
> - SVN_ERR (svn_wc_adm_probe_open (&adm_access, NULL, target, TRUE, TRUE, pool));
> + SVN_ERR (svn_wc_adm_probe_open_depth (&adm_access, NULL, target, TRUE,
> + -1, pool));
> @@ -554,8 +556,8 @@
> - SVN_ERR (svn_wc_adm_probe_open (&adm_access, NULL, target, FALSE, TRUE,
> - pool));
> + SVN_ERR (svn_wc_adm_probe_open_depth (&adm_access, NULL, target,
> + FALSE, -1, pool));
> @@ -911,8 +913,8 @@
> - SVN_ERR (svn_wc_adm_probe_open (&adm_access, NULL, target, FALSE, TRUE,
> - pool));
> + SVN_ERR (svn_wc_adm_probe_open_depth (&adm_access, NULL, target,
> + FALSE, -1, pool));
Why do setting, getting and listing properties require a recursive lock?
Surely this can be non-recursive?
> Index: subversion/libsvn_client/update.c
> ===================================================================
> --- subversion/libsvn_client/update.c (revision 8957)
> +++ subversion/libsvn_client/update.c (working copy)
> @@ -74,7 +74,8 @@
> /* Use PATH to get the update's anchor and targets and get a write lock */
> SVN_ERR (svn_wc_get_actual_target (path, &anchor, &target, pool));
> - SVN_ERR (svn_wc_adm_open (&adm_access, NULL, anchor, TRUE, TRUE, pool));
> + SVN_ERR (svn_wc_adm_open_depth (&adm_access, NULL, anchor, TRUE, -1,
> + pool));
Non-recursive updates require a recursive lock? Is this related to the
"auth data can be stored" comment in status.c?
Kind regards,
Jon Foster
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: The new svn_wc_adm_*_depth functions.
Posted by "D.J. Heap" <dj...@shadyvale.net>.
Julian Foad wrote:
> D.J. Heap wrote:
>
>>
>> Log:
>>
>> Change calls from the old svn_wc_adm_* functions to the new
>> svn_wc_adm_*_depth functions and mark the old functions as deprecated.
[snip]
>>
>> * subversion/libsvn_client/relocate.c
>> * subversion/libsvn_client/url.c
>> Call the new depth aware svn_wc_adm_* functions and set the
>> eol-style property to native.
>
>
> That last comment is a confusing. Setting the files' EOL style to
> 'native' is completely unrelated and should be done in a separate
> patch. There are several other files in our repository that need this
> setting to be made, too. I will do it.
>
> - Julian
Thanks! I will update the patch and re-submit, also fixing the subject.
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: The new svn_wc_adm_*_depth functions.
Posted by Julian Foad <ju...@btopenworld.com>.
D.J. Heap wrote:
>
> Log:
>
> Change calls from the old svn_wc_adm_* functions to the new
> svn_wc_adm_*_depth functions and mark the old functions as deprecated.
>
> * subversion/include/svn_wc.h
> (svn_wc_adm_open, svn_wc_adm_probe_open, svn_wc_adm_probe_try)
> Mark as deprecated.
[...]
> * subversion/clients/cmdline/main.c
> * subversion/clients/cmdline/propedit-cmd.c
> * subversion/clients/cmdline/info-cmd.c
> Call the new depth aware svn_wc_adm_* functions.
>
> * subversion/libsvn_client/relocate.c
> * subversion/libsvn_client/url.c
> Call the new depth aware svn_wc_adm_* functions and set the
> eol-style property to native.
That last comment is a confusing. Setting the files' EOL style to 'native' is completely unrelated and should be done in a separate patch. There are several other files in our repository that need this setting to be made, too. I will do it.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: The new svn_wc_adm_*_depth functions.
Posted by "D.J. Heap" <dj...@shadyvale.net>.
subversion@millenix.mailshell.com wrote:
[snip]
>
> Perhaps this should mention "with 1.0 release" so that anyone reading
> the code can determine when it can and/or will be removed according to
> the compatibility guidelines?
[snip]
Yes, and I missed another comment, also, thanks. Updated patch attached.
Log:
Change calls from the old svn_wc_adm_* functions to the new
svn_wc_adm_*_depth functions and mark the old functions as deprecated.
* subversion/include/svn_wc.h
(svn_wc_adm_open, svn_wc_adm_probe_open, svn_wc_adm_probe_try)
Mark as deprecated.
* subversion/libsvn_wc/wc.h
Tweak comment for svn_wc__adm_pre_open to refer to new function.
* subversion/libsvn_wc/copy.c
* subversion/libsvn_wc/log.c
* subversion/libsvn_wc/adm_ops.c
* subversion/libsvn_wc/status.c
* subversion/libsvn_wc/adm_files.c
* subversion/libsvn_wc/update_editor.c
* subversion/libsvn_client/switch.c
* subversion/libsvn_client/delete.c
* subversion/libsvn_client/externals.c
* subversion/libsvn_client/export.c
* subversion/libsvn_client/revisions.c
* subversion/libsvn_client/status.c
* subversion/libsvn_client/prop_commands.c
* subversion/libsvn_client/checkout.c
* subversion/libsvn_client/revert.c
* subversion/libsvn_client/diff.c
* subversion/libsvn_client/copy.c
* subversion/libsvn_client/log.c
* subversion/libsvn_client/update.c
* subversion/libsvn_client/resolved.c
* subversion/libsvn_client/commit_util.c
* subversion/libsvn_client/add.c
* subversion/libsvn_client/commit.c
* subversion/clients/cmdline/switch-cmd.c
* subversion/clients/cmdline/main.c
* subversion/clients/cmdline/propedit-cmd.c
* subversion/clients/cmdline/info-cmd.c
Call the new depth aware svn_wc_adm_* functions.
* subversion/libsvn_client/relocate.c
* subversion/libsvn_client/url.c
Call the new depth aware svn_wc_adm_* functions and set the
eol-style property to native.
Re: The new svn_wc_adm_*_depth functions.
Posted by su...@millenix.mailshell.com.
D.J. Heap wrote:
> [snip message and log]
> Index: subversion/include/svn_wc.h
> ===================================================================
> --- subversion/include/svn_wc.h (revision 8957)
> +++ subversion/include/svn_wc.h (working copy)
> @@ -107,10 +107,10 @@
> apr_pool_t *pool);
>
> /**
> - * Similar to svn_wc_adm_open_depth(). @a depth is set to -1 if @a tree_lock is
> - * @c TRUE, else 0.
> + * @deprecated Provided for backward compatibility.
Perhaps this should mention "with 1.0 release" so that anyone reading the
code can determine when it can and/or will be removed according to the
compatibility guidelines?
> *
> - * Provided for backward compatibility with the Subversion 1.0.0 API.
Note that that info appeared in the old comment.
Philip Miller
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: The new svn_wc_adm_*_depth functions.
Posted by "D.J. Heap" <dj...@shadyvale.net>.
D.J. Heap wrote:
> kfogel@collab.net wrote:
[snip]
>> Okay -- you're way ahead of me, I hereby back off :-). Go for it,
>> I'll watch for the patch here. If you want to mark the old APIs as
>> deprecated in the same patch, IMHO that's fine.
>>
>> -K
[snip]
Here's the patch.
Log:
Change calls from the old svn_wc_adm_* functions to the new
svn_wc_adm_*_depth functions and mark the old functions as deprecated.
* subversion/include/svn_wc.h
(svn_wc_adm_open, svn_wc_adm_probe_open, svn_wc_adm_probe_try)
Mark as deprecated.
* subversion/libsvn_wc/copy.c
* subversion/libsvn_wc/log.c
* subversion/libsvn_wc/adm_ops.c
* subversion/libsvn_wc/status.c
* subversion/libsvn_wc/adm_files.c
* subversion/libsvn_wc/update_editor.c
* subversion/libsvn_client/switch.c
* subversion/libsvn_client/delete.c
* subversion/libsvn_client/externals.c
* subversion/libsvn_client/export.c
* subversion/libsvn_client/revisions.c
* subversion/libsvn_client/status.c
* subversion/libsvn_client/prop_commands.c
* subversion/libsvn_client/checkout.c
* subversion/libsvn_client/revert.c
* subversion/libsvn_client/diff.c
* subversion/libsvn_client/copy.c
* subversion/libsvn_client/log.c
* subversion/libsvn_client/update.c
* subversion/libsvn_client/resolved.c
* subversion/libsvn_client/commit_util.c
* subversion/libsvn_client/add.c
* subversion/libsvn_client/commit.c
* subversion/clients/cmdline/switch-cmd.c
* subversion/clients/cmdline/main.c
* subversion/clients/cmdline/propedit-cmd.c
* subversion/clients/cmdline/info-cmd.c
Call the new depth aware svn_wc_adm_* functions.
* subversion/libsvn_client/relocate.c
* subversion/libsvn_client/url.c
Call the new depth aware svn_wc_adm_* functions and set the
eol-style property to native.
Re: The new svn_wc_adm_*_depth functions.
Posted by "D.J. Heap" <dj...@shadyvale.net>.
kfogel@collab.net wrote:
> "D.J. Heap" <dj...@shadyvale.net> writes:
>
>>As far as converting Subversion to use these new functions so they can
>>be truly 'deprecated', I am willing to do that and submit a patch --
>>it should be quite straightforward as far as I can see.
>
>
> Okay -- you're way ahead of me, I hereby back off :-). Go for it,
> I'll watch for the patch here. If you want to mark the old APIs as
> deprecated in the same patch, IMHO that's fine.
>
> -K
Hmm, sorry if I came off as being aggressive about it, I did not mean to
be. I just feel a certain amount of responsibility to see it through
and make sure everyone is happy with it since I was a vocal supporter
and committer. And I certainly didn't want to make more work for
someone else.
I have a patch finished and am running it through the tests now. Are
the function/parameter names ok, or does someone have a better idea?
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: The new svn_wc_adm_*_depth functions.
Posted by kf...@collab.net.
"D.J. Heap" <dj...@shadyvale.net> writes:
> As far as converting Subversion to use these new functions so they can
> be truly 'deprecated', I am willing to do that and submit a patch --
> it should be quite straightforward as far as I can see.
Okay -- you're way ahead of me, I hereby back off :-). Go for it,
I'll watch for the patch here. If you want to mark the old APIs as
deprecated in the same patch, IMHO that's fine.
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: The new svn_wc_adm_*_depth functions.
Posted by "D.J. Heap" <dj...@shadyvale.net>.
kfogel@collab.net wrote:
> Philip Martin <ph...@codematters.co.uk> writes:
>
>>>This seems to be saying you agree with my proposal to deprecate, then.
>>
>>It might be a little premature, Subversion itself is still using the
>>non-depth interface.
>
>
> The 'depth' parameter introduced in svn_wc_adm_*_depth doesn't really
> correspond to the '--depth' option we've talked about in the past.
> Maybe that's an argument for renaming the parameter... Or a parameter
> for renaming the argument?
>
> Excuse me, I get carried away sometimes.
>
> Anyway, I think that despite both using the word "depth", these two
> things aren't really tied to each other. From svn_wc.h:
>
[snip]
But it's at least somewhat related since it's a pre-requisite to the
--depth option, isn't it? (At least for working copy operations, though
not for direct repo operations.)
For example, in order to do an arbitrary depth 'svn status' or 'svn
commit' or 'svn update' all these commands need a way to do get an
arbitrary depth 'lock' on the working copy which is what these new
functions can do.
Or am I misunderstanding you, and you are saying they are not really
required since you can just take out a whole tree lock and then just
work on the depth you care about? While true, that has some performance
problems which is why this patch was submitted. Or perhaps I'm just
completely missing the point?
As far as converting Subversion to use these new functions so they can
be truly 'deprecated', I am willing to do that and submit a patch -- it
should be quite straightforward as far as I can see.
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: The new svn_wc_adm_*_depth functions.
Posted by kf...@collab.net.
Philip Martin <ph...@codematters.co.uk> writes:
> > This seems to be saying you agree with my proposal to deprecate, then.
>
> It might be a little premature, Subversion itself is still using the
> non-depth interface.
The 'depth' parameter introduced in svn_wc_adm_*_depth doesn't really
correspond to the '--depth' option we've talked about in the past.
Maybe that's an argument for renaming the parameter... Or a parameter
for renaming the argument?
Excuse me, I get carried away sometimes.
Anyway, I think that despite both using the word "depth", these two
things aren't really tied to each other. From svn_wc.h:
* @a depth specifies how much to lock. Zero means just the specified
* directory. Any negative value means to lock the entire working copy
* directory hierarchy under @a path. A positive value indicates the number of
* levels of directories to lock - 1 means just immediate subdirectories, 2
* means immediate subdirectories and their subdirectories, etc. All the
* access batons will become part of the set containing @a adm_access. This
* is an all-or-nothing option, if it is not possible to lock all the
* requested directories then an error will be returned and @a adm_access will
* be invalid, with the exception that subdirectories of @a path that are
* missing from the physical filesystem will not be locked and will not cause
* an error. The error @c SVN_ERR_WC_LOCKED will be returned if a
* subdirectory of @a path is already write locked.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: The new svn_wc_adm_*_depth functions.
Posted by Philip Martin <ph...@codematters.co.uk>.
kfogel@collab.net writes:
> This seems to be saying you agree with my proposal to deprecate, then.
It might be a little premature, Subversion itself is still using the
non-depth interface.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: The new svn_wc_adm_*_depth functions.
Posted by kf...@collab.net.
Philip Martin <ph...@codematters.co.uk> writes:
> If this patch had appeared pre-1.0 I'd have replaced the non-depth
> interface with the depth one, i.e. I'd have made svn_wc_adm_open take
> a depth argument. If we ever produce the fabled "new libsvn_wc" then
> it will probably use a completely different interface without any of
> the current names--well, that's true for the one I'd write. I don't
> know whether that's a 2.0 thing.
This seems to be saying you agree with my proposal to deprecate, then.
(And if we rewrite libsvn_wc for 2.0, that's just more deprecation.
Uh, *much* more deprecation :-) ).
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: The new svn_wc_adm_*_depth functions.
Posted by Philip Martin <ph...@codematters.co.uk>.
kfogel@collab.net writes:
> Is it our intention that these new functions
>
> svn_wc_adm_open_depth
> svn_wc_adm_probe_try_depth
> svn_wc_adm_probe_open_depth
>
> supersede their non-depth predecessors, and that (eventually) all
> callers of the predecessors be converted to use the depth versions? I
> would think so, but just want to make sure here first.
>
> If so, it's time to mark the predecessors
>
> svn_wc_adm_open
> svn_wc_adm_probe_try
> svn_wc_adm_probe_open
>
> as 'deprecated'. This should be pretty simple: just make a special
> 'deprecated' marker that's easy to grep for, and open a single new
> issue in the 2.0 milestone:
If this patch had appeared pre-1.0 I'd have replaced the non-depth
interface with the depth one, i.e. I'd have made svn_wc_adm_open take
a depth argument. If we ever produce the fabled "new libsvn_wc" then
it will probably use a completely different interface without any of
the current names--well, that's true for the one I'd write. I don't
know whether that's a 2.0 thing.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: The new svn_wc_adm_*_depth functions.
Posted by kf...@collab.net.
"D.J. Heap" <dj...@shadyvale.net> writes:
> I can do this and submit a patch tonight or tomorrow if it's not
> time-critical, which I wouldn't think it would be. Is there a
> standard 'deprecated' marker already? I seem to recall a similar
> discussion, but I can't find it and don't see anything in HACKING.
Adding this material to HACKING is part of what I'd planned, so maybe
it's simpler if I just do it (thanks though).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: The new svn_wc_adm_*_depth functions.
Posted by kf...@collab.net.
"Chris Foote" <cf...@v21.me.uk> writes:
> You could use the standard one provided by doxygen. i.e. @deprecated
> This has the advantage that it will be documented consistently and you can
> also provide a description.
>
> For more see:
> http://www.stack.nl/~dimitri/doxygen/commands.html#cmddeprecated
I was planning to look for just such a beast, now I don't have to --
thanks for the pointer.
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: The new svn_wc_adm_*_depth functions.
Posted by Chris Foote <cf...@v21.me.uk>.
You could use the standard one provided by doxygen. i.e. @deprecated
This has the advantage that it will be documented consistently and you can
also provide a description.
For more see:
http://www.stack.nl/~dimitri/doxygen/commands.html#cmddeprecated
Regards
Chris
----- Original Message -----
From: "D.J. Heap" <dj...@shadyvale.net>
To: <kf...@collab.net>
Cc: <de...@subversion.tigris.org>
Sent: Monday, March 08, 2004 6:36 PM
Subject: Re: The new svn_wc_adm_*_depth functions.
> kfogel@collab.net wrote:
> > Is it our intention that these new functions
> >
> > svn_wc_adm_open_depth
> > svn_wc_adm_probe_try_depth
> > svn_wc_adm_probe_open_depth
> >
> > supersede their non-depth predecessors, and that (eventually) all
> > callers of the predecessors be converted to use the depth versions? I
> > would think so, but just want to make sure here first.
>
>
> I believe so, based on past discussions and the need they address.
>
>
> >
> > If so, it's time to mark the predecessors
> >
> > svn_wc_adm_open
> > svn_wc_adm_probe_try
> > svn_wc_adm_probe_open
> >
> > as 'deprecated'. This should be pretty simple: just make a special
> > 'deprecated' marker that's easy to grep for, and open a single new
> > issue in the 2.0 milestone:
> >
> > "Remove deprecated 1.x interfaces."
> >
> > That issue will give instructions to grep for the special marker. It
> > doesn't really matter whether we convert all the predecessors now, or
> > just make a note in the deprecated function's doc string that it needs
> > to be done before removing the function. (I'd prefer to do it now, if
> > it's not a big task, though, to avoid having bad examples in our
> > code.)
> >
> > I can take care of all this; I just want to make sure it's what we
> > intend.
> >
> > -Karl
>
>
> I can do this and submit a patch tonight or tomorrow if it's not
> time-critical, which I wouldn't think it would be. Is there a standard
> 'deprecated' marker already? I seem to recall a similar discussion, but
> I can't find it and don't see anything in HACKING.
>
> DJ
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: The new svn_wc_adm_*_depth functions.
Posted by "D.J. Heap" <dj...@shadyvale.net>.
kfogel@collab.net wrote:
> Is it our intention that these new functions
>
> svn_wc_adm_open_depth
> svn_wc_adm_probe_try_depth
> svn_wc_adm_probe_open_depth
>
> supersede their non-depth predecessors, and that (eventually) all
> callers of the predecessors be converted to use the depth versions? I
> would think so, but just want to make sure here first.
I believe so, based on past discussions and the need they address.
>
> If so, it's time to mark the predecessors
>
> svn_wc_adm_open
> svn_wc_adm_probe_try
> svn_wc_adm_probe_open
>
> as 'deprecated'. This should be pretty simple: just make a special
> 'deprecated' marker that's easy to grep for, and open a single new
> issue in the 2.0 milestone:
>
> "Remove deprecated 1.x interfaces."
>
> That issue will give instructions to grep for the special marker. It
> doesn't really matter whether we convert all the predecessors now, or
> just make a note in the deprecated function's doc string that it needs
> to be done before removing the function. (I'd prefer to do it now, if
> it's not a big task, though, to avoid having bad examples in our
> code.)
>
> I can take care of all this; I just want to make sure it's what we
> intend.
>
> -Karl
I can do this and submit a patch tonight or tomorrow if it's not
time-critical, which I wouldn't think it would be. Is there a standard
'deprecated' marker already? I seem to recall a similar discussion, but
I can't find it and don't see anything in HACKING.
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org