Re: [WIP PATCH 1/7] Add skeleton RA svnclient
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Jun 25, 2010, 00:14 UTC
- Message-ID
- <20100625001427.GA4683@burratino>
- In-Reply-To
- <1277310140-16891-2-git-send-email-artagnon@gmail.com>
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?
Show 9 quoted lines
> --- /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?
[...]
> + void *wrapped_edit_baton;
[...]
> + void *edit_baton; > + void *wrapped_dir_baton;
[...]
> + void *edit_baton; > + void *wrapped_file_baton;
Are these opaque types necessary?
Show 11 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? Where does the output go?
> +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?
Show 33 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.
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
[...]
Show 14 quoted lines
> +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? 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