You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Nuutti Kotivuori <na...@iki.fi> on 2002/10/06 15:10:03 UTC

Configuration file support for date formats

I'm wonder how to proceed on this. As per previous discussion, the
configuration support for the user-definable part of the date was
planned after the config file support was there.

So now it is - and it's time to do the date customization. Just that I
wonder, how?

The function to do conversion to a human representable date is
'svn_time_to_human_nts'. It lives in libsvn_subr. It needs to somehow
get the configuration option that specifies how the date should
look. There's three places atleast where this is called from - the
command line client, svnadmin and svnlook. The command line client is
the most important - I do not know if svnadmin and svnlook should even
use the configuration file.

So, where should parsing happen and how should the results be passed
to the function?

My first idea would be to just read in the config file when entering
the log parsing routine, passing the option in the baton for the log
display function - and then passing it on to svn_time_to_human_nts
somehow and being done with it. But maybe there's a better way?

-- Naked


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Context parameter in public API

Posted by Nuutti Kotivuori <na...@iki.fi>.
Greg Stein wrote:
> On Fri, Oct 11, 2002 at 02:37:15PM +0300, Nuutti Kotivuori wrote:
>> Roughly as pseudocode:
>> 
>>  svn_error_t *svn_client_context_open(svn_context_t **ctx);
>> 
>>  svn_error_t *svn_client_context_close(svn_context_t *ctx);
> 
> The open would take a pool, and the close would be implied by
> clearing or destroying that pool. (that's the typical pattern of use
> in SVN)

Ofcourse, thanks.

>> And in usage, for example:
>> 
>>  svn_error_t *
>>  svn_client_ls (apr_hash_t **dirents,
>>                 const char *url,
>>                 svn_opt_revision_t *revision,
>>                 svn_client_auth_baton_t *auth_baton,
>>                 svn_boolean_t recurse,
>>                 apr_pool_t *pool,
>>                 svn_context_t *ctx);
> 
> The auth_baton would be incorporated into the context.
> 
> If people wanted multiple auth within a single app, then they can
> use multiple context objects. Or maybe the context has a way to
> change auth values.

Yes, this sounds reasonable as well.

>> Is the 'config' file a feature of svn_client libraries or is it
>> something that only the command-line client uses?
> 
> The config file is one way to store options. The context should be a
> generic mechanism for passing option state. A GUI app might actually
> store all that stuff within its own state files.

What I had in mind as well.

-- Naked

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Context parameter in public API

Posted by Greg Stein <gs...@lyra.org>.
On Fri, Oct 11, 2002 at 02:37:15PM +0300, Nuutti Kotivuori wrote:
> Karl Fogel wrote:
> > Nuutti Kotivuori <na...@iki.fi> writes:
> >>> So, how about adding a context parameter to every function? I
> >>> know that's a big change, but I think we'll be forced to do that
> >>> sooner or later anyway.

Agreed.

>...
> OK, let me think this over a bit - a context parameter. How I
> understand what that would mean is that the svn_client library has a
> context, which is to be first opened, then passed to every function
> called from the library, and then closed at the end. And that context
> would be opaque to users of the client library. Roughly as pseudocode:
> 
>  svn_error_t *svn_client_context_open(svn_context_t **ctx);
> 
>  svn_error_t *svn_client_context_close(svn_context_t *ctx);

