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 Hudson <gh...@MIT.EDU> on 2003/12/12 16:02:08 UTC
API issues we might want to solve for 1.0
As it's much easier to make API changes before 1.0 than after, here
are a list of issues we might want to solve soon. The first three
seem important; the rest are just unfortunate foibles (except for #6,
but that would require a serious push to fix). Many of these have
received attention in the past, but time marched on without anyone
fixing them.
#1: We define symbols in the APR namespace:
In svn_sorts.h: apr_hash_sorted_keys
apr_array_prepend
In svn_types.h: APR_ARRAY_IDX
APR_ARRAY_PUSH
apr_atoui64
IMPACT OF PROBLEM: We're not very sociable, and might conflict with
a future version of APR which includes these symbols.
DIFFICULTY OF FIX: Depends on how we fix them.
#2: The svn_client interface accepts a pointer to a context
structure which is allocated by the caller and filled in by the
caller.
IMPACT OF PROBLEM: We have no way of extending this structure with
new fields. We should provide a function to create and
initialize a context structure and mandate that the caller use
it.
DIFFICULTY OF FIX: Minor.
#3: Recursion is specified as a boolean in svn_ra operations, but
we have vague plans to support either tri-state (0/1/infinity)
recursion or arbitrary recursion depth.
IMPACT OF PROBLEM: Supporting enhanced recursion will require a
difficult API change.
DIFFICULTY OF FIX: Medium; we need to agree on a new API, and then
all ra layers need a trivial change across half a dozen
functions.
#4: Even though we introduced "" as a no-op path element years ago,
svn_wc_get_actual_target() returns NULL in *target if anchor is
the actual target, and various APIs support NULL path elements
for compatibility with that.
IMPACT OF PROBLEM: API is somewhat inconsistent and confusing.
Fixing after 1.0 is unlikely to ever be worth the effort.
DIFFICULTY OF FIX: Medium, and with some change of destabilization.
(I had a go at this in issue #999.)
#5: svn_ra_init_ra_libs and friends use a void * baton instead of an
opaque type.
IMPACT OF PROBLEM: The interface is needlessly non-type-safe.
DIFFICULT OF FIX: Minor.
#6: We still use getdate.y, which implements a bizarre and arbitrary
set of date formats. (Did you know that "3" is a date?)
IMPACT OF PROBLEM: We'll be stuck with getdate.y forever.
DIFFICULTY OF FIX: Major. And we already have people (including
developers) relying on getdate's foibles. Bleah.
#7: Overly defensive programming in:
svn_path_uri_endode
svn_path_uri_decode
svn_cstring_split_append
The path_uri functions actually document that they will pass
through NULL, even though there is no use case which makes that
especially convenient. svn_cstring_split_append accepts a NULL
input even though it is not documented that it does so.
IMPACT OF PROBLEM: Minor; we will be trapped into providing
interface guarantees for these functions which we don't provide
most places.
DIFFICULTY OF FIX: Trivial.
#8: log_message_receiver_t accepts a hash of changed paths instead
of an array. This appears to be for the convenience of existing
callers.
IMPACT OF PROBLEM: Minor; changed_paths is not a pointer to const
since you can't iterate over a const hash table.
DIFFICULTY OF FIX: Medium.
#9: log_message_receiver_t and commit_callback_t pass dates as const
char * instead of apr_time_t, as a nod to the fact that dates
are stored as text properties on revisions.
IMPACT OF PROBLEM: API is not as clean as it could be; users of
those types must perform extra input validation because of the
possibility of malformed date strings.
DIFFICULTY OF FIX: Medium.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: API issues we might want to solve for 1.0
Posted by kf...@collab.net.
Re: API issues we might want to solve for 1.0
Posted by Greg Hudson <gh...@MIT.EDU>.
On Mon, 2003-12-15 at 15:04, kfogel@collab.net wrote:
> > So I'm thinking, maybe we don't need an svn_ra_init_ra_libs. What do
> > other people think?
>
> In other words, svn_ra_get_ra_library() would just do internally
> whatever it is that svn_ra_init_ra_libs() does right now, so that
> svn_ra_get_ra_library() wouldn't need to take the baton argument at
> all?
Right.
> What is the exception in 'svn switch --relocate'?
It calls svn_ra_init_ra_libs(), assigns the result to a baton field, and
uses the baton in the validator function. See relocate.c, not
switch.c. I'm not entirely sure how often the validator gets called,
but since it has to open an RA session, the cost of the initialization
pales in comparison to the other work it has to do.
> There does seem to be a bit of an exception in libsvn_client/diff.c
I missed that. So, two exceptions. Still, the logic holds: if you're
opening an RA session, that's going to be more work that initializing a
table. (And also more work than dynamically loading some libraries, if
the the future ever brings us to dynamically loaded ra libraries.)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: API issues we might want to solve for 1.0
Posted by kf...@collab.net.
Greg Hudson <gh...@MIT.EDU> writes:
> > #5: svn_ra_init_ra_libs and friends use a void * baton instead of an
> > opaque type.
> > IMPACT OF PROBLEM: The interface is needlessly non-type-safe.
> > DIFFICULT OF FIX: Minor.
>
> I took a look at this and found that in every single case except for
> one (svn switch --relocate), we initialize the ra baton, use it once
> immediately, and discard it.
>
> So I'm thinking, maybe we don't need an svn_ra_init_ra_libs. What do
> other people think?
In other words, svn_ra_get_ra_library() would just do internally
whatever it is that svn_ra_init_ra_libs() does right now, so that
svn_ra_get_ra_library() wouldn't need to take the baton argument at
all?
What is the exception in 'svn switch --relocate'? In the code of
svn_client_switch(), it seems the ra_baton is used in exactly the same
way it is everywhere else...
There does seem to be a bit of an exception in libsvn_client/diff.c,
where the ra_baton is passed as an argument to two successive calls to
single_file_merge_get_file().
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: API issues we might want to solve for 1.0
Posted by Greg Hudson <gh...@MIT.EDU>.
> #5: svn_ra_init_ra_libs and friends use a void * baton instead of an
> opaque type.
> IMPACT OF PROBLEM: The interface is needlessly non-type-safe.
> DIFFICULT OF FIX: Minor.
I took a look at this and found that in every single case except for
one (svn switch --relocate), we initialize the ra baton, use it once
immediately, and discard it.
So I'm thinking, maybe we don't need an svn_ra_init_ra_libs. What do
other people think?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: API issues we might want to solve for 1.0
Posted by Ben Reser <be...@reser.org>.
On Sun, Dec 14, 2003 at 06:40:22PM -0500, Greg Hudson wrote:
> On Sun, 2003-12-14 at 18:36, Ben Reser wrote:
> > > > - svn_client_ctx_t ctx = { 0 };
> > > > + svn_client_ctx_t *ctx = NULL;
> > >
> > > Don't initialize the pointer to an invalid value; all that serves to do
> > > is defeat the compiler's logic for determining whether the pointer is
> > > used before it's initialized to a valid value.
> >
> > Then someone should fix this 3 lines below my change too:
> > const svn_opt_subcommand_desc_t *subcommand = NULL;
>
> No, NULL is a valid value for subcommand. It is not a valid value for
> ctx.
Okay I see now. For some reason I missed that subcommand wasn't being
set before the first time it could be used.
--
Ben Reser <be...@reser.org>
http://ben.reser.org
"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: API issues we might want to solve for 1.0
Posted by Greg Hudson <gh...@MIT.EDU>.
On Sun, 2003-12-14 at 18:36, Ben Reser wrote:
> > > - svn_client_ctx_t ctx = { 0 };
> > > + svn_client_ctx_t *ctx = NULL;
> >
> > Don't initialize the pointer to an invalid value; all that serves to do
> > is defeat the compiler's logic for determining whether the pointer is
> > used before it's initialized to a valid value.
>
> Then someone should fix this 3 lines below my change too:
> const svn_opt_subcommand_desc_t *subcommand = NULL;
No, NULL is a valid value for subcommand. It is not a valid value for
ctx.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: API issues we might want to solve for 1.0
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On Dec 15, 2003, at 5:29 PM, Ben Reser wrote:
> On Sun, Dec 14, 2003 at 03:36:53PM -0800, Ben Reser wrote:
>> I'll wait to see if there's a consensus on the naming issue and regen
>> the patch with these fixes, unless someone applies it with the fixes
>> themselves.
>
> Looks like svn_client_create_context it is...
>
> The attached patch now uses that name, fixes the style issues that
> ghudson noticed.
>
> It now returns svn_error_t so that the function is no longer dependent
> upon the internal implementation not being able to fail. Clients will
> already be checking the return so such a change would not require an
> API
> or ABI visible change.
I committed a slightly modified version of this patch (just tweaked the
documentation comments mostly) in revision 8053.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: API issues we might want to solve for 1.0
Posted by Ben Reser <be...@reser.org>.
On Sun, Dec 14, 2003 at 03:36:53PM -0800, Ben Reser wrote:
> I'll wait to see if there's a consensus on the naming issue and regen
> the patch with these fixes, unless someone applies it with the fixes
> themselves.
Looks like svn_client_create_context it is...
The attached patch now uses that name, fixes the style issues that
ghudson noticed.
It now returns svn_error_t so that the function is no longer dependent
upon the internal implementation not being able to fail. Clients will
already be checking the return so such a change would not require an API
or ABI visible change.
[[[
Future proof the ABI by providing an allocator/initializer function for
the client context.
* subversion/libsvn_client/ctx.c
* subversion/include/svn_client.h
(svn_client_create_context): New function
* subversion/clients/cmdline/main.c
* subversion/tools/examples/minimal_client.c
Use svn_client_create_context().
]]]
--
Ben Reser <be...@reser.org>
http://ben.reser.org
"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken
Re: API issues we might want to solve for 1.0
Posted by Ben Reser <be...@reser.org>.
On Sun, Dec 14, 2003 at 06:11:40PM -0500, Greg Hudson wrote:
> On Sun, 2003-12-14 at 17:36, Ben Reser wrote:
> > The only way we might break that is by adding a member that causes the
> > compiler to pad the struct. Future changes might add more members that
> > then remove the need for the pad.
>
> The standard makes this unlikely to ever be a problem. There's a rule
> which says that if you put two structures with a common initial sequence
> inside a union, then you're allowed to set those fields within one
> structure and examine them within another, and it has to work. Although
> this guarantee technically only applies to unions, the only practical
> way to implement the rule is to ensure that the padding before each
> field within a structure never depends on the subsequent fields, only
> the previous ones.
Great, so even then we can workaround it by doing a union...
> I'm not sure that svn_config_ensure is really required. If you want to
> fill in ctx.config with your own configuration data, I think that's
> allowed.
That's true. So then adding it would violate the absolutely necessary
principle I put forth and it probably should stay out of the function.
> On the patch:
>
> > - svn_client_ctx_t ctx = { 0 };
> > + svn_client_ctx_t *ctx = NULL;
>
> Don't initialize the pointer to an invalid value; all that serves to do
> is defeat the compiler's logic for determining whether the pointer is
> used before it's initialized to a valid value.
Then someone should fix this 3 lines below my change too:
const svn_opt_subcommand_desc_t *subcommand = NULL;
:)
> > + svn_client_open(&ctx,pool);
>
> We always put a space after commas, and in this file we put a space
> before the open paren.
>
> (tools/examples/minimal_client.c has the same pair of problems.)
Whoops, and I thought I was being careful about the style.
I'll wait to see if there's a consensus on the naming issue and regen
the patch with these fixes, unless someone applies it with the fixes
themselves.
--
Ben Reser <be...@reser.org>
http://ben.reser.org
"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: API issues we might want to solve for 1.0
Posted by Branko Čibej <br...@xbc.nu>.
Greg Hudson wrote:
>On Sun, 2003-12-14 at 17:36, Ben Reser wrote:
>
>
>>The only way we might break that is by adding a member that causes the
>>compiler to pad the struct. Future changes might add more members that
>>then remove the need for the pad.
>>
>>
>
>The standard makes this unlikely to ever be a problem.
>
The standard in fact guarantees that this will never be a problem.
Adding members to the end of a structure cannot change the layout of
previous structure members.
--
Brane Čibej <br...@xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: API issues we might want to solve for 1.0
Posted by Greg Hudson <gh...@MIT.EDU>.
On Sun, 2003-12-14 at 17:36, Ben Reser wrote:
> The only way we might break that is by adding a member that causes the
> compiler to pad the struct. Future changes might add more members that
> then remove the need for the pad.
The standard makes this unlikely to ever be a problem. There's a rule
which says that if you put two structures with a common initial sequence
inside a union, then you're allowed to set those fields within one
structure and examine them within another, and it has to work. Although
this guarantee technically only applies to unions, the only practical
way to implement the rule is to ensure that the padding before each
field within a structure never depends on the subsequent fields, only
the previous ones.
> * Per the comment in main.c of the cmdline client it'd be nice if there
> was a place to put svn_config_ensure. svn_client_open is of course just
> such a place now. If were were to set a hard limit that only
> initialization that is *REQUIRED* for the client to do (as
> svn_config_ensure is in this case) could go in the svn_client_open
> function then it probably would be okay.
I'm not sure that svn_config_ensure is really required. If you want to
fill in ctx.config with your own configuration data, I think that's
allowed.
On the patch:
> - svn_client_ctx_t ctx = { 0 };
> + svn_client_ctx_t *ctx = NULL;
Don't initialize the pointer to an invalid value; all that serves to do
is defeat the compiler's logic for determining whether the pointer is
used before it's initialized to a valid value.
> + svn_client_open(&ctx,pool);
We always put a space after commas, and in this file we put a space
before the open paren.
(tools/examples/minimal_client.c has the same pair of problems.)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: API issues we might want to solve for 1.0
Posted by Tobias Ringstrom <to...@ringstrom.mine.nu>.
Ben Reser wrote:
> svn_client_create_context (suggested by ghudson, but seems long IMHO).
+1 for this name since it complies better with the naming of the other
svn_client_* functions. A long name is OK if it makes the purpose of
the function obvious, and this function is not used all over the code.
/Tobias
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: API issues we might want to solve for 1.0
Posted by Ben Reser <be...@reser.org>.
On Sun, Dec 14, 2003 at 06:05:02PM -0500, Garrett Rooney wrote:
> svn_client_ctx_create ?
That one had occured to me and I forgot to put it on the list.
> Some comments on the patch...
>
> +void
> +svn_client_open (svn_client_ctx_t **ctx,
> + apr_pool_t *pool)
> +{
> + *ctx = apr_pcalloc(pool,sizeof(svn_client_ctx_t));
> +}
>
> If this function cannot fail, it's considerably more readable to have
> it return a pointer to the svn_client_ctx_t than to have to pass it in
> as an argument to be filled in.
>
> On the other hand, perhaps it would make sense to have this function do
> some of the initialization of the svn_client_ctx_t for us, like reading
> in config files and things like that. In that case it could
> potentially fail, so you'd want to be able to return an svn_error_t *.
Maybe we should make it return an svn_error_t that's hard coded to
return SVN_NO_ERROR for the time being. Then if we ever need to add
code that could fail it wouldn't nessitate an API change.
svn_auth_open is the same way, perhaps that should be changed as well?
I'm starting to wonder if svn_auth_open was a bad example to follow.
--
Ben Reser <be...@reser.org>
http://ben.reser.org
"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: API issues we might want to solve for 1.0
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On Dec 14, 2003, at 5:36 PM, Ben Reser wrote:
> Done and attached. Some comments on this.
>
> * svn_client_open may not be the best name. I used that becasue we
> have svn_auth_open which roughly serves the same purpose. Ohter
> possible names that may be better are:
> svn_client_init (suggested in the comment above the svn_config_ensure
> call in main.c of the cmdline client).
> svn_client_create_context (suggested by ghudson, but seems long
> IMHO).
> svn_client_context_create (suggested by mbk, also long).
svn_client_ctx_create ?
Some comments on the patch...
+void
+svn_client_open (svn_client_ctx_t **ctx,
+ apr_pool_t *pool)
+{
+ *ctx = apr_pcalloc(pool,sizeof(svn_client_ctx_t));
+}
If this function cannot fail, it's considerably more readable to have
it return a pointer to the svn_client_ctx_t than to have to pass it in
as an argument to be filled in.
On the other hand, perhaps it would make sense to have this function do
some of the initialization of the svn_client_ctx_t for us, like reading
in config files and things like that. In that case it could
potentially fail, so you'd want to be able to return an svn_error_t *.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: API issues we might want to solve for 1.0
Posted by Ben Reser <be...@reser.org>.
On Fri, Dec 12, 2003 at 11:34:10PM -0800, Ben Reser wrote:
> On Fri, Dec 12, 2003 at 11:02:08AM -0500, Greg Hudson wrote:
> > #2: The svn_client interface accepts a pointer to a context
> > structure which is allocated by the caller and filled in by the
> > caller.
> > IMPACT OF PROBLEM: We have no way of extending this structure with
> > new fields. We should provide a function to create and
> > initialize a context structure and mandate that the caller use
> > it.
> > DIFFICULTY OF FIX: Minor.
>
> +1
>
> The current situation is ugly IMHO. If nobody else wants to do this I'd
> be happy to do this. I was going to do something along this lines for
> the perl bindings anyway. So if we want it in the main API, I might as
> well just do it there instead.
Done and attached. Some comments on this.
* svn_client_open may not be the best name. I used that becasue we
have svn_auth_open which roughly serves the same purpose. Ohter
possible names that may be better are:
svn_client_init (suggested in the comment above the svn_config_ensure
call in main.c of the cmdline client).
svn_client_create_context (suggested by ghudson, but seems long IMHO).
svn_client_context_create (suggested by mbk, also long).
I already despise the long function names we have like:
svn_client_get_ssl_client_cert_file_provider()
Though granted svn_client_(create_context|context_create) aren't nearly
as bad. So if svn_auth_open is a bad example that we don't want to
follow here then I'd say svn_client_init is best.
* The function was added to a new file ctx.c because I didn't see any
other obvious place to put it in the client lib.
* In order to achieve presurvation of the ABI this assumes that the size
of existing members never changes and that their order stays the same in
the struct. If we only add members and always add them at the end then
this should keep us compatable.
The only way we might break that is by adding a member that causes the
compiler to pad the struct. Future changes might add more members that
then remove the need for the pad. We are currently only storing
pointers in the struct and as long as we don't forsee wanting to store
anything else other than pointers in the future then this should not be
an issue.
Further, the only way to resolve the possible problem of a padding
change would be to write and require clients to use accessor functions.
While this would provide a higher level of ABI protection, it's probably
not worth the complexity for the gain. The cases where we would need to
break this ABI should be far and few between and can be scheduled with
other changes that would necessitate an ABI change in spite of using
accessor functions.
* Per the comment in main.c of the cmdline client it'd be nice if there
was a place to put svn_config_ensure. svn_client_open is of course just
such a place now. If were were to set a hard limit that only
initialization that is *REQUIRED* for the client to do (as
svn_config_ensure is in this case) could go in the svn_client_open
function then it probably would be okay. New requirements that would
get added would likely be pretty rare and would of course necessitate
breaking the ABI because the client would have something new to do. In
these cases I think it would make sense moving those initalizations to
the svn_client_open function. However, to keep the impact of this patch
law I did not make such a change yet. But thought I would raise the
issue now.
* This patch will not break any existing clients. Clients that are
doing their own allocation now will continue to build and work despite
this patch going in. However, clients that don't change to use
svn_client_open would do so at the risk of breaking compatability at
some point down the line. This is good and bad. The good is that it
is unlikely to break anything. The bad is that client authors who
aren't paying attention might not make the appropriate change and then
break far down the line. I do not think this is a compelling argument
against implementing this. It's just something to note and that we
should loudly advertise this in the first release it goes out in.
Now that my wordy commentary is out of the way, here's the patch:
[[[
Future proof the ABI by providing an allocator/initializer function for
the client context.
* subversion/libsvn_client/ctx.c
* subversion/include/svn_client.h
(svn_client_open): New function
* subversion/clients/cmdline/main.c
* subversion/tools/examples/minimal_client.c
Use svn_client_open().
]]]
Attached due to existing lines that were too long and cause wrap.
--
Ben Reser <be...@reser.org>
http://ben.reser.org
"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken
Re: API issues we might want to solve for 1.0
Posted by Ben Reser <be...@reser.org>.
On Fri, Dec 12, 2003 at 11:02:08AM -0500, Greg Hudson wrote:
> #2: The svn_client interface accepts a pointer to a context
> structure which is allocated by the caller and filled in by the
> caller.
> IMPACT OF PROBLEM: We have no way of extending this structure with
> new fields. We should provide a function to create and
> initialize a context structure and mandate that the caller use
> it.
> DIFFICULTY OF FIX: Minor.
+1
The current situation is ugly IMHO. If nobody else wants to do this I'd
be happy to do this. I was going to do something along this lines for
the perl bindings anyway. So if we want it in the main API, I might as
well just do it there instead.
--
Ben Reser <be...@reser.org>
http://ben.reser.org
"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: API issues we might want to solve for 1.0
Posted by mark benedetto king <mb...@lowlatency.com>.
On Fri, Dec 12, 2003 at 12:52:48PM -0500, Greg Hudson wrote:
> On Fri, 2003-12-12 at 12:45, mark benedetto king wrote:
> > However, I can much more easily (and readily) produce a getdate.y
> > replacement that recognizes a more managable set of date formats, one
> > that could be extended in the future to support localization.
>
> +1. (I'm not sure if you're proposing just changing the yacc grammar to
> be more restrictive, or ditching yacc altogether, but either seems like
> an improvement.)
Might as well ditch yacc, to simplify the build process (and save Brane
some troubles).
I'll put together a concrete proposal over the weekend.
--ben
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: API issues we might want to solve for 1.0
Posted by Greg Hudson <gh...@MIT.EDU>.
On Fri, 2003-12-12 at 12:45, mark benedetto king wrote:
> However, I can much more easily (and readily) produce a getdate.y
> replacement that recognizes a more managable set of date formats, one
> that could be extended in the future to support localization.
+1. (I'm not sure if you're proposing just changing the yacc grammar to
be more restrictive, or ditching yacc altogether, but either seems like
an improvement.)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: API issues we might want to solve for 1.0
Posted by mark benedetto king <mb...@lowlatency.com>.
On Fri, Dec 12, 2003 at 11:02:08AM -0500, Greg Hudson wrote:
> #2: The svn_client interface accepts a pointer to a context
> structure which is allocated by the caller and filled in by the
> caller.
> IMPACT OF PROBLEM: We have no way of extending this structure with
> new fields. We should provide a function to create and
> initialize a context structure and mandate that the caller use
> it.
> DIFFICULTY OF FIX: Minor.
I believe that the svn_client interface is also prone to ABI changes,
The following booleans are available as options to svn_client functions:
force, recurse, ignore ancestry, dry run, no diff deleted, strict node
history, discover changed paths, descend, get_all, update, no_ignore,
nonrecursive.
Some or all of these may become relevant to functions that they are
not currently supported with. Also, some new boolean options may become
relevant to some or all of the existing interface functions.
I think it will be a significant but worthwhile effort to feature-proof
these interfaces (and the command-line/configuration system).
http://www.contactor.se/~dast/svn/archive-2003-07/0197.shtml
This is probably too much to accomplish before 1.0, though.
> #6: We still use getdate.y, which implements a bizarre and arbitrary
> set of date formats. (Did you know that "3" is a date?)
> IMPACT OF PROBLEM: We'll be stuck with getdate.y forever.
> DIFFICULTY OF FIX: Major. And we already have people (including
> developers) relying on getdate's foibles. Bleah.
This is my fault. I've been promising to work on this for a long time.
I've started on it several times, but haven't had the resolve to see it
through to completion.
However, I can much more easily (and readily) produce a getdate.y
replacement that recognizes a more managable set of date formats, one
that could be extended in the future to support localization.
If we are willing to abandon the localization support, I can produce
a new, simplified date parser before the end of the year.
--ben
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: API issues we might want to solve for 1.0
Posted by Mike Mason <mg...@thoughtworks.net>.
Branko Čibej wrote:
> I for one would be totally happy if it accepted only ISO-ish dates and
>
>times for 1.0, and be damned with CVS compatibility.
>
>
Totally agree.
Just out of interest, why has the code made it this far without being
replaced? Was it a direct grab from CVS when Subversion was starting out
and deemed "good enough" to get the new system going?
Cheers,
Mike.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: API issues we might want to solve for 1.0
Posted by Branko Čibej <br...@xbc.nu>.
kfogel@collab.net wrote:
>Greg Hudson <gh...@MIT.EDU> writes:
>
>
>> #6: We still use getdate.y, which implements a bizarre and arbitrary
>> set of date formats. (Did you know that "3" is a date?)
>> IMPACT OF PROBLEM: We'll be stuck with getdate.y forever.
>> DIFFICULTY OF FIX: Major. And we already have people (including
>> developers) relying on getdate's foibles. Bleah.
>>
>>
>
>We don't really need to fix this for 1.0, as long as whatever date
>formats we extend to in the future are a superset of what we currently
>support. (Or a superset of all the ones people actually use, anyway
>:-) ).
>
>What are the particular foibles people are relying on already?
>
>I'd like to avoid messing with this, unless the current interface
>truly guarantees brokenness down the road.
>
>
I'm sure I've said before that getdate.y is a totall mess. Even ignoring
the fact that there's no way to localise it, it accepts far too many
things as a valid date string. "Last tuesday" indeed.
I for one would be totally happy if it accepted only ISO-ish dates and
times for 1.0, and be damned with CVS compatibility.
--
Brane Čibej <br...@xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: API issues we might want to solve for 1.0
Posted by Greg Hudson <gh...@MIT.EDU>.
On Fri, 2003-12-12 at 14:10, kfogel@collab.net wrote:
> > #1: We define symbols in the APR namespace:
> I suggest we simply switch the prefixes (prefices?) to "svn_apr_" and
> "SVN_APR_", to avoid any possibility of conflict, then remove our
> definitions and switch them back when and if APR gets these functions.
Except that removing function is an API change.
If we're pretty sure these will get into APR, we could name them
svn__apr_foo and SVN__APR_FOO, make sure there are no doxygen comments
for them, and note in the header files that they're for internal use
only. Then it should be reasonably safe to remove them from the
libraries in the future.
If we think some of them won't make it into APR, then we should pick
names we're comfortable with supporting.
> > #3: Recursion is specified as a boolean in svn_ra operations, but
> > we have vague plans to support either tri-state (0/1/infinity)
> > recursion or arbitrary recursion depth.
> The API change itself is no more difficult later than it is now. We
> will break code & binary compatibility, but we've always accepted that
> future versions will do that, and we have a release numbering scheme
> to handle it.
I think we will find (either the hard way or the easy way) that after
1.0, API changes have to be batched up and made at rare intervals--like,
every five years or so. We'll need to develop ways for people to have
old and new versions of Subversion installed on the same system, and
we'll have to write special documentation to help people transition to
the new API. So, if we want to support new features without a lot of
pain, we either have to do it without breaking backward compatibility
(i.e. adding new functions, but preserving old ones) or we have to
anticipate them now.
> > #4: Even though we introduced "" as a no-op path element years ago,
> > svn_wc_get_actual_target() returns NULL in *target if anchor is
> > the actual target, and various APIs support NULL path elements
> > for compatibility with that.
> (I don't see why it would be worth it now, but not worth it after
> 1.0.)
Because after 1.0, we have to worry about application code which calls
svn_wc_get_actual_target() and expects to see NULL in *target for
directories, or application code which calls the various APIs which
support NULL path elements and passes NULL.
> > #5: svn_ra_init_ra_libs and friends use a void * baton instead of an
> > opaque type.
> Sure. Not a tragedy if it doesn't get fixed before 1.0; I'm of a mind
> to punt on the 1.0 release line, but maybe get a fix into trunk when
> sometime when we're going to break binary compatibility anyway.
> Oh wait, this wouldn't even break binary compatibility, would it?
It would break source compatibility, but not binary compatibility.
> > #6: We still use getdate.y, which implements a bizarre and arbitrary
> > set of date formats. (Did you know that "3" is a date?)
> We don't really need to fix this for 1.0, as long as whatever date
> formats we extend to in the future are a superset of what we currently
> support. (Or a superset of all the ones people actually use, anyway
> :-) ).
>
> What are the particular foibles people are relying on already?
I get the impression that people rely on things like "yesterday." I'm
okay with that particular one, but there are some particularly weird
things like "6/30 1440" for 2:40pm on June 30. Or "4 days ago 3" for
3am four days ago.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: API issues we might want to solve for 1.0
Posted by kf...@collab.net.
Greg Hudson <gh...@MIT.EDU> writes:
> As it's much easier to make API changes before 1.0 than after, here
> are a list of issues we might want to solve soon. The first three
> seem important; the rest are just unfortunate foibles (except for #6,
> but that would require a serious push to fix). Many of these have
> received attention in the past, but time marched on without anyone
> fixing them.
Wow, nice list! Glad you were keeping track.
I'll make some specific comments below. After a bit of thread
discussion here to reach any obvious conclusions, I'll file issues for
all of these that remain. They'll be a lot easier to manage that way;
we can schedule them individually and have clear record of what we've
decided.
> #1: We define symbols in the APR namespace:
> In svn_sorts.h: apr_hash_sorted_keys
> apr_array_prepend
> In svn_types.h: APR_ARRAY_IDX
> APR_ARRAY_PUSH
> apr_atoui64
> IMPACT OF PROBLEM: We're not very sociable, and might conflict with
> a future version of APR which includes these symbols.
> DIFFICULTY OF FIX: Depends on how we fix them.
On dev@apr, Sander Striker is proposing to get at least some of these
into APR. That may happen quickly, or may not, but in any case it's
too late for us because we need to work with the APR that ships today.
I suggest we simply switch the prefixes (prefices?) to "svn_apr_" and
"SVN_APR_", to avoid any possibility of conflict, then remove our
definitions and switch them back when and if APR gets these functions.
I can do this, if we agree.
(My goal here is: least impact on Subversion while maintaining
compatibility with future APRs.)
> #2: The svn_client interface accepts a pointer to a context
> structure which is allocated by the caller and filled in by the
> caller.
> IMPACT OF PROBLEM: We have no way of extending this structure with
> new fields. We should provide a function to create and
> initialize a context structure and mandate that the caller use
> it.
> DIFFICULTY OF FIX: Minor.
Agree with solution. Again volunteer, though am happy if someone else
wants to do it.
> #3: Recursion is specified as a boolean in svn_ra operations, but
> we have vague plans to support either tri-state (0/1/infinity)
> recursion or arbitrary recursion depth.
> IMPACT OF PROBLEM: Supporting enhanced recursion will require a
> difficult API change.
> DIFFICULTY OF FIX: Medium; we need to agree on a new API, and then
> all ra layers need a trivial change across half a dozen
> functions.
I think we should punt on this for 1.0; the discussion would be a big
thread, and we're going to have enough of those between now and 1.0.
The API change itself is no more difficult later than it is now. We
will break code & binary compatibility, but we've always accepted that
future versions will do that, and we have a release numbering scheme
to handle it.
> #4: Even though we introduced "" as a no-op path element years ago,
> svn_wc_get_actual_target() returns NULL in *target if anchor is
> the actual target, and various APIs support NULL path elements
> for compatibility with that.
> IMPACT OF PROBLEM: API is somewhat inconsistent and confusing.
> Fixing after 1.0 is unlikely to ever be worth the effort.
> DIFFICULTY OF FIX: Medium, and with some change of destabilization.
> (I had a go at this in issue #999.)
I agree with Greg Stein's comment in the issue, that this isn't
necessary for 1.0. If you have the motivation to finish #999, go for
it, but I think we can live with things as they are for 1.0.
(I don't see why it would be worth it now, but not worth it after
1.0.)
> #5: svn_ra_init_ra_libs and friends use a void * baton instead of an
> opaque type.
> IMPACT OF PROBLEM: The interface is needlessly non-type-safe.
> DIFFICULT OF FIX: Minor.
Sure. Not a tragedy if it doesn't get fixed before 1.0; I'm of a mind
to punt on the 1.0 release line, but maybe get a fix into trunk when
sometime when we're going to break binary compatibility anyway.
Oh wait, this wouldn't even break binary compatibility, would it?
> #6: We still use getdate.y, which implements a bizarre and arbitrary
> set of date formats. (Did you know that "3" is a date?)
> IMPACT OF PROBLEM: We'll be stuck with getdate.y forever.
> DIFFICULTY OF FIX: Major. And we already have people (including
> developers) relying on getdate's foibles. Bleah.
We don't really need to fix this for 1.0, as long as whatever date
formats we extend to in the future are a superset of what we currently
support. (Or a superset of all the ones people actually use, anyway
:-) ).
What are the particular foibles people are relying on already?
I'd like to avoid messing with this, unless the current interface
truly guarantees brokenness down the road.
> #7: Overly defensive programming in:
> svn_path_uri_endode
> svn_path_uri_decode
> svn_cstring_split_append
> The path_uri functions actually document that they will pass
> through NULL, even though there is no use case which makes that
> especially convenient. svn_cstring_split_append accepts a NULL
> input even though it is not documented that it does so.
> IMPACT OF PROBLEM: Minor; we will be trapped into providing
> interface guarantees for these functions which we don't provide
> most places.
> DIFFICULTY OF FIX: Trivial.
I'd be happy with just removing the doc promises now and changing the
code later, or changing both now. What color would we like to paint
the bikeshed today? :-)
> #8: log_message_receiver_t accepts a hash of changed paths instead
> of an array. This appears to be for the convenience of existing
> callers.
> IMPACT OF PROBLEM: Minor; changed_paths is not a pointer to const
> since you can't iterate over a const hash table.
> DIFFICULTY OF FIX: Medium.
IMHO the cost of fixing isn't worth the benefits of const.
> #9: log_message_receiver_t and commit_callback_t pass dates as const
> char * instead of apr_time_t, as a nod to the fact that dates
> are stored as text properties on revisions.
> IMPACT OF PROBLEM: API is not as clean as it could be; users of
> those types must perform extra input validation because of the
> possibility of malformed date strings.
> DIFFICULTY OF FIX: Medium.
Hmm. The current API was a deliberate decision. There are certainly
drawbacks to both ways, but I'm not sure apr_time_t would be a
definite win over the status quo.
By the way, for those that we are fixing, I don't think it matters
much whether the fixes go into the 0.35 branch or wait for the
1.0-stabilization branch. Whenever they're ready, they can go in.
(We should do them on trunk and then merge, though).
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: API issues we might want to solve for 1.0
Posted by "C. Michael Pilato" <cm...@collab.net>.
Greg Hudson <gh...@MIT.EDU> writes:
> As it's much easier to make API changes before 1.0 than after, here
> are a list of issues we might want to solve soon. The first three
> seem important; the rest are just unfortunate foibles (except for #6,
> but that would require a serious push to fix). Many of these have
> received attention in the past, but time marched on without anyone
> fixing them.
>
> #1: We define symbols in the APR namespace:
> In svn_sorts.h: apr_hash_sorted_keys
> apr_array_prepend
> In svn_types.h: APR_ARRAY_IDX
> APR_ARRAY_PUSH
> apr_atoui64
> IMPACT OF PROBLEM: We're not very sociable, and might conflict with
> a future version of APR which includes these symbols.
> DIFFICULTY OF FIX: Depends on how we fix them.
Follow-up to this that's been on my TODO list (though anyone can do it
if they so desire):
Change APR_ARRAY_PUSH definition from
#define APR_ARRAY_PUSH(ary,type) (*((type *)apr_array_push (ary)))
to
#define APR_ARRAY_PUSH(ary,type,item) \
(*((type *)apr_array_push (ary))) = item
And for what it's worth, I agree with the other assessments in your
mail (though I'm not terribly bothered about the log_receiver taking
changed_paths as a hash instead of an array).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: API issues we might want to solve for 1.0
Posted by kf...@collab.net.
Tobias Ringström <to...@ringstrom.mine.nu> writes:
> And here is the patch. It looks big, but it's mostly code that's been
> moved and reindented. The patch does not touch code outside the SSL
> client certificate functionality, so I think it's quite safe.
>
> Is this code desirable/acceptable for 1.0?
We can decide that separately -- certainly it's desirable for trunk (I
haven't reviewed the patch itself, I just mean "in principle" :-) ).
In general, we need to not confuse the question of whether a change is
good with the question of whether it goes into 1.0. By committing it
on the trunk today, you are not committing it to 1.0. We can decide
afterwards whether or not to merge it.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: API issues we might want to solve for 1.0
Posted by Ben Reser <be...@reser.org>.
On Sat, Dec 13, 2003 at 03:42:04PM +0100, Tobias Ringström wrote:
> And here is the patch. It looks big, but it's mostly code that's been
> moved and reindented. The patch does not touch code outside the SSL
> client certificate functionality, so I think it's quite safe.
>
> Is this code desirable/acceptable for 1.0?
Looks good to me. We're going to need to make this API change sooner or
later. I'd say we should make it now. It'll be painful later, painless
now. +1
Because you added the realm parameter if this goes in the perl binding
patch that's pending above to add the support for the prompt function
needs a patch. Attached is what should be applied on top of it if this
goes in first or later if this goes in after.
Ignore the revs they're from my own repo. But the patch should apply
properly anyway.
[[[
Fix prompt functions to match API for the retry patch for
client certs.
* subversion/bindings/swig/swigutil_pl.c
* subversion/bindings/swig/swigutil_pl.h
(svn_swig_pl_thunk_ssl_client_cert_pw_prompt,
svn_swig_pl_thunk_ssl_client_cert_prompt):
Add realm parameter and pass it through to
the perl side.
]]]
Index: subversion/bindings/swig/swigutil_pl.c
===================================================================
--- subversion/bindings/swig/swigutil_pl.c (revision 106)
+++ subversion/bindings/swig/swigutil_pl.c (revision 107)
@@ -995,6 +995,7 @@
svn_error_t *svn_swig_pl_thunk_ssl_client_cert_prompt(
svn_auth_cred_ssl_client_cert_t **cred,
void *baton,
+ const char * realm,
apr_pool_t *pool)
{
SV *result;
@@ -1011,8 +1012,8 @@
perl_callback_thunk (CALL_CV,
baton, &result,
- "SS", *cred, credinfo,
- pool, poolinfo);
+ "SsS", *cred, credinfo,
+ realm, pool, poolinfo);
return SVN_NO_ERROR;
}
@@ -1020,6 +1021,7 @@
svn_error_t *svn_swig_pl_thunk_ssl_client_cert_pw_prompt(
svn_auth_cred_ssl_client_cert_pw_t **cred,
void *baton,
+ const char *realm,
apr_pool_t *pool)
{
SV *result;
@@ -1036,8 +1038,8 @@
perl_callback_thunk (CALL_CV,
baton, &result,
- "SS", *cred, credinfo,
- pool, poolinfo);
+ "SsS", *cred, credinfo,
+ realm, pool, poolinfo);
return SVN_NO_ERROR;
}
Index: subversion/bindings/swig/swigutil_pl.h
===================================================================
--- subversion/bindings/swig/swigutil_pl.h (revision 106)
+++ subversion/bindings/swig/swigutil_pl.h (revision 107)
@@ -155,12 +155,14 @@
svn_error_t *svn_swig_pl_thunk_ssl_client_cert_prompt (
svn_auth_cred_ssl_client_cert_t **cred,
void *baton,
+ const char *realm,
apr_pool_t *pool);
/* thunked ssl_client_cert_pw callback function */
svn_error_t *svn_swig_pl_thunk_ssl_client_cert_pw_prompt (
svn_auth_cred_ssl_client_cert_pw_t **cred,
void *baton,
+ const char *realm,
apr_pool_t *pool);
/* thunked callback for svn_ra_get_wc_prop_func_t */
--
Ben Reser <be...@reser.org>
http://ben.reser.org
"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: API issues we might want to solve for 1.0
Posted by Tobias Ringström <to...@ringstrom.mine.nu>.
Tobias Ringström wrote:
> Ben Reser wrote:
>
>> On Fri, Dec 12, 2003 at 11:02:08AM -0500, Greg Hudson wrote:
>>
>> Add another one to the list:
>>
>> #10: svn_client_get_ssl_client_cert_prompt_provider and
>> svn_client_get_ssl_client_cert_pw_prompt_provider
>> do not have a retry limit. IMPACT OF PROBLEM: Clients
>> implemnting SSL certificate
>> support will only be able to give end users one chance
>> at providing the proper file for the cert and the pass
>> for that certificate. It seems almost certain that
>> people are going to want this fixed. To do it would
>> require a change in the API.
>> DIFFICULTY OF FIX: Medium, there may be complications
>> of trying to fix this that I'm not aware of. But
>> given that simple_prompt already supports retries
>> I'd image it would be straightforward unless neon
>> presents a roadblock on this.
>>
>> I mentioned this in the past but nobody replied to it.
>
> I'm working on fixing some other authentication annoyances at the
> moment, so I'll fix this one as well. If it ends up in 1.0 or not is
> another issue of course. I think it should be included because the fix
> is straightforward, and it does not touch any sensitive code, but we'll
> see.
And here is the patch. It looks big, but it's mostly code that's been
moved and reindented. The patch does not touch code outside the SSL
client certificate functionality, so I think it's quite safe.
Is this code desirable/acceptable for 1.0?
/Tobias
-----------------------------------------------------------------------
Implement retries for the SSL client certificate filename and password
prompts. Add a realm string to the prompt functions so that the user
can know what information that being asked for.
* subversion/include/svn_client.h
(svn_client_get_ssl_client_cert_prompt_provider,
svn_client_get_ssl_client_cert_pw_prompt_provider):
Add a retry_limit argument.
* subversion/include/svn_auth.h
(svn_auth_ssl_client_cert_prompt_func_t,
svn_auth_ssl_client_cert_pw_prompt_func_t):
Add a realm argument.
* subversion/libsvn_client/ssl_client_cert_pw_providers.c
(ssl_client_cert_pw_prompt_provider_baton_t):
Add a retry_limit struct member.
(ssl_client_cert_pw_prompt_iter_baton_t): New struct.
(ssl_client_cert_pw_prompt_first_cred): Allocate and initialize an
iteration baton. Provide the realmstring to the prompt function.
(ssl_client_cert_pw_prompt_next_cred): New function to handle
authentication retries.
(client_cert_pw_prompt_provider): Initialize the next_cred member.
(svn_client_get_ssl_client_cert_pw_prompt_provider): Initialize the
new retry_limit in the provider baton.
* subversion/libsvn_client/ssl_client_cert_providers.c
(ssl_client_cert_prompt_provider_baton_t):
Add a retry_limit struct member.
(ssl_client_cert_prompt_iter_baton_t): New struct.
(ssl_client_cert_prompt_first_cred): Allocate and initialize an
iteration baton. Provide the realmstring to the prompt function.
(ssl_client_cert_prompt_next_cred): New function to handle
authentication retries.
(client_cert_prompt_provider): Initialize the next_cred member.
(svn_client_get_ssl_client_cert_prompt_provider): Initialize the
new retry_limit in the provider baton.
* subversion/clients/cmdline/cl.h
(svn_cl__auth_ssl_client_cert_prompt): Add a realm argument.
(svn_cl__auth_ssl_client_cert_pw_prompt): Add a realm argument.
* subversion/clients/cmdline/prompt.c
(svn_cl__auth_ssl_client_cert_prompt): Print the realm.
(svn_cl__auth_ssl_client_cert_pw_prompt): Print the realm.
Do not handle the empty string specially, and do not check for NULL
since it cannot happen (and no other user seems to do it).
* subversion/clients/cmdline/main.c
(main): Use 2 as retry limit for the client cert prompt functions.
* subversion/libsvn_ra_dav/session.c
(client_ssl_keypw_callback): Removed.
(client_ssl_decrypt_cert): New function used to decrypt an encrypted
client cerificate using the auth system. Handles retries.
(client_ssl_callback): Implement a realm string for client certs.
Handle retries if the cert cannot not be loaded. Use the new
client_ssl_decrypt_cert to decrypt the certificate if needed.
Re: API issues we might want to solve for 1.0
Posted by Tobias Ringström <to...@ringstrom.mine.nu>.
Ben Reser wrote:
> On Fri, Dec 12, 2003 at 11:02:08AM -0500, Greg Hudson wrote:
>
> Add another one to the list:
>
> #10: svn_client_get_ssl_client_cert_prompt_provider and
> svn_client_get_ssl_client_cert_pw_prompt_provider
> do not have a retry limit.
> IMPACT OF PROBLEM: Clients implemnting SSL certificate
> support will only be able to give end users one chance
> at providing the proper file for the cert and the pass
> for that certificate. It seems almost certain that
> people are going to want this fixed. To do it would
> require a change in the API.
> DIFFICULTY OF FIX: Medium, there may be complications
> of trying to fix this that I'm not aware of. But
> given that simple_prompt already supports retries
> I'd image it would be straightforward unless neon
> presents a roadblock on this.
>
> I mentioned this in the past but nobody replied to it.
I'm working on fixing some other authentication annoyances at the
moment, so I'll fix this one as well. If it ends up in 1.0 or not is
another issue of course. I think it should be included because the fix
is straightforward, and it does not touch any sensitive code, but we'll see.
/Tobias
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: API issues we might want to solve for 1.0
Posted by Ben Reser <be...@reser.org>.
On Fri, Dec 12, 2003 at 11:02:08AM -0500, Greg Hudson wrote:
> As it's much easier to make API changes before 1.0 than after, here
> are a list of issues we might want to solve soon. The first three
> seem important; the rest are just unfortunate foibles (except for #6,
> but that would require a serious push to fix). Many of these have
> received attention in the past, but time marched on without anyone
> fixing them.
Add another one to the list:
#10: svn_client_get_ssl_client_cert_prompt_provider and
svn_client_get_ssl_client_cert_pw_prompt_provider
do not have a retry limit.
IMPACT OF PROBLEM: Clients implemnting SSL certificate
support will only be able to give end users one chance
at providing the proper file for the cert and the pass
for that certificate. It seems almost certain that
people are going to want this fixed. To do it would
require a change in the API.
DIFFICULTY OF FIX: Medium, there may be complications
of trying to fix this that I'm not aware of. But
given that simple_prompt already supports retries
I'd image it would be straightforward unless neon
presents a roadblock on this.
I mentioned this in the past but nobody replied to it.
--
Ben Reser <be...@reser.org>
http://ben.reser.org
"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org