{"thread":{"id":"26304","subject":"[RFC PATCH v2 0/5] Towards a Git-to-SVN bridge","startedAt":"2011-01-19T05:44:54Z","lastAt":"2011-01-22T19:18:01Z","messageCount":18,"participants":["Ramkumar Ramachandra","Junio C Hamano","Jonathan Nieder","Drew Northup"],"isPatch":true,"patchVersion":2,"patchTotal":5},"messages":[{"id":"159617","messageId":"1295415899-1192-1-git-send-email-artagnon@gmail.com","threadId":"26304","inReplyTo":null,"subject":"[RFC PATCH v2 0/5] Towards a Git-to-SVN bridge","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-01-19T05:44:54Z","receivedAt":"2011-01-19T05:44:54Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nThere are two major changes this time around:\n1. I've managed to inline the blobs in the fast-export stream. This\ncompletely eliminates the need for persisting blobs.\n2. I've also managed to produce relevant directory nodes, keeping\ndirectory state in a new dir_cache interface. I followed Jonathan's\nsuggestion to keep it as a string_list for the moment. This is highly\ninefficient and should improve soon- perhaps ressurect the treap we\nhad? Or perhaps find a simple trie implementation somewhere?\nSuggestions are welcome.\n\nUnfortunately, I've still not had the time to clean up- there's a\nprobably a lot of cruft, and everything \"just works\" for now. Next\ntime, I'll also try to write some tests, so we don't have to test it\nby hand.\n\nThanks for reading.\n\nRamkumar Ramachandra (5):\n date: Expose the time_to_tm function\n vcs-svn: Start working on the dumpfile producer\n Build an svn-fi target in contrib/svn-fe\n fast-export: Introduce --inline-blobs\n vcs-svn: Add dir_cache for svnload\n\n Documentation/git-fast-export.txt | 5 +\n Makefile | 4 +-\n builtin/fast-export.c | 23 +++-\n cache.h | 1 +\n contrib/svn-fe/Makefile | 23 +++-\n contrib/svn-fe/svn-fi.c | 16 ++\n contrib/svn-fe/svn-fi.txt | 28 ++++\n date.c | 2 +-\n vcs-svn/dir_cache.c | 40 +++++\n vcs-svn/dir_cache.h | 12 ++\n vcs-svn/dump_export.c | 141 ++++++++++++++++++\n vcs-svn/dump_export.h | 33 +++++\n vcs-svn/svnload.c | 286 +++++++++++++++++++++++++++++++++++++\n vcs-svn/svnload.h | 10 ++\n 14 files changed, 617 insertions(+), 7 deletions(-)\n create mode 100644 contrib/svn-fe/svn-fi.c\n create mode 100644 contrib/svn-fe/svn-fi.txt\n create mode 100644 vcs-svn/dir_cache.c\n create mode 100644 vcs-svn/dir_cache.h\n create mode 100644 vcs-svn/dump_export.c\n create mode 100644 vcs-svn/dump_export.h\n create mode 100644 vcs-svn/svnload.c\n create mode 100644 vcs-svn/svnload.h\n\n-- \n1.7.4.rc1.7.g2cf08.dirty\n"},{"id":"159618","messageId":"1295415899-1192-2-git-send-email-artagnon@gmail.com","threadId":"26304","inReplyTo":"1295415899-1192-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 1/5] date: Expose the time_to_tm function","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-01-19T05:44:55Z","receivedAt":"2011-01-19T05:44:55Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Signed-off-by: Ramkumar Ramachandra \n---\n cache.h | 1 +\n date.c | 2 +-\n 2 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex d83d68c..95fea31 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -816,6 +816,7 @@ enum date_mode {\n \tDATE_RAW\n };\n \n+struct tm *time_to_tm(unsigned long time, int tz);\n const char *show_date(unsigned long time, int timezone, enum date_mode mode);\n const char *show_date_relative(unsigned long time, int tz,\n \t\t\t const struct timeval *now,\ndiff --git a/date.c b/date.c\nindex 00f9eb5..e601a50 100644\n--- a/date.c\n+++ b/date.c\n@@ -54,7 +54,7 @@ static time_t gm_time_t(unsigned long time, int tz)\n * thing, which means that tz -0100 is passed in as the integer -100,\n * even though it means \"sixty minutes off\"\n */\n-static struct tm *time_to_tm(unsigned long time, int tz)\n+struct tm *time_to_tm(unsigned long time, int tz)\n {\n \ttime_t t = gm_time_t(time, tz);\n \treturn gmtime(&t);\n-- \n1.7.4.rc1.7.g2cf08.dirty\n"},{"id":"159619","messageId":"1295415899-1192-3-git-send-email-artagnon@gmail.com","threadId":"26304","inReplyTo":"1295415899-1192-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 2/5] vcs-svn: Start working on the dumpfile producer","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-01-19T05:44:56Z","receivedAt":"2011-01-19T05:44:56Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Start off with some broad design sketches. Compile succeeds, but\nparser is incorrect. Include a Makefile rule to build it into\nvcs-svn/lib.a.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n Makefile | 2 +-\n vcs-svn/dump_export.c | 73 ++++++++++++\n vcs-svn/svnload.c | 294 +++++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 368 insertions(+), 1 deletions(-)\n create mode 100644 vcs-svn/dump_export.c\n create mode 100644 vcs-svn/svnload.c\n\ndiff --git a/Makefile b/Makefile\nindex 1345c38..40f6691 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1834,7 +1834,7 @@ ifndef NO_CURL\n endif\n XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \\\n \txdiff/xmerge.o xdiff/xpatience.o\n-VCSSVN_OBJS = vcs-svn/line_buffer.o \\\n+VCSSVN_OBJS = vcs-svn/line_buffer.o vcs-svn/svnload.o vcs-svn/dump_export.o \\\n \tvcs-svn/repo_tree.o vcs-svn/fast_export.o vcs-svn/sliding_window.o \\\n \tvcs-svn/svndiff.o vcs-svn/svndump.o\n VCSSVN_TEST_OBJS = test-obj-pool.o \\\ndiff --git a/vcs-svn/dump_export.c b/vcs-svn/dump_export.c\nnew file mode 100644\nindex 0000000..04ede06\n--- /dev/null\n+++ b/vcs-svn/dump_export.c\n@@ -0,0 +1,73 @@\n+/*\n+ * Licensed under a two-clause BSD-style license.\n+ * See LICENSE for details.\n+ */\n+\n+#include \"git-compat-util.h\"\n+#include \"strbuf.h\"\n+#include \"line_buffer.h\"\n+#include \"dump_export.h\"\n+\n+void dump_export_begin_rev(int revision, const char *revprops,\n+\t\t\tint prop_len) {\n+\tprintf(\"Revision-number: %d\\n\", revision);\n+\tprintf(\"Prop-content-length: %d\\n\", prop_len);\n+\tprintf(\"Content-length: %d\\n\\n\", prop_len);\n+\tprintf(\"%s\\n\", revprops);\n+}\n+\n+void dump_export_node(const char *path, enum node_kind kind,\n+\t\tenum node_action action, unsigned long text_len,\n+\t\tunsigned long copyfrom_rev, const char *copyfrom_path) {\n+\tprintf(\"Node-path: %s\\n\", path);\n+\tprintf(\"Node-kind: \");\n+\tswitch (action) {\n+\tcase NODE_KIND_NORMAL:\n+\t\tprintf(\"file\\n\");\n+\t\tbreak;\n+\tcase NODE_KIND_EXECUTABLE:\n+\t\tprintf(\"file\\n\");\n+\t\tbreak;\n+\tcase NODE_KIND_SYMLINK:\n+\t\tprintf(\"file\\n\");\n+\t\tbreak;\n+\tcase NODE_KIND_GITLINK:\n+\t\tprintf(\"file\\n\");\n+\t\tbreak;\n+\tcase NODE_KIND_SUBDIR:\n+\t\tdie(\"Unsupported: subdirectory\");\n+\tdefault:\n+\t\tbreak;\n+\t}\n+\tprintf(\"Node-action: \");\n+\tswitch (action) {\n+\tcase NODE_ACTION_CHANGE:\n+\t\tprintf(\"change\\n\");\n+\t\tbreak;\n+\tcase NODE_ACTION_ADD:\n+\t\tprintf(\"add\\n\");\n+\t\tbreak;\n+\tcase NODE_ACTION_REPLACE:\n+\t\tprintf(\"replace\\n\");\n+\t\tbreak;\n+\tcase NODE_ACTION_DELETE:\n+\t\tprintf(\"delete\\n\");\n+\t\tbreak;\n+\tdefault:\n+\t\tbreak;\n+\t}\n+\tif (copyfrom_rev != SVN_INVALID_REV) {\n+\t\tprintf(\"Node-copyfrom-rev: %lu\\n\", copyfrom_rev);\n+\t\tprintf(\"Node-copyfrom-path: %s\\n\", copyfrom_path);\n+\t}\n+\tprintf(\"Prop-delta: false\\n\");\n+\tprintf(\"Prop-content-length: 10\\n\"); /* Constant 10 for \"PROPS-END\" */\n+\tprintf(\"Text-delta: false\\n\");\n+\tprintf(\"Text-content-length: %lu\\n\", text_len);\n+\tprintf(\"Content-length: %lu\\n\\n\", text_len + 10);\n+\tprintf(\"PROPS-END\\n\\n\");\n+}\n+\n+void dump_export_text(struct line_buffer *data, off_t len) {\n+\tbuffer_copy_bytes(data, len);\n+}\ndiff --git a/vcs-svn/svnload.c b/vcs-svn/svnload.c\nnew file mode 100644\nindex 0000000..7043ae7\n--- /dev/null\n+++ b/vcs-svn/svnload.c\n@@ -0,0 +1,294 @@\n+/*\n+ * Produce a dumpfile v3 from a fast-import stream.\n+ * Load the dump into the SVN repository with:\n+ * svnrdump load 1170199019 +0100 */\n+\tstrbuf_reset(name);\n+\tstrbuf_reset(email);\n+\tstrbuf_reset(date);\n+\ttz_off = strrchr(val, ' ');\n+\t*tz_off++ = '\\0';\n+\tt = strrchr(val, ' ');\n+\t*(t - 1) = '\\0'; /* Ignore '>' from email */\n+\tt ++;\n+\ttm_time = time_to_tm(strtoul(t, NULL, 10), atoi(tz_off));\n+\tstrftime(time_buf, SVN_DATE_LEN, SVN_DATE_FORMAT, tm_time);\n+\tstrbuf_add(date, time_buf, SVN_DATE_LEN);\n+\tt = strchr(val, '<');\n+\t*(t - 1) = '\\0'; /* Ignore ' <' from email */\n+\tt ++;\n+\tstrbuf_add(email, t, strlen(t));\n+\tstrbuf_add(name, val, strlen(val));\n+}\n+\n+void svnload_read(void) {\n+\tchar *t, *val;\n+\tint mode_incr;\n+\tstruct strbuf *to_dump;\n+\n+\twhile ((t = buffer_read_line(&input))) {\n+\t\tval = strchr(t, ' ');\n+\t\tif (!val) {\n+\t\t\tif (!memcmp(t, \"blob\", 4))\n+\t\t\t\tactive_ctx = BLOB_CTX;\n+\t\t\telse if (!memcmp(t, \"deleteall\", 9))\n+\t\t\t\t;\n+\t\t\tcontinue;\n+\t\t}\n+\t\t*val++ = '\\0';\n+\n+\t\t/* strlen(key) */\n+\t\tswitch (val - t - 1) {\n+\t\tcase 1:\n+\t\t\tif (!memcmp(t, \"D\", 1)) {\n+\t\t\t\tnode_ctx.action = NODE_ACTION_DELETE;\n+\t\t\t}\n+\t\t\telse if (!memcmp(t, \"C\", 1)) {\n+\t\t\t\tnode_ctx.action = NODE_ACTION_ADD;\n+\t\t\t}\n+\t\t\telse if (!memcmp(t, \"R\", 1)) {\n+\t\t\t\tnode_ctx.action = NODE_ACTION_REPLACE;\n+\t\t\t}\n+\t\t\telse if (!memcmp(t, \"M\", 1)) {\n+\t\t\t\tnode_ctx.action = NODE_ACTION_CHANGE;\n+\t\t\t\tmode_incr = 7;\n+\t\t\t\tif (!memcmp(val, \"100644\", 6))\n+\t\t\t\t\tnode_ctx.kind = NODE_KIND_NORMAL;\n+\t\t\t\telse if (!memcmp(val, \"100755\", 6))\n+\t\t\t\t\tnode_ctx.kind = NODE_KIND_EXECUTABLE;\n+\t\t\t\telse if (!memcmp(val, \"120000\", 6))\n+\t\t\t\t\tnode_ctx.kind = NODE_KIND_SYMLINK;\n+\t\t\t\telse if (!memcmp(val, \"160000\", 6))\n+\t\t\t\t\tnode_ctx.kind = NODE_KIND_GITLINK;\n+\t\t\t\telse if (!memcmp(val, \"040000\", 6))\n+\t\t\t\t\tnode_ctx.kind = NODE_KIND_SUBDIR;\n+\t\t\t\telse {\n+\t\t\t\t\tif (!memcmp(val, \"755\", 3))\n+\t\t\t\t\t\tnode_ctx.kind = NODE_KIND_EXECUTABLE;\n+\t\t\t\t\telse if(!memcmp(val, \"644\", 3))\n+\t\t\t\t\t\tnode_ctx.kind = NODE_KIND_NORMAL;\n+\t\t\t\t\telse\n+\t\t\t\t\t\tdie(\"Unrecognized mode: %s\", val);\n+\t\t\t\t\tmode_incr = 4;\n+\t\t\t\t}\n+\t\t\t\tval += mode_incr;\n+\t\t\t\tt = strchr(val, ' ');\n+\t\t\t\t*t++ = '\\0';\n+\t\t\t\tstrbuf_reset(&node_ctx.path);\n+\t\t\t\tstrbuf_add(&node_ctx.path, t, strlen(t));\n+\t\t\t\tif (!memcmp(val + 1, \"inline\", 6))\n+\t\t\t\t\tdie(\"Unsupported dataref: inline\");\n+\t\t\t\telse if (*val == ':')\n+\t\t\t\t\tto_dump = &blobs[strtoul(val + 1, NULL, 10)];\n+\t\t\t\telse\n+\t\t\t\t\tdie(\"Unsupported dataref: sha1\");\n+\t\t\t\tdump_export_node(node_ctx.path.buf, node_ctx.kind,\n+\t\t\t\t\t\tnode_ctx.action, to_dump->len,\n+\t\t\t\t\t\t0, NULL);\n+\t\t\t\tprintf(\"%s\", to_dump->buf);\n+\t\t\t}\n+\t\t\tbreak;\n+\t\tcase 3:\n+\t\t\tif (!memcmp(t, \"tag\", 3))\n+\t\t\t\tcontinue;\n+\t\t\tbreak;\n+\t\tcase 4:\n+\t\t\tif (!memcmp(t, \"mark\", 4))\n+\t\t\t\tswitch(active_ctx) {\n+\t\t\t\tcase COMMIT_CTX:\n+\t\t\t\t\t/* What do we do with commit marks? */\n+\t\t\t\t\tcontinue;\n+\t\t\t\tcase BLOB_CTX:\n+\t\t\t\t\tnode_ctx.mark = strtoul(val + 1, NULL, 10);\n+\t\t\t\t\tbreak;\n+\t\t\t\tdefault:\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n+\t\t\telse if (!memcmp(t, \"from\", 4))\n+\t\t\t\tcontinue;\n+\t\t\telse if (!memcmp(t, \"data\", 4)) {\n+\t\t\t\tswitch (active_ctx) {\n+\t\t\t\tcase COMMIT_CTX:\n+\t\t\t\t\tstrbuf_reset(&rev_ctx.log);\n+\t\t\t\t\tbuffer_read_binary(&input,\n+\t\t\t\t\t\t\t&rev_ctx.log,\n+\t\t\t\t\t\t\tstrtoul(val, NULL, 10));\n+\t\t\t\t\tpopulate_props(&rev_ctx.props,\n+\t\t\t\t\t\trev_ctx.svn_author.buf,\n+\t\t\t\t\t\trev_ctx.log.buf,\n+\t\t\t\t\t\trev_ctx.author_date.buf);\n+\t\t\t\t\tdump_export_begin_rev(rev_ctx.rev,\n+\t\t\t\t\t\t\trev_ctx.props.buf,\n+\t\t\t\t\t\t\trev_ctx.props.len);\n+\t\t\t\t\tbreak;\n+\t\t\t\tcase BLOB_CTX:\n+\t\t\t\t\tnode_ctx.text_len = strtoul(val, NULL, 10);\n+\t\t\t\t\tbuffer_read_binary(&input,\n+\t\t\t\t\t\t\t&blobs[node_ctx.mark],\n+\t\t\t\t\t\t\tnode_ctx.text_len);\n+\t\t\t\t\tbreak;\n+\t\t\t\tdefault:\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\tbreak;\n+\t\tcase 5:\n+\t\t\tif (!memcmp(t, \"reset\", 5))\n+\t\t\t\tcontinue;\n+\t\t\tif (!memcmp(t, \"merge\", 5))\n+\t\t\t\tcontinue;\n+\t\t\tbreak;\n+\t\tcase 6:\n+\t\t\tif (!memcmp(t, \"author\", 6)) {\n+\t\t\t\tparse_author_line(val, &rev_ctx.author,\n+\t\t\t\t\t\t&rev_ctx.author_email,\n+\t\t\t\t\t\t&rev_ctx.author_date);\n+\t\t\t\t/* Build svn_author */\n+\t\t\t\tt = strchr(rev_ctx.author_email.buf, '@');\n+\t\t\t\tstrbuf_reset(&rev_ctx.svn_author);\n+\t\t\t\tstrbuf_add(&rev_ctx.svn_author,\n+\t\t\t\t\trev_ctx.author_email.buf,\n+\t\t\t\t\tt - rev_ctx.author_email.buf);\n+\n+\t\t\t}\n+\t\t\telse if (!memcmp(t, \"commit\", 6)) {\n+\t\t\t\trev_ctx.rev ++;\n+\t\t\t\tactive_ctx = COMMIT_CTX;\n+\t\t\t}\n+\t\t\tbreak;\n+\t\tcase 9:\n+\t\t\tif (!memcmp(t, \"committer\", 9))\n+\t\t\t\tparse_author_line(val, &rev_ctx.committer,\n+\t\t\t\t\t\t&rev_ctx.committer_email,\n+\t\t\t\t\t\t&rev_ctx.committer_date);\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+}\n+\n+int svnload_init(const char *filename)\n+{\n+\tint i;\n+\tif (buffer_init(&input, filename))\n+\t\treturn error(\"cannot open %s: %s\", filename, strerror(errno));\n+\tactive_ctx = UNKNOWN_CTX;\n+\tstrbuf_init(&rev_ctx.props, MAX_GITSVN_LINE_LEN);\n+\tstrbuf_init(&rev_ctx.log, MAX_GITSVN_LINE_LEN);\n+\tstrbuf_init(&rev_ctx.author, MAX_GITSVN_LINE_LEN);\n+\tstrbuf_init(&rev_ctx.committer, MAX_GITSVN_LINE_LEN);\n+\tstrbuf_init(&rev_ctx.author_date, MAX_GITSVN_LINE_LEN);\n+\tstrbuf_init(&rev_ctx.committer_date, MAX_GITSVN_LINE_LEN);\n+\tstrbuf_init(&rev_ctx.author_email, MAX_GITSVN_LINE_LEN);\n+\tstrbuf_init(&rev_ctx.committer_email, MAX_GITSVN_LINE_LEN);\n+\tstrbuf_init(&node_ctx.path, MAX_GITSVN_LINE_LEN);\n+\tstrbuf_init(&node_ctx.copyfrom_path, MAX_GITSVN_LINE_LEN);\n+\tfor (i = 0; i < 100; i ++)\n+\t\tstrbuf_init(&blobs[i], 10000);\n+\treturn 0;\n+}\n+\n+void svnload_deinit(void)\n+{\n+\tint i;\n+\treset_rev_ctx(0);\n+\treset_node_ctx();\n+\tstrbuf_release(&rev_ctx.props);\n+\tstrbuf_release(&rev_ctx.log);\n+\tstrbuf_release(&rev_ctx.author);\n+\tstrbuf_release(&rev_ctx.committer);\n+\tstrbuf_release(&rev_ctx.author_date);\n+\tstrbuf_release(&rev_ctx.committer_date);\n+\tstrbuf_release(&rev_ctx.author_email);\n+\tstrbuf_release(&rev_ctx.committer_email);\n+\tstrbuf_release(&node_ctx.path);\n+\tstrbuf_release(&node_ctx.copyfrom_path);\n+\tfor (i = 0; i < 100; i ++)\n+\t\tstrbuf_release(&blobs[i]);\n+\tif (buffer_deinit(&input))\n+\t\tfprintf(stderr, \"Input error\\n\");\n+\tif (ferror(stdout))\n+\t\tfprintf(stderr, \"Output error\\n\");\n+}\n-- \n1.7.4.rc1.7.g2cf08.dirty\n"},{"id":"159620","messageId":"1295415899-1192-4-git-send-email-artagnon@gmail.com","threadId":"26304","inReplyTo":"1295415899-1192-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 3/5] Build an svn-fi target in contrib/svn-fe","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-01-19T05:44:57Z","receivedAt":"2011-01-19T05:44:57Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Build an svn-fi target for testing the dumpfile producer in vcs-svn/.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n contrib/svn-fe/Makefile | 23 +++++++++++++++++++++--\n contrib/svn-fe/svn-fi.c | 16 ++++++++++++++++\n contrib/svn-fe/svn-fi.txt | 28 ++++++++++++++++++++++++++++\n 3 files changed, 65 insertions(+), 2 deletions(-)\n create mode 100644 contrib/svn-fe/svn-fi.c\n create mode 100644 contrib/svn-fe/svn-fi.txt\n\ndiff --git a/contrib/svn-fe/Makefile b/contrib/svn-fe/Makefile\nindex 360d8da..555a8ff 100644\n--- a/contrib/svn-fe/Makefile\n+++ b/contrib/svn-fe/Makefile\n@@ -37,7 +37,7 @@ 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+svn-fe.o: svn-fe.c ../../vcs-svn/svnload.h\n \t$(QUIET_CC)$(CC) -I../../vcs-svn -o $*.o -c $(ALL_CFLAGS) $<\n \n svn-fe.html: svn-fe.txt\n@@ -51,6 +51,24 @@ svn-fe.1: svn-fe.txt\n \t\t../contrib/svn-fe/$@\n \t$(MV) ../../Documentation/svn-fe.1 .\n \n+svn-fi$X: svn-fi.o $(VCSSVN_LIB) $(GIT_LIB)\n+\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ svn-fi.o \\\n+\t\t$(ALL_LDFLAGS) $(LIBS)\n+\n+svn-fi.o: svn-fi.c ../../vcs-svn/svnload.h\n+\t$(QUIET_CC)$(CC) -I../../vcs-svn -o $*.o -c $(ALL_CFLAGS) $<\n+\n+svn-fi.html: svn-fi.txt\n+\t$(QUIET_SUBDIR0)../../Documentation $(QUIET_SUBDIR1) \\\n+\t\tMAN_TXT=../contrib/svn-fe/svn-fi.txt \\\n+\t\t../contrib/svn-fe/$@\n+\n+svn-fi.1: svn-fi.txt\n+\t$(QUIET_SUBDIR0)../../Documentation $(QUIET_SUBDIR1) \\\n+\t\tMAN_TXT=../contrib/svn-fe/svn-fi.txt \\\n+\t\t../contrib/svn-fe/$@\n+\t$(MV) ../../Documentation/svn-fi.1 .\n+\n ../../vcs-svn/lib.a: FORCE\n \t$(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) vcs-svn/lib.a\n \n@@ -58,6 +76,7 @@ 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 \\\n+\tsvn-fi$X svn-fi.o svn-fi.html svn-fi.xml svn-fi.1\n \n .PHONY: all clean FORCE\ndiff --git a/contrib/svn-fe/svn-fi.c b/contrib/svn-fe/svn-fi.c\nnew file mode 100644\nindex 0000000..81347b0\n--- /dev/null\n+++ b/contrib/svn-fe/svn-fi.c\n@@ -0,0 +1,16 @@\n+/*\n+ * This file is in the public domain.\n+ * You may freely use, modify, distribute, and relicense it.\n+ */\n+\n+#include \n+#include \"svnload.h\"\n+\n+int main(int argc, char **argv)\n+{\n+\tif (svnload_init(NULL))\n+\t\treturn 1;\n+\tsvnload_read();\n+\tsvnload_deinit();\n+\treturn 0;\n+}\ndiff --git a/contrib/svn-fe/svn-fi.txt b/contrib/svn-fe/svn-fi.txt\nnew file mode 100644\nindex 0000000..996a175\n--- /dev/null\n+++ b/contrib/svn-fe/svn-fi.txt\n@@ -0,0 +1,28 @@\n+svn-fe(1)\n+=========\n+\n+NAME\n+----\n+svn-fi - convert fast-import stream to an SVN \"dumpfile\"\n+\n+SYNOPSIS\n+--------\n+[verse]\n+svn-fi\n+\n+DESCRIPTION\n+-----------\n+\n+Converts a git-fast-import(1) stream into a Subversion dumpfile.\n+\n+INPUT FORMAT\n+-------------\n+The fast-import format is documented by the git-fast-import(1)\n+manual page.\n+\n+OUTPUT FORMAT\n+------------\n+Subversion's repository dump format is documented in full in\n+`notes/dump-load-format.txt` from the Subversion source tree.\n+Files in this format can be generated using the 'svnadmin dump' or\n+'svk admin dump' command.\n-- \n1.7.4.rc1.7.g2cf08.dirty\n"},{"id":"159622","messageId":"1295415899-1192-5-git-send-email-artagnon@gmail.com","threadId":"26304","inReplyTo":"1295415899-1192-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 4/5] fast-export: Introduce --inline-blobs","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-01-19T05:44:58Z","receivedAt":"2011-01-19T05:44:58Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Introduce a new command-line option --inline-blobs that always inlines\nblobs instead of referring to them via marks or their original SHA-1\nhash.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n Documentation/git-fast-export.txt | 5 +++++\n builtin/fast-export.c | 23 +++++++++++++++++++++--\n 2 files changed, 26 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt\nindex e05b686..cd48b4b 100644\n--- a/Documentation/git-fast-export.txt\n+++ b/Documentation/git-fast-export.txt\n@@ -90,6 +90,11 @@ marks the same across runs.\n \tresulting stream can only be used by a repository which\n \talready contains the necessary objects.\n \n+--inline-blobs::\n+\tInline all blobs, instead of referring to them via marks or\n+\ttheir original SHA-1 hash. This is useful to parsers, as they\n+\tdon't need to persist blobs.\n+\n --full-tree::\n \tThis option will cause fast-export to issue a \"deleteall\"\n \tdirective for each commit followed by a full list of all files\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex c8fd46b..202a3b9 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -27,6 +27,7 @@ static enum { ABORT, VERBATIM, WARN, STRIP } signed_tag_mode = ABORT;\n static enum { ERROR, DROP, REWRITE } tag_of_filtered_mode = ABORT;\n static int fake_missing_tagger;\n static int no_data;\n+static int inline_blobs;\n static int full_tree;\n \n static int parse_opt_signed_tag_mode(const struct option *opt,\n@@ -118,7 +119,7 @@ static void handle_object(const unsigned char *sha1)\n \tchar *buf;\n \tstruct object *object;\n \n-\tif (no_data)\n+\tif (no_data || inline_blobs)\n \t\treturn;\n \n \tif (is_null_sha1(sha1))\n@@ -218,7 +219,23 @@ static void show_filemodify(struct diff_queue_struct *q,\n \t\t\tif (no_data || S_ISGITLINK(spec->mode))\n \t\t\t\tprintf(\"M %06o %s %s\\n\", spec->mode,\n \t\t\t\t sha1_to_hex(spec->sha1), spec->path);\n-\t\t\telse {\n+\t\t\telse if (inline_blobs) {\n+\t\t\t\tunsigned long size;\n+\t\t\t\tenum object_type type;\n+\t\t\t\tchar *buf;\n+\n+\t\t\t\tbuf = read_sha1_file(spec->sha1, &type, &size);\n+\t\t\t\tif (!buf)\n+\t\t\t\t\tdie (\"Could not read blob %s\",\n+\t\t\t\t\t\tsha1_to_hex(spec->sha1));\n+\t\t\t\tprintf(\"M %06o inline %s\\n\", spec->mode, spec->path);\n+\t\t\t\tprintf(\"data %lu\\n\", size);\n+\t\t\t\tif (size && fwrite(buf, size, 1, stdout) != 1)\n+\t\t\t\t\tdie_errno (\"Could not write blob '%s'\",\n+\t\t\t\t\t\tsha1_to_hex(spec->sha1));\n+\t\t\t\tprintf(\"\\n\");\n+\n+\t\t\t} else {\n \t\t\t\tstruct object *object = lookup_object(spec->sha1);\n \t\t\t\tprintf(\"M %06o :%d %s\\n\", spec->mode,\n \t\t\t\t get_object_mark(object), spec->path);\n@@ -627,6 +644,8 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \t\t\t \"Fake a tagger when tags lack one\"),\n \t\tOPT_BOOLEAN(0, \"full-tree\", &full_tree,\n \t\t\t \"Output full tree for each commit\"),\n+\t\tOPT_BOOLEAN(0, \"inline-blobs\", &inline_blobs,\n+\t\t\t \"Output all blobs inline\"),\n \t\t{ OPTION_NEGBIT, 0, \"data\", &no_data, NULL,\n \t\t\t\"Skip output of blob data\",\n \t\t\tPARSE_OPT_NOARG | PARSE_OPT_NEGHELP, NULL, 1 },\n-- \n1.7.4.rc1.7.g2cf08.dirty\n"},{"id":"159621","messageId":"1295415899-1192-6-git-send-email-artagnon@gmail.com","threadId":"26304","inReplyTo":"1295415899-1192-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 5/5] vcs-svn: Add dir_cache for svnload","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-01-19T05:44:59Z","receivedAt":"2011-01-19T05:44:59Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"The logic for creating directories recursively is now\nimplemented. Unfortunately, dir_cache is currently implemented\ninefficiently using string_list- should probably use a treap or trie\nin future.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n Makefile | 2 +-\n vcs-svn/dir_cache.c | 40 +++++++++++++++\n vcs-svn/dir_cache.h | 12 +++++\n vcs-svn/dump_export.c | 104 ++++++++++++++++++++++++++++++++-------\n vcs-svn/dump_export.h | 33 ++++++++++++\n vcs-svn/svnload.c | 130 +++++++++++++++++++++++--------------------------\n vcs-svn/svnload.h | 10 ++++\n 7 files changed, 243 insertions(+), 88 deletions(-)\n create mode 100644 vcs-svn/dir_cache.c\n create mode 100644 vcs-svn/dir_cache.h\n create mode 100644 vcs-svn/dump_export.h\n create mode 100644 vcs-svn/svnload.h\n\ndiff --git a/Makefile b/Makefile\nindex 40f6691..d9c2442 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1836,7 +1836,7 @@ XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \\\n \txdiff/xmerge.o xdiff/xpatience.o\n VCSSVN_OBJS = vcs-svn/line_buffer.o vcs-svn/svnload.o vcs-svn/dump_export.o \\\n \tvcs-svn/repo_tree.o vcs-svn/fast_export.o vcs-svn/sliding_window.o \\\n-\tvcs-svn/svndiff.o vcs-svn/svndump.o\n+\tvcs-svn/svndiff.o vcs-svn/svndump.o vcs-svn/dir_cache.o\n VCSSVN_TEST_OBJS = test-obj-pool.o \\\n \ttest-line-buffer.o test-treap.o test-svn-fe.o\n OBJECTS := $(GIT_OBJS) $(XDIFF_OBJS) $(VCSSVN_OBJS)\ndiff --git a/vcs-svn/dir_cache.c b/vcs-svn/dir_cache.c\nnew file mode 100644\nindex 0000000..9a608ce\n--- /dev/null\n+++ b/vcs-svn/dir_cache.c\n@@ -0,0 +1,40 @@\n+/*\n+ * Licensed under a two-clause BSD-style license.\n+ * See LICENSE for details.\n+ */\n+\n+#include \"git-compat-util.h\"\n+#include \"string-list.h\"\n+#include \"line_buffer.h\"\n+#include \"dump_export.h\"\n+\n+static struct string_list dirents = STRING_LIST_INIT_DUP;\n+static struct string_list_item *dir = NULL;\n+\n+void dir_cache_add(const char *path, enum node_kind kind) {\n+\tdir = string_list_insert(&dirents, path);\n+\tdir->util = malloc(sizeof(enum node_kind));\n+\t*((enum node_kind *)(dir->util)) = kind;\n+}\n+\n+void dir_cache_remove(const char *path) {\n+\tdir = string_list_lookup(&dirents, path);\n+\tif (dir)\n+\t\t*((enum node_kind *)(dir->util)) = NODE_KIND_UNKNOWN;\n+}\n+\n+enum node_kind dir_cache_lookup(const char *path) {\n+\tdir = string_list_lookup(&dirents, path);\n+\tif (dir)\n+\t\treturn *((enum node_kind *)(dir->util));\n+\telse\n+\t\treturn NODE_KIND_UNKNOWN;\n+}\n+\n+void dir_cache_init() {\n+\treturn;\n+}\n+\n+void dir_cache_deinit() {\n+\tstring_list_clear(&dirents, 1);\n+}\ndiff --git a/vcs-svn/dir_cache.h b/vcs-svn/dir_cache.h\nnew file mode 100644\nindex 0000000..43c3797\n--- /dev/null\n+++ b/vcs-svn/dir_cache.h\n@@ -0,0 +1,12 @@\n+#ifndef DIR_CACHE_H_\n+#define DIR_CACHE_H_\n+\n+#include \"dump_export.h\"\n+\n+void dir_cache_add(const char *path, enum node_kind kind);\n+void dir_cache_remove(const char *path);\n+enum node_kind dir_cache_lookup(const char *path);\n+void dir_cache_init();\n+void dir_cache_deinit();\n+\n+#endif\ndiff --git a/vcs-svn/dump_export.c b/vcs-svn/dump_export.c\nindex 04ede06..6f2a9d7 100644\n--- a/vcs-svn/dump_export.c\n+++ b/vcs-svn/dump_export.c\n@@ -7,6 +7,9 @@\n #include \"strbuf.h\"\n #include \"line_buffer.h\"\n #include \"dump_export.h\"\n+#include \"dir_cache.h\"\n+\n+static struct strbuf props;\n \n void dump_export_begin_rev(int revision, const char *revprops,\n \t\t\tint prop_len) {\n@@ -19,39 +22,47 @@ void dump_export_begin_rev(int revision, const char *revprops,\n void dump_export_node(const char *path, enum node_kind kind,\n \t\tenum node_action action, unsigned long text_len,\n \t\tunsigned long copyfrom_rev, const char *copyfrom_path) {\n+\tint dump_props = 1; /* Boolean */\n+\tstrbuf_reset(&props);\n \tprintf(\"Node-path: %s\\n\", path);\n-\tprintf(\"Node-kind: \");\n-\tswitch (action) {\n+\tswitch (kind) {\n \tcase NODE_KIND_NORMAL:\n-\t\tprintf(\"file\\n\");\n+\t\tprintf(\"Node-kind: file\\n\");\n \t\tbreak;\n \tcase NODE_KIND_EXECUTABLE:\n-\t\tprintf(\"file\\n\");\n+\t\tprintf(\"Node-kind: file\\n\");\n+\t\tstrbuf_addf(&props, \"K\\nsvn:executable\\nV\\n1\\n\");\n \t\tbreak;\n \tcase NODE_KIND_SYMLINK:\n-\t\tprintf(\"file\\n\");\n+\t\tprintf(\"Node-kind: file\\n\");\n+\t\tstrbuf_addf(&props, \"K\\nsvn:special\\nV\\n1\\n\");\n \t\tbreak;\n \tcase NODE_KIND_GITLINK:\n-\t\tprintf(\"file\\n\");\n+\t\tprintf(\"Node-kind: file\\n\");\n+\t\tbreak;\n+\tcase NODE_KIND_DIR:\n+\t\tprintf(\"Node-kind: dir\\n\");\n \t\tbreak;\n \tcase NODE_KIND_SUBDIR:\n \t\tdie(\"Unsupported: subdirectory\");\n \tdefault:\n-\t\tbreak;\n+\t\tbreak; /* When a node is being removed, nothing is printed */\n \t}\n-\tprintf(\"Node-action: \");\n+\tstrbuf_add(&props, \"PROPS-END\\n\", 10);\n+\n \tswitch (action) {\n \tcase NODE_ACTION_CHANGE:\n-\t\tprintf(\"change\\n\");\n+\t\tprintf(\"Node-action: change\\n\");\n \t\tbreak;\n \tcase NODE_ACTION_ADD:\n-\t\tprintf(\"add\\n\");\n+\t\tprintf(\"Node-action: add\\n\");\n \t\tbreak;\n \tcase NODE_ACTION_REPLACE:\n-\t\tprintf(\"replace\\n\");\n+\t\tprintf(\"Node-action: replace\\n\");\n \t\tbreak;\n \tcase NODE_ACTION_DELETE:\n-\t\tprintf(\"delete\\n\");\n+\t\tprintf(\"Node-action: delete\\n\");\n+\t\tdump_props = 0;\n \t\tbreak;\n \tdefault:\n \t\tbreak;\n@@ -60,14 +71,71 @@ void dump_export_node(const char *path, enum node_kind kind,\n \t\tprintf(\"Node-copyfrom-rev: %lu\\n\", copyfrom_rev);\n \t\tprintf(\"Node-copyfrom-path: %s\\n\", copyfrom_path);\n \t}\n-\tprintf(\"Prop-delta: false\\n\");\n-\tprintf(\"Prop-content-length: 10\\n\"); /* Constant 10 for \"PROPS-END\" */\n-\tprintf(\"Text-delta: false\\n\");\n-\tprintf(\"Text-content-length: %lu\\n\", text_len);\n-\tprintf(\"Content-length: %lu\\n\\n\", text_len + 10);\n-\tprintf(\"PROPS-END\\n\\n\");\n+\tif (dump_props) {\n+\t\tprintf(\"Prop-delta: false\\n\");\n+\t\tprintf(\"Prop-content-length: %lu\\n\", props.len);\n+\t}\n+\tif (text_len) {\n+\t\tprintf(\"Text-delta: false\\n\");\t\t\n+\t\tprintf(\"Text-content-length: %lu\\n\", text_len);\n+\t}\n+\tif (text_len || dump_props) {\n+\t\tprintf(\"Content-length: %lu\\n\\n\", text_len + props.len);\n+\t\tprintf(\"%s\", props.buf);\n+\t}\n+\tif (!text_len)\n+\t\tprintf(\"\\n\");\n+}\n+\n+void dump_export_node_r(const char *path, enum node_kind kind,\n+\t\t\tenum node_action action, unsigned long text_len,\n+\t\t\tunsigned long copyfrom_rev, const char *copyfrom_path) {\n+\tchar *start, *t;\n+\tstart = (char *) path;\n+\n+\twhile((t = strchr(start, '/'))) {\n+\t\t\t*t = '\\0';\n+\t\t\tif (dir_cache_lookup(path) == NODE_KIND_UNKNOWN) {\n+\t\t\t\tdir_cache_add(path, NODE_KIND_NORMAL);\n+\t\t\t\tdump_export_node(path, NODE_KIND_DIR,\n+\t\t\t\t\t\tNODE_ACTION_ADD, 0,\n+\t\t\t\t\t\tSVN_INVALID_REV, NULL);\n+\t\t\t}\n+\t\t\t*t = '/'; /* Change it back */\n+\t\t\tstart = t + 1;\n+\t}\n+\tswitch (dir_cache_lookup(path)) {\n+\tcase NODE_KIND_UNKNOWN:\n+\t\taction = NODE_ACTION_ADD;\n+\t\tbreak;\n+\tcase NODE_KIND_SYMLINK:\n+\t\tdir_cache_remove(path);\n+\t\tdump_export_node(path, NODE_KIND_UNKNOWN,\n+\t\t\t\tNODE_ACTION_DELETE,\n+\t\t\t\t0, SVN_INVALID_REV, NULL);\n+\t\tbreak;\n+\tcase NODE_KIND_DIR:\n+\t\tdie(\"File was previously a directory?\");\n+\t\tbreak;\n+\tcase NODE_KIND_SUBDIR:\n+\t\tdie(\"Subdirectories unsupported\");\n+\t\tbreak;\n+\tdefault:\n+\t\taction = NODE_ACTION_CHANGE;\n+\t\tbreak;\n+\t}\n+\tdir_cache_add(path, kind);\n+\tdump_export_node(path, kind, action, text_len, copyfrom_rev, copyfrom_path);\n }\n \n void dump_export_text(struct line_buffer *data, off_t len) {\n \tbuffer_copy_bytes(data, len);\n }\n+\n+void dump_export_init() {\n+\tstrbuf_init(&props, MAX_GITSVN_LINE_LEN);\n+}\n+\n+void dump_export_deinit() {\n+\tstrbuf_release(&props);\n+}\ndiff --git a/vcs-svn/dump_export.h b/vcs-svn/dump_export.h\nnew file mode 100644\nindex 0000000..e9f51a3\n--- /dev/null\n+++ b/vcs-svn/dump_export.h\n@@ -0,0 +1,33 @@\n+#ifndef DUMP_EXPORT_H_\n+#define DUMP_EXPORT_H_\n+\n+#define MAX_GITSVN_LINE_LEN 4096\n+#define SVN_INVALID_REV 0\n+\n+enum node_action {\n+\tNODE_ACTION_UNKNOWN,\n+\tNODE_ACTION_CHANGE,\n+\tNODE_ACTION_ADD,\n+\tNODE_ACTION_DELETE,\n+\tNODE_ACTION_REPLACE\n+};\n+\n+enum node_kind {\n+\tNODE_KIND_UNKNOWN, /* Missing node */\n+\tNODE_KIND_NORMAL,\n+\tNODE_KIND_EXECUTABLE,\n+\tNODE_KIND_SYMLINK,\n+\tNODE_KIND_GITLINK,\n+\tNODE_KIND_DIR, /* SVN-specific */\n+\tNODE_KIND_SUBDIR\n+};\n+\n+void dump_export_begin_rev(int revision, const char *revprops, int prop_len);\n+void dump_export_text(struct line_buffer *data, off_t len);\n+void dump_export_node_r(const char *path, enum node_kind kind,\n+\t\t\tenum node_action action, unsigned long text_len,\n+\t\t\tunsigned long copyfrom_rev, const char *copyfrom_path);\n+void dump_export_init();\n+void dump_export_deinit();\n+\n+#endif\ndiff --git a/vcs-svn/svnload.c b/vcs-svn/svnload.c\nindex 7043ae7..19c4689 100644\n--- a/vcs-svn/svnload.c\n+++ b/vcs-svn/svnload.c\n@@ -11,17 +11,16 @@\n #include \"git-compat-util.h\"\n #include \"line_buffer.h\"\n #include \"dump_export.h\"\n-#include \"strbuf.h\"\n+#include \"dir_cache.h\"\n \n #define SVN_DATE_FORMAT \"%Y-%m-%dT%H:%M:%S.000000Z\"\n #define SVN_DATE_LEN 28\n #define LENGTH_UNKNOWN (~0)\n \n static struct line_buffer input = LINE_BUFFER_INIT;\n-static struct strbuf blobs[100];\n-\t\n+\n static struct {\n-\tunsigned long prop_len, text_len, copyfrom_rev, mark;\n+\tunsigned long prop_len, text_len, copyfrom_rev;\n \tint text_delta, prop_delta; /* Boolean */\n \tenum node_action action;\n \tenum node_kind kind;\n@@ -60,15 +59,14 @@ static void reset_node_ctx(void)\n {\n \tnode_ctx.prop_len = LENGTH_UNKNOWN;\n \tnode_ctx.text_len = LENGTH_UNKNOWN;\n-\tnode_ctx.mark = 0;\n-\tnode_ctx.copyfrom_rev = 0;\n+\tnode_ctx.copyfrom_rev = SVN_INVALID_REV;\n \tnode_ctx.text_delta = -1;\n \tnode_ctx.prop_delta = -1;\n \tstrbuf_reset(&node_ctx.copyfrom_path);\n \tstrbuf_reset(&node_ctx.path);\n }\n \n-static void populate_props(struct strbuf *props, const char *author,\n+static void populate_revprops(struct strbuf *props, const char *author,\n \t\t\tconst char *log, const char *date) {\n \tstrbuf_reset(props);\t\n \tstrbuf_addf(props, \"K\\nsvn:author\\nV\\n%s\\n\", author);\n@@ -83,7 +81,7 @@ static void parse_author_line(char *val, struct strbuf *name,\n \tchar time_buf[SVN_DATE_LEN];\n \tconst struct tm *tm_time;\n \n-\t/* Simon Hausmann 1170199019 +0100 */\n+\t/* Author Name 1170199019 +0530 */\n \tstrbuf_reset(name);\n \tstrbuf_reset(email);\n \tstrbuf_reset(date);\n@@ -104,46 +102,42 @@ static void parse_author_line(char *val, struct strbuf *name,\n \n void svnload_read(void) {\n \tchar *t, *val;\n-\tint mode_incr;\n-\tstruct strbuf *to_dump;\n+\tint len, mode_incr;\n \n \twhile ((t = buffer_read_line(&input))) {\n \t\tval = strchr(t, ' ');\n-\t\tif (!val) {\n-\t\t\tif (!memcmp(t, \"blob\", 4))\n-\t\t\t\tactive_ctx = BLOB_CTX;\n-\t\t\telse if (!memcmp(t, \"deleteall\", 9))\n-\t\t\t\t;\n-\t\t\tcontinue;\n+\t\tif (!val)\n+\t\t\tlen = strlen(t);\n+\t\telse {\n+\t\t\t*val++ = '\\0';\n+\t\t\tlen = val - t - 1;\n \t\t}\n-\t\t*val++ = '\\0';\n \n-\t\t/* strlen(key) */\n-\t\tswitch (val - t - 1) {\n+\t\tswitch (len) {\n \t\tcase 1:\n \t\t\tif (!memcmp(t, \"D\", 1)) {\n \t\t\t\tnode_ctx.action = NODE_ACTION_DELETE;\n-\t\t\t}\n-\t\t\telse if (!memcmp(t, \"C\", 1)) {\n+\t\t\t} else if (!memcmp(t, \"C\", 1)) {\n \t\t\t\tnode_ctx.action = NODE_ACTION_ADD;\n-\t\t\t}\n-\t\t\telse if (!memcmp(t, \"R\", 1)) {\n+\t\t\t} else if (!memcmp(t, \"R\", 1)) {\n \t\t\t\tnode_ctx.action = NODE_ACTION_REPLACE;\n-\t\t\t}\n-\t\t\telse if (!memcmp(t, \"M\", 1)) {\n+\t\t\t} else if (!memcmp(t, \"M\", 1)) {\n \t\t\t\tnode_ctx.action = NODE_ACTION_CHANGE;\n-\t\t\t\tmode_incr = 7;\n-\t\t\t\tif (!memcmp(val, \"100644\", 6))\n-\t\t\t\t\tnode_ctx.kind = NODE_KIND_NORMAL;\n-\t\t\t\telse if (!memcmp(val, \"100755\", 6))\n-\t\t\t\t\tnode_ctx.kind = NODE_KIND_EXECUTABLE;\n-\t\t\t\telse if (!memcmp(val, \"120000\", 6))\n-\t\t\t\t\tnode_ctx.kind = NODE_KIND_SYMLINK;\n-\t\t\t\telse if (!memcmp(val, \"160000\", 6))\n-\t\t\t\t\tnode_ctx.kind = NODE_KIND_GITLINK;\n-\t\t\t\telse if (!memcmp(val, \"040000\", 6))\n-\t\t\t\t\tnode_ctx.kind = NODE_KIND_SUBDIR;\n-\t\t\t\telse {\n+\t\t\t\tif (strlen(val) >= 6) {\n+\t\t\t\t\tif (!memcmp(val, \"100644\", 6))\n+\t\t\t\t\t\tnode_ctx.kind = NODE_KIND_NORMAL;\n+\t\t\t\t\telse if (!memcmp(val, \"100755\", 6))\n+\t\t\t\t\t\tnode_ctx.kind = NODE_KIND_EXECUTABLE;\n+\t\t\t\t\telse if (!memcmp(val, \"120000\", 6))\n+\t\t\t\t\t\tnode_ctx.kind = NODE_KIND_SYMLINK;\n+\t\t\t\t\telse if (!memcmp(val, \"160000\", 6))\n+\t\t\t\t\t\tnode_ctx.kind = NODE_KIND_GITLINK;\n+\t\t\t\t\telse if (!memcmp(val, \"040000\", 6))\n+\t\t\t\t\t\tnode_ctx.kind = NODE_KIND_SUBDIR;\n+\t\t\t\t\telse\n+\t\t\t\t\t\tdie(\"Unrecognized mode: %s\", val);\n+\t\t\t\t\tmode_incr = 7;\n+\t\t\t\t} else if (strlen(val) >= 3) {\n \t\t\t\t\tif (!memcmp(val, \"755\", 3))\n \t\t\t\t\t\tnode_ctx.kind = NODE_KIND_EXECUTABLE;\n \t\t\t\t\telse if(!memcmp(val, \"644\", 3))\n@@ -151,40 +145,35 @@ void svnload_read(void) {\n \t\t\t\t\telse\n \t\t\t\t\t\tdie(\"Unrecognized mode: %s\", val);\n \t\t\t\t\tmode_incr = 4;\n-\t\t\t\t}\n+\t\t\t\t} else\n+\t\t\t\t\tdie (\"Malformed filemodify: Missing mode\");\n \t\t\t\tval += mode_incr;\n \t\t\t\tt = strchr(val, ' ');\n+\t\t\t\tif (!t)\n+\t\t\t\t\tdie(\"Malformed filemodify: Missing dataref\");\n \t\t\t\t*t++ = '\\0';\n \t\t\t\tstrbuf_reset(&node_ctx.path);\n \t\t\t\tstrbuf_add(&node_ctx.path, t, strlen(t));\n-\t\t\t\tif (!memcmp(val + 1, \"inline\", 6))\n-\t\t\t\t\tdie(\"Unsupported dataref: inline\");\n+\t\t\t\tif (!strncmp(val, \"inline\", 6))\n+\t\t\t\t\tactive_ctx = BLOB_CTX;\n \t\t\t\telse if (*val == ':')\n-\t\t\t\t\tto_dump = &blobs[strtoul(val + 1, NULL, 10)];\n+\t\t\t\t\tdie(\"Unsupported dataref: marks\");\n \t\t\t\telse\n \t\t\t\t\tdie(\"Unsupported dataref: sha1\");\n-\t\t\t\tdump_export_node(node_ctx.path.buf, node_ctx.kind,\n-\t\t\t\t\t\tnode_ctx.action, to_dump->len,\n-\t\t\t\t\t\t0, NULL);\n-\t\t\t\tprintf(\"%s\", to_dump->buf);\n \t\t\t}\n \t\t\tbreak;\n+\t\tcase 2:\n+\t\t\tif (!memcmp(t, \"ls\", 2))\n+\t\t\t\tdie(\"ls not supported\");\n \t\tcase 3:\n \t\t\tif (!memcmp(t, \"tag\", 3))\n \t\t\t\tcontinue;\n \t\t\tbreak;\n \t\tcase 4:\n-\t\t\tif (!memcmp(t, \"mark\", 4))\n-\t\t\t\tswitch(active_ctx) {\n-\t\t\t\tcase COMMIT_CTX:\n-\t\t\t\t\t/* What do we do with commit marks? */\n-\t\t\t\t\tcontinue;\n-\t\t\t\tcase BLOB_CTX:\n-\t\t\t\t\tnode_ctx.mark = strtoul(val + 1, NULL, 10);\n-\t\t\t\t\tbreak;\n-\t\t\t\tdefault:\n-\t\t\t\t\tbreak;\n-\t\t\t\t}\n+\t\t\tif (!memcmp(t, \"blob\", 4))\n+\t\t\t\tdie(\"Please inline blobs in the fast-import stream\");\n+\t\t\telse if (!memcmp(t, \"mark\", 4))\n+\t\t\t\tcontinue;\n \t\t\telse if (!memcmp(t, \"from\", 4))\n \t\t\t\tcontinue;\n \t\t\telse if (!memcmp(t, \"data\", 4)) {\n@@ -194,7 +183,7 @@ void svnload_read(void) {\n \t\t\t\t\tbuffer_read_binary(&input,\n \t\t\t\t\t\t\t&rev_ctx.log,\n \t\t\t\t\t\t\tstrtoul(val, NULL, 10));\n-\t\t\t\t\tpopulate_props(&rev_ctx.props,\n+\t\t\t\t\tpopulate_revprops(&rev_ctx.props,\n \t\t\t\t\t\trev_ctx.svn_author.buf,\n \t\t\t\t\t\trev_ctx.log.buf,\n \t\t\t\t\t\trev_ctx.author_date.buf);\n@@ -204,9 +193,10 @@ void svnload_read(void) {\n \t\t\t\t\tbreak;\n \t\t\t\tcase BLOB_CTX:\n \t\t\t\t\tnode_ctx.text_len = strtoul(val, NULL, 10);\n-\t\t\t\t\tbuffer_read_binary(&input,\n-\t\t\t\t\t\t\t&blobs[node_ctx.mark],\n-\t\t\t\t\t\t\tnode_ctx.text_len);\n+\t\t\t\t\tdump_export_node_r(node_ctx.path.buf, node_ctx.kind,\n+\t\t\t\t\t\t\tnode_ctx.action, node_ctx.text_len,\n+\t\t\t\t\t\t\tSVN_INVALID_REV, NULL);\n+\t\t\t\t\tbuffer_copy_bytes(&input, node_ctx.text_len);\n \t\t\t\t\tbreak;\n \t\t\t\tdefault:\n \t\t\t\t\tbreak;\n@@ -231,14 +221,18 @@ void svnload_read(void) {\n \t\t\t\t\trev_ctx.author_email.buf,\n \t\t\t\t\tt - rev_ctx.author_email.buf);\n \n-\t\t\t}\n-\t\t\telse if (!memcmp(t, \"commit\", 6)) {\n+\t\t\t} else if (!memcmp(t, \"commit\", 6)) {\n \t\t\t\trev_ctx.rev ++;\n \t\t\t\tactive_ctx = COMMIT_CTX;\n \t\t\t}\n \t\t\tbreak;\n+\t\tcase 8:\n+\t\t\tif (!memcmp(t, \"cat-blob\", 8))\n+\t\t\t\tdie(\"cat-blob unsupported\");\n \t\tcase 9:\n-\t\t\tif (!memcmp(t, \"committer\", 9))\n+\t\t\tif (!memcmp(t, \"deleteall\", 9))\n+\t\t\t\tcontinue;\n+\t\t\telse if (!memcmp(t, \"committer\", 9))\n \t\t\t\tparse_author_line(val, &rev_ctx.committer,\n \t\t\t\t\t\t&rev_ctx.committer_email,\n \t\t\t\t\t\t&rev_ctx.committer_date);\n@@ -251,7 +245,6 @@ void svnload_read(void) {\n \n int svnload_init(const char *filename)\n {\n-\tint i;\n \tif (buffer_init(&input, filename))\n \t\treturn error(\"cannot open %s: %s\", filename, strerror(errno));\n \tactive_ctx = UNKNOWN_CTX;\n@@ -265,14 +258,13 @@ int svnload_init(const char *filename)\n \tstrbuf_init(&rev_ctx.committer_email, MAX_GITSVN_LINE_LEN);\n \tstrbuf_init(&node_ctx.path, MAX_GITSVN_LINE_LEN);\n \tstrbuf_init(&node_ctx.copyfrom_path, MAX_GITSVN_LINE_LEN);\n-\tfor (i = 0; i < 100; i ++)\n-\t\tstrbuf_init(&blobs[i], 10000);\n+\tdump_export_init();\n+\tdir_cache_init();\n \treturn 0;\n }\n \n void svnload_deinit(void)\n {\n-\tint i;\n \treset_rev_ctx(0);\n \treset_node_ctx();\n \tstrbuf_release(&rev_ctx.props);\n@@ -285,8 +277,8 @@ void svnload_deinit(void)\n \tstrbuf_release(&rev_ctx.committer_email);\n \tstrbuf_release(&node_ctx.path);\n \tstrbuf_release(&node_ctx.copyfrom_path);\n-\tfor (i = 0; i < 100; i ++)\n-\t\tstrbuf_release(&blobs[i]);\n+\tdump_export_deinit();\n+\tdir_cache_deinit();\n \tif (buffer_deinit(&input))\n \t\tfprintf(stderr, \"Input error\\n\");\n \tif (ferror(stdout))\ndiff --git a/vcs-svn/svnload.h b/vcs-svn/svnload.h\nnew file mode 100644\nindex 0000000..0c8fe8b\n--- /dev/null\n+++ b/vcs-svn/svnload.h\n@@ -0,0 +1,10 @@\n+#ifndef SVNLOAD_H_\n+#define SVNLOAD_H_\n+\n+#define SVN_INVALID_REV -1\n+\n+int svnload_init(const char *filename);\n+void svnload_deinit(void);\n+void svnload_read(void);\n+\n+#endif\n-- \n1.7.4.rc1.7.g2cf08.dirty\n"},{"id":"159667","messageId":"7vtyh4smer.fsf@alter.siamese.dyndns.org","threadId":"26304","inReplyTo":"1295415899-1192-5-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 4/5] fast-export: Introduce --inline-blobs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-01-19T19:50:36Z","receivedAt":"2011-01-19T19:50:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> Introduce a new command-line option --inline-blobs that always inlines\n> blobs instead of referring to them via marks or their original SHA-1\n> hash.\n>\n> Signed-off-by: Ramkumar Ramachandra \n> ---\n> Documentation/git-fast-export.txt | 5 +++++\n> builtin/fast-export.c | 23 +++++++++++++++++++++--\n> 2 files changed, 26 insertions(+), 2 deletions(-)\n\nHmm, this smells somewhat fishy.\n\nWasn't G-F-I designed to be a common stream format for other SCMs to\ngenerate streams, so that importers and exporters can be written once for\neach SCM to interoperate?\n\nThis patch will allow you to write an importer that can only take a stream\nwith inlined blobs without any references to previous occurrences, but if\nthe exporter for an SCM that you are trying to interoperate with does not\nsupport --inline-blobs, you are screwed.\n\nWhat is the problem you are really trying to solve? If it is \"it is\ncumbersome to keep track of blob references\", wouldn't it be nicer to\ninstead make it easier for importers to support referenced blobs?\n\nJust thinking aloud, but is it possible to write a filter that converts an\narbitrary G-F-I stream with referenced blobs into a G-F-I stream without\nreferenced blobs by inlining all the blobs? Then other people do not have\nto implement --inline-blobs to their own exporter.\n\nIf that is not possible, should/can we do something to at least allow\npeople to check if an existing stream is compatible with an importer that\ncannot take referenced blobs without actually trying to run import (and\nsee it fail)? Do we need a way to encourage people to add --inline-blobs\nsupport to their exporters? I suspect this series leads to make G-F-I\nless useful by fragmenting the compatible subset of stream formats without\nsuch effort, no?\n"},{"id":"159673","messageId":"20110119214827.GA31733@burratino","threadId":"26304","inReplyTo":"7vtyh4smer.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/5] fast-export: Introduce --inline-blobs","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-01-19T21:48:27Z","receivedAt":"2011-01-19T21:48:27Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Ramkumar Ramachandra writes:\n\n>> Introduce a new command-line option --inline-blobs that always inlines\n>> blobs instead of referring to them via marks or their original SHA-1\n>> hash.\n[...]\n> Hmm, this smells somewhat fishy.\n>\n> Wasn't G-F-I designed to be a common stream format for other SCMs to\n> generate streams, so that importers and exporters can be written once for\n> each SCM to interoperate?\n\nHere is one way to sell it:\n\n\tWith the inline blobs feature, fast-import backends have to\n\tmaintain less state. Using it should speed up exporting.\n\n\tThis is made optional because ...\n\nI haven't thought through whether it ought to be optional or measured\nthe effect on import performance.\n\nA separate question is what an svn fast-import backend should do with\nall those blobs that are not ready to be written to dump. As a hack\nwhile prototyping, one can rely on the \"current\" fast-export output,\neven though that is not flexible or futureproof. Longer term, the\nfolllowing sounds very interesting\n\n> Just thinking aloud, but is it possible to write a filter that converts an\n> arbitrary G-F-I stream with referenced blobs into a G-F-I stream without\n> referenced blobs by inlining all the blobs?\n\nto avoid complexity in the svn fast-import backend itself.\n(Complicating detail: such a filter would presumably take responsibility\nfor --export-marks, so it might want a way to retrieve commit marks\nfrom its downstream.)\n"},{"id":"159681","messageId":"20110120045046.GB9493@kytes","threadId":"26304","inReplyTo":"20110119214827.GA31733@burratino","subject":"Re: [PATCH 4/5] fast-export: Introduce --inline-blobs","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-01-20T04:50:48Z","receivedAt":"2011-01-20T04:50:48Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nJonathan Nieder writes:\n> Junio C Hamano wrote:\n> > Ramkumar Ramachandra writes:\n> \n> >> Introduce a new command-line option --inline-blobs that always inlines\n> >> blobs instead of referring to them via marks or their original SHA-1\n> >> hash.\n> [...]\n> > Hmm, this smells somewhat fishy.\n> >\n> > Wasn't G-F-I designed to be a common stream format for other SCMs to\n> > generate streams, so that importers and exporters can be written once for\n> > each SCM to interoperate?\n> \n> Here is one way to sell it:\n> \n> \tWith the inline blobs feature, fast-import backends have to\n> \tmaintain less state. Using it should speed up exporting.\n> \n> \tThis is made optional because ...\n> \n> I haven't thought through whether it ought to be optional or measured\n> the effect on import performance.\n\nIt simplifies other fast-import backends greatly, because persisting\nblobs can be complicated and expensive. I was thinking of making\nsvn-fe support both inlined blobs, and blobs referenced by marks. When\nit's possible to be cheap by optionally having inlined blobs, why not\noptionally have them? The filter we develop later can be used for\nolder fast-import streams that don't have inlined blobs.\n\nOn a related note, does it make sense to version our fast-import\nstream format? It's certainly going to keep evolving with time, and we\nneed backward compatibility.\n\n> A separate question is what an svn fast-import backend should do with\n> all those blobs that are not ready to be written to dump. As a hack\n> while prototyping, one can rely on the \"current\" fast-export output,\n> even though that is not flexible or futureproof. Longer term, the\n> folllowing sounds very interesting\n\nGood point. The functionality to persist blobs that are refenced by\nmarks probably shouldn't be in svn-fe at all.\n\n> > Just thinking aloud, but is it possible to write a filter that converts an\n> > arbitrary G-F-I stream with referenced blobs into a G-F-I stream without\n> > referenced blobs by inlining all the blobs?\n> \n> to avoid complexity in the svn fast-import backend itself.\n> (Complicating detail: such a filter would presumably take responsibility\n> for --export-marks, so it might want a way to retrieve commit marks\n> from its downstream.)\n\nThis filter will need to persist every blob for the entire lifetime of\nthe program. We can't possibly do it in-memory, so we have to find\nsome way to persist them on-disk and retrieve them very\nquickly. Jonathan suggested using something like ToyoCabinet earlier-\nI'll start working and see what I come up with.\n\n-- Ram\n"},{"id":"159682","messageId":"20110120054126.GA4770@burratino","threadId":"26304","inReplyTo":"7vtyh4smer.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/5] fast-export: Introduce --inline-blobs","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-01-20T05:41:26Z","receivedAt":"2011-01-20T05:41:26Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> Just thinking aloud, but is it possible to write a filter that converts an\n> arbitrary G-F-I stream with referenced blobs into a G-F-I stream without\n> referenced blobs by inlining all the blobs?\n\nA few details to watch out for:\n\n- A mark, as in\n\n\tM 100644 :1 path/to/file\n\n can refer to a blob from a previous import. A mark can even refer\n to a manually prepared marks file.\n\n- The syntax\n\n\tM 100644 0409ac9fd3f1ea36680189e07116e58b2630ccad path/to/file\n\n refers to a blob that might not have been mentioned elsewhere in the\n stream. This is the variant used by \"git fast-export --no-data\" to\n avoid transferring blob data. (In general, non-git backends would\n presumably use something other than git blob IDs if they use this\n feature. A filter of the kind we are describing would probably pass\n them through.)\n\n These datarefs can be acquired out of band (probably not a big deal)\n or by using the \"ls\" command to copy from a previous revision:\n\n\t> ls :3 \"path/to/other/file\"\n\t100644 blob 0409ac9fd3f1ea36680189e07116e58b2630ccad\tgit.c\n\t> M 100644 0409ac9fd3f1ea36680189e07116e58b2630ccad path/to/file\n\n- The cat-blob command (\"cat-blob :1\") allows frontends to request\n the content of a previously imported blob (presumably in order to\n apply a delta to it).\n\nSo while something like the filter you describe seems possible, it\ncannot be as simple as\n\n\tmkfifo replies &&\n\tfast-export-frontend 3 3>replies\n\nfor general and . The frontend might try to cat\nblobs by mark number or to pick off where it left off in a previous\nrun using a marks file.\n"},{"id":"159683","messageId":"20110120054813.GB4770@burratino","threadId":"26304","inReplyTo":"20110120045046.GB9493@kytes","subject":"Re: [PATCH 4/5] fast-export: Introduce --inline-blobs","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-01-20T05:48:13Z","receivedAt":"2011-01-20T05:48:13Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> The functionality to persist blobs that are refenced by\n> marks probably shouldn't be in svn-fe at all.\n\nDo you mean svn-fi?\n\n> This filter will need to persist every blob for the entire lifetime of\n> the program.\n\nDepending on the interface, couldn't it be possible to rely on svn for\nthe content of blobs that have already been exported? If so, one\nwould only need a place to stash (1) a mapping from mark numbers to\n(svn rev, path) pairs and (2) the full text of blobs that have not\nbeen exported as part of a rev yet.\n\nCheers,\nJonathan\n"},{"id":"159684","messageId":"20110120062837.GA27624@kytes","threadId":"26304","inReplyTo":"20110120054813.GB4770@burratino","subject":"Re: [PATCH 4/5] fast-export: Introduce --inline-blobs","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-01-20T06:28:39Z","receivedAt":"2011-01-20T06:28:39Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Jonathan,\n\nJonathan Nieder writes:\n> Ramkumar Ramachandra wrote:\n> \n> > The functionality to persist blobs that are refenced by\n> > marks probably shouldn't be in svn-fe at all.\n> \n> Do you mean svn-fi?\n\nYeah. Sorry about the typo. Just to make it clear:\n\nsvn-fi should only ever support inlined blobs. For older streams,\nthere's an extra overhead- it'll have to be chained along with a\nhelper program that transforms the stream to inline all the blobs.\n\n> > This filter will need to persist every blob for the entire lifetime of\n> > the program.\n> \n> Depending on the interface, couldn't it be possible to rely on svn for\n> the content of blobs that have already been exported? If so, one\n> would only need a place to stash (1) a mapping from mark numbers to\n> (svn rev, path) pairs and (2) the full text of blobs that have not\n> been exported as part of a rev yet.\n\nOh yes. We discussed this on IRC :) I'm just afraid that it won't be\nfast enough- my idea is to essentially use `svnrdump dump` to replay\nthe blobs in a certain (path, revision); let me know if you think\nthere's a quicker way.\n\n-- Ram\n"},{"id":"159690","messageId":"1295531623.4298.26.camel@drew-northup.unet.maine.edu","threadId":"26304","inReplyTo":"20110120045046.GB9493@kytes","subject":"Re: [PATCH 4/5] fast-export: Introduce --inline-blobs","fromName":"Drew Northup","fromEmail":"drew.northup@maine.edu","sentAt":"2011-01-20T13:53:43Z","receivedAt":"2011-01-20T13:53:43Z","isPatch":true,"sender":{"key":"drew.northup@maine.edu","avatar":"https://avatars.githubusercontent.com/u/18331571?v=4"},"body":"\nOn Thu, 2011-01-20 at 10:20 +0530, Ramkumar Ramachandra wrote:\n> Hi,\n> \n> Jonathan Nieder writes:\n> > Junio C Hamano wrote:\n> > > Ramkumar Ramachandra writes:\n> > > Just thinking aloud, but is it possible to write a filter that converts an\n> > > arbitrary G-F-I stream with referenced blobs into a G-F-I stream without\n> > > referenced blobs by inlining all the blobs?\n> > \n> > to avoid complexity in the svn fast-import backend itself.\n> > (Complicating detail: such a filter would presumably take responsibility\n> > for --export-marks, so it might want a way to retrieve commit marks\n> > from its downstream.)\n> \n> This filter will need to persist every blob for the entire lifetime of\n> the program. We can't possibly do it in-memory, so we have to find\n> some way to persist them on-disk and retrieve them very\n> quickly. Jonathan suggested using something like ToyoCabinet earlier-\n> I'll start working and see what I come up with.\n\nIs it worth including the extra dependency? Most systems that I'm in\nfrequent contact with already have some lightweight BDB implementation\nalready. I don't currently know of any with TokyoCabinet (or\nKyotoCabinet for that matter) already in place. Besides, if all you're\ndoing is persisting blobs that you're likely to write out to disk\neventually anyway you might as well just do so once you have them and\nkeep an \"index\" (not to be confused with the Git Index, just lacking a\nbetter word right now) of what you have in some standard in-memory\nformat (a heap?). From there you can build each commit into the Git\nIndex in the proper order once you have the required parts for\neach--perhaps even re-using the blobs you've already dumped to disk\n(mv'ing them or something).\n\nGranted, there's a good likelihood that I'm missing something here, but\nI don't see the point of adding external complexity (beyond what you're\ncurrently stuck dealing with).\n\n-- \n-Drew Northup\n________________________________________________\n\"As opposed to vegetable or mineral error?\"\n-John Pescatore, SANS NewsBites Vol. 12 Num. 59\n"},{"id":"159780","messageId":"7v39olok4l.fsf@alter.siamese.dyndns.org","threadId":"26304","inReplyTo":"1295415899-1192-3-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 2/5] vcs-svn: Start working on the dumpfile producer","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-01-22T00:30:18Z","receivedAt":"2011-01-22T00:30:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> Start off with some broad design sketches. Compile succeeds, but\n> parser is incorrect. Include a Makefile rule to build it into\n> vcs-svn/lib.a.\n>\n> Signed-off-by: Ramkumar Ramachandra \n\nI initially thought I should refrain from doing a usual picky review for\ncontrib/ material, but realized this does not go to contrib/, so...\n\n> diff --git a/vcs-svn/dump_export.c b/vcs-svn/dump_export.c\n> new file mode 100644\n> index 0000000..04ede06\n> --- /dev/null\n> +++ b/vcs-svn/dump_export.c\n> @@ -0,0 +1,73 @@\n> +/*\n> + * Licensed under a two-clause BSD-style license.\n> + * See LICENSE for details.\n> + */\n> +\n> +#include \"git-compat-util.h\"\n> +#include \"strbuf.h\"\n> +#include \"line_buffer.h\"\n> +#include \"dump_export.h\"\n> +\n> +void dump_export_begin_rev(int revision, const char *revprops,\n> +\t\t\tint prop_len) {\n\nStyle. \"{\" at the beginning of the next line (same everywhere).\n\n> +void dump_export_node(const char *path, enum node_kind kind,\n> +\t\tenum node_action action, unsigned long text_len,\n> +\t\tunsigned long copyfrom_rev, const char *copyfrom_path) {\n> +\tprintf(\"Node-path: %s\\n\", path);\n> +\tprintf(\"Node-kind: \");\n> +\tswitch (action) {\n> +\tcase NODE_KIND_NORMAL:\n> +\t\tprintf(\"file\\n\");\n> +\t\tbreak;\n> +\tcase NODE_KIND_EXECUTABLE:\n> +\t\tprintf(\"file\\n\");\n> +\t\tbreak;\n> +\tcase NODE_KIND_SYMLINK:\n> +\t\tprintf(\"file\\n\");\n> +\t\tbreak;\n> +\tcase NODE_KIND_GITLINK:\n> +\t\tprintf(\"file\\n\");\n> +\t\tbreak;\n> +\tcase NODE_KIND_SUBDIR:\n> +\t\tdie(\"Unsupported: subdirectory\");\n> +\tdefault:\n> +\t\tbreak;\n> +\t}\n\nHmph, is it expected that the repertoire of node-kind stay the same, or\nwill it be extended over time? It might make sense to change the above\nswitch a more table-driven one. The same for node-action that follows\nthis part of the code.\n\n> diff --git a/vcs-svn/svnload.c b/vcs-svn/svnload.c\n> new file mode 100644\n> index 0000000..7043ae7\n> --- /dev/null\n> +++ b/vcs-svn/svnload.c\n> @@ -0,0 +1,294 @@\n> ...\n> +#define SVN_DATE_FORMAT \"%Y-%m-%dT%H:%M:%S.000000Z\"\n> +#define SVN_DATE_LEN 28\n> +#define LENGTH_UNKNOWN (~0)\n> +\n> +static struct line_buffer input = LINE_BUFFER_INIT;\n> +static struct strbuf blobs[100];\n> +\t\n\nIs there a rationale for \"100\", or is it just a random number that can be\ntweakable at the source level? Would we want to have a symbolic constant\nfor it?\n\nThe \"blank\" line has a trailing whitespace.\n\n> +static struct {\n> +\tunsigned long prop_len, text_len, copyfrom_rev, mark;\n> +\tint text_delta, prop_delta; /* Boolean */\n> +\tenum node_action action;\n> +\tenum node_kind kind;\n> +\tstruct strbuf copyfrom_path, path;\n> +} node_ctx;\n> + ...\n> +static void reset_node_ctx(void)\n> +{\n> +\tnode_ctx.prop_len = LENGTH_UNKNOWN;\n> +\tnode_ctx.text_len = LENGTH_UNKNOWN;\n> +\tnode_ctx.mark = 0;\n> +\tnode_ctx.copyfrom_rev = 0;\n> +\tnode_ctx.text_delta = -1;\n> +\tnode_ctx.prop_delta = -1;\n\nDoes your \"Boolean\" type take values 0 or -1? Or is it perhaps a tristate\nfalse=0, true=1, unknown=-1? If so, the comment on the type above needs\nto be fixed.\n\n> +\tstrbuf_reset(&node_ctx.copyfrom_path);\n> +\tstrbuf_reset(&node_ctx.path);\n> +}\n> +\n> +static void populate_props(struct strbuf *props, const char *author,\n> +\t\t\tconst char *log, const char *date) {\n\nStyle on \"{\" (look everywhere).\n\n> +\tstrbuf_reset(props);\t\n\nTrailing whitespace.\n\n> +static void parse_author_line(char *val, struct strbuf *name,\n> +\t\t\tstruct strbuf *email, struct strbuf *date) {\n> +\tchar *t, *tz_off;\n> +\tchar time_buf[SVN_DATE_LEN];\n> +\tconst struct tm *tm_time;\n> +\n> +\t/* Simon Hausmann 1170199019 +0100 */\n> +\tstrbuf_reset(name);\n> +\tstrbuf_reset(email);\n> +\tstrbuf_reset(date);\n> +\ttz_off = strrchr(val, ' ');\n> +\t*tz_off++ = '\\0';\n> +\tt = strrchr(val, ' ');\n> +\t*(t - 1) = '\\0'; /* Ignore '>' from email */\n> +\tt ++;\n\nUnwanted SP before \"++\" (look everywhere).\n\n> +\ttm_time = time_to_tm(strtoul(t, NULL, 10), atoi(tz_off));\n\nHas your caller already verified tz_off is a reasonable integer?\n\n> +void svnload_read(void) {\n> ...\n> +\t\tswitch (val - t - 1) {\n> +\t\tcase 1:\n> +\t\t\tif (!memcmp(t, \"D\", 1)) {\n> +\t\t\t\tnode_ctx.action = NODE_ACTION_DELETE;\n> +\t\t\t}\n> +\t\t\telse if (!memcmp(t, \"C\", 1)) {\n\nStyle: \"} else if (...) {\".\n\n> +\t\t\t\tnode_ctx.action = NODE_ACTION_ADD;\n> +\t\t\t}\n> +\t\t\telse if (!memcmp(t, \"R\", 1)) {\n> +\t\t\t\tnode_ctx.action = NODE_ACTION_REPLACE;\n> +\t\t\t}\n> +\t\t\telse if (!memcmp(t, \"M\", 1)) {\n> +\t\t\t\tnode_ctx.action = NODE_ACTION_CHANGE;\n> +\t\t\t\tmode_incr = 7;\n> +\t\t\t\tif (!memcmp(val, \"100644\", 6))\n> +\t\t\t\t\tnode_ctx.kind = NODE_KIND_NORMAL;\n> +\t\t\t\telse if (!memcmp(val, \"100755\", 6))\n> +\t\t\t\t\tnode_ctx.kind = NODE_KIND_EXECUTABLE;\n> +\t\t\t\telse if (!memcmp(val, \"120000\", 6))\n> +\t\t\t\t\tnode_ctx.kind = NODE_KIND_SYMLINK;\n> +\t\t\t\telse if (!memcmp(val, \"160000\", 6))\n> +\t\t\t\t\tnode_ctx.kind = NODE_KIND_GITLINK;\n> +\t\t\t\telse if (!memcmp(val, \"040000\", 6))\n\nIs guaranteed to be a 6-digit octal, padded with 0 to the left?\n\nThe manual page of git-fast-import seems to hint that is the case, but I\nam double checking, as the leading zero is an error in the tree object.\n\n> +\t\t\t\t\tnode_ctx.kind = NODE_KIND_SUBDIR;\n> +\t\t\t\telse {\n> +\t\t\t\t\tif (!memcmp(val, \"755\", 3))\n> +\t\t\t\t\t\tnode_ctx.kind = NODE_KIND_EXECUTABLE;\n> +\t\t\t\t\telse if(!memcmp(val, \"644\", 3))\n\nStyle; missing SP after \"if/switch\" (look everywhere).\n\n> +\t\t\t\t\t\tnode_ctx.kind = NODE_KIND_NORMAL;\n> +\t\t\t\t\telse\n> +\t\t\t\t\t\tdie(\"Unrecognized mode: %s\", val);\n> +\t\t\t\t\tmode_incr = 4;\n> +\t\t\t\t}\n> +\t\t\t\tval += mode_incr;\n\nHmm, this whole thing is ugly.\n\nPerhaps a table-lookup, or at least a helper function that translates\n\"val\" to node-kind (while advancing val as the side effect) may make it\neasier to read?\n\n> +\t\t\t\tt = strchr(val, ' ');\n> +\t\t\t\t*t++ = '\\0';\n> +\t\t\t\tstrbuf_reset(&node_ctx.path);\n> +\t\t\t\tstrbuf_add(&node_ctx.path, t, strlen(t));\n> +\t\t\t\tif (!memcmp(val + 1, \"inline\", 6))\n> +\t\t\t\t\tdie(\"Unsupported dataref: inline\");\n> +\t\t\t\telse if (*val == ':')\n> +\t\t\t\t\tto_dump = &blobs[strtoul(val + 1, NULL, 10)];\n\nHas anybody so far verified the decimal number at (val+1) is a reasonable\nvalue, or am I seeing a future CVE here?\n\nDo we care what follows the len of digits on this line? Does a line in G-F-I\nstream allow trailing garbage (this question is not limited to this part\nof the code)?\n> +int svnload_init(const char *filename)\n> +{\n> +\tint i;\n> +\tif (buffer_init(&input, filename))\n> +\t\treturn error(\"cannot open %s: %s\", filename, strerror(errno));\n> +\tactive_ctx = UNKNOWN_CTX;\n> +\tstrbuf_init(&rev_ctx.props, MAX_GITSVN_LINE_LEN);\n> +\t...\n> +\tstrbuf_init(&node_ctx.copyfrom_path, MAX_GITSVN_LINE_LEN);\n> +\tfor (i = 0; i < 100; i ++)\n> +\t\tstrbuf_init(&blobs[i], 10000);\n> +\treturn 0;\n> +}\n> +\n> +void svnload_deinit(void)\n> +{\n> +\tint i;\n> +\treset_rev_ctx(0);\n> +\treset_node_ctx();\n> +\tstrbuf_release(&rev_ctx.props);\n> +\t...\n> +\tstrbuf_release(&node_ctx.copyfrom_path);\n> +\tfor (i = 0; i < 100; i ++)\n> +\t\tstrbuf_release(&blobs[i]);\n> +\tif (buffer_deinit(&input))\n> +\t\tfprintf(stderr, \"Input error\\n\");\n> +\tif (ferror(stdout))\n> +\t\tfprintf(stderr, \"Output error\\n\");\n> +}\n\nWhoever needs to add an element to rev_ctx must consistently touch reset,\ninit and deinit. Would it help him/her if you moved the code so that\nthese functions are close together from the beginning?\n"},{"id":"159781","messageId":"7vvd1hn5jp.fsf@alter.siamese.dyndns.org","threadId":"26304","inReplyTo":"1295415899-1192-5-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 4/5] fast-export: Introduce --inline-blobs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-01-22T00:30:34Z","receivedAt":"2011-01-22T00:30:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> +\t\t\t\tbuf = read_sha1_file(spec->sha1, &type, &size);\n> +\t\t\t\tif (!buf)\n> +\t\t\t\t\tdie (\"Could not read blob %s\",\n> +\t\t\t\t\t\tsha1_to_hex(spec->sha1));\n\nStyle; no SP after \"die\", as it is a function, not syntactic elements like\nif/switch/while (look everywhere).\n"},{"id":"159784","messageId":"20110122092416.GA7827@kytes","threadId":"26304","inReplyTo":"1295531623.4298.26.camel@drew-northup.unet.maine.edu","subject":"Re: [PATCH 4/5] fast-export: Introduce --inline-blobs","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-01-22T09:24:20Z","receivedAt":"2011-01-22T09:24:20Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Drew,\n\nDrew Northup writes:\n> On Thu, 2011-01-20 at 10:20 +0530, Ramkumar Ramachandra wrote:\n> > Hi,\n> > Jonathan Nieder writes:\n> > > Junio C Hamano wrote:\n> > > > Ramkumar Ramachandra writes:\n> > > > Just thinking aloud, but is it possible to write a filter that converts an\n> > > > arbitrary G-F-I stream with referenced blobs into a G-F-I stream without\n> > > > referenced blobs by inlining all the blobs?\n> > > \n> > > to avoid complexity in the svn fast-import backend itself.\n> > > (Complicating detail: such a filter would presumably take responsibility\n> > > for --export-marks, so it might want a way to retrieve commit marks\n> > > from its downstream.)\n> > \n> > This filter will need to persist every blob for the entire lifetime of\n> > the program. We can't possibly do it in-memory, so we have to find\n> > some way to persist them on-disk and retrieve them very\n> > quickly. Jonathan suggested using something like ToyoCabinet earlier-\n> > I'll start working and see what I come up with.\n> \n> Is it worth including the extra dependency? Most systems that I'm in\n> frequent contact with already have some lightweight BDB implementation\n> already. I don't currently know of any with TokyoCabinet (or\n> KyotoCabinet for that matter) already in place. Besides, if all you're\n> doing is persisting blobs that you're likely to write out to disk\n> eventually anyway you might as well just do so once you have them and\n> keep an \"index\" (not to be confused with the Git Index, just lacking a\n> better word right now) of what you have in some standard in-memory\n> format (a heap?). From there you can build each commit into the Git\n> Index in the proper order once you have the required parts for\n> each--perhaps even re-using the blobs you've already dumped to disk\n> (mv'ing them or something).\n\nAgreed. I wouldn't like to introduce an extra dependency either. I was\ntalking about using it for prototyping- if the final version includes\nan extra dependency, it's unlikely to get merged into git.git :) The\nfinal design will probably use an in-memory B+ tree, but I haven't\nthought about that hard enough.\n\n-- Ram\n"},{"id":"159785","messageId":"20110122094556.GB7827@kytes","threadId":"26304","inReplyTo":"7v39olok4l.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/5] vcs-svn: Start working on the dumpfile producer","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-01-22T09:45:59Z","receivedAt":"2011-01-22T09:45:59Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Junio,\n\nJunio C Hamano writes:\n> Ramkumar Ramachandra writes:\n> > Start off with some broad design sketches. Compile succeeds, but\n> > parser is incorrect. Include a Makefile rule to build it into\n> > vcs-svn/lib.a.\n> >\n> > Signed-off-by: Ramkumar Ramachandra \n> \n> I initially thought I should refrain from doing a usual picky review for\n> contrib/ material, but realized this does not go to contrib/, so...\n\nThanks for the review. A new iteration including tests is almost\nready- your review came in at the perfect time :)\n\n> > diff --git a/vcs-svn/dump_export.c b/vcs-svn/dump_export.c\n> > new file mode 100644\n> > index 0000000..04ede06\n> > --- /dev/null\n> > +++ b/vcs-svn/dump_export.c\n> > ...\n> > +void dump_export_begin_rev(int revision, const char *revprops,\n> > +\t\t\tint prop_len) {\n> \n> Style. \"{\" at the beginning of the next line (same everywhere).\n\nFixed.\n\n> > +void dump_export_node(const char *path, enum node_kind kind,\n> > +\t\tenum node_action action, unsigned long text_len,\n> > +\t\tunsigned long copyfrom_rev, const char *copyfrom_path) {\n> > +\tprintf(\"Node-path: %s\\n\", path);\n> > +\tprintf(\"Node-kind: \");\n> > +\tswitch (action) {\n> > +\tcase NODE_KIND_NORMAL:\n> > +\t\tprintf(\"file\\n\");\n> > +\t\tbreak;\n> > +\tcase NODE_KIND_EXECUTABLE:\n> > +\t\tprintf(\"file\\n\");\n> > +\t\tbreak;\n> > +\tcase NODE_KIND_SYMLINK:\n> > +\t\tprintf(\"file\\n\");\n> > +\t\tbreak;\n> > +\tcase NODE_KIND_GITLINK:\n> > +\t\tprintf(\"file\\n\");\n> > +\t\tbreak;\n> > +\tcase NODE_KIND_SUBDIR:\n> > +\t\tdie(\"Unsupported: subdirectory\");\n> > +\tdefault:\n> > +\t\tbreak;\n> > +\t}\n> \n> Hmph, is it expected that the repertoire of node-kind stay the same, or\n> will it be extended over time? It might make sense to change the above\n> switch a more table-driven one. The same for node-action that follows\n> this part of the code.\n\nI needed more than a lookup table, because I wanted the flexibility to\nexecute arbitrary statements in each case. It's extended in the latest\nversion (will come up on the list soon).\n\n> > diff --git a/vcs-svn/svnload.c b/vcs-svn/svnload.c\n> > new file mode 100644\n> > index 0000000..7043ae7\n> > --- /dev/null\n> > +++ b/vcs-svn/svnload.c\n> > @@ -0,0 +1,294 @@\n> > ...\n> > +static struct strbuf blobs[100];\n> > +\t\n> \n> Is there a rationale for \"100\", or is it just a random number that can be\n> tweakable at the source level? Would we want to have a symbolic constant\n> for it?\n> \n> The \"blank\" line has a trailing whitespace.\n\nFixed. I used this to illustrate the problem with persisting blobs- in\nthis version, I persist 100 blobs; this is totally arbitrary and\ninextensible. Since the latest version leverages the --inline-blobs\nfeature of fast export, this is totally unecessary.\n\n> > +static struct {\n> > +\tunsigned long prop_len, text_len, copyfrom_rev, mark;\n> > +\tint text_delta, prop_delta; /* Boolean */\n> > ...\n> > +static void reset_node_ctx(void)\n> > ...\n> > +\tnode_ctx.text_delta = -1;\n> > +\tnode_ctx.prop_delta = -1;\n> \n> Does your \"Boolean\" type take values 0 or -1? Or is it perhaps a tristate\n> false=0, true=1, unknown=-1? If so, the comment on the type above needs\n> to be fixed.\n\nFixed. Thanks for pointing this out.\n\n> > +\tstrbuf_reset(&node_ctx.copyfrom_path);\n> > +\tstrbuf_reset(&node_ctx.path);\n> > +}\n> > +\n> > +static void populate_props(struct strbuf *props, const char *author,\n> > +\t\t\tconst char *log, const char *date) {\n> \n> Style on \"{\" (look everywhere).\n\nFixed.\n\n> > +\tstrbuf_reset(props);\t\n> \n> Trailing whitespace.\n\nFixed.\n\n> > ...\n> > +\tt ++;\n> \n> Unwanted SP before \"++\" (look everywhere).\n\nFixed.\n\n> > +\ttm_time = time_to_tm(strtoul(t, NULL, 10), atoi(tz_off));\n> \n> Has your caller already verified tz_off is a reasonable integer?\n\nFixed.\n\n> > ...\n> > +\t\t\tif (!memcmp(t, \"D\", 1)) {\n> > +\t\t\t\tnode_ctx.action = NODE_ACTION_DELETE;\n> > +\t\t\t}\n> > +\t\t\telse if (!memcmp(t, \"C\", 1)) {\n> \n> Style: \"} else if (...) {\".\n\nFixed.\n\n> > ...\n> > +\t\t\t\t\tnode_ctx.kind = NODE_KIND_GITLINK;\n> > +\t\t\t\telse if (!memcmp(val, \"040000\", 6))\n> \n> Is guaranteed to be a 6-digit octal, padded with 0 to the left?\n> \n> The manual page of git-fast-import seems to hint that is the case, but I\n> am double checking, as the leading zero is an error in the tree object.\n\nThe documentation and commit messages seem to hint that this is the\ncase. I'm digging to see if there's anything wrong with this.\n\n> > +\t\t\t\t\tnode_ctx.kind = NODE_KIND_SUBDIR;\n> > +\t\t\t\telse {\n> > +\t\t\t\t\tif (!memcmp(val, \"755\", 3))\n> > +\t\t\t\t\t\tnode_ctx.kind = NODE_KIND_EXECUTABLE;\n> > +\t\t\t\t\telse if(!memcmp(val, \"644\", 3))\n> \n> Style; missing SP after \"if/switch\" (look everywhere).\n\nFixed.\n\n> > +\t\t\t\t\t\tnode_ctx.kind = NODE_KIND_NORMAL;\n> > +\t\t\t\t\telse\n> > +\t\t\t\t\t\tdie(\"Unrecognized mode: %s\", val);\n> > +\t\t\t\t\tmode_incr = 4;\n> > +\t\t\t\t}\n> > +\t\t\t\tval += mode_incr;\n> \n> Hmm, this whole thing is ugly.\n> \n> Perhaps a table-lookup, or at least a helper function that translates\n> \"val\" to node-kind (while advancing val as the side effect) may make it\n> easier to read?\n\nFixed. I used a helper function.\n\n> > ...\n> > +\t\t\t\t\tto_dump = &blobs[strtoul(val + 1, NULL, 10)];\n> \n> Has anybody so far verified the decimal number at (val+1) is a reasonable\n> value, or am I seeing a future CVE here?\n\nFixed.\n\n> Do we care what follows the len of digits on this line? Does a line in G-F-I\n> stream allow trailing garbage (this question is not limited to this part\n> of the code)?\n\nHm, I haven't thought about this hard enough. Currently, both svn-fi\nand svn-fe ignore any trailing garbage/ commands they don't\nunderstand. Are there some issues that we haven't anticipated?\n\n> > +int svnload_init(const char *filename)\n> > ...\n> > +void svnload_deinit(void)\n>\n> Whoever needs to add an element to rev_ctx must consistently touch reset,\n> init and deinit. Would it help him/her if you moved the code so that\n> these functions are close together from the beginning?\n\nFixed.\n\n-- Ram\n"},{"id":"159789","messageId":"20110122191801.GB13023@burratino","threadId":"26304","inReplyTo":"20110122092416.GA7827@kytes","subject":"Re: [PATCH 4/5] fast-export: Introduce --inline-blobs","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-01-22T19:18:01Z","receivedAt":"2011-01-22T19:18:01Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> Agreed. I wouldn't like to introduce an extra dependency either. I was\n> talking about using it for prototyping- if the final version includes\n> an extra dependency, it's unlikely to get merged into git.git :) The\n> final design will probably use an in-memory B+ tree, but I haven't\n> thought about that hard enough.\n\nImmediate reaction:\n\nPlease no. There is a value to simplicity.\n\nAs you mention, the final form is a way off, so as long as people are\ncareful not to get locked into bad implementation decisions, I think\nit is okay. I have refrained from nitpicking the implementation so\nfar because the design and interface are not obvious yet.\n\nIn this particular case, I am dreaming that we will discover a hidden\n\"mkdir -p\" node-action in the dumpfile format so the list of\ndirectories will not be needed. ;-)\n\nRe Junio's critique:\n\nis it possible to use\n\n1) a table with callbacks? See the source code to unifdef for\n inspiration.\n\n2) separate code paths for different input states? fast-import.c\n does this.\n\n3) separate \"parsing\" and \"acting\" code? That can open the door to a\n little paralellism, though not necessarily enough to matter.\n\n\tparser actor\n\n\tread a chunk\n\tdetermine the first \"thing to do\"\n\t pick up the first \"thing to do\"\n\t do it\n\tdetermine the next \"thing to do\"\n\t pick up the next \"thing to do\"\n\t do it\n\n There is potential parallelism because the parser can keep\n chugging along if the actor is blocked, say, writing its\n output to the network.\n\n Syntactically: the actor function (write_dump) calls the parser\n function (next_command) to ask what to do next. If wanted, a later\n refactoring could make that parser function just grab an action off\n of a queue, while the parser proper runs in the background.\n \n And of course if the \"thing to do\" data structure is simple enough,\n this can also make the code easier to read.\n\nI mention these ideas because some of them (especially #2) could make\nthe prototyping a lot easier as well as resulting in code that is\neasier to review.\n\nRe error handling:\n\nWriting robust code (e.g., checking for errors) is also a lot easier\nwhen done from the start. The svn-fe error handling is known to be a\nproblem (see BUGS in contrib/svn-fe/svn-fe.txt). So yes, I also\nconsider avoiding segfaults and deadlocks and catching parse errors to\nbe worthwhile things.\n\nThanks.\nJonathan\n"}]}