Ben Collins-Sussman <sussman@collab.net> wrote:
> Take a look at minimal_client.c . All a commandline-based client
> application does is parse arguments, initialize some context
> structures, load some authentication providers, and then ask
> libsvn_client to do the real work. What sort of things would
> libsvn_cmdline offer? I don't see the 'itch'.
>
> Chia-liang Kao wrote:
> > I guess it's mainly for those prompt callbacks used by the auth
> > provider functions.
>
> Those are awfully simple things -- something which merely prompts the
> user for a string? It seems a bit of overkill to create a whole
> shared library for such a thing, don't you think? I would imagine
> that writing something in python or perl would be *much* simpler than
> calling a C function.
It's easy to create a minimal Subversion client, but a full-fledged
client is more work. Sure, it's easy to add a few callbacks to prompt
the user for information, but when you add up all the little functions
you get a complex system. There's almost 9000 lines of code in the
clients/cmdline directory -- I don't think that's trivial at all.
Who would use libsvn_cmdline? Anyone who wants to add functionality to
the standard command-line client. If you want to just change a few
functions from the command-line client, it is very helpful to have
access to the rest of the implementation.
In order to allow the Python bindings to be tested by the standard
command-line test suite, I'm trying to rewrite the interesting
functionality from the Subversion command-line client in Python, piece
by piece. If I can implement and replace each function separately, it
will make my life a lot easier. This will only be possible if we have a
libsvn_cmdline library.
I'm sure the libsvn_cmdline library will help other people too. My
recommendation: Convert the current command-line client into a library
right now, but mark the API as experimental. Once people start using
the command-line functions as a library, we'll start to see what needs
to be abstracted.
Karl Fogel <kfogel@collab.net> writes:
> David James <james82@gmail.com> writes:
> > Questions:
> > - Is it OK to create the new libsvn_cmdline library? It will make
> > my command-line SWIG work this summer much easier.
> Personally, I would like to see some of the abstraction patches
> first. Those patches can include the new libsvn_cmdline/ subdir
> if necessary. Then, assuming the patches are okay to check in on the
> mainline, we can do so.
Works for me. I can send in my abstraction patches first. I'd like to
get the libsvn_cmdline library up and running soon, though. Even if we
just converted the current command-line client directly into a library,
without making any changes besides renaming the 'main' function, I
think this library would be quite useful to many people, including
myself.
Karl Fogel <kfogel@collab.net> writes:
> > - Is it OK to use malloc / free in the SWIG bindings? In some
> > cases, this seems to be the only solution because APR pools are
> > not always available in a typemap.
> Isn't the solution to change the typemap? Are we using malloc/free in
> other bindings?
Here's an example of a function (from main.c) which is difficult to
Swigify without using malloc/free.
int svn_cmdline_main (int argc, const char * const *argv)
(This is the 'main' function from clients/cmdline/main.c)
When you call the svn_cmdline_main function using Python strings, we
need to allocate memory to store C-compatible versions of these
strings. We can easily accomplish this using malloc/free. Allocating a
brand new APR pool for this purpose would be very wasteful, especially
since the purpose of the svn_cmdline_main function already allocates
its own root pool. So, I think the best solution here is to use
malloc/free. What do you think?
svn_cmdline_main isn't the only function which suffers from this
problem. Here are some more examples from the bindings:
/* Convert Python strings to svn_string_buf_t * objects */
%typemap(python,in) svn_stringbuf_t * {
if (!PyString_Check($input)) {
PyErr_SetString(PyExc_TypeError, "not a string");
return NULL;
}
$1 = svn_stringbuf_ncreate(PyString_AS_STRING($input),
PyString_GET_SIZE($input),
/* ### gah... what pool to use? */
_global_pool);
}
The current typemap for svn_stringbuf_t uses the _global_pool variable,
even though this variable does not exist in all contexts. As a result,
our SWIG bindings are less complete than they could be. Right now,
our SWIG bindings ignore all functions and structures which use
string objects but do not use pools. Read below for details:
From svn_delta.i:
/* mark window.new_data as readonly since we would need a pool to set
it properly (e.g. to allocate an svn_string_t structure).
*/
%immutable svn_txdelta_window_t::new_data;
From svn_wc.i:
/* ### these functions require a pool, which we don't have immediately
### handy. just eliminate these funcs for now.
*/
%ignore svn_wc_set_auth_file;
/* ### ignore this structure because the accessors will need a pool */
%ignore svn_wc_keywords_t;
malloc/free won't solve the above problems. Long-lived objects need to
live in pools to prevent memory leaks. But for simple functions, such
as the 'main' function, where the lifetime of the object is same as the
lifetime of the function call, a simple malloc/free solution will do
the trick.
For more complicated functions, we'll need to implement a 'default
pool' system with reference-counting for Python. I'd love to see that
implemented -- it'd hugely improve the Python bindings.
--
David James -- http://www.cs.toronto.edu/~james
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Jun 29 18:52:03 2005