{"thread":{"id":"24313","subject":"[PATCH 02/13] Add skeleton SVN client and Makefile","startedAt":"2010-07-07T00:14:40Z","lastAt":"2010-07-26T17:53:27Z","messageCount":31,"participants":["Ramkumar Ramachandra","Jonathan Nieder","Daniel Shahaf","Julian Foad"],"isPatch":true,"patchVersion":1,"patchTotal":13},"messages":[{"id":"144977","messageId":"1278461693-3828-1-git-send-email-artagnon@gmail.com","threadId":"24313","inReplyTo":null,"subject":"[GSoC update] git-remote-svn: Week 10","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-07-07T00:14:40Z","receivedAt":"2010-07-07T00:14:40Z","isPatch":false,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nAfter an extended weekend of hacking, I'm happy to report that I've\nfinally managed to finish the SVN client (now renamed svndumpr) and\nvalidate it against 10,000 revisions of the ASF repository. It doesn't\nlook like I can go much further with the validation until the\ndumpfilev3 parser is completed due to hardware limitations. However,\nAvar has been kind enough to lend me access to a more powerful machine\non which I intend to run more tests over the next few days.\n\nThe last patch is a nice validation script; I would request everyone\nwho's interested in the project to test it against their repositories\nso we can weed out any remaining bugs. The code is also available on\nmy GitHub [1]. Also, please review the series thoroughly and don't\nhesitate to ask trivial questions: I'm new to developing with libsvn\nmyself.\n\nPlease note that it has been built and tested only against the\nSubversion trunk: for Subversion 1.6, you can try using my\nra-svn-1.6. Also, there seems to be some unresolved issue on 64-bit\nsystems. We're working on fixing this.\n\nFinal note: I'll begin preparing to merge this into the Subversion\ntrunk soon.\n\nThanks for reading.\n\n[1]: ra-svn and ra-svn-rollout branches of\ngithub.com/artagnon/svn-dump-fast-export\n\n-- Ram\n\nRamkumar Ramachandra (13):\n Add LICENSE\n Add skeleton SVN client and Makefile\n Add debug editor from Subversion trunk\n Add skeleton dump editor\n Drive the debug editor\n Dump the revprops at the start of every revision\n Implement open_root and close_edit\n Implement dump_node\n Implement directory-related functions\n Implement file-related functions\n Implement apply_textdelta\n Implement close_file\n Add a validation script\n"},{"id":"144980","messageId":"1278461693-3828-2-git-send-email-artagnon@gmail.com","threadId":"24313","inReplyTo":"1278461693-3828-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 01/13] Add LICENSE","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-07-07T00:14:41Z","receivedAt":"2010-07-07T00:14:41Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"License the project under a two-clause BSD-style license. A dual\nlicense will be required later when attempting to merge into\nSubversion.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n LICENSE | 26 ++++++++++++++++++++++++++\n 1 files changed, 26 insertions(+), 0 deletions(-)\n create mode 100644 LICENSE\n\ndiff --git a/LICENSE b/LICENSE\nnew file mode 100644\nindex 0000000..4367b7c\n--- /dev/null\n+++ b/LICENSE\n@@ -0,0 +1,26 @@\n+Copyright (C) 2010 Ramkumar Ramachandra\n+All rights reserved.\n+\n+Redistribution and use in source and binary forms, with or without\n+modification, are permitted provided that the following conditions\n+are met:\n+1. Redistributions of source code must retain the above copyright\n+ notice(s), this list of conditions and the following disclaimer\n+ unmodified other than the allowable addition of one or more\n+ copyright notices.\n+2. Redistributions in binary form must reproduce the above copyright\n+ notice(s), this list of conditions and the following disclaimer in\n+ the documentation and/or other materials provided with the\n+ distribution.\n+\n+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDER(S) ``AS IS'' AND ANY\n+EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE\n+IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR\n+PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER(S) BE\n+LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR\n+CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF\n+SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR\n+BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,\n+WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE\n+OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE,\n+EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.\n-- \n1.7.1\n"},{"id":"144974","messageId":"1278461693-3828-3-git-send-email-artagnon@gmail.com","threadId":"24313","inReplyTo":"1278461693-3828-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 02/13] Add skeleton SVN client and Makefile","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-07-07T00:14:42Z","receivedAt":"2010-07-07T00:14:42Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Add a basic SVN command-line client along with a Makefile that does\njust enough to establish a connection with the ASF subversion server;\nit initializes a memory pool, sees that configuration files are in\norder, builds up a context object, sets up an authentication baton,\nand finally opens a session to the subversion server.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n Makefile | 8 +++++++\n svndumpr.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 76 insertions(+), 0 deletions(-)\n create mode 100644 Makefile\n create mode 100644 svndumpr.c\n\ndiff --git a/Makefile b/Makefile\nnew file mode 100644\nindex 0000000..a6022f7\n--- /dev/null\n+++ b/Makefile\n@@ -0,0 +1,8 @@\n+svndumpr: *.c *.h\n+\t$(CC) -Wall -Werror -DAPR_POOL_DEBUG -ggdb3 -O0 -o $@ svndumpr.c -lsvn_client-1 -I. -I/usr/include/subversion-1 -I/usr/include/apr-1.0\n+\n+svndumpr_bench: *.c *.h\n+\t$(CC) -O2 -o $@ svndumpr.c -lsvn_client-1 -I. -I/usr/include/subversion-1 -I/usr/include/apr-1.0\n+\n+clean:\n+\t$(RM) svndumpr svndumpr_bench\ndiff --git a/svndumpr.c b/svndumpr.c\nnew file mode 100644\nindex 0000000..737c4aa\n--- /dev/null\n+++ b/svndumpr.c\n@@ -0,0 +1,68 @@\n+/* Licensed under a two-clause BSD-style license.\n+ * See LICENSE for details.\n+ */\n+\n+#include \"svn_pools.h\"\n+#include \"svn_cmdline.h\"\n+#include \"svn_client.h\"\n+#include \"svn_ra.h\"\n+#include \"svn_repos.h\"\n+\n+static apr_pool_t *pool = NULL;\n+static svn_client_ctx_t *ctx = NULL;\n+static svn_ra_session_t *session = NULL;\n+\n+svn_error_t *populate_context()\n+{\n+\tconst char *http_library;\n+\t\n+\tSVN_ERR(svn_config_get_config(&(ctx->config), NULL, pool));\n+\t\n+\thttp_library = getenv(\"SVN_HTTP_LIBRARY\");\n+\tif (http_library)\n+\t\tsvn_config_set(apr_hash_get(ctx->config, \"servers\", APR_HASH_KEY_STRING),\n+\t\t \"global\", \"http-library\", http_library);\n+\treturn SVN_NO_ERROR;\n+}\n+\n+svn_error_t *open_connection(const char *url)\n+{\n+\tSVN_ERR(svn_config_ensure (NULL, pool));\n+\tSVN_ERR(svn_client_create_context (&ctx, pool));\n+\tSVN_ERR(svn_ra_initialize(pool));\n+\n+#if defined(WIN32) || defined(__CYGWIN__)\n+\tif (getenv(\"SVN_ASP_DOT_NET_HACK\"))\n+\t\tSVN_ERR(svn_wc_set_adm_dir(\"_svn\", pool));\n+#endif\n+\n+\tSVN_ERR(populate_context());\n+\tSVN_ERR(svn_cmdline_create_auth_baton(&(ctx->auth_baton), TRUE,\n+\t\t\t\t\t NULL, NULL, NULL, FALSE,\n+\t\t\t\t\t FALSE, NULL, NULL, NULL,\n+\t\t\t\t\t pool));\n+\tSVN_ERR(svn_client_open_ra_session(&session, url, ctx, pool));\n+\treturn SVN_NO_ERROR;\n+}\n+\n+svn_error_t *replay_range(svn_revnum_t start_revision, svn_revnum_t end_revision)\n+{\n+\treturn SVN_NO_ERROR;\n+}\n+\n+int main()\n+{\n+\tconst char url[] = \"http://svn.apache.org/repos/asf\";\n+\tsvn_revnum_t start_revision = 1, end_revision = 500;\n+\tif (svn_cmdline_init (\"svndumpr\", stderr) != EXIT_SUCCESS)\n+\t\treturn 1;\n+\n+\tpool = svn_pool_create(NULL);\n+\n+\tSVN_INT_ERR(open_connection(url));\n+\tSVN_INT_ERR(replay_range(start_revision, end_revision));\n+\n+\tsvn_pool_destroy(pool);\n+\t\n+\treturn 0;\n+}\n-- \n1.7.1\n"},{"id":"144975","messageId":"1278461693-3828-4-git-send-email-artagnon@gmail.com","threadId":"24313","inReplyTo":"1278461693-3828-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 03/13] Add debug editor from Subversion trunk","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-07-07T00:14:43Z","receivedAt":"2010-07-07T00:14:43Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Add the debug editor from subversion/libsvn_delta/debug_editor.c along\nwith a header to expose the svn_delta__get_debug_editor function.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n Makefile | 4 +-\n debug_editor.c | 402 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n debug_editor.h | 10 ++\n 3 files changed, 414 insertions(+), 2 deletions(-)\n create mode 100644 debug_editor.c\n create mode 100644 debug_editor.h\n\ndiff --git a/Makefile b/Makefile\nindex a6022f7..e4d106e 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1,8 +1,8 @@\n svndumpr: *.c *.h\n-\t$(CC) -Wall -Werror -DAPR_POOL_DEBUG -ggdb3 -O0 -o $@ svndumpr.c -lsvn_client-1 -I. -I/usr/include/subversion-1 -I/usr/include/apr-1.0\n+\t$(CC) -Wall -Werror -DAPR_POOL_DEBUG -ggdb3 -O0 -o $@ svndumpr.c debug_editor.c -lsvn_client-1 -I. -I/usr/local/include/subversion-1 -I/usr/include/apr-1.0\n \n svndumpr_bench: *.c *.h\n-\t$(CC) -O2 -o $@ svndumpr.c -lsvn_client-1 -I. -I/usr/include/subversion-1 -I/usr/include/apr-1.0\n+\t$(CC) -O2 -o $@ svndumpr.c debug_editor.c -lsvn_client-1 -I. -I/usr/local/include/subversion-1 -I/usr/include/apr-1.0\n \n clean:\n \t$(RM) svndumpr svndumpr_bench\ndiff --git a/debug_editor.c b/debug_editor.c\nnew file mode 100644\nindex 0000000..8164477\n--- /dev/null\n+++ b/debug_editor.c\n@@ -0,0 +1,402 @@\n+/* Licensed under a two-clause BSD-style license.\n+ * See LICENSE for details.\n+ */\n+\n+#include \"svn_pools.h\"\n+#include \"svn_cmdline.h\"\n+#include \"svn_client.h\"\n+#include \"svn_ra.h\"\n+\n+#include \"debug_editor.h\"\n+\n+struct edit_baton\n+{\n+\tconst svn_delta_editor_t *wrapped_editor;\n+\tvoid *wrapped_edit_baton;\n+\n+\tint indent_level;\n+\n+\tsvn_stream_t *out;\n+};\n+\n+struct dir_baton\n+{\n+\tvoid *edit_baton;\n+\tvoid *wrapped_dir_baton;\n+};\n+\n+struct file_baton\n+{\n+\tvoid *edit_baton;\n+\tvoid *wrapped_file_baton;\n+};\n+\n+static svn_error_t *write_indent(struct edit_baton *eb, apr_pool_t *pool)\n+{\n+\tint i;\n+\n+\tfor (i = 0; i < eb->indent_level; ++i)\n+\t\tSVN_ERR(svn_stream_printf(eb->out, pool, \" \"));\n+\n+\treturn SVN_NO_ERROR;\n+}\n+\n+static svn_error_t *set_target_revision(void *edit_baton,\n+\t\t\t\t\tsvn_revnum_t target_revision,\n+\t\t\t\t\tapr_pool_t *pool)\n+{\n+\tstruct edit_baton *eb = edit_baton;\n+\n+\tSVN_ERR(write_indent(eb, pool));\n+\tSVN_ERR(svn_stream_printf(eb->out, pool, \"set_target_revision : %ld\\n\",\n+\t\t\t\t target_revision));\n+\n+\treturn eb->wrapped_editor->set_target_revision(eb->wrapped_edit_baton,\n+\t\t\t\t\t\t target_revision,\n+\t\t\t\t\t\t pool);\n+}\n+\n+static svn_error_t *open_root(void *edit_baton,\n+\t\t\t svn_revnum_t base_revision,\n+\t\t\t apr_pool_t *pool,\n+\t\t\t void **root_baton)\n+{\n+\tstruct edit_baton *eb = edit_baton;\n+\tstruct dir_baton *dir_baton = apr_palloc(pool, sizeof(*dir_baton));\n+\n+\tSVN_ERR(write_indent(eb, pool));\n+\tSVN_ERR(svn_stream_printf(eb->out, pool, \"open_root : %ld\\n\",\n+\t\t\t\t base_revision));\n+\teb->indent_level++;\n+\n+\tSVN_ERR(eb->wrapped_editor->open_root(eb->wrapped_edit_baton,\n+\t\t\t\t\t base_revision,\n+\t\t\t\t\t pool,\n+\t\t\t\t\t &dir_baton->wrapped_dir_baton));\n+\n+\tdir_baton->edit_baton = edit_baton;\n+\n+\t*root_baton = dir_baton;\n+\n+\treturn SVN_NO_ERROR;\n+}\n+\n+static svn_error_t *delete_entry(const char *path,\n+\t\t\t\t svn_revnum_t base_revision,\n+\t\t\t\t void *parent_baton,\n+\t\t\t\t apr_pool_t *pool)\n+{\n+\tstruct dir_baton *pb = parent_baton;\n+\tstruct edit_baton *eb = pb->edit_baton;\n+\n+\tSVN_ERR(write_indent(eb, pool));\n+\tSVN_ERR(svn_stream_printf(eb->out, pool, \"delete_entry : %s:%ld\\n\",\n+\t\t\t\t path, base_revision));\n+\n+\treturn eb->wrapped_editor->delete_entry(path,\n+\t\t\t\t\t\tbase_revision,\n+\t\t\t\t\t\tpb->wrapped_dir_baton,\n+\t\t\t\t\t\tpool);\n+}\n+\n+static svn_error_t *add_directory(const char *path,\n+\t\t\t\t void *parent_baton,\n+\t\t\t\t const char *copyfrom_path,\n+\t\t\t\t svn_revnum_t copyfrom_revision,\n+\t\t\t\t apr_pool_t *pool,\n+\t\t\t\t void **child_baton)\n+{\n+\tstruct dir_baton *pb = parent_baton;\n+\tstruct edit_baton *eb = pb->edit_baton;\n+\tstruct dir_baton *b = apr_palloc(pool, sizeof(*b));\n+\n+\tSVN_ERR(write_indent(eb, pool));\n+\tSVN_ERR(svn_stream_printf(eb->out, pool,\n+\t\t\t\t \"add_directory : '%s' [from '%s':%ld]\\n\",\n+\t\t\t\t path, copyfrom_path, copyfrom_revision));\n+\teb->indent_level++;\n+\n+\tSVN_ERR(eb->wrapped_editor->add_directory(path,\n+\t\t\t\t\t\t pb->wrapped_dir_baton,\n+\t\t\t\t\t\t copyfrom_path,\n+\t\t\t\t\t\t copyfrom_revision,\n+\t\t\t\t\t\t pool,\n+\t\t\t\t\t\t &b->wrapped_dir_baton));\n+\n+\tb->edit_baton = eb;\n+\t*child_baton = b;\n+\n+\treturn SVN_NO_ERROR;\n+}\n+\n+static svn_error_t *open_directory(const char *path,\n+\t\t\t\t void *parent_baton,\n+\t\t\t\t svn_revnum_t base_revision,\n+\t\t\t\t apr_pool_t *pool,\n+\t\t\t\t void **child_baton)\n+{\n+\tstruct dir_baton *pb = parent_baton;\n+\tstruct edit_baton *eb = pb->edit_baton;\n+\tstruct dir_baton *db = apr_palloc(pool, sizeof(*db));\n+\n+\tSVN_ERR(write_indent(eb, pool));\n+\tSVN_ERR(svn_stream_printf(eb->out, pool, \"open_directory : '%s':%ld\\n\",\n+\t\t\t\t path, base_revision));\n+\teb->indent_level++;\n+\n+\tSVN_ERR(eb->wrapped_editor->open_directory(path,\n+\t\t\t\t\t\t pb->wrapped_dir_baton,\n+\t\t\t\t\t\t base_revision,\n+\t\t\t\t\t\t pool,\n+\t\t\t\t\t\t &db->wrapped_dir_baton));\n+\n+\tdb->edit_baton = eb;\n+\t*child_baton = db;\n+\n+\treturn SVN_NO_ERROR;\n+}\n+\n+static svn_error_t *add_file(const char *path,\n+\t\t\t void *parent_baton,\n+\t\t\t const char *copyfrom_path,\n+\t\t\t svn_revnum_t copyfrom_revision,\n+\t\t\t apr_pool_t *pool,\n+\t\t\t void **file_baton)\n+{\n+\tstruct dir_baton *pb = parent_baton;\n+\tstruct edit_baton *eb = pb->edit_baton;\n+\tstruct file_baton *fb = apr_palloc(pool, sizeof(*fb));\n+\n+\tSVN_ERR(write_indent(eb, pool));\n+\tSVN_ERR(svn_stream_printf(eb->out, pool,\n+\t\t\t\t \"add_file : '%s' [from '%s':%ld]\\n\",\n+\t\t\t\t path, copyfrom_path, copyfrom_revision));\n+\n+\teb->indent_level++;\n+\n+\tSVN_ERR(eb->wrapped_editor->add_file(path,\n+\t\t\t\t\t pb->wrapped_dir_baton,\n+\t\t\t\t\t copyfrom_path,\n+\t\t\t\t\t copyfrom_revision,\n+\t\t\t\t\t pool,\n+\t\t\t\t\t &fb->wrapped_file_baton));\n+\n+\tfb->edit_baton = eb;\n+\t*file_baton = fb;\n+\n+\treturn SVN_NO_ERROR;\n+}\n+\n+static svn_error_t *open_file(const char *path,\n+\t\t\t void *parent_baton,\n+\t\t\t svn_revnum_t base_revision,\n+\t\t\t apr_pool_t *pool,\n+\t\t\t void **file_baton)\n+{\n+\tstruct dir_baton *pb = parent_baton;\n+\tstruct edit_baton *eb = pb->edit_baton;\n+\tstruct file_baton *fb = apr_palloc(pool, sizeof(*fb));\n+\n+\tSVN_ERR(write_indent(eb, pool));\n+\tSVN_ERR(svn_stream_printf(eb->out, pool, \"open_file : '%s':%ld\\n\",\n+\t\t\t\t path, base_revision));\n+\n+\teb->indent_level++;\n+\n+\tSVN_ERR(eb->wrapped_editor->open_file(path,\n+\t\t\t\t\t pb->wrapped_dir_baton,\n+\t\t\t\t\t base_revision,\n+\t\t\t\t\t pool,\n+\t\t\t\t\t &fb->wrapped_file_baton));\n+\n+\tfb->edit_baton = eb;\n+\t*file_baton = fb;\n+\n+\treturn SVN_NO_ERROR;\n+}\n+\n+static svn_error_t *apply_textdelta(void *file_baton,\n+\t\t\t\t const char *base_checksum,\n+\t\t\t\t apr_pool_t *pool,\n+\t\t\t\t svn_txdelta_window_handler_t *handler,\n+\t\t\t\t void **handler_baton)\n+{\n+\tstruct file_baton *fb = file_baton;\n+\tstruct edit_baton *eb = fb->edit_baton;\n+\n+\tSVN_ERR(write_indent(eb, pool));\n+\tSVN_ERR(svn_stream_printf(eb->out, pool, \"apply_textdelta : %s\\n\",\n+\t\t\t\t base_checksum));\n+\n+\tSVN_ERR(eb->wrapped_editor->apply_textdelta(fb->wrapped_file_baton,\n+\t\t\t\t\t\t base_checksum,\n+\t\t\t\t\t\t pool,\n+\t\t\t\t\t\t handler,\n+\t\t\t\t\t\t handler_baton));\n+\n+\treturn SVN_NO_ERROR;\n+}\n+\n+static svn_error_t *close_file(void *file_baton,\n+\t\t\t const char *text_checksum,\n+\t\t\t apr_pool_t *pool)\n+{\n+\tstruct file_baton *fb = file_baton;\n+\tstruct edit_baton *eb = fb->edit_baton;\n+\n+\teb->indent_level--;\n+\n+\tSVN_ERR(write_indent(eb, pool));\n+\tSVN_ERR(svn_stream_printf(eb->out, pool, \"close_file : %s\\n\",\n+\t\t\t\t text_checksum));\n+\n+\tSVN_ERR(eb->wrapped_editor->close_file(fb->wrapped_file_baton,\n+\t\t\t\t\t text_checksum, pool));\n+\n+\treturn SVN_NO_ERROR;\n+}\n+\n+static svn_error_t *absent_file(const char *path,\n+\t\t\t\tvoid *file_baton,\n+\t\t\t\tapr_pool_t *pool)\n+{\n+\tstruct file_baton *fb = file_baton;\n+\tstruct edit_baton *eb = fb->edit_baton;\n+\n+\tSVN_ERR(write_indent(eb, pool));\n+\tSVN_ERR(svn_stream_printf(eb->out, pool, \"absent_file : %s\\n\", path));\n+\n+\tSVN_ERR(eb->wrapped_editor->absent_file(path, fb->wrapped_file_baton,\n+\t\t\t\t\t\tpool));\n+\n+\treturn SVN_NO_ERROR;\n+}\n+\n+static svn_error_t *close_directory(void *dir_baton,\n+\t\t\t\t apr_pool_t *pool)\n+{\n+\tstruct dir_baton *db = dir_baton;\n+\tstruct edit_baton *eb = db->edit_baton;\n+\n+\teb->indent_level--;\n+\tSVN_ERR(write_indent(eb, pool));\n+\tSVN_ERR(svn_stream_printf(eb->out, pool, \"close_directory\\n\"));\n+\n+\tSVN_ERR(eb->wrapped_editor->close_directory(db->wrapped_dir_baton,\n+\t\t\t\t\t\t pool));\n+\n+\treturn SVN_NO_ERROR;\n+}\n+\n+static svn_error_t *absent_directory(const char *path,\n+\t\t\t\t void *dir_baton,\n+\t\t\t\t apr_pool_t *pool)\n+{\n+\tstruct dir_baton *db = dir_baton;\n+\tstruct edit_baton *eb = db->edit_baton;\n+\n+\tSVN_ERR(write_indent(eb, pool));\n+\tSVN_ERR(svn_stream_printf(eb->out, pool, \"absent_directory : %s\\n\",\n+\t\t\t\t path));\n+\n+\tSVN_ERR(eb->wrapped_editor->absent_directory(path, db->wrapped_dir_baton,\n+\t\t\t\t\t\t pool));\n+\n+\treturn SVN_NO_ERROR;\n+}\n+\n+static svn_error_t *change_file_prop(void *file_baton,\n+\t\t\t\t const char *name,\n+\t\t\t\t const svn_string_t *value,\n+\t\t\t\t apr_pool_t *pool)\n+{\n+\tstruct file_baton *fb = file_baton;\n+\tstruct edit_baton *eb = fb->edit_baton;\n+\n+\tSVN_ERR(write_indent(eb, pool));\n+\tSVN_ERR(svn_stream_printf(eb->out, pool, \"change_file_prop : %s\\n\",\n+\t\t\t\t name));\n+\n+\tSVN_ERR(eb->wrapped_editor->change_file_prop(fb->wrapped_file_baton,\n+\t\t\t\t\t\t name,\n+\t\t\t\t\t\t value,\n+\t\t\t\t\t\t pool));\n+\n+\treturn SVN_NO_ERROR;\n+}\n+\n+static svn_error_t *change_dir_prop(void *dir_baton,\n+\t\t\t\t const char *name,\n+\t\t\t\t const svn_string_t *value,\n+\t\t\t\t apr_pool_t *pool)\n+{\n+\tstruct dir_baton *db = dir_baton;\n+\tstruct edit_baton *eb = db->edit_baton;\n+\n+\tSVN_ERR(write_indent(eb, pool));\n+\tSVN_ERR(svn_stream_printf(eb->out, pool, \"change_dir_prop : %s\\n\", name));\n+\n+\tSVN_ERR(eb->wrapped_editor->change_dir_prop(db->wrapped_dir_baton,\n+\t\t\t\t\t\t name,\n+\t\t\t\t\t\t value,\n+\t\t\t\t\t\t pool));\n+\n+\treturn SVN_NO_ERROR;\n+}\n+\n+static svn_error_t *close_edit(void *edit_baton,\n+\t\t\t apr_pool_t *pool)\n+{\n+\tstruct edit_baton *eb = edit_baton;\n+\n+\tSVN_ERR(write_indent(eb, pool));\n+\tSVN_ERR(svn_stream_printf(eb->out, pool, \"close_edit\\n\"));\n+\n+\tSVN_ERR(eb->wrapped_editor->close_edit(eb->wrapped_edit_baton, pool));\n+\n+\treturn SVN_NO_ERROR;\n+}\n+\n+svn_error_t *svn_delta__get_debug_editor(const svn_delta_editor_t **editor,\n+\t\t\t\t\t void **edit_baton,\n+\t\t\t\t\t const svn_delta_editor_t *wrapped_editor,\n+\t\t\t\t\t void *wrapped_edit_baton,\n+\t\t\t\t\t apr_pool_t *pool)\n+{\n+\tsvn_delta_editor_t *tree_editor = svn_delta_default_editor(pool);\n+\tstruct edit_baton *eb = apr_palloc(pool, sizeof(*eb));\n+\tapr_file_t *errfp;\n+\tsvn_stream_t *out;\n+\n+\tapr_status_t apr_err = apr_file_open_stderr(&errfp, pool);\n+\tif (apr_err)\n+\t\treturn svn_error_wrap_apr(apr_err, \"Problem opening stderr\");\n+\n+\tout = svn_stream_from_aprfile2(errfp, TRUE, pool);\n+\n+\ttree_editor->set_target_revision = set_target_revision;\n+\ttree_editor->open_root = open_root;\n+\ttree_editor->delete_entry = delete_entry;\n+\ttree_editor->add_directory = add_directory;\n+\ttree_editor->open_directory = open_directory;\n+\ttree_editor->change_dir_prop = change_dir_prop;\n+\ttree_editor->close_directory = close_directory;\n+\ttree_editor->absent_directory = absent_directory;\n+\ttree_editor->add_file = add_file;\n+\ttree_editor->open_file = open_file;\n+\ttree_editor->apply_textdelta = apply_textdelta;\n+\ttree_editor->change_file_prop = change_file_prop;\n+\ttree_editor->close_file = close_file;\n+\ttree_editor->absent_file = absent_file;\n+\ttree_editor->close_edit = close_edit;\n+\n+\teb->wrapped_editor = wrapped_editor;\n+\teb->wrapped_edit_baton = wrapped_edit_baton;\n+\teb->out = out;\n+\teb->indent_level = 0;\n+\n+\t*editor = tree_editor;\n+\t*edit_baton = eb;\n+\n+\treturn SVN_NO_ERROR;\n+}\ndiff --git a/debug_editor.h b/debug_editor.h\nnew file mode 100644\nindex 0000000..a0d412a\n--- /dev/null\n+++ b/debug_editor.h\n@@ -0,0 +1,10 @@\n+#ifndef DEBUG_EDITOR_H_\n+#define DEBUG_EDITOR_H_\n+\n+svn_error_t *svn_delta__get_debug_editor(const svn_delta_editor_t **editor,\n+\t\t\t\t\t void **edit_baton,\n+\t\t\t\t\t const svn_delta_editor_t *wrapped_editor,\n+\t\t\t\t\t void *wrapped_edit_baton,\n+\t\t\t\t\t apr_pool_t *pool);\n+\n+#endif\n-- \n1.7.1\n"},{"id":"144986","messageId":"1278461693-3828-5-git-send-email-artagnon@gmail.com","threadId":"24313","inReplyTo":"1278461693-3828-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 04/13] Add skeleton dump editor","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-07-07T00:14:44Z","receivedAt":"2010-07-07T00:14:44Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Add a dump editor and write out skeleton callback functions according\nto the API documentation of svn_delta_editor_t. Also expose\nget_dump_editor through a header.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n Makefile | 4 +-\n dump_editor.c | 143 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n dump_editor.h | 8 +++\n dumpr_util.h | 29 ++++++++++++\n 4 files changed, 182 insertions(+), 2 deletions(-)\n create mode 100644 dump_editor.c\n create mode 100644 dump_editor.h\n create mode 100644 dumpr_util.h\n\ndiff --git a/Makefile b/Makefile\nindex e4d106e..fea646e 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1,8 +1,8 @@\n svndumpr: *.c *.h\n-\t$(CC) -Wall -Werror -DAPR_POOL_DEBUG -ggdb3 -O0 -o $@ svndumpr.c debug_editor.c -lsvn_client-1 -I. -I/usr/local/include/subversion-1 -I/usr/include/apr-1.0\n+\t$(CC) -Wall -Werror -DAPR_POOL_DEBUG -ggdb3 -O0 -o $@ svndumpr.c debug_editor.c dump_editor.c -lsvn_client-1 -I. -I/usr/local/include/subversion-1 -I/usr/include/apr-1.0\n \n svndumpr_bench: *.c *.h\n-\t$(CC) -O2 -o $@ svndumpr.c debug_editor.c -lsvn_client-1 -I. -I/usr/local/include/subversion-1 -I/usr/include/apr-1.0\n+\t$(CC) -O2 -o $@ svndumpr.c debug_editor.c dump_editor.c -lsvn_client-1 -I. -I/usr/local/include/subversion-1 -I/usr/include/apr-1.0\n \n clean:\n \t$(RM) svndumpr svndumpr_bench\ndiff --git a/dump_editor.c b/dump_editor.c\nnew file mode 100644\nindex 0000000..2fdf93c\n--- /dev/null\n+++ b/dump_editor.c\n@@ -0,0 +1,143 @@\n+/* Licensed under a two-clause BSD-style license.\n+ * See LICENSE for details.\n+ */\n+\n+#include \"svn_pools.h\"\n+#include \"svn_error.h\"\n+#include \"svn_iter.h\"\n+#include \"svn_repos.h\"\n+#include \"svn_string.h\"\n+#include \"svn_dirent_uri.h\"\n+#include \"svn_path.h\"\n+#include \"svn_time.h\"\n+#include \"svn_checksum.h\"\n+#include \"svn_props.h\"\n+\n+#include \"dumpr_util.h\"\n+\n+svn_error_t *open_root(void *edit_baton,\n+ svn_revnum_t base_revision,\n+ apr_pool_t *pool,\n+ void **root_baton)\n+{\n+\treturn SVN_NO_ERROR;\n+}\n+\n+svn_error_t *delete_entry(const char *path,\n+ svn_revnum_t revision,\n+ void *parent_baton,\n+ apr_pool_t *pool)\n+{\n+\treturn SVN_NO_ERROR;\n+}\n+\n+svn_error_t *add_directory(const char *path,\n+ void *parent_baton,\n+ const char *copyfrom_path,\n+ svn_revnum_t copyfrom_rev,\n+ apr_pool_t *pool,\n+ void **child_baton)\n+{\n+\treturn SVN_NO_ERROR;\n+}\n+\n+svn_error_t *open_directory(const char *path,\n+ void *parent_baton,\n+ svn_revnum_t base_revision,\n+ apr_pool_t *pool,\n+ void **child_baton)\n+{\n+\treturn SVN_NO_ERROR;\n+}\n+\n+svn_error_t *close_directory(void *dir_baton,\n+ apr_pool_t *pool)\n+{\n+\treturn SVN_NO_ERROR;\n+}\n+\n+svn_error_t *add_file(const char *path,\n+ void *parent_baton,\n+ const char *copyfrom_path,\n+ svn_revnum_t copyfrom_rev,\n+ apr_pool_t *pool,\n+ void **file_baton)\n+{\n+\treturn SVN_NO_ERROR;\n+}\n+\n+svn_error_t *open_file(const char *path,\n+ void *parent_baton,\n+ svn_revnum_t ancestor_revision,\n+ apr_pool_t *pool,\n+ void **file_baton)\n+{\n+\treturn SVN_NO_ERROR;\n+}\n+\n+svn_error_t *change_dir_prop(void *parent_baton,\n+ const char *name,\n+ const svn_string_t *value,\n+ apr_pool_t *pool)\n+{\n+\treturn SVN_NO_ERROR;\n+}\n+\n+svn_error_t *change_file_prop(void *file_baton,\n+ const char *name,\n+ const svn_string_t *value,\n+ apr_pool_t *pool)\n+{\n+\treturn SVN_NO_ERROR;\n+}\n+\n+svn_error_t *apply_textdelta(void *file_baton, const char *base_checksum,\n+ apr_pool_t *pool,\n+ svn_txdelta_window_handler_t *handler,\n+ void **handler_baton)\n+{\n+\treturn SVN_NO_ERROR;\n+}\n+\n+svn_error_t *close_file(void *file_baton,\n+\t\t\tconst char *text_checksum,\n+\t\t\tapr_pool_t *pool)\n+{\n+\treturn SVN_NO_ERROR;\n+}\n+\n+svn_error_t *close_edit(void *edit_baton, apr_pool_t *pool)\n+{\n+\treturn SVN_NO_ERROR;\n+}\n+\n+svn_error_t *get_dump_editor(const svn_delta_editor_t **editor,\n+ void **edit_baton,\n+ svn_revnum_t from_rev,\n+ apr_pool_t *pool)\n+{\n+\tstruct edit_baton *eb = apr_pcalloc(pool, sizeof(struct edit_baton));\n+\teb->current_rev = from_rev;\n+\tSVN_ERR(svn_stream_for_stdout(&(eb->stream), pool));\n+\tsvn_delta_editor_t *de = svn_delta_default_editor(pool);\n+\t\n+\tde->open_root = open_root;\n+\tde->delete_entry = delete_entry;\n+\tde->add_directory = add_directory;\n+\tde->open_directory = open_directory;\n+\tde->close_directory = close_directory;\n+\tde->change_dir_prop = change_dir_prop;\n+\tde->change_file_prop = change_file_prop;\n+\tde->apply_textdelta = apply_textdelta;\n+\tde->add_file = add_file;\n+\tde->open_file = open_file;\n+\tde->close_file = close_file;\n+\tde->close_edit = close_edit;\n+\n+\t/* Set the edit_baton and editor */\n+\t*edit_baton = eb;\n+\t*editor = de;\n+\n+\treturn SVN_NO_ERROR;\n+}\n+ \ndiff --git a/dump_editor.h b/dump_editor.h\nnew file mode 100644\nindex 0000000..9c70b74\n--- /dev/null\n+++ b/dump_editor.h\n@@ -0,0 +1,8 @@\n+#ifndef DUMP_EDITOR_H_\n+#define DUMP_EDITOR_H_\n+\n+svn_error_t *get_dump_editor(const svn_delta_editor_t **editor,\n+\t\t\t void **edit_baton,\n+\t\t\t svn_revnum_t to_rev,\n+\t\t\t apr_pool_t *pool);\n+#endif\ndiff --git a/dumpr_util.h b/dumpr_util.h\nnew file mode 100644\nindex 0000000..d206c19\n--- /dev/null\n+++ b/dumpr_util.h\n@@ -0,0 +1,29 @@\n+#ifndef DUMPR_UTIL_H_\n+#define DUMPR_UTIL_H_\n+\n+struct edit_baton {\n+\t/* The stream to dump to: stdout */\n+\tsvn_stream_t *stream;\n+\n+\t/* pool is for per-edit-session allocations */\n+\tapr_pool_t *pool;\n+\n+\tsvn_revnum_t current_rev;\n+\t\n+\t/* Store the properties that changed */\n+\tapr_hash_t *properties;\n+\tapr_hash_t *del_properties; /* Value is always 0x1 */\n+\tsvn_stringbuf_t *propstring;\n+\n+\t/* Path of changed file */\n+\tconst char *changed_path;\n+\n+\t/* Was a copy command issued? */\n+\tsvn_boolean_t is_copy;\n+\n+\t/* Temporary file to write delta to along with its checksum */\n+\tchar *temp_filepath;\n+\tsvn_checksum_t *checksum;\n+};\n+\n+#endif\n-- \n1.7.1\n"},{"id":"144981","messageId":"1278461693-3828-6-git-send-email-artagnon@gmail.com","threadId":"24313","inReplyTo":"1278461693-3828-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 05/13] Drive the debug editor","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-07-07T00:14:45Z","receivedAt":"2010-07-07T00:14:45Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Wrap the dummy dump editor in the debug editor and drive the debug\neditor to print out all the actions that occur during the replay to\nstderr.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n dump_editor.c | 2 +-\n dumpr_util.h | 5 +++++\n svndumpr.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 55 insertions(+), 1 deletions(-)\n\ndiff --git a/dump_editor.c b/dump_editor.c\nindex 2fdf93c..70d6c0b 100644\n--- a/dump_editor.c\n+++ b/dump_editor.c\n@@ -128,7 +128,7 @@ svn_error_t *get_dump_editor(const svn_delta_editor_t **editor,\n \tde->close_directory = close_directory;\n \tde->change_dir_prop = change_dir_prop;\n \tde->change_file_prop = change_file_prop;\n-\tde->apply_textdelta = apply_textdelta;\n+\t/* de->apply_textdelta = apply_textdelta; */\n \tde->add_file = add_file;\n \tde->open_file = open_file;\n \tde->close_file = close_file;\ndiff --git a/dumpr_util.h b/dumpr_util.h\nindex d206c19..166e214 100644\n--- a/dumpr_util.h\n+++ b/dumpr_util.h\n@@ -1,6 +1,11 @@\n #ifndef DUMPR_UTIL_H_\n #define DUMPR_UTIL_H_\n \n+struct replay_baton {\n+\tconst svn_delta_editor_t *editor;\n+\tvoid *baton;\n+};\n+\n struct edit_baton {\n \t/* The stream to dump to: stdout */\n \tsvn_stream_t *stream;\ndiff --git a/svndumpr.c b/svndumpr.c\nindex 737c4aa..853facd 100644\n--- a/svndumpr.c\n+++ b/svndumpr.c\n@@ -8,10 +8,40 @@\n #include \"svn_ra.h\"\n #include \"svn_repos.h\"\n \n+#include \"debug_editor.h\"\n+#include \"dump_editor.h\"\n+#include \"dumpr_util.h\"\n+\n static apr_pool_t *pool = NULL;\n static svn_client_ctx_t *ctx = NULL;\n static svn_ra_session_t *session = NULL;\n \n+static svn_error_t *replay_revstart(svn_revnum_t revision,\n+ void *replay_baton,\n+ const svn_delta_editor_t **editor,\n+ void **edit_baton,\n+ apr_hash_t *rev_props,\n+ apr_pool_t *pool)\n+{\n+\t/* Extract editor and editor_baton from the replay_baton and\n+\t set them so that the editor callbacks can use them */\n+\tstruct replay_baton *rb = replay_baton;\n+\t*editor = rb->editor;\n+\t*edit_baton = rb->baton;\n+\n+\treturn SVN_NO_ERROR;\n+}\n+\n+static svn_error_t *replay_revend(svn_revnum_t revision,\n+ void *replay_baton,\n+ const svn_delta_editor_t *editor,\n+ void *edit_baton,\n+ apr_hash_t *rev_props,\n+ apr_pool_t *pool)\n+{\n+\treturn SVN_NO_ERROR;\n+}\n+\n svn_error_t *populate_context()\n {\n \tconst char *http_library;\n@@ -47,6 +77,25 @@ svn_error_t *open_connection(const char *url)\n \n svn_error_t *replay_range(svn_revnum_t start_revision, svn_revnum_t end_revision)\n {\n+\tconst svn_delta_editor_t *dump_editor, *debug_editor;\n+\tvoid *debug_baton, *dump_baton;\n+\n+\tSVN_ERR(get_dump_editor(&dump_editor,\n+\t &dump_baton, start_revision, pool));\n+\n+\tSVN_ERR(svn_delta__get_debug_editor(&debug_editor,\n+\t &debug_baton,\n+\t dump_editor,\n+\t dump_baton, pool));\n+\n+\tstruct replay_baton *replay_baton = apr_palloc(pool, sizeof(struct replay_baton));\n+\treplay_baton->editor = debug_editor;\n+\treplay_baton->baton = debug_baton;\n+\tSVN_ERR(svn_cmdline_printf(pool, SVN_REPOS_DUMPFILE_MAGIC_HEADER \": %d\\n\",\n+\t\t\t\t SVN_REPOS_DUMPFILE_FORMAT_VERSION));\n+\tSVN_ERR(svn_ra_replay_range(session, start_revision, end_revision,\n+\t 0, TRUE, replay_revstart, replay_revend,\n+\t replay_baton, pool));\n \treturn SVN_NO_ERROR;\n }\n \n-- \n1.7.1\n"},{"id":"144979","messageId":"1278461693-3828-7-git-send-email-artagnon@gmail.com","threadId":"24313","inReplyTo":"1278461693-3828-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 06/13] Dump the revprops at the start of every revision","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-07-07T00:14:46Z","receivedAt":"2010-07-07T00:14:46Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Fill in replay_revstart to dump the revprops at the start of every\nrevision. Add an additional write_hash_to_stringbuf helper function.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n Makefile | 4 +-\n dumpr_util.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n dumpr_util.h | 5 ++++\n svndumpr.c | 38 ++++++++++++++++++++++++++++++++-\n 4 files changed, 107 insertions(+), 4 deletions(-)\n create mode 100644 dumpr_util.c\n\ndiff --git a/Makefile b/Makefile\nindex fea646e..c0b5c8a 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1,8 +1,8 @@\n svndumpr: *.c *.h\n-\t$(CC) -Wall -Werror -DAPR_POOL_DEBUG -ggdb3 -O0 -o $@ svndumpr.c debug_editor.c dump_editor.c -lsvn_client-1 -I. -I/usr/local/include/subversion-1 -I/usr/include/apr-1.0\n+\t$(CC) -Wall -Werror -DAPR_POOL_DEBUG -ggdb3 -O0 -o $@ svndumpr.c debug_editor.c dump_editor.c dumpr_util.c -lsvn_client-1 -I. -I/usr/local/include/subversion-1 -I/usr/include/apr-1.0\n \n svndumpr_bench: *.c *.h\n-\t$(CC) -O2 -o $@ svndumpr.c debug_editor.c dump_editor.c -lsvn_client-1 -I. -I/usr/local/include/subversion-1 -I/usr/include/apr-1.0\n+\t$(CC) -O2 -o $@ svndumpr.c debug_editor.c dump_editor.c dumpr_util.c -lsvn_client-1 -I. -I/usr/local/include/subversion-1 -I/usr/include/apr-1.0\n \n clean:\n \t$(RM) svndumpr svndumpr_bench\ndiff --git a/dumpr_util.c b/dumpr_util.c\nnew file mode 100644\nindex 0000000..41940d4\n--- /dev/null\n+++ b/dumpr_util.c\n@@ -0,0 +1,64 @@\n+/* Licensed under a two-clause BSD-style license.\n+ * See LICENSE for details.\n+ */\n+\n+#include \"svn_pools.h\"\n+#include \"svn_cmdline.h\"\n+#include \"svn_client.h\"\n+#include \"svn_ra.h\"\n+#include \"svn_repos.h\"\n+\n+#include \"dumpr_util.h\"\n+\n+void write_hash_to_stringbuf(apr_hash_t *properties,\n+\t\t\t svn_boolean_t deleted,\n+\t\t\t svn_stringbuf_t **strbuf,\n+\t\t\t apr_pool_t *pool)\n+{\n+\tapr_hash_index_t *this;\n+\tconst void *key;\n+\tvoid *val;\n+\tapr_ssize_t keylen;\n+\tsvn_string_t *value;\n+\t\n+\tif (!deleted) {\n+\t\tfor (this = apr_hash_first(pool, properties); this;\n+\t\t this = apr_hash_next(this)) {\n+\t\t\t/* Get this key and val. */\n+\t\t\tapr_hash_this(this, &key, &keylen, &val);\n+\t\t\tvalue = val;\n+\n+\t\t\t/* Output name length, then name. */\n+\t\t\tsvn_stringbuf_appendcstr(*strbuf,\n+\t\t\t\t\t\t apr_psprintf(pool, \"K %\" APR_SSIZE_T_FMT \"\\n\",\n+\t\t\t\t\t\t\t keylen));\n+\n+\t\t\tsvn_stringbuf_appendbytes(*strbuf, (const char *) key, keylen);\n+\t\t\tsvn_stringbuf_appendbytes(*strbuf, \"\\n\", 1);\n+\n+\t\t\t/* Output value length, then value. */\n+\t\t\tsvn_stringbuf_appendcstr(*strbuf,\n+\t\t\t\t\t\t apr_psprintf(pool, \"V %\" APR_SIZE_T_FMT \"\\n\",\n+\t\t\t\t\t\t\t value->len));\n+\n+\t\t\tsvn_stringbuf_appendbytes(*strbuf, value->data, value->len);\n+\t\t\tsvn_stringbuf_appendbytes(*strbuf, \"\\n\", 1);\n+\t\t}\n+\t}\n+\telse {\n+\t\t/* Output a \"D \" entry for each deleted property */\n+\t\tfor (this = apr_hash_first(pool, properties); this;\n+\t\t this = apr_hash_next(this)) {\n+\t\t\t/* Get this key */\n+\t\t\tapr_hash_this(this, &key, &keylen, NULL);\n+\n+\t\t\t/* Output name length, then name */\n+\t\t\tsvn_stringbuf_appendcstr(*strbuf,\n+\t\t\t\t\t\t apr_psprintf(pool, \"D %\" APR_SSIZE_T_FMT \"\\n\",\n+\t\t\t\t\t\t\t keylen));\n+\n+\t\t\tsvn_stringbuf_appendbytes(*strbuf, (const char *) key, keylen);\n+\t\t\tsvn_stringbuf_appendbytes(*strbuf, \"\\n\", 1);\n+\t\t}\n+\t}\n+}\ndiff --git a/dumpr_util.h b/dumpr_util.h\nindex 166e214..1a5752b 100644\n--- a/dumpr_util.h\n+++ b/dumpr_util.h\n@@ -31,4 +31,9 @@ struct edit_baton {\n \tsvn_checksum_t *checksum;\n };\n \n+void write_hash_to_stringbuf(apr_hash_t *properties,\n+\t\t\t svn_boolean_t deleted,\n+\t\t\t svn_stringbuf_t **strbuf,\n+\t\t\t apr_pool_t *pool);\n+\n #endif\ndiff --git a/svndumpr.c b/svndumpr.c\nindex 853facd..011941f 100644\n--- a/svndumpr.c\n+++ b/svndumpr.c\n@@ -23,6 +23,37 @@ static svn_error_t *replay_revstart(svn_revnum_t revision,\n apr_hash_t *rev_props,\n apr_pool_t *pool)\n {\n+\t/* Editing this revision has just started; dump the revprops\n+\t before invoking the editor callbacks */\n+\tsvn_stringbuf_t *propstring = svn_stringbuf_create(\"\", pool);\n+\tsvn_stream_t *stdout_stream;\n+\n+\t/* Create an stdout stream */\n+\tsvn_stream_for_stdout(&stdout_stream, pool);\n+\n+ /* Print revision number and prepare the propstring */\n+\tSVN_ERR(svn_stream_printf(stdout_stream, pool,\n+\t\t\t\t SVN_REPOS_DUMPFILE_REVISION_NUMBER\n+\t\t\t\t \": %ld\\n\", revision));\n+\twrite_hash_to_stringbuf(rev_props, FALSE, &propstring, pool);\n+\tsvn_stringbuf_appendbytes(propstring, \"PROPS-END\\n\", 10);\n+\n+\t/* prop-content-length header */\n+\tSVN_ERR(svn_stream_printf(stdout_stream, pool,\n+\t\t\t\t SVN_REPOS_DUMPFILE_PROP_CONTENT_LENGTH\n+\t\t\t\t \": %\" APR_SIZE_T_FMT \"\\n\", propstring->len));\n+\n+\t/* content-length header */\n+\tSVN_ERR(svn_stream_printf(stdout_stream, pool,\n+\t\t\t\t SVN_REPOS_DUMPFILE_CONTENT_LENGTH\n+\t\t\t\t \": %\" APR_SIZE_T_FMT \"\\n\\n\", propstring->len));\n+\n+\t/* Print the revprops now */\n+\tSVN_ERR(svn_stream_write(stdout_stream, propstring->data,\n+\t\t\t\t &(propstring->len)));\n+\n+\tsvn_stream_close(stdout_stream);\n+\n \t/* Extract editor and editor_baton from the replay_baton and\n \t set them so that the editor callbacks can use them */\n \tstruct replay_baton *rb = replay_baton;\n@@ -39,6 +70,9 @@ static svn_error_t *replay_revend(svn_revnum_t revision,\n apr_hash_t *rev_props,\n apr_pool_t *pool)\n {\n+\t/* Editor has finished for this revision and close_edit has\n+\t been called; do nothing: just continue to the next\n+\t revision */\n \treturn SVN_NO_ERROR;\n }\n \n@@ -89,8 +123,8 @@ svn_error_t *replay_range(svn_revnum_t start_revision, svn_revnum_t end_revision\n \t dump_baton, pool));\n \n \tstruct replay_baton *replay_baton = apr_palloc(pool, sizeof(struct replay_baton));\n-\treplay_baton->editor = debug_editor;\n-\treplay_baton->baton = debug_baton;\n+\treplay_baton->editor = dump_editor;\n+\treplay_baton->baton = dump_baton;\n \tSVN_ERR(svn_cmdline_printf(pool, SVN_REPOS_DUMPFILE_MAGIC_HEADER \": %d\\n\",\n \t\t\t\t SVN_REPOS_DUMPFILE_FORMAT_VERSION));\n \tSVN_ERR(svn_ra_replay_range(session, start_revision, end_revision,\n-- \n1.7.1\n"},{"id":"144976","messageId":"1278461693-3828-8-git-send-email-artagnon@gmail.com","threadId":"24313","inReplyTo":"1278461693-3828-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 07/13] Implement open_root and close_edit","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-07-07T00:14:47Z","receivedAt":"2010-07-07T00:14:47Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"open_root first creates a special edit_baton pool, and then allocates\nmemory from that pool to various items in edit_baton. Then it creates\na new directory baton to set as the root_baton. close_edit destroys\nthe special edit_baton pool (hence destroying all the items that were\nallocated there), and increments current_rev. Add related\nmake_dir_baton function and dir_baton_t structure.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n dump_editor.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n dumpr_util.h | 30 ++++++++++++++++++++++++\n 2 files changed, 100 insertions(+), 0 deletions(-)\n\ndiff --git a/dump_editor.c b/dump_editor.c\nindex 70d6c0b..6e9b0f5 100644\n--- a/dump_editor.c\n+++ b/dump_editor.c\n@@ -15,11 +15,77 @@\n \n #include \"dumpr_util.h\"\n \n+\n+/* Make a directory baton to represent the directory was path\n+ (relative to EDIT_BATON's path) is PATH.\n+\n+ CMP_PATH/CMP_REV are the path/revision against which this directory\n+ should be compared for changes. If either is omitted (NULL for the\n+ path, SVN_INVALID_REVNUM for the rev), just compare this directory\n+ PATH against itself in the previous revision.\n+\n+ PARENT_DIR_BATON is the directory baton of this directory's parent,\n+ or NULL if this is the top-level directory of the edit. ADDED\n+ indicated if this directory is newly added in this revision.\n+ Perform all allocations in POOL. */\n+struct dir_baton *make_dir_baton(const char *path,\n+ const char *cmp_path,\n+ svn_revnum_t cmp_rev,\n+ void *edit_baton,\n+ void *parent_dir_baton,\n+ svn_boolean_t added,\n+ apr_pool_t *pool)\n+{\n+\tstruct edit_baton *eb = edit_baton;\n+\tstruct dir_baton *pb = parent_dir_baton;\n+\tstruct dir_baton *new_db = apr_pcalloc(pool, sizeof(*new_db));\n+\tconst char *full_path;\n+\tapr_array_header_t *compose_path = apr_array_make(pool, 2, sizeof(const char *));\n+\n+\t/* A path relative to nothing? I don't think so. */\n+\tSVN_ERR_ASSERT_NO_RETURN(!path || pb);\n+\n+\t/* Construct the full path of this node. */\n+\tif (pb) {\n+\t\tAPR_ARRAY_PUSH(compose_path, const char *) = \"/\";\n+\t\tAPR_ARRAY_PUSH(compose_path, const char *) = path;\n+\t\tfull_path = svn_path_compose(compose_path, pool);\n+\t}\n+\telse\n+\t\tfull_path = apr_pstrdup(pool, \"/\");\n+\n+\t/* Remove leading slashes from copyfrom paths. */\n+\tif (cmp_path)\n+\t\tcmp_path = ((*cmp_path == '/') ? cmp_path + 1 : cmp_path);\n+\n+\tnew_db->eb = eb;\n+\tnew_db->parent_dir_baton = pb;\n+\tnew_db->path = full_path;\n+\tnew_db->cmp_path = cmp_path ? apr_pstrdup(pool, cmp_path) : NULL;\n+\tnew_db->cmp_rev = cmp_rev;\n+\tnew_db->added = added;\n+\tnew_db->written_out = FALSE;\n+\tnew_db->deleted_entries = apr_hash_make(pool);\n+\tnew_db->pool = pool;\n+\n+\treturn new_db;\n+}\n svn_error_t *open_root(void *edit_baton,\n svn_revnum_t base_revision,\n apr_pool_t *pool,\n void **root_baton)\n {\n+\t/* Allocate a special pool for the edit_baton to avoid pool\n+\t lifetime issues */\n+\tstruct edit_baton *eb = edit_baton;\n+\teb->pool = svn_pool_create(pool);\n+\teb->properties = apr_hash_make(eb->pool);\n+\teb->del_properties = apr_hash_make(eb->pool);\n+\teb->propstring = svn_stringbuf_create(\"\", eb->pool);\n+\teb->is_copy = FALSE;\n+\n+\t*root_baton = make_dir_baton(NULL, NULL, SVN_INVALID_REVNUM,\n+\t edit_baton, NULL, FALSE, pool);\n \treturn SVN_NO_ERROR;\n }\n \n@@ -108,6 +174,10 @@ svn_error_t *close_file(void *file_baton,\n \n svn_error_t *close_edit(void *edit_baton, apr_pool_t *pool)\n {\n+\tstruct edit_baton *eb = edit_baton;\n+\tsvn_pool_destroy(eb->pool);\n+\t(eb->current_rev) ++;\n+\n \treturn SVN_NO_ERROR;\n }\n \ndiff --git a/dumpr_util.h b/dumpr_util.h\nindex 1a5752b..96670ff 100644\n--- a/dumpr_util.h\n+++ b/dumpr_util.h\n@@ -31,6 +31,36 @@ struct edit_baton {\n \tsvn_checksum_t *checksum;\n };\n \n+struct dir_baton {\n+\tstruct edit_baton *eb;\n+\tstruct dir_baton *parent_dir_baton;\n+\n+\t/* is this directory a new addition to this revision? */\n+\tsvn_boolean_t added;\n+\n+\t/* has this directory been written to the output stream? */\n+\tsvn_boolean_t written_out;\n+\n+\t/* the absolute path to this directory */\n+\tconst char *path;\n+\n+\t/* the comparison path and revision of this directory. if both of\n+\t these are valid, use them as a source against which to compare\n+\t the directory instead of the default comparison source of PATH in\n+\t the previous revision. */\n+\tconst char *cmp_path;\n+\tsvn_revnum_t cmp_rev;\n+\n+\t/* hash of paths that need to be deleted, though some -might- be\n+\t replaced. maps const char * paths to this dir_baton. (they're\n+\t full paths, because that's what the editor driver gives us. but\n+\t really, they're all within this directory.) */\n+\tapr_hash_t *deleted_entries;\n+\n+\t/* pool to be used for deleting the hash items */\n+\tapr_pool_t *pool;\n+};\n+\n void write_hash_to_stringbuf(apr_hash_t *properties,\n \t\t\t svn_boolean_t deleted,\n \t\t\t svn_stringbuf_t **strbuf,\n-- \n1.7.1\n"},{"id":"144978","messageId":"1278461693-3828-9-git-send-email-artagnon@gmail.com","threadId":"24313","inReplyTo":"1278461693-3828-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 08/13] Implement dump_node","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-07-07T00:14:48Z","receivedAt":"2010-07-07T00:14:48Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Add a new dump_node function to dump node headers Node-path,\nNode-kind, Node-action and set the right variables to trigger dumping\nproperties. Copies are also handled appropriately using eb->is_copy,\ncopyfrom_path, copyfrom_rev informatino from the caller. Also add a\nrelated dump_props helper function that uses write_hash_to_stringbuf\nto dump properties.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n dump_editor.c | 132 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n dumpr_util.c | 48 +++++++++++++++++++++\n dumpr_util.h | 5 ++\n 3 files changed, 185 insertions(+), 0 deletions(-)\n\ndiff --git a/dump_editor.c b/dump_editor.c\nindex 6e9b0f5..0f7d231 100644\n--- a/dump_editor.c\n+++ b/dump_editor.c\n@@ -15,6 +15,10 @@\n \n #include \"dumpr_util.h\"\n \n+#define ARE_VALID_COPY_ARGS(p,r) ((p) && SVN_IS_VALID_REVNUM(r))\n+\n+svn_boolean_t must_dump_props = FALSE, must_dump_text = FALSE,\n+\tdump_props_pending = FALSE;\n \n /* Make a directory baton to represent the directory was path\n (relative to EDIT_BATON's path) is PATH.\n@@ -70,6 +74,134 @@ struct dir_baton *make_dir_baton(const char *path,\n \n \treturn new_db;\n }\n+/*\n+ * Write out a node record for PATH of type KIND under EB->FS_ROOT.\n+ * ACTION describes what is happening to the node (see enum svn_node_action).\n+ * Write record to writable EB->STREAM, using EB->BUFFER to write in chunks.\n+ *\n+ * If the node was itself copied, IS_COPY is TRUE and the\n+ * path/revision of the copy source are in CMP_PATH/CMP_REV. If\n+ * IS_COPY is FALSE, yet CMP_PATH/CMP_REV are valid, this node is part\n+ * of a copied subtree.\n+ */\n+svn_error_t *dump_node(struct edit_baton *eb,\n+\t\t const char *path, /* an absolute path. */\n+\t\t svn_node_kind_t kind,\n+\t\t enum svn_node_action action,\n+\t\t const char *cmp_path,\n+\t\t svn_revnum_t cmp_rev,\n+\t\t apr_pool_t *pool)\n+{\n+\t/* Some pending properties to dump? */\n+\tSVN_ERR(dump_props(eb, &dump_props_pending, TRUE, pool));\n+\n+\t/* Write out metadata headers for this file node. */\n+\tSVN_ERR(svn_stream_printf(eb->stream, pool,\n+\t\t\t\t SVN_REPOS_DUMPFILE_NODE_PATH \": %s\\n\",\n+\t\t\t\t (*path == '/') ? path + 1 : path));\n+\n+\tif (kind == svn_node_file)\n+\t\tSVN_ERR(svn_stream_printf(eb->stream, pool,\n+\t\t SVN_REPOS_DUMPFILE_NODE_KIND \": file\\n\"));\n+\telse if (kind == svn_node_dir)\n+\t\tSVN_ERR(svn_stream_printf(eb->stream, pool,\n+\t\t SVN_REPOS_DUMPFILE_NODE_KIND \": dir\\n\"));\n+\n+\t/* Remove leading slashes from copyfrom paths. */\n+\tif (cmp_path)\n+\t\tcmp_path = ((*cmp_path == '/') ? cmp_path + 1 : cmp_path);\n+\n+\tswitch (action) {\n+\t\t/* Appropriately handle the four svn_node_action actions */\n+\n+\tcase svn_node_action_change:\n+\t\tSVN_ERR(svn_stream_printf(eb->stream, pool,\n+\t\t SVN_REPOS_DUMPFILE_NODE_ACTION\n+\t\t \": change\\n\"));\n+\t\tbreak;\n+\n+\tcase svn_node_action_replace:\n+\t\tif (!eb->is_copy) {\n+\t\t\t/* a simple delete+add, implied by a single 'replace' action. */\n+\t\t\tSVN_ERR(svn_stream_printf(eb->stream, pool,\n+\t\t\t SVN_REPOS_DUMPFILE_NODE_ACTION\n+\t\t\t \": replace\\n\"));\n+\n+\t\t\t/* definitely need to dump all content for a replace. */\n+\t\t\tmust_dump_props = TRUE;\n+\t\t\tbreak;\n+\t\t}\n+\t\t/* More complex case: eb->is_copy is true, and\n+\t\t cmp_path/ cmp_rev are present: delete the original,\n+\t\t and then re-add it */\n+\n+\t\t/* the path & kind headers have already been printed; just\n+\t\t add a delete action, and end the current record.*/\n+\t\tSVN_ERR(svn_stream_printf(eb->stream, pool,\n+\t\t SVN_REPOS_DUMPFILE_NODE_ACTION\n+\t\t \": delete\\n\\n\"));\n+\n+\t\t/* recurse: print an additional add-with-history record. */\n+\t\tSVN_ERR(dump_node(eb, path, kind, svn_node_action_add,\n+\t\t cmp_path, cmp_rev, pool));\n+\n+\t\t/* we can leave this routine quietly now, don't need to dump\n+\t\t any content; that was already done in the second record. */\n+\t\tmust_dump_props = FALSE;\n+\t\teb->is_copy = FALSE;\n+\t\tbreak;\n+\n+\tcase svn_node_action_delete:\n+\t\tSVN_ERR(svn_stream_printf(eb->stream, pool,\n+\t\t SVN_REPOS_DUMPFILE_NODE_ACTION\n+\t\t \": delete\\n\"));\n+\n+\t\t/* we can leave this routine quietly now, don't need to dump\n+\t\t any content. */\n+\t\tSVN_ERR(svn_stream_printf(eb->stream, pool, \"\\n\\n\"));\n+\t\tmust_dump_props = FALSE;\n+\t\tbreak;\n+\n+\tcase svn_node_action_add:\n+\t\tSVN_ERR(svn_stream_printf(eb->stream, pool,\n+\t\t SVN_REPOS_DUMPFILE_NODE_ACTION \": add\\n\"));\n+\n+\t\tif (!eb->is_copy) {\n+\t\t\t/* If it's a file or directory not copied from\n+\t\t\t somewhere, wait for change_file_prop or\n+\t\t\t change_directory_prop */\n+\t\t\tdump_props_pending = TRUE;\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\tSVN_ERR(svn_stream_printf(eb->stream, pool,\n+\t\t SVN_REPOS_DUMPFILE_NODE_COPYFROM_REV\n+\t\t \": %ld\\n\"\n+\t\t SVN_REPOS_DUMPFILE_NODE_COPYFROM_PATH\n+\t\t \": %s\\n\",\n+\t\t cmp_rev, cmp_path));\n+\n+\t\t/* Dump the text only if apply_textdelta sets\n+\t\t must_dump_text */\n+\n+\t\t/* UGLY hack: If a directory was copied from a\n+\t\t previous revision, nothing else can be done, and\n+\t\t close_file won't be called to write two blank\n+\t\t lines; write them here */\n+\t\tif (kind == svn_node_dir)\n+\t\t\tSVN_ERR(svn_stream_printf(eb->stream, pool, \"\\n\\n\"));\n+\n+\t\teb->is_copy = FALSE;\n+\n+\t\tbreak;\n+\t}\n+\n+\t/* Dump property headers */\n+\tSVN_ERR(dump_props(eb, &must_dump_props, FALSE, pool));\n+\n+\treturn SVN_NO_ERROR;\n+}\n+\n svn_error_t *open_root(void *edit_baton,\n svn_revnum_t base_revision,\n apr_pool_t *pool,\ndiff --git a/dumpr_util.c b/dumpr_util.c\nindex 41940d4..a3328b6 100644\n--- a/dumpr_util.c\n+++ b/dumpr_util.c\n@@ -62,3 +62,51 @@ void write_hash_to_stringbuf(apr_hash_t *properties,\n \t\t}\n \t}\n }\n+\n+svn_error_t *dump_props(struct edit_baton *eb,\n+\t\t\tsvn_boolean_t *trigger_var,\n+\t\t\tsvn_boolean_t dump_data_too,\n+\t\t\tapr_pool_t *pool)\n+{\n+\tif (trigger_var && !*trigger_var)\n+\t\treturn SVN_NO_ERROR;\n+\n+\t/* Build a propstring to print */\n+\tsvn_stringbuf_setempty(eb->propstring);\n+\twrite_hash_to_stringbuf(eb->properties,\n+\t\t\t\tFALSE,\n+\t\t\t\t&(eb->propstring), eb->pool);\n+\twrite_hash_to_stringbuf(eb->del_properties,\n+\t\t\t\tTRUE,\n+\t\t\t\t&(eb->propstring), eb->pool);\n+\tsvn_stringbuf_appendbytes(eb->propstring, \"PROPS-END\\n\", 10);\n+\n+\t/* prop-delta header */\n+\tSVN_ERR(svn_stream_printf(eb->stream, pool,\n+\t\t\t\t SVN_REPOS_DUMPFILE_PROP_DELTA\n+\t\t\t\t \": true\\n\"));\n+\n+\t/* prop-content-length header */\n+\tSVN_ERR(svn_stream_printf(eb->stream, pool,\n+\t\t\t\t SVN_REPOS_DUMPFILE_PROP_CONTENT_LENGTH\n+\t\t\t\t \": %\" APR_SIZE_T_FMT \"\\n\", eb->propstring->len));\n+\n+\tif (dump_data_too) {\n+\t\t/* content-length header */\n+\t\tSVN_ERR(svn_stream_printf(eb->stream, pool,\n+\t\t\t\t\t SVN_REPOS_DUMPFILE_CONTENT_LENGTH\n+\t\t\t\t\t \": %\" APR_SIZE_T_FMT \"\\n\\n\",\n+\t\t\t\t\t eb->propstring->len));\n+\n+\t\t/* the properties themselves */\n+\t\tSVN_ERR(svn_stream_write(eb->stream, eb->propstring->data,\n+\t\t\t\t\t &(eb->propstring->len)));\n+\n+\t\t/* Cleanup so that data is never dumped twice */\n+\t\tapr_hash_clear(eb->properties);\n+\t\tapr_hash_clear(eb->del_properties);\n+\t\tif (trigger_var)\n+\t\t\t*trigger_var = FALSE;\n+\t}\n+\treturn SVN_NO_ERROR;\n+}\ndiff --git a/dumpr_util.h b/dumpr_util.h\nindex 96670ff..79de1ab 100644\n--- a/dumpr_util.h\n+++ b/dumpr_util.h\n@@ -66,4 +66,9 @@ void write_hash_to_stringbuf(apr_hash_t *properties,\n \t\t\t svn_stringbuf_t **strbuf,\n \t\t\t apr_pool_t *pool);\n \n+svn_error_t *dump_props(struct edit_baton *eb,\n+\t\t\tsvn_boolean_t *trigger_var,\n+\t\t\tsvn_boolean_t dump_data_too,\n+\t\t\tapr_pool_t *pool);\n+\n #endif\n-- \n1.7.1\n"},{"id":"144987","messageId":"1278461693-3828-10-git-send-email-artagnon@gmail.com","threadId":"24313","inReplyTo":"1278461693-3828-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 09/13] Implement directory-related functions","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-07-07T00:14:49Z","receivedAt":"2010-07-07T00:14:49Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Implement open_directory, add_directory, change_dir_prop and\nclose_directory. All of them involve adding and removing entries from\nthe directory_baton and dumping the related node information. Note\nthat open_directory doesn't need any corresponding node information to\nbe printed.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n dump_editor.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 101 insertions(+), 0 deletions(-)\n\ndiff --git a/dump_editor.c b/dump_editor.c\nindex 0f7d231..6077e5c 100644\n--- a/dump_editor.c\n+++ b/dump_editor.c\n@@ -226,6 +226,12 @@ svn_error_t *delete_entry(const char *path,\n void *parent_baton,\n apr_pool_t *pool)\n {\n+\tstruct dir_baton *pb = parent_baton;\n+\tconst char *mypath = apr_pstrdup(pb->pool, path);\n+\n+\t/* remember this path needs to be deleted */\n+\tapr_hash_set(pb->deleted_entries, mypath, APR_HASH_KEY_STRING, pb);\n+\n \treturn SVN_NO_ERROR;\n }\n \n@@ -236,6 +242,32 @@ svn_error_t *add_directory(const char *path,\n apr_pool_t *pool,\n void **child_baton)\n {\n+\tstruct dir_baton *pb = parent_baton;\n+\tvoid *val;\n+\tstruct dir_baton *new_db\n+\t\t= make_dir_baton(path, copyfrom_path, copyfrom_rev, pb->eb, pb, TRUE, pool);\n+\n+\t/* This might be a replacement -- is the path already deleted? */\n+\tval = apr_hash_get(pb->deleted_entries, path, APR_HASH_KEY_STRING);\n+\n+\t/* Detect an add-with-history */\n+\tpb->eb->is_copy = ARE_VALID_COPY_ARGS(copyfrom_path, copyfrom_rev);\n+\n+\t/* Dump the node */\n+\tSVN_ERR(dump_node(pb->eb, path,\n+\t svn_node_dir,\n+\t val ? svn_node_action_replace : svn_node_action_add,\n+\t pb->eb->is_copy ? copyfrom_path : NULL,\n+\t pb->eb->is_copy ? copyfrom_rev : SVN_INVALID_REVNUM,\n+\t pool));\n+\n+\tif (val)\n+\t\t/* Delete the path, it's now been dumped */\n+\t\tapr_hash_set(pb->deleted_entries, path, APR_HASH_KEY_STRING, NULL);\n+\n+\tnew_db->written_out = TRUE;\n+\n+\t*child_baton = new_db;\n \treturn SVN_NO_ERROR;\n }\n \n@@ -245,12 +277,53 @@ svn_error_t *open_directory(const char *path,\n apr_pool_t *pool,\n void **child_baton)\n {\n+\tstruct dir_baton *pb = parent_baton;\n+\tstruct dir_baton *new_db;\n+\tconst char *cmp_path = NULL;\n+\tsvn_revnum_t cmp_rev = SVN_INVALID_REVNUM;\n+\tapr_array_header_t *compose_path = apr_array_make(pool, 2, sizeof(const char *));\n+\n+\t/* If the parent directory has explicit comparison path and rev,\n+\t record the same for this one. */\n+\tif (pb && ARE_VALID_COPY_ARGS(pb->cmp_path, pb->cmp_rev)) {\n+\t\tAPR_ARRAY_PUSH(compose_path, const char *) = pb->cmp_path;\n+\t\tAPR_ARRAY_PUSH(compose_path, const char *) = svn_dirent_basename(path, pool);\n+\t\tcmp_path = svn_path_compose(compose_path, pool);\n+\t\tcmp_rev = pb->cmp_rev;\n+\t}\n+\n+\tnew_db = make_dir_baton(path, cmp_path, cmp_rev, pb->eb, pb, FALSE, pool);\n+\t*child_baton = new_db;\n \treturn SVN_NO_ERROR;\n }\n \n svn_error_t *close_directory(void *dir_baton,\n apr_pool_t *pool)\n {\n+\tstruct dir_baton *db = dir_baton;\n+\tstruct edit_baton *eb = db->eb;\n+\tapr_hash_index_t *hi;\n+\tapr_pool_t *subpool = svn_pool_create(pool);\n+\n+\t/* Some pending properties to dump? */\n+\tSVN_ERR(dump_props(eb, &dump_props_pending, TRUE, pool));\n+\n+\t/* Dump the directory entries */\n+\tfor (hi = apr_hash_first(pool, db->deleted_entries); hi;\n+\t hi = apr_hash_next(hi)) {\n+\t\tconst void *key;\n+\t\tconst char *path;\n+\t\tapr_hash_this(hi, &key, NULL, NULL);\n+\t\tpath = key;\n+\n+\t\tsvn_pool_clear(subpool);\n+\n+\t\tSVN_ERR(dump_node(db->eb, path,\n+\t\t svn_node_unknown, svn_node_action_delete,\n+\t\t NULL, SVN_INVALID_REVNUM, subpool));\n+\t}\n+\n+\tsvn_pool_destroy(subpool);\n \treturn SVN_NO_ERROR;\n }\n \n@@ -278,6 +351,34 @@ svn_error_t *change_dir_prop(void *parent_baton,\n const svn_string_t *value,\n apr_pool_t *pool)\n {\n+\tstruct dir_baton *db = parent_baton;\n+\n+\tif (svn_property_kind(NULL, name) != svn_prop_regular_kind)\n+\t\treturn SVN_NO_ERROR;\n+\n+\tvalue ? apr_hash_set(db->eb->properties, apr_pstrdup(pool, name),\n+\t APR_HASH_KEY_STRING, svn_string_dup(value, pool)) :\n+\t\tapr_hash_set(db->eb->del_properties, apr_pstrdup(pool, name),\n+\t\t APR_HASH_KEY_STRING, (void *)0x1);\n+\n+\t/* This function is what distinguishes between a directory that is\n+\t opened to merely get somewhere, vs. one that is opened because it\n+\t actually changed by itself */\n+\tif (! db->written_out) {\n+\t\tSVN_ERR(dump_node(db->eb, db->path,\n+\t\t svn_node_dir, svn_node_action_change,\n+\t\t db->cmp_path, db->cmp_rev, pool));\n+\n+\t\t/* If dump_props_pending was set, it means that the\n+\t\t node information corresponding to add_directory has already\n+\t\t been written; just don't unset it and dump_node will dump\n+\t\t the properties before doing anything else. If it wasn't\n+\t\t set, node information hasn't been written yet: so dump the\n+\t\t node itself before dumping the props */\n+\n+\t\tSVN_ERR(dump_props(db->eb, NULL, TRUE, pool));\n+\t\tdb->written_out = TRUE;\n+\t}\n \treturn SVN_NO_ERROR;\n }\n \n-- \n1.7.1\n"},{"id":"144982","messageId":"1278461693-3828-11-git-send-email-artagnon@gmail.com","threadId":"24313","inReplyTo":"1278461693-3828-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 10/13] Implement file-related functions","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-07-07T00:14:50Z","receivedAt":"2010-07-07T00:14:50Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Implement add_file, open_file and change_file_prop. All of them\ninvolve dumping the corresponding node information and setting up the\nfile_baton for apply_textdelta and close_file to use.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n dump_editor.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 62 insertions(+), 0 deletions(-)\n\ndiff --git a/dump_editor.c b/dump_editor.c\nindex 6077e5c..7006a2c 100644\n--- a/dump_editor.c\n+++ b/dump_editor.c\n@@ -334,6 +334,31 @@ svn_error_t *add_file(const char *path,\n apr_pool_t *pool,\n void **file_baton)\n {\n+\tstruct dir_baton *pb = parent_baton;\n+\tvoid *val;\n+\n+\t/* This might be a replacement -- is the path already deleted? */\n+\tval = apr_hash_get(pb->deleted_entries, path, APR_HASH_KEY_STRING);\n+\n+\t/* Detect add-with-history. */\n+\tpb->eb->is_copy = ARE_VALID_COPY_ARGS(copyfrom_path, copyfrom_rev);\n+\n+\t/* Dump the node. */\n+\tSVN_ERR(dump_node(pb->eb, path,\n+\t svn_node_file,\n+\t val ? svn_node_action_replace : svn_node_action_add,\n+\t pb->eb->is_copy ? copyfrom_path : NULL,\n+\t pb->eb->is_copy ? copyfrom_rev : SVN_INVALID_REVNUM,\n+\t pool));\n+\n+\tif (val)\n+\t\t/* delete the path, it's now been dumped. */\n+\t\tapr_hash_set(pb->deleted_entries, path, APR_HASH_KEY_STRING, NULL);\n+\n+\t/* Build a nice file baton to pass to change_file_prop and apply_textdelta */\n+\tpb->eb->changed_path = path;\n+\t*file_baton = pb->eb;\n+\n \treturn SVN_NO_ERROR;\n }\n \n@@ -343,6 +368,28 @@ svn_error_t *open_file(const char *path,\n apr_pool_t *pool,\n void **file_baton)\n {\n+\tstruct dir_baton *pb = parent_baton;\n+\tconst char *cmp_path = NULL;\n+\tsvn_revnum_t cmp_rev = SVN_INVALID_REVNUM;\n+\n+\tapr_array_header_t *compose_path = apr_array_make(pool, 2, sizeof(const char *));\n+\t/* If the parent directory has explicit comparison path and rev,\n+\t record the same for this one. */\n+\tif (pb && ARE_VALID_COPY_ARGS(pb->cmp_path, pb->cmp_rev)) {\n+\t\tAPR_ARRAY_PUSH(compose_path, const char *) = pb->cmp_path;\n+\t\tAPR_ARRAY_PUSH(compose_path, const char *) = svn_dirent_basename(path, pool);\n+\t\tcmp_path = svn_path_compose(compose_path, pool);\n+\t\tcmp_rev = pb->cmp_rev;\n+\t}\n+\n+\tSVN_ERR(dump_node(pb->eb, path,\n+\t svn_node_file, svn_node_action_change,\n+\t cmp_path, cmp_rev, pool));\n+\n+\t/* Build a nice file baton to pass to change_file_prop and apply_textdelta */\n+\tpb->eb->changed_path = path;\n+\t*file_baton = pb->eb;\n+\n \treturn SVN_NO_ERROR;\n }\n \n@@ -387,6 +434,21 @@ svn_error_t *change_file_prop(void *file_baton,\n const svn_string_t *value,\n apr_pool_t *pool)\n {\n+\tstruct edit_baton *eb = file_baton;\n+\n+\tif (svn_property_kind(NULL, name) != svn_prop_regular_kind)\n+\t\treturn SVN_NO_ERROR;\n+\n+\tvalue ? apr_hash_set(eb->properties, apr_pstrdup(pool, name),\n+\t APR_HASH_KEY_STRING, svn_string_dup(value, pool)) :\n+\t\tapr_hash_set(eb->del_properties, apr_pstrdup(pool, name),\n+\t\t APR_HASH_KEY_STRING, (void *)0x1);\n+\n+\t/* Dump the property headers and wait; close_file might need\n+\t to write text headers too depending on whether\n+\t apply_textdelta is called */\n+\tdump_props_pending = TRUE;\n+\n \treturn SVN_NO_ERROR;\n }\n \n-- \n1.7.1\n"},{"id":"144985","messageId":"1278461693-3828-12-git-send-email-artagnon@gmail.com","threadId":"24313","inReplyTo":"1278461693-3828-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 11/13] Implement apply_textdelta","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-07-07T00:14:51Z","receivedAt":"2010-07-07T00:14:51Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"apply_textdelta picks up information from the file_baton (set by\nadd_file or open_file) and sets handler/ handler_baton to write a text\ndelta. It uses a temporary file because the content length of the\ndelta needs to be determined. Also add a related window_handler helper\nfunction gets the path of the temporary file written and cleans up\nafter apply_textdelta. The text and prop deltas will finally be dumped\nin close_file.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n dump_editor.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n dumpr_util.h | 15 ++++++++++++++\n 2 files changed, 74 insertions(+), 1 deletions(-)\n\ndiff --git a/dump_editor.c b/dump_editor.c\nindex 7006a2c..51d5ebe 100644\n--- a/dump_editor.c\n+++ b/dump_editor.c\n@@ -452,11 +452,69 @@ svn_error_t *change_file_prop(void *file_baton,\n \treturn SVN_NO_ERROR;\n }\n \n+svn_error_t *window_handler(svn_txdelta_window_t *window, void *baton)\n+{\n+\tstruct handler_baton *hb = baton;\n+\tstruct edit_baton *eb = hb->eb;\n+\tsvn_error_t *err;\n+\n+\terr = hb->apply_handler(window, hb->apply_baton);\n+\tif (window != NULL && !err)\n+\t\treturn SVN_NO_ERROR;\n+\n+\tif (err)\n+\t\tSVN_ERR(err);\n+\n+\t/* Write information about the filepath to hb->eb */\n+\teb->temp_filepath = apr_pstrdup(eb->pool,\n+\t\t\t\t\thb->temp_filepath);\n+\n+\t/* Cleanup */\n+\tSVN_ERR(svn_io_file_close(hb->temp_file, hb->pool));\n+\tSVN_ERR(svn_stream_close(hb->temp_filestream));\n+\tsvn_pool_destroy(hb->pool);\n+\treturn SVN_NO_ERROR;\n+}\n+\n svn_error_t *apply_textdelta(void *file_baton, const char *base_checksum,\n apr_pool_t *pool,\n svn_txdelta_window_handler_t *handler,\n void **handler_baton)\n {\n+\tsvn_stream_t *source_stream;\n+\tstruct edit_baton *eb = file_baton;\n+\tapr_status_t apr_err;\n+\n+\t/* Custom handler_baton allocated in a separate pool */\n+\tapr_pool_t *handler_pool = svn_pool_create(pool);\n+\tstruct handler_baton *hb = apr_pcalloc(handler_pool, sizeof(*hb));\n+\thb->pool = handler_pool;\n+\thb->eb = eb;\n+\n+\t/* Unset both handlers: To be set by two different functions later */\n+\thb->apply_handler = NULL;\n+\n+\t/* Use a temporary file to measure the text-content-length */\n+\thb->temp_filepath = apr_psprintf(eb->pool, \"/tmp/svn-fe/XXXXXX\");\n+\tapr_err = apr_file_mktemp(&(hb->temp_file), hb->temp_filepath,\n+\t\t\t\t APR_CREATE | APR_READ | APR_WRITE | APR_EXCL,\n+\t\t\t\t hb->pool);\n+\tif (apr_err != APR_SUCCESS)\n+\t\tSVN_ERR(svn_error_wrap_apr(apr_err, NULL));\n+\n+\thb->temp_filestream = svn_stream_from_aprfile2(hb->temp_file, TRUE, hb->pool);\n+\tsource_stream = svn_stream_empty(hb->pool);\n+\n+\t/* Prepare to write the delta to the temporary file */\n+\tsvn_txdelta_to_svndiff2(&(hb->apply_handler), &(hb->apply_baton),\n+\t hb->temp_filestream, 0, hb->pool);\n+\tmust_dump_text = TRUE;\n+\n+\t/* The actual writing takes place when this function has finished */\n+\t/* Set the handler and handler_baton */\n+\t*handler = window_handler;\n+\t*handler_baton = hb;\n+\n \treturn SVN_NO_ERROR;\n }\n \n@@ -493,7 +551,7 @@ svn_error_t *get_dump_editor(const svn_delta_editor_t **editor,\n \tde->close_directory = close_directory;\n \tde->change_dir_prop = change_dir_prop;\n \tde->change_file_prop = change_file_prop;\n-\t/* de->apply_textdelta = apply_textdelta; */\n+\tde->apply_textdelta = apply_textdelta;\n \tde->add_file = add_file;\n \tde->open_file = open_file;\n \tde->close_file = close_file;\ndiff --git a/dumpr_util.h b/dumpr_util.h\nindex 79de1ab..3830a1d 100644\n--- a/dumpr_util.h\n+++ b/dumpr_util.h\n@@ -61,6 +61,21 @@ struct dir_baton {\n \tapr_pool_t *pool;\n };\n \n+struct handler_baton\n+{\n+\tsvn_txdelta_window_handler_t apply_handler;\n+\tvoid *apply_baton;\n+\tapr_pool_t *pool;\n+\n+\t/* Information about the path of the tempoarary file used */\n+\tchar *temp_filepath;\n+\tapr_file_t *temp_file;\n+\tsvn_stream_t *temp_filestream;\n+\n+\t/* To fill in the edit baton fields */\n+\tstruct edit_baton *eb;\n+};\n+\n void write_hash_to_stringbuf(apr_hash_t *properties,\n \t\t\t svn_boolean_t deleted,\n \t\t\t svn_stringbuf_t **strbuf,\n-- \n1.7.1\n"},{"id":"144983","messageId":"1278461693-3828-13-git-send-email-artagnon@gmail.com","threadId":"24313","inReplyTo":"1278461693-3828-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 12/13] Implement close_file","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-07-07T00:14:52Z","receivedAt":"2010-07-07T00:14:52Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"close_file measures the length of the temporary file to write text\nheaders and full text before cleaning up the temporary file. It also\nwrites props and prop deltas if necessary.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n dump_editor.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 76 insertions(+), 0 deletions(-)\n\ndiff --git a/dump_editor.c b/dump_editor.c\nindex 51d5ebe..6f74af9 100644\n--- a/dump_editor.c\n+++ b/dump_editor.c\n@@ -522,6 +522,82 @@ svn_error_t *close_file(void *file_baton,\n \t\t\tconst char *text_checksum,\n \t\t\tapr_pool_t *pool)\n {\n+\tstruct edit_baton *eb = file_baton;\n+\tapr_file_t *temp_file;\n+\tsvn_stream_t *temp_filestream;\n+\tapr_finfo_t *info = apr_pcalloc(pool, sizeof(apr_finfo_t));\n+\n+\t/* We didn't write the property headers because we were\n+\t waiting for file_prop_change; write them now */\n+\tSVN_ERR(dump_props(eb, &dump_props_pending, FALSE, pool));\n+\n+\t/* The prop headers have already been dumped in dump_node */\n+\t/* Dump the text headers */\n+\tif (must_dump_text) {\n+\t\t/* text-delta header */\n+\t\tSVN_ERR(svn_stream_printf(eb->stream, pool,\n+\t\t\t\t\t SVN_REPOS_DUMPFILE_TEXT_DELTA\n+\t\t\t\t\t \": true\\n\"));\n+\n+\t\t/* Measure the length */\n+\t\tSVN_ERR(svn_io_stat(info, eb->temp_filepath, APR_FINFO_SIZE, pool));\n+\n+\t\t/* text-content-length header */\n+\t\tSVN_ERR(svn_stream_printf(eb->stream, pool,\n+\t\t\t\t\t SVN_REPOS_DUMPFILE_TEXT_CONTENT_LENGTH\n+\t\t\t\t\t \": %lu\\n\",\n+\t\t\t\t\t (unsigned long)info->size));\n+\t\t/* text-content-md5 header */\n+\t\tSVN_ERR(svn_stream_printf(eb->stream, pool,\n+\t\t\t\t\t SVN_REPOS_DUMPFILE_TEXT_CONTENT_MD5\n+\t\t\t\t\t \": %s\\n\",\n+\t\t\t\t\t text_checksum));\n+\t}\n+\n+\t/* content-length header: if both text and props are absent,\n+\t skip this block */\n+\tif (must_dump_props || dump_props_pending)\n+\t\tSVN_ERR(svn_stream_printf(eb->stream, pool,\n+\t\t\t\t\t SVN_REPOS_DUMPFILE_CONTENT_LENGTH\n+\t\t\t\t\t \": %ld\\n\\n\",\n+\t\t\t\t\t (unsigned long)info->size + eb->propstring->len));\n+\telse if (must_dump_text)\n+\t\tSVN_ERR(svn_stream_printf(eb->stream, pool,\n+\t\t\t\t\t SVN_REPOS_DUMPFILE_CONTENT_LENGTH\n+\t\t\t\t\t \": %ld\\n\\n\",\n+\t\t\t\t\t (unsigned long)info->size));\n+\n+\t/* Dump the props; the propstring should have already been\n+\t written in dump_node or above */\n+\tif (must_dump_props || dump_props_pending) {\n+\t\tSVN_ERR(svn_stream_write(eb->stream, eb->propstring->data,\n+\t\t\t\t\t &(eb->propstring->len)));\n+\n+\t\t/* Cleanup */\n+\t\tmust_dump_props = dump_props_pending = FALSE;\n+\t\tapr_hash_clear(eb->properties);\n+\t\tapr_hash_clear(eb->del_properties);\n+\t}\n+\n+\t/* Dump the text */\n+\tif (must_dump_text) {\n+\n+\t\t/* Open the temporary file, map it to a stream, copy\n+\t\t the stream to eb->stream, close and delete the\n+\t\t file */\n+\t\tSVN_ERR(svn_io_file_open(&temp_file, eb->temp_filepath, APR_READ, 0600, pool));\n+\t\ttemp_filestream = svn_stream_from_aprfile2(temp_file, TRUE, pool);\n+\t\tSVN_ERR(svn_stream_copy3(temp_filestream, eb->stream, NULL, NULL, pool));\n+\n+\t\t/* Cleanup */\n+\t\tSVN_ERR(svn_io_file_close(temp_file, pool));\n+\t\tSVN_ERR(svn_stream_close(temp_filestream));\n+\t\tSVN_ERR(svn_io_remove_file2(eb->temp_filepath, TRUE, pool));\n+\t\tmust_dump_text = FALSE;\n+\t}\n+\n+\tSVN_ERR(svn_stream_printf(eb->stream, pool, \"\\n\\n\"));\n+\n \treturn SVN_NO_ERROR;\n }\n \n-- \n1.7.1\n"},{"id":"144984","messageId":"1278461693-3828-14-git-send-email-artagnon@gmail.com","threadId":"24313","inReplyTo":"1278461693-3828-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 13/13] Add a validation script","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-07-07T00:14:53Z","receivedAt":"2010-07-07T00:14:53Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Add a validation script along with a .gitignore. Using an existing\ndump known to be correct (possibly generated using `svnsync` and\n`svnadmin dump --deltas`), it compares the outputs produced by\n`svnadmin load` when fed with this dump and the dump from the program.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n .gitignore | 4 ++++\n validate.sh | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 54 insertions(+), 0 deletions(-)\n create mode 100644 .gitignore\n create mode 100755 validate.sh\n\ndiff --git a/.gitignore b/.gitignore\nnew file mode 100644\nindex 0000000..3e9b906\n--- /dev/null\n+++ b/.gitignore\n@@ -0,0 +1,4 @@\n+*~\n+t/\n+svndumpr\n+svndumpr_bench\ndiff --git a/validate.sh b/validate.sh\nnew file mode 100755\nindex 0000000..7b25db6\n--- /dev/null\n+++ b/validate.sh\n@@ -0,0 +1,50 @@\n+#!/bin/sh\n+# asf.dump must exist in t/\n+# Compile the program with end_revision = $2 when using the second branch\n+\n+case $1 in\n+ generate)\n+\t[ -z $2 ] && { echo \"Usage: $0 $1 \"; exit 1; } || echo \"Starting generation ...\";\n+\tif test -e \"t/asf.dump\"\n+\t then :;\n+\telse\n+\t echo \"Need t/asf.dump (dumpfile v3) first. Generate it yourself or steal it from someone.\";\n+\t exit 1;\n+\tfi\n+\trm -rf t/repo;\n+\tmkdir t/repo;\n+\tsvnadmin create t/repo;\n+\tgawk \"/Revision-number: $(($2 + 1))/ { exit 1 }; { print \\$0 };\" t/asf.dump > \"t/asf-$2.dump\";\n+\t[ $? = 1 ] && echo \"Cut $2 succeeded!\" || { echo \"Cut $2 failed. Check t/asf.dump and validate.sh.\"; exit 1; }\n+\tsvnadmin load t/repo < \"t/asf-$2.dump\" 1>\"t/asf-$2-import.log\" 2>\"t/asf-$2-import.error\";\n+\t[ $? = 0 ] && echo \"Load $2 succeeded!\" || { echo \"Load $2 failed. See t/asf-$2-import.debug for details.\"; exit 1; }\n+\techo \"Successfully generated asf-$2.dump and asf-$2-import.log. You can now run validate.sh validate $2\"\n+\texit 0;;\n+ validate)\n+\t[ -z $2 ] && { echo \"Usage: $0 $1 \"; exit 1; } || echo \"Starting validation ...\";\n+\tif test -e \"t/asf-$2.dump\" && test -e \"t/asf-$2-import.log\"\n+\t then :;\n+\telse\n+\t echo \"Run validate.sh genereate $2 first.\";\n+\t exit 1;\n+\tfi\n+\trm -rf /tmp/svn-fe;\n+\tmkdir /tmp/svn-fe;\n+\trm -rf t/repo;\n+\tmkdir t/repo;\n+\tsvnadmin create t/repo;\n+\tmake svndumpr > /dev/null;\n+\t[ $? = 0 ] && echo \"Make succeeded!\" || { echo \"Make failed. Check the program.\"; exit 1; }\n+\t./svndumpr 1>t/asf-mine.dump;\n+\t[ $? = 0 ] && echo \"Run succeeded!\" || { echo \"Run failed. See t/asf.debug for details.\"; exit 1; }\n+\tdiff -au \"t/asf-$2.dump\" t/asf-mine.dump > t/dump-diff.error;\n+\tgawk '$0 !~ \"Prop-delta: true|Text-delta-base-|sha1|Text-copy-source-|^-$\" && $0 ~ \"^+|^-\" { print; }' t/dump-diff.error > t/dump-diff-filtered.error;\n+\tsvnadmin load t/repo < t/asf-mine.dump 1>t/asf-mine-import.log 2>t/asf-mine-import.error;\n+\t[ $? = 0 ] && echo \"Load $2 succeeded!\" || { echo \"Load failed. See t/asf-mine-import.error, t/dump-diff.error, and t/dump-diff-filtered.error for details.\"; exit 1; }\n+\tdiff -au \"t/asf-$2-import.log\" t/asf-mine-import.log > t/import-diff.error;\n+\t[ $? = 0 ] && echo \"Validation $2 succeeded!\" || { echo \"Validation failed. See t/import-diff.error for details.\"; exit 1; }\n+\texit 0;;\n+ *)\n+\techo \"Usage: $0 \";\n+\texit 1;;\n+esac\n-- \n1.7.1\n"},{"id":"145031","messageId":"20100707162516.GA1529@burratino","threadId":"24313","inReplyTo":"1278461693-3828-3-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 02/13] Add skeleton SVN client and Makefile","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-07T16:25:16Z","receivedAt":"2010-07-07T16:25:16Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> Add a basic SVN command-line client along with a Makefile that does\n> just enough to establish a connection with the ASF subversion server;\n\nThanks for splitting this out.\n\nLet’s see what’s needed to set up a connection:\n\n> +++ b/Makefile\n> @@ -0,0 +1,8 @@\n> +svndumpr: *.c *.h\n> +\t$(CC) -Wall -Werror -DAPR_POOL_DEBUG -ggdb3 -O0 -o $@ svndumpr.c -lsvn_client-1 -I. -I/usr/include/subversion-1 -I/usr/include/apr-1.0\n\nLinks against libsvnclient-1. Good.\n\nI assume the details of the Makefile are not important, since it is\nprobably going to be revamped in the style of the svn build system\nanyway.\n\n> +++ b/svndumpr.c\n> @@ -0,0 +1,68 @@\n[...]\n> +svn_error_t *populate_context()\n[...]\n> +svn_error_t *open_connection(const char *url)\n[...]\n> +svn_error_t *replay_range(svn_revnum_t start_revision, svn_revnum_t end_revision)\n\nWhy not static?\n\n> +svn_error_t *populate_context()\n> +{\n> +\tconst char *http_library;\n> +\t\n> +\tSVN_ERR(svn_config_get_config(&(ctx->config), NULL, pool));\n> +\t\n> +\thttp_library = getenv(\"SVN_HTTP_LIBRARY\");\n> +\tif (http_library)\n> +\t\tsvn_config_set(apr_hash_get(ctx->config, \"servers\", APR_HASH_KEY_STRING),\n> +\t\t \"global\", \"http-library\", http_library);\n\nI tried googling for this SVN_HTTP_LIBRARY setting, but no\nuseful hints. I take it that this overrides the [global] http-library\nsetting from ~/.subversion/servers? Do other commands honor this\nenvironment variable or just svndumpr?\n\n[...]\n> +svn_error_t *open_connection(const char *url)\n> +{\n> +\tSVN_ERR(svn_config_ensure (NULL, pool));\n> +\tSVN_ERR(svn_client_create_context (&ctx, pool));\n> +\tSVN_ERR(svn_ra_initialize(pool));\n> +\n> +#if defined(WIN32) || defined(__CYGWIN__)\n> +\tif (getenv(\"SVN_ASP_DOT_NET_HACK\"))\n> +\t\tSVN_ERR(svn_wc_set_adm_dir(\"_svn\", pool));\n> +#endif\n\nI guess it’s water under the bridge now (from 5 years ago), but why do\nclients have to do this themselves? It would not be so difficult for\nlibsvnclient to automatically set the admin dir according to whether\nSVN_ASP_DOT_NET_HACK is set or not, or at least to provide a single\nfunction to call and do so.\n\nBut that is not the topic for the moment. I am tempted to suggest\nchecking SVN_ASP_DOT_NET_HACK unconditionally (i.e., on Unix, too),\njust so the function is easier to scan. Or there could be a separate\nset_appropriate_adm_dir function in svndumpr.c:\n\n\t#if defined(WIN32) || ...\n\tstatic svn_error_t *set_appropriate_adm_dir(...)\n\t{\n\t\tif (getenv...\n\t\t...\n\t}\n\t#else\n\tstatic svn_error_t *set_appropriate_adm_dir(...\n\t{\n\t\treturn SVN_NO_ERROR;\n\t}\n\t#endif\n\nFeel free to ignore me here. :)\n\n> +\n> +\tSVN_ERR(populate_context());\n> +\tSVN_ERR(svn_cmdline_create_auth_baton(&(ctx->auth_baton), TRUE,\n> +\t\t\t\t\t NULL, NULL, NULL, FALSE,\n> +\t\t\t\t\t FALSE, NULL, NULL, NULL,\n> +\t\t\t\t\t pool));\n\nMaybe comments would help, for the boolean arguments.\n\n> +\tSVN_ERR(svn_client_open_ra_session(&session, url, ctx, pool));\n> +\treturn SVN_NO_ERROR;\n> +}\n> +\n> +svn_error_t *replay_range(svn_revnum_t start_revision, svn_revnum_t end_revision)\n> +{\n> +\treturn SVN_NO_ERROR;\n> +}\n\nMight be more self-explanatory without this function, but that\nis just nitpicking.\n\n> +\n> +int main()\n> +{\n> +\tconst char url[] = \"http://svn.apache.org/repos/asf\";\n> +\tsvn_revnum_t start_revision = 1, end_revision = 500;\n> +\tif (svn_cmdline_init (\"svndumpr\", stderr) != EXIT_SUCCESS)\n> +\t\treturn 1;\n> +\n> +\tpool = svn_pool_create(NULL);\n> +\n> +\tSVN_INT_ERR(open_connection(url));\n> +\tSVN_INT_ERR(replay_range(start_revision, end_revision));\n> +\n> +\tsvn_pool_destroy(pool);\n> +\t\n> +\treturn 0;\n> +}\n\nSo: this is an expensive no-op.\n\nThanks for the pleasant reading.\n\nJonathan\n"},{"id":"145034","messageId":"20100707170926.GC18806@debian","threadId":"24313","inReplyTo":"20100707162516.GA1529@burratino","subject":"Re: [PATCH 02/13] Add skeleton SVN client and Makefile","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-07-07T17:09:26Z","receivedAt":"2010-07-07T17:09:26Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Jonathan,\n\nJonathan Nieder writes:\n> Ramkumar Ramachandra wrote:\n> \n> > Add a basic SVN command-line client along with a Makefile that does\n> > just enough to establish a connection with the ASF subversion server;\n> \n> Thanks for splitting this out.\n\nThanks for getting the review process started :)\n\n> Let’s see what’s needed to set up a connection:\n> \n> > +++ b/Makefile\n> > @@ -0,0 +1,8 @@\n> > +svndumpr: *.c *.h\n> > +\t$(CC) -Wall -Werror -DAPR_POOL_DEBUG -ggdb3 -O0 -o $@ svndumpr.c -lsvn_client-1 -I. -I/usr/include/subversion-1 -I/usr/include/apr-1.0\n> \n> Links against libsvnclient-1. Good.\n> \n> I assume the details of the Makefile are not important, since it is\n> probably going to be revamped in the style of the svn build system\n> anyway.\n\nRight.\n\n> > +++ b/svndumpr.c\n> > @@ -0,0 +1,68 @@\n> [...]\n> > +svn_error_t *populate_context()\n> [...]\n> > +svn_error_t *open_connection(const char *url)\n> [...]\n> > +svn_error_t *replay_range(svn_revnum_t start_revision, svn_revnum_t end_revision)\n> \n> Why not static?\n\nChanged (see end of email).\n\n> > +svn_error_t *populate_context()\n> > +{\n> > +\tconst char *http_library;\n> > +\t\n> > +\tSVN_ERR(svn_config_get_config(&(ctx->config), NULL, pool));\n> > +\t\n> > +\thttp_library = getenv(\"SVN_HTTP_LIBRARY\");\n> > +\tif (http_library)\n> > +\t\tsvn_config_set(apr_hash_get(ctx->config, \"servers\", APR_HASH_KEY_STRING),\n> > +\t\t \"global\", \"http-library\", http_library);\n> \n> I tried googling for this SVN_HTTP_LIBRARY setting, but no\n> useful hints. I take it that this overrides the [global] http-library\n> setting from ~/.subversion/servers? Do other commands honor this\n> environment variable or just svndumpr?\n\nI originally had this to switch between neon and serf libraries. It\nseems that this is no more necessary.\n\n> [...]\n> > +svn_error_t *open_connection(const char *url)\n> > +{\n> > +\tSVN_ERR(svn_config_ensure (NULL, pool));\n> > +\tSVN_ERR(svn_client_create_context (&ctx, pool));\n> > +\tSVN_ERR(svn_ra_initialize(pool));\n> > +\n> > +#if defined(WIN32) || defined(__CYGWIN__)\n> > +\tif (getenv(\"SVN_ASP_DOT_NET_HACK\"))\n> > +\t\tSVN_ERR(svn_wc_set_adm_dir(\"_svn\", pool));\n> > +#endif\n> \n> I guess it’s water under the bridge now (from 5 years ago), but why do\n> clients have to do this themselves? It would not be so difficult for\n> libsvnclient to automatically set the admin dir according to whether\n> SVN_ASP_DOT_NET_HACK is set or not, or at least to provide a single\n> function to call and do so.\n\nGood question. I copied this out from some legacy code in the\nSubversion trunk. Additionally, since I don't have a working copy,\nthis is completely unnecessary. Removed now.\n\n> > +\n> > +\tSVN_ERR(populate_context());\n> > +\tSVN_ERR(svn_cmdline_create_auth_baton(&(ctx->auth_baton), TRUE,\n> > +\t\t\t\t\t NULL, NULL, NULL, FALSE,\n> > +\t\t\t\t\t FALSE, NULL, NULL, NULL,\n> > +\t\t\t\t\t pool));\n> \n> Maybe comments would help, for the boolean arguments.\n\nFixed.\n\n> > +\tSVN_ERR(svn_client_open_ra_session(&session, url, ctx, pool));\n> > +\treturn SVN_NO_ERROR;\n> > +}\n> > +\n> > +svn_error_t *replay_range(svn_revnum_t start_revision, svn_revnum_t end_revision)\n> > +{\n> > +\treturn SVN_NO_ERROR;\n> > +}\n> \n> Might be more self-explanatory without this function, but that\n> is just nitpicking.\n\nI've filled in the function in a future patch. It's just to say that\nI'm \"opening a connection and then calling replay_range, in that\norder\". Yes, replay_range does nothing in this patch.\n\n> > +\n> > +int main()\n> > +{\n> > +\tconst char url[] = \"http://svn.apache.org/repos/asf\";\n> > +\tsvn_revnum_t start_revision = 1, end_revision = 500;\n> > +\tif (svn_cmdline_init (\"svndumpr\", stderr) != EXIT_SUCCESS)\n> > +\t\treturn 1;\n> > +\n> > +\tpool = svn_pool_create(NULL);\n> > +\n> > +\tSVN_INT_ERR(open_connection(url));\n> > +\tSVN_INT_ERR(replay_range(start_revision, end_revision));\n> > +\n> > +\tsvn_pool_destroy(pool);\n> > +\t\n> > +\treturn 0;\n> > +}\n> \n> So: this is an expensive no-op.\n\nExactly. I built up the patches so that at every stage, the program\ncompiles and runs fine, implementing part of full functionality.\n\nHere's a diff of the modifications I made after your review:\n\ndiff --git a/svndumpr.c b/svndumpr.c\nindex 011941f..f3117aa 100644\n--- a/svndumpr.c\n+++ b/svndumpr.c\n@@ -76,31 +76,19 @@ static svn_error_t *replay_revend(svn_revnum_t revision,\n \treturn SVN_NO_ERROR;\n }\n \n-svn_error_t *populate_context()\n-{\n-\tconst char *http_library;\n-\t\n-\tSVN_ERR(svn_config_get_config(&(ctx->config), NULL, pool));\n-\t\n-\thttp_library = getenv(\"SVN_HTTP_LIBRARY\");\n-\tif (http_library)\n-\t\tsvn_config_set(apr_hash_get(ctx->config, \"servers\", APR_HASH_KEY_STRING),\n-\t\t \"global\", \"http-library\", http_library);\n-\treturn SVN_NO_ERROR;\n-}\n-\n-svn_error_t *open_connection(const char *url)\n+static svn_error_t *open_connection(const char *url)\n {\n \tSVN_ERR(svn_config_ensure (NULL, pool));\n \tSVN_ERR(svn_client_create_context (&ctx, pool));\n \tSVN_ERR(svn_ra_initialize(pool));\n \n-#if defined(WIN32) || defined(__CYGWIN__)\n-\tif (getenv(\"SVN_ASP_DOT_NET_HACK\"))\n-\t\tSVN_ERR(svn_wc_set_adm_dir(\"_svn\", pool));\n-#endif\n+\tSVN_ERR(svn_config_get_config(&(ctx->config), NULL, pool));\n \n-\tSVN_ERR(populate_context());\n+\t/* Populte ctx->auth_baton with the auth baton\n+\t non-interactively. Arguments 3, 4 and 5 are for username,\n+\t password and config_dir which is NULL in this case. Set\n+\t no_auth_cache and trust_serv_cert to FALSE, don't provide a\n+\t config, and omit cancel_func/ cancel_baton */\n \tSVN_ERR(svn_cmdline_create_auth_baton(&(ctx->auth_baton), TRUE,\n \t\t\t\t\t NULL, NULL, NULL, FALSE,\n \t\t\t\t\t FALSE, NULL, NULL, NULL,\n@@ -109,7 +97,7 @@ svn_error_t *open_connection(const char *url)\n \treturn SVN_NO_ERROR;\n }\n \n-svn_error_t *replay_range(svn_revnum_t start_revision, svn_revnum_t end_revision)\n+static svn_error_t *replay_range(svn_revnum_t start_revision, svn_revnum_t end_revision)\n {\n \tconst svn_delta_editor_t *dump_editor, *debug_editor;\n \tvoid *debug_baton, *dump_baton;\n\n\n-- Ram\n"},{"id":"145040","messageId":"alpine.561.2.00.1007072048100.569@daniel1.local","threadId":"24313","inReplyTo":"20100707162516.GA1529@burratino","subject":"Re: [PATCH 02/13] Add skeleton SVN client and Makefile","fromName":"Daniel Shahaf","fromEmail":"d.s@daniel.shahaf.name","sentAt":"2010-07-07T17:51:17Z","receivedAt":"2010-07-07T17:51:17Z","isPatch":true,"sender":{"key":"d.s@daniel.shahaf.name","avatar":null},"body":"Jonathan Nieder wrote on Wed, 7 Jul 2010 at 11:25 -0500:\n> Ramkumar Ramachandra wrote:\n> > +svn_error_t *populate_context()\n> > +{\n> > +\tconst char *http_library;\n> > +\t\n> > +\tSVN_ERR(svn_config_get_config(&(ctx->config), NULL, pool));\n> > +\t\n> > +\thttp_library = getenv(\"SVN_HTTP_LIBRARY\");\n> > +\tif (http_library)\n> > +\t\tsvn_config_set(apr_hash_get(ctx->config, \"servers\", APR_HASH_KEY_STRING),\n> > +\t\t \"global\", \"http-library\", http_library);\n> \n> I tried googling for this SVN_HTTP_LIBRARY setting, but no\n> useful hints. I take it that this overrides the [global] http-library\n> setting from ~/.subversion/servers? Do other commands honor this\n> environment variable or just svndumpr?\n\nOther commands use --config-option. SVN_HTTP_LIBRARY is not recognized\nby any released Subversion, nor by trunk@HEAD of Subversion.\n\nIt's a debugging relic, anyway. Can be removed. The functionality is \nprovided by editing $CONFIG_DIR/servers.\n"},{"id":"145038","messageId":"20100707175530.GA2480@burratino","threadId":"24313","inReplyTo":"1278461693-3828-4-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 03/13] Add debug editor from Subversion trunk","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-07T17:55:30Z","receivedAt":"2010-07-07T17:55:30Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi again,\n\nRamkumar Ramachandra wrote:\n\n> Add the debug editor from subversion/libsvn_delta/debug_editor.c along\n> with a header to expose the svn_delta__get_debug_editor function.\n\nThe description does not tell what the debug editor is for. Is it\nfor tracing?\n\nIn what follows, I am going to pretend this is all new code, since\nfor someone unfamiliar to svn like me, that is easier than reviewing\nthe differences. Upshot: you can probably ignore most of what I say. :)\n\n> +++ b/debug_editor.c\n> @@ -0,0 +1,402 @@\n> +/* Licensed under a two-clause BSD-style license.\n> + * See LICENSE for details.\n> + */\n\nIs this true?\n\n> +\n> +#include \"svn_pools.h\"\n> +#include \"svn_cmdline.h\"\n> +#include \"svn_client.h\"\n> +#include \"svn_ra.h\"\n> +\n> +#include \"debug_editor.h\"\n> +\n> +struct edit_baton\n> +{\n> +\tconst svn_delta_editor_t *wrapped_editor;\n> +\tvoid *wrapped_edit_baton;\n> +\n> +\tint indent_level;\n> +\n> +\tsvn_stream_t *out;\n> +};\n\nThis object represents context while describing changes in a revision.\nThe indent_level gets incremented whenever we move to a subdirectory,\nwrapped_editor is a set of callbacks to actually do something with\nthe changes, wrapped_edit_baton its state cookie, and out a stream\nto write the debugging information to.\n\n> +\n> +struct dir_baton\n> +{\n> +\tvoid *edit_baton;\n> +\tvoid *wrapped_dir_baton;\n> +};\n\nContext when traversing a directory.\n\nMaybe the debugger’s state should be consistently before or\nconsistently after the wrapped state. But this is just nitpicking.\n\nAnother nitpick: Is the prevailing style in subversion to use void *\nfor related context objects like edit_baton? If not, I would suggest\nusing struct edit_baton * to document that that is always its\ntype; but if so, nothing to see here, please carry on.\n\n> +struct file_baton\n> +{\n> +\tvoid *edit_baton;\n> +\tvoid *wrapped_file_baton;\n> +};\n\nSimilar.\n\n[...]\n> +static svn_error_t *set_target_revision(void *edit_baton,\n> +\t\t\t\t\tsvn_revnum_t target_revision,\n> +\t\t\t\t\tapr_pool_t *pool)\n> +{\n> +\tstruct edit_baton *eb = edit_baton;\n> +\n> +\tSVN_ERR(write_indent(eb, pool));\n> +\tSVN_ERR(svn_stream_printf(eb->out, pool, \"set_target_revision : %ld\\n\",\n> +\t\t\t\t target_revision));\n> +\n> +\treturn eb->wrapped_editor->set_target_revision(eb->wrapped_edit_baton,\n> +\t\t\t\t\t\t target_revision,\n> +\t\t\t\t\t\t pool);\n> +}\n\nThis is unfortunately long for how little it does.\n\nC question (I’m just curious): would it be allowed to use\n\n static svn_Error_t *set_target_revision(struct edit_baton *eb,\n\tetc\n\nIn other words, does C allow a function with struct foo *\nargument to be called through a pointer to function with void *\nargument?\n\n> +\n> +static svn_error_t *open_root(void *edit_baton,\n> +\t\t\t svn_revnum_t base_revision,\n> +\t\t\t apr_pool_t *pool,\n> +\t\t\t void **root_baton)\n> +{\n> +\tstruct edit_baton *eb = edit_baton;\n> +\tstruct dir_baton *dir_baton = apr_palloc(pool, sizeof(*dir_baton));\n> +\n> +\tSVN_ERR(write_indent(eb, pool));\n> +\tSVN_ERR(svn_stream_printf(eb->out, pool, \"open_root : %ld\\n\",\n> +\t\t\t\t base_revision));\n> +\teb->indent_level++;\n> +\n> +\tSVN_ERR(eb->wrapped_editor->open_root(eb->wrapped_edit_baton,\n> +\t\t\t\t\t base_revision,\n> +\t\t\t\t\t pool,\n> +\t\t\t\t\t &dir_baton->wrapped_dir_baton));\n> +\n> +\tdir_baton->edit_baton = edit_baton;\n> +\n> +\t*root_baton = dir_baton;\n> +\n> +\treturn SVN_NO_ERROR;\n> +}\n\nSimilar. Maybe:\n\n\tstatic svn_error_t *open_root(...\n\t{\n\t\tstruct edit_baton *eb = edit_baton;\n\t\tstruct dir_baton *dir_baton;\n\n\t\tSVN_ERR(write_indent...\n\t\tSVN_ERR(svn_stream_printf...\n\n\t\tdir_baton = apr_palloc(...\n\t\tdir_baton->edit_baton = eb;\n\t\tSVN_ERR(eb->wrapped_editor->open_root(...\n\n\t\t*root_baton = dir_baton;\n\t\teb->indent_level++;\n\t\treturn SVN_NO_ERROR;\n\t}\n\n[...]\n> +static svn_error_t *add_directory(const char *path,\n[...]\n> +static svn_error_t *open_directory(const char *path,\n[...]\n> +static svn_error_t *add_file(const char *path,\n[...]\n> +static svn_error_t *open_file(const char *path,\n\nSimilar.\n\n> +static svn_error_t *close_file(void *file_baton,\n> +\t\t\t const char *text_checksum,\n> +\t\t\t apr_pool_t *pool)\n> +{\n> +\tstruct file_baton *fb = file_baton;\n> +\tstruct edit_baton *eb = fb->edit_baton;\n> +\n> +\teb->indent_level--;\n> +\n> +\tSVN_ERR(write_indent(eb, pool));\n> +\tSVN_ERR(svn_stream_printf(eb->out, pool, \"close_file : %s\\n\",\n> +\t\t\t\t text_checksum));\n> +\n> +\tSVN_ERR(eb->wrapped_editor->close_file(fb->wrapped_file_baton,\n> +\t\t\t\t\t text_checksum, pool));\n> +\n> +\treturn SVN_NO_ERROR;\n> +}\n\nThe context pointers for each file and directory in each revision are\ncollected in a single pool and not freed, well, ever. I assume that\nis not a problem in practice; if it is, one can always start making\nsubpools later.\n\n> +svn_error_t *svn_delta__get_debug_editor(const svn_delta_editor_t **editor,\n> +\t\t\t\t\t void **edit_baton,\n> +\t\t\t\t\t const svn_delta_editor_t *wrapped_editor,\n> +\t\t\t\t\t void *wrapped_edit_baton,\n> +\t\t\t\t\t apr_pool_t *pool)\n> +{\n> +\tsvn_delta_editor_t *tree_editor = svn_delta_default_editor(pool);\n> +\tstruct edit_baton *eb = apr_palloc(pool, sizeof(*eb));\n> +\tapr_file_t *errfp;\n> +\tsvn_stream_t *out;\n> +\n> +\tapr_status_t apr_err = apr_file_open_stderr(&errfp, pool);\n> +\tif (apr_err)\n> +\t\treturn svn_error_wrap_apr(apr_err, \"Problem opening stderr\");\n\nIs there no function for this that returns svn_error_t *?\n\n[...]\n> +++ b/debug_editor.h\n> @@ -0,0 +1,10 @@\n> +#ifndef DEBUG_EDITOR_H_\n> +#define DEBUG_EDITOR_H_\n> +\n> +svn_error_t *svn_delta__get_debug_editor(const svn_delta_editor_t **editor,\n> +\t\t\t\t\t void **edit_baton,\n> +\t\t\t\t\t const svn_delta_editor_t *wrapped_editor,\n> +\t\t\t\t\t void *wrapped_edit_baton,\n> +\t\t\t\t\t apr_pool_t *pool);\n\nUsable from other code. Caller provides the pool. No example user\nyet.\n\nWell, it looks like it should work. :)\n"},{"id":"145042","messageId":"20100707181619.GA2617@burratino","threadId":"24313","inReplyTo":"1278461693-3828-5-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 04/13] Add skeleton dump editor","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-07T18:16:19Z","receivedAt":"2010-07-07T18:16:19Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> Add a dump editor and write out skeleton callback functions according\n> to the API documentation of svn_delta_editor_t. Also expose\n> get_dump_editor through a header.\n\nThis commit message tells me nothing... Maybe:\n\n\tAdd a no-op svn_editor. The function to retrieve it is called\n\tget_dump_editor because it is planned to tweak it to write a\n\tdumpfile. But for now it is more useful when used with the\n\tdebug_editor, to get a list of editor operations printed to\n\tstderr.\n\nIt could make sense to squash this with patch 5 as a demo.\n\n> --- /dev/null\n> +++ b/dump_editor.c\n> @@ -0,0 +1,143 @@\n> +/* Licensed under a two-clause BSD-style license.\n> + * See LICENSE for details.\n> + */\n> +\n> +#include \"svn_pools.h\"\n> +#include \"svn_error.h\"\n> +#include \"svn_iter.h\"\n> +#include \"svn_repos.h\"\n> +#include \"svn_string.h\"\n> +#include \"svn_dirent_uri.h\"\n> +#include \"svn_path.h\"\n> +#include \"svn_time.h\"\n> +#include \"svn_checksum.h\"\n> +#include \"svn_props.h\"\n\nAre these all needed?\n\n[...]\n> +svn_error_t *open_root(void *edit_baton,\n> + svn_revnum_t base_revision,\n> + apr_pool_t *pool,\n> + void **root_baton)\n> +{\n> +\treturn SVN_NO_ERROR;\n> +}\n\nMight make sense to use\n\n\t*root_baton = NULL;\n\nfor easier debugging.\n\n[...]\n> +svn_error_t *add_directory(const char *path,\n[...]\n> +svn_error_t *open_directory(const char *path,\n[...]\n> +svn_error_t *add_file(const char *path,\n[...]\n> +svn_error_t *open_file(const char *path,\n[...]\n> +svn_error_t *apply_textdelta(void *file_baton, const char *base_checksum,\n\nLikewise.\n\n[...]\n> +++ b/dumpr_util.h\n> @@ -0,0 +1,29 @@\n> +#ifndef DUMPR_UTIL_H_\n> +#define DUMPR_UTIL_H_\n> +\n> +struct edit_baton {\n\nA more specific name might be nice (or might not, depending on the\nprevailing style in svn; I ought to check but I am too lazy).\n\n> +\t/* The stream to dump to: stdout */\n> +\tsvn_stream_t *stream;\n> +\n> +\t/* pool is for per-edit-session allocations */\n> +\tapr_pool_t *pool;\n\nUnused; probably should delay adding these until there is a user to\nexplain them.\n\n> +\n> +\tsvn_revnum_t current_rev;\n\nUsed.\n\n> +\t\n> +\t/* Store the properties that changed */\n> +\tapr_hash_t *properties;\n> +\tapr_hash_t *del_properties; /* Value is always 0x1 */\n> +\tsvn_stringbuf_t *propstring;\n> +\n> +\t/* Path of changed file */\n> +\tconst char *changed_path;\n> +\n> +\t/* Was a copy command issued? */\n> +\tsvn_boolean_t is_copy;\n> +\n> +\t/* Temporary file to write delta to along with its checksum */\n> +\tchar *temp_filepath;\n> +\tsvn_checksum_t *checksum;\n\nAll unused.\n\n> +};\n"},{"id":"145043","messageId":"20100707182631.GB2617@burratino","threadId":"24313","inReplyTo":"1278461693-3828-6-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 05/13] Drive the debug editor","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-07T18:26:31Z","receivedAt":"2010-07-07T18:26:31Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> +++ b/dump_editor.c\n> @@ -128,7 +128,7 @@ svn_error_t *get_dump_editor(const svn_delta_editor_t **editor,\n> \tde->close_directory = close_directory;\n> \tde->change_dir_prop = change_dir_prop;\n> \tde->change_file_prop = change_file_prop;\n> -\tde->apply_textdelta = apply_textdelta;\n> +\t/* de->apply_textdelta = apply_textdelta; */\n\nHmm...\n\n[...]\n> +++ b/dumpr_util.h\n> @@ -1,6 +1,11 @@\n> #ifndef DUMPR_UTIL_H_\n> #define DUMPR_UTIL_H_\n> \n> +struct replay_baton {\n> +\tconst svn_delta_editor_t *editor;\n> +\tvoid *baton;\n> +};\n> +\n\nContext during svnsync-like replay ops:\n\n - a diff replayer\n - its context object\n\nMaybe \"void *edit_baton\" would be clearer.\n\n> struct edit_baton {\n\nWhich might involve renaming this to dump_edit_baton to avoid\nconfusion.\n\n> +++ b/svndumpr.c\n> @@ -8,10 +8,40 @@\n[...]\n> +static svn_error_t *replay_revstart(svn_revnum_t revision,\n> + void *replay_baton,\n> + const svn_delta_editor_t **editor,\n> + void **edit_baton,\n> + apr_hash_t *rev_props,\n> + apr_pool_t *pool)\n\nThis function is called to acquire an editor to replay one revision.\n\n> +{\n> +\t/* Extract editor and editor_baton from the replay_baton and\n> +\t set them so that the editor callbacks can use them */\n\nThis comment just paraphrases the code. What in particular requires\nexplanation here?\n\n> +\tstruct replay_baton *rb = replay_baton;\n> +\t*editor = rb->editor;\n> +\t*edit_baton = rb->baton;\n> +\n> +\treturn SVN_NO_ERROR;\n> +}\n\n[...]\n> @@ -47,6 +77,25 @@ svn_error_t *open_connection(const char *url)\n> \n> svn_error_t *replay_range(svn_revnum_t start_revision, svn_revnum_t end_revision)\n> {\n[...]\n> +\tSVN_ERR(svn_cmdline_printf(pool, SVN_REPOS_DUMPFILE_MAGIC_HEADER \": %d\\n\",\n> +\t\t\t\t SVN_REPOS_DUMPFILE_FORMAT_VERSION));\n\nDid this sneak in from a later patch?\n\n> +\tSVN_ERR(svn_ra_replay_range(session, start_revision, end_revision,\n> +\t 0, TRUE, replay_revstart, replay_revend,\n> +\t replay_baton, pool));\n\nMakes sense.\n"},{"id":"145044","messageId":"20100707190434.GA2732@burratino","threadId":"24313","inReplyTo":"1278461693-3828-7-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 06/13] Dump the revprops at the start of every revision","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-07T19:04:34Z","receivedAt":"2010-07-07T19:04:34Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> Fill in replay_revstart to dump the revprops at the start of every\n> revision. Add an additional write_hash_to_stringbuf helper function.\n\n\tA write_hash_to_stringbuf helper does the work of\n\tconverting the property hashtable to dumpfile format.\n\n> +++ b/dumpr_util.c\n[...]\n> +void write_hash_to_stringbuf(apr_hash_t *properties,\n> +\t\t\t svn_boolean_t deleted,\n> +\t\t\t svn_stringbuf_t **strbuf,\n> +\t\t\t apr_pool_t *pool)\n\nThis function looks like:\n\n\tvoid write_hash_to_stringbuf(...\n\t{\n\t\tif (!deleted) {\n\t\t\tfor (each prop) {\n\t\t\t\tappend the new value of the prop;\n\t\t\t}\n\t\t} else {\n\t\t\tfor (each prop) {\n\t\t\t\tmention that it has been deleted;\n\t\t\t}\n\t\t}\n\t}\n\nIt would be simpler to put the body of the loop in its own function,\nlike this:\n\n\tstatic void write_prop_to_stringbuf(...\n\t{\n\t\tif (deleted) {\n\t\t\tappend deletion notice;\n\t\t\treturn;\n\t\t}\n\t\tappend new value of prop;\n\t}\n\n\tvoid write_hash_to_stringbuf(...\n\t{\n\t\tfor (each prop)\n\t\t\twrite_prop_to_stringbuf(...\n\t}\n\nOr even:\n\n\tstatic void write_prop(...\n\tstatic void write_deleted_prop(...\n\n\tvoid write_prop_data_to_stringbuf(...\n\t{\n\t\tfor (each prop)\n\t\t\twrite_prop(...\n\t}\n\tvoid write_deleted_prop_data_to_stringbuf(...\n\t{\n\t\tfor (each prop)\n\t\t\twrite_deleted_prop(...\n\t}\n\nwhich would make the arguments from the caller less opaque.\n\nI did not check whether the \"return early in the simpler case\" is\nidiomatic for svn code. Of course you should respect whatever\nconvention is prevalent.\n\n> +{\n> +\tapr_hash_index_t *this;\n> +\tconst void *key;\n> +\tvoid *val;\n> +\tapr_ssize_t keylen;\n> +\tsvn_string_t *value;\n> +\t\n> +\tif (!deleted) {\n> +\t\tfor (this = apr_hash_first(pool, properties); this;\n> +\t\t this = apr_hash_next(this)) {\n> +\t\t\t/* Get this key and val. */\n> +\t\t\tapr_hash_this(this, &key, &keylen, &val);\n> +\t\t\tvalue = val;\n> +\n> +\t\t\t/* Output name length, then name. */\n> +\t\t\tsvn_stringbuf_appendcstr(*strbuf,\n> +\t\t\t\t\t\t apr_psprintf(pool, \"K %\" APR_SSIZE_T_FMT \"\\n\",\n> +\t\t\t\t\t\t\t keylen));\n> +\n> +\t\t\tsvn_stringbuf_appendbytes(*strbuf, (const char *) key, keylen);\n\nIs the cast needed? (The answer might be \"yes\" if this code is meant\nto be usable with C++ compilers.)\n\n> +++ b/svndumpr.c\n> @@ -23,6 +23,37 @@ static svn_error_t *replay_revstart(svn_revnum_t revision,\n> apr_hash_t *rev_props,\n> apr_pool_t *pool)\n> {\n> +\t/* Editing this revision has just started; dump the revprops\n> +\t before invoking the editor callbacks */\n> +\tsvn_stringbuf_t *propstring = svn_stringbuf_create(\"\", pool);\n> +\tsvn_stream_t *stdout_stream;\n\nStyle: better to say in comments what we are trying to do than what we\nactually do. So:\n\n\t/* First, dump revision properties. */\n\nMaybe dumping revision properties should be its own function to make\nthat comment unnecessary (and make replay_revstart() less daunting as\nit grows).\n\n> +\n> +\t/* Create an stdout stream */\n> +\tsvn_stream_for_stdout(&stdout_stream, pool);\n\nUseless comment.\n\n> +\n> + /* Print revision number and prepare the propstring */\n> +\tSVN_ERR(svn_stream_printf(stdout_stream, pool,\n> +\t\t\t\t SVN_REPOS_DUMPFILE_REVISION_NUMBER\n> +\t\t\t\t \": %ld\\n\", revision));\n> +\twrite_hash_to_stringbuf(rev_props, FALSE, &propstring, pool);\n> +\tsvn_stringbuf_appendbytes(propstring, \"PROPS-END\\n\", 10);\n\nUnhelpful comment. Maybe:\n\n\t/* Revision-number: 19 */\n\tSVN_ERR(svn_stream_printf(stdout_stream, pool,\n\t\t\t\t SVN_REPOS_DUMPFILE_REVISION_NUMBER\n\t\t\t\t \": %ld\\n\", revision));\n\n\twrite_hash_to_stringbuf(rev_props, FALSE, &propstring, pool);\n\tsvn_stringbuf_appendbytes(propstring, \"PROPS-END\\n\", 10);\n\n\t/* Prop-content-length: 13 */\n\tSVN_ERR(svn_stream_printf(stdout_stream, pool,\n\t\t\t\t SVN_REPOS_DUMPFILE_PROP_CONTENT_LENGTH\n\t\t\t\t \": %\" APR_SIZE_T_FMT \"\\n\", propstring->len));\n\t...\n\nThis would make it particularly easy to grep for a particular header\n(even if grepping for SVN_REPOS_DUMPFILE_PROP_CONTENT_LENGTH is not\nthat hard).\n\n[...]\n> +\t/* Print the revprops now */\n> +\tSVN_ERR(svn_stream_write(stdout_stream, propstring->data,\n> +\t\t\t\t &(propstring->len)));\n\nMaybe:\n\n\t/* Property data. */\n\tSVN_ERR(svn_stream_write(stdout_stream, propstring->data,\n\t\t\t\t &(propstring->len)));\n\nThe whole function so far has been about printing revprops.\n\n> +\n> +\tsvn_stream_close(stdout_stream);\n\nThis does not actually fclose(stdout), does it?\n\n> @@ -39,6 +70,9 @@ static svn_error_t *replay_revend(svn_revnum_t revision,\n> apr_hash_t *rev_props,\n> apr_pool_t *pool)\n> {\n> +\t/* Editor has finished for this revision and close_edit has\n> +\t been called; do nothing: just continue to the next\n> +\t revision */\n\nI’d leave out the comment, or:\n\n\t/* No resources to free. */\n\nHTH,\nJonathan\n"},{"id":"145045","messageId":"20100707190813.GA16065@debian","threadId":"24313","inReplyTo":"20100707182631.GB2617@burratino","subject":"Re: [PATCH 05/13] Drive the debug editor","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-07-07T19:08:13Z","receivedAt":"2010-07-07T19:08:13Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Jonathan,\n\nJonathan Nieder writes:\n> Ramkumar Ramachandra wrote:\n> \n> > +++ b/dump_editor.c\n> > @@ -128,7 +128,7 @@ svn_error_t *get_dump_editor(const svn_delta_editor_t **editor,\n> > \tde->close_directory = close_directory;\n> > \tde->change_dir_prop = change_dir_prop;\n> > \tde->change_file_prop = change_file_prop;\n> > -\tde->apply_textdelta = apply_textdelta;\n> > +\t/* de->apply_textdelta = apply_textdelta; */\n> \n> Hmm...\n\nWithout this, the program segfaults because the necessary setup for\napplying a text delta hasn't been set up. Perhaps I should explain\nthis in my commit message?\n\n> [...]\n> > +++ b/dumpr_util.h\n> > @@ -1,6 +1,11 @@\n> > #ifndef DUMPR_UTIL_H_\n> > #define DUMPR_UTIL_H_\n> > \n> > +struct replay_baton {\n> > +\tconst svn_delta_editor_t *editor;\n> > +\tvoid *baton;\n> > +};\n> > +\n> \n> Context during svnsync-like replay ops:\n> \n> - a diff replayer\n> - its context object\n> \n> Maybe \"void *edit_baton\" would be clearer.\n> \n> > struct edit_baton {\n> \n> Which might involve renaming this to dump_edit_baton to avoid\n> confusion.\n\nDone. I renamed both.\n\n> > +++ b/svndumpr.c\n> > @@ -8,10 +8,40 @@\n> [...]\n> > +static svn_error_t *replay_revstart(svn_revnum_t revision,\n> > + void *replay_baton,\n> > + const svn_delta_editor_t **editor,\n> > + void **edit_baton,\n> > + apr_hash_t *rev_props,\n> > + apr_pool_t *pool)\n> \n> This function is called to acquire an editor to replay one revision.\n> \n> > +{\n> > +\t/* Extract editor and editor_baton from the replay_baton and\n> > +\t set them so that the editor callbacks can use them */\n> \n> This comment just paraphrases the code. What in particular requires\n> explanation here?\n\nThis concept took me some time to wrap my head around: I had to stuff\nthe replay_baton with the editor/ editor_baton so that I could set\nthem for use in the callback functions. Comment moved to a later\npatch.\n\n> > +\tstruct replay_baton *rb = replay_baton;\n> > +\t*editor = rb->editor;\n> > +\t*edit_baton = rb->baton;\n> > +\n> > +\treturn SVN_NO_ERROR;\n> > +}\n> \n> [...]\n> > @@ -47,6 +77,25 @@ svn_error_t *open_connection(const char *url)\n> > \n> > svn_error_t *replay_range(svn_revnum_t start_revision, svn_revnum_t end_revision)\n> > {\n> [...]\n> > +\tSVN_ERR(svn_cmdline_printf(pool, SVN_REPOS_DUMPFILE_MAGIC_HEADER \": %d\\n\",\n> > +\t\t\t\t SVN_REPOS_DUMPFILE_FORMAT_VERSION));\n> \n> Did this sneak in from a later patch?\n\nYes. Fixed now. I moved it this change to the next patch.\n\n> > +\tSVN_ERR(svn_ra_replay_range(session, start_revision, end_revision,\n> > +\t 0, TRUE, replay_revstart, replay_revend,\n> > +\t replay_baton, pool));\n> \n> Makes sense.\n\nThanks for the excellent review.\n\n-- Ram\n"},{"id":"145046","messageId":"20100707193017.GA2911@burratino","threadId":"24313","inReplyTo":"20100707170926.GC18806@debian","subject":"Re: [PATCH 02/13] Add skeleton SVN client and Makefile","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-07T19:30:17Z","receivedAt":"2010-07-07T19:30:17Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> Here's a diff of the modifications I made after your review:\n\nThat’s quite helpful.\n\n> +++ b/svndumpr.c\n> @@ -76,31 +76,19 @@ static svn_error_t *replay_revend(svn_revnum_t revision,\n[...]\n> +\t/* Populte ctx->auth_baton with the auth baton\n> +\t non-interactively. Arguments 3, 4 and 5 are for username,\n> +\t password and config_dir which is NULL in this case. Set\n> +\t no_auth_cache and trust_serv_cert to FALSE, don't provide a\n> +\t config, and omit cancel_func/ cancel_baton */\n> \tSVN_ERR(svn_cmdline_create_auth_baton(&(ctx->auth_baton), TRUE,\n> \t\t\t\t\t NULL, NULL, NULL, FALSE,\n> \t\t\t\t\t FALSE, NULL, NULL, NULL,\n\nI think you took my suggestion too seriously here. Such a comment\nprobably will not help people much; instead, maybe a more focused\ncomment can help the curious avoid looking up\nsvn_cmdline_create_auth_baton:\n\n\t/* Default authentication providers for noninteractive\n\t use. */\n\tSVN_ERR(svn_cmdline_create_auth_baton(...\n\nLooking this up, I notice that function was added in svn 1.6.\nHopefully that is okay, since this code is destined for svn trunk.\n\nExcept as noted above,\n\nReviewed-by: Jonathan Nieder \n\nThanks.\n"},{"id":"145047","messageId":"20100707195321.GB2911@burratino","threadId":"24313","inReplyTo":"20100707190813.GA16065@debian","subject":"Re: [PATCH 05/13] Drive the debug editor","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-07T19:53:21Z","receivedAt":"2010-07-07T19:53:21Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n> Jonathan Nieder writes:\n>> Ramkumar Ramachandra wrote:\n\n>>> -\tde->apply_textdelta = apply_textdelta;\n>>> +\t/* de->apply_textdelta = apply_textdelta; */\n[...]\n> Without this, the program segfaults because the necessary setup for\n> applying a text delta hasn't been set up. Perhaps I should explain\n> this in my commit message?\n\nIs the default apply_textdelta not a no-op? What work does it have to\ndo, and can the skeleton editor be convinced to do the same in patch 4?\n\n>>> +{\n>>> +\t/* Extract editor and editor_baton from the replay_baton and\n>>> +\t set them so that the editor callbacks can use them */\n>>\n>> This comment just paraphrases the code. What in particular requires\n>> explanation here?\n>\n> This concept took me some time to wrap my head around: I had to stuff\n> the replay_baton with the editor/ editor_baton so that I could set\n> them for use in the callback functions.\n\nAh, okay. Then I suppose it belongs in the commit message.\n\nAlternatively: why does the tree editor have to persist between calls\nreplaying the various revisions? That information could help the\nreader understand what is going on.\n"},{"id":"145050","messageId":"20100707202426.GD16065@debian","threadId":"24313","inReplyTo":"1278461693-3828-1-git-send-email-artagnon@gmail.com","subject":"Re: [GSoC update] git-remote-svn: Week 10","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-07-07T20:24:26Z","receivedAt":"2010-07-07T20:24:26Z","isPatch":false,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nRamkumar Ramachandra writes:\n> Please note that it has been built and tested only against the\n> Subversion trunk: for Subversion 1.6, you can try using my\n> ra-svn-1.6. Also, there seems to be some unresolved issue on 64-bit\n> systems. We're working on fixing this.\n\nThis is fixed. I'm currently running a large validation on Avar's\nserver.\n\nNOTE to reviewers: The must_dump_props and dump_props_pending\nvariables in patches 7-12 are very confusing. I've fixed this and\nadded a long explanatory note in the latest commits. Please ignore it\nfor the moment and review the rest of the series.\n\n-- Ram\n"},{"id":"145055","messageId":"20100707204702.GE16065@debian","threadId":"24313","inReplyTo":"20100707193017.GA2911@burratino","subject":"Re: [PATCH 02/13] Add skeleton SVN client and Makefile","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-07-07T20:47:02Z","receivedAt":"2010-07-07T20:47:02Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nJonathan Nieder writes:\n> Ramkumar Ramachandra wrote:\n> \n> > Here's a diff of the modifications I made after your review:\n> \n> That’s quite helpful.\n> \n> > +++ b/svndumpr.c\n> > @@ -76,31 +76,19 @@ static svn_error_t *replay_revend(svn_revnum_t revision,\n> [...]\n> > +\t/* Populte ctx->auth_baton with the auth baton\n> > +\t non-interactively. Arguments 3, 4 and 5 are for username,\n> > +\t password and config_dir which is NULL in this case. Set\n> > +\t no_auth_cache and trust_serv_cert to FALSE, don't provide a\n> > +\t config, and omit cancel_func/ cancel_baton */\n> > \tSVN_ERR(svn_cmdline_create_auth_baton(&(ctx->auth_baton), TRUE,\n> > \t\t\t\t\t NULL, NULL, NULL, FALSE,\n> > \t\t\t\t\t FALSE, NULL, NULL, NULL,\n> \n> I think you took my suggestion too seriously here. Such a comment\n> probably will not help people much; instead, maybe a more focused\n> comment can help the curious avoid looking up\n> svn_cmdline_create_auth_baton:\n> \n> \t/* Default authentication providers for noninteractive\n> \t use. */\n> \tSVN_ERR(svn_cmdline_create_auth_baton(...\n\nFixed.\n\n> Looking this up, I notice that function was added in svn 1.6.\n> Hopefully that is okay, since this code is destined for svn trunk.\n\nI have a working 1.6 fork now that I intend to merge into\ngit.git. When there's a new release of Subversion that includes my\npatch, I'll remove it from git.git.\n\n> Reviewed-by: Jonathan Nieder \n\nThanks.\n\n-- Ram\n"},{"id":"145088","messageId":"20100708060453.GA3264@debian","threadId":"24313","inReplyTo":"20100707195321.GB2911@burratino","subject":"Re: [PATCH 05/13] Drive the debug editor","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-07-08T06:04:53Z","receivedAt":"2010-07-08T06:04:53Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Jonathan Nieder writes:\n> Ramkumar Ramachandra wrote:\n> > Jonathan Nieder writes:\n> >> Ramkumar Ramachandra wrote:\n> \n> >>> -\tde->apply_textdelta = apply_textdelta;\n> >>> +\t/* de->apply_textdelta = apply_textdelta; */\n> [...]\n> > Without this, the program segfaults because the necessary setup for\n> > applying a text delta hasn't been set up. Perhaps I should explain\n> > this in my commit message?\n> \n> Is the default apply_textdelta not a no-op? What work does it have to\n> do, and can the skeleton editor be convinced to do the same in patch 4?\n\nThe default editor does this:\n *handler = svn_delta_noop_window_handler;\n *handler_baton = NULL;\n return SVN_NO_ERROR;\n\nFixed.\n\n> >>> +{\n> >>> +\t/* Extract editor and editor_baton from the replay_baton and\n> >>> +\t set them so that the editor callbacks can use them */\n> >>\n> >> This comment just paraphrases the code. What in particular requires\n> >> explanation here?\n> >\n> > This concept took me some time to wrap my head around: I had to stuff\n> > the replay_baton with the editor/ editor_baton so that I could set\n> > them for use in the callback functions.\n> \n> Ah, okay. Then I suppose it belongs in the commit message.\n>\n> Alternatively: why does the tree editor have to persist between calls\n> replaying the various revisions? That information could help the\n> reader understand what is going on.\n\nRight. The editor_baton is the key item that's guaranteed to be passed\naround. Fixed.\n\n-- Ram\n"},{"id":"145090","messageId":"20100708061736.GB3264@debian","threadId":"24313","inReplyTo":"20100707181619.GA2617@burratino","subject":"Re: [PATCH 04/13] Add skeleton dump editor","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-07-08T06:17:36Z","receivedAt":"2010-07-08T06:17:36Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nJonathan Nieder writes:\n> Ramkumar Ramachandra wrote:\n> \n> > Add a dump editor and write out skeleton callback functions according\n> > to the API documentation of svn_delta_editor_t. Also expose\n> > get_dump_editor through a header.\n> \n> This commit message tells me nothing... Maybe:\n> \n> \tAdd a no-op svn_editor. The function to retrieve it is called\n> \tget_dump_editor because it is planned to tweak it to write a\n> \tdumpfile. But for now it is more useful when used with the\n> \tdebug_editor, to get a list of editor operations printed to\n> \tstderr.\n\nFixed.\n\n> It could make sense to squash this with patch 5 as a demo.\n\nHm, I'm not too competent with handling lots of conflicts during an\ninteractive rebase: I'll try this out in a new branch and let you know\nhow it went.\n\n> > --- /dev/null\n> > +++ b/dump_editor.c\n> > @@ -0,0 +1,143 @@\n> > +/* Licensed under a two-clause BSD-style license.\n> > + * See LICENSE for details.\n> > + */\n> > +\n> > +#include \"svn_pools.h\"\n> > +#include \"svn_error.h\"\n> > +#include \"svn_iter.h\"\n> > +#include \"svn_repos.h\"\n> > +#include \"svn_string.h\"\n> > +#include \"svn_dirent_uri.h\"\n> > +#include \"svn_path.h\"\n> > +#include \"svn_time.h\"\n> > +#include \"svn_checksum.h\"\n> > +#include \"svn_props.h\"\n> \n> Are these all needed?\n\nI figured that adding some implies adding some others. Fixed.\n\n> [...]\n> > +svn_error_t *open_root(void *edit_baton,\n> > + svn_revnum_t base_revision,\n> > + apr_pool_t *pool,\n> > + void **root_baton)\n> > +{\n> > +\treturn SVN_NO_ERROR;\n> > +}\n> \n> Might make sense to use\n> \n> \t*root_baton = NULL;\n> \n> for easier debugging.\n> \n> [...]\n> > +svn_error_t *add_directory(const char *path,\n> [...]\n> > +svn_error_t *open_directory(const char *path,\n> [...]\n> > +svn_error_t *add_file(const char *path,\n> [...]\n> > +svn_error_t *open_file(const char *path,\n> [...]\n> > +svn_error_t *apply_textdelta(void *file_baton, const char *base_checksum,\n> \n> Likewise.\n\nFixed. Excellent suggestion.\n\n> [...]\n> > +++ b/dumpr_util.h\n> > @@ -0,0 +1,29 @@\n> > +#ifndef DUMPR_UTIL_H_\n> > +#define DUMPR_UTIL_H_\n> > +\n> > +struct edit_baton {\n> \n> A more specific name might be nice (or might not, depending on the\n> prevailing style in svn; I ought to check but I am too lazy).\n> \n> > +\t/* The stream to dump to: stdout */\n> > +\tsvn_stream_t *stream;\n> > +\n> > +\t/* pool is for per-edit-session allocations */\n> > +\tapr_pool_t *pool;\n> \n> Unused; probably should delay adding these until there is a user to\n> explain them.\n> \n> > +\n> > +\tsvn_revnum_t current_rev;\n> \n> Used.\n> \n> > +\t\n> > +\t/* Store the properties that changed */\n> > +\tapr_hash_t *properties;\n> > +\tapr_hash_t *del_properties; /* Value is always 0x1 */\n> > +\tsvn_stringbuf_t *propstring;\n> > +\n> > +\t/* Path of changed file */\n> > +\tconst char *changed_path;\n> > +\n> > +\t/* Was a copy command issued? */\n> > +\tsvn_boolean_t is_copy;\n> > +\n> > +\t/* Temporary file to write delta to along with its checksum */\n> > +\tchar *temp_filepath;\n> > +\tsvn_checksum_t *checksum;\n> \n> All unused.\n> \n> > +};\n\nHere's the diff after your review. It took me quite a long time to get\nthe interactive rebase right.\n\nAdd skeleton dump editor\n\nAdd a no-op svn_editor and expose the function get_dump_editor; this\ndump editor will later be filled in to write the dumpfile. Currently,\nit is more useful to wrap it in the debug_editor to get a list of\neditor operations printed to stderr.\n\n\ndiff --git a/dump_editor.c b/dump_editor.c\nindex 2fdf93c..e1f3fca 100644\n--- a/dump_editor.c\n+++ b/dump_editor.c\n@@ -3,14 +3,8 @@\n */\n \n #include \"svn_pools.h\"\n-#include \"svn_error.h\"\n-#include \"svn_iter.h\"\n #include \"svn_repos.h\"\n-#include \"svn_string.h\"\n-#include \"svn_dirent_uri.h\"\n #include \"svn_path.h\"\n-#include \"svn_time.h\"\n-#include \"svn_checksum.h\"\n #include \"svn_props.h\"\n \n #include \"dumpr_util.h\"\n@@ -20,6 +14,7 @@ svn_error_t *open_root(void *edit_baton,\n apr_pool_t *pool,\n void **root_baton)\n {\n+\t*root_baton = NULL;\n \treturn SVN_NO_ERROR;\n }\n \n@@ -38,6 +33,7 @@ svn_error_t *add_directory(const char *path,\n apr_pool_t *pool,\n void **child_baton)\n {\n+\t*child_baton = NULL;\n \treturn SVN_NO_ERROR;\n }\n \n@@ -47,6 +43,7 @@ svn_error_t *open_directory(const char *path,\n apr_pool_t *pool,\n void **child_baton)\n {\n+\t*child_baton = NULL;\n \treturn SVN_NO_ERROR;\n }\n \n@@ -63,6 +60,7 @@ svn_error_t *add_file(const char *path,\n apr_pool_t *pool,\n void **file_baton)\n {\n+\t*file_baton = NULL;\n \treturn SVN_NO_ERROR;\n }\n \n@@ -72,6 +70,7 @@ svn_error_t *open_file(const char *path,\n apr_pool_t *pool,\n void **file_baton)\n {\n+\t*file_baton = NULL;\n \treturn SVN_NO_ERROR;\n }\n \n@@ -96,6 +95,8 @@ svn_error_t *apply_textdelta(void *file_baton, const char *base_checksum,\n svn_txdelta_window_handler_t *handler,\n void **handler_baton)\n {\n+\t*handler = svn_delta_noop_window_handler;\n+\t*handler_baton = NULL;\n \treturn SVN_NO_ERROR;\n }\n\ndiff --git a/dumpr_util.h b/dumpr_util.h\nindex d206c19..2906543 100644\n--- a/dumpr_util.h\n+++ b/dumpr_util.h\n@@ -2,28 +2,8 @@\n #define DUMPR_UTIL_H_\n \n struct edit_baton {\n-\t/* The stream to dump to: stdout */\n \tsvn_stream_t *stream;\n-\n-\t/* pool is for per-edit-session allocations */\n-\tapr_pool_t *pool;\n-\n \tsvn_revnum_t current_rev;\n-\t\n-\t/* Store the properties that changed */\n-\tapr_hash_t *properties;\n-\tapr_hash_t *del_properties; /* Value is always 0x1 */\n-\tsvn_stringbuf_t *propstring;\n-\n-\t/* Path of changed file */\n-\tconst char *changed_path;\n-\n-\t/* Was a copy command issued? */\n-\tsvn_boolean_t is_copy;\n-\n-\t/* Temporary file to write delta to along with its checksum */\n-\tchar *temp_filepath;\n-\tsvn_checksum_t *checksum;\n };\n \n #endif\n"},{"id":"145948","messageId":"20100721185513.GB23839@kytes","threadId":"24313","inReplyTo":"20100707190434.GA2732@burratino","subject":"Re: [PATCH 06/13] Dump the revprops at the start of every revision","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-07-21T18:55:13Z","receivedAt":"2010-07-21T18:55:13Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Jonathan,\n\nI stashed this review away while working on some other important\nchanges. I finally got around to responding to this review- sorry that\nit took so long.\n\nJonathan Nieder writes:\n> > Fill in replay_revstart to dump the revprops at the start of every\n> > revision. Add an additional write_hash_to_stringbuf helper function.\n> \n> \tA write_hash_to_stringbuf helper does the work of\n> \tconverting the property hashtable to dumpfile format.\n> \n> > +++ b/dumpr_util.c\n> [...]\n> > +void write_hash_to_stringbuf(apr_hash_t *properties,\n> > +\t\t\t svn_boolean_t deleted,\n> > +\t\t\t svn_stringbuf_t **strbuf,\n> > +\t\t\t apr_pool_t *pool)\n[...]\n\nFixed, but not exactly in the way you've suggested.\n\n> > +\t\t\t/* Output name length, then name. */\n> > +\t\t\tsvn_stringbuf_appendcstr(*strbuf,\n> > +\t\t\t\t\t\t apr_psprintf(pool, \"K %\" APR_SSIZE_T_FMT \"\\n\",\n> > +\t\t\t\t\t\t\t keylen));\n> > +\n> > +\t\t\tsvn_stringbuf_appendbytes(*strbuf, (const char *) key, keylen);\n> \n> Is the cast needed? (The answer might be \"yes\" if this code is meant\n> to be usable with C++ compilers.)\n\nThese casts are all over in the source tree, so I'm guessing the\nanswer is \"yes\".\n\n> Style: better to say in comments what we are trying to do than what we\n> actually do. So:\n> \n> \t/* First, dump revision properties. */\n\nI've fixed all the comments in the entire source tree. Thanks :)\n\n-- Ram\n"},{"id":"146376","messageId":"1280153007.2404.65.camel@edith","threadId":"24313","inReplyTo":"20100721185513.GB23839@kytes","subject":"Re: [PATCH 06/13] Dump the revprops at the start of every revision","fromName":"Julian Foad","fromEmail":"julian.foad@wandisco.com","sentAt":"2010-07-26T14:03:27Z","receivedAt":"2010-07-26T14:03:27Z","isPatch":true,"sender":{"key":"julian.foad@wandisco.com","avatar":null},"body":"On Thu, 2010-07-22, Ramkumar Ramachandra wrote:\n> Jonathan Nieder writes:\n[...]\n> > > +\t\t\t/* Output name length, then name. */\n> > > +\t\t\tsvn_stringbuf_appendcstr(*strbuf,\n> > > +\t\t\t\t\t\t apr_psprintf(pool, \"K %\" APR_SSIZE_T_FMT \"\\n\",\n> > > +\t\t\t\t\t\t\t keylen));\n> > > +\n> > > +\t\t\tsvn_stringbuf_appendbytes(*strbuf, (const char *) key, keylen);\n> > \n> > Is the cast needed? (The answer might be \"yes\" if this code is meant\n> > to be usable with C++ compilers.)\n> \n> These casts are all over in the source tree, so I'm guessing the\n> answer is \"yes\".\n\nActually no - Subversion C code is not intended to be compilable as C++\nand that cast is not needed. (Other casts that you see in Subversion\ncode are for different situations.)\n\n- Julian\n"},{"id":"146400","messageId":"20100726175325.GN17468@kytes","threadId":"24313","inReplyTo":"1280153007.2404.65.camel@edith","subject":"Re: [PATCH 06/13] Dump the revprops at the start of every revision","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-07-26T17:53:27Z","receivedAt":"2010-07-26T17:53:27Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Julian,\n\nJulian Foad writes:\n> On Thu, 2010-07-22, Ramkumar Ramachandra wrote:\n> > Jonathan Nieder writes:\n> [...]\n> > > > +\t\t\t/* Output name length, then name. */\n> > > > +\t\t\tsvn_stringbuf_appendcstr(*strbuf,\n> > > > +\t\t\t\t\t\t apr_psprintf(pool, \"K %\" APR_SSIZE_T_FMT \"\\n\",\n> > > > +\t\t\t\t\t\t\t keylen));\n> > > > +\n> > > > +\t\t\tsvn_stringbuf_appendbytes(*strbuf, (const char *) key, keylen);\n> > > \n> > > Is the cast needed? (The answer might be \"yes\" if this code is meant\n> > > to be usable with C++ compilers.)\n> > \n> > These casts are all over in the source tree, so I'm guessing the\n> > answer is \"yes\".\n> \n> Actually no - Subversion C code is not intended to be compilable as C++\n> and that cast is not needed. (Other casts that you see in Subversion\n> code are for different situations.)\n\nThanks for pointing that out. I'll fix them in my future commits.\n\n-- Ram\n"}]}