artag...@apache.org wrote on Sun, 25 Jul 2010 at 12:21 -0000: > Author: artagnon > Date: Sun Jul 25 09:20:55 2010 > New Revision: 979011 > > URL: http://svn.apache.org/viewvc?rev=979011&view=rev > Log: > Add a build.conf and get svnrload to build > > * build.conf > (svnrdump): Add new section to build svnrdump. > (__ALL__): Add svnrdump. > * subversion/svnrload > (svn:ignore): Copy properties from svnrdump. > * subversion/svnrload/load_editor.c: Change the path to the load > header. > * subversion/svnrload/parse_dumpstream.c > Correct ident to match GNU style > (main): Stub out the callbacks and use the default parser. Also wrap > functions in SVN_INT_ERR appropriately for error handling. > * subversion/svnrload/svnrload.c > (main): Stub out function completely in favor of the main in > parse_dumpstream.c >
Nice log message. (Next time you might consider adding blank lines before every /^*/ line, for readability.) > Modified: > subversion/branches/svnrload/build.conf > subversion/branches/svnrload/subversion/svnrload/ (props changed) > subversion/branches/svnrload/subversion/svnrload/load_editor.c > subversion/branches/svnrload/subversion/svnrload/parse_dumpstream.c > subversion/branches/svnrload/subversion/svnrload/svnrload.c > I'm pretty sure you want to add it in a few other places; at least in build/transform_libtool_scripts.sh. And maybe in a few others... best bet is to 'grep -R svnversion $trunk'. > Modified: subversion/branches/svnrload/build.conf > URL: > http://svn.apache.org/viewvc/subversion/branches/svnrload/build.conf?rev=979011&r1=979010&r2=979011&view=diff > ============================================================================== > --- subversion/branches/svnrload/build.conf (original) > +++ subversion/branches/svnrload/build.conf Sun Jul 25 09:20:55 2010 > @@ -175,6 +175,13 @@ libs = libsvn_client libsvn_ra libsvn_de > install = bin > manpages = subversion/svnrdump/svnrdump.1 > > +[svnrload] > +description = Subversion remote repository loader > +type = exe > +path = subversion/svnrload > +libs = libsvn_client libsvn_ra libsvn_delta libsvn_subr apr > +install = bin For consistency, please: +manpages = subversion/svnrload/svnrload.1 > + > # Support for GNOME Keyring > [libsvn_auth_gnome_keyring] > description = Subversion GNOME Keyring Library > @@ -1073,7 +1080,7 @@ libs = libsvn_fs_base libsvn_fs_fs > [__ALL__] > type = project > path = build/win32 > -libs = svn svnserve svnadmin svnlook svnversion svnrdump svndumpfilter > +libs = svn svnserve svnadmin svnlook svnversion svnrdump svnrload > svndumpfilter > mod_authz_svn mod_dav_svn svnsync > > [__ALL_TESTS__] > > Modified: subversion/branches/svnrload/subversion/svnrload/parse_dumpstream.c > URL: > http://svn.apache.org/viewvc/subversion/branches/svnrload/subversion/svnrload/parse_dumpstream.c?rev=979011&r1=979010&r2=979011&view=diff > ============================================================================== > --- subversion/branches/svnrload/subversion/svnrload/parse_dumpstream.c > (original) > +++ subversion/branches/svnrload/subversion/svnrload/parse_dumpstream.c Sun > Jul 25 09:20:55 2010 > @@ -9,8 +9,8 @@ new_revision_record(void **revision_bato > void *parse_baton, > apr_pool_t *pool) > { > - printf("new_revision_record called"); > - return SVN_NO_ERROR; > + printf("new_revision_record called"); > + return SVN_NO_ERROR; > } > Please don't mix whitespace/indentation changes and semantic changes in the same commit. (I'm sure HACKING says that --- doesn't it?) > -int main() > +int > +main(int argc, char **argv) > { > - apr_pool_t *pool; > - apr_file_t *dumpfile; > - svn_stream_t *dumpstream; > - svn_repos_parse_fns2_t *parser; > - > - if (svn_cmdline_init ("parse_dumpstream", stderr) != EXIT_SUCCESS) > - return EXIT_FAILURE; > - pool = svn_pool_create(NULL); > - > - parser = apr_pcalloc(pool, sizeof(*parser)); > - > - parser->new_revision_record = new_revision_record; > - parser->new_node_record = new_node_record; > - parser->uuid_record = uuid_record; > - parser->set_revision_property = set_revision_property; > - parser->set_node_property = set_node_property; > - parser->remove_node_props = remove_node_props; > - parser->set_fulltext = set_fulltext; > - parser->close_node = close_node; > - parser->close_revision = close_revision; > - parser->delete_node_property = delete_node_property; > - parser->apply_textdelta = apply_textdelta; > - > - apr_file_open_stdin(&dumpfile, pool); > - dumpstream = svn_stream_from_aprfile2(dumpfile, FALSE, pool); > - svn_repos_parse_dumpstream2(dumpstream, parser, NULL, NULL, NULL, pool); > - svn_pool_destroy(pool); > - return 0; > + apr_pool_t *pool; > + apr_file_t *dumpfile; > + svn_stream_t *dumpstream; > + svn_repos_parse_fns2_t *parser; How am I supposed to review the changes to this function? > + struct parser_baton_t *parser_baton; > + > + if (svn_cmdline_init ("parse_dumpstream", stderr) != EXIT_SUCCESS) > + return EXIT_FAILURE; > + pool = svn_pool_create(NULL); > + > + parser = apr_pcalloc(pool, sizeof(*parser)); > + > + /* parser->new_revision_record = new_revision_record; */ > + /* parser->new_node_record = new_node_record; */ > + /* parser->uuid_record = uuid_record; */ > + /* parser->set_revision_property = set_revision_property; */ > + /* parser->set_node_property = set_node_property; */ > + /* parser->remove_node_props = remove_node_props; */ > + /* parser->set_fulltext = set_fulltext; */ > + /* parser->close_node = close_node; */ > + /* parser->close_revision = close_revision; */ > + /* parser->delete_node_property = delete_node_property; */ > + /* parser->apply_textdelta = apply_textdelta; */ > + All this are "to be done", I guess? > + apr_file_open_stdin(&dumpfile, pool); > + dumpstream = svn_stream_from_aprfile2(dumpfile, FALSE, pool); > + SVN_INT_ERR(svn_repos_parse_dumpstream2(dumpstream, parser, NULL, > + NULL, NULL, pool)); > + SVN_INT_ERR(svn_stream_close(dumpstream)); > + svn_pool_destroy(pool); > + return 0; > } > > +/* const char *url = NULL; */ > +/* char *revision_cut = NULL; */ > +/* svn_revnum_t start_revision = svn_opt_revision_unspecified; */ > +/* svn_revnum_t end_revision = svn_opt_revision_unspecified; */ > +/* svn_revnum_t latest_revision = svn_opt_revision_unspecified; */ > +/* svn_boolean_t quiet = FALSE; */ > +/* apr_pool_t *pool = NULL; */ > +/* svn_ra_session_t *session = NULL; */ > +/* const char *config_dir = NULL; */ > +/* const char *username = NULL; */ > +/* const char *password = NULL; */ > +/* svn_boolean_t no_auth_cache = FALSE; */ > +/* svn_boolean_t non_interactive = FALSE; */ > +/* apr_getopt_t *os; */ > + Why not "#if 0" the block? > +/* SVN_INT_ERR(svn_cmdline_fprintf(stderr, pool, */ > +/* "LOWER cannot be greater " */ > +/* "than UPPER.\n")); */ Unrelated, but again the error message says LOWER and UPPER. Could you rephrase it in a way that uses the semantics of the arguments rather than names in the help syntax? e.g., I'm sure we already have such a message somewhere... (/me opens fr.po) Here it is: #: ../libsvn_repos/dump.c:1048 ../libsvn_repos/dump.c:1290 #, c-format msgid "Start revision %ld is greater than end revision %ld" msgstr "La révision de début %ld est plus grande que celle de fin %ld" Daniel (btw, just last week I included the name of one of a function's formal parameters in an error message given in response to an API violation. Which, arguably, is disaligned with what I just preached.)
