Re: [WIP PATCH 1/7] Add skeleton RA svnclient
- From
- Daniel Shahaf <daniel@shahaf.name>
- Date
- Jun 25, 2010, 09:07 UTC
- Message-ID
- <alpine.561.2.00.1006251156180.2064@daniel2.local>
- In-Reply-To
- <20100625001427.GA4683@burratino>
Jonathan Nieder wrote on Fri, 25 Jun 2010 at 03:14 -0000:
Show 25 quoted lines
> Ramkumar Ramachandra wrote: > > > In future, it will dump the data in every revision > > to stdout in dumpfile format (hopefully) without resorting to the FS > > API. > > For now, just some naïve questions. Warning: I know nothing about > svn internals. > > I assume this corresponds to the ra-svn branch of > <http://github.com/artagnon/svn-dump-fast-export.git>. Has the > relevant code changed much since you sent it? > > > --- /dev/null > > +++ b/debug_editor.c > > @@ -0,0 +1,413 @@ > > +#include "svn_pools.h" > > +#include "svn_cmdline.h" > > +#include "svn_client.h" > > +#include "svn_ra.h" > > + > > +struct edit_baton > > What is a baton? >
The context object for a callback.
You call:
some_function(your_callback_function, your_baton)
which then calls:
your_callback_function(your_baton, other_arguments)
Show 11 quoted lines
> [...] > > + void *wrapped_edit_baton; > [...] > > + void *edit_baton; > > + void *wrapped_dir_baton; > [...] > > + void *edit_baton; > > + void *wrapped_file_baton; > > Are these opaque types necessary? >
The convention in Subversion's code is to convert the void * to a concrete_baton_t * only inside the callback. If you wish to declare these, e.g., as
debug_editor_baton_t *wrapped_baton;
You can probably do that too.
Show 13 quoted lines
> > +
> > +static svn_error_t *
> > +write_indent(struct edit_baton *eb, apr_pool_t *pool)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < eb->indent_level; ++i)
> > + SVN_ERR(svn_stream_printf(eb->out, pool, " "));
> > +
> > + return SVN_NO_ERROR;
> > +}
>
> What does this do? Is SVN_ERR for debugging?That's how we implement exception throwing in C. SVN_ERR means "if this returned a non-NULL svn_error_t *, then return that error to our caller".
The other pattern does
svn_error_t *err = svn_stream_printf();
and then inspects err and err->apr_err to decide whether to ignore the error or return it (possibly wrapped).
> Where does the output go? >
SVN_ERR does not print anything. It may return(), though.
Show 15 quoted lines
> > +static svn_error_t * > > +set_target_revision(void *edit_baton, > > + svn_revnum_t target_revision, > > + apr_pool_t *pool) > [...] > > +static svn_error_t * > > +open_root(void *edit_baton, > [...] > > +static svn_error_t * > > +close_edit(void *edit_baton, > [...] > > I take it these are callbacks? Is there overview documentation for > them somewhere? >
svn_delta_editor_t in http://svn.apache.org/repos/asf/subversion/trunk/subversion/include/svn_delta.h
Show 37 quoted lines
> > +svn_error_t *
> > +svn_delta__get_debug_editor(const svn_delta_editor_t **editor,
> > + void **edit_baton,
> > + const svn_delta_editor_t *wrapped_editor,
> > + void *wrapped_edit_baton,
> > + apr_pool_t *pool)
> > +{
> > + svn_delta_editor_t *tree_editor = svn_delta_default_editor(pool);
> > + struct edit_baton *eb = apr_palloc(pool, sizeof(*eb));
> > + apr_file_t *errfp;
> > + svn_stream_t *out;
> > +
> > + apr_status_t apr_err = apr_file_open_stderr(&errfp, pool);
> > + if (apr_err)
> > + return svn_error_wrap_apr(apr_err, "Problem opening stderr");
> > +
> > + out = svn_stream_from_aprfile2(errfp, TRUE, pool);
> > +
> > + tree_editor->set_target_revision = set_target_revision;
> > + tree_editor->open_root = open_root;
> > + tree_editor->delete_entry = delete_entry;
> > + tree_editor->add_directory = add_directory;
> > + tree_editor->open_directory = open_directory;
> > + tree_editor->change_dir_prop = change_dir_prop;
> > + tree_editor->close_directory = close_directory;
> > + tree_editor->absent_directory = absent_directory;
> > + tree_editor->add_file = add_file;
> > + tree_editor->open_file = open_file;
> > + tree_editor->apply_textdelta = apply_textdelta;
> > + tree_editor->change_file_prop = change_file_prop;
> > + tree_editor->close_file = close_file;
> > + tree_editor->absent_file = absent_file;
> > + tree_editor->close_edit = close_edit;
>
> I take it that the fields of svn_delta_editor_t do not have a
> well-defined order? Ugh.
> I don't understand. It seems that the fields here appear in the same order as in the definition of struct svn_delta_editor_t.
Show 29 quoted lines
> In any case, I suspect this would be easier to read rearranged a little:
>
> 1. declarations for callbacks
> 2. get_debug_editor implementation
> 3. definitions of types not needed in get_debug_editor()
> 4. implementations of callbacks
>
> That way, a person reading straight through can figure out what’s
> going on a little earlier.
>
> > --- /dev/null
> > +++ b/svnclient_ra.c
> [...]
> > +int main()
> > +{
> > + const char url[] = "http://svn.apache.org/repos/asf";
> > + svn_revnum_t start_revision = 1, end_revision = 5;
> > + if (svn_cmdline_init ("svnclient_ra", stderr) != EXIT_SUCCESS)
> > + return 1;
> > + pool = svn_pool_create(NULL);
> > +
> > + SVN_INT_ERR(open_connection(url));
> > + SVN_INT_ERR(replay_range(start_revision, end_revision));
> > +
> > + close_connection();
> > + return 0;
> > +}
>
> What is svn_cmdline_init?A Subversion API that does some necessary initializations (e.g., calls apr_initialize()). See subversion/include/svn_cmdline.h for docs (and subversion/libsvn_subr/cmdline.c for the implementation).
Show 5 quoted lines
> Is this code destined for inclusion in svn upstream, and if so, where > can one find the surrounding code this should fit in with? > > Jonathan >