You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2008/09/20 06:06:39 UTC
Re: svn commit: r33207 - branches/file-externals/subversion/libsvn_client
On Fri, Sep 19, 2008 at 10:33 PM, <bl...@tigris.org> wrote:
>...
> +++ branches/file-externals/subversion/libsvn_client/externals.c Fri Sep 19 22:33:04 2008 (r33207)
> @@ -347,9 +347,8 @@ switch_file_external(const char *path,
> conflict on the directory. To prevent resolving a conflict
> due to another change on the directory, do not allow a file
> external to be added when one exists. */
> - SVN_ERR(svn_wc_entry(&anchor_dir_entry, anchor, adm_access, FALSE,
> - subpool));
> - SVN_ERR_ASSERT(anchor_dir_entry);
> + SVN_ERR(svn_wc__entry_versioned(&anchor_dir_entry, anchor, adm_access,
> + FALSE, subpool));
> SVN_ERR(svn_wc_conflicted_p2(&text_conflicted, &prop_conflicted,
> &has_tree_conflicted_children,
> anchor, anchor_dir_entry, subpool));
Wait a sec here... the client library can't use internal WC functions.
Cheers,
-g
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Private header files
Posted by Greg Stein <gs...@gmail.com>.
Sorry, hard to respond inline.
Yes, i've already assumed it is incumbent on me to suggest any change.
The current arrangement makes sense, tho it makes me a but
uncomfortable. It's on me, so don't waste ur time.
Cheers,
-g
On Sep 20, 2008, at 17:14, Karl Fogel <kf...@red-bean.com> wrote:
> Greg Stein <gs...@gmail.com> writes:
>> Hmm. I saw it more as private subsystems, or sharing data between
>> client and server bits.
>>
>> The readme is kinda hard to argue with... Hmm. I'll take a look. It
>> just seems that if a library needs functionality from another, then
>> we
>> should tweak the public api cuz somebody else may need it.
>
> Making an API public is a commitment to maintaining it, including
> doing
> the deprecation dance whenever we change it.
>
> Meanwhile, when it comes to consuming Subversion APIs, Subversion's
> own
> libraries are a special case. We know this from experience -- that's
> exactly why we made the include/private/ headers in the first place.
> There were things we wanted to use that it just seemed unlikely anyone
> else would want. (And if people do want it, then the answer is
> easy: we
> just promote it out of private/ and maintain it like any other public
> API from then on.)
>
> I mean, this wasn't something that got developed just for kicks. It
> solved a real problem: that there was no level of accessibility
> between
> library-internal and fully-public, even though we found we kept
> needing
> that intermediate level of accessibility.
>
> Now would be the time to come up with some concrete example, and I'm
> too
> lazy (read: swamped) to do that :-). But if I haven't convinced you,
> maybe I can dig around and find one...
>
> -Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Private header files
Posted by Karl Fogel <kf...@red-bean.com>.
Greg Stein <gs...@gmail.com> writes:
> Hmm. I saw it more as private subsystems, or sharing data between
> client and server bits.
>
> The readme is kinda hard to argue with... Hmm. I'll take a look. It
> just seems that if a library needs functionality from another, then we
> should tweak the public api cuz somebody else may need it.
Making an API public is a commitment to maintaining it, including doing
the deprecation dance whenever we change it.
Meanwhile, when it comes to consuming Subversion APIs, Subversion's own
libraries are a special case. We know this from experience -- that's
exactly why we made the include/private/ headers in the first place.
There were things we wanted to use that it just seemed unlikely anyone
else would want. (And if people do want it, then the answer is easy: we
just promote it out of private/ and maintain it like any other public
API from then on.)
I mean, this wasn't something that got developed just for kicks. It
solved a real problem: that there was no level of accessibility between
library-internal and fully-public, even though we found we kept needing
that intermediate level of accessibility.
Now would be the time to come up with some concrete example, and I'm too
lazy (read: swamped) to do that :-). But if I haven't convinced you,
maybe I can dig around and find one...
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Private header files [was Re: svn commit: r33207 - branches/file-externals/subversion/libsvn_client]
Posted by Blair Zajac <bl...@orcaware.com>.
Greg Stein wrote:
> Hmm. I saw it more as private subsystems, or sharing data between client
> and server bits.
>
> The readme is kinda hard to argue with... Hmm. I'll take a look. It just
> seems that if a library needs functionality from another, then we should
> tweak the public api cuz somebody else may need it.
Yeah, there's nothing stopping us from promoting a function from private to public.
It seems better to put a function in private unless you know you'll need it in
the public API. We do enough versioning of functions as it is, better to let an
API develop in private with our own internal usage and promote it to public when
needed then have a potentially unbaked call sit in public that we need to
maintain the ABI.
Blair
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Private header files [was Re: svn commit: r33207 - branches/file-externals/subversion/libsvn_client]
Posted by Greg Stein <gs...@gmail.com>.
Hmm. I saw it more as private subsystems, or sharing data between
client and server bits.
The readme is kinda hard to argue with... Hmm. I'll take a look. It
just seems that if a library needs functionality from another, then we
should tweak the public api cuz somebody else may need it.
Cheers,
-g
On Sep 20, 2008, at 11:18, Blair Zajac <bl...@orcaware.com> wrote:
> Greg Stein wrote:
>> On Fri, Sep 19, 2008 at 11:09 PM, Blair Zajac <bl...@orcaware.com>
>> wrote:
>>> Greg Stein wrote:
>>> Presumably being in include/private means it can be used across
>>> lib's.
>> This isn't right... Our libraries should not have privileged access
>> to
>> certain APIs that other users cannot reach. IOW, all 38 lines must
>> change :-P
>
> That's the purpose of all the headers in private:
>
> $ cat subversion/include/private/README
> Header files in this private/ directory are for internal APIs shared
> across Subversion's implementation. They are not part of the public
> API, nor are they ever copied into or under the include/ directory
> (e.g. by the installation process).
>
> You're effectively suggesting removing the entire include/private
> directory and promoting all the functions to public. Why would we
> want to do that? There may be APIs that we only want to support
> internally and not have the maintain in a public API.
>
> The only case I see for changing this is if we were to allow our
> libraries to be versioned independently so if you chance a private
> API without versioning it you can break the ABI, but we don't do
> that, so if we change a private API, it'll still be internally
> consistent across all the libs.
>
> I don't feel that strongly about it, as long as I can use the
> functions I need to make my coding easier :)
>
> Blair
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Private header files [was Re: svn commit: r33207 - branches/file-externals/subversion/libsvn_client]
Posted by Blair Zajac <bl...@orcaware.com>.
Greg Stein wrote:
> On Fri, Sep 19, 2008 at 11:09 PM, Blair Zajac <bl...@orcaware.com> wrote:
>> Greg Stein wrote:
>> Presumably being in include/private means it can be used across lib's.
>
> This isn't right... Our libraries should not have privileged access to
> certain APIs that other users cannot reach. IOW, all 38 lines must
> change :-P
That's the purpose of all the headers in private:
$ cat subversion/include/private/README
Header files in this private/ directory are for internal APIs shared
across Subversion's implementation. They are not part of the public
API, nor are they ever copied into or under the include/ directory
(e.g. by the installation process).
You're effectively suggesting removing the entire include/private directory and
promoting all the functions to public. Why would we want to do that? There may
be APIs that we only want to support internally and not have the maintain in a
public API.
The only case I see for changing this is if we were to allow our libraries to be
versioned independently so if you chance a private API without versioning it you
can break the ABI, but we don't do that, so if we change a private API, it'll
still be internally consistent across all the libs.
I don't feel that strongly about it, as long as I can use the functions I need
to make my coding easier :)
Blair
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r33207 - branches/file-externals/subversion/libsvn_client
Posted by Greg Stein <gs...@gmail.com>.
On Fri, Sep 19, 2008 at 11:09 PM, Blair Zajac <bl...@orcaware.com> wrote:
> Greg Stein wrote:
>> On Fri, Sep 19, 2008 at 10:33 PM, <bl...@tigris.org> wrote:
>>> ...
>>> +++ branches/file-externals/subversion/libsvn_client/externals.c
>>> Fri Sep 19 22:33:04 2008 (r33207)
>>> @@ -347,9 +347,8 @@ switch_file_external(const char *path,
>>> conflict on the directory. To prevent resolving a conflict
>>> due to another change on the directory, do not allow a file
>>> external to be added when one exists. */
>>> - SVN_ERR(svn_wc_entry(&anchor_dir_entry, anchor, adm_access, FALSE,
>>> - subpool));
>>> - SVN_ERR_ASSERT(anchor_dir_entry);
>>> + SVN_ERR(svn_wc__entry_versioned(&anchor_dir_entry, anchor,
>>> adm_access,
>>> + FALSE, subpool));
>>> SVN_ERR(svn_wc_conflicted_p2(&text_conflicted, &prop_conflicted,
>>> &has_tree_conflicted_children,
>>> anchor, anchor_dir_entry, subpool));
>>
>> Wait a sec here... the client library can't use internal WC functions.
>
> It's declared in subversion/include/private/svn_wc_private.h and it's used
> throughout libsvn_client:
>
> $ grep svn_wc__entry_versioned subversion/libsvn_client/*c | wc -l
> 38
>
> Presumably being in include/private means it can be used across lib's.
This isn't right... Our libraries should not have privileged access to
certain APIs that other users cannot reach. IOW, all 38 lines must
change :-P
Cheers,
-g
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r33207 - branches/file-externals/subversion/libsvn_client
Posted by Blair Zajac <bl...@orcaware.com>.
Greg Stein wrote:
> On Fri, Sep 19, 2008 at 10:33 PM, <bl...@tigris.org> wrote:
>> ...
>> +++ branches/file-externals/subversion/libsvn_client/externals.c Fri Sep 19 22:33:04 2008 (r33207)
>> @@ -347,9 +347,8 @@ switch_file_external(const char *path,
>> conflict on the directory. To prevent resolving a conflict
>> due to another change on the directory, do not allow a file
>> external to be added when one exists. */
>> - SVN_ERR(svn_wc_entry(&anchor_dir_entry, anchor, adm_access, FALSE,
>> - subpool));
>> - SVN_ERR_ASSERT(anchor_dir_entry);
>> + SVN_ERR(svn_wc__entry_versioned(&anchor_dir_entry, anchor, adm_access,
>> + FALSE, subpool));
>> SVN_ERR(svn_wc_conflicted_p2(&text_conflicted, &prop_conflicted,
>> &has_tree_conflicted_children,
>> anchor, anchor_dir_entry, subpool));
>
> Wait a sec here... the client library can't use internal WC functions.
It's declared in subversion/include/private/svn_wc_private.h and it's used
throughout libsvn_client:
$ grep svn_wc__entry_versioned subversion/libsvn_client/*c | wc -l
38
Presumably being in include/private means it can be used across lib's.
Blair
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org