The open would take a pool, and the close would be implied by clearing or
destroying that pool. (that's the typical pattern of use in SVN)

> And in usage, for example:
> 
>  svn_error_t *
>  svn_client_ls (apr_hash_t **dirents,
>                 const char *url,
>                 svn_opt_revision_t *revision,
>                 svn_client_auth_baton_t *auth_baton,
>                 svn_boolean_t recurse,
>                 apr_pool_t *pool,
>                 svn_context_t *ctx);

The auth_baton would be incorporated into the context.

If people wanted multiple auth within a single app, then they can use
multiple context objects. Or maybe the context has a way to change auth
values.

>...
> Is the 'config' file a feature of svn_client libraries or is it
> something that only the command-line client uses?

The config file is one way to store options. The context should be a generic
mechanism for passing option state. A GUI app might actually store all that
stuff within its own state files.

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: Context parameter in public API

Posted by Branko Čibej <br...@xbc.nu>.
Karl Fogel wrote:

>"Bill Tutt" <ra...@lyra.org> writes:
>  
>
>>>Or, instead of a context baton, should we just have a global
>>>Configuration object?  (Please, no reflexive "globals are evil"
>>>responses.  If this particular use of a global object is evil, say
>>>exactly why.)
>>>      
>>>
>>The general answer to as to why globals are evil ends up revolving
>>around thread safety.
>>    
>>
>
>Right, but in this case we'd initialize a global data structure at
>startup time, based on the data in ~/.subversion/*, and then never
>modify that structure afterwards.
>
>So is thread-safety really an issue here?
>  
>

Who says it'd be a global constant? The config file implementation, for 
instance, uses an APR has table to store the config data. APR doesn't 
say anything about the thread-safety of lookups in that table.

Besides, there's another issue -- sooner or later, we'll want to support 
multithreaded clients. If we introduce a global, the client's threads 
will have to serialize on Subversion API calls. That's not acceptable.

-- 
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: Context parameter in public API

Posted by mark benedetto king <bk...@inquira.com>.
On Sun, Oct 13, 2002 at 12:13:37AM -0700, Greg Stein wrote:
> We just need to be *really* careful that we don't start lumping all kinds of
> crap into the structure. If there is any possibility that two calls will
> need to be made to a given API, and the calls will have different params,
> then we don't want to embed those params into the structure.
> 
> Cheers,
> -g
> 

Aye, there's the rub. :-)

--ben


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Context parameter in public API

Posted by Greg Stein <gs...@lyra.org>.
On Sat, Oct 12, 2002 at 01:02:57PM -0400, mark benedetto king wrote:
>...
> My feeling is that the config reading is not done at the right level
> of abstraction.  It should be read once, by the consumer of the 
> libsvn_client API.
> 
> Something like, for example:
> 
>   svn_client_t *client;
>   svn_config_t *config;
> 
>   SVN_ERR(svn_config_read_config(&config, pool));
>   SVN_ERR(svn_client_create(&client, config, pool));

Yup. You could also argue for a reversal. The config reader updates the
client object. Not sure that I have an opinion either way, tho.

> It would be Nice(TM) if the relationship between
> the configuration and the command-line switches were
> formalized.  Something like this, in main.c:
> 
>   SVN_ERR(svn_config_read_config(&config, pool));
> 
>   SVN_ERR(svn_cl__apply_options(&config,
>                                  os,
>                                  svn_cl__option_config_mapping_table,
>                                  pool));
> 
>   SVN_ERR(svn_client_create(&client, config, pool));
> 
> Obviously, that would be UI-specific.

Yes, that would be quite nice.

> Why does config need to be pulled out of the internals and
> passed along in a client context instead?
> 
> 
> 1.) Different clients might be configured differently.
>     $HOME/.subversion is really fairly unix-command-line centric.
> 
>     In the near future I suspect we'll see lots of different
>     configuration/preferences database/registries, including
>     GNOME, KDE, Win32, etc.

Yup!

> 2.) Some applications may need to support multiple concurrent
>     configuration.  The only one that comes to mind immediately
>     would be a svn-backed application that supported multiple
>     concurrent users, each with their own preferences.  Maybe

A bit harder to argue, but the APIs should *not* prevent one application
from "instantiating" SVN with different configs. Thus, a global config would
be big suckage.

>     this wiki thing people keep talking about.

Yup. To be concreate, let's say that some app is written in, say,
mod_python. That app loads a config when a request comes in, and uses it to
satisfy the operation. But woah! There is *another* thread doing the same
thing. Each thread wants their own configuration.

Globals are evil. Not just for thread *safety*, but for partitioning
operation from one thread to another.

>...
> 3.) A client context that contains the configuration "feature-proofs"
>     the existing APIs (actually, we'd probably wind up *removing*
>     parameters from them).  Right now, most simple command-line changes
>     require the addition of parameters through three or more layers of
>     APIs.  If we want to freeze these APIs (and svn_client_* is definitely
>     an API we'd like to freeze) I think it's *critical* that we put
>     this support in sooner rather than later.

Yup. This problem is also why I suggested (and will finish one day) the
libsvn_auth stuff. It would allow for changes in auth systems and mechanisms
without reflecting those changes throughout the whole codebase. The client
API is very similar. "oop! need notification. let's add it to the whole
system."

We just need to be *really* careful that we don't start lumping all kinds of
crap into the structure. If there is any possibility that two calls will
need to be made to a given API, and the calls will have different params,
then we don't want to embed those params into the structure.

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: Context parameter in public API

Posted by mark benedetto king <bk...@inquira.com>.
On Fri, Oct 11, 2002 at 11:32:18PM -0500, Karl Fogel wrote:
> Greg Hudson <gh...@MIT.EDU> writes:
> > It can be.  If a library which has to be thread-safe wants to do
> > Subversion operations, it can't be sure that two instances of itself
> > don't do "startup time" stuff at the same time.
> 
> What's wrong with two instances reading?

My feeling is that the config reading is not done at the right level
of abstraction.  It should be read once, by the consumer of the 
libsvn_client API.

Something like, for example:

  svn_client_t *client;
  svn_config_t *config;

  SVN_ERR(svn_config_read_config(&config, pool));
  SVN_ERR(svn_client_create(&client, config, pool));

It would be Nice(TM) if the relationship between
the configuration and the command-line switches were
formalized.  Something like this, in main.c:

  SVN_ERR(svn_config_read_config(&config, pool));

  SVN_ERR(svn_cl__apply_options(&config,
                                 os,
                                 svn_cl__option_config_mapping_table,
                                 pool));

  SVN_ERR(svn_client_create(&client, config, pool));

Obviously, that would be UI-specific.



Why does config need to be pulled out of the internals and
passed along in a client context instead?


1.) Different clients might be configured differently.
    $HOME/.subversion is really fairly unix-command-line centric.

    In the near future I suspect we'll see lots of different
    configuration/preferences database/registries, including
    GNOME, KDE, Win32, etc.

2.) Some applications may need to support multiple concurrent
    configuration.  The only one that comes to mind immediately
    would be a svn-backed application that supported multiple
    concurrent users, each with their own preferences.  Maybe
    this wiki thing people keep talking about.

    IHNJ, IJLS "wiki".

    wiki. wiki. wiki.

3.) A client context that contains the configuration "feature-proofs"
    the existing APIs (actually, we'd probably wind up *removing*
    parameters from them).  Right now, most simple command-line changes
    require the addition of parameters through three or more layers of
    APIs.  If we want to freeze these APIs (and svn_client_* is definitely
    an API we'd like to freeze) I think it's *critical* that we put
    this support in sooner rather than later.


--ben


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Context parameter in public API

Posted by Nuutti Kotivuori <na...@iki.fi>.
Karl Fogel wrote:
> cmpilato@collab.net writes:
>> We specifically made a config-for-all so that not every client
>> would have to implement the exact same things over and over again,
>> like proxy values.  That just puts a burder on client developers.
>> So I think we should let all the client use our configuration
>> files, and just encourage the developers of those clients to
>> protect their client-specific stuff with some sort of
>> namespace-protected sections, like [rapidsvn-general] or
>> [gsvn-general].
> 
> I think that's what Nuutti is allowing for.  The idea is that our
> library interfaces takes a context baton object.  
> 
> One way to fill that object is to call
> svn_config_read_config(&object) or whatever.
> 
> But another way is for some GUI client, say, to have its *own*
> mechanism for filling the object.  However, such a client is under
> no obligation to fill the object its own way.  It is free to just
> call svn_config_read_config(&object) like our client does.
> 
> Whatever way it gets filled, the object is then passed down to the
> client layer.  If you pass NULL, you get default behaviors.
> 
> Make sense?

Perfectly :-) This is indeed what I mean.

-- Naked

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Context parameter in public API

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
cmpilato@collab.net writes:
> We specifically made a config-for-all so that not every client would
> have to implement the exact same things over and over again, like
> proxy values.  That just puts a burder on client developers.  So I
> think we should let all the client use our configuration files, and
> just encourage the developers of those clients to protect their
> client-specific stuff with some sort of namespace-protected sections,
> like [rapidsvn-general] or [gsvn-general].

I think that's what Nuutti is allowing for.  The idea is that our
library interfaces takes a context baton object.  

One way to fill that object is to call svn_config_read_config(&object)
or whatever.

But another way is for some GUI client, say, to have its *own*
mechanism for filling the object.  However, such a client is under no
obligation to fill the object its own way.  It is free to just call
svn_config_read_config(&object) like our client does.

Whatever way it gets filled, the object is then passed down to the
client layer.  If you pass NULL, you get default behaviors.

Make sense?

-K

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Context parameter in public API

Posted by cm...@collab.net.
Nuutti Kotivuori <na...@iki.fi> writes:

> Firstly - about the configuration file in general.
> 
> I think the configuration file should be a feature of the command-line
> client only. Ofcourse a Subversion library can provide access to a
> common configuration file, but the "svn_client.h" functions should not
> internally use the configuration file. Instead, some configuration
> parameter should be given to them. Someone mentioned a lot of good
> reasons already - but basically I think this is correct because
> there's going to be quite a few places where different clients are
> going to be wanting to store their configurations.

We specifically made a config-for-all so that not every client would
have to implement the exact same things over and over again, like
proxy values.  That just puts a burder on client developers.  So I
think we should let all the client use our configuration files, and
just encourage the developers of those clients to protect their
client-specific stuff with some sort of namespace-protected sections,
like [rapidsvn-general] or [gsvn-general].

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Context parameter in public API

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Nuutti Kotivuori <na...@iki.fi> writes:
> I think svn_time_to_human_nts should be modified to accept the format
> as a parameter somehow. It might be mandatory, or there might be an
> easy hard-coded default that can be used. Then the command-line client
> subcommands log and info, svnadmin and svnlook would be modified to
> just read the configuration file themselves and pass the date format
> parameter to svn_time_to_human_nts. And then issues should be filed
> about getting sense into the configuration file processing in the long
> term.
> 
> Thoughts? ;-)

Well... hmmm.

Greg Stein distinguishes between "good kluges" and "bad kluges".  A
bad kluge is when you do work that you *know* must later be undone
because a better (but more complex) fix will be implemented.

I feel like we're verging on a bad kluge here.

How hard would it be to do the generic configuration context thing
first, and then do configurable date parsing the Right Way?

-K

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Context parameter in public API

Posted by Nuutti Kotivuori <na...@iki.fi>.
Karl Fogel wrote:

[...]

> Thoughts?,

Yes, lots :-)

Firstly - about the configuration file in general.

I think the configuration file should be a feature of the command-line
client only. Ofcourse a Subversion library can provide access to a
common configuration file, but the "svn_client.h" functions should not
internally use the configuration file. Instead, some configuration
parameter should be given to them. Someone mentioned a lot of good
reasons already - but basically I think this is correct because
there's going to be quite a few places where different clients are
going to be wanting to store their configurations.

Then - about the human representation of dates.

I think the human representation of dates is purely the responsibility
of the client. By that I mean that the command-line client, svnadmin
and svnlook are the ones wanting that behaviour - and it's their
business only. A GUI client is most probably wanting to use a vastly
different representation of dates, as are other kind of clients. The
fact that libsvn_subr provides the function is merely a convenience
for all the users of that library. And through this I also claim that
it should have no knowledge what so ever of any configuration files -
instead it should somehow take in the desired format.

And still more - about interface separation.

I think it should be much more clearly defined that what includes and
libraries are a part of the "Subversion client library" - and what are
just convenience functions that can be used by the caller, if they
wish. Personally, I think that the "svn_client.h" should be
standalone. Callers would include that file and use the functions
there to perform all operations. But this would mean that all the
results of the functions should be directly parseable by APR functions
- or the parsing functions need to be made a part of that
interface. Today this is not so, for example because the
svn_client_log function returns dates as strings to the log message
receiver - and you need to use svn_time_from_nts to get an APR date
out of that.

So - what to do then?

I think svn_time_to_human_nts should be modified to accept the format
as a parameter somehow. It might be mandatory, or there might be an
easy hard-coded default that can be used. Then the command-line client
subcommands log and info, svnadmin and svnlook would be modified to
just read the configuration file themselves and pass the date format
parameter to svn_time_to_human_nts. And then issues should be filed
about getting sense into the configuration file processing in the long
term.

Thoughts? ;-)

-- Naked

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Context parameter in public API

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Greg Hudson <gh...@MIT.EDU> writes:
> Going back to the original question, though, I'm worried about the
> amount of garbage going into the libsvn_client interface.  This is a
> very granular interface; performance-wise, no one is likely to notice if
> we allocate a pool, parse a configuration file, and maybe factor a few
> smallish composite numbers each time a call is made to a libsvn_client
> function.  However, we ask the caller to canonicalize paths, convert any
> string data to UTF-8, pass in a valid pool, and now maybe pass in a
> valid context as well.  That may be good for the libsvn_wc interface,
> which is fine-grained (yes?) and used by other libraries, but for
> libsvn_client we might want to adopt a more caller-friendly convention.

Hmmmm.  Yeah, but I think the issue here is that we'll now depend on
the config for every conversion of apr_timet_t to human-readable
string.  These conversions don't just happen at the initial call into
libsvn_client; they might happen any time, really.

Nuutti, perhaps a way to start on solving this is to first see where
we do such conversions, and try to delay them until we get out to the
original caller (the command line client itself, for example).
There's no real reason libsvn_wc has to convert dates to strings in a
way that conforms with the user's configuration, is there?  I wasn't
thinking this would apply to keyword expansion, only to printing out
dates and accepting them as input -- both of which are functions of
the client layer nearest the user.  So perhaps we should just have a
separate date conversion function that reads the config (yes,
expensive) and converts accordingly, and only the client code would
use it.

Thoughts?,
-K


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Context parameter in public API

Posted by Greg Hudson <gh...@MIT.EDU>.
On Sat, 2002-10-12 at 01:23, Karl Fogel wrote:
> Let me be clearer: possibly the initialization code will need to be
> careful, sure.  It will need to get a mutex on the data structure, and
> then when it has the mutex, check some flag in the data structure to
> determine if config initialization has already been completed.  IOW,
> I'm certainly not saying we can ignore thread issues here, just that
> it seems to [possibly naive] me that they're quite solveable...

I'm not sure what you're proposing.

If you're saying, "Let's put locks around our initialization code,"
then:

  * I think it means introducing mutex manipulation into our code for
    the first time.  It appears APR has thread support, but I don't know
    if there are any complications from using it.

  * Assuming we have a way of rereading the config files or otherwise
    changing configuration data (which a GUI client could want), then
    any code which accesses the configuration data will need to grab a
    mutex first.  That could, in theory, become a performance killer.

If you're saying, "Let's document that the caller must insure that the
initialization code is called exactly once, before anything else
happens," then we no longer have to worry about mutexes, but we've
merely pushed the problem onto the caller.  If the caller is an
application, this is no problem, but if the caller is a library, now the
library has to play with mutexes when it might not otherwise have needed
to.

In general, it's more elegant, more robust, and more scalable to write
reentrant code.

Going back to the original question, though, I'm worried about the
amount of garbage going into the libsvn_client interface.  This is a
very granular interface; performance-wise, no one is likely to notice if
we allocate a pool, parse a configuration file, and maybe factor a few
smallish composite numbers each time a call is made to a libsvn_client
function.  However, we ask the caller to canonicalize paths, convert any
string data to UTF-8, pass in a valid pool, and now maybe pass in a
valid context as well.  That may be good for the libsvn_wc interface,
which is fine-grained (yes?) and used by other libraries, but for
libsvn_client we might want to adopt a more caller-friendly convention.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Context parameter in public API

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Karl Fogel <kf...@newton.ch.collab.net> writes:
> > It can be.  If a library which has to be thread-safe wants to do
> > Subversion operations, it can't be sure that two instances of itself
> > don't do "startup time" stuff at the same time.
> 
> What's wrong with two instances reading?
> 
> (Subversion doesn't modify those files, it just reads them.)
> 
> Sorry, not being deliberately obtuse here, I'm just not seeing the
> mechanism by which this *particular* instance of globality is
> problematic...

Let me be clearer: possibly the initialization code will need to be
careful, sure.  It will need to get a mutex on the data structure, and
then when it has the mutex, check some flag in the data structure to
determine if config initialization has already been completed.  IOW,
I'm certainly not saying we can ignore thread issues here, just that
it seems to [possibly naive] me that they're quite solveable...

But I'm not an experienced multi-threaded programmer, so perhaps I'm
missing something.

-K

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Context parameter in public API

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Greg Hudson <gh...@MIT.EDU> writes:
> It can be.  If a library which has to be thread-safe wants to do
> Subversion operations, it can't be sure that two instances of itself
> don't do "startup time" stuff at the same time.

What's wrong with two instances reading?

(Subversion doesn't modify those files, it just reads them.)

Sorry, not being deliberately obtuse here, I'm just not seeing the
mechanism by which this *particular* instance of globality is
problematic...

?,
-K

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Context parameter in public API

Posted by Greg Hudson <gh...@MIT.EDU>.
On Sat, 2002-10-12 at 00:04, Karl Fogel wrote:
> Right, but in this case we'd initialize a global data structure at
> startup time, based on the data in ~/.subversion/*, and then never
> modify that structure afterwards.

A GUI client might want a way to reread that, actually.  But,
independently of that:

> So is thread-safety really an issue here?

It can be.  If a library which has to be thread-safe wants to do
Subversion operations, it can't be sure that two instances of itself
don't do "startup time" stuff at the same time.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Context parameter in public API

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
"Bill Tutt" <ra...@lyra.org> writes:
> > Or, instead of a context baton, should we just have a global
> > Configuration object?  (Please, no reflexive "globals are evil"
> > responses.  If this particular use of a global object is evil, say
> > exactly why.)
> 
> The general answer to as to why globals are evil ends up revolving
> around thread safety.

Right, but in this case we'd initialize a global data structure at
startup time, based on the data in ~/.subversion/*, and then never
modify that structure afterwards.

So is thread-safety really an issue here?

-K

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

RE: Re: Context parameter in public API

Posted by Bill Tutt <ra...@lyra.org>.

> From: Karl Fogel [mailto:kfogel@newton.ch.collab.net]
> 
> Nuutti Kotivuori <na...@iki.fi> writes:
> > Is the 'config' file a feature of svn_client libraries or is it
> > something that only the command-line client uses?
> 
> It's a feature of all of clients, IMHO.  Our other configuration files
> (proxies) affect all clients, no reason why this one shouldn't as
> well.  There might eventually be configuration options which are
> specific to certain clients, but we can deal with that when it arises.
> 
> > If it is a part of the svn_client library functionality - then it
> > sprouts a few more questions. Is svn_time_to_human_nts part of the
> > svn_client library? I guess it is since it's supposed to be called
by
> > the callers of svn_client library. Does that mean that if the
context
> > is implemented, you cannot call svn_time_to_human_nts without having
a
> > context? How does this affect svnadmin and svnlook which need it
too?
> 
> Hmmm.  I mean, we could define svn_time_to_human_nts's behavior when
> the context baton is null, that's one solution.
> 
> Or, instead of a context baton, should we just have a global
> Configuration object?  (Please, no reflexive "globals are evil"
> responses.  If this particular use of a global object is evil, say
> exactly why.)
> 

The general answer to as to why globals are evil ends up revolving
around thread safety.

Bill


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Context parameter in public API

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Nuutti Kotivuori <na...@iki.fi> writes:
> Is the 'config' file a feature of svn_client libraries or is it
> something that only the command-line client uses?

It's a feature of all of clients, IMHO.  Our other configuration files
(proxies) affect all clients, no reason why this one shouldn't as
well.  There might eventually be configuration options which are
specific to certain clients, but we can deal with that when it arises.

> If it is a part of the svn_client library functionality - then it
> sprouts a few more questions. Is svn_time_to_human_nts part of the
> svn_client library? I guess it is since it's supposed to be called by
> the callers of svn_client library. Does that mean that if the context
> is implemented, you cannot call svn_time_to_human_nts without having a
> context? How does this affect svnadmin and svnlook which need it too?

Hmmm.  I mean, we could define svn_time_to_human_nts's behavior when
the context baton is null, that's one solution.

Or, instead of a context baton, should we just have a global
Configuration object?  (Please, no reflexive "globals are evil"
responses.  If this particular use of a global object is evil, say
exactly why.)

> So, I am a bit of a loss here - this is your territory, not mine :-)

I guess I'll need to work on expanding your territory to include all
of Subversion, then :-).  

(Heh, not trying to avoid the discussion, just saying that you don't
need to limit your scope in Subversion.)

-K

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Context parameter in public API

Posted by Nuutti Kotivuori <na...@iki.fi>.
Karl Fogel wrote:
> Nuutti Kotivuori <na...@iki.fi> writes:
>>> So, how about adding a context parameter to every function? I
>>> know that's a big change, but I think we'll be forced to do that
>>> sooner or later anyway.
>> 
>> Er um er. My knees are already shaking. Every function? :-)
>> 
>> I think someone more knowledgeable than me should define a bit more
>> carefully what indeed is 'every' function - and how the whole thing
>> should be handled.
> 
> Why don't we start by adding it to the call paths where we actually
> need it this time?  That'll probably teach us something, then we can
> decide about other paths...
> 
> And when I say "we", I mean Nuutti, of course :-).

OK, let me think this over a bit - a context parameter. How I
understand what that would mean is that the svn_client library has a
context, which is to be first opened, then passed to every function
called from the library, and then closed at the end. And that context
would be opaque to users of the client library. Roughly as pseudocode:

 svn_error_t *svn_client_context_open(svn_context_t **ctx);

 svn_error_t *svn_client_context_close(svn_context_t *ctx);

And in usage, for example:

 svn_error_t *
 svn_client_ls (apr_hash_t **dirents,
                const char *url,
                svn_opt_revision_t *revision,
                svn_client_auth_baton_t *auth_baton,
                svn_boolean_t recurse,
                apr_pool_t *pool,
                svn_context_t *ctx);

This sounds reasonable - and fine - and cool. But I'm not sure how to
handle this current problem at hand. Actually it boils down to a
question that must be clear to you but which hasn't really gotten
through to me yet.

Is the 'config' file a feature of svn_client libraries or is it
something that only the command-line client uses?

If it is a part of the svn_client library functionality - then it
sprouts a few more questions. Is svn_time_to_human_nts part of the
svn_client library? I guess it is since it's supposed to be called by
the callers of svn_client library. Does that mean that if the context
is implemented, you cannot call svn_time_to_human_nts without having a
context? How does this affect svnadmin and svnlook which need it too?

Or, on the other hand, if the 'config' file is a part of the cmdline
client - what does the context have to do with anything? Or do you
mean a context in the sense that the commandline client should open
the config file at start and pass it on to its own functions. But how
would the information get to svn_time_to_human_nts then? Additional
parameter perhaps? Again to all callers?

So, I am a bit of a loss here - this is your territory, not mine :-)

-- Naked


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Context parameter in public API

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Nuutti Kotivuori <na...@iki.fi> writes:
> > So, how about adding a context parameter to every function? I know
> > that's a big change, but I think we'll be forced to do that sooner
> > or later anyway.
> 
> Er um er. My knees are already shaking. Every function? :-)
> 
> I think someone more knowledgeable than me should define a bit more
> carefully what indeed is 'every' function - and how the whole thing
> should be handled.

Why don't we start by adding it to the call paths where we actually
need it this time?  That'll probably teach us something, then we can
decide about other paths...

And when I say "we", I mean Nuutti, of course :-).

-K

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Context parameter in public API

Posted by Nuutti Kotivuori <na...@iki.fi>.
brane@xbc.nu wrote:
> We're fairly screwed by not having a context parameter in the public
> API. That means we can't pass configuration data down the call
> chain, among other things (such a beast would be ideal for
> cancellation processing, too...).

Right.

> We're reading and parsing the config files multiple times already,
> which is bad.

Agreed.

> So, how about adding a context parameter to every function? I know
> that's a big change, but I think we'll be forced to do that sooner
> or later anyway.

Er um er. My knees are already shaking. Every function? :-)

I think someone more knowledgeable than me should define a bit more
carefully what indeed is 'every' function - and how the whole thing
should be handled.

In the meantime, I think I'll go do something else :-)

-- Naked

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Configuration file support for date formats

Posted by Branko Čibej <br...@xbc.nu>.
Nuutti Kotivuori wrote:

>I'm wonder how to proceed on this. As per previous discussion, the
>configuration support for the user-definable part of the date was
>planned after the config file support was there.
>
>So now it is - and it's time to do the date customization. Just that I
>wonder, how?
>
>The function to do conversion to a human representable date is
>'svn_time_to_human_nts'. It lives in libsvn_subr. It needs to somehow
>get the configuration option that specifies how the date should
>look. There's three places atleast where this is called from - the
>command line client, svnadmin and svnlook. The command line client is
>the most important - I do not know if svnadmin and svnlook should even
>use the configuration file.
>
>So, where should parsing happen and how should the results be passed
>to the function?
>
>My first idea would be to just read in the config file when entering
>the log parsing routine, passing the option in the baton for the log
>display function - and then passing it on to svn_time_to_human_nts
>somehow and being done with it. But maybe there's a better way?
>  
>
We're fairly screwed by not having a context parameter in the public 
API. That means we can't pass configuration data down the call chain, 
among other things (such a beast would be ideal for cancellation 
processing, too...). We're reading and parsing the config files multiple 
times already, which is bad.

So, how about adding a context parameter to every function? I know 
that's a big change, but I think we'll be forced to do that sooner or 
later anyway.

-- 
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