{"thread":{"id":"31252","subject":"[PATCH/RFC v3 00/16] GSOC remote-svn","startedAt":"2012-08-14T19:13:02Z","lastAt":"2012-08-15T21:06:20Z","messageCount":36,"participants":["Florian Achleitner","Junio C Hamano","David Michael Barr"],"isPatch":true,"patchVersion":3,"patchTotal":16},"messages":[{"id":"196994","messageId":"1344971598-8213-1-git-send-email-florian.achleitner.2.6.31@gmail.com","threadId":"31252","inReplyTo":null,"subject":"[PATCH/RFC v3 00/16] GSOC remote-svn","fromName":"Florian Achleitner","fromEmail":"florian.achleitner.2.6.31@gmail.com","sentAt":"2012-08-14T19:13:02Z","receivedAt":"2012-08-14T19:13:02Z","isPatch":true,"sender":{"key":"florian.achleitner.2.6.31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/880777?v=4"},"body":"Hi.\n\nVersion 3 of this series adds the 'bidi-import' capability, as suggested\nJonathan. \nDiff details are attached to the patches.\n04 and 05 are completely new.\n\n[PATCH/RFC v3 01/16] Implement a remote helper for svn in C.\n[PATCH/RFC v3 02/16] Integrate remote-svn into svn-fe/Makefile.\n[PATCH/RFC v3 03/16] Add svndump_init_fd to allow reading dumps from\n[PATCH/RFC v3 04/16] Connect fast-import to the remote-helper via\n[PATCH/RFC v3 05/16] Add documentation for the 'bidi-import'\n[PATCH/RFC v3 06/16] remote-svn, vcs-svn: Enable fetching to private\n[PATCH/RFC v3 07/16] Add a symlink 'git-remote-svn' in base dir.\n[PATCH/RFC v3 08/16] Allow reading svn dumps from files via file://\n[PATCH/RFC v3 09/16] vcs-svn: add fast_export_note to create notes\n[PATCH/RFC v3 10/16] Create a note for every imported commit\n[PATCH/RFC v3 11/16] When debug==1, start fast-import with \"--stats\"\n[PATCH/RFC v3 12/16] remote-svn: add incremental import.\n[PATCH/RFC v3 13/16] Add a svnrdump-simulator replaying a dump file\n[PATCH/RFC v3 14/16] transport-helper: add import|export-marks to\n[PATCH/RFC v3 15/16] remote-svn: add marks-file regeneration.\n[PATCH/RFC v3 16/16] Add a test script for remote-svn.\n"},{"id":"196995","messageId":"1344971598-8213-2-git-send-email-florian.achleitner.2.6.31@gmail.com","threadId":"31252","inReplyTo":"1344971598-8213-1-git-send-email-florian.achleitner.2.6.31@gmail.com","subject":"[PATCH/RFC v3 01/16] Implement a remote helper for svn in C.","fromName":"Florian Achleitner","fromEmail":"florian.achleitner.2.6.31@gmail.com","sentAt":"2012-08-14T19:13:03Z","receivedAt":"2012-08-14T19:13:03Z","isPatch":true,"sender":{"key":"florian.achleitner.2.6.31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/880777?v=4"},"body":"Enable basic fetching from subversion repositories. When processing remote URLs\nstarting with svn::, git invokes this remote-helper.\nIt starts svnrdump to extract revisions from the subversion repository in the\n'dump file format', and converts them to a git-fast-import stream using\nthe functions of vcs-svn/.\n\nImported refs are created in a private namespace at refs/svn/<remote-name/master.\nThe revision history is imported linearly (no branch detection) and completely,\ni.e. from revision 0 to HEAD.\n\nThe 'bidi-import' capability is used. The remote-helper expects data from\nfast-import on its stdin. It buffers a batch of 'import' command lines\nin a string_list before starting to process them.\n\nSigned-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n---\ndiff:\n- incorporate review\n- remove redundant strbuf_init\n- add 'bidi-import' to capabilities\n- buffer all lines of a command batch in string_list\n\n contrib/svn-fe/remote-svn.c |  183 +++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 183 insertions(+)\n create mode 100644 contrib/svn-fe/remote-svn.c\n\ndiff --git a/contrib/svn-fe/remote-svn.c b/contrib/svn-fe/remote-svn.c\nnew file mode 100644\nindex 0000000..ce59344\n--- /dev/null\n+++ b/contrib/svn-fe/remote-svn.c\n@@ -0,0 +1,183 @@\n+\n+#include \"cache.h\"\n+#include \"remote.h\"\n+#include \"strbuf.h\"\n+#include \"url.h\"\n+#include \"exec_cmd.h\"\n+#include \"run-command.h\"\n+#include \"svndump.h\"\n+#include \"notes.h\"\n+#include \"argv-array.h\"\n+\n+static const char *url;\n+static const char *private_ref;\n+static const char *remote_ref = \"refs/heads/master\";\n+\n+static int cmd_capabilities(const char *line);\n+static int cmd_import(const char *line);\n+static int cmd_list(const char *line);\n+\n+typedef int (*input_command_handler)(const char *);\n+struct input_command_entry {\n+\tconst char *name;\n+\tinput_command_handler fct;\n+\tunsigned char batchable;\t/* whether the command starts or is part of a batch */\n+};\n+\n+static const struct input_command_entry input_command_list[] = {\n+\t\t{ \"capabilities\", cmd_capabilities, 0 },\n+\t\t{ \"import\", cmd_import, 1 },\n+\t\t{ \"list\", cmd_list, 0 },\n+\t\t{ NULL, NULL }\n+};\n+\n+static int cmd_capabilities(const char *line) {\n+\tprintf(\"import\\n\");\n+\tprintf(\"bidi-import\\n\");\n+\tprintf(\"refspec %s:%s\\n\\n\", remote_ref, private_ref);\n+\tfflush(stdout);\n+\treturn 0;\n+}\n+\n+static void terminate_batch(void)\n+{\n+\t/* terminate a current batch's fast-import stream */\n+\t\tprintf(\"done\\n\");\n+\t\tfflush(stdout);\n+}\n+\n+static int cmd_import(const char *line)\n+{\n+\tint code;\n+\tint dumpin_fd;\n+\tunsigned int startrev = 0;\n+\tstruct argv_array svndump_argv = ARGV_ARRAY_INIT;\n+\tstruct child_process svndump_proc;\n+\n+\tmemset(&svndump_proc, 0, sizeof (struct child_process));\n+\tsvndump_proc.out = -1;\n+\targv_array_push(&svndump_argv, \"svnrdump\");\n+\targv_array_push(&svndump_argv, \"dump\");\n+\targv_array_push(&svndump_argv, url);\n+\targv_array_pushf(&svndump_argv, \"-r%u:HEAD\", startrev);\n+\tsvndump_proc.argv = svndump_argv.argv;\n+\n+\tcode = start_command(&svndump_proc);\n+\tif (code)\n+\t\tdie(\"Unable to start %s, code %d\", svndump_proc.argv[0], code);\n+\tdumpin_fd = svndump_proc.out;\n+\n+\tcode = start_command(&svndump_proc);\n+\tif (code)\n+\t\tdie(\"Unable to start %s, code %d\", svndump_proc.argv[0], code);\n+\tdumpin_fd = svndump_proc.out;\n+\n+\tsvndump_init_fd(dumpin_fd, STDIN_FILENO);\n+\tsvndump_read(url, private_ref);\n+\tsvndump_deinit();\n+\tsvndump_reset();\n+\n+\tclose(dumpin_fd);\n+\tcode = finish_command(&svndump_proc);\n+\tif (code)\n+\t\twarning(\"%s, returned %d\", svndump_proc.argv[0], code);\n+\targv_array_clear(&svndump_argv);\n+\n+\treturn 0;\n+}\n+\n+static int cmd_list(const char *line)\n+{\n+\tprintf(\"? %s\\n\\n\", remote_ref);\n+\tfflush(stdout);\n+\treturn 0;\n+}\n+\n+static int do_command(struct strbuf *line)\n+{\n+\tconst struct input_command_entry *p = input_command_list;\n+\tstatic struct string_list batchlines = STRING_LIST_INIT_DUP;\n+\tstatic const struct input_command_entry *batch_cmd;\n+\t/*\n+\t * commands can be grouped together in a batch.\n+\t * Batches are ended by \\n. If no batch is active the program ends.\n+\t * During a batch all lines are buffered and passed to the handler function\n+\t * when the batch is terminated.\n+\t */\n+\tif (line->len == 0) {\n+\t\tif (batch_cmd) {\n+\t\t\tstruct string_list_item *item;\n+\t\t\tfor_each_string_list_item(item, &batchlines)\n+\t\t\t\tbatch_cmd->fct(item->string);\n+\t\t\tterminate_batch();\n+\t\t\tbatch_cmd = NULL;\n+\t\t\tstring_list_clear(&batchlines, 0);\n+\t\t\treturn 0;\t/* end of the batch, continue reading other commands. */\n+\t\t}\n+\t\treturn 1;\t/* end of command stream, quit */\n+\t}\n+\tif (batch_cmd) {\n+\t\tif (strcmp(batch_cmd->name, line->buf))\n+\t\t\tdie(\"Active %s batch interrupted by %s\", batch_cmd->name, line->buf);\n+\t\t/* buffer batch lines */\n+\t\tstring_list_append(&batchlines, line->buf);\n+\t\treturn 0;\n+\t}\n+\n+\tfor(p = input_command_list; p->name; p++) {\n+\t\tif (!prefixcmp(line->buf, p->name) &&\n+\t\t\t\t(strlen(p->name) == line->len || line->buf[strlen(p->name)] == ' ')) {\n+\t\t\tif (p->batchable) {\n+\t\t\t\tbatch_cmd = p;\n+\t\t\t\tstring_list_append(&batchlines, line->buf);\n+\t\t\t\treturn 0;\n+\t\t\t}\n+\t\t\treturn p->fct(line->buf);\n+\t\t}\n+\t}\n+\twarning(\"Unknown command '%s'\\n\", line->buf);\n+\treturn 0;\n+}\n+\n+int main(int argc, const char **argv)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tint nongit;\n+\tstatic struct remote *remote;\n+\tconst char *url_in;\n+\n+\tgit_extract_argv0_path(argv[0]);\n+\tsetup_git_directory_gently(&nongit);\n+\tif (argc < 2 || argc > 3) {\n+\t\tusage(\"git-remote-svn <remote-name> [<url>]\");\n+\t\treturn 1;\n+\t}\n+\n+\tremote = remote_get(argv[1]);\n+\turl_in = remote->url[0];\n+\tif (argc == 3)\n+\t\turl_in = argv[2];\n+\n+\tend_url_with_slash(&buf, url_in);\n+\turl = strbuf_detach(&buf, NULL);\n+\n+\tstrbuf_addf(&buf, \"refs/svn/%s/master\", remote->name);\n+\tprivate_ref = strbuf_detach(&buf, NULL);\n+\n+\twhile(1) {\n+\t\tif (strbuf_getline(&buf, stdin, '\\n') == EOF) {\n+\t\t\tif (ferror(stdin))\n+\t\t\t\tdie(\"Error reading command stream\");\n+\t\t\telse\n+\t\t\t\tdie(\"Unexpected end of command stream\");\n+\t\t}\n+\t\tif (do_command(&buf))\n+\t\t\tbreak;\n+\t\tstrbuf_reset(&buf);\n+\t}\n+\n+\tstrbuf_release(&buf);\n+\tfree((void*)url);\n+\tfree((void*)private_ref);\n+\treturn 0;\n+}\n-- \n1.7.9.5\n"},{"id":"196996","messageId":"1344971598-8213-3-git-send-email-florian.achleitner.2.6.31@gmail.com","threadId":"31252","inReplyTo":"1344971598-8213-2-git-send-email-florian.achleitner.2.6.31@gmail.com","subject":"[PATCH/RFC v3 02/16] Integrate remote-svn into svn-fe/Makefile.","fromName":"Florian Achleitner","fromEmail":"florian.achleitner.2.6.31@gmail.com","sentAt":"2012-08-14T19:13:04Z","receivedAt":"2012-08-14T19:13:04Z","isPatch":true,"sender":{"key":"florian.achleitner.2.6.31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/880777?v=4"},"body":"Requires some sha.h to be used and the libraries\nto be linked, this is currently hardcoded.\n\nSigned-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n---\n contrib/svn-fe/Makefile |   16 ++++++++++------\n 1 file changed, 10 insertions(+), 6 deletions(-)\n\ndiff --git a/contrib/svn-fe/Makefile b/contrib/svn-fe/Makefile\nindex 360d8da..8f0eec2 100644\n--- a/contrib/svn-fe/Makefile\n+++ b/contrib/svn-fe/Makefile\n@@ -1,14 +1,14 @@\n-all:: svn-fe$X\n+all:: svn-fe$X remote-svn$X\n \n CC = gcc\n RM = rm -f\n MV = mv\n \n-CFLAGS = -g -O2 -Wall\n+CFLAGS = -g -O2 -Wall -DSHA1_HEADER='<openssl/sha.h>' -Wdeclaration-after-statement\n LDFLAGS =\n ALL_CFLAGS = $(CFLAGS)\n ALL_LDFLAGS = $(LDFLAGS)\n-EXTLIBS =\n+EXTLIBS = -lssl -lcrypto -lpthread ../../xdiff/lib.a\n \n GIT_LIB = ../../libgit.a\n VCSSVN_LIB = ../../vcs-svn/lib.a\n@@ -37,8 +37,12 @@ svn-fe$X: svn-fe.o $(VCSSVN_LIB) $(GIT_LIB)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ svn-fe.o \\\n \t\t$(ALL_LDFLAGS) $(LIBS)\n \n-svn-fe.o: svn-fe.c ../../vcs-svn/svndump.h\n-\t$(QUIET_CC)$(CC) -I../../vcs-svn -o $*.o -c $(ALL_CFLAGS) $<\n+remote-svn$X: remote-svn.o $(VCSSVN_LIB) $(GIT_LIB)\n+\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ remote-svn.o \\\n+\t\t$(ALL_LDFLAGS) $(LIBS)\n+\t\t\n+%.o: %.c ../../vcs-svn/svndump.h\n+\t$(QUIET_CC)$(CC) -I../../vcs-svn -I../../ -o $*.o -c $(ALL_CFLAGS) $<\n \n svn-fe.html: svn-fe.txt\n \t$(QUIET_SUBDIR0)../../Documentation $(QUIET_SUBDIR1) \\\n@@ -58,6 +62,6 @@ svn-fe.1: svn-fe.txt\n \t$(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) libgit.a\n \n clean:\n-\t$(RM) svn-fe$X svn-fe.o svn-fe.html svn-fe.xml svn-fe.1\n+\t$(RM) svn-fe$X svn-fe.o svn-fe.html svn-fe.xml svn-fe.1 remote-svn.o\n \n .PHONY: all clean FORCE\n-- \n1.7.9.5\n"},{"id":"196997","messageId":"1344971598-8213-4-git-send-email-florian.achleitner.2.6.31@gmail.com","threadId":"31252","inReplyTo":"1344971598-8213-3-git-send-email-florian.achleitner.2.6.31@gmail.com","subject":"[PATCH/RFC v3 03/16] Add svndump_init_fd to allow reading dumps from arbitrary FDs.","fromName":"Florian Achleitner","fromEmail":"florian.achleitner.2.6.31@gmail.com","sentAt":"2012-08-14T19:13:05Z","receivedAt":"2012-08-14T19:13:05Z","isPatch":true,"sender":{"key":"florian.achleitner.2.6.31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/880777?v=4"},"body":"The existing function only allows reading from a filename or\nfrom stdin. Allow passing of a FD and an additional FD for\nthe back report pipe. This allows us to retrieve the name of\nthe pipe in the caller.\n\nFixes the filename could be NULL bug.\n\nSigned-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n---\n- dup input file descriptor, because buffer_deinit closes the fd.\n vcs-svn/svndump.c |   22 ++++++++++++++++++----\n vcs-svn/svndump.h |    1 +\n 2 files changed, 19 insertions(+), 4 deletions(-)\n\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex 2b168ae..d81a078 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -468,11 +468,9 @@ void svndump_read(const char *url)\n \t\tend_revision();\n }\n \n-int svndump_init(const char *filename)\n+static void init(int report_fd)\n {\n-\tif (buffer_init(&input, filename))\n-\t\treturn error(\"cannot open %s: %s\", filename, strerror(errno));\n-\tfast_export_init(REPORT_FILENO);\n+\tfast_export_init(report_fd);\n \tstrbuf_init(&dump_ctx.uuid, 4096);\n \tstrbuf_init(&dump_ctx.url, 4096);\n \tstrbuf_init(&rev_ctx.log, 4096);\n@@ -482,6 +480,22 @@ int svndump_init(const char *filename)\n \treset_dump_ctx(NULL);\n \treset_rev_ctx(0);\n \treset_node_ctx(NULL);\n+\treturn;\n+}\n+\n+int svndump_init(const char *filename)\n+{\n+\tif (buffer_init(&input, filename))\n+\t\treturn error(\"cannot open %s: %s\", filename ? filename : \"NULL\", strerror(errno));\n+\tinit(REPORT_FILENO);\n+\treturn 0;\n+}\n+\n+int svndump_init_fd(int in_fd, int back_fd)\n+{\n+\tif(buffer_fdinit(&input, xdup(in_fd)))\n+\t\treturn error(\"cannot open fd %d: %s\", in_fd, strerror(errno));\n+\tinit(xdup(back_fd));\n \treturn 0;\n }\n \ndiff --git a/vcs-svn/svndump.h b/vcs-svn/svndump.h\nindex df9ceb0..acb5b47 100644\n--- a/vcs-svn/svndump.h\n+++ b/vcs-svn/svndump.h\n@@ -2,6 +2,7 @@\n #define SVNDUMP_H_\n \n int svndump_init(const char *filename);\n+int svndump_init_fd(int in_fd, int back_fd);\n void svndump_read(const char *url);\n void svndump_deinit(void);\n void svndump_reset(void);\n-- \n1.7.9.5\n"},{"id":"197000","messageId":"1344971598-8213-5-git-send-email-florian.achleitner.2.6.31@gmail.com","threadId":"31252","inReplyTo":"1344971598-8213-4-git-send-email-florian.achleitner.2.6.31@gmail.com","subject":"[PATCH/RFC v3 04/16] Connect fast-import to the remote-helper via pipe, adding 'bidi-import' capability.","fromName":"Florian Achleitner","fromEmail":"florian.achleitner.2.6.31@gmail.com","sentAt":"2012-08-14T19:13:06Z","receivedAt":"2012-08-14T19:13:06Z","isPatch":true,"sender":{"key":"florian.achleitner.2.6.31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/880777?v=4"},"body":"The fast-import commands 'cat-blob' and 'ls' can be used by remote-helpers\nto retrieve information about blobs and trees that already exist in\nfast-import's memory. This requires a channel from fast-import to the\nremote-helper.\nremote-helpers that use this features shall advertise the new 'bidi-import'\ncapability so signal that they require the communication channel.\nWhen forking fast-import in transport-helper.c connect it to a dup of\nthe remote-helper's stdin-pipe. The additional file descriptor is passed\nto fast-import via it's command line (--cat-blob-fd).\nIt follows that git and fast-import are connected to the remote-helpers's\nstdin.\nBecause git can send multiple commands to the remote-helper on it's stdin,\nit is required that helpers that advertise 'bidi-import' buffer all input\ncommands until the batch of 'import' commands is ended by a newline\nbefore sending data to fast-import.\nThis is to prevent mixing commands and fast-import responses on the\nhelper's stdin.\n\nSigned-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n---\n transport-helper.c |   45 ++++++++++++++++++++++++++++++++-------------\n 1 file changed, 32 insertions(+), 13 deletions(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex cfe0988..257274b 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -10,6 +10,7 @@\n #include \"string-list.h\"\n #include \"thread-utils.h\"\n #include \"sigchain.h\"\n+#include \"argv-array.h\"\n \n static int debug;\n \n@@ -19,6 +20,7 @@ struct helper_data {\n \tFILE *out;\n \tunsigned fetch : 1,\n \t\timport : 1,\n+\t\tbidi_import : 1,\n \t\texport : 1,\n \t\toption : 1,\n \t\tpush : 1,\n@@ -101,6 +103,7 @@ static void do_take_over(struct transport *transport)\n static struct child_process *get_helper(struct transport *transport)\n {\n \tstruct helper_data *data = transport->data;\n+\tstruct argv_array argv = ARGV_ARRAY_INIT;\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct child_process *helper;\n \tconst char **refspecs = NULL;\n@@ -122,11 +125,10 @@ static struct child_process *get_helper(struct transport *transport)\n \thelper->in = -1;\n \thelper->out = -1;\n \thelper->err = 0;\n-\thelper->argv = xcalloc(4, sizeof(*helper->argv));\n-\tstrbuf_addf(&buf, \"git-remote-%s\", data->name);\n-\thelper->argv[0] = strbuf_detach(&buf, NULL);\n-\thelper->argv[1] = transport->remote->name;\n-\thelper->argv[2] = remove_ext_force(transport->url);\n+\targv_array_pushf(&argv, \"git-remote-%s\", data->name);\n+\targv_array_push(&argv, transport->remote->name);\n+\targv_array_push(&argv, remove_ext_force(transport->url));\n+\thelper->argv = argv.argv;\n \thelper->git_cmd = 0;\n \thelper->silent_exec_failure = 1;\n \n@@ -141,6 +143,8 @@ static struct child_process *get_helper(struct transport *transport)\n \n \tdata->helper = helper;\n \tdata->no_disconnect_req = 0;\n+\tfree((void*) helper_env[1]);\n+\targv_array_clear(&argv);\n \n \t/*\n \t * Open the output as FILE* so strbuf_getline() can be used.\n@@ -178,6 +182,8 @@ static struct child_process *get_helper(struct transport *transport)\n \t\t\tdata->push = 1;\n \t\telse if (!strcmp(capname, \"import\"))\n \t\t\tdata->import = 1;\n+\t\telse if (!strcmp(capname, \"bidi-import\"))\n+\t\t\tdata->bidi_import = 1;\n \t\telse if (!strcmp(capname, \"export\"))\n \t\t\tdata->export = 1;\n \t\telse if (!data->refspecs && !prefixcmp(capname, \"refspec \")) {\n@@ -241,8 +247,6 @@ static int disconnect_helper(struct transport *transport)\n \t\tclose(data->helper->out);\n \t\tfclose(data->out);\n \t\tres = finish_command(data->helper);\n-\t\tfree((char *)data->helper->argv[0]);\n-\t\tfree(data->helper->argv);\n \t\tfree(data->helper);\n \t\tdata->helper = NULL;\n \t}\n@@ -376,14 +380,24 @@ static int fetch_with_fetch(struct transport *transport,\n static int get_importer(struct transport *transport, struct child_process *fastimport)\n {\n \tstruct child_process *helper = get_helper(transport);\n+\tstruct helper_data *data = transport->data;\n+\tstruct argv_array argv = ARGV_ARRAY_INIT;\n+\tint cat_blob_fd, code;\n \tmemset(fastimport, 0, sizeof(*fastimport));\n \tfastimport->in = helper->out;\n-\tfastimport->argv = xcalloc(5, sizeof(*fastimport->argv));\n-\tfastimport->argv[0] = \"fast-import\";\n-\tfastimport->argv[1] = \"--quiet\";\n+\targv_array_push(&argv, \"fast-import\");\n+\targv_array_push(&argv, \"--quiet\");\n \n+\tif (data->bidi_import) {\n+\t\tcat_blob_fd = xdup(helper->in);\n+\t\targv_array_pushf(&argv, \"--cat-blob-fd=%d\", cat_blob_fd);\n+\t}\n+\tfastimport->argv = argv.argv;\n \tfastimport->git_cmd = 1;\n-\treturn start_command(fastimport);\n+\n+\tcode = start_command(fastimport);\n+\targv_array_clear(&argv);\n+\treturn code;\n }\n \n static int get_exporter(struct transport *transport,\n@@ -438,11 +452,16 @@ static int fetch_with_import(struct transport *transport,\n \t}\n \n \twrite_constant(data->helper->in, \"\\n\");\n+\t/*\n+\t * remote-helpers that advertise the bidi-import capability are required to\n+\t * buffer the complete batch of import commands until this newline before\n+\t * sending data to fast-import.\n+\t * These helpers read back data from fast-import on their stdin, which could\n+\t * be mixed with import commands, otherwise.\n+\t */\n \n \tif (finish_command(&fastimport))\n \t\tdie(\"Error while running fast-import\");\n-\tfree(fastimport.argv);\n-\tfastimport.argv = NULL;\n \n \t/*\n \t * The fast-import stream of a remote helper that advertises\n-- \n1.7.9.5\n"},{"id":"196999","messageId":"1344971598-8213-6-git-send-email-florian.achleitner.2.6.31@gmail.com","threadId":"31252","inReplyTo":"1344971598-8213-5-git-send-email-florian.achleitner.2.6.31@gmail.com","subject":"[PATCH/RFC v3 05/16] Add documentation for the 'bidi-import' capability of remote-helpers.","fromName":"Florian Achleitner","fromEmail":"florian.achleitner.2.6.31@gmail.com","sentAt":"2012-08-14T19:13:07Z","receivedAt":"2012-08-14T19:13:07Z","isPatch":true,"sender":{"key":"florian.achleitner.2.6.31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/880777?v=4"},"body":"Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n---\n Documentation/git-remote-helpers.txt |   21 ++++++++++++++++++++-\n 1 file changed, 20 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt\nindex f5836e4..5faa48e 100644\n--- a/Documentation/git-remote-helpers.txt\n+++ b/Documentation/git-remote-helpers.txt\n@@ -98,6 +98,20 @@ advertised with this capability must cover all refs reported by\n the list command.  If no 'refspec' capability is advertised,\n there is an implied `refspec *:*`.\n \n+'bidi-import'::\n+\tThe fast-import commands 'cat-blob' and 'ls' can be used by remote-helpers\n+    to retrieve information about blobs and trees that already exist in\n+    fast-import's memory. This requires a channel from fast-import to the\n+    remote-helper.\n+    If it is advertised in addition to \"import\", git establishes a pipe from\n+\tfast-import to the remote-helper's stdin.\n+\tIt follows that git and fast-import are both connected to the\n+\tremote-helper's stdin. Because git can send multiple commands to\n+\tthe remote-helper it is required that helpers that use 'bidi-import'\n+\tbuffer all 'import' commands of a batch before sending data to fast-import.\n+    This is to prevent mixing commands and fast-import responses on the\n+    helper's stdin.\n+\n Capabilities for Pushing\n ~~~~~~~~~~~~~~~~~~~~~~~~\n 'connect'::\n@@ -286,7 +300,12 @@ terminated with a blank line. For each batch of 'import', the remote\n helper should produce a fast-import stream terminated by a 'done'\n command.\n +\n-Supported if the helper has the \"import\" capability.\n+Note that if the 'bidi-import' capability is used the complete batch\n+sequence has to be buffered before starting to send data to fast-import\n+to prevent mixing of commands and fast-import responses on the helper's\n+stdin.\n++\n+Supported if the helper has the 'import' capability.\n \n 'connect' <service>::\n \tConnects to given service. Standard input and standard output\n-- \n1.7.9.5\n"},{"id":"196998","messageId":"1344971598-8213-7-git-send-email-florian.achleitner.2.6.31@gmail.com","threadId":"31252","inReplyTo":"1344971598-8213-6-git-send-email-florian.achleitner.2.6.31@gmail.com","subject":"[PATCH/RFC v3 06/16] remote-svn, vcs-svn: Enable fetching to private refs.","fromName":"Florian Achleitner","fromEmail":"florian.achleitner.2.6.31@gmail.com","sentAt":"2012-08-14T19:13:08Z","receivedAt":"2012-08-14T19:13:08Z","isPatch":true,"sender":{"key":"florian.achleitner.2.6.31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/880777?v=4"},"body":"The reference to update by the fast-import stream is hard-coded.\nWhen fetching from a remote the remote-helper shall update refs\nin a private namespace, i.e. a private subdir of refs/.\nThis namespace is defined by the 'refspec' capability, that the\nremote-helper advertises as a reply to the 'capablilities' command.\n\nExtend svndump and fast-export to allow passing the target ref.\nUpdate svn-fe to be compatible.\n\nSigned-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n---\n- fix hard-coded ref in test-svn-fe.c. Broke a testcase.\n\n contrib/svn-fe/svn-fe.c |    2 +-\n test-svn-fe.c           |    2 +-\n vcs-svn/fast_export.c   |    4 ++--\n vcs-svn/fast_export.h   |    2 +-\n vcs-svn/svndump.c       |   14 +++++++-------\n vcs-svn/svndump.h       |    2 +-\n 6 files changed, 13 insertions(+), 13 deletions(-)\n\ndiff --git a/contrib/svn-fe/svn-fe.c b/contrib/svn-fe/svn-fe.c\nindex 35db24f..c796cc0 100644\n--- a/contrib/svn-fe/svn-fe.c\n+++ b/contrib/svn-fe/svn-fe.c\n@@ -10,7 +10,7 @@ int main(int argc, char **argv)\n {\n \tif (svndump_init(NULL))\n \t\treturn 1;\n-\tsvndump_read((argc > 1) ? argv[1] : NULL);\n+\tsvndump_read((argc > 1) ? argv[1] : NULL, \"refs/heads/master\");\n \tsvndump_deinit();\n \tsvndump_reset();\n \treturn 0;\ndiff --git a/test-svn-fe.c b/test-svn-fe.c\nindex 83633a2..cb0d80f 100644\n--- a/test-svn-fe.c\n+++ b/test-svn-fe.c\n@@ -40,7 +40,7 @@ int main(int argc, char *argv[])\n \tif (argc == 2) {\n \t\tif (svndump_init(argv[1]))\n \t\t\treturn 1;\n-\t\tsvndump_read(NULL);\n+\t\tsvndump_read(NULL, \"refs/heads/master\");\n \t\tsvndump_deinit();\n \t\tsvndump_reset();\n \t\treturn 0;\ndiff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c\nindex 1f04697..11f8f94 100644\n--- a/vcs-svn/fast_export.c\n+++ b/vcs-svn/fast_export.c\n@@ -72,7 +72,7 @@ static char gitsvnline[MAX_GITSVN_LINE_LEN];\n void fast_export_begin_commit(uint32_t revision, const char *author,\n \t\t\tconst struct strbuf *log,\n \t\t\tconst char *uuid, const char *url,\n-\t\t\tunsigned long timestamp)\n+\t\t\tunsigned long timestamp, const char *local_ref)\n {\n \tstatic const struct strbuf empty = STRBUF_INIT;\n \tif (!log)\n@@ -84,7 +84,7 @@ void fast_export_begin_commit(uint32_t revision, const char *author,\n \t} else {\n \t\t*gitsvnline = '\\0';\n \t}\n-\tprintf(\"commit refs/heads/master\\n\");\n+\tprintf(\"commit %s\\n\", local_ref);\n \tprintf(\"mark :%\"PRIu32\"\\n\", revision);\n \tprintf(\"committer %s <%s@%s> %ld +0000\\n\",\n \t\t   *author ? author : \"nobody\",\ndiff --git a/vcs-svn/fast_export.h b/vcs-svn/fast_export.h\nindex 8823aca..17eb13b 100644\n--- a/vcs-svn/fast_export.h\n+++ b/vcs-svn/fast_export.h\n@@ -11,7 +11,7 @@ void fast_export_delete(const char *path);\n void fast_export_modify(const char *path, uint32_t mode, const char *dataref);\n void fast_export_begin_commit(uint32_t revision, const char *author,\n \t\t\tconst struct strbuf *log, const char *uuid,\n-\t\t\tconst char *url, unsigned long timestamp);\n+\t\t\tconst char *url, unsigned long timestamp, const char *local_ref);\n void fast_export_end_commit(uint32_t revision);\n void fast_export_data(uint32_t mode, off_t len, struct line_buffer *input);\n void fast_export_blob_delta(uint32_t mode,\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex d81a078..288bb42 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -299,22 +299,22 @@ static void handle_node(void)\n \t\t\t\tnode_ctx.text_length, &input);\n }\n \n-static void begin_revision(void)\n+static void begin_revision(const char *remote_ref)\n {\n \tif (!rev_ctx.revision)\t/* revision 0 gets no git commit. */\n \t\treturn;\n \tfast_export_begin_commit(rev_ctx.revision, rev_ctx.author.buf,\n \t\t&rev_ctx.log, dump_ctx.uuid.buf, dump_ctx.url.buf,\n-\t\trev_ctx.timestamp);\n+\t\trev_ctx.timestamp, remote_ref);\n }\n \n-static void end_revision(void)\n+static void end_revision()\n {\n \tif (rev_ctx.revision)\n \t\tfast_export_end_commit(rev_ctx.revision);\n }\n \n-void svndump_read(const char *url)\n+void svndump_read(const char *url, const char *local_ref)\n {\n \tchar *val;\n \tchar *t;\n@@ -353,7 +353,7 @@ void svndump_read(const char *url)\n \t\t\tif (active_ctx == NODE_CTX)\n \t\t\t\thandle_node();\n \t\t\tif (active_ctx == REV_CTX)\n-\t\t\t\tbegin_revision();\n+\t\t\t\tbegin_revision(local_ref);\n \t\t\tif (active_ctx != DUMP_CTX)\n \t\t\t\tend_revision();\n \t\t\tactive_ctx = REV_CTX;\n@@ -366,7 +366,7 @@ void svndump_read(const char *url)\n \t\t\t\tif (active_ctx == NODE_CTX)\n \t\t\t\t\thandle_node();\n \t\t\t\tif (active_ctx == REV_CTX)\n-\t\t\t\t\tbegin_revision();\n+\t\t\t\t\tbegin_revision(local_ref);\n \t\t\t\tactive_ctx = NODE_CTX;\n \t\t\t\treset_node_ctx(val);\n \t\t\t\tbreak;\n@@ -463,7 +463,7 @@ void svndump_read(const char *url)\n \tif (active_ctx == NODE_CTX)\n \t\thandle_node();\n \tif (active_ctx == REV_CTX)\n-\t\tbegin_revision();\n+\t\tbegin_revision(local_ref);\n \tif (active_ctx != DUMP_CTX)\n \t\tend_revision();\n }\ndiff --git a/vcs-svn/svndump.h b/vcs-svn/svndump.h\nindex acb5b47..febeecb 100644\n--- a/vcs-svn/svndump.h\n+++ b/vcs-svn/svndump.h\n@@ -3,7 +3,7 @@\n \n int svndump_init(const char *filename);\n int svndump_init_fd(int in_fd, int back_fd);\n-void svndump_read(const char *url);\n+void svndump_read(const char *url, const char *local_ref);\n void svndump_deinit(void);\n void svndump_reset(void);\n \n-- \n1.7.9.5\n"},{"id":"197001","messageId":"1344971598-8213-8-git-send-email-florian.achleitner.2.6.31@gmail.com","threadId":"31252","inReplyTo":"1344971598-8213-7-git-send-email-florian.achleitner.2.6.31@gmail.com","subject":"[PATCH/RFC v3 07/16] Add a symlink 'git-remote-svn' in base dir.","fromName":"Florian Achleitner","fromEmail":"florian.achleitner.2.6.31@gmail.com","sentAt":"2012-08-14T19:13:09Z","receivedAt":"2012-08-14T19:13:09Z","isPatch":true,"sender":{"key":"florian.achleitner.2.6.31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/880777?v=4"},"body":"Allow execution of git-remote-svn even if the binary\ncurrently is located in contrib/svn-fe/.\n\nSigned-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n---\n git-remote-svn |    1 +\n 1 file changed, 1 insertion(+)\n create mode 120000 git-remote-svn\n\ndiff --git a/git-remote-svn b/git-remote-svn\nnew file mode 120000\nindex 0000000..d3b1c07\n--- /dev/null\n+++ b/git-remote-svn\n@@ -0,0 +1 @@\n+contrib/svn-fe/remote-svn\n\\ No newline at end of file\n-- \n1.7.9.5\n"},{"id":"197002","messageId":"1344971598-8213-9-git-send-email-florian.achleitner.2.6.31@gmail.com","threadId":"31252","inReplyTo":"1344971598-8213-8-git-send-email-florian.achleitner.2.6.31@gmail.com","subject":"[PATCH/RFC v3 08/16] Allow reading svn dumps from files via file:// urls.","fromName":"Florian Achleitner","fromEmail":"florian.achleitner.2.6.31@gmail.com","sentAt":"2012-08-14T19:13:10Z","receivedAt":"2012-08-14T19:13:10Z","isPatch":true,"sender":{"key":"florian.achleitner.2.6.31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/880777?v=4"},"body":"For testing as well as for importing large, already\navailable dumps, it's useful to bypass svnrdump and\nreplay the svndump from a file directly.\n\nAdd support for file:// urls in the remote url.\ne.g. svn::file:///path/to/dump\nWhen the remote helper finds an url starting with\nfile:// it tries to open that file instead of invoking svnrdump.\n\nSigned-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n---\n contrib/svn-fe/remote-svn.c |   59 ++++++++++++++++++++++++++-----------------\n 1 file changed, 36 insertions(+), 23 deletions(-)\n\ndiff --git a/contrib/svn-fe/remote-svn.c b/contrib/svn-fe/remote-svn.c\nindex ce59344..df1babc 100644\n--- a/contrib/svn-fe/remote-svn.c\n+++ b/contrib/svn-fe/remote-svn.c\n@@ -10,6 +10,7 @@\n #include \"argv-array.h\"\n \n static const char *url;\n+static int dump_from_file;\n static const char *private_ref;\n static const char *remote_ref = \"refs/heads/master\";\n \n@@ -54,34 +55,39 @@ static int cmd_import(const char *line)\n \tstruct argv_array svndump_argv = ARGV_ARRAY_INIT;\n \tstruct child_process svndump_proc;\n \n-\tmemset(&svndump_proc, 0, sizeof (struct child_process));\n-\tsvndump_proc.out = -1;\n-\targv_array_push(&svndump_argv, \"svnrdump\");\n-\targv_array_push(&svndump_argv, \"dump\");\n-\targv_array_push(&svndump_argv, url);\n-\targv_array_pushf(&svndump_argv, \"-r%u:HEAD\", startrev);\n-\tsvndump_proc.argv = svndump_argv.argv;\n-\n-\tcode = start_command(&svndump_proc);\n-\tif (code)\n-\t\tdie(\"Unable to start %s, code %d\", svndump_proc.argv[0], code);\n-\tdumpin_fd = svndump_proc.out;\n-\n-\tcode = start_command(&svndump_proc);\n-\tif (code)\n-\t\tdie(\"Unable to start %s, code %d\", svndump_proc.argv[0], code);\n-\tdumpin_fd = svndump_proc.out;\n+\tif(dump_from_file) {\n+\t\tdumpin_fd = open(url, O_RDONLY);\n+\t\tif(dumpin_fd < 0) {\n+\t\t\tdie_errno(\"Couldn't open svn dump file %s.\", url);\n+\t\t}\n+\t}\n+\telse {\n+\t\tmemset(&svndump_proc, 0, sizeof (struct child_process));\n+\t\tsvndump_proc.out = -1;\n+\t\targv_array_push(&svndump_argv, \"svnrdump\");\n+\t\targv_array_push(&svndump_argv, \"dump\");\n+\t\targv_array_push(&svndump_argv, url);\n+\t\targv_array_pushf(&svndump_argv, \"-r%u:HEAD\", startrev);\n+\t\tsvndump_proc.argv = svndump_argv.argv;\n+\n+\t\tcode = start_command(&svndump_proc);\n+\t\tif (code)\n+\t\t\tdie(\"Unable to start %s, code %d\", svndump_proc.argv[0], code);\n+\t\tdumpin_fd = svndump_proc.out;\n \n+\t}\n \tsvndump_init_fd(dumpin_fd, STDIN_FILENO);\n \tsvndump_read(url, private_ref);\n \tsvndump_deinit();\n \tsvndump_reset();\n \n \tclose(dumpin_fd);\n-\tcode = finish_command(&svndump_proc);\n-\tif (code)\n-\t\twarning(\"%s, returned %d\", svndump_proc.argv[0], code);\n-\targv_array_clear(&svndump_argv);\n+\tif(!dump_from_file) {\n+\t\tcode = finish_command(&svndump_proc);\n+\t\tif (code)\n+\t\t\twarning(\"%s, returned %d\", svndump_proc.argv[0], code);\n+\t\targv_array_clear(&svndump_argv);\n+\t}\n \n \treturn 0;\n }\n@@ -158,8 +164,15 @@ int main(int argc, const char **argv)\n \tif (argc == 3)\n \t\turl_in = argv[2];\n \n-\tend_url_with_slash(&buf, url_in);\n-\turl = strbuf_detach(&buf, NULL);\n+\tif (!prefixcmp(url_in, \"file://\")) {\n+\t\tdump_from_file = 1;\n+\t\turl = url_decode(url_in + sizeof(\"file://\")-1);\n+\t}\n+\telse {\n+\t\tdump_from_file = 0;\n+\t\tend_url_with_slash(&buf, url_in);\n+\t\turl = strbuf_detach(&buf, NULL);\n+\t}\n \n \tstrbuf_addf(&buf, \"refs/svn/%s/master\", remote->name);\n \tprivate_ref = strbuf_detach(&buf, NULL);\n-- \n1.7.9.5\n"},{"id":"197003","messageId":"1344971598-8213-10-git-send-email-florian.achleitner.2.6.31@gmail.com","threadId":"31252","inReplyTo":"1344971598-8213-9-git-send-email-florian.achleitner.2.6.31@gmail.com","subject":"[PATCH/RFC v3 09/16] vcs-svn: add fast_export_note to create notes","fromName":"Florian Achleitner","fromEmail":"florian.achleitner.2.6.31@gmail.com","sentAt":"2012-08-14T19:13:11Z","receivedAt":"2012-08-14T19:13:11Z","isPatch":true,"sender":{"key":"florian.achleitner.2.6.31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/880777?v=4"},"body":"From: Dmitry Ivankov <divanorama@gmail.com>\n\nfast_export lacked a method to writes notes to fast-import stream.\nAdd two new functions fast_export_note which is similar to\nfast_export_modify. And also add fast_export_buf_to_data to be able\nto write inline blobs that don't come from a line_buffer or from delta\napplication.\n\nTo be used like this:\nfast_export_begin_commit(\"refs/notes/somenotes\", ...)\n\nfast_export_note(\"refs/heads/master\", \"inline\")\nfast_export_buf_to_data(&data)\nor maybe\nfast_export_note(\"refs/heads/master\", sha1)\n\nSigned-off-by: Dmitry Ivankov <divanorama@gmail.com>\nSigned-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n---\n vcs-svn/fast_export.c |   12 ++++++++++++\n vcs-svn/fast_export.h |    2 ++\n 2 files changed, 14 insertions(+)\n\ndiff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c\nindex 11f8f94..1ecae4b 100644\n--- a/vcs-svn/fast_export.c\n+++ b/vcs-svn/fast_export.c\n@@ -68,6 +68,11 @@ void fast_export_modify(const char *path, uint32_t mode, const char *dataref)\n \tputchar('\\n');\n }\n \n+void fast_export_note(const char *committish, const char *dataref)\n+{\n+\tprintf(\"N %s %s\\n\", dataref, committish);\n+}\n+\n static char gitsvnline[MAX_GITSVN_LINE_LEN];\n void fast_export_begin_commit(uint32_t revision, const char *author,\n \t\t\tconst struct strbuf *log,\n@@ -222,6 +227,13 @@ static long apply_delta(off_t len, struct line_buffer *input,\n \treturn ret;\n }\n \n+void fast_export_buf_to_data(const struct strbuf *data)\n+{\n+\tprintf(\"data %\"PRIuMAX\"\\n\", (uintmax_t)data->len);\n+\tfwrite(data->buf, data->len, 1, stdout);\n+\tfputc('\\n', stdout);\n+}\n+\n void fast_export_data(uint32_t mode, off_t len, struct line_buffer *input)\n {\n \tassert(len >= 0);\ndiff --git a/vcs-svn/fast_export.h b/vcs-svn/fast_export.h\nindex 17eb13b..9b32f1e 100644\n--- a/vcs-svn/fast_export.h\n+++ b/vcs-svn/fast_export.h\n@@ -9,11 +9,13 @@ void fast_export_deinit(void);\n \n void fast_export_delete(const char *path);\n void fast_export_modify(const char *path, uint32_t mode, const char *dataref);\n+void fast_export_note(const char *committish, const char *dataref);\n void fast_export_begin_commit(uint32_t revision, const char *author,\n \t\t\tconst struct strbuf *log, const char *uuid,\n \t\t\tconst char *url, unsigned long timestamp, const char *local_ref);\n void fast_export_end_commit(uint32_t revision);\n void fast_export_data(uint32_t mode, off_t len, struct line_buffer *input);\n+void fast_export_buf_to_data(const struct strbuf *data);\n void fast_export_blob_delta(uint32_t mode,\n \t\t\tuint32_t old_mode, const char *old_data,\n \t\t\toff_t len, struct line_buffer *input);\n-- \n1.7.9.5\n"},{"id":"197004","messageId":"1344971598-8213-11-git-send-email-florian.achleitner.2.6.31@gmail.com","threadId":"31252","inReplyTo":"1344971598-8213-10-git-send-email-florian.achleitner.2.6.31@gmail.com","subject":"[PATCH/RFC v3 10/16] Create a note for every imported commit containing svn metadata.","fromName":"Florian Achleitner","fromEmail":"florian.achleitner.2.6.31@gmail.com","sentAt":"2012-08-14T19:13:12Z","receivedAt":"2012-08-14T19:13:12Z","isPatch":true,"sender":{"key":"florian.achleitner.2.6.31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/880777?v=4"},"body":"To provide metadata from svn dumps for further processing, e.g.\nbranch detection, attach a note to each imported commit that\nstores additional information.\nThe notes are currently hard-coded in refs/notes/svn/revs.\nCurrently the following lines from the svn dump are directly\naccumulated in the note. This can be refined on purpose, of course.\n- \"Revision-number\"\n- \"Node-path\"\n- \"Node-kind\"\n- \"Node-action\"\n- \"Node-copyfrom-path\"\n- \"Node-copyfrom-rev\"\n\nSigned-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n---\n vcs-svn/fast_export.c |   13 +++++++++++++\n vcs-svn/fast_export.h |    2 ++\n vcs-svn/svndump.c     |   21 +++++++++++++++++++--\n 3 files changed, 34 insertions(+), 2 deletions(-)\n\ndiff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c\nindex 1ecae4b..796dd1a 100644\n--- a/vcs-svn/fast_export.c\n+++ b/vcs-svn/fast_export.c\n@@ -12,6 +12,7 @@\n #include \"svndiff.h\"\n #include \"sliding_window.h\"\n #include \"line_buffer.h\"\n+#include \"cache.h\"\n \n #define MAX_GITSVN_LINE_LEN 4096\n \n@@ -68,6 +69,18 @@ void fast_export_modify(const char *path, uint32_t mode, const char *dataref)\n \tputchar('\\n');\n }\n \n+void fast_export_begin_note(uint32_t revision, const char *author,\n+\t\tconst char *log, unsigned long timestamp)\n+{\n+\ttimestamp = 1341914616;\n+\tsize_t loglen = strlen(log);\n+\tprintf(\"commit refs/notes/svn/revs\\n\");\n+\tprintf(\"committer %s <%s@%s> %ld +0000\\n\", author, author, \"local\", timestamp);\n+\tprintf(\"data %\"PRIuMAX\"\\n\", loglen);\n+\tfwrite(log, loglen, 1, stdout);\n+\tfputc('\\n', stdout);\n+}\n+\n void fast_export_note(const char *committish, const char *dataref)\n {\n \tprintf(\"N %s %s\\n\", dataref, committish);\ndiff --git a/vcs-svn/fast_export.h b/vcs-svn/fast_export.h\nindex 9b32f1e..c2f6f11 100644\n--- a/vcs-svn/fast_export.h\n+++ b/vcs-svn/fast_export.h\n@@ -10,6 +10,8 @@ void fast_export_deinit(void);\n void fast_export_delete(const char *path);\n void fast_export_modify(const char *path, uint32_t mode, const char *dataref);\n void fast_export_note(const char *committish, const char *dataref);\n+void fast_export_begin_note(uint32_t revision, const char *author,\n+\t\tconst char *log, unsigned long timestamp);\n void fast_export_begin_commit(uint32_t revision, const char *author,\n \t\t\tconst struct strbuf *log, const char *uuid,\n \t\t\tconst char *url, unsigned long timestamp, const char *local_ref);\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex 288bb42..cd65b51 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -48,7 +48,7 @@ static struct {\n static struct {\n \tuint32_t revision;\n \tunsigned long timestamp;\n-\tstruct strbuf log, author;\n+\tstruct strbuf log, author, note;\n } rev_ctx;\n \n static struct {\n@@ -77,6 +77,7 @@ static void reset_rev_ctx(uint32_t revision)\n \trev_ctx.timestamp = 0;\n \tstrbuf_reset(&rev_ctx.log);\n \tstrbuf_reset(&rev_ctx.author);\n+\tstrbuf_reset(&rev_ctx.note);\n }\n \n static void reset_dump_ctx(const char *url)\n@@ -310,8 +311,15 @@ static void begin_revision(const char *remote_ref)\n \n static void end_revision()\n {\n-\tif (rev_ctx.revision)\n+\tstruct strbuf mark = STRBUF_INIT;\n+\tif (rev_ctx.revision) {\n \t\tfast_export_end_commit(rev_ctx.revision);\n+\t\tfast_export_begin_note(rev_ctx.revision, \"remote-svn\",\n+\t\t\t\t\"Note created by remote-svn.\", rev_ctx.timestamp);\n+\t\tstrbuf_addf(&mark, \":%\"PRIu32, rev_ctx.revision);\n+\t\tfast_export_note(mark.buf, \"inline\");\n+\t\tfast_export_buf_to_data(&rev_ctx.note);\n+\t}\n }\n \n void svndump_read(const char *url, const char *local_ref)\n@@ -358,6 +366,7 @@ void svndump_read(const char *url, const char *local_ref)\n \t\t\t\tend_revision();\n \t\t\tactive_ctx = REV_CTX;\n \t\t\treset_rev_ctx(atoi(val));\n+\t\t\tstrbuf_addf(&rev_ctx.note, \"%s\\n\", t);\n \t\t\tbreak;\n \t\tcase sizeof(\"Node-path\"):\n \t\t\tif (constcmp(t, \"Node-\"))\n@@ -369,10 +378,12 @@ void svndump_read(const char *url, const char *local_ref)\n \t\t\t\t\tbegin_revision(local_ref);\n \t\t\t\tactive_ctx = NODE_CTX;\n \t\t\t\treset_node_ctx(val);\n+\t\t\t\tstrbuf_addf(&rev_ctx.note, \"%s\\n\", t);\n \t\t\t\tbreak;\n \t\t\t}\n \t\t\tif (constcmp(t + strlen(\"Node-\"), \"kind\"))\n \t\t\t\tcontinue;\n+\t\t\tstrbuf_addf(&rev_ctx.note, \"%s\\n\", t);\n \t\t\tif (!strcmp(val, \"dir\"))\n \t\t\t\tnode_ctx.type = REPO_MODE_DIR;\n \t\t\telse if (!strcmp(val, \"file\"))\n@@ -383,6 +394,7 @@ void svndump_read(const char *url, const char *local_ref)\n \t\tcase sizeof(\"Node-action\"):\n \t\t\tif (constcmp(t, \"Node-action\"))\n \t\t\t\tcontinue;\n+\t\t\tstrbuf_addf(&rev_ctx.note, \"%s\\n\", t);\n \t\t\tif (!strcmp(val, \"delete\")) {\n \t\t\t\tnode_ctx.action = NODEACT_DELETE;\n \t\t\t} else if (!strcmp(val, \"add\")) {\n@@ -401,11 +413,13 @@ void svndump_read(const char *url, const char *local_ref)\n \t\t\t\tcontinue;\n \t\t\tstrbuf_reset(&node_ctx.src);\n \t\t\tstrbuf_addstr(&node_ctx.src, val);\n+\t\t\tstrbuf_addf(&rev_ctx.note, \"%s\\n\", t);\n \t\t\tbreak;\n \t\tcase sizeof(\"Node-copyfrom-rev\"):\n \t\t\tif (constcmp(t, \"Node-copyfrom-rev\"))\n \t\t\t\tcontinue;\n \t\t\tnode_ctx.srcRev = atoi(val);\n+\t\t\tstrbuf_addf(&rev_ctx.note, \"%s\\n\", t);\n \t\t\tbreak;\n \t\tcase sizeof(\"Text-content-length\"):\n \t\t\tif (constcmp(t, \"Text\") && constcmp(t, \"Prop\"))\n@@ -475,6 +489,7 @@ static void init(int report_fd)\n \tstrbuf_init(&dump_ctx.url, 4096);\n \tstrbuf_init(&rev_ctx.log, 4096);\n \tstrbuf_init(&rev_ctx.author, 4096);\n+\tstrbuf_init(&rev_ctx.note, 4096);\n \tstrbuf_init(&node_ctx.src, 4096);\n \tstrbuf_init(&node_ctx.dst, 4096);\n \treset_dump_ctx(NULL);\n@@ -506,6 +521,8 @@ void svndump_deinit(void)\n \treset_rev_ctx(0);\n \treset_node_ctx(NULL);\n \tstrbuf_release(&rev_ctx.log);\n+\tstrbuf_release(&rev_ctx.author);\n+\tstrbuf_release(&rev_ctx.note);\n \tstrbuf_release(&node_ctx.src);\n \tstrbuf_release(&node_ctx.dst);\n \tif (buffer_deinit(&input))\n-- \n1.7.9.5\n"},{"id":"197005","messageId":"1344971598-8213-12-git-send-email-florian.achleitner.2.6.31@gmail.com","threadId":"31252","inReplyTo":"1344971598-8213-11-git-send-email-florian.achleitner.2.6.31@gmail.com","subject":"[PATCH/RFC v3 11/16] When debug==1, start fast-import with \"--stats\" instead of \"--quiet\".","fromName":"Florian Achleitner","fromEmail":"florian.achleitner.2.6.31@gmail.com","sentAt":"2012-08-14T19:13:13Z","receivedAt":"2012-08-14T19:13:13Z","isPatch":true,"sender":{"key":"florian.achleitner.2.6.31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/880777?v=4"},"body":"fast-import prints statistics that could be interesting to the\ndeveloper of remote helpers.\n\nSigned-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n---\n transport-helper.c |    2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 257274b..7fb52d4 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -386,7 +386,7 @@ static int get_importer(struct transport *transport, struct child_process *fasti\n \tmemset(fastimport, 0, sizeof(*fastimport));\n \tfastimport->in = helper->out;\n \targv_array_push(&argv, \"fast-import\");\n-\targv_array_push(&argv, \"--quiet\");\n+\targv_array_push(&argv, debug ? \"--stats\" : \"--quiet\");\n \n \tif (data->bidi_import) {\n \t\tcat_blob_fd = xdup(helper->in);\n-- \n1.7.9.5\n"},{"id":"197007","messageId":"1344971598-8213-13-git-send-email-florian.achleitner.2.6.31@gmail.com","threadId":"31252","inReplyTo":"1344971598-8213-12-git-send-email-florian.achleitner.2.6.31@gmail.com","subject":"[PATCH/RFC v3 12/16] remote-svn: add incremental import.","fromName":"Florian Achleitner","fromEmail":"florian.achleitner.2.6.31@gmail.com","sentAt":"2012-08-14T19:13:14Z","receivedAt":"2012-08-14T19:13:14Z","isPatch":true,"sender":{"key":"florian.achleitner.2.6.31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/880777?v=4"},"body":"Search for a note attached to the ref to update and read it's\n'Revision-number:'-line. Start import from the next svn revision.\n\nIf there is no next revision in the svn repo, svnrdump terminates\nwith a message on stderr an non-zero return value. This looks a\nlittle weird, but there is no other way to know whether there is\na new revision in the svn repo.\n\nOn the start of an incremental import, the parent of the first commit\nin the fast-import stream is set to the branch name to update. All\nfollowing commits specify their parent by a mark number. Previous\nmark files are currently not reused.\n\nSigned-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n---\n contrib/svn-fe/remote-svn.c |   66 +++++++++++++++++++++++++++++++++++++++++--\n contrib/svn-fe/svn-fe.c     |    3 +-\n test-svn-fe.c               |    2 +-\n vcs-svn/fast_export.c       |   16 ++++++++---\n vcs-svn/fast_export.h       |    6 ++--\n vcs-svn/svndump.c           |   12 ++++----\n vcs-svn/svndump.h           |    2 +-\n 7 files changed, 89 insertions(+), 18 deletions(-)\n\ndiff --git a/contrib/svn-fe/remote-svn.c b/contrib/svn-fe/remote-svn.c\nindex df1babc..d659a0e 100644\n--- a/contrib/svn-fe/remote-svn.c\n+++ b/contrib/svn-fe/remote-svn.c\n@@ -13,6 +13,8 @@ static const char *url;\n static int dump_from_file;\n static const char *private_ref;\n static const char *remote_ref = \"refs/heads/master\";\n+static const char *notes_ref;\n+struct rev_note { unsigned int rev_nr; };\n \n static int cmd_capabilities(const char *line);\n static int cmd_import(const char *line);\n@@ -47,14 +49,70 @@ static void terminate_batch(void)\n \t\tfflush(stdout);\n }\n \n+/* NOTE: 'ref' refers to a git reference, while 'rev' refers to a svn revision. */\n+static char *read_ref_note(const unsigned char sha1[20]) {\n+\tconst unsigned char *note_sha1;\n+\tchar *msg = NULL;\n+\tunsigned long msglen;\n+\tenum object_type type;\n+\tinit_notes(NULL, notes_ref, NULL, 0);\n+\tif(\t(note_sha1 = get_note(NULL, sha1)) == NULL ||\n+\t\t\t!(msg = read_sha1_file(note_sha1, &type, &msglen)) ||\n+\t\t\t!msglen || type != OBJ_BLOB) {\n+\t\tfree(msg);\n+\t\treturn NULL;\n+\t}\n+\tfree_notes(NULL);\n+\treturn msg;\n+}\n+\n+static int parse_rev_note(const char *msg, struct rev_note *res) {\n+\tconst char *key, *value, *end;\n+\tsize_t len;\n+\twhile(*msg) {\n+\t\tend = strchr(msg, '\\n');\n+\t\tlen = end ? end - msg : strlen(msg);\n+\n+\t\tkey = \"Revision-number: \";\n+\t\tif(!prefixcmp(msg, key)) {\n+\t\t\tlong i;\n+\t\t\tvalue = msg + strlen(key);\n+\t\t\ti = atol(value);\n+\t\t\tif(i < 0 || i > UINT32_MAX)\n+\t\t\t\treturn 1;\n+\t\t\tres->rev_nr = i;\n+\t\t}\n+\t\tmsg += len + 1;\n+\t}\n+\treturn 0;\n+}\n+\n static int cmd_import(const char *line)\n {\n \tint code;\n \tint dumpin_fd;\n-\tunsigned int startrev = 0;\n+\tchar *note_msg;\n+\tunsigned char head_sha1[20];\n+\tunsigned int startrev;\n \tstruct argv_array svndump_argv = ARGV_ARRAY_INIT;\n \tstruct child_process svndump_proc;\n \n+\tif(read_ref(private_ref, head_sha1))\n+\t\tstartrev = 0;\n+\telse {\n+\t\tnote_msg = read_ref_note(head_sha1);\n+\t\tif(note_msg == NULL) {\n+\t\t\twarning(\"No note found for %s.\", private_ref);\n+\t\t\tstartrev = 0;\n+\t\t}\n+\t\telse {\n+\t\t\tstruct rev_note note = { 0 };\n+\t\t\tparse_rev_note(note_msg, &note);\n+\t\t\tstartrev = note.rev_nr + 1;\n+\t\t\tfree(note_msg);\n+\t\t}\n+\t}\n+\n \tif(dump_from_file) {\n \t\tdumpin_fd = open(url, O_RDONLY);\n \t\tif(dumpin_fd < 0) {\n@@ -77,7 +135,7 @@ static int cmd_import(const char *line)\n \n \t}\n \tsvndump_init_fd(dumpin_fd, STDIN_FILENO);\n-\tsvndump_read(url, private_ref);\n+\tsvndump_read(url, private_ref, notes_ref);\n \tsvndump_deinit();\n \tsvndump_reset();\n \n@@ -177,6 +235,9 @@ int main(int argc, const char **argv)\n \tstrbuf_addf(&buf, \"refs/svn/%s/master\", remote->name);\n \tprivate_ref = strbuf_detach(&buf, NULL);\n \n+\tstrbuf_addf(&buf, \"refs/notes/%s/revs\", remote->name);\n+\tnotes_ref = strbuf_detach(&buf, NULL);\n+\n \twhile(1) {\n \t\tif (strbuf_getline(&buf, stdin, '\\n') == EOF) {\n \t\t\tif (ferror(stdin))\n@@ -192,5 +253,6 @@ int main(int argc, const char **argv)\n \tstrbuf_release(&buf);\n \tfree((void*)url);\n \tfree((void*)private_ref);\n+\tfree((void*)notes_ref);\n \treturn 0;\n }\ndiff --git a/contrib/svn-fe/svn-fe.c b/contrib/svn-fe/svn-fe.c\nindex c796cc0..f363505 100644\n--- a/contrib/svn-fe/svn-fe.c\n+++ b/contrib/svn-fe/svn-fe.c\n@@ -10,7 +10,8 @@ int main(int argc, char **argv)\n {\n \tif (svndump_init(NULL))\n \t\treturn 1;\n-\tsvndump_read((argc > 1) ? argv[1] : NULL, \"refs/heads/master\");\n+\tsvndump_read((argc > 1) ? argv[1] : NULL, \"refs/heads/master\",\n+\t\t\t\"refs/notes/svn/revs\");\n \tsvndump_deinit();\n \tsvndump_reset();\n \treturn 0;\ndiff --git a/test-svn-fe.c b/test-svn-fe.c\nindex cb0d80f..0f2d9a4 100644\n--- a/test-svn-fe.c\n+++ b/test-svn-fe.c\n@@ -40,7 +40,7 @@ int main(int argc, char *argv[])\n \tif (argc == 2) {\n \t\tif (svndump_init(argv[1]))\n \t\t\treturn 1;\n-\t\tsvndump_read(NULL, \"refs/heads/master\");\n+\t\tsvndump_read(NULL, \"refs/heads/master\", \"refs/notes/svn/revs\");\n \t\tsvndump_deinit();\n \t\tsvndump_reset();\n \t\treturn 0;\ndiff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c\nindex 796dd1a..32f71a1 100644\n--- a/vcs-svn/fast_export.c\n+++ b/vcs-svn/fast_export.c\n@@ -70,14 +70,20 @@ void fast_export_modify(const char *path, uint32_t mode, const char *dataref)\n }\n \n void fast_export_begin_note(uint32_t revision, const char *author,\n-\t\tconst char *log, unsigned long timestamp)\n+\t\tconst char *log, unsigned long timestamp, const char *note_ref)\n {\n+\tstatic int firstnote = 1;\n \ttimestamp = 1341914616;\n \tsize_t loglen = strlen(log);\n-\tprintf(\"commit refs/notes/svn/revs\\n\");\n+\tprintf(\"commit %s\\n\", note_ref);\n \tprintf(\"committer %s <%s@%s> %ld +0000\\n\", author, author, \"local\", timestamp);\n \tprintf(\"data %\"PRIuMAX\"\\n\", loglen);\n \tfwrite(log, loglen, 1, stdout);\n+\tif (firstnote) {\n+\t\tif (revision > 1)\n+\t\t\tprintf(\"from %s^0\", note_ref);\n+\t\tfirstnote = 0;\n+\t}\n \tfputc('\\n', stdout);\n }\n \n@@ -90,7 +96,7 @@ static char gitsvnline[MAX_GITSVN_LINE_LEN];\n void fast_export_begin_commit(uint32_t revision, const char *author,\n \t\t\tconst struct strbuf *log,\n \t\t\tconst char *uuid, const char *url,\n-\t\t\tunsigned long timestamp, const char *local_ref)\n+\t\t\tunsigned long timestamp, const char *local_ref, const char *parent)\n {\n \tstatic const struct strbuf empty = STRBUF_INIT;\n \tif (!log)\n@@ -113,7 +119,9 @@ void fast_export_begin_commit(uint32_t revision, const char *author,\n \tfwrite(log->buf, log->len, 1, stdout);\n \tprintf(\"%s\\n\", gitsvnline);\n \tif (!first_commit_done) {\n-\t\tif (revision > 1)\n+\t\tif(parent)\n+\t\t\tprintf(\"from %s^0\\n\", parent);\n+\t\telse if (revision > 1)\n \t\t\tprintf(\"from :%\"PRIu32\"\\n\", revision - 1);\n \t\tfirst_commit_done = 1;\n \t}\ndiff --git a/vcs-svn/fast_export.h b/vcs-svn/fast_export.h\nindex c2f6f11..5174aae 100644\n--- a/vcs-svn/fast_export.h\n+++ b/vcs-svn/fast_export.h\n@@ -11,10 +11,10 @@ void fast_export_delete(const char *path);\n void fast_export_modify(const char *path, uint32_t mode, const char *dataref);\n void fast_export_note(const char *committish, const char *dataref);\n void fast_export_begin_note(uint32_t revision, const char *author,\n-\t\tconst char *log, unsigned long timestamp);\n+\t\tconst char *log, unsigned long timestamp, const char *note_ref);\n void fast_export_begin_commit(uint32_t revision, const char *author,\n-\t\t\tconst struct strbuf *log, const char *uuid,\n-\t\t\tconst char *url, unsigned long timestamp, const char *local_ref);\n+\t\t\tconst struct strbuf *log, const char *uuid,const char *url,\n+\t\t\tunsigned long timestamp, const char *local_ref, const char *parent);\n void fast_export_end_commit(uint32_t revision);\n void fast_export_data(uint32_t mode, off_t len, struct line_buffer *input);\n void fast_export_buf_to_data(const struct strbuf *data);\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex cd65b51..e567e67 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -306,23 +306,23 @@ static void begin_revision(const char *remote_ref)\n \t\treturn;\n \tfast_export_begin_commit(rev_ctx.revision, rev_ctx.author.buf,\n \t\t&rev_ctx.log, dump_ctx.uuid.buf, dump_ctx.url.buf,\n-\t\trev_ctx.timestamp, remote_ref);\n+\t\trev_ctx.timestamp, remote_ref, rev_ctx.revision == 1 ? NULL : remote_ref);\n }\n \n-static void end_revision()\n+static void end_revision(const char *note_ref)\n {\n \tstruct strbuf mark = STRBUF_INIT;\n \tif (rev_ctx.revision) {\n \t\tfast_export_end_commit(rev_ctx.revision);\n \t\tfast_export_begin_note(rev_ctx.revision, \"remote-svn\",\n-\t\t\t\t\"Note created by remote-svn.\", rev_ctx.timestamp);\n+\t\t\t\t\"Note created by remote-svn.\", rev_ctx.timestamp, note_ref);\n \t\tstrbuf_addf(&mark, \":%\"PRIu32, rev_ctx.revision);\n \t\tfast_export_note(mark.buf, \"inline\");\n \t\tfast_export_buf_to_data(&rev_ctx.note);\n \t}\n }\n \n-void svndump_read(const char *url, const char *local_ref)\n+void svndump_read(const char *url, const char *local_ref, const char *notes_ref)\n {\n \tchar *val;\n \tchar *t;\n@@ -363,7 +363,7 @@ void svndump_read(const char *url, const char *local_ref)\n \t\t\tif (active_ctx == REV_CTX)\n \t\t\t\tbegin_revision(local_ref);\n \t\t\tif (active_ctx != DUMP_CTX)\n-\t\t\t\tend_revision();\n+\t\t\t\tend_revision(notes_ref);\n \t\t\tactive_ctx = REV_CTX;\n \t\t\treset_rev_ctx(atoi(val));\n \t\t\tstrbuf_addf(&rev_ctx.note, \"%s\\n\", t);\n@@ -479,7 +479,7 @@ void svndump_read(const char *url, const char *local_ref)\n \tif (active_ctx == REV_CTX)\n \t\tbegin_revision(local_ref);\n \tif (active_ctx != DUMP_CTX)\n-\t\tend_revision();\n+\t\tend_revision(notes_ref);\n }\n \n static void init(int report_fd)\ndiff --git a/vcs-svn/svndump.h b/vcs-svn/svndump.h\nindex febeecb..b8eb129 100644\n--- a/vcs-svn/svndump.h\n+++ b/vcs-svn/svndump.h\n@@ -3,7 +3,7 @@\n \n int svndump_init(const char *filename);\n int svndump_init_fd(int in_fd, int back_fd);\n-void svndump_read(const char *url, const char *local_ref);\n+void svndump_read(const char *url, const char *local_ref, const char *notes_ref);\n void svndump_deinit(void);\n void svndump_reset(void);\n \n-- \n1.7.9.5\n"},{"id":"197008","messageId":"1344971598-8213-14-git-send-email-florian.achleitner.2.6.31@gmail.com","threadId":"31252","inReplyTo":"1344971598-8213-13-git-send-email-florian.achleitner.2.6.31@gmail.com","subject":"[PATCH/RFC v3 13/16] Add a svnrdump-simulator replaying a dump file for testing.","fromName":"Florian Achleitner","fromEmail":"florian.achleitner.2.6.31@gmail.com","sentAt":"2012-08-14T19:13:15Z","receivedAt":"2012-08-14T19:13:15Z","isPatch":true,"sender":{"key":"florian.achleitner.2.6.31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/880777?v=4"},"body":"To ease testing without depending on a reachable svn server, this\ncompact python script mimics parts of svnrdumps behaviour.\nIt requires the remote url to start with sim://.\nStart and end revisions are evaluated.\nIf the requested revision doesn't exist, as it is the case with\nincremental imports, if no new commit was added, it returns 1\n(like svnrdump).\nTo allow using the same dump file for simulating multiple\nincremental imports the highest revision can be limited by setting\nthe environment variable SVNRMAX to that value. This simulates the\nsituation where higher revs don't exist yet.\n\nSigned-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n---\n contrib/svn-fe/svnrdump_sim.py |   53 ++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 53 insertions(+)\n create mode 100755 contrib/svn-fe/svnrdump_sim.py\n\ndiff --git a/contrib/svn-fe/svnrdump_sim.py b/contrib/svn-fe/svnrdump_sim.py\nnew file mode 100755\nindex 0000000..ab4ccf1\n--- /dev/null\n+++ b/contrib/svn-fe/svnrdump_sim.py\n@@ -0,0 +1,53 @@\n+#!/usr/bin/python\n+\"\"\"\n+Simulates svnrdump by replaying an existing dump from a file, taking care\n+of the specified revision range.\n+To simulate incremental imports the environment variable SVNRMAX can be set\n+to the highest revision that should be available.\n+\"\"\"\n+import sys, os\n+\n+\n+def getrevlimit():\n+\tvar = 'SVNRMAX'\n+\tif os.environ.has_key(var):\n+\t\treturn os.environ[var]\n+\treturn None\n+\t\n+def writedump(url, lower, upper):\n+\tif url.startswith('sim://'):\n+\t\tfilename = url[6:]\n+\t\tif filename[-1] == '/': filename = filename[:-1] #remove terminating slash\n+\telse:\n+\t\traise ValueError('sim:// url required')\n+\tf = open(filename, 'r');\n+\tstate = 'header'\n+\twroterev = False\n+\twhile(True):\n+\t\tl = f.readline()\n+\t\tif l == '': break\n+\t\tif state == 'header' and l.startswith('Revision-number: '):\n+\t\t\tstate = 'prefix'\n+\t\tif state == 'prefix' and l == 'Revision-number: %s\\n' % lower:\n+\t\t\tstate = 'selection'\n+\t\tif not upper == 'HEAD' and state == 'selection' and l == 'Revision-number: %s\\n' % upper:\n+\t\t\tbreak;\n+\n+\t\tif state == 'header' or state == 'selection':\n+\t\t\tif state == 'selection': wroterev = True\n+\t\t\tsys.stdout.write(l)\n+\treturn wroterev\n+\n+if __name__ == \"__main__\":\n+\tif not (len(sys.argv) in (3, 4, 5)):\n+\t\tprint \"usage: %s dump URL -rLOWER:UPPER\"\n+\t\tsys.exit(1)\n+\tif not sys.argv[1] == 'dump': raise NotImplementedError('only \"dump\" is suppported.')\n+\turl = sys.argv[2]\n+\tr = ('0', 'HEAD')\n+\tif len(sys.argv) == 4 and sys.argv[3][0:2] == '-r':\n+\t\tr = sys.argv[3][2:].lstrip().split(':')\n+\tif not getrevlimit() is None: r[1] = getrevlimit()\n+\tif writedump(url, r[0], r[1]): ret = 0\n+\telse: ret = 1\n+\tsys.exit(ret)\n-- \n1.7.9.5\n"},{"id":"197009","messageId":"1344971598-8213-15-git-send-email-florian.achleitner.2.6.31@gmail.com","threadId":"31252","inReplyTo":"1344971598-8213-14-git-send-email-florian.achleitner.2.6.31@gmail.com","subject":"[PATCH/RFC v3 14/16] transport-helper: add import|export-marks to fast-import command line.","fromName":"Florian Achleitner","fromEmail":"florian.achleitner.2.6.31@gmail.com","sentAt":"2012-08-14T19:13:16Z","receivedAt":"2012-08-14T19:13:16Z","isPatch":true,"sender":{"key":"florian.achleitner.2.6.31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/880777?v=4"},"body":"fast-import internally uses marks that refer to an object via its sha1.\nThose marks are created during import to find previously created objects.\nAt exit the accumulated marks can be exported to a file and reloaded at\nstartup, so that the previous marks are available.\nAdd command line options to the fast-import command line to enable this.\nThe mark files are stored in info/fast-import/marks/<remote-name>.\n\nSigned-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n---\n transport-helper.c |    3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 7fb52d4..47db055 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -387,6 +387,9 @@ static int get_importer(struct transport *transport, struct child_process *fasti\n \tfastimport->in = helper->out;\n \targv_array_push(&argv, \"fast-import\");\n \targv_array_push(&argv, debug ? \"--stats\" : \"--quiet\");\n+\targv_array_push(&argv, \"--relative-marks\");\n+\targv_array_pushf(&argv, \"--import-marks-if-exists=marks/%s\", transport->remote->name);\n+\targv_array_pushf(&argv, \"--export-marks=marks/%s\", transport->remote->name);\n \n \tif (data->bidi_import) {\n \t\tcat_blob_fd = xdup(helper->in);\n-- \n1.7.9.5\n"},{"id":"197006","messageId":"1344971598-8213-16-git-send-email-florian.achleitner.2.6.31@gmail.com","threadId":"31252","inReplyTo":"1344971598-8213-15-git-send-email-florian.achleitner.2.6.31@gmail.com","subject":"[PATCH/RFC v3 15/16] remote-svn: add marks-file regeneration.","fromName":"Florian Achleitner","fromEmail":"florian.achleitner.2.6.31@gmail.com","sentAt":"2012-08-14T19:13:17Z","receivedAt":"2012-08-14T19:13:17Z","isPatch":true,"sender":{"key":"florian.achleitner.2.6.31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/880777?v=4"},"body":"fast-import mark files are stored outside the object database and are therefore\nnot fetched and can be lost somehow else.\nmarks provide a svn revision --> git sha1 mapping, while the notes that are attached\nto each commit when it is imported provide a git sha1 --> svn revision.\n\nIf the marks file is not available or not plausible, regenerate it by walking through\nthe notes tree.\n, i.e.\nThe plausibility check tests if the highest revision in the marks file matches the\nrevision of the top ref. It doesn't ensure that the mark file is completely correct.\nThis could only be done with an effort equal to unconditional regeneration.\n\nSigned-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n---\n contrib/svn-fe/remote-svn.c |   69 ++++++++++++++++++++++++++++++++++++++++++-\n 1 file changed, 68 insertions(+), 1 deletion(-)\n\ndiff --git a/contrib/svn-fe/remote-svn.c b/contrib/svn-fe/remote-svn.c\nindex d659a0e..94e5196 100644\n--- a/contrib/svn-fe/remote-svn.c\n+++ b/contrib/svn-fe/remote-svn.c\n@@ -13,7 +13,7 @@ static const char *url;\n static int dump_from_file;\n static const char *private_ref;\n static const char *remote_ref = \"refs/heads/master\";\n-static const char *notes_ref;\n+static const char *notes_ref, *marksfilename;\n struct rev_note { unsigned int rev_nr; };\n \n static int cmd_capabilities(const char *line);\n@@ -87,6 +87,68 @@ static int parse_rev_note(const char *msg, struct rev_note *res) {\n \treturn 0;\n }\n \n+static int note2mark_cb(const unsigned char *object_sha1,\n+\t\tconst unsigned char *note_sha1, char *note_path,\n+\t\tvoid *cb_data) {\n+\tFILE *file = (FILE *)cb_data;\n+\tchar *msg;\n+\tunsigned long msglen;\n+\tenum object_type type;\n+\tstruct rev_note note;\n+\tif (!(msg = read_sha1_file(note_sha1, &type, &msglen)) ||\n+\t\t\t!msglen || type != OBJ_BLOB) {\n+\t\tfree(msg);\n+\t\treturn 1;\n+\t}\n+\tif (parse_rev_note(msg, &note))\n+\t\treturn 2;\n+\tif (fprintf(file, \":%d %s\\n\", note.rev_nr, sha1_to_hex(object_sha1)) < 1)\n+\t\treturn 3;\n+\treturn 0;\n+}\n+\n+static void regenerate_marks() {\n+\tint ret;\n+\tFILE *marksfile;\n+\tmarksfile = fopen(marksfilename, \"w+\");\n+\tif (!marksfile)\n+\t\tdie_errno(\"Couldn't create mark file %s.\", marksfilename);\n+\tret = for_each_note(NULL, 0, note2mark_cb, marksfile);\n+\tif (ret)\n+\t\tdie(\"Regeneration of marks failed, returned %d.\", ret);\n+\tfclose(marksfile);\n+}\n+\n+static void check_or_regenerate_marks(int latestrev) {\n+\tFILE *marksfile;\n+\tchar *line = NULL;\n+\tsize_t linelen = 0;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tint found = 0;\n+\n+\tif (latestrev < 1)\n+\t\treturn;\n+\n+\tinit_notes(NULL, notes_ref, NULL, 0);\n+\tmarksfile = fopen(marksfilename, \"r\");\n+\tif (!marksfile)\n+\t\tregenerate_marks(marksfile);\n+\telse {\n+\t\tstrbuf_addf(&sb, \":%d \", latestrev);\n+\t\twhile (getline(&line, &linelen, marksfile) != -1) {\n+\t\t\tif (!prefixcmp(line, sb.buf)) {\n+\t\t\t\tfound++;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\t\tfclose(marksfile);\n+\t\tif (!found)\n+\t\t\tregenerate_marks();\n+\t}\n+\tfree_notes(NULL);\n+\tstrbuf_release(&sb);\n+}\n+\n static int cmd_import(const char *line)\n {\n \tint code;\n@@ -112,6 +174,7 @@ static int cmd_import(const char *line)\n \t\t\tfree(note_msg);\n \t\t}\n \t}\n+\tcheck_or_regenerate_marks(startrev - 1);\n \n \tif(dump_from_file) {\n \t\tdumpin_fd = open(url, O_RDONLY);\n@@ -238,6 +301,9 @@ int main(int argc, const char **argv)\n \tstrbuf_addf(&buf, \"refs/notes/%s/revs\", remote->name);\n \tnotes_ref = strbuf_detach(&buf, NULL);\n \n+\tstrbuf_addf(&buf, \"%s/info/fast-import/marks/%s\", get_git_dir(), remote->name);\n+\tmarksfilename = strbuf_detach(&buf, NULL);\n+\n \twhile(1) {\n \t\tif (strbuf_getline(&buf, stdin, '\\n') == EOF) {\n \t\t\tif (ferror(stdin))\n@@ -254,5 +320,6 @@ int main(int argc, const char **argv)\n \tfree((void*)url);\n \tfree((void*)private_ref);\n \tfree((void*)notes_ref);\n+\tfree((void*)marksfilename);\n \treturn 0;\n }\n-- \n1.7.9.5\n"},{"id":"197010","messageId":"1344971598-8213-17-git-send-email-florian.achleitner.2.6.31@gmail.com","threadId":"31252","inReplyTo":"1344971598-8213-16-git-send-email-florian.achleitner.2.6.31@gmail.com","subject":"[PATCH/RFC v3 16/16] Add a test script for remote-svn.","fromName":"Florian Achleitner","fromEmail":"florian.achleitner.2.6.31@gmail.com","sentAt":"2012-08-14T19:13:18Z","receivedAt":"2012-08-14T19:13:18Z","isPatch":true,"sender":{"key":"florian.achleitner.2.6.31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/880777?v=4"},"body":"Use svnrdump_sim.py to emulate svnrdump without an svn server.\nTests fetching, incremental fetching, fetching from file://,\nand the regeneration of fast-import's marks file.\n\nSigned-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n---\n t/t9020-remote-svn.sh |   69 +++++++++++++++++++++++++++++++++++++++++++++++++\n transport-helper.c    |   15 ++++++-----\n 2 files changed, 77 insertions(+), 7 deletions(-)\n create mode 100755 t/t9020-remote-svn.sh\n\ndiff --git a/t/t9020-remote-svn.sh b/t/t9020-remote-svn.sh\nnew file mode 100755\nindex 0000000..a0c6a21\n--- /dev/null\n+++ b/t/t9020-remote-svn.sh\n@@ -0,0 +1,69 @@\n+#!/bin/sh\n+\n+test_description='tests remote-svn'\n+\n+. ./test-lib.sh\n+\n+# We override svnrdump by placing a symlink to the svnrdump-emulator in .\n+export PATH=\"$HOME:$PATH\"\n+ln -sf $GIT_BUILD_DIR/contrib/svn-fe/svnrdump_sim.py \"$HOME/svnrdump\"\n+\n+init_git () {\n+\trm -fr .git &&\n+\tgit init &&\n+\t#git remote add svnsim svn::sim:///$TEST_DIRECTORY/t9020/example.svnrdump\n+\t# let's reuse an exisiting dump file!?\n+\tgit remote add svnsim svn::sim:///$TEST_DIRECTORY/t9154/svn.dump\n+\tgit remote add svnfile svn::file:///$TEST_DIRECTORY/t9154/svn.dump\n+}\n+\n+test_debug '\n+\tgit --version\n+\twhich git\n+\twhich svnrdump\n+'\n+\n+test_expect_success 'simple fetch' '\n+\tinit_git &&\n+\tgit fetch svnsim &&\n+\ttest_cmp .git/refs/svn/svnsim/master .git/refs/remotes/svnsim/master  &&\n+\tcp .git/refs/remotes/svnsim/master master.good\n+'\n+\n+test_debug '\n+\tcat .git/refs/svn/svnsim/master\n+\tcat .git/refs/remotes/svnsim/master\n+'\n+\n+test_expect_success 'repeated fetch, nothing shall change' '\n+\tgit fetch svnsim &&\n+\ttest_cmp master.good .git/refs/remotes/svnsim/master\n+'\n+\n+test_expect_success 'fetch from a file:// url gives the same result' '\n+\tgit fetch svnfile \n+'\n+\n+test_expect_failure 'the sha1 differ because the git-svn-id line in the commit msg contains the url' '\n+\ttest_cmp .git/refs/remotes/svnfile/master .git/refs/remotes/svnsim/master\n+'\n+\n+test_expect_success 'mark-file regeneration' '\n+\tmv .git/info/fast-import/marks/svnsim .git/info/fast-import/marks/svnsim.old &&\n+\tgit fetch svnsim &&\n+\ttest_cmp .git/info/fast-import/marks/svnsim.old .git/info/fast-import/marks/svnsim\n+'\n+\n+test_expect_success 'incremental imports must lead to the same head' '\n+\texport SVNRMAX=3 &&\n+\tinit_git &&\n+\tgit fetch svnsim &&\n+\ttest_cmp .git/refs/svn/svnsim/master .git/refs/remotes/svnsim/master  &&\n+\tunset SVNRMAX &&\n+\tgit fetch svnsim &&\n+\ttest_cmp master.good .git/refs/remotes/svnsim/master\n+'\n+\n+test_debug 'git branch -a'\t\n+\n+test_done\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 47db055..a363f2c 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -17,6 +17,7 @@ static int debug;\n struct helper_data {\n \tconst char *name;\n \tstruct child_process *helper;\n+\tstruct argv_array argv;\n \tFILE *out;\n \tunsigned fetch : 1,\n \t\timport : 1,\n@@ -103,7 +104,6 @@ static void do_take_over(struct transport *transport)\n static struct child_process *get_helper(struct transport *transport)\n {\n \tstruct helper_data *data = transport->data;\n-\tstruct argv_array argv = ARGV_ARRAY_INIT;\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct child_process *helper;\n \tconst char **refspecs = NULL;\n@@ -125,10 +125,11 @@ static struct child_process *get_helper(struct transport *transport)\n \thelper->in = -1;\n \thelper->out = -1;\n \thelper->err = 0;\n-\targv_array_pushf(&argv, \"git-remote-%s\", data->name);\n-\targv_array_push(&argv, transport->remote->name);\n-\targv_array_push(&argv, remove_ext_force(transport->url));\n-\thelper->argv = argv.argv;\n+\targv_array_init(&data->argv);\n+\targv_array_pushf(&data->argv, \"git-remote-%s\", data->name);\n+\targv_array_push(&data->argv, transport->remote->name);\n+\targv_array_push(&data->argv, remove_ext_force(transport->url));\n+\thelper->argv = data->argv.argv;\n \thelper->git_cmd = 0;\n \thelper->silent_exec_failure = 1;\n \n@@ -143,8 +144,6 @@ static struct child_process *get_helper(struct transport *transport)\n \n \tdata->helper = helper;\n \tdata->no_disconnect_req = 0;\n-\tfree((void*) helper_env[1]);\n-\targv_array_clear(&argv);\n \n \t/*\n \t * Open the output as FILE* so strbuf_getline() can be used.\n@@ -247,6 +246,8 @@ static int disconnect_helper(struct transport *transport)\n \t\tclose(data->helper->out);\n \t\tfclose(data->out);\n \t\tres = finish_command(data->helper);\n+\t\tfree((void*) data->helper->env[1]);\n+\t\targv_array_clear(&data->argv);\n \t\tfree(data->helper);\n \t\tdata->helper = NULL;\n \t}\n-- \n1.7.9.5\n"},{"id":"197011","messageId":"7vhas59r0b.fsf@alter.siamese.dyndns.org","threadId":"31252","inReplyTo":"1344971598-8213-2-git-send-email-florian.achleitner.2.6.31@gmail.com","subject":"Re: [PATCH/RFC v3 01/16] Implement a remote helper for svn in C.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-14T20:07:32Z","receivedAt":"2012-08-14T20:07:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Florian Achleitner <florian.achleitner.2.6.31@gmail.com> writes:\n\n> Enable basic fetching from subversion repositories. When processing remote URLs\n> starting with svn::, git invokes this remote-helper.\n> It starts svnrdump to extract revisions from the subversion repository in the\n> 'dump file format', and converts them to a git-fast-import stream using\n> the functions of vcs-svn/.\n\n(nit) the above is a bit too wide, isn't it?\n\n> Imported refs are created in a private namespace at refs/svn/<remote-name/master.\n\n(nit) missing closing '>'?\n\n> The revision history is imported linearly (no branch detection) and completely,\n> i.e. from revision 0 to HEAD.\n>\n> The 'bidi-import' capability is used. The remote-helper expects data from\n> fast-import on its stdin. It buffers a batch of 'import' command lines\n> in a string_list before starting to process them.\n>\n> Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n> ---\n> diff:\n> - incorporate review\n> - remove redundant strbuf_init\n> - add 'bidi-import' to capabilities\n> - buffer all lines of a command batch in string_list\n>\n>  contrib/svn-fe/remote-svn.c |  183 +++++++++++++++++++++++++++++++++++++++++++\n>  1 file changed, 183 insertions(+)\n>  create mode 100644 contrib/svn-fe/remote-svn.c\n>\n> diff --git a/contrib/svn-fe/remote-svn.c b/contrib/svn-fe/remote-svn.c\n> new file mode 100644\n> index 0000000..ce59344\n> --- /dev/null\n> +++ b/contrib/svn-fe/remote-svn.c\n> @@ -0,0 +1,183 @@\n> +\n\nRemove.\n\n> +#include \"cache.h\"\n> +#include \"remote.h\"\n> +#include \"strbuf.h\"\n> +#include \"url.h\"\n> +#include \"exec_cmd.h\"\n> +#include \"run-command.h\"\n> +#include \"svndump.h\"\n> +#include \"notes.h\"\n> +#include \"argv-array.h\"\n> +\n> +static const char *url;\n> +static const char *private_ref;\n> +static const char *remote_ref = \"refs/heads/master\";\n\nJust wondering; is this name \"master\" (or \"refs/heads/\" for that\nmatter) significant in any way when talking to a subversion remote?\n\n> +static int cmd_capabilities(const char *line);\n> +static int cmd_import(const char *line);\n> +static int cmd_list(const char *line);\n> +\n> +typedef int (*input_command_handler)(const char *);\n> +struct input_command_entry {\n> +\tconst char *name;\n> +\tinput_command_handler fct;\n> +\tunsigned char batchable;\t/* whether the command starts or is part of a batch */\n> +};\n> +\n> +static const struct input_command_entry input_command_list[] = {\n> +\t\t{ \"capabilities\", cmd_capabilities, 0 },\n\nOne level too deeply indented?\n\n> +\t\t{ \"import\", cmd_import, 1 },\n> +\t\t{ \"list\", cmd_list, 0 },\n> +\t\t{ NULL, NULL }\n> +};\n> +\n> +static int cmd_capabilities(const char *line) {\n> +\tprintf(\"import\\n\");\n> +\tprintf(\"bidi-import\\n\");\n> +\tprintf(\"refspec %s:%s\\n\\n\", remote_ref, private_ref);\n> +\tfflush(stdout);\n> +\treturn 0;\n> +}\n> +\n> +static void terminate_batch(void)\n> +{\n> +\t/* terminate a current batch's fast-import stream */\n> +\t\tprintf(\"done\\n\");\n\nLikewise.\n\n> +\t\tfflush(stdout);\n> +}\n> +\n> +static int cmd_import(const char *line)\n> +{\n> +\tint code;\n> +\tint dumpin_fd;\n> +\tunsigned int startrev = 0;\n> +\tstruct argv_array svndump_argv = ARGV_ARRAY_INIT;\n> +\tstruct child_process svndump_proc;\n> +\n> +\tmemset(&svndump_proc, 0, sizeof (struct child_process));\n\nPlease lose SP between sizeof and '('.\n\n> +\tsvndump_proc.out = -1;\n> +\targv_array_push(&svndump_argv, \"svnrdump\");\n> +\targv_array_push(&svndump_argv, \"dump\");\n> +\targv_array_push(&svndump_argv, url);\n> +\targv_array_pushf(&svndump_argv, \"-r%u:HEAD\", startrev);\n> +\tsvndump_proc.argv = svndump_argv.argv;\n\n(just me making a mental note) We read from \"svnrdump\", which would\nread (if it ever does) from the same stdin as ours and spits (if it\never does) its errors to the same stderr as ours.\n\n> +\n> +\tcode = start_command(&svndump_proc);\n> +\tif (code)\n> +\t\tdie(\"Unable to start %s, code %d\", svndump_proc.argv[0], code);\n> +\tdumpin_fd = svndump_proc.out;\n> +\n> +\tcode = start_command(&svndump_proc);\n> +\tif (code)\n> +\t\tdie(\"Unable to start %s, code %d\", svndump_proc.argv[0], code);\n> +\tdumpin_fd = svndump_proc.out;\n\nYou start it twice without finishing the first invocation, or just a\ndouble paste?\n\n> +\tsvndump_init_fd(dumpin_fd, STDIN_FILENO);\n> +\tsvndump_read(url, private_ref);\n> +\tsvndump_deinit();\n> +\tsvndump_reset();\n> +\n> +\tclose(dumpin_fd);\n\n(my mental note) And at this point, we finished feeding whatever\ncomes out of \"svnrdump\" to svndump_read().\n\n> +\tcode = finish_command(&svndump_proc);\n> +\tif (code)\n> +\t\twarning(\"%s, returned %d\", svndump_proc.argv[0], code);\n> +\targv_array_clear(&svndump_argv);\n> +\n> +\treturn 0;\n\nOther than the \"twice?\" puzzle, this function looks straightforward.\n\n> +}\n> +\n> +static int cmd_list(const char *line)\n> +{\n> +\tprintf(\"? %s\\n\\n\", remote_ref);\n> +\tfflush(stdout);\n> +\treturn 0;\n> +}\n> +\n> +static int do_command(struct strbuf *line)\n> +{\n> +\tconst struct input_command_entry *p = input_command_list;\n> +\tstatic struct string_list batchlines = STRING_LIST_INIT_DUP;\n> +\tstatic const struct input_command_entry *batch_cmd;\n> +\t/*\n> +\t * commands can be grouped together in a batch.\n> +\t * Batches are ended by \\n. If no batch is active the program ends.\n> +\t * During a batch all lines are buffered and passed to the handler function\n> +\t * when the batch is terminated.\n> +\t */\n> +\tif (line->len == 0) {\n> +\t\tif (batch_cmd) {\n> +\t\t\tstruct string_list_item *item;\n> +\t\t\tfor_each_string_list_item(item, &batchlines)\n> +\t\t\t\tbatch_cmd->fct(item->string);\n\n(style) I think we tend to call these unnamed functions \"fn\" in our\ncodebase.\n\n> +\t\t\tterminate_batch();\n> +\t\t\tbatch_cmd = NULL;\n> +\t\t\tstring_list_clear(&batchlines, 0);\n> +\t\t\treturn 0;\t/* end of the batch, continue reading other commands. */\n> +\t\t}\n> +\t\treturn 1;\t/* end of command stream, quit */\n> +\t}\n> +\tif (batch_cmd) {\n> +\t\tif (strcmp(batch_cmd->name, line->buf))\n> +\t\t\tdie(\"Active %s batch interrupted by %s\", batch_cmd->name, line->buf);\n> +\t\t/* buffer batch lines */\n> +\t\tstring_list_append(&batchlines, line->buf);\n> +\t\treturn 0;\n> +\t}\n\nA \"batch-able\" command, e.g. \"import\", will first cause the\nbatch_cmd to point at the command structure in this function, and\nthen the next and subsequent lines, as long as the input line is\nexactly the same as the current batch_cmd->name, e.g. \"import\", is\nappended into batchlines.\n\nWould this mean that you can feed something like this:\n\n\timport foobar\n        import\n        import\n        import\n\n        another command\n\nand buffer the four \"import\" lines in batchlines, and then on the\nempty line, have the for-each-string-list-item loop to call\ncmd_import() on \"import foobar\", \"import\", \"import\", then \"import\"\n(literally, without anything other than \"import\" on the line).\n\nHow is that useful?  With that \"if (strcmp(batch_cmd->name, line->buf))\",\nI cannot think of other valid input to make this \"batch\" mechanism\nto trigger and do something useful.  Am I missing something?\n\n> +\n> +\tfor(p = input_command_list; p->name; p++) {\n\nHave a SP between for and '('.\n\n> +\t\tif (!prefixcmp(line->buf, p->name) &&\n> +\t\t\t\t(strlen(p->name) == line->len || line->buf[strlen(p->name)] == ' ')) {\n\nA line way too wide.\n\n> +\t\t\tif (p->batchable) {\n> +\t\t\t\tbatch_cmd = p;\n> +\t\t\t\tstring_list_append(&batchlines, line->buf);\n> +\t\t\t\treturn 0;\n> +\t\t\t}\n> +\t\t\treturn p->fct(line->buf);\n> +\t\t}\n\nOK, so a command word on a line by itself, or a command word\nfollowed by a SP (probably followed by its arguments) on a line\ntriggers a command lookup, and individual command implementation\nparses the line.\n\n> +\t}\n> +\twarning(\"Unknown command '%s'\\n\", line->buf);\n> +\treturn 0;\n\nWhy isn't this an error?\n\n> +}\n> +\n> +int main(int argc, const char **argv)\n> +{\n> +\tstruct strbuf buf = STRBUF_INIT;\n> +\tint nongit;\n> +\tstatic struct remote *remote;\n> +\tconst char *url_in;\n> +\n> +\tgit_extract_argv0_path(argv[0]);\n> +\tsetup_git_directory_gently(&nongit);\n> +\tif (argc < 2 || argc > 3) {\n> +\t\tusage(\"git-remote-svn <remote-name> [<url>]\");\n> +\t\treturn 1;\n> +\t}\n\nIf this is an importer, you would be importing _into_ a git\nrepository, no?  How can you not error out when you are not in one?\nIn other words, why &nongit with *_gently()?\n\n> +\tremote = remote_get(argv[1]);\n> +\turl_in = remote->url[0];\n> +\tif (argc == 3)\n> +\t\turl_in = argv[2];\n\nShouldn't it be more like this?\n\n\turl_in = (argc == 3) ? argv[2] : remote->url[0];\n\n> +\tend_url_with_slash(&buf, url_in);\n> +\turl = strbuf_detach(&buf, NULL);\n> +\n> +\tstrbuf_addf(&buf, \"refs/svn/%s/master\", remote->name);\n> +\tprivate_ref = strbuf_detach(&buf, NULL);\n> +\n> +\twhile(1) {\n> +\t\tif (strbuf_getline(&buf, stdin, '\\n') == EOF) {\n> +\t\t\tif (ferror(stdin))\n> +\t\t\t\tdie(\"Error reading command stream\");\n> +\t\t\telse\n> +\t\t\t\tdie(\"Unexpected end of command stream\");\n> +\t\t}\n> +\t\tif (do_command(&buf))\n> +\t\t\tbreak;\n> +\t\tstrbuf_reset(&buf);\n> +\t}\n> +\n> +\tstrbuf_release(&buf);\n> +\tfree((void*)url);\n> +\tfree((void*)private_ref);\n> +\treturn 0;\n> +}\n"},{"id":"197013","messageId":"7vd32t9qp7.fsf@alter.siamese.dyndns.org","threadId":"31252","inReplyTo":"1344971598-8213-3-git-send-email-florian.achleitner.2.6.31@gmail.com","subject":"Re: [PATCH/RFC v3 02/16] Integrate remote-svn into svn-fe/Makefile.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-14T20:14:12Z","receivedAt":"2012-08-14T20:14:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Florian Achleitner <florian.achleitner.2.6.31@gmail.com> writes:\n\n> Requires some sha.h to be used and the libraries\n> to be linked, this is currently hardcoded.\n>\n> Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n> ---\n>  contrib/svn-fe/Makefile |   16 ++++++++++------\n>  1 file changed, 10 insertions(+), 6 deletions(-)\n>\n> diff --git a/contrib/svn-fe/Makefile b/contrib/svn-fe/Makefile\n> index 360d8da..8f0eec2 100644\n> --- a/contrib/svn-fe/Makefile\n> +++ b/contrib/svn-fe/Makefile\n> @@ -1,14 +1,14 @@\n> -all:: svn-fe$X\n> +all:: svn-fe$X remote-svn$X\n>  \n>  CC = gcc\n>  RM = rm -f\n>  MV = mv\n>  \n> -CFLAGS = -g -O2 -Wall\n> +CFLAGS = -g -O2 -Wall -DSHA1_HEADER='<openssl/sha.h>' -Wdeclaration-after-statement\n>  LDFLAGS =\n>  ALL_CFLAGS = $(CFLAGS)\n>  ALL_LDFLAGS = $(LDFLAGS)\n> -EXTLIBS =\n> +EXTLIBS = -lssl -lcrypto -lpthread ../../xdiff/lib.a\n\nI haven't looked carefully, but didn't we have to do a bit more\nelaborate when linking with ssl/crypto in our main Makefile to be\nportable across various vintages of OpenSSL libraries?\n\nDoes contrib/svn-fe/ already depend on OpenSSL by the way?  It needs\nto be documented somewhere in the same directory.\n\nIf one builds the main Git binary with NO_OPENSSL, can this still be\nbuilt and linked?\n\nWhat does this use xdiff/lib.a for?\n\nThe above are just mental notes; I didn't read the later patches in\nthe series that may already address these issues.\n\n>  GIT_LIB = ../../libgit.a\n>  VCSSVN_LIB = ../../vcs-svn/lib.a\n> @@ -37,8 +37,12 @@ svn-fe$X: svn-fe.o $(VCSSVN_LIB) $(GIT_LIB)\n>  \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ svn-fe.o \\\n>  \t\t$(ALL_LDFLAGS) $(LIBS)\n>  \n> -svn-fe.o: svn-fe.c ../../vcs-svn/svndump.h\n> -\t$(QUIET_CC)$(CC) -I../../vcs-svn -o $*.o -c $(ALL_CFLAGS) $<\n> +remote-svn$X: remote-svn.o $(VCSSVN_LIB) $(GIT_LIB)\n> +\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ remote-svn.o \\\n> +\t\t$(ALL_LDFLAGS) $(LIBS)\n> +\t\t\n> +%.o: %.c ../../vcs-svn/svndump.h\n> +\t$(QUIET_CC)$(CC) -I../../vcs-svn -I../../ -o $*.o -c $(ALL_CFLAGS) $<\n>  \n>  svn-fe.html: svn-fe.txt\n>  \t$(QUIET_SUBDIR0)../../Documentation $(QUIET_SUBDIR1) \\\n> @@ -58,6 +62,6 @@ svn-fe.1: svn-fe.txt\n>  \t$(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) libgit.a\n>  \n>  clean:\n> -\t$(RM) svn-fe$X svn-fe.o svn-fe.html svn-fe.xml svn-fe.1\n> +\t$(RM) svn-fe$X svn-fe.o svn-fe.html svn-fe.xml svn-fe.1 remote-svn.o\n>  \n>  .PHONY: all clean FORCE\n"},{"id":"197014","messageId":"7v8vdh9qmx.fsf@alter.siamese.dyndns.org","threadId":"31252","inReplyTo":"1344971598-8213-4-git-send-email-florian.achleitner.2.6.31@gmail.com","subject":"Re: [PATCH/RFC v3 03/16] Add svndump_init_fd to allow reading dumps from arbitrary FDs.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-14T20:15:34Z","receivedAt":"2012-08-14T20:15:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Florian Achleitner <florian.achleitner.2.6.31@gmail.com> writes:\n\n> The existing function only allows reading from a filename or\n> from stdin. Allow passing of a FD and an additional FD for\n> the back report pipe. This allows us to retrieve the name of\n> the pipe in the caller.\n>\n> Fixes the filename could be NULL bug.\n\nWhat bug?  Was this line meant to go in the log message?\n\n> Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n> ---\n> - dup input file descriptor, because buffer_deinit closes the fd.\n>  vcs-svn/svndump.c |   22 ++++++++++++++++++----\n>  vcs-svn/svndump.h |    1 +\n>  2 files changed, 19 insertions(+), 4 deletions(-)\n>\n> diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\n> index 2b168ae..d81a078 100644\n> --- a/vcs-svn/svndump.c\n> +++ b/vcs-svn/svndump.c\n> @@ -468,11 +468,9 @@ void svndump_read(const char *url)\n>  \t\tend_revision();\n>  }\n>  \n> -int svndump_init(const char *filename)\n> +static void init(int report_fd)\n>  {\n> -\tif (buffer_init(&input, filename))\n> -\t\treturn error(\"cannot open %s: %s\", filename, strerror(errno));\n> -\tfast_export_init(REPORT_FILENO);\n> +\tfast_export_init(report_fd);\n>  \tstrbuf_init(&dump_ctx.uuid, 4096);\n>  \tstrbuf_init(&dump_ctx.url, 4096);\n>  \tstrbuf_init(&rev_ctx.log, 4096);\n> @@ -482,6 +480,22 @@ int svndump_init(const char *filename)\n>  \treset_dump_ctx(NULL);\n>  \treset_rev_ctx(0);\n>  \treset_node_ctx(NULL);\n> +\treturn;\n> +}\n> +\n> +int svndump_init(const char *filename)\n> +{\n> +\tif (buffer_init(&input, filename))\n> +\t\treturn error(\"cannot open %s: %s\", filename ? filename : \"NULL\", strerror(errno));\n> +\tinit(REPORT_FILENO);\n> +\treturn 0;\n> +}\n> +\n> +int svndump_init_fd(int in_fd, int back_fd)\n> +{\n> +\tif(buffer_fdinit(&input, xdup(in_fd)))\n> +\t\treturn error(\"cannot open fd %d: %s\", in_fd, strerror(errno));\n> +\tinit(xdup(back_fd));\n>  \treturn 0;\n>  }\n>  \n> diff --git a/vcs-svn/svndump.h b/vcs-svn/svndump.h\n> index df9ceb0..acb5b47 100644\n> --- a/vcs-svn/svndump.h\n> +++ b/vcs-svn/svndump.h\n> @@ -2,6 +2,7 @@\n>  #define SVNDUMP_H_\n>  \n>  int svndump_init(const char *filename);\n> +int svndump_init_fd(int in_fd, int back_fd);\n>  void svndump_read(const char *url);\n>  void svndump_deinit(void);\n>  void svndump_reset(void);\n"},{"id":"197017","messageId":"7v4no59phn.fsf@alter.siamese.dyndns.org","threadId":"31252","inReplyTo":"1344971598-8213-5-git-send-email-florian.achleitner.2.6.31@gmail.com","subject":"Re: [PATCH/RFC v3 04/16] Connect fast-import to the remote-helper via pipe, adding 'bidi-import' capability.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-14T20:40:20Z","receivedAt":"2012-08-14T20:40:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Florian Achleitner <florian.achleitner.2.6.31@gmail.com> writes:\n\n> The fast-import commands 'cat-blob' and 'ls' can be used by remote-helpers\n> to retrieve information about blobs and trees that already exist in\n> fast-import's memory. This requires a channel from fast-import to the\n> remote-helper.\n> remote-helpers that use this features shall advertise the new 'bidi-import'\n\ns/this fea/these fea/\n\n> capability so signal that they require the communication channel.\n\ns/so sig/to sig/, I think.\n\n> When forking fast-import in transport-helper.c connect it to a dup of\n> the remote-helper's stdin-pipe. The additional file descriptor is passed\n> to fast-import via it's command line (--cat-blob-fd).\n\ns/via it's/via its/;\n\n> It follows that git and fast-import are connected to the remote-helpers's\n> stdin.\n> Because git can send multiple commands to the remote-helper on it's stdin,\n> it is required that helpers that advertise 'bidi-import' buffer all input\n> commands until the batch of 'import' commands is ended by a newline\n> before sending data to fast-import.\n> This is to prevent mixing commands and fast-import responses on the\n> helper's stdin.\n\nPlease have a blank line each between paragraphs; a solid block of\ntext is very hard to follow.\n\n> Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n> ---\n>  transport-helper.c |   45 ++++++++++++++++++++++++++++++++-------------\n>  1 file changed, 32 insertions(+), 13 deletions(-)\n>\n> diff --git a/transport-helper.c b/transport-helper.c\n> index cfe0988..257274b 100644\n> --- a/transport-helper.c\n> +++ b/transport-helper.c\n> @@ -10,6 +10,7 @@\n>  #include \"string-list.h\"\n>  #include \"thread-utils.h\"\n>  #include \"sigchain.h\"\n> +#include \"argv-array.h\"\n>  \n>  static int debug;\n>  \n> @@ -19,6 +20,7 @@ struct helper_data {\n>  \tFILE *out;\n>  \tunsigned fetch : 1,\n>  \t\timport : 1,\n> +\t\tbidi_import : 1,\n>  \t\texport : 1,\n>  \t\toption : 1,\n>  \t\tpush : 1,\n> @@ -101,6 +103,7 @@ static void do_take_over(struct transport *transport)\n>  static struct child_process *get_helper(struct transport *transport)\n>  {\n>  \tstruct helper_data *data = transport->data;\n> +\tstruct argv_array argv = ARGV_ARRAY_INIT;\n>  \tstruct strbuf buf = STRBUF_INIT;\n>  \tstruct child_process *helper;\n>  \tconst char **refspecs = NULL;\n> @@ -122,11 +125,10 @@ static struct child_process *get_helper(struct transport *transport)\n>  \thelper->in = -1;\n>  \thelper->out = -1;\n>  \thelper->err = 0;\n> -\thelper->argv = xcalloc(4, sizeof(*helper->argv));\n> -\tstrbuf_addf(&buf, \"git-remote-%s\", data->name);\n> -\thelper->argv[0] = strbuf_detach(&buf, NULL);\n> -\thelper->argv[1] = transport->remote->name;\n> -\thelper->argv[2] = remove_ext_force(transport->url);\n> +\targv_array_pushf(&argv, \"git-remote-%s\", data->name);\n> +\targv_array_push(&argv, transport->remote->name);\n> +\targv_array_push(&argv, remove_ext_force(transport->url));\n> +\thelper->argv = argv.argv;\n\nMuch nicer than before thanks to argv_array ;-)\n\n>  \thelper->git_cmd = 0;\n>  \thelper->silent_exec_failure = 1;\n>  \n> @@ -141,6 +143,8 @@ static struct child_process *get_helper(struct transport *transport)\n>  \n>  \tdata->helper = helper;\n>  \tdata->no_disconnect_req = 0;\n> +\tfree((void*) helper_env[1]);\n\nWhat is this free() for???\n\n> +\targv_array_clear(&argv);\n\nSee below.\n\n>  \t/*\n>  \t * Open the output as FILE* so strbuf_getline() can be used.\n> @@ -178,6 +182,8 @@ static struct child_process *get_helper(struct transport *transport)\n>  \t\t\tdata->push = 1;\n>  \t\telse if (!strcmp(capname, \"import\"))\n>  \t\t\tdata->import = 1;\n> +\t\telse if (!strcmp(capname, \"bidi-import\"))\n> +\t\t\tdata->bidi_import = 1;\n>  \t\telse if (!strcmp(capname, \"export\"))\n>  \t\t\tdata->export = 1;\n>  \t\telse if (!data->refspecs && !prefixcmp(capname, \"refspec \")) {\n> @@ -241,8 +247,6 @@ static int disconnect_helper(struct transport *transport)\n>  \t\tclose(data->helper->out);\n>  \t\tfclose(data->out);\n>  \t\tres = finish_command(data->helper);\n> -\t\tfree((char *)data->helper->argv[0]);\n> -\t\tfree(data->helper->argv);\n>  \t\tfree(data->helper);\n>  \t\tdata->helper = NULL;\n>  \t}\n> @@ -376,14 +380,24 @@ static int fetch_with_fetch(struct transport *transport,\n>  static int get_importer(struct transport *transport, struct child_process *fastimport)\n>  {\n>  \tstruct child_process *helper = get_helper(transport);\n> +\tstruct helper_data *data = transport->data;\n> +\tstruct argv_array argv = ARGV_ARRAY_INIT;\n> +\tint cat_blob_fd, code;\n>  \tmemset(fastimport, 0, sizeof(*fastimport));\n>  \tfastimport->in = helper->out;\n> -\tfastimport->argv = xcalloc(5, sizeof(*fastimport->argv));\n> -\tfastimport->argv[0] = \"fast-import\";\n> -\tfastimport->argv[1] = \"--quiet\";\n> +\targv_array_push(&argv, \"fast-import\");\n> +\targv_array_push(&argv, \"--quiet\");\n>  \n> +\tif (data->bidi_import) {\n> +\t\tcat_blob_fd = xdup(helper->in);\n> +\t\targv_array_pushf(&argv, \"--cat-blob-fd=%d\", cat_blob_fd);\n> +\t}\n> +\tfastimport->argv = argv.argv;\n>  \tfastimport->git_cmd = 1;\n> -\treturn start_command(fastimport);\n> +\n> +\tcode = start_command(fastimport);\n> +\targv_array_clear(&argv);\n> +\treturn code;\n>  }\n>  \n>  static int get_exporter(struct transport *transport,\n> @@ -438,11 +452,16 @@ static int fetch_with_import(struct transport *transport,\n>  \t}\n>  \n>  \twrite_constant(data->helper->in, \"\\n\");\n> +\t/*\n> +\t * remote-helpers that advertise the bidi-import capability are required to\n> +\t * buffer the complete batch of import commands until this newline before\n> +\t * sending data to fast-import.\n> +\t * These helpers read back data from fast-import on their stdin, which could\n> +\t * be mixed with import commands, otherwise.\n> +\t */\n>  \n>  \tif (finish_command(&fastimport))\n>  \t\tdie(\"Error while running fast-import\");\n> -\tfree(fastimport.argv);\n> -\tfastimport.argv = NULL;\n\nThe updated code frees argv[] immediately after start_command()\nreturns, and it may happen to be safe to do so with the current\nimplementation of start_command() and friends, but I think it is a\nbad taste to free argv[] (or env[] for that matter) before calling\nfinish_command().  These pieces of memory are still pointed by the\nchild_process structure, and users of the structure may want to use\ncontents of them (especially, argv[0]) for reporting errors and\nvarious other purposes, e.g.\n\n\tchild = get_helper();\n\n        trace(\"started %s\\n\", child->argv[0]);\n\n\tif (finish_command(child))\n        \treturn error(\"failed to cleanly finish %s\", child->argv[0]);\n"},{"id":"197018","messageId":"7vzk5x8aru.fsf@alter.siamese.dyndns.org","threadId":"31252","inReplyTo":"1344971598-8213-8-git-send-email-florian.achleitner.2.6.31@gmail.com","subject":"Re: [PATCH/RFC v3 07/16] Add a symlink 'git-remote-svn' in base dir.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-14T20:43:33Z","receivedAt":"2012-08-14T20:43:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Florian Achleitner <florian.achleitner.2.6.31@gmail.com> writes:\n\n> Allow execution of git-remote-svn even if the binary\n> currently is located in contrib/svn-fe/.\n>\n> Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n> ---\n>  git-remote-svn |    1 +\n>  1 file changed, 1 insertion(+)\n>  create mode 120000 git-remote-svn\n>\n> diff --git a/git-remote-svn b/git-remote-svn\n> new file mode 120000\n> index 0000000..d3b1c07\n> --- /dev/null\n> +++ b/git-remote-svn\n> @@ -0,0 +1 @@\n> +contrib/svn-fe/remote-svn\n> \\ No newline at end of file\n\nNot paying enough attention to the patch you are sending?\n"},{"id":"197019","messageId":"7vvcgl8amk.fsf@alter.siamese.dyndns.org","threadId":"31252","inReplyTo":"1344971598-8213-8-git-send-email-florian.achleitner.2.6.31@gmail.com","subject":"Re: [PATCH/RFC v3 07/16] Add a symlink 'git-remote-svn' in base dir.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-14T20:46:43Z","receivedAt":"2012-08-14T20:46:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Florian Achleitner <florian.achleitner.2.6.31@gmail.com> writes:\n\n> Allow execution of git-remote-svn even if the binary\n> currently is located in contrib/svn-fe/.\n>\n> Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n> ---\n>  git-remote-svn |    1 +\n>  1 file changed, 1 insertion(+)\n>  create mode 120000 git-remote-svn\n>\n> diff --git a/git-remote-svn b/git-remote-svn\n> new file mode 120000\n> index 0000000..d3b1c07\n> --- /dev/null\n> +++ b/git-remote-svn\n> @@ -0,0 +1 @@\n> +contrib/svn-fe/remote-svn\n> \\ No newline at end of file\n\nPlease scratch my previous comment.  I thought you were adding an\nentry to .gitignore or something.\n\nI'd rather not to see such a symbolic link that points at a build\nproduct in the source tree.  Making a symlink from the toplevel\nMakefile _after_ we built it in contrib/svn-fe/ (and removing it\nupon \"make clean\") is OK, though.\n"},{"id":"197030","messageId":"CAFfmPPO4FRBocLDZr8WU2GJOTVXFY6+8jjO8mEoL+aRPtp6k2Q@mail.gmail.com","threadId":"31252","inReplyTo":"1344971598-8213-1-git-send-email-florian.achleitner.2.6.31@gmail.com","subject":"Re: [PATCH/RFC v3 00/16] GSOC remote-svn","fromName":"David Michael Barr","fromEmail":"davidbarr@google.com","sentAt":"2012-08-14T23:59:12Z","receivedAt":"2012-08-14T23:59:12Z","isPatch":true,"sender":{"key":"davidbarr@google.com","avatar":"https://avatars.githubusercontent.com/u/220594?v=4"},"body":"On Wed, Aug 15, 2012 at 5:13 AM, Florian Achleitner\n<florian.achleitner.2.6.31@gmail.com> wrote:\n> Hi.\n>\n> Version 3 of this series adds the 'bidi-import' capability, as suggested\n> Jonathan.\n> Diff details are attached to the patches.\n> 04 and 05 are completely new.\n>\n> [PATCH/RFC v3 01/16] Implement a remote helper for svn in C.\n> [PATCH/RFC v3 02/16] Integrate remote-svn into svn-fe/Makefile.\n> [PATCH/RFC v3 03/16] Add svndump_init_fd to allow reading dumps from\n> [PATCH/RFC v3 04/16] Connect fast-import to the remote-helper via\n> [PATCH/RFC v3 05/16] Add documentation for the 'bidi-import'\n> [PATCH/RFC v3 06/16] remote-svn, vcs-svn: Enable fetching to private\n> [PATCH/RFC v3 07/16] Add a symlink 'git-remote-svn' in base dir.\n> [PATCH/RFC v3 08/16] Allow reading svn dumps from files via file://\n> [PATCH/RFC v3 09/16] vcs-svn: add fast_export_note to create notes\n> [PATCH/RFC v3 10/16] Create a note for every imported commit\n> [PATCH/RFC v3 11/16] When debug==1, start fast-import with \"--stats\"\n> [PATCH/RFC v3 12/16] remote-svn: add incremental import.\n> [PATCH/RFC v3 13/16] Add a svnrdump-simulator replaying a dump file\n> [PATCH/RFC v3 14/16] transport-helper: add import|export-marks to\n> [PATCH/RFC v3 15/16] remote-svn: add marks-file regeneration.\n> [PATCH/RFC v3 16/16] Add a test script for remote-svn.\n\nThank you Florian, this series was a great read. My apologies for the\nlimited interaction over the course of summer. You have done well and\nengaged with the community to produce this result.\n\nThank you Jonathan for the persistent reviews. No doubt they have\ncontributed to the quality of the series.\n\nThank you Junio for your dedication to reviewing the traffic on this\nmailing list.\n\nI will no longer be reachable on this address after Friday.\n\nI hope to make future contributions with the identity:\nDavid Michael Barr <b@rr-dav.id.au>\nThis will be my persistent address.\n\n--\nDavid Barr\n"},{"id":"197035","messageId":"2098867.sfVpB1786h@flomedio","threadId":"31252","inReplyTo":"7vd32t9qp7.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC v3 02/16] Integrate remote-svn into svn-fe/Makefile.","fromName":"Florian Achleitner","fromEmail":"florian.achleitner.2.6.31@gmail.com","sentAt":"2012-08-15T08:54:39Z","receivedAt":"2012-08-15T08:54:39Z","isPatch":true,"sender":{"key":"florian.achleitner.2.6.31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/880777?v=4"},"body":"On Tuesday 14 August 2012 13:14:12 Junio C Hamano wrote:\n> Florian Achleitner <florian.achleitner.2.6.31@gmail.com> writes:\n> > Requires some sha.h to be used and the libraries\n> > to be linked, this is currently hardcoded.\n> > \n> > Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n> > ---\n> > \n> >  contrib/svn-fe/Makefile |   16 ++++++++++------\n> >  1 file changed, 10 insertions(+), 6 deletions(-)\n> > \n> > diff --git a/contrib/svn-fe/Makefile b/contrib/svn-fe/Makefile\n> > index 360d8da..8f0eec2 100644\n> > --- a/contrib/svn-fe/Makefile\n> > +++ b/contrib/svn-fe/Makefile\n> > @@ -1,14 +1,14 @@\n> > -all:: svn-fe$X\n> > +all:: svn-fe$X remote-svn$X\n> > \n> >  CC = gcc\n> >  RM = rm -f\n> >  MV = mv\n> > \n> > -CFLAGS = -g -O2 -Wall\n> > +CFLAGS = -g -O2 -Wall -DSHA1_HEADER='<openssl/sha.h>'\n> > -Wdeclaration-after-statement> \n> >  LDFLAGS =\n> >  ALL_CFLAGS = $(CFLAGS)\n> >  ALL_LDFLAGS = $(LDFLAGS)\n> > \n> > -EXTLIBS =\n> > +EXTLIBS = -lssl -lcrypto -lpthread ../../xdiff/lib.a\n> \n> I haven't looked carefully, but didn't we have to do a bit more\n> elaborate when linking with ssl/crypto in our main Makefile to be\n> portable across various vintages of OpenSSL libraries?\n> \n> Does contrib/svn-fe/ already depend on OpenSSL by the way?  It needs\n> to be documented somewhere in the same directory.\n> \n> If one builds the main Git binary with NO_OPENSSL, can this still be\n> built and linked?\n> \n> What does this use xdiff/lib.a for?\n> \n> The above are just mental notes; I didn't read the later patches in\n> the series that may already address these issues.\n\nFor the makefile, I've to say that this is just a hack to make it work. I'm not \nsure how it would be correctly integrated into git's makefile hierarchy.\nThe OPENSSL header and the xdiff/lib.a are here because it doesn't work \notherwise. I need to dig into that to find out why. Any tips how to do it \nright?\n \n> >  GIT_LIB = ../../libgit.a\n> >  VCSSVN_LIB = ../../vcs-svn/lib.a\n> > \n> > @@ -37,8 +37,12 @@ svn-fe$X: svn-fe.o $(VCSSVN_LIB) $(GIT_LIB)\n> > \n> >  \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ svn-fe.o \\\n> >  \t\n> >  \t\t$(ALL_LDFLAGS) $(LIBS)\n> > \n> > -svn-fe.o: svn-fe.c ../../vcs-svn/svndump.h\n> > -\t$(QUIET_CC)$(CC) -I../../vcs-svn -o $*.o -c $(ALL_CFLAGS) $<\n> > +remote-svn$X: remote-svn.o $(VCSSVN_LIB) $(GIT_LIB)\n> > +\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ remote-svn.o \\\n> > +\t\t$(ALL_LDFLAGS) $(LIBS)\n> > +\n> > +%.o: %.c ../../vcs-svn/svndump.h\n> > +\t$(QUIET_CC)$(CC) -I../../vcs-svn -I../../ -o $*.o -c $(ALL_CFLAGS) $<\n> > \n> >  svn-fe.html: svn-fe.txt\n> >  \n> >  \t$(QUIET_SUBDIR0)../../Documentation $(QUIET_SUBDIR1) \\\n> > \n> > @@ -58,6 +62,6 @@ svn-fe.1: svn-fe.txt\n> > \n> >  \t$(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) libgit.a\n> >  \n> >  clean:\n> > -\t$(RM) svn-fe$X svn-fe.o svn-fe.html svn-fe.xml svn-fe.1\n> > +\t$(RM) svn-fe$X svn-fe.o svn-fe.html svn-fe.xml svn-fe.1 remote-svn.o\n> > \n> >  .PHONY: all clean FORCE\n"},{"id":"197036","messageId":"2021296.QelIutNuid@flomedio","threadId":"31252","inReplyTo":"7vvcgl8amk.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC v3 07/16] Add a symlink 'git-remote-svn' in base dir.","fromName":"Florian Achleitner","fromEmail":"florian.achleitner.2.6.31@gmail.com","sentAt":"2012-08-15T09:20:34Z","receivedAt":"2012-08-15T09:20:34Z","isPatch":true,"sender":{"key":"florian.achleitner.2.6.31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/880777?v=4"},"body":"On Tuesday 14 August 2012 13:46:43 Junio C Hamano wrote:\n> Florian Achleitner <florian.achleitner.2.6.31@gmail.com> writes:\n> > Allow execution of git-remote-svn even if the binary\n> > currently is located in contrib/svn-fe/.\n> > \n> > Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n> > ---\n> > \n> >  git-remote-svn |    1 +\n> >  1 file changed, 1 insertion(+)\n> >  create mode 120000 git-remote-svn\n> > \n> > diff --git a/git-remote-svn b/git-remote-svn\n> > new file mode 120000\n> > index 0000000..d3b1c07\n> > --- /dev/null\n> > +++ b/git-remote-svn\n> > @@ -0,0 +1 @@\n> > +contrib/svn-fe/remote-svn\n> > \\ No newline at end of file\n> \n> Please scratch my previous comment.  I thought you were adding an\n> entry to .gitignore or something.\n> \n> I'd rather not to see such a symbolic link that points at a build\n> product in the source tree.  Making a symlink from the toplevel\n> Makefile _after_ we built it in contrib/svn-fe/ (and removing it\n> upon \"make clean\") is OK, though.\n\nAs with the makefile in contrib/svn-fe, this is just a hack. The toplevel \nMakefile doesn't seem to build contrib/* at all. I always need to call make \nexplicitly in these subdirs.\n"},{"id":"197038","messageId":"11611888.mV2cRFPk88@flomedio","threadId":"31252","inReplyTo":"1344971598-8213-17-git-send-email-florian.achleitner.2.6.31@gmail.com","subject":"Re: [PATCH/RFC v3 16/16] Add a test script for remote-svn.","fromName":"Florian Achleitner","fromEmail":"florian.achleitner2.6.31@gmail.com","sentAt":"2012-08-15T11:46:26Z","receivedAt":"2012-08-15T11:46:26Z","isPatch":true,"sender":{"key":"florian.achleitner.2.6.31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/880777?v=4"},"body":"Forget this patch! It contains some unwanted content. Something with rebasing \nwent wrong..\n\nOn Tuesday 14 August 2012 21:13:18 Florian Achleitner wrote:\n> Use svnrdump_sim.py to emulate svnrdump without an svn server.\n> Tests fetching, incremental fetching, fetching from file://,\n> and the regeneration of fast-import's marks file.\n> \n> Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n> ---\n>  t/t9020-remote-svn.sh |   69\n> +++++++++++++++++++++++++++++++++++++++++++++++++ transport-helper.c    |  \n> 15 ++++++-----\n>  2 files changed, 77 insertions(+), 7 deletions(-)\n>  create mode 100755 t/t9020-remote-svn.sh\n> \n> diff --git a/t/t9020-remote-svn.sh b/t/t9020-remote-svn.sh\n> new file mode 100755\n> index 0000000..a0c6a21\n> --- /dev/null\n> +++ b/t/t9020-remote-svn.sh\n> @@ -0,0 +1,69 @@\n> +#!/bin/sh\n> +\n> +test_description='tests remote-svn'\n> +\n> +. ./test-lib.sh\n> +\n> +# We override svnrdump by placing a symlink to the svnrdump-emulator in .\n> +export PATH=\"$HOME:$PATH\"\n> +ln -sf $GIT_BUILD_DIR/contrib/svn-fe/svnrdump_sim.py \"$HOME/svnrdump\"\n> +\n> +init_git () {\n> +\trm -fr .git &&\n> +\tgit init &&\n> +\t#git remote add svnsim svn::sim:///$TEST_DIRECTORY/t9020/example.svnrdump\n> +\t# let's reuse an exisiting dump file!?\n> +\tgit remote add svnsim svn::sim:///$TEST_DIRECTORY/t9154/svn.dump\n> +\tgit remote add svnfile svn::file:///$TEST_DIRECTORY/t9154/svn.dump\n> +}\n> +\n> +test_debug '\n> +\tgit --version\n> +\twhich git\n> +\twhich svnrdump\n> +'\n> +\n> +test_expect_success 'simple fetch' '\n> +\tinit_git &&\n> +\tgit fetch svnsim &&\n> +\ttest_cmp .git/refs/svn/svnsim/master .git/refs/remotes/svnsim/master  &&\n> +\tcp .git/refs/remotes/svnsim/master master.good\n> +'\n> +\n> +test_debug '\n> +\tcat .git/refs/svn/svnsim/master\n> +\tcat .git/refs/remotes/svnsim/master\n> +'\n> +\n> +test_expect_success 'repeated fetch, nothing shall change' '\n> +\tgit fetch svnsim &&\n> +\ttest_cmp master.good .git/refs/remotes/svnsim/master\n> +'\n> +\n> +test_expect_success 'fetch from a file:// url gives the same result' '\n> +\tgit fetch svnfile\n> +'\n> +\n> +test_expect_failure 'the sha1 differ because the git-svn-id line in the\n> commit msg contains the url' ' +\ttest_cmp .git/refs/remotes/svnfile/master\n> .git/refs/remotes/svnsim/master +'\n> +\n> +test_expect_success 'mark-file regeneration' '\n> +\tmv .git/info/fast-import/marks/svnsim\n> .git/info/fast-import/marks/svnsim.old && +\tgit fetch svnsim &&\n> +\ttest_cmp .git/info/fast-import/marks/svnsim.old\n> .git/info/fast-import/marks/svnsim +'\n> +\n> +test_expect_success 'incremental imports must lead to the same head' '\n> +\texport SVNRMAX=3 &&\n> +\tinit_git &&\n> +\tgit fetch svnsim &&\n> +\ttest_cmp .git/refs/svn/svnsim/master .git/refs/remotes/svnsim/master  &&\n> +\tunset SVNRMAX &&\n> +\tgit fetch svnsim &&\n> +\ttest_cmp master.good .git/refs/remotes/svnsim/master\n> +'\n> +\n> +test_debug 'git branch -a'\n> +\n> +test_done\n> diff --git a/transport-helper.c b/transport-helper.c\n> index 47db055..a363f2c 100644\n> --- a/transport-helper.c\n> +++ b/transport-helper.c\n> @@ -17,6 +17,7 @@ static int debug;\n>  struct helper_data {\n>  \tconst char *name;\n>  \tstruct child_process *helper;\n> +\tstruct argv_array argv;\n>  \tFILE *out;\n>  \tunsigned fetch : 1,\n>  \t\timport : 1,\n> @@ -103,7 +104,6 @@ static void do_take_over(struct transport *transport)\n>  static struct child_process *get_helper(struct transport *transport)\n>  {\n>  \tstruct helper_data *data = transport->data;\n> -\tstruct argv_array argv = ARGV_ARRAY_INIT;\n>  \tstruct strbuf buf = STRBUF_INIT;\n>  \tstruct child_process *helper;\n>  \tconst char **refspecs = NULL;\n> @@ -125,10 +125,11 @@ static struct child_process *get_helper(struct\n> transport *transport) helper->in = -1;\n>  \thelper->out = -1;\n>  \thelper->err = 0;\n> -\targv_array_pushf(&argv, \"git-remote-%s\", data->name);\n> -\targv_array_push(&argv, transport->remote->name);\n> -\targv_array_push(&argv, remove_ext_force(transport->url));\n> -\thelper->argv = argv.argv;\n> +\targv_array_init(&data->argv);\n> +\targv_array_pushf(&data->argv, \"git-remote-%s\", data->name);\n> +\targv_array_push(&data->argv, transport->remote->name);\n> +\targv_array_push(&data->argv, remove_ext_force(transport->url));\n> +\thelper->argv = data->argv.argv;\n>  \thelper->git_cmd = 0;\n>  \thelper->silent_exec_failure = 1;\n> \n> @@ -143,8 +144,6 @@ static struct child_process *get_helper(struct transport\n> *transport)\n> \n>  \tdata->helper = helper;\n>  \tdata->no_disconnect_req = 0;\n> -\tfree((void*) helper_env[1]);\n> -\targv_array_clear(&argv);\n> \n>  \t/*\n>  \t * Open the output as FILE* so strbuf_getline() can be used.\n> @@ -247,6 +246,8 @@ static int disconnect_helper(struct transport\n> *transport) close(data->helper->out);\n>  \t\tfclose(data->out);\n>  \t\tres = finish_command(data->helper);\n> +\t\tfree((void*) data->helper->env[1]);\n> +\t\targv_array_clear(&data->argv);\n>  \t\tfree(data->helper);\n>  \t\tdata->helper = NULL;\n>  \t}\n"},{"id":"197039","messageId":"38446278.5ZUZChB0NF@flomedio","threadId":"31252","inReplyTo":"7vhas59r0b.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC v3 01/16] Implement a remote helper for svn in C.","fromName":"Florian Achleitner","fromEmail":"florian.achleitner.2.6.31@gmail.com","sentAt":"2012-08-15T12:00:31Z","receivedAt":"2012-08-15T12:00:31Z","isPatch":true,"sender":{"key":"florian.achleitner.2.6.31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/880777?v=4"},"body":"On Tuesday 14 August 2012 13:07:32 Junio C Hamano wrote:\n> Florian Achleitner <florian.achleitner.2.6.31@gmail.com> writes:\n> > Enable basic fetching from subversion repositories. When processing remote\n> > URLs starting with svn::, git invokes this remote-helper.\n> > It starts svnrdump to extract revisions from the subversion repository in\n> > the 'dump file format', and converts them to a git-fast-import stream\n> > using the functions of vcs-svn/.\n> \n> (nit) the above is a bit too wide, isn't it?\n> \n> > Imported refs are created in a private namespace at\n> > refs/svn/<remote-name/master.\n> (nit) missing closing '>'?\n> \n> > The revision history is imported linearly (no branch detection) and\n> > completely, i.e. from revision 0 to HEAD.\n> > \n> > The 'bidi-import' capability is used. The remote-helper expects data from\n> > fast-import on its stdin. It buffers a batch of 'import' command lines\n> > in a string_list before starting to process them.\n> > \n> > Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n> > ---\n> > diff:\n> > - incorporate review\n> > - remove redundant strbuf_init\n> > - add 'bidi-import' to capabilities\n> > - buffer all lines of a command batch in string_list\n> > \n> >  contrib/svn-fe/remote-svn.c |  183\n> >  +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 183\n> >  insertions(+)\n> >  create mode 100644 contrib/svn-fe/remote-svn.c\n> > \n> > diff --git a/contrib/svn-fe/remote-svn.c b/contrib/svn-fe/remote-svn.c\n> > new file mode 100644\n> > index 0000000..ce59344\n> > --- /dev/null\n> > +++ b/contrib/svn-fe/remote-svn.c\n> > @@ -0,0 +1,183 @@\n> > +\n> \n> Remove.\n> \n> > +#include \"cache.h\"\n> > +#include \"remote.h\"\n> > +#include \"strbuf.h\"\n> > +#include \"url.h\"\n> > +#include \"exec_cmd.h\"\n> > +#include \"run-command.h\"\n> > +#include \"svndump.h\"\n> > +#include \"notes.h\"\n> > +#include \"argv-array.h\"\n> > +\n> > +static const char *url;\n> > +static const char *private_ref;\n> > +static const char *remote_ref = \"refs/heads/master\";\n> \n> Just wondering; is this name \"master\" (or \"refs/heads/\" for that\n> matter) significant in any way when talking to a subversion remote?\n\nNo, it isn't. But it has to specify something in the list command.\n\n> \n> > +static int cmd_capabilities(const char *line);\n> > +static int cmd_import(const char *line);\n> > +static int cmd_list(const char *line);\n> > +\n> > +typedef int (*input_command_handler)(const char *);\n> > +struct input_command_entry {\n> > +\tconst char *name;\n> > +\tinput_command_handler fct;\n> > +\tunsigned char batchable;\t/* whether the command starts or is part of a\n> > batch */ +};\n> > +\n> > +static const struct input_command_entry input_command_list[] = {\n> > +\t\t{ \"capabilities\", cmd_capabilities, 0 },\n> \n> One level too deeply indented?\n> \n> > +\t\t{ \"import\", cmd_import, 1 },\n> > +\t\t{ \"list\", cmd_list, 0 },\n> > +\t\t{ NULL, NULL }\n> > +};\n> > +\n> > +static int cmd_capabilities(const char *line) {\n> > +\tprintf(\"import\\n\");\n> > +\tprintf(\"bidi-import\\n\");\n> > +\tprintf(\"refspec %s:%s\\n\\n\", remote_ref, private_ref);\n> > +\tfflush(stdout);\n> > +\treturn 0;\n> > +}\n> > +\n> > +static void terminate_batch(void)\n> > +{\n> > +\t/* terminate a current batch's fast-import stream */\n> > +\t\tprintf(\"done\\n\");\n> \n> Likewise.\n> \n> > +\t\tfflush(stdout);\n> > +}\n> > +\n> > +static int cmd_import(const char *line)\n> > +{\n> > +\tint code;\n> > +\tint dumpin_fd;\n> > +\tunsigned int startrev = 0;\n> > +\tstruct argv_array svndump_argv = ARGV_ARRAY_INIT;\n> > +\tstruct child_process svndump_proc;\n> > +\n> > +\tmemset(&svndump_proc, 0, sizeof (struct child_process));\n> \n> Please lose SP between sizeof and '('.\n> \n> > +\tsvndump_proc.out = -1;\n> > +\targv_array_push(&svndump_argv, \"svnrdump\");\n> > +\targv_array_push(&svndump_argv, \"dump\");\n> > +\targv_array_push(&svndump_argv, url);\n> > +\targv_array_pushf(&svndump_argv, \"-r%u:HEAD\", startrev);\n> > +\tsvndump_proc.argv = svndump_argv.argv;\n> \n> (just me making a mental note) We read from \"svnrdump\", which would\n> read (if it ever does) from the same stdin as ours and spits (if it\n> ever does) its errors to the same stderr as ours.\n\nYes. svnrdump sometimes queries passwords, in this case it reads stdin, from \nthe the terminal, and it writes errors and the password prompt to stderr, the \nterminal.\n\n> \n> > +\n> > +\tcode = start_command(&svndump_proc);\n> > +\tif (code)\n> > +\t\tdie(\"Unable to start %s, code %d\", svndump_proc.argv[0], code);\n> > +\tdumpin_fd = svndump_proc.out;\n> > +\n> > +\tcode = start_command(&svndump_proc);\n> > +\tif (code)\n> > +\t\tdie(\"Unable to start %s, code %d\", svndump_proc.argv[0], code);\n> > +\tdumpin_fd = svndump_proc.out;\n> \n> You start it twice without finishing the first invocation, or just a\n> double paste?\n\nSorry, thats garbage. It's meant to be only once. This was a mistake in a \nrebase merge.\n\n> \n> > +\tsvndump_init_fd(dumpin_fd, STDIN_FILENO);\n> > +\tsvndump_read(url, private_ref);\n> > +\tsvndump_deinit();\n> > +\tsvndump_reset();\n> > +\n> > +\tclose(dumpin_fd);\n> \n> (my mental note) And at this point, we finished feeding whatever\n> comes out of \"svnrdump\" to svndump_read().\n> \n> > +\tcode = finish_command(&svndump_proc);\n> > +\tif (code)\n> > +\t\twarning(\"%s, returned %d\", svndump_proc.argv[0], code);\n> > +\targv_array_clear(&svndump_argv);\n> > +\n> > +\treturn 0;\n> \n> Other than the \"twice?\" puzzle, this function looks straightforward.\n> \n> > +}\n> > +\n> > +static int cmd_list(const char *line)\n> > +{\n> > +\tprintf(\"? %s\\n\\n\", remote_ref);\n> > +\tfflush(stdout);\n> > +\treturn 0;\n> > +}\n> > +\n> > +static int do_command(struct strbuf *line)\n> > +{\n> > +\tconst struct input_command_entry *p = input_command_list;\n> > +\tstatic struct string_list batchlines = STRING_LIST_INIT_DUP;\n> > +\tstatic const struct input_command_entry *batch_cmd;\n> > +\t/*\n> > +\t * commands can be grouped together in a batch.\n> > +\t * Batches are ended by \\n. If no batch is active the program ends.\n> > +\t * During a batch all lines are buffered and passed to the handler\n> > function +\t * when the batch is terminated.\n> > +\t */\n> > +\tif (line->len == 0) {\n> > +\t\tif (batch_cmd) {\n> > +\t\t\tstruct string_list_item *item;\n> > +\t\t\tfor_each_string_list_item(item, &batchlines)\n> > +\t\t\t\tbatch_cmd->fct(item->string);\n> \n> (style) I think we tend to call these unnamed functions \"fn\" in our\n> codebase.\n> \n> > +\t\t\tterminate_batch();\n> > +\t\t\tbatch_cmd = NULL;\n> > +\t\t\tstring_list_clear(&batchlines, 0);\n> > +\t\t\treturn 0;\t/* end of the batch, continue reading other commands. */\n> > +\t\t}\n> > +\t\treturn 1;\t/* end of command stream, quit */\n> > +\t}\n> > +\tif (batch_cmd) {\n> > +\t\tif (strcmp(batch_cmd->name, line->buf))\n> > +\t\t\tdie(\"Active %s batch interrupted by %s\", batch_cmd->name, line-\n>buf);\n> > +\t\t/* buffer batch lines */\n> > +\t\tstring_list_append(&batchlines, line->buf);\n> > +\t\treturn 0;\n> > +\t}\n> \n> A \"batch-able\" command, e.g. \"import\", will first cause the\n> batch_cmd to point at the command structure in this function, and\n> then the next and subsequent lines, as long as the input line is\n> exactly the same as the current batch_cmd->name, e.g. \"import\", is\n> appended into batchlines.\n> \n> Would this mean that you can feed something like this:\n> \n> \timport foobar\n>         import\n>         import\n>         import\n> \n>         another command\n> \n> and buffer the four \"import\" lines in batchlines, and then on the\n> empty line, have the for-each-string-list-item loop to call\n> cmd_import() on \"import foobar\", \"import\", \"import\", then \"import\"\n> (literally, without anything other than \"import\" on the line).\n> \n> How is that useful?  With that \"if (strcmp(batch_cmd->name, line->buf))\",\n> I cannot think of other valid input to make this \"batch\" mechanism\n> to trigger and do something useful.  Am I missing something?\n> \nThat's a mistake I made when adding the line buffering. It should allow \ncommands like:\n\nimport foo\nimport bar\n\nsome other command.\n\nFor this helper in it's current state it's anyways not useful to send more \nthan one import command, because it can only import revisions it doesn't yet \nhave. So a second import would always do nothing.\nThe import batches are specified in Documentation/git-remote-helpers.txt.\nSo this helper should support it, I thought.\nThe buffering of import lines is necessary with the bidi-import capability, \nsuggested by Jonathan. It uses the helper's stdin for command input and for \nreading fast-imports output. So we must make sure these two datastreams don't \nmix.\n\n> > +\n> > +\tfor(p = input_command_list; p->name; p++) {\n> \n> Have a SP between for and '('.\n> \n> > +\t\tif (!prefixcmp(line->buf, p->name) &&\n> > +\t\t\t\t(strlen(p->name) == line->len || line->buf[strlen(p->name)] == ' \n'))\n> > {\n> \n> A line way too wide.\n> \n> > +\t\t\tif (p->batchable) {\n> > +\t\t\t\tbatch_cmd = p;\n> > +\t\t\t\tstring_list_append(&batchlines, line->buf);\n> > +\t\t\t\treturn 0;\n> > +\t\t\t}\n> > +\t\t\treturn p->fct(line->buf);\n> > +\t\t}\n> \n> OK, so a command word on a line by itself, or a command word\n> followed by a SP (probably followed by its arguments) on a line\n> triggers a command lookup, and individual command implementation\n> parses the line.\n> \n> > +\t}\n> > +\twarning(\"Unknown command '%s'\\n\", line->buf);\n> > +\treturn 0;\n> \n> Why isn't this an error?\n\nYeah, it should.\n\n> \n> > +}\n> > +\n> > +int main(int argc, const char **argv)\n> > +{\n> > +\tstruct strbuf buf = STRBUF_INIT;\n> > +\tint nongit;\n> > +\tstatic struct remote *remote;\n> > +\tconst char *url_in;\n> > +\n> > +\tgit_extract_argv0_path(argv[0]);\n> > +\tsetup_git_directory_gently(&nongit);\n> > +\tif (argc < 2 || argc > 3) {\n> > +\t\tusage(\"git-remote-svn <remote-name> [<url>]\");\n> > +\t\treturn 1;\n> > +\t}\n> \n> If this is an importer, you would be importing _into_ a git\n> repository, no?  How can you not error out when you are not in one?\n> In other words, why &nongit with *_gently()?\n\nHm .. in fact it only feeds into fast-import. But you're right, it doesn't \nmake much sense. I took remote-curl.c as a guideline for these first lines.\nWill use the non-gentle function.\n\n> \n> > +\tremote = remote_get(argv[1]);\n> > +\turl_in = remote->url[0];\n> > +\tif (argc == 3)\n> > +\t\turl_in = argv[2];\n> \n> Shouldn't it be more like this?\n> \n> \turl_in = (argc == 3) ? argv[2] : remote->url[0];\n\nYes, much nicer.\n\n> \n> > +\tend_url_with_slash(&buf, url_in);\n> > +\turl = strbuf_detach(&buf, NULL);\n> > +\n> > +\tstrbuf_addf(&buf, \"refs/svn/%s/master\", remote->name);\n> > +\tprivate_ref = strbuf_detach(&buf, NULL);\n> > +\n> > +\twhile(1) {\n> > +\t\tif (strbuf_getline(&buf, stdin, '\\n') == EOF) {\n> > +\t\t\tif (ferror(stdin))\n> > +\t\t\t\tdie(\"Error reading command stream\");\n> > +\t\t\telse\n> > +\t\t\t\tdie(\"Unexpected end of command stream\");\n> > +\t\t}\n> > +\t\tif (do_command(&buf))\n> > +\t\t\tbreak;\n> > +\t\tstrbuf_reset(&buf);\n> > +\t}\n> > +\n> > +\tstrbuf_release(&buf);\n> > +\tfree((void*)url);\n> > +\tfree((void*)private_ref);\n> > +\treturn 0;\n> > +}\n"},{"id":"197040","messageId":"2444647.P1AdWcSsQk@flomedio","threadId":"31252","inReplyTo":"7v4no59phn.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC v3 04/16] Connect fast-import to the remote-helper via pipe, adding 'bidi-import' capability.","fromName":"Florian Achleitner","fromEmail":"florian.achleitner.2.6.31@gmail.com","sentAt":"2012-08-15T12:00:33Z","receivedAt":"2012-08-15T12:00:33Z","isPatch":true,"sender":{"key":"florian.achleitner.2.6.31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/880777?v=4"},"body":"On Tuesday 14 August 2012 13:40:20 Junio C Hamano wrote:\n> Florian Achleitner <florian.achleitner.2.6.31@gmail.com> writes:\n> > The fast-import commands 'cat-blob' and 'ls' can be used by remote-helpers\n> > to retrieve information about blobs and trees that already exist in\n> > fast-import's memory. This requires a channel from fast-import to the\n> > remote-helper.\n> > remote-helpers that use this features shall advertise the new\n> > 'bidi-import'\n> \n> s/this fea/these fea/\n> \n> > capability so signal that they require the communication channel.\n> \n> s/so sig/to sig/, I think.\n> \n> > When forking fast-import in transport-helper.c connect it to a dup of\n> > the remote-helper's stdin-pipe. The additional file descriptor is passed\n> > to fast-import via it's command line (--cat-blob-fd).\n> \n> s/via it's/via its/;\n> \n> > It follows that git and fast-import are connected to the remote-helpers's\n> > stdin.\n> > Because git can send multiple commands to the remote-helper on it's stdin,\n> > it is required that helpers that advertise 'bidi-import' buffer all input\n> > commands until the batch of 'import' commands is ended by a newline\n> > before sending data to fast-import.\n> > This is to prevent mixing commands and fast-import responses on the\n> > helper's stdin.\n> \n> Please have a blank line each between paragraphs; a solid block of\n> text is very hard to follow.\n> \n> > Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n> > ---\n> > \n> >  transport-helper.c |   45 ++++++++++++++++++++++++++++++++-------------\n> >  1 file changed, 32 insertions(+), 13 deletions(-)\n> > \n> > diff --git a/transport-helper.c b/transport-helper.c\n> > index cfe0988..257274b 100644\n> > --- a/transport-helper.c\n> > +++ b/transport-helper.c\n> > @@ -10,6 +10,7 @@\n> > \n> >  #include \"string-list.h\"\n> >  #include \"thread-utils.h\"\n> >  #include \"sigchain.h\"\n> > \n> > +#include \"argv-array.h\"\n> > \n> >  static int debug;\n> > \n> > @@ -19,6 +20,7 @@ struct helper_data {\n> > \n> >  \tFILE *out;\n> >  \tunsigned fetch : 1,\n> >  \t\n> >  \t\timport : 1,\n> > \n> > +\t\tbidi_import : 1,\n> > \n> >  \t\texport : 1,\n> >  \t\toption : 1,\n> >  \t\tpush : 1,\n> > \n> > @@ -101,6 +103,7 @@ static void do_take_over(struct transport *transport)\n> > \n> >  static struct child_process *get_helper(struct transport *transport)\n> >  {\n> >  \n> >  \tstruct helper_data *data = transport->data;\n> > \n> > +\tstruct argv_array argv = ARGV_ARRAY_INIT;\n> > \n> >  \tstruct strbuf buf = STRBUF_INIT;\n> >  \tstruct child_process *helper;\n> >  \tconst char **refspecs = NULL;\n> > \n> > @@ -122,11 +125,10 @@ static struct child_process *get_helper(struct\n> > transport *transport)> \n> >  \thelper->in = -1;\n> >  \thelper->out = -1;\n> >  \thelper->err = 0;\n> > \n> > -\thelper->argv = xcalloc(4, sizeof(*helper->argv));\n> > -\tstrbuf_addf(&buf, \"git-remote-%s\", data->name);\n> > -\thelper->argv[0] = strbuf_detach(&buf, NULL);\n> > -\thelper->argv[1] = transport->remote->name;\n> > -\thelper->argv[2] = remove_ext_force(transport->url);\n> > +\targv_array_pushf(&argv, \"git-remote-%s\", data->name);\n> > +\targv_array_push(&argv, transport->remote->name);\n> > +\targv_array_push(&argv, remove_ext_force(transport->url));\n> > +\thelper->argv = argv.argv;\n> \n> Much nicer than before thanks to argv_array ;-)\n> \n> >  \thelper->git_cmd = 0;\n> >  \thelper->silent_exec_failure = 1;\n> > \n> > @@ -141,6 +143,8 @@ static struct child_process *get_helper(struct\n> > transport *transport)> \n> >  \tdata->helper = helper;\n> >  \tdata->no_disconnect_req = 0;\n> > \n> > +\tfree((void*) helper_env[1]);\n> \n> What is this free() for???\n\nSorry, legacy from previous versions, will be deleted.\n> \n> > +\targv_array_clear(&argv);\n> \n> See below.\n> \n> >  \t/*\n> >  \t\n> >  \t * Open the output as FILE* so strbuf_getline() can be used.\n> > \n> > @@ -178,6 +182,8 @@ static struct child_process *get_helper(struct\n> > transport *transport)> \n> >  \t\t\tdata->push = 1;\n> >  \t\t\n> >  \t\telse if (!strcmp(capname, \"import\"))\n> >  \t\t\n> >  \t\t\tdata->import = 1;\n> > \n> > +\t\telse if (!strcmp(capname, \"bidi-import\"))\n> > +\t\t\tdata->bidi_import = 1;\n> > \n> >  \t\telse if (!strcmp(capname, \"export\"))\n> >  \t\t\n> >  \t\t\tdata->export = 1;\n> >  \t\t\n> >  \t\telse if (!data->refspecs && !prefixcmp(capname, \"refspec \")) {\n> > \n> > @@ -241,8 +247,6 @@ static int disconnect_helper(struct transport\n> > *transport)> \n> >  \t\tclose(data->helper->out);\n> >  \t\tfclose(data->out);\n> >  \t\tres = finish_command(data->helper);\n> > \n> > -\t\tfree((char *)data->helper->argv[0]);\n> > -\t\tfree(data->helper->argv);\n> > \n> >  \t\tfree(data->helper);\n> >  \t\tdata->helper = NULL;\n> >  \t\n> >  \t}\n> > \n> > @@ -376,14 +380,24 @@ static int fetch_with_fetch(struct transport\n> > *transport,> \n> >  static int get_importer(struct transport *transport, struct child_process\n> >  *fastimport) {\n> >  \n> >  \tstruct child_process *helper = get_helper(transport);\n> > \n> > +\tstruct helper_data *data = transport->data;\n> > +\tstruct argv_array argv = ARGV_ARRAY_INIT;\n> > +\tint cat_blob_fd, code;\n> > \n> >  \tmemset(fastimport, 0, sizeof(*fastimport));\n> >  \tfastimport->in = helper->out;\n> > \n> > -\tfastimport->argv = xcalloc(5, sizeof(*fastimport->argv));\n> > -\tfastimport->argv[0] = \"fast-import\";\n> > -\tfastimport->argv[1] = \"--quiet\";\n> > +\targv_array_push(&argv, \"fast-import\");\n> > +\targv_array_push(&argv, \"--quiet\");\n> > \n> > +\tif (data->bidi_import) {\n> > +\t\tcat_blob_fd = xdup(helper->in);\n> > +\t\targv_array_pushf(&argv, \"--cat-blob-fd=%d\", cat_blob_fd);\n> > +\t}\n> > +\tfastimport->argv = argv.argv;\n> > \n> >  \tfastimport->git_cmd = 1;\n> > \n> > -\treturn start_command(fastimport);\n> > +\n> > +\tcode = start_command(fastimport);\n> > +\targv_array_clear(&argv);\n> > +\treturn code;\n> > \n> >  }\n> >  \n> >  static int get_exporter(struct transport *transport,\n> > \n> > @@ -438,11 +452,16 @@ static int fetch_with_import(struct transport\n> > *transport,> \n> >  \t}\n> >  \t\n> >  \twrite_constant(data->helper->in, \"\\n\");\n> > \n> > +\t/*\n> > +\t * remote-helpers that advertise the bidi-import capability are required\n> > to +\t * buffer the complete batch of import commands until this newline\n> > before +\t * sending data to fast-import.\n> > +\t * These helpers read back data from fast-import on their stdin, which\n> > could +\t * be mixed with import commands, otherwise.\n> > +\t */\n> > \n> >  \tif (finish_command(&fastimport))\n> >  \t\n> >  \t\tdie(\"Error while running fast-import\");\n> > \n> > -\tfree(fastimport.argv);\n> > -\tfastimport.argv = NULL;\n> \n> The updated code frees argv[] immediately after start_command()\n> returns, and it may happen to be safe to do so with the current\n> implementation of start_command() and friends, but I think it is a\n> bad taste to free argv[] (or env[] for that matter) before calling\n> finish_command().  These pieces of memory are still pointed by the\n> child_process structure, and users of the structure may want to use\n> contents of them (especially, argv[0]) for reporting errors and\n> various other purposes, e.g.\n> \n> \tchild = get_helper();\n> \n>         trace(\"started %s\\n\", child->argv[0]);\n> \n> \tif (finish_command(child))\n>         \treturn error(\"failed to cleanly finish %s\", child->argv[0]);\n\nYes, sounds reasonable. The present of immedate clearing has the advantage \nthat I don't have to store the struct argv_array, as struct child_process only \nhas a member for const char **argv.\nI'll improve postpone the free until the command finishes.\n"},{"id":"197046","messageId":"7vboic6oik.fsf@alter.siamese.dyndns.org","threadId":"31252","inReplyTo":"2444647.P1AdWcSsQk@flomedio","subject":"Re: [PATCH/RFC v3 04/16] Connect fast-import to the remote-helper via pipe, adding 'bidi-import' capability.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-15T17:41:55Z","receivedAt":"2012-08-15T17:41:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Florian Achleitner <florian.achleitner.2.6.31@gmail.com> writes:\n\n>> The updated code frees argv[] immediately after start_command()\n>> returns, and it may happen to be safe to do so with the current\n>> implementation of start_command() and friends, but I think it is a\n>> bad taste to free argv[] (or env[] for that matter) before calling\n>> finish_command().  These pieces of memory are still pointed by the\n>> child_process structure, and users of the structure may want to use\n>> contents of them (especially, argv[0]) for reporting errors and\n>> various other purposes, e.g.\n>> \n>> \tchild = get_helper();\n>> \n>>         trace(\"started %s\\n\", child->argv[0]);\n>> \n>> \tif (finish_command(child))\n>>         \treturn error(\"failed to cleanly finish %s\", child->argv[0]);\n>\n> Yes, sounds reasonable. The present of immedate clearing has the advantage \n> that I don't have to store the struct argv_array, as struct child_process only \n> has a member for const char **argv.\n\nAnd updated code shouldn't have to store struct argv_array either.\nIf you just give the ownership of argv_array.argv to child_process\nand clean it as part of destroying the child_process, you do not\nhave to worry about argv_array at all.\n\nIn order to cleanly support that use case at the API level, we may\nwant to introduce argv_array_detach() that is similar in spirit to\nstrbuf_detach(), which transfers ownership of the underlying memory\nto the caller.\n"},{"id":"197067","messageId":"7v1uj85427.fsf@alter.siamese.dyndns.org","threadId":"31252","inReplyTo":"1344971598-8213-11-git-send-email-florian.achleitner.2.6.31@gmail.com","subject":"Re: [PATCH/RFC v3 10/16] Create a note for every imported commit containing svn metadata.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-15T19:49:04Z","receivedAt":"2012-08-15T19:49:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Florian Achleitner <florian.achleitner.2.6.31@gmail.com> writes:\n\n> To provide metadata from svn dumps for further processing, e.g.\n> branch detection, attach a note to each imported commit that\n> stores additional information.\n> The notes are currently hard-coded in refs/notes/svn/revs.\n> Currently the following lines from the svn dump are directly\n> accumulated in the note. This can be refined on purpose, of course.\n> - \"Revision-number\"\n> - \"Node-path\"\n> - \"Node-kind\"\n> - \"Node-action\"\n> - \"Node-copyfrom-path\"\n> - \"Node-copyfrom-rev\"\n>\n> Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n> ---\n>  vcs-svn/fast_export.c |   13 +++++++++++++\n>  vcs-svn/fast_export.h |    2 ++\n>  vcs-svn/svndump.c     |   21 +++++++++++++++++++--\n>  3 files changed, 34 insertions(+), 2 deletions(-)\n>\n> diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c\n> index 1ecae4b..796dd1a 100644\n> --- a/vcs-svn/fast_export.c\n> +++ b/vcs-svn/fast_export.c\n> @@ -12,6 +12,7 @@\n>  #include \"svndiff.h\"\n>  #include \"sliding_window.h\"\n>  #include \"line_buffer.h\"\n> +#include \"cache.h\"\n\nShouldn't it be near the beginning?  Also if you include \"cache.h\",\nit probably makes git-compat-util and strbuf redundant.\n\n>  \n>  #define MAX_GITSVN_LINE_LEN 4096\n>  \n> @@ -68,6 +69,18 @@ void fast_export_modify(const char *path, uint32_t mode, const char *dataref)\n>  \tputchar('\\n');\n>  }\n>  \n> +void fast_export_begin_note(uint32_t revision, const char *author,\n> +\t\tconst char *log, unsigned long timestamp)\n> +{\n> +\ttimestamp = 1341914616;\n\nThe magic number needs some comment.\n\n> +\tsize_t loglen = strlen(log);\n\ndecl-after-statement.  I am starting to suspect that the assignment\nis a leftover from an earlier debugging effort, though.\n"},{"id":"197066","messageId":"7vwr103pg1.fsf@alter.siamese.dyndns.org","threadId":"31252","inReplyTo":"1344971598-8213-12-git-send-email-florian.achleitner.2.6.31@gmail.com","subject":"Re: [PATCH/RFC v3 11/16] When debug==1, start fast-import with \"--stats\" instead of \"--quiet\".","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-15T19:50:06Z","receivedAt":"2012-08-15T19:50:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Florian Achleitner <florian.achleitner.2.6.31@gmail.com> writes:\n\n> fast-import prints statistics that could be interesting to the\n> developer of remote helpers.\n>\n> Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n> ---\n\nSounds sensible and could be useful outside the context of this\nseries.  Perhaps place it earlier in the series?\n\n>  transport-helper.c |    2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/transport-helper.c b/transport-helper.c\n> index 257274b..7fb52d4 100644\n> --- a/transport-helper.c\n> +++ b/transport-helper.c\n> @@ -386,7 +386,7 @@ static int get_importer(struct transport *transport, struct child_process *fasti\n>  \tmemset(fastimport, 0, sizeof(*fastimport));\n>  \tfastimport->in = helper->out;\n>  \targv_array_push(&argv, \"fast-import\");\n> -\targv_array_push(&argv, \"--quiet\");\n> +\targv_array_push(&argv, debug ? \"--stats\" : \"--quiet\");\n>  \n>  \tif (data->bidi_import) {\n>  \t\tcat_blob_fd = xdup(helper->in);\n"},{"id":"197061","messageId":"7vsjbo3pbo.fsf@alter.siamese.dyndns.org","threadId":"31252","inReplyTo":"1344971598-8213-15-git-send-email-florian.achleitner.2.6.31@gmail.com","subject":"Re: [PATCH/RFC v3 14/16] transport-helper: add import|export-marks to fast-import command line.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-15T19:52:43Z","receivedAt":"2012-08-15T19:52:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Florian Achleitner <florian.achleitner.2.6.31@gmail.com> writes:\n\n> fast-import internally uses marks that refer to an object via its sha1.\n> Those marks are created during import to find previously created objects.\n> At exit the accumulated marks can be exported to a file and reloaded at\n> startup, so that the previous marks are available.\n> Add command line options to the fast-import command line to enable this.\n> The mark files are stored in info/fast-import/marks/<remote-name>.\n>\n> Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n> ---\n>  transport-helper.c |    3 +++\n>  1 file changed, 3 insertions(+)\n>\n> diff --git a/transport-helper.c b/transport-helper.c\n> index 7fb52d4..47db055 100644\n> --- a/transport-helper.c\n> +++ b/transport-helper.c\n> @@ -387,6 +387,9 @@ static int get_importer(struct transport *transport, struct child_process *fasti\n>  \tfastimport->in = helper->out;\n>  \targv_array_push(&argv, \"fast-import\");\n>  \targv_array_push(&argv, debug ? \"--stats\" : \"--quiet\");\n> +\targv_array_push(&argv, \"--relative-marks\");\n> +\targv_array_pushf(&argv, \"--import-marks-if-exists=marks/%s\", transport->remote->name);\n> +\targv_array_pushf(&argv, \"--export-marks=marks/%s\", transport->remote->name);\n\nIs this something we want to do unconditionally?\n"},{"id":"197064","messageId":"3474533.sTggvtNqe8@flomedio","threadId":"31252","inReplyTo":"7v1uj85427.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC v3 10/16] Create a note for every imported commit containing svn metadata.","fromName":"Florian Achleitner","fromEmail":"florian.achleitner2.6.31@gmail.com","sentAt":"2012-08-15T20:10:18Z","receivedAt":"2012-08-15T20:10:18Z","isPatch":true,"sender":{"key":"florian.achleitner.2.6.31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/880777?v=4"},"body":"On Wednesday 15 August 2012 12:49:04 Junio C Hamano wrote:\n> Florian Achleitner <florian.achleitner.2.6.31@gmail.com> writes:\n> > To provide metadata from svn dumps for further processing, e.g.\n> > branch detection, attach a note to each imported commit that\n> > stores additional information.\n> > The notes are currently hard-coded in refs/notes/svn/revs.\n> > Currently the following lines from the svn dump are directly\n> > accumulated in the note. This can be refined on purpose, of course.\n> > - \"Revision-number\"\n> > - \"Node-path\"\n> > - \"Node-kind\"\n> > - \"Node-action\"\n> > - \"Node-copyfrom-path\"\n> > - \"Node-copyfrom-rev\"\n> > \n> > Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n> > ---\n> > \n> >  vcs-svn/fast_export.c |   13 +++++++++++++\n> >  vcs-svn/fast_export.h |    2 ++\n> >  vcs-svn/svndump.c     |   21 +++++++++++++++++++--\n> >  3 files changed, 34 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c\n> > index 1ecae4b..796dd1a 100644\n> > --- a/vcs-svn/fast_export.c\n> > +++ b/vcs-svn/fast_export.c\n> > @@ -12,6 +12,7 @@\n> > \n> >  #include \"svndiff.h\"\n> >  #include \"sliding_window.h\"\n> >  #include \"line_buffer.h\"\n> > \n> > +#include \"cache.h\"\n> \n> Shouldn't it be near the beginning?  Also if you include \"cache.h\",\n> it probably makes git-compat-util and strbuf redundant.\n\nAck.\n\n> \n> >  #define MAX_GITSVN_LINE_LEN 4096\n> > \n> > @@ -68,6 +69,18 @@ void fast_export_modify(const char *path, uint32_t\n> > mode, const char *dataref)> \n> >  \tputchar('\\n');\n> >  \n> >  }\n> > \n> > +void fast_export_begin_note(uint32_t revision, const char *author,\n> > +\t\tconst char *log, unsigned long timestamp)\n> > +{\n> > +\ttimestamp = 1341914616;\n> \n> The magic number needs some comment.\n> \n> > +\tsize_t loglen = strlen(log);\n> \n> decl-after-statement.  I am starting to suspect that the assignment\n> is a leftover from an earlier debugging effort, though.\n\nOh yes sorry. Leftover from a previous experiment.\nThx for your reviews Junio, I got too blind to see this.\n"},{"id":"197065","messageId":"1501407.T1vfOr6Yzb@flomedio","threadId":"31252","inReplyTo":"7vsjbo3pbo.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC v3 14/16] transport-helper: add import|export-marks to fast-import command line.","fromName":"Florian Achleitner","fromEmail":"florian.achleitner.2.6.31@gmail.com","sentAt":"2012-08-15T20:20:45Z","receivedAt":"2012-08-15T20:20:45Z","isPatch":true,"sender":{"key":"florian.achleitner.2.6.31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/880777?v=4"},"body":"On Wednesday 15 August 2012 12:52:43 Junio C Hamano wrote:\n> Florian Achleitner <florian.achleitner.2.6.31@gmail.com> writes:\n> > fast-import internally uses marks that refer to an object via its sha1.\n> > Those marks are created during import to find previously created objects.\n> > At exit the accumulated marks can be exported to a file and reloaded at\n> > startup, so that the previous marks are available.\n> > Add command line options to the fast-import command line to enable this.\n> > The mark files are stored in info/fast-import/marks/<remote-name>.\n> > \n> > Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n> > ---\n> > \n> >  transport-helper.c |    3 +++\n> >  1 file changed, 3 insertions(+)\n> > \n> > diff --git a/transport-helper.c b/transport-helper.c\n> > index 7fb52d4..47db055 100644\n> > --- a/transport-helper.c\n> > +++ b/transport-helper.c\n> > @@ -387,6 +387,9 @@ static int get_importer(struct transport *transport,\n> > struct child_process *fasti> \n> >  \tfastimport->in = helper->out;\n> >  \targv_array_push(&argv, \"fast-import\");\n> >  \targv_array_push(&argv, debug ? \"--stats\" : \"--quiet\");\n> > \n> > +\targv_array_push(&argv, \"--relative-marks\");\n> > +\targv_array_pushf(&argv, \"--import-marks-if-exists=marks/%s\",\n> > transport->remote->name); +\targv_array_pushf(&argv,\n> > \"--export-marks=marks/%s\", transport->remote->name);\n> Is this something we want to do unconditionally?\n\nGood question. It doesn't hurt, but it maybe . We could add another capability \nfor remote-helpers, that tells us if it needs masks. What do you think?\n"},{"id":"197062","messageId":"1579004.p8CLksap2K@flomedio","threadId":"31252","inReplyTo":"1501407.T1vfOr6Yzb@flomedio","subject":"Re: [PATCH/RFC v3 14/16] transport-helper: add import|export-marks to fast-import command line.","fromName":"Florian Achleitner","fromEmail":"florian.achleitner.2.6.31@gmail.com","sentAt":"2012-08-15T21:06:20Z","receivedAt":"2012-08-15T21:06:20Z","isPatch":true,"sender":{"key":"florian.achleitner.2.6.31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/880777?v=4"},"body":"On Wednesday 15 August 2012 22:20:45 Florian Achleitner wrote:\n> On Wednesday 15 August 2012 12:52:43 Junio C Hamano wrote:\n> > Florian Achleitner <florian.achleitner.2.6.31@gmail.com> writes:\n> > > fast-import internally uses marks that refer to an object via its sha1.\n> > > Those marks are created during import to find previously created\n> > > objects.\n> > > At exit the accumulated marks can be exported to a file and reloaded at\n> > > startup, so that the previous marks are available.\n> > > Add command line options to the fast-import command line to enable this.\n> > > The mark files are stored in info/fast-import/marks/<remote-name>.\n> > > \n> > > Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>\n> > > ---\n> > > \n> > >  transport-helper.c |    3 +++\n> > >  1 file changed, 3 insertions(+)\n> > > \n> > > diff --git a/transport-helper.c b/transport-helper.c\n> > > index 7fb52d4..47db055 100644\n> > > --- a/transport-helper.c\n> > > +++ b/transport-helper.c\n> > > @@ -387,6 +387,9 @@ static int get_importer(struct transport *transport,\n> > > struct child_process *fasti>\n> > > \n> > >  \tfastimport->in = helper->out;\n> > >  \targv_array_push(&argv, \"fast-import\");\n> > >  \targv_array_push(&argv, debug ? \"--stats\" : \"--quiet\");\n> > > \n> > > +\targv_array_push(&argv, \"--relative-marks\");\n> > > +\targv_array_pushf(&argv, \"--import-marks-if-exists=marks/%s\",\n> > > transport->remote->name); +\targv_array_pushf(&argv,\n> > > \"--export-marks=marks/%s\", transport->remote->name);\n> > \n> > Is this something we want to do unconditionally?\n> \n> Good question. It doesn't hurt, but it maybe . We could add another\n> capability for remote-helpers, that tells us if it needs masks. What do you\n> think?\n\nBtw, for fast-export, there is already such a capability. It specifies a \nfilename, in addition.\n"}]}