{"thread":{"id":"25776","subject":"[PATCH 0/2] svn-fe: recognize v3 dumps","startedAt":"2010-11-18T05:00:23Z","lastAt":"2011-03-06T12:29:42Z","messageCount":40,"participants":["Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"156078","messageId":"20101118050023.GA14861@burratino","threadId":"25776","inReplyTo":null,"subject":"[PATCH 0/2] svn-fe: recognize v3 dumps","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-18T05:00:23Z","receivedAt":"2010-11-18T05:00:23Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"This mini-series is part of an effort to get David and Ram's svn\nimport work integrated into mainline git[1]. It fixes a longstanding\nsvn-fe bug that bit me a few times: when fed v3 (= --deltas) dumps,\ncurrent svn-fe produces nonsense --- a directory full of svn-format\ndeltas --- rather than erroring out. For example:\n\n\t; git init\n\t; svnrdump dump -r1:10 http://cvs2svn.tigris.org/svn/cvs2svn |\n\t\tsvn-fe |\n\t\tgit fast-import\n\t; git show HEAD:trunk/README\n\tSVN^@^@^@M-^N^D^CM-^N^DM-^@M-^N^DThis is a program to convert a CVS repository to Subversion.\n\nAfter this series, svn-fe would instead die with the message \"fatal:\ntext and property deltas not supported\".\n\nRam and David: this code will probably look familiar. It is roughly\nbased on David's \"vcs-svn: Extend svndump to parse version 3 format\"\nfrom about a month ago[2]. I would be particularly interested in\nhearing whether this still seems sensible in design and execution,\nso it can be deployed to a wider audience with your blessing if\nappropriate.\n\nThoughts? Improvements?\n\nDavid Barr (1):\n vcs-svn: Allow simple v3 dumps (no deltas yet)\n\nJonathan Nieder (1):\n vcs-svn: Error out for v3 dumps\n\n t/t9010-svn-fe.sh | 403 ++++++++++++++++++++++++++++++++++++++++++++++++++---\n vcs-svn/svndump.c | 28 ++++-\n 2 files changed, 406 insertions(+), 25 deletions(-)\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/160875\n\nThe dumpfile v3 code proper has been proving more trouble to review\nthan anticipated. Mostly because it involves tracking down details of\nthe svn repository model that aren't clearly documented in any obvious\nplace:\n\n * What do the five node actions (replace, delete, add, modify,\n unknown) represent? How can we deal with them without repeating\n the same code four times?\n\n * Which actions preserve properties? Which preserve text?\n\n[2] http://thread.gmane.org/gmane.comp.version-control.git/159117/focus=159114\n"},{"id":"156079","messageId":"20101118050248.GB14861@burratino","threadId":"25776","inReplyTo":"20101118050023.GA14861@burratino","subject":"[PATCH 1/2] vcs-svn: Error out for v3 dumps","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-18T05:02:48Z","receivedAt":"2010-11-18T05:02:48Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"By ignoring the Text-Delta and Prop-Delta node fields, current svn-fe\nhappily mistakes deltas for full text and instead of cleanly erroring\nout, it produces a valid but semantically bogus fast-import stream\nwhen fed a dump file in the modern \"svnadmin dump --deltas\" format.\n\nDump file parsers are supposed to ignore header fields they don't\nunderstand (to allow for backward-compatible extensions), but they are\nalso supposed to check the SVN-fs-dump-format-version header to\nprevent misinterpretation of non backward-compatible extensions.\nDo so.\n\nSigned-off-by: Jonathan Nieder \n---\n t/t9010-svn-fe.sh | 59 +++++++++++++++++++++++++++++++++-------------------\n vcs-svn/svndump.c | 13 +++++++++--\n 2 files changed, 47 insertions(+), 25 deletions(-)\n\ndiff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh\nindex a713dfc..e9e46ea 100755\n--- a/t/t9010-svn-fe.sh\n+++ b/t/t9010-svn-fe.sh\n@@ -4,29 +4,44 @@ test_description='check svn dumpfile importer'\n \n . ./lib-git-svn.sh\n \n-test_dump() {\n-\tlabel=$1\n-\tdump=$2\n-\ttest_expect_success \"$dump\" '\n-\t\tsvnadmin create \"$label-svn\" &&\n-\t\tsvnadmin load \"$label-svn\" < \"$TEST_DIRECTORY/$dump\" &&\n-\t\tsvn_cmd export \"file://$PWD/$label-svn\" \"$label-svnco\" &&\n-\t\tgit init \"$label-git\" &&\n-\t\ttest-svn-fe \"$TEST_DIRECTORY/$dump\" >\"$label.fe\" &&\n-\t\t(\n-\t\t\tcd \"$label-git\" &&\n-\t\t\tgit fast-import < ../\"$label.fe\"\n-\t\t) &&\n-\t\t(\n-\t\t\tcd \"$label-svnco\" &&\n-\t\t\tgit init &&\n-\t\t\tgit add . &&\n-\t\t\tgit fetch \"../$label-git\" master &&\n-\t\t\tgit diff --exit-code FETCH_HEAD\n-\t\t)\n-\t'\n+reinit_git () {\n+\trm -fr .git &&\n+\tgit init\n }\n \n-test_dump simple t9135/svn.dump\n+>empty\n+\n+test_expect_success 'empty dump' '\n+\treinit_git &&\n+\techo \"SVN-fs-dump-format-version: 2\" >input &&\n+\ttest-svn-fe input >stream &&\n+\tgit fast-import input &&\n+\ttest_must_fail test-svn-fe input >stream &&\n+\ttest_cmp empty stream\n+'\n+\n+test_expect_success 't9135/svn.dump' '\n+\tsvnadmin create simple-svn &&\n+\tsvnadmin load simple-svn <\"$TEST_DIRECTORY/t9135/svn.dump\" &&\n+\tsvn_cmd export \"file://$PWD/simple-svn\" simple-svnco &&\n+\tgit init simple-git &&\n+\ttest-svn-fe \"$TEST_DIRECTORY/t9135/svn.dump\" >simple.fe &&\n+\t(\n+\t\tcd simple-git &&\n+\t\tgit fast-import <../simple.fe\n+\t) &&\n+\t(\n+\t\tcd simple-svnco &&\n+\t\tgit init &&\n+\t\tgit add . &&\n+\t\tgit fetch ../simple-git master &&\n+\t\tgit diff --exit-code FETCH_HEAD\n+\t)\n+'\n \n test_done\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex 53d0215..fa580e6 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -51,14 +51,14 @@ static struct {\n } rev_ctx;\n \n static struct {\n-\tuint32_t uuid, url;\n+\tuint32_t version, uuid, url;\n } dump_ctx;\n \n static struct {\n \tuint32_t svn_log, svn_author, svn_date, svn_executable, svn_special, uuid,\n \t\trevision_number, node_path, node_kind, node_action,\n \t\tnode_copyfrom_path, node_copyfrom_rev, text_content_length,\n-\t\tprop_content_length, content_length;\n+\t\tprop_content_length, content_length, svn_fs_dump_format_version;\n } keys;\n \n static void reset_node_ctx(char *fname)\n@@ -85,6 +85,7 @@ static void reset_rev_ctx(uint32_t revision)\n static void reset_dump_ctx(uint32_t url)\n {\n \tdump_ctx.url = url;\n+\tdump_ctx.version = 1;\n \tdump_ctx.uuid = ~0;\n }\n \n@@ -105,6 +106,7 @@ static void init_keys(void)\n \tkeys.text_content_length = pool_intern(\"Text-content-length\");\n \tkeys.prop_content_length = pool_intern(\"Prop-content-length\");\n \tkeys.content_length = pool_intern(\"Content-length\");\n+\tkeys.svn_fs_dump_format_version = pool_intern(\"SVN-fs-dump-format-version\");\n }\n \n static void read_props(void)\n@@ -206,7 +208,12 @@ void svndump_read(const char *url)\n \t\t*val++ = '\\0';\n \t\tkey = pool_intern(t);\n \n-\t\tif (key == keys.uuid) {\n+\t\tif (key == keys.svn_fs_dump_format_version) {\n+\t\t\tdump_ctx.version = atoi(val);\n+\t\t\tif (dump_ctx.version > 2)\n+\t\t\t\tdie(\"expected svn dump format version <= 2, found %d\",\n+\t\t\t\t dump_ctx.version);\n+\t\t} else if (key == keys.uuid) {\n \t\t\tdump_ctx.uuid = pool_intern(val);\n \t\t} else if (key == keys.revision_number) {\n \t\t\tif (active_ctx == NODE_CTX)\n-- \n1.7.2.3.551.g13682.dirty\n"},{"id":"156080","messageId":"20101118050351.GC14861@burratino","threadId":"25776","inReplyTo":"20101118050023.GA14861@burratino","subject":"[PATCH 2/2] vcs-svn: Allow simple v3 dumps (no deltas yet)","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-18T05:03:51Z","receivedAt":"2010-11-18T05:03:51Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: David Barr \n\nSince the dumpfile version 1 days, the Subversion dump format\ngained some new fields:\n\n - a unique identifier for the repository (version 2 format)\n - whether the text and properties for a node should be\n interpreted as deltas\n - checksums for a delta's preimage\n - SHA-1 sums as alternatives to the existing MD5 checksums for\n copy source and the payload (delta).\n\nFor now what is relevant to us is the Text-delta and Prop-delta\nfields, since not noticing these causes a dump file to be\nmisinterpreted (see the previous commit).\n\n[jn: with tests]\n\nSigned-off-by: David Barr \nSigned-off-by: Jonathan Nieder \n---\n t/t9010-svn-fe.sh | 350 ++++++++++++++++++++++++++++++++++++++++++++++++++++-\n vcs-svn/svndump.c | 21 +++-\n 2 files changed, 365 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh\nindex e9e46ea..be5372a 100755\n--- a/t/t9010-svn-fe.sh\n+++ b/t/t9010-svn-fe.sh\n@@ -9,6 +9,30 @@ reinit_git () {\n \tgit init\n }\n \n+properties () {\n+\twhile test \"$#\" -ne 0\n+\tdo\n+\t\tproperty=\"$1\" &&\n+\t\tvalue=\"$2\" &&\n+\t\tprintf \"%s\\n\" \"K ${#property}\" &&\n+\t\tprintf \"%s\\n\" \"$property\" &&\n+\t\tprintf \"%s\\n\" \"V ${#value}\" &&\n+\t\tprintf \"%s\\n\" \"$value\" &&\n+\t\tshift 2 ||\n+\t\treturn 1\n+\tdone\n+}\n+\n+text_no_props () {\n+\ttext=\"$1\n+\" &&\n+\tprintf \"%s\\n\" \"Prop-content-length: 10\" &&\n+\tprintf \"%s\\n\" \"Text-content-length: ${#text}\" &&\n+\tprintf \"%s\\n\" \"Content-length: $((${#text} + 10))\" &&\n+\tprintf \"%s\\n\" \"\" \"PROPS-END\" &&\n+\tprintf \"%s\\n\" \"$text\"\n+}\n+\n >empty\n \n test_expect_success 'empty dump' '\n@@ -18,13 +42,333 @@ test_expect_success 'empty dump' '\n \tgit fast-import input &&\n-\ttest_must_fail test-svn-fe input >stream &&\n+\techo \"SVN-fs-dump-format-version: 4\" >v4.dump &&\n+\ttest_must_fail test-svn-fe v4.dump >stream &&\n \ttest_cmp empty stream\n '\n \n+test_expect_failure 'empty revision' '\n+\treinit_git &&\n+\tprintf \"rev : %s\\n\" \"\" \"\" >expect &&\n+\tcat >emptyrev.dump <<-\\EOF &&\n+\tSVN-fs-dump-format-version: 3\n+\n+\tRevision-number: 1\n+\tProp-content-length: 0\n+\tContent-length: 0\n+\n+\tRevision-number: 2\n+\tProp-content-length: 0\n+\tContent-length: 0\n+\n+\tEOF\n+\ttest-svn-fe emptyrev.dump >stream &&\n+\tgit fast-import : %s\" HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'empty properties' '\n+\treinit_git &&\n+\tprintf \"rev : %s\\n\" \"\" \"\" >expect &&\n+\tcat >emptyprop.dump <<-\\EOF &&\n+\tSVN-fs-dump-format-version: 3\n+\n+\tRevision-number: 1\n+\tProp-content-length: 10\n+\tContent-length: 10\n+\n+\tPROPS-END\n+\n+\tRevision-number: 2\n+\tProp-content-length: 10\n+\tContent-length: 10\n+\n+\tPROPS-END\n+\tEOF\n+\ttest-svn-fe emptyprop.dump >stream &&\n+\tgit fast-import : %s\" HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'author name and commit message' '\n+\treinit_git &&\n+\techo \"\" >expect.author &&\n+\tcat >message <<-\\EOF &&\n+\tA concise summary of the change\n+\n+\tA detailed description of the change, why it is needed, what\n+\twas broken and why applying this is the best course of action.\n+\n+\t* file.c\n+\t Details pertaining to an individual file.\n+\tEOF\n+\t{\n+\t\tproperties \\\n+\t\t\tsvn:author author@example.com \\\n+\t\t\tsvn:log \"$(cat message)\" &&\n+\t\techo PROPS-END\n+\t} >props &&\n+\t{\n+\t\techo \"SVN-fs-dump-format-version: 3\" &&\n+\t\techo &&\n+\t\techo \"Revision-number: 1\" &&\n+\t\techo Prop-content-length: $(wc -c log.dump &&\n+\ttest-svn-fe log.dump >stream &&\n+\tgit fast-import actual.log &&\n+\tgit log --format=\"<%an, %ae>\" >actual.author &&\n+\ttest_cmp message actual.log &&\n+\ttest_cmp expect.author actual.author\n+'\n+\n+test_expect_success 'unsupported properties are ignored' '\n+\treinit_git &&\n+\techo author >expect &&\n+\tcat >extraprop.dump <<-\\EOF &&\n+\tSVN-fs-dump-format-version: 3\n+\n+\tRevision-number: 1\n+\tProp-content-length: 56\n+\tContent-length: 56\n+\n+\tK 8\n+\tnonsense\n+\tV 1\n+\ty\n+\tK 10\n+\tsvn:author\n+\tV 6\n+\tauthor\n+\tPROPS-END\n+\tEOF\n+\ttest-svn-fe extraprop.dump >stream &&\n+\tgit fast-import actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_failure 'timestamp and empty file' '\n+\techo author@example.com >expect.author &&\n+\techo 1999-01-01 >expect.date &&\n+\techo file >expect.files &&\n+\treinit_git &&\n+\t{\n+\t\tproperties \\\n+\t\t\tsvn:author author@example.com \\\n+\t\t\tsvn:date \"1999-01-01T00:01:002.000000Z\" \\\n+\t\t\tsvn:log \"add empty file\" &&\n+\t\techo PROPS-END\n+\t} >props &&\n+\t{\n+\t\tcat <<-EOF &&\n+\t\tSVN-fs-dump-format-version: 3\n+\n+\t\tRevision-number: 1\n+\t\tEOF\n+\t\techo Prop-content-length: $(wc -c emptyfile.dump &&\n+\ttest-svn-fe emptyfile.dump >stream &&\n+\tgit fast-import actual.author &&\n+\tgit log --date=short --format=%ad HEAD >actual.date &&\n+\tgit ls-tree -r --name-only HEAD >actual.files &&\n+\ttest_cmp expect.author actual.author &&\n+\ttest_cmp expect.date actual.date &&\n+\ttest_cmp expect.files actual.files &&\n+\tgit checkout HEAD empty-file &&\n+\ttest_cmp empty file\n+'\n+\n+test_expect_success 'directory with files' '\n+\treinit_git &&\n+\tprintf \"%s\\n\" directory/file1 directory/file2 >expect.files &&\n+\techo hi >hi &&\n+\techo hello >hello &&\n+\t{\n+\t\tproperties \\\n+\t\t\tsvn:author author@example.com \\\n+\t\t\tsvn:date \"1999-02-01T00:01:002.000000Z\" \\\n+\t\t\tsvn:log \"add directory with some files in it\" &&\n+\t\techo PROPS-END\n+\t} >props &&\n+\t{\n+\t\tcat <<-EOF &&\n+\t\tSVN-fs-dump-format-version: 3\n+\n+\t\tRevision-number: 1\n+\t\tEOF\n+\t\techo Prop-content-length: $(wc -c directory.dump &&\n+\ttest-svn-fe directory.dump >stream &&\n+\tgit fast-import actual.files &&\n+\tgit checkout HEAD directory &&\n+\ttest_cmp expect.files actual.files &&\n+\ttest_cmp hello directory/file1 &&\n+\ttest_cmp hi directory/file2\n+'\n+\n+test_expect_success 'deltas not supported' '\n+\t{\n+\t\t# (old) h + (inline) ello + (old) \\n\n+\t\tprintf \"SVNQ%b%b%s\" \"Q\\003\\006\\005\\004\" \"\\001Q\\0204\\001\\002\" \"ello\" |\n+\t\tq_to_nul\n+\t} >delta &&\n+\t{\n+\t\tproperties \\\n+\t\t\tsvn:author author@example.com \\\n+\t\t\tsvn:date \"1999-01-05T00:01:002.000000Z\" \\\n+\t\t\tsvn:log \"add greeting\" &&\n+\t\techo PROPS-END\n+\t} >props &&\n+\t{\n+\t\tproperties \\\n+\t\t\tsvn:author author@example.com \\\n+\t\t\tsvn:date \"1999-01-06T00:01:002.000000Z\" \\\n+\t\t\tsvn:log \"change it\" &&\n+\t\techo PROPS-END\n+\t} >props2 &&\n+\t{\n+\t\techo SVN-fs-dump-format-version: 3 &&\n+\t\techo &&\n+\t\techo Revision-number: 1 &&\n+\t\techo Prop-content-length: $(wc -c delta.dump &&\n+\ttest_must_fail test-svn-fe delta.dump\n+'\n+\n+test_expect_success 'property deltas not supported' '\n+\t{\n+\t\tproperties \\\n+\t\t\tsvn:author author@example.com \\\n+\t\t\tsvn:date \"1999-03-06T00:01:002.000000Z\" \\\n+\t\t\tsvn:log \"make an executable, or chmod -x it\" &&\n+\t\techo PROPS-END\n+\t} >revprops &&\n+\t{\n+\t\techo SVN-fs-dump-format-version: 3 &&\n+\t\techo &&\n+\t\techo Revision-number: 1 &&\n+\t\techo Prop-content-length: $(wc -c propdelta.dump &&\n+\ttest_must_fail test-svn-fe propdelta.dump\n+'\n+\n test_expect_success 't9135/svn.dump' '\n \tsvnadmin create simple-svn &&\n \tsvnadmin load simple-svn <\"$TEST_DIRECTORY/t9135/svn.dump\" &&\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex fa580e6..6b64c1b 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -42,6 +42,7 @@ static char* log_copy(uint32_t length, char *log)\n static struct {\n \tuint32_t action, propLength, textLength, srcRev, srcMode, mark, type;\n \tuint32_t src[REPO_MAX_PATH_DEPTH], dst[REPO_MAX_PATH_DEPTH];\n+\tuint32_t text_delta, prop_delta;\n } node_ctx;\n \n static struct {\n@@ -58,7 +59,9 @@ static struct {\n \tuint32_t svn_log, svn_author, svn_date, svn_executable, svn_special, uuid,\n \t\trevision_number, node_path, node_kind, node_action,\n \t\tnode_copyfrom_path, node_copyfrom_rev, text_content_length,\n-\t\tprop_content_length, content_length, svn_fs_dump_format_version;\n+\t\tprop_content_length, content_length, svn_fs_dump_format_version,\n+\t\t/* version 3 format */\n+\t\ttext_delta, prop_delta;\n } keys;\n \n static void reset_node_ctx(char *fname)\n@@ -72,6 +75,8 @@ static void reset_node_ctx(char *fname)\n \tnode_ctx.srcMode = 0;\n \tpool_tok_seq(REPO_MAX_PATH_DEPTH, node_ctx.dst, \"/\", fname);\n \tnode_ctx.mark = 0;\n+\tnode_ctx.text_delta = 0;\n+\tnode_ctx.prop_delta = 0;\n }\n \n static void reset_rev_ctx(uint32_t revision)\n@@ -107,6 +112,9 @@ static void init_keys(void)\n \tkeys.prop_content_length = pool_intern(\"Prop-content-length\");\n \tkeys.content_length = pool_intern(\"Content-length\");\n \tkeys.svn_fs_dump_format_version = pool_intern(\"SVN-fs-dump-format-version\");\n+\t/* version 3 format (Subversion 1.1.0) */\n+\tkeys.text_delta = pool_intern(\"Text-delta\");\n+\tkeys.prop_delta = pool_intern(\"Prop-delta\");\n }\n \n static void read_props(void)\n@@ -144,6 +152,9 @@ static void read_props(void)\n \n static void handle_node(void)\n {\n+\tif (node_ctx.text_delta || node_ctx.prop_delta)\n+\t\tdie(\"text and property deltas not supported\");\n+\n \tif (node_ctx.propLength != LENGTH_UNKNOWN && node_ctx.propLength)\n \t\tread_props();\n \n@@ -210,8 +221,8 @@ void svndump_read(const char *url)\n \n \t\tif (key == keys.svn_fs_dump_format_version) {\n \t\t\tdump_ctx.version = atoi(val);\n-\t\t\tif (dump_ctx.version > 2)\n-\t\t\t\tdie(\"expected svn dump format version <= 2, found %d\",\n+\t\t\tif (dump_ctx.version > 3)\n+\t\t\t\tdie(\"expected svn dump format version <= 3, found %d\",\n \t\t\t\t dump_ctx.version);\n \t\t} else if (key == keys.uuid) {\n \t\t\tdump_ctx.uuid = pool_intern(val);\n@@ -255,6 +266,10 @@ void svndump_read(const char *url)\n \t\t\tnode_ctx.textLength = atoi(val);\n \t\t} else if (key == keys.prop_content_length) {\n \t\t\tnode_ctx.propLength = atoi(val);\n+\t\t} else if (key == keys.text_delta) {\n+\t\t\tnode_ctx.text_delta = !strcmp(val, \"true\");\n+\t\t} else if (key == keys.prop_delta) {\n+\t\t\tnode_ctx.prop_delta = !strcmp(val, \"true\");\n \t\t} else if (key == keys.content_length) {\n \t\t\tlen = atoi(val);\n \t\t\tbuffer_read_line();\n-- \n1.7.2.3.551.g13682.dirty\n"},{"id":"156225","messageId":"20101120004525.GA17445@burratino","threadId":"25776","inReplyTo":"20101118050023.GA14861@burratino","subject":"[RFC/PATCH 0/15] svn-fe: support for property deltas (but not text deltas yet)","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-20T00:45:25Z","receivedAt":"2010-11-20T00:45:25Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nJonathan Nieder wrote:\n\n> This mini-series is part of an effort to get David and Ram's svn\n> import work integrated into mainline git[1].\n\nAnother chapter. This one is mostly David's work. One can test\nit (I haven't tried) by taking out the safety\n\n\tdie(\"text deltas not supported\");\n\nand letting it import from some real-world deltified dump file,\ntreating deltas as the full text. Then one can validate as usual, by\nchecking the modes of files in each revision of the imported tree.\n\nI've done a little refactoring to more easily understand the patch,\nbut the heart of the work is still David's. Any bugs and infelicities\nare probably mine (especially the evil hack to support switching\nbetween executables and symlinks).\n\nEspecially useful would be a nicer set of tests. A tool to create\ndump files based on a more concise description would make this and\nsome future tasks a lot easier (hint, hint).\n\nThoughts? Improvements? Any of these ready to go? I can rearrange\nto put the uncontroversial patches at the beginning once it is clear\nwhat those are.\n\nDavid Barr (1):\n vcs-svn: Implement Prop-delta handling\n\nJonathan Nieder (14):\n vcs-svn: Check for errors from open()\n vcs-svn: Eliminate node_ctx.srcRev global\n vcs-svn: Eliminate node_ctx.mark global\n vcs-svn: Unclutter handle_node by introducing have_props var\n vcs-svn: Use mark to indicate nodes with text\n vcs-svn: handle_node: Handle deletion case early\n vcs-svn: Replace = Delete + Add\n vcs-svn: Merge repo_replace and repo_modify functions\n vcs-svn: Delay read of per-path properties\n vcs-svn: Reject path nodes without Node-action\n vcs-svn: More dump format sanity checks\n vcs-svn: Make source easier to read on small screens\n vcs-svn: Split off function for handling of individual properties\n vcs-svn: Sharpen parsing of property lines\n\n contrib/svn-fe/svn-fe.c | 3 +-\n t/t9010-svn-fe.sh | 301 ++++++++++++++++++++++++++++++++++++++++++++++-\n test-svn-fe.c | 3 +-\n vcs-svn/repo_tree.c | 21 +--\n vcs-svn/repo_tree.h | 3 +-\n vcs-svn/svndump.c | 191 +++++++++++++++++++-----------\n vcs-svn/svndump.h | 2 +-\n 7 files changed, 436 insertions(+), 88 deletions(-)\n\n-- \n1.7.2.3\n"},{"id":"156226","messageId":"20101120004606.GB17445@burratino","threadId":"25776","inReplyTo":"20101120004525.GA17445@burratino","subject":"[PATCH 01/15] vcs-svn: Check for errors from open()","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-20T00:46:06Z","receivedAt":"2010-11-20T00:46:06Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"test-svn-fe segfaults when passed a bogus path. Simplify debugging by\nexiting with a meaningful error message instead.\n\nSigned-off-by: Jonathan Nieder \n---\nDoesn't belong in this series, just something I noticed while\ndebugging.\n\n contrib/svn-fe/svn-fe.c | 3 ++-\n test-svn-fe.c | 3 ++-\n vcs-svn/svndump.c | 6 ++++--\n vcs-svn/svndump.h | 2 +-\n 4 files changed, 9 insertions(+), 5 deletions(-)\n\ndiff --git a/contrib/svn-fe/svn-fe.c b/contrib/svn-fe/svn-fe.c\nindex a2677b0..35db24f 100644\n--- a/contrib/svn-fe/svn-fe.c\n+++ b/contrib/svn-fe/svn-fe.c\n@@ -8,7 +8,8 @@\n \n int main(int argc, char **argv)\n {\n-\tsvndump_init(NULL);\n+\tif (svndump_init(NULL))\n+\t\treturn 1;\n \tsvndump_read((argc > 1) ? argv[1] : NULL);\n \tsvndump_deinit();\n \tsvndump_reset();\ndiff --git a/test-svn-fe.c b/test-svn-fe.c\nindex 77cf78a..b42ba78 100644\n--- a/test-svn-fe.c\n+++ b/test-svn-fe.c\n@@ -9,7 +9,8 @@ int main(int argc, char *argv[])\n {\n \tif (argc != 2)\n \t\tusage(\"test-svn-fe \");\n-\tsvndump_init(argv[1]);\n+\tif (svndump_init(argv[1]))\n+\t\treturn 1;\n \tsvndump_read(NULL);\n \tsvndump_deinit();\n \tsvndump_reset();\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex 6b64c1b..db11851 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -290,14 +290,16 @@ void svndump_read(const char *url)\n \t\thandle_revision();\n }\n \n-void svndump_init(const char *filename)\n+int svndump_init(const char *filename)\n {\n-\tbuffer_init(filename);\n+\tif (buffer_init(filename))\n+\t\treturn error(\"cannot open %s: %s\", filename, strerror(errno));\n \trepo_init();\n \treset_dump_ctx(~0);\n \treset_rev_ctx(0);\n \treset_node_ctx(NULL);\n \tinit_keys();\n+\treturn 0;\n }\n \n void svndump_deinit(void)\ndiff --git a/vcs-svn/svndump.h b/vcs-svn/svndump.h\nindex 93c412f..df9ceb0 100644\n--- a/vcs-svn/svndump.h\n+++ b/vcs-svn/svndump.h\n@@ -1,7 +1,7 @@\n #ifndef SVNDUMP_H_\n #define SVNDUMP_H_\n \n-void svndump_init(const char *filename);\n+int svndump_init(const char *filename);\n void svndump_read(const char *url);\n void svndump_deinit(void);\n void svndump_reset(void);\n-- \n1.7.2.3\n"},{"id":"156227","messageId":"20101120004622.GC17445@burratino","threadId":"25776","inReplyTo":"20101120004525.GA17445@burratino","subject":"[PATCH 02/15] vcs-svn: Eliminate node_ctx.srcRev global","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-20T00:46:22Z","receivedAt":"2010-11-20T00:46:22Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The srcRev variable is only used in handle_node(); its purpose\nis to hold the old mode for a path, to only be used if properties\nare not being changed. Narrow its scope to make its meaningful\nlifetime more obvious.\n\nNo functional change intended. Add some tests as a sanity-check\nfor the simplest case (no renames).\n\nSigned-off-by: Jonathan Nieder \n---\n t/t9010-svn-fe.sh | 158 +++++++++++++++++++++++++++++++++++++++++++++++++++++\n vcs-svn/svndump.c | 15 +++--\n 2 files changed, 166 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh\nindex be5372a..729e73d 100755\n--- a/t/t9010-svn-fe.sh\n+++ b/t/t9010-svn-fe.sh\n@@ -252,6 +252,164 @@ test_expect_success 'directory with files' '\n \ttest_cmp hi directory/file2\n '\n \n+test_expect_failure 'change file mode but keep old content' '\n+\treinit_git &&\n+\tcat >expect <<-\\EOF &&\n+\tOBJID\n+\t:120000 100644 OBJID OBJID T\tgreeting\n+\tOBJID\n+\t:100644 120000 OBJID OBJID T\tgreeting\n+\tOBJID\n+\t:000000 100644 OBJID OBJID A\tgreeting\n+\tEOF\n+\techo \"link hello\" >expect.blob &&\n+\techo hello >hello &&\n+\tcat >filemode.dump <<-\\EOF &&\n+\tSVN-fs-dump-format-version: 3\n+\n+\tRevision-number: 1\n+\tProp-content-length: 10\n+\tContent-length: 10\n+\n+\tPROPS-END\n+\n+\tNode-path: greeting\n+\tNode-kind: file\n+\tNode-action: add\n+\tProp-content-length: 10\n+\tText-content-length: 11\n+\tContent-length: 21\n+\n+\tPROPS-END\n+\tlink hello\n+\n+\tRevision-number: 2\n+\tProp-content-length: 10\n+\tContent-length: 10\n+\n+\tPROPS-END\n+\n+\tNode-path: greeting\n+\tNode-kind: file\n+\tNode-action: change\n+\tProp-content-length: 33\n+\tContent-length: 33\n+\n+\tK 11\n+\tsvn:special\n+\tV 1\n+\t*\n+\tPROPS-END\n+\n+\tRevision-number: 3\n+\tProp-content-length: 10\n+\tContent-length: 10\n+\n+\tPROPS-END\n+\n+\tNode-path: greeting\n+\tNode-kind: file\n+\tNode-action: change\n+\tProp-content-length: 10\n+\tContent-length: 10\n+\n+\tPROPS-END\n+\tEOF\n+\ttest-svn-fe filemode.dump >stream &&\n+\tgit fast-import actual &&\n+\tgit show HEAD:greeting >actual.blob &&\n+\tgit show HEAD^:greeting >actual.target &&\n+\ttest_cmp expect actual &&\n+\ttest_cmp expect.blob actual.blob &&\n+\ttest_cmp hello actual.target\n+'\n+\n+test_expect_success 'change file mode and reiterate content' '\n+\treinit_git &&\n+\tcat >expect <<-\\EOF &&\n+\tOBJID\n+\t:120000 100644 OBJID OBJID T\tgreeting\n+\tOBJID\n+\t:100644 120000 OBJID OBJID T\tgreeting\n+\tOBJID\n+\t:000000 100644 OBJID OBJID A\tgreeting\n+\tEOF\n+\techo \"link hello\" >expect.blob &&\n+\techo hello >hello &&\n+\tcat >filemode.dump <<-\\EOF &&\n+\tSVN-fs-dump-format-version: 3\n+\n+\tRevision-number: 1\n+\tProp-content-length: 10\n+\tContent-length: 10\n+\n+\tPROPS-END\n+\n+\tNode-path: greeting\n+\tNode-kind: file\n+\tNode-action: add\n+\tProp-content-length: 10\n+\tText-content-length: 11\n+\tContent-length: 21\n+\n+\tPROPS-END\n+\tlink hello\n+\n+\tRevision-number: 2\n+\tProp-content-length: 10\n+\tContent-length: 10\n+\n+\tPROPS-END\n+\n+\tNode-path: greeting\n+\tNode-kind: file\n+\tNode-action: change\n+\tProp-content-length: 33\n+\tText-content-length: 11\n+\tContent-length: 44\n+\n+\tK 11\n+\tsvn:special\n+\tV 1\n+\t*\n+\tPROPS-END\n+\tlink hello\n+\n+\tRevision-number: 3\n+\tProp-content-length: 10\n+\tContent-length: 10\n+\n+\tPROPS-END\n+\n+\tNode-path: greeting\n+\tNode-kind: file\n+\tNode-action: change\n+\tProp-content-length: 10\n+\tText-content-length: 11\n+\tContent-length: 21\n+\n+\tPROPS-END\n+\tlink hello\n+\tEOF\n+\ttest-svn-fe filemode.dump >stream &&\n+\tgit fast-import actual &&\n+\tgit show HEAD:greeting >actual.blob &&\n+\tgit show HEAD^:greeting >actual.target &&\n+\ttest_cmp expect actual &&\n+\ttest_cmp expect.blob actual.blob &&\n+\ttest_cmp hello actual.target\n+'\n+\n test_expect_success 'deltas not supported' '\n \t{\n \t\t# (old) h + (inline) ello + (old) \\n\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex db11851..65bd335 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -40,7 +40,7 @@ static char* log_copy(uint32_t length, char *log)\n }\n \n static struct {\n-\tuint32_t action, propLength, textLength, srcRev, srcMode, mark, type;\n+\tuint32_t action, propLength, textLength, srcRev, mark, type;\n \tuint32_t src[REPO_MAX_PATH_DEPTH], dst[REPO_MAX_PATH_DEPTH];\n \tuint32_t text_delta, prop_delta;\n } node_ctx;\n@@ -72,7 +72,6 @@ static void reset_node_ctx(char *fname)\n \tnode_ctx.textLength = LENGTH_UNKNOWN;\n \tnode_ctx.src[0] = ~0;\n \tnode_ctx.srcRev = 0;\n-\tnode_ctx.srcMode = 0;\n \tpool_tok_seq(REPO_MAX_PATH_DEPTH, node_ctx.dst, \"/\", fname);\n \tnode_ctx.mark = 0;\n \tnode_ctx.text_delta = 0;\n@@ -152,6 +151,8 @@ static void read_props(void)\n \n static void handle_node(void)\n {\n+\tuint32_t old_mode = 0;\n+\n \tif (node_ctx.text_delta || node_ctx.prop_delta)\n \t\tdie(\"text and property deltas not supported\");\n \n@@ -159,7 +160,7 @@ static void handle_node(void)\n \t\tread_props();\n \n \tif (node_ctx.srcRev)\n-\t\tnode_ctx.srcMode = repo_copy(node_ctx.srcRev, node_ctx.src, node_ctx.dst);\n+\t\told_mode = repo_copy(node_ctx.srcRev, node_ctx.src, node_ctx.dst);\n \n \tif (node_ctx.textLength != LENGTH_UNKNOWN &&\n \t node_ctx.type != REPO_MODE_DIR)\n@@ -175,19 +176,19 @@ static void handle_node(void)\n \t\telse if (node_ctx.propLength != LENGTH_UNKNOWN)\n \t\t\trepo_modify(node_ctx.dst, node_ctx.type, node_ctx.mark);\n \t\telse if (node_ctx.textLength != LENGTH_UNKNOWN)\n-\t\t\tnode_ctx.srcMode = repo_replace(node_ctx.dst, node_ctx.mark);\n+\t\t\told_mode = repo_replace(node_ctx.dst, node_ctx.mark);\n \t} else if (node_ctx.action == NODEACT_ADD) {\n \t\tif (node_ctx.srcRev && node_ctx.propLength != LENGTH_UNKNOWN)\n \t\t\trepo_modify(node_ctx.dst, node_ctx.type, node_ctx.mark);\n \t\telse if (node_ctx.srcRev && node_ctx.textLength != LENGTH_UNKNOWN)\n-\t\t\tnode_ctx.srcMode = repo_replace(node_ctx.dst, node_ctx.mark);\n+\t\t\told_mode = repo_replace(node_ctx.dst, node_ctx.mark);\n \t\telse if ((node_ctx.type == REPO_MODE_DIR && !node_ctx.srcRev) ||\n \t\t\t node_ctx.textLength != LENGTH_UNKNOWN)\n \t\t\trepo_add(node_ctx.dst, node_ctx.type, node_ctx.mark);\n \t}\n \n-\tif (node_ctx.propLength == LENGTH_UNKNOWN && node_ctx.srcMode)\n-\t\tnode_ctx.type = node_ctx.srcMode;\n+\tif (node_ctx.propLength == LENGTH_UNKNOWN && old_mode)\n+\t\tnode_ctx.type = old_mode;\n \n \tif (node_ctx.mark)\n \t\tfast_export_blob(node_ctx.type, node_ctx.mark, node_ctx.textLength);\n-- \n1.7.2.3\n"},{"id":"156228","messageId":"20101120004654.GD17445@burratino","threadId":"25776","inReplyTo":"20101120004525.GA17445@burratino","subject":"[PATCH 03/15] vcs-svn: Eliminate node_ctx.mark global","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-20T00:46:54Z","receivedAt":"2010-11-20T00:46:54Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The mark variable is only used in handle_node(). Its life is\nvery short and simple: first, a new mark number is allocated if\nthis node has text attached, then that mark is recorded in the\nin-core tree being built up, and lastly the mark is communicated\nto fast-import in the stream along with the associated text.\n\nA new reader may worry about interaction with other code, especially\nsince mark is not initialized to zero in handle_node() itself.\nDisperse such worries by making it local. No functional change\nintended.\n\nSigned-off-by: Jonathan Nieder \n---\n vcs-svn/svndump.c | 23 +++++++++++------------\n 1 files changed, 11 insertions(+), 12 deletions(-)\n\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex 65bd335..1fb7f82 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -40,7 +40,7 @@ static char* log_copy(uint32_t length, char *log)\n }\n \n static struct {\n-\tuint32_t action, propLength, textLength, srcRev, mark, type;\n+\tuint32_t action, propLength, textLength, srcRev, type;\n \tuint32_t src[REPO_MAX_PATH_DEPTH], dst[REPO_MAX_PATH_DEPTH];\n \tuint32_t text_delta, prop_delta;\n } node_ctx;\n@@ -73,7 +73,6 @@ static void reset_node_ctx(char *fname)\n \tnode_ctx.src[0] = ~0;\n \tnode_ctx.srcRev = 0;\n \tpool_tok_seq(REPO_MAX_PATH_DEPTH, node_ctx.dst, \"/\", fname);\n-\tnode_ctx.mark = 0;\n \tnode_ctx.text_delta = 0;\n \tnode_ctx.prop_delta = 0;\n }\n@@ -151,7 +150,7 @@ static void read_props(void)\n \n static void handle_node(void)\n {\n-\tuint32_t old_mode = 0;\n+\tuint32_t old_mode = 0, mark = 0;\n \n \tif (node_ctx.text_delta || node_ctx.prop_delta)\n \t\tdie(\"text and property deltas not supported\");\n@@ -164,7 +163,7 @@ static void handle_node(void)\n \n \tif (node_ctx.textLength != LENGTH_UNKNOWN &&\n \t node_ctx.type != REPO_MODE_DIR)\n-\t\tnode_ctx.mark = next_blob_mark();\n+\t\tmark = next_blob_mark();\n \n \tif (node_ctx.action == NODEACT_DELETE) {\n \t\trepo_delete(node_ctx.dst);\n@@ -172,26 +171,26 @@ static void handle_node(void)\n \t\t\t node_ctx.action == NODEACT_REPLACE) {\n \t\tif (node_ctx.action == NODEACT_REPLACE &&\n \t\t node_ctx.type == REPO_MODE_DIR)\n-\t\t\trepo_replace(node_ctx.dst, node_ctx.mark);\n+\t\t\trepo_replace(node_ctx.dst, mark);\n \t\telse if (node_ctx.propLength != LENGTH_UNKNOWN)\n-\t\t\trepo_modify(node_ctx.dst, node_ctx.type, node_ctx.mark);\n+\t\t\trepo_modify(node_ctx.dst, node_ctx.type, mark);\n \t\telse if (node_ctx.textLength != LENGTH_UNKNOWN)\n-\t\t\told_mode = repo_replace(node_ctx.dst, node_ctx.mark);\n+\t\t\told_mode = repo_replace(node_ctx.dst, mark);\n \t} else if (node_ctx.action == NODEACT_ADD) {\n \t\tif (node_ctx.srcRev && node_ctx.propLength != LENGTH_UNKNOWN)\n-\t\t\trepo_modify(node_ctx.dst, node_ctx.type, node_ctx.mark);\n+\t\t\trepo_modify(node_ctx.dst, node_ctx.type, mark);\n \t\telse if (node_ctx.srcRev && node_ctx.textLength != LENGTH_UNKNOWN)\n-\t\t\told_mode = repo_replace(node_ctx.dst, node_ctx.mark);\n+\t\t\told_mode = repo_replace(node_ctx.dst, mark);\n \t\telse if ((node_ctx.type == REPO_MODE_DIR && !node_ctx.srcRev) ||\n \t\t\t node_ctx.textLength != LENGTH_UNKNOWN)\n-\t\t\trepo_add(node_ctx.dst, node_ctx.type, node_ctx.mark);\n+\t\t\trepo_add(node_ctx.dst, node_ctx.type, mark);\n \t}\n \n \tif (node_ctx.propLength == LENGTH_UNKNOWN && old_mode)\n \t\tnode_ctx.type = old_mode;\n \n-\tif (node_ctx.mark)\n-\t\tfast_export_blob(node_ctx.type, node_ctx.mark, node_ctx.textLength);\n+\tif (mark)\n+\t\tfast_export_blob(node_ctx.type, mark, node_ctx.textLength);\n \telse if (node_ctx.textLength != LENGTH_UNKNOWN)\n \t\tbuffer_skip_bytes(node_ctx.textLength);\n }\n-- \n1.7.2.3\n"},{"id":"156229","messageId":"20101120004740.GE17445@burratino","threadId":"25776","inReplyTo":"20101120004525.GA17445@burratino","subject":"[PATCH 04/15] vcs-svn: Unclutter handle_node by introducing have_props var","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-20T00:47:41Z","receivedAt":"2010-11-20T00:47:41Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"It is possible for a path node in an SVN-format dump file to leave out\nthe properties section. svn-fe handles this by carrying over the\nproperties (in particular, file type) from the old version of that\nnode.\n\nTo support this, handle_node tests several times whether a\nProp-content-length field is present. Ancient Subversion actually\nleaves out the Prop-content-length field even for nodes with\nproperties, so that's not quite the right check. Besides, this detail\nof mechanism is distracting when the question at hand is instead what\ncontent the new node should have.\n\nSo introduce a local have_props variable. The semantics are the same\nas before; the adaptations to support ancient streams that leave out\nthe prop-content-length can wait until someone needs them.\n\nSigned-off-by: Jonathan Nieder \n---\n vcs-svn/svndump.c | 9 +++++----\n 1 files changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex 1fb7f82..45f0e47 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -151,11 +151,12 @@ static void read_props(void)\n static void handle_node(void)\n {\n \tuint32_t old_mode = 0, mark = 0;\n+\tconst int have_props = node_ctx.propLength != LENGTH_UNKNOWN;\n \n \tif (node_ctx.text_delta || node_ctx.prop_delta)\n \t\tdie(\"text and property deltas not supported\");\n \n-\tif (node_ctx.propLength != LENGTH_UNKNOWN && node_ctx.propLength)\n+\tif (have_props && node_ctx.propLength)\n \t\tread_props();\n \n \tif (node_ctx.srcRev)\n@@ -172,12 +173,12 @@ static void handle_node(void)\n \t\tif (node_ctx.action == NODEACT_REPLACE &&\n \t\t node_ctx.type == REPO_MODE_DIR)\n \t\t\trepo_replace(node_ctx.dst, mark);\n-\t\telse if (node_ctx.propLength != LENGTH_UNKNOWN)\n+\t\telse if (have_props)\n \t\t\trepo_modify(node_ctx.dst, node_ctx.type, mark);\n \t\telse if (node_ctx.textLength != LENGTH_UNKNOWN)\n \t\t\told_mode = repo_replace(node_ctx.dst, mark);\n \t} else if (node_ctx.action == NODEACT_ADD) {\n-\t\tif (node_ctx.srcRev && node_ctx.propLength != LENGTH_UNKNOWN)\n+\t\tif (node_ctx.srcRev && have_props)\n \t\t\trepo_modify(node_ctx.dst, node_ctx.type, mark);\n \t\telse if (node_ctx.srcRev && node_ctx.textLength != LENGTH_UNKNOWN)\n \t\t\told_mode = repo_replace(node_ctx.dst, mark);\n@@ -186,7 +187,7 @@ static void handle_node(void)\n \t\t\trepo_add(node_ctx.dst, node_ctx.type, mark);\n \t}\n \n-\tif (node_ctx.propLength == LENGTH_UNKNOWN && old_mode)\n+\tif (!have_props && old_mode)\n \t\tnode_ctx.type = old_mode;\n \n \tif (mark)\n-- \n1.7.2.3\n"},{"id":"156230","messageId":"20101120004851.GF17445@burratino","threadId":"25776","inReplyTo":"20101120004525.GA17445@burratino","subject":"[PATCH 05/15] vcs-svn: Use mark to indicate nodes with included text","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-20T00:48:51Z","receivedAt":"2010-11-20T00:48:51Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Allocate a mark if needed as soon as possible so later code can use\n\"if (mark)\" to check if this node has text attached rather than\nexplicitly checking for Text-content-length.\n\nWhile at it, reject directory nodes with text attached; the presence\nof such a node would indicate a bug in the dump generator or svn-fe's\nunderstanding. In the long term, it would be nice to be able to\ncontinue parsing and save the error for later, but for now it is\nsimpler to error out right away.\n\nSigned-off-by: Jonathan Nieder \n---\n vcs-svn/svndump.c | 16 ++++++++--------\n 1 files changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex 45f0e47..844076b 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -156,15 +156,17 @@ static void handle_node(void)\n \tif (node_ctx.text_delta || node_ctx.prop_delta)\n \t\tdie(\"text and property deltas not supported\");\n \n+\tif (node_ctx.textLength != LENGTH_UNKNOWN)\n+\t\tmark = next_blob_mark();\n+\n \tif (have_props && node_ctx.propLength)\n \t\tread_props();\n \n \tif (node_ctx.srcRev)\n \t\told_mode = repo_copy(node_ctx.srcRev, node_ctx.src, node_ctx.dst);\n \n-\tif (node_ctx.textLength != LENGTH_UNKNOWN &&\n-\t node_ctx.type != REPO_MODE_DIR)\n-\t\tmark = next_blob_mark();\n+\tif (mark && node_ctx.type == REPO_MODE_DIR)\n+\t\tdie(\"invalid dump: directories cannot have text attached\");\n \n \tif (node_ctx.action == NODEACT_DELETE) {\n \t\trepo_delete(node_ctx.dst);\n@@ -175,15 +177,15 @@ static void handle_node(void)\n \t\t\trepo_replace(node_ctx.dst, mark);\n \t\telse if (have_props)\n \t\t\trepo_modify(node_ctx.dst, node_ctx.type, mark);\n-\t\telse if (node_ctx.textLength != LENGTH_UNKNOWN)\n+\t\telse if (mark)\n \t\t\told_mode = repo_replace(node_ctx.dst, mark);\n \t} else if (node_ctx.action == NODEACT_ADD) {\n \t\tif (node_ctx.srcRev && have_props)\n \t\t\trepo_modify(node_ctx.dst, node_ctx.type, mark);\n-\t\telse if (node_ctx.srcRev && node_ctx.textLength != LENGTH_UNKNOWN)\n+\t\telse if (node_ctx.srcRev && mark)\n \t\t\told_mode = repo_replace(node_ctx.dst, mark);\n \t\telse if ((node_ctx.type == REPO_MODE_DIR && !node_ctx.srcRev) ||\n-\t\t\t node_ctx.textLength != LENGTH_UNKNOWN)\n+\t\t\t mark)\n \t\t\trepo_add(node_ctx.dst, node_ctx.type, mark);\n \t}\n \n@@ -192,8 +194,6 @@ static void handle_node(void)\n \n \tif (mark)\n \t\tfast_export_blob(node_ctx.type, mark, node_ctx.textLength);\n-\telse if (node_ctx.textLength != LENGTH_UNKNOWN)\n-\t\tbuffer_skip_bytes(node_ctx.textLength);\n }\n \n static void handle_revision(void)\n-- \n1.7.2.3\n"},{"id":"156231","messageId":"20101120004917.GG17445@burratino","threadId":"25776","inReplyTo":"20101120004525.GA17445@burratino","subject":"[PATCH 06/15] vcs-svn: handle_node: Handle deletion case early","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-20T00:49:17Z","receivedAt":"2010-11-20T00:49:17Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Take care of \"Node-action: delete\" as soon as possible, so we can stop\nworrying about that case in the rest of the function.\n\nFunctional change: catch deletion nodes with features that would not\napply to them (text, properties, or origin data) and error out for\nthose cases.\n\nSigned-off-by: Jonathan Nieder \n---\n vcs-svn/svndump.c | 11 ++++++++---\n 1 files changed, 8 insertions(+), 3 deletions(-)\n\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex 844076b..bc70023 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -159,6 +159,13 @@ static void handle_node(void)\n \tif (node_ctx.textLength != LENGTH_UNKNOWN)\n \t\tmark = next_blob_mark();\n \n+\tif (node_ctx.action == NODEACT_DELETE) {\n+\t\tif (mark || have_props || node_ctx.srcRev)\n+\t\t\tdie(\"invalid dump: deletion node has \"\n+\t\t\t\t\"copyfrom info, text, or properties\");\n+\t\treturn repo_delete(node_ctx.dst);\n+\t}\n+\n \tif (have_props && node_ctx.propLength)\n \t\tread_props();\n \n@@ -168,9 +175,7 @@ static void handle_node(void)\n \tif (mark && node_ctx.type == REPO_MODE_DIR)\n \t\tdie(\"invalid dump: directories cannot have text attached\");\n \n-\tif (node_ctx.action == NODEACT_DELETE) {\n-\t\trepo_delete(node_ctx.dst);\n-\t} else if (node_ctx.action == NODEACT_CHANGE ||\n+\tif (node_ctx.action == NODEACT_CHANGE ||\n \t\t\t node_ctx.action == NODEACT_REPLACE) {\n \t\tif (node_ctx.action == NODEACT_REPLACE &&\n \t\t node_ctx.type == REPO_MODE_DIR)\n-- \n1.7.2.3\n"},{"id":"156232","messageId":"20101120004955.GH17445@burratino","threadId":"25776","inReplyTo":"20101120004525.GA17445@burratino","subject":"[PATCH 07/15] vcs-svn: Replace = Delete + Add","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-20T00:49:55Z","receivedAt":"2010-11-20T00:49:55Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Simplify by reducing the \"Node-action: replace\" case to \"Node-action:\nadd\". This way, the main part of handle_node() only has to deal with\n\"add\" and \"change\" nodes.\n\nFunctional change: replacing a symlink or executable without setting\nproperties will reset the mode.\n\nSigned-off-by: Jonathan Nieder \n---\nI was happy to discover this.\n\n vcs-svn/svndump.c | 13 +++++++------\n 1 files changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex bc70023..6a6aaf9 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -166,6 +166,11 @@ static void handle_node(void)\n \t\treturn repo_delete(node_ctx.dst);\n \t}\n \n+\tif (node_ctx.action == NODEACT_REPLACE) {\n+\t\trepo_delete(node_ctx.dst);\n+\t\tnode_ctx.action = NODEACT_ADD;\n+\t}\n+\n \tif (have_props && node_ctx.propLength)\n \t\tread_props();\n \n@@ -175,12 +180,8 @@ static void handle_node(void)\n \tif (mark && node_ctx.type == REPO_MODE_DIR)\n \t\tdie(\"invalid dump: directories cannot have text attached\");\n \n-\tif (node_ctx.action == NODEACT_CHANGE ||\n-\t\t\t node_ctx.action == NODEACT_REPLACE) {\n-\t\tif (node_ctx.action == NODEACT_REPLACE &&\n-\t\t node_ctx.type == REPO_MODE_DIR)\n-\t\t\trepo_replace(node_ctx.dst, mark);\n-\t\telse if (have_props)\n+\tif (node_ctx.action == NODEACT_CHANGE) {\n+\t\tif (have_props)\n \t\t\trepo_modify(node_ctx.dst, node_ctx.type, mark);\n \t\telse if (mark)\n \t\t\told_mode = repo_replace(node_ctx.dst, mark);\n-- \n1.7.2.3\n"},{"id":"156233","messageId":"20101120005150.GI17445@burratino","threadId":"25776","inReplyTo":"20101120004525.GA17445@burratino","subject":"[PATCH 08/15] vcs-svn: Combine repo_replace and repo_modify functions","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-20T00:51:50Z","receivedAt":"2010-11-20T00:51:50Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"There are two functions to change the staged content for a path in the\nsvn importer's active commit: repo_replace, which changes the text and\nreturns the mode, and repo_modify, which changes the text and mode and\nreturns nothing.\n\nWorse, there are more subtle differences:\n\n - A mark of 0 passed to repo_modify means \"use the existing content\".\n repo_replace uses it as mark :0 and produces a corrupt stream.\n\n - When passed a path that is not part of the active commit,\n repo_replace returns without doing anything. repo_modify\n transparently adds a new directory entry.\n\nGet rid of both and introduce a new function with the best features of\nboth: repo_modify_path modifies the mode, content, or both for a path,\ndepending on which arguments are zero. If no such dirent already\nexists, it does nothing and reports the error by returning 0.\nOtherwise, the return value is the resulting mode.\n\nSigned-off-by: Jonathan Nieder \n---\nDavid, I think we discussed doing something like this a few months\nago. It seems appropriate because Subversion's Replace FS operation\nis not similar to repo_replace after all.\n\n vcs-svn/repo_tree.c | 21 +++++++--------------\n vcs-svn/repo_tree.h | 3 +--\n vcs-svn/svndump.c | 8 ++++----\n 3 files changed, 12 insertions(+), 20 deletions(-)\n\ndiff --git a/vcs-svn/repo_tree.c b/vcs-svn/repo_tree.c\nindex e94d91d..7214ac8 100644\n--- a/vcs-svn/repo_tree.c\n+++ b/vcs-svn/repo_tree.c\n@@ -175,25 +175,18 @@ void repo_add(uint32_t *path, uint32_t mode, uint32_t blob_mark)\n \trepo_write_dirent(path, mode, blob_mark, 0);\n }\n \n-uint32_t repo_replace(uint32_t *path, uint32_t blob_mark)\n+uint32_t repo_modify_path(uint32_t *path, uint32_t mode, uint32_t blob_mark)\n {\n-\tuint32_t mode = 0;\n \tstruct repo_dirent *src_dent;\n \tsrc_dent = repo_read_dirent(active_commit, path);\n-\tif (src_dent != NULL) {\n-\t\tmode = src_dent->mode;\n-\t\trepo_write_dirent(path, mode, blob_mark, 0);\n-\t}\n-\treturn mode;\n-}\n-\n-void repo_modify(uint32_t *path, uint32_t mode, uint32_t blob_mark)\n-{\n-\tstruct repo_dirent *src_dent;\n-\tsrc_dent = repo_read_dirent(active_commit, path);\n-\tif (src_dent != NULL && blob_mark == 0)\n+\tif (!src_dent)\n+\t\treturn 0;\n+\tif (!blob_mark)\n \t\tblob_mark = src_dent->content_offset;\n+\tif (!mode)\n+\t\tmode = src_dent->mode;\n \trepo_write_dirent(path, mode, blob_mark, 0);\n+\treturn mode;\n }\n \n void repo_delete(uint32_t *path)\ndiff --git a/vcs-svn/repo_tree.h b/vcs-svn/repo_tree.h\nindex 5476175..68baeb5 100644\n--- a/vcs-svn/repo_tree.h\n+++ b/vcs-svn/repo_tree.h\n@@ -14,8 +14,7 @@\n uint32_t next_blob_mark(void);\n uint32_t repo_copy(uint32_t revision, uint32_t *src, uint32_t *dst);\n void repo_add(uint32_t *path, uint32_t mode, uint32_t blob_mark);\n-uint32_t repo_replace(uint32_t *path, uint32_t blob_mark);\n-void repo_modify(uint32_t *path, uint32_t mode, uint32_t blob_mark);\n+uint32_t repo_modify_path(uint32_t *path, uint32_t mode, uint32_t blob_mark);\n void repo_delete(uint32_t *path);\n void repo_commit(uint32_t revision, uint32_t author, char *log, uint32_t uuid,\n \t\t uint32_t url, long unsigned timestamp);\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex 6a6aaf9..e40be58 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -182,14 +182,14 @@ static void handle_node(void)\n \n \tif (node_ctx.action == NODEACT_CHANGE) {\n \t\tif (have_props)\n-\t\t\trepo_modify(node_ctx.dst, node_ctx.type, mark);\n+\t\t\trepo_modify_path(node_ctx.dst, node_ctx.type, mark);\n \t\telse if (mark)\n-\t\t\told_mode = repo_replace(node_ctx.dst, mark);\n+\t\t\told_mode = repo_modify_path(node_ctx.dst, 0, mark);\n \t} else if (node_ctx.action == NODEACT_ADD) {\n \t\tif (node_ctx.srcRev && have_props)\n-\t\t\trepo_modify(node_ctx.dst, node_ctx.type, mark);\n+\t\t\trepo_modify_path(node_ctx.dst, node_ctx.type, mark);\n \t\telse if (node_ctx.srcRev && mark)\n-\t\t\told_mode = repo_replace(node_ctx.dst, mark);\n+\t\t\told_mode = repo_modify_path(node_ctx.dst, 0, mark);\n \t\telse if ((node_ctx.type == REPO_MODE_DIR && !node_ctx.srcRev) ||\n \t\t\t mark)\n \t\t\trepo_add(node_ctx.dst, node_ctx.type, mark);\n-- \n1.7.2.3\n"},{"id":"156234","messageId":"20101120005228.GJ17445@burratino","threadId":"25776","inReplyTo":"20101120004525.GA17445@burratino","subject":"[PATCH 09/15] vcs-svn: Delay read of per-path properties","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-20T00:52:28Z","receivedAt":"2010-11-20T00:52:28Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The mode for each file in an svn-format dump is kept in the properties\nsection. The properties section is read as soon as possible to allow\nthe correct mode to be filled in when registering the file with the\nrepo_tree lib.\n\nTo support nodes with a missing properties section, svn-fe determines\nthe mode in three stages:\n\n - The kind (directory or file) of the node is read from the dump and\n used to make an initial estimate (040000 or 100644).\n - Properties are read in and allowed to override this for symlinks\n and executables.\n - If there is no properties section, the mode from the previous\n content of the path is left alone, overriding the above\n considerations.\n\nThis is a bit of a mess, and worse, it would get even more complicated\nonce we start to support property deltas. If we could only register\nthe file with a provisional value for mode and then change it later\nwhen properties say so, the procedure would be much simpler.\n\n... oh, right, we can.\n\nSigned-off-by: Jonathan Nieder \n---\n vcs-svn/svndump.c | 42 +++++++++++++++++++-----------------------\n 1 files changed, 19 insertions(+), 23 deletions(-)\n\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex e40be58..4fdfcbb 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -150,7 +150,8 @@ static void read_props(void)\n \n static void handle_node(void)\n {\n-\tuint32_t old_mode = 0, mark = 0;\n+\tuint32_t mark = 0;\n+\tconst uint32_t type = node_ctx.type;\n \tconst int have_props = node_ctx.propLength != LENGTH_UNKNOWN;\n \n \tif (node_ctx.text_delta || node_ctx.prop_delta)\n@@ -171,32 +172,27 @@ static void handle_node(void)\n \t\tnode_ctx.action = NODEACT_ADD;\n \t}\n \n-\tif (have_props && node_ctx.propLength)\n-\t\tread_props();\n+\tif (node_ctx.srcRev) {\n+\t\trepo_copy(node_ctx.srcRev, node_ctx.src, node_ctx.dst);\n+\t\tnode_ctx.action = NODEACT_CHANGE;\n+\t}\n \n-\tif (node_ctx.srcRev)\n-\t\told_mode = repo_copy(node_ctx.srcRev, node_ctx.src, node_ctx.dst);\n-\n-\tif (mark && node_ctx.type == REPO_MODE_DIR)\n+\tif (mark && type == REPO_MODE_DIR)\n \t\tdie(\"invalid dump: directories cannot have text attached\");\n \n-\tif (node_ctx.action == NODEACT_CHANGE) {\n-\t\tif (have_props)\n-\t\t\trepo_modify_path(node_ctx.dst, node_ctx.type, mark);\n-\t\telse if (mark)\n-\t\t\told_mode = repo_modify_path(node_ctx.dst, 0, mark);\n-\t} else if (node_ctx.action == NODEACT_ADD) {\n-\t\tif (node_ctx.srcRev && have_props)\n-\t\t\trepo_modify_path(node_ctx.dst, node_ctx.type, mark);\n-\t\telse if (node_ctx.srcRev && mark)\n-\t\t\told_mode = repo_modify_path(node_ctx.dst, 0, mark);\n-\t\telse if ((node_ctx.type == REPO_MODE_DIR && !node_ctx.srcRev) ||\n-\t\t\t mark)\n-\t\t\trepo_add(node_ctx.dst, node_ctx.type, mark);\n-\t}\n+\tif (node_ctx.action == NODEACT_CHANGE)\n+\t\tnode_ctx.type = repo_modify_path(node_ctx.dst, 0, mark);\n+\telse\t/* Node-action: add */\n+\t\trepo_add(node_ctx.dst, type, mark);\n \n-\tif (!have_props && old_mode)\n-\t\tnode_ctx.type = old_mode;\n+\tif (have_props) {\n+\t\tconst uint32_t old_mode = node_ctx.type;\n+\t\tnode_ctx.type = type;\n+\t\tif (node_ctx.propLength)\n+\t\t\tread_props();\n+\t\tif (node_ctx.type != old_mode)\n+\t\t\trepo_modify_path(node_ctx.dst, node_ctx.type, mark);\n+\t}\n \n \tif (mark)\n \t\tfast_export_blob(node_ctx.type, mark, node_ctx.textLength);\n-- \n1.7.2.3\n"},{"id":"156235","messageId":"20101120005259.GK17445@burratino","threadId":"25776","inReplyTo":"20101120004525.GA17445@burratino","subject":"[PATCH 10/15] vcs-svn: Reject path nodes without Node-action","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-20T00:52:59Z","receivedAt":"2010-11-20T00:52:59Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"It would be better to flag such errors and let the import proceed\nanyway, but for now it is simpler not to worry about recovery\nfrom such weird cases.\n\nSigned-off-by: Jonathan Nieder \n---\n t/t9010-svn-fe.sh | 20 ++++++++++++++++++++\n vcs-svn/svndump.c | 7 +++++--\n 2 files changed, 25 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh\nindex 729e73d..cb9a236 100755\n--- a/t/t9010-svn-fe.sh\n+++ b/t/t9010-svn-fe.sh\n@@ -252,6 +252,26 @@ test_expect_success 'directory with files' '\n \ttest_cmp hi directory/file2\n '\n \n+test_expect_success 'node without action' '\n+\tcat >inaction.dump <<-\\EOF &&\n+\tSVN-fs-dump-format-version: 3\n+\n+\tRevision-number: 1\n+\tProp-content-length: 10\n+\tContent-length: 10\n+\n+\tPROPS-END\n+\n+\tNode-path: directory\n+\tNode-kind: dir\n+\tProp-content-length: 10\n+\tContent-length: 10\n+\n+\tPROPS-END\n+\tEOF\n+\ttest_must_fail test-svn-fe inaction.dump\n+'\n+\n test_expect_failure 'change file mode but keep old content' '\n \treinit_git &&\n \tcat >expect <<-\\EOF &&\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex 4fdfcbb..78c2de0 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -174,7 +174,8 @@ static void handle_node(void)\n \n \tif (node_ctx.srcRev) {\n \t\trepo_copy(node_ctx.srcRev, node_ctx.src, node_ctx.dst);\n-\t\tnode_ctx.action = NODEACT_CHANGE;\n+\t\tif (node_ctx.action == NODEACT_ADD)\n+\t\t\t node_ctx.action = NODEACT_CHANGE;\n \t}\n \n \tif (mark && type == REPO_MODE_DIR)\n@@ -182,8 +183,10 @@ static void handle_node(void)\n \n \tif (node_ctx.action == NODEACT_CHANGE)\n \t\tnode_ctx.type = repo_modify_path(node_ctx.dst, 0, mark);\n-\telse\t/* Node-action: add */\n+\telse if (node_ctx.action == NODEACT_ADD)\n \t\trepo_add(node_ctx.dst, type, mark);\n+\telse\n+\t\tdie(\"invalid dump: Node-path block lacks Node-action\");\n \n \tif (have_props) {\n \t\tconst uint32_t old_mode = node_ctx.type;\n-- \n1.7.2.3\n"},{"id":"156236","messageId":"20101120005334.GL17445@burratino","threadId":"25776","inReplyTo":"20101120004525.GA17445@burratino","subject":"[PATCH 11/15] vcs-svn: More dump format sanity checks","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-20T00:53:34Z","receivedAt":"2010-11-20T00:53:34Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Node-action: change is not appropriate when switching between file and\ndirectory or adding a new file. Current svn-fe silently accepts such\nnodes and the resulting tree has missing files in the \"changed when\nmeant to add\" case.\n\nNode-action: add requires some content (text or directory); there is\nno such thing as an \"intent to add\" node in svn dumps. Current svn-fe\naccepts such contentless adds but produces an invalid fast-import\nstream that refers to nonexistent mark :0 in response.\n\nSigned-off-by: Jonathan Nieder \n---\n t/t9010-svn-fe.sh | 21 +++++++++++++++++++++\n vcs-svn/svndump.c | 18 ++++++++++++++----\n 2 files changed, 35 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh\nindex cb9a236..f1e8799 100755\n--- a/t/t9010-svn-fe.sh\n+++ b/t/t9010-svn-fe.sh\n@@ -272,6 +272,27 @@ test_expect_success 'node without action' '\n \ttest_must_fail test-svn-fe inaction.dump\n '\n \n+test_expect_success 'action: add node without text' '\n+\tcat >textless.dump <<-\\EOF &&\n+\tSVN-fs-dump-format-version: 3\n+\n+\tRevision-number: 1\n+\tProp-content-length: 10\n+\tContent-length: 10\n+\n+\tPROPS-END\n+\n+\tNode-path: textless\n+\tNode-kind: file\n+\tNode-action: add\n+\tProp-content-length: 10\n+\tContent-length: 10\n+\n+\tPROPS-END\n+\tEOF\n+\ttest_must_fail test-svn-fe textless.dump\n+'\n+\n test_expect_failure 'change file mode but keep old content' '\n \treinit_git &&\n \tcat >expect <<-\\EOF &&\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex 78c2de0..62bb148 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -181,12 +181,22 @@ static void handle_node(void)\n \tif (mark && type == REPO_MODE_DIR)\n \t\tdie(\"invalid dump: directories cannot have text attached\");\n \n-\tif (node_ctx.action == NODEACT_CHANGE)\n-\t\tnode_ctx.type = repo_modify_path(node_ctx.dst, 0, mark);\n-\telse if (node_ctx.action == NODEACT_ADD)\n+\tif (node_ctx.action == NODEACT_CHANGE) {\n+\t\tuint32_t mode = repo_modify_path(node_ctx.dst, 0, mark);\n+\t\tif (!mode)\n+\t\t\tdie(\"invalid dump: path to be modified is missing\");\n+\t\tif (mode == REPO_MODE_DIR && type != REPO_MODE_DIR)\n+\t\t\tdie(\"invalid dump: cannot modify a directory into a file\");\n+\t\tif (mode != REPO_MODE_DIR && type == REPO_MODE_DIR)\n+\t\t\tdie(\"invalid dump: cannot modify a file into a directory\");\n+\t\tnode_ctx.type = mode;\n+\t} else if (node_ctx.action == NODEACT_ADD) {\n+\t\tif (!mark && type != REPO_MODE_DIR)\n+\t\t\tdie(\"invalid dump: adds node without text\");\n \t\trepo_add(node_ctx.dst, type, mark);\n-\telse\n+\t} else {\n \t\tdie(\"invalid dump: Node-path block lacks Node-action\");\n+\t}\n \n \tif (have_props) {\n \t\tconst uint32_t old_mode = node_ctx.type;\n-- \n1.7.2.3\n"},{"id":"156237","messageId":"20101120005354.GM17445@burratino","threadId":"25776","inReplyTo":"20101120004525.GA17445@burratino","subject":"[PATCH 12/15] vcs-svn: Make source easier to read on small screens","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-20T00:53:54Z","receivedAt":"2010-11-20T00:53:54Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Remove some newlines from handle_node() that are not needed for\nclarity.\n\nSigned-off-by: Jonathan Nieder \n---\n vcs-svn/svndump.c | 8 --------\n 1 files changed, 0 insertions(+), 8 deletions(-)\n\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex 62bb148..5de32e1 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -156,31 +156,25 @@ static void handle_node(void)\n \n \tif (node_ctx.text_delta || node_ctx.prop_delta)\n \t\tdie(\"text and property deltas not supported\");\n-\n \tif (node_ctx.textLength != LENGTH_UNKNOWN)\n \t\tmark = next_blob_mark();\n-\n \tif (node_ctx.action == NODEACT_DELETE) {\n \t\tif (mark || have_props || node_ctx.srcRev)\n \t\t\tdie(\"invalid dump: deletion node has \"\n \t\t\t\t\"copyfrom info, text, or properties\");\n \t\treturn repo_delete(node_ctx.dst);\n \t}\n-\n \tif (node_ctx.action == NODEACT_REPLACE) {\n \t\trepo_delete(node_ctx.dst);\n \t\tnode_ctx.action = NODEACT_ADD;\n \t}\n-\n \tif (node_ctx.srcRev) {\n \t\trepo_copy(node_ctx.srcRev, node_ctx.src, node_ctx.dst);\n \t\tif (node_ctx.action == NODEACT_ADD)\n \t\t\t node_ctx.action = NODEACT_CHANGE;\n \t}\n-\n \tif (mark && type == REPO_MODE_DIR)\n \t\tdie(\"invalid dump: directories cannot have text attached\");\n-\n \tif (node_ctx.action == NODEACT_CHANGE) {\n \t\tuint32_t mode = repo_modify_path(node_ctx.dst, 0, mark);\n \t\tif (!mode)\n@@ -197,7 +191,6 @@ static void handle_node(void)\n \t} else {\n \t\tdie(\"invalid dump: Node-path block lacks Node-action\");\n \t}\n-\n \tif (have_props) {\n \t\tconst uint32_t old_mode = node_ctx.type;\n \t\tnode_ctx.type = type;\n@@ -206,7 +199,6 @@ static void handle_node(void)\n \t\tif (node_ctx.type != old_mode)\n \t\t\trepo_modify_path(node_ctx.dst, node_ctx.type, mark);\n \t}\n-\n \tif (mark)\n \t\tfast_export_blob(node_ctx.type, mark, node_ctx.textLength);\n }\n-- \n1.7.2.3\n"},{"id":"156238","messageId":"20101120005420.GN17445@burratino","threadId":"25776","inReplyTo":"20101120004525.GA17445@burratino","subject":"[PATCH 13/15] vcs-svn: Split off function for handling of individual properties","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-20T00:54:20Z","receivedAt":"2010-11-20T00:54:20Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The handle_property function is the part of read_props that would be\ninteresting for most people: semantics of properties rather than the\nalgorithm for parsing them.\n\nSigned-off-by: Jonathan Nieder \n---\n vcs-svn/svndump.c | 33 +++++++++++++++++++--------------\n 1 files changed, 19 insertions(+), 14 deletions(-)\n\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex 5de32e1..3acc36c 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -30,7 +30,7 @@\n /* Create memory pool for log messages */\n obj_pool_gen(log, char, 4096)\n \n-static char* log_copy(uint32_t length, char *log)\n+static char *log_copy(uint32_t length, const char *log)\n {\n \tchar *buffer;\n \tlog_free(log_pool.size);\n@@ -115,6 +115,23 @@ static void init_keys(void)\n \tkeys.prop_delta = pool_intern(\"Prop-delta\");\n }\n \n+static void handle_property(uint32_t key, const char *val, uint32_t len)\n+{\n+\tif (key == keys.svn_log) {\n+\t\t/* Value length excludes terminating nul. */\n+\t\trev_ctx.log = log_copy(len + 1, val);\n+\t} else if (key == keys.svn_author) {\n+\t\trev_ctx.author = pool_intern(val);\n+\t} else if (key == keys.svn_date) {\n+\t\tif (parse_date_basic(val, &rev_ctx.timestamp, NULL))\n+\t\t\tfprintf(stderr, \"Invalid timestamp: %s\\n\", val);\n+\t} else if (key == keys.svn_executable) {\n+\t\tnode_ctx.type = REPO_MODE_EXE;\n+\t} else if (key == keys.svn_special) {\n+\t\tnode_ctx.type = REPO_MODE_LNK;\n+\t}\n+}\n+\n static void read_props(void)\n {\n \tuint32_t len;\n@@ -129,19 +146,7 @@ static void read_props(void)\n \t\t} else if (!strncmp(t, \"V \", 2)) {\n \t\t\tlen = atoi(&t[2]);\n \t\t\tval = buffer_read_string(len);\n-\t\t\tif (key == keys.svn_log) {\n-\t\t\t\t/* Value length excludes terminating nul. */\n-\t\t\t\trev_ctx.log = log_copy(len + 1, val);\n-\t\t\t} else if (key == keys.svn_author) {\n-\t\t\t\trev_ctx.author = pool_intern(val);\n-\t\t\t} else if (key == keys.svn_date) {\n-\t\t\t\tif (parse_date_basic(val, &rev_ctx.timestamp, NULL))\n-\t\t\t\t\tfprintf(stderr, \"Invalid timestamp: %s\\n\", val);\n-\t\t\t} else if (key == keys.svn_executable) {\n-\t\t\t\tnode_ctx.type = REPO_MODE_EXE;\n-\t\t\t} else if (key == keys.svn_special) {\n-\t\t\t\tnode_ctx.type = REPO_MODE_LNK;\n-\t\t\t}\n+\t\t\thandle_property(key, val, len);\n \t\t\tkey = ~0;\n \t\t\tbuffer_read_line();\n \t\t}\n-- \n1.7.2.3\n"},{"id":"156239","messageId":"20101120005444.GO17445@burratino","threadId":"25776","inReplyTo":"20101120004525.GA17445@burratino","subject":"[PATCH 14/15] vcs-svn: Sharpen parsing of property lines","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-20T00:54:45Z","receivedAt":"2010-11-20T00:54:45Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Prepare to add a new type of property line (the 'D' line) to\nhandle property deltas.\n\nSigned-off-by: Jonathan Nieder \n---\n vcs-svn/svndump.c | 30 +++++++++++++++++++-----------\n 1 files changed, 19 insertions(+), 11 deletions(-)\n\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex 3acc36c..c3e1e32 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -134,21 +134,29 @@ static void handle_property(uint32_t key, const char *val, uint32_t len)\n \n static void read_props(void)\n {\n-\tuint32_t len;\n \tuint32_t key = ~0;\n-\tchar *val = NULL;\n-\tchar *t;\n+\tconst char *t;\n \twhile ((t = buffer_read_line()) && strcmp(t, \"PROPS-END\")) {\n-\t\tif (!strncmp(t, \"K \", 2)) {\n-\t\t\tlen = atoi(&t[2]);\n-\t\t\tkey = pool_intern(buffer_read_string(len));\n-\t\t\tbuffer_read_line();\n-\t\t} else if (!strncmp(t, \"V \", 2)) {\n-\t\t\tlen = atoi(&t[2]);\n-\t\t\tval = buffer_read_string(len);\n+\t\tuint32_t len;\n+\t\tconst char *val;\n+\t\tconst char type = t[0];\n+\n+\t\tif (!type || t[1] != ' ')\n+\t\t\tdie(\"invalid property line: %s\\n\", t);\n+\t\tlen = atoi(&t[2]);\n+\t\tval = buffer_read_string(len);\n+\t\tbuffer_skip_bytes(1);\t/* Discard trailing newline. */\n+\n+\t\tswitch (type) {\n+\t\tcase 'K':\n+\t\t\tkey = pool_intern(val);\n+\t\t\tcontinue;\n+\t\tcase 'V':\n \t\t\thandle_property(key, val, len);\n \t\t\tkey = ~0;\n-\t\t\tbuffer_read_line();\n+\t\t\tcontinue;\n+\t\tdefault:\n+\t\t\tdie(\"invalid property line: %s\\n\", t);\n \t\t}\n \t}\n }\n-- \n1.7.2.3\n"},{"id":"156240","messageId":"20101120005746.GP17445@burratino","threadId":"25776","inReplyTo":"20101120004525.GA17445@burratino","subject":"[PATCH 15/15] vcs-svn: Implement Prop-delta handling","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-20T00:57:46Z","receivedAt":"2010-11-20T00:57:46Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: David Barr \n\nThe rules for what file is used as delta source for each file are not\ndocumented in dump-load-format.txt. Luckily, the Apache Software\nFoundation repository has rich enough examples to figure out most of\nthe rules:\n\nNode-action: replace implies the empty property set and empty text as\npreimage for deltas. Otherwise, if a copyfrom source is given, that\nnode is the preimage for deltas. Lastly, if none of the above applies\nand the node path exists in the current revision, then that version\nforms the basis.\n\n[jn: refactored, with tests]\n\nSigned-off-by: David Barr \nSigned-off-by: Jonathan Nieder \n---\nThat's the end of this chapter. Thanks for reading.\n\nIn the next chapter, the line_buffer lib will gain the ability to read\nfrom multiple files (based on some patches seen on the list before)\nand use this ability to manage the cat-blob-fd and some temporary\nfiles for applying deltas.\n\nReading from the cat-blob-fd with stdio functions carries a great risk\nof deadlock, so we need to find a way to do this without fdopen()\n(some routines for doing this are already written and usable, but they\nhaven't been made into a nice library yet).\n\n t/t9010-svn-fe.sh | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++-\n vcs-svn/svndump.c | 54 +++++++++++++++++++++++-----\n 2 files changed, 144 insertions(+), 12 deletions(-)\n\ndiff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh\nindex f1e8799..7dc0670 100755\n--- a/t/t9010-svn-fe.sh\n+++ b/t/t9010-svn-fe.sh\n@@ -514,7 +514,12 @@ test_expect_success 'deltas not supported' '\n \ttest_must_fail test-svn-fe delta.dump\n '\n \n-test_expect_success 'property deltas not supported' '\n+test_expect_success 'property deltas supported' '\n+\treinit_git &&\n+\tcat >expect <<-\\EOF &&\n+\tOBJID\n+\t:100755 100644 OBJID OBJID M\tscript.sh\n+\tEOF\n \t{\n \t\tproperties \\\n \t\t\tsvn:author author@example.com \\\n@@ -565,7 +570,100 @@ test_expect_success 'property deltas not supported' '\n \t\tPROPS-END\n \t\tEOF\n \t} >propdelta.dump &&\n-\ttest_must_fail test-svn-fe propdelta.dump\n+\ttest-svn-fe propdelta.dump >stream &&\n+\tgit fast-import actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'deltas for typechange' '\n+\treinit_git &&\n+\tcat >expect <<-\\EOF &&\n+\tOBJID\n+\t:120000 100644 OBJID OBJID T\ttest-file\n+\tOBJID\n+\t:100755 120000 OBJID OBJID T\ttest-file\n+\tOBJID\n+\t:000000 100755 OBJID OBJID A\ttest-file\n+\tEOF\n+\tcat >deleteprop.dump <<-\\EOF &&\n+\tSVN-fs-dump-format-version: 3\n+\n+\tRevision-number: 1\n+\tProp-content-length: 10\n+\tContent-length: 10\n+\n+\tPROPS-END\n+\n+\tNode-path: test-file\n+\tNode-kind: file\n+\tNode-action: add\n+\tProp-delta: true\n+\tProp-content-length: 35\n+\tText-content-length: 17\n+\tContent-length: 52\n+\n+\tK 14\n+\tsvn:executable\n+\tV 0\n+\n+\tPROPS-END\n+\tlink testing 123\n+\n+\tRevision-number: 2\n+\tProp-content-length: 10\n+\tContent-length: 10\n+\n+\tPROPS-END\n+\n+\tNode-path: test-file\n+\tNode-kind: file\n+\tNode-action: change\n+\tProp-delta: true\n+\tProp-content-length: 53\n+\tText-content-length: 17\n+\tContent-length: 70\n+\n+\tK 11\n+\tsvn:special\n+\tV 1\n+\t*\n+\tD 14\n+\tsvn:executable\n+\tPROPS-END\n+\tlink testing 231\n+\n+\tRevision-number: 3\n+\tProp-content-length: 10\n+\tContent-length: 10\n+\n+\tPROPS-END\n+\n+\tNode-path: test-file\n+\tNode-kind: file\n+\tNode-action: change\n+\tProp-delta: true\n+\tProp-content-length: 27\n+\tText-content-length: 17\n+\tContent-length: 44\n+\n+\tD 11\n+\tsvn:special\n+\tPROPS-END\n+\tlink testing 321\n+\tEOF\n+\ttest-svn-fe deleteprop.dump >stream &&\n+\tgit fast-import actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success 't9135/svn.dump' '\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex c3e1e32..8a224ac 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -115,20 +115,35 @@ static void init_keys(void)\n \tkeys.prop_delta = pool_intern(\"Prop-delta\");\n }\n \n-static void handle_property(uint32_t key, const char *val, uint32_t len)\n+static void handle_property(uint32_t key, const char *val, uint32_t len,\n+\t\t\t\tuint32_t *type_set)\n {\n \tif (key == keys.svn_log) {\n+\t\tif (!val)\n+\t\t\tdie(\"invalid dump: unsets svn:log\");\n \t\t/* Value length excludes terminating nul. */\n \t\trev_ctx.log = log_copy(len + 1, val);\n \t} else if (key == keys.svn_author) {\n \t\trev_ctx.author = pool_intern(val);\n \t} else if (key == keys.svn_date) {\n+\t\tif (!val)\n+\t\t\tdie(\"invalid dump: unsets svn:date\");\n \t\tif (parse_date_basic(val, &rev_ctx.timestamp, NULL))\n-\t\t\tfprintf(stderr, \"Invalid timestamp: %s\\n\", val);\n-\t} else if (key == keys.svn_executable) {\n-\t\tnode_ctx.type = REPO_MODE_EXE;\n-\t} else if (key == keys.svn_special) {\n-\t\tnode_ctx.type = REPO_MODE_LNK;\n+\t\t\twarning(\"invalid timestamp: %s\", val);\n+\t} else if (key == keys.svn_executable || key == keys.svn_special) {\n+\t\tif (*type_set) {\n+\t\t\tif (!val)\n+\t\t\t\treturn;\n+\t\t\tdie(\"invalid dump: sets type twice\");\n+\t\t}\n+\t\tif (!val) {\n+\t\t\tnode_ctx.type = REPO_MODE_BLB;\n+\t\t\treturn;\n+\t\t}\n+\t\t*type_set = 1;\n+\t\tnode_ctx.type = key == keys.svn_executable ?\n+\t\t\t\tREPO_MODE_EXE :\n+\t\t\t\tREPO_MODE_LNK;\n \t}\n }\n \n@@ -136,6 +151,19 @@ static void read_props(void)\n {\n \tuint32_t key = ~0;\n \tconst char *t;\n+\t/*\n+\t * NEEDSWORK: to support simple mode changes like\n+\t *\tK 11\n+\t *\tsvn:special\n+\t *\tV 1\n+\t *\t*\n+\t *\tD 14\n+\t *\tsvn:executable\n+\t * we keep track of whether a mode has been set and reset to\n+\t * plain file only if not. We should be keeping track of the\n+\t * symlink and executable bits separately instead.\n+\t */\n+\tuint32_t type_set = 0;\n \twhile ((t = buffer_read_line()) && strcmp(t, \"PROPS-END\")) {\n \t\tuint32_t len;\n \t\tconst char *val;\n@@ -151,8 +179,13 @@ static void read_props(void)\n \t\tcase 'K':\n \t\t\tkey = pool_intern(val);\n \t\t\tcontinue;\n+\t\tcase 'D':\n+\t\t\tkey = pool_intern(val);\n+\t\t\tval = NULL;\n+\t\t\tlen = 0;\n+\t\t\t/* fall through */\n \t\tcase 'V':\n-\t\t\thandle_property(key, val, len);\n+\t\t\thandle_property(key, val, len, &type_set);\n \t\t\tkey = ~0;\n \t\t\tcontinue;\n \t\tdefault:\n@@ -167,8 +200,8 @@ static void handle_node(void)\n \tconst uint32_t type = node_ctx.type;\n \tconst int have_props = node_ctx.propLength != LENGTH_UNKNOWN;\n \n-\tif (node_ctx.text_delta || node_ctx.prop_delta)\n-\t\tdie(\"text and property deltas not supported\");\n+\tif (node_ctx.text_delta)\n+\t\tdie(\"text deltas not supported\");\n \tif (node_ctx.textLength != LENGTH_UNKNOWN)\n \t\tmark = next_blob_mark();\n \tif (node_ctx.action == NODEACT_DELETE) {\n@@ -206,7 +239,8 @@ static void handle_node(void)\n \t}\n \tif (have_props) {\n \t\tconst uint32_t old_mode = node_ctx.type;\n-\t\tnode_ctx.type = type;\n+\t\tif (!node_ctx.prop_delta)\n+\t\t\tnode_ctx.type = type;\n \t\tif (node_ctx.propLength)\n \t\t\tread_props();\n \t\tif (node_ctx.type != old_mode)\n-- \n1.7.2.3\n"},{"id":"156252","messageId":"20101120145306.GA9804@burratino","threadId":"25776","inReplyTo":"20101120005259.GK17445@burratino","subject":"Re: [PATCH 10/15] vcs-svn: Reject path nodes without Node-action","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-20T14:53:07Z","receivedAt":"2010-11-20T14:53:07Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n\n> --- a/vcs-svn/svndump.c\n> +++ b/vcs-svn/svndump.c\n> @@ -174,7 +174,8 @@ static void handle_node(void)\n> \n> \tif (node_ctx.srcRev) {\n> \t\trepo_copy(node_ctx.srcRev, node_ctx.src, node_ctx.dst);\n> -\t\tnode_ctx.action = NODEACT_CHANGE;\n> +\t\tif (node_ctx.action == NODEACT_ADD)\n> +\t\t\t node_ctx.action = NODEACT_CHANGE;\n\nWhitespace damage. Here's a fixup.\n\nSigned-off-by: Jonathan Nieder \n---\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex 78c2de0..0af8ac6 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -175,7 +175,7 @@ static void handle_node(void)\n \tif (node_ctx.srcRev) {\n \t\trepo_copy(node_ctx.srcRev, node_ctx.src, node_ctx.dst);\n \t\tif (node_ctx.action == NODEACT_ADD)\n-\t\t\t node_ctx.action = NODEACT_CHANGE;\n+\t\t\tnode_ctx.action = NODEACT_CHANGE;\n \t}\n \n \tif (mark && type == REPO_MODE_DIR)\n"},{"id":"156256","messageId":"20101120192153.GA17823@burratino","threadId":"25776","inReplyTo":"20101120004525.GA17445@burratino","subject":"[WIP/PATCH 0/8] svn-fe: support for text deltas","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-20T19:21:53Z","receivedAt":"2010-11-20T19:21:53Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n> Jonathan Nieder wrote:\n\n>> This mini-series is part of an effort to get David and Ram's svn\n>> import work integrated into mainline git[1].\n\nThe next chapter. This adds handling of text deltas. I broke it\nin the process, though, as you can see with the attached test script:\n\n\t$ testme.sh http://svn.apache.org/repos/asf 100 asf.git\n\t[...]\n\t* Dumped revision 35.\n\terror: Preimage ends early\n\tfatal: cannot apply delta\n\nPerhaps you can take this as a puzzle. Where does the patch series\ngo wrong?\n\nI like to think some of the cleanup will make this more maintainable\nin the future, even if it is broken now.\n\nBuilds on the db/fast-import-cat-blob[1] and jn/svndiff0[2] topics.\n\nThoughts, improvements, complaints welcome.\n\nDavid Barr (2):\n vcs-svn: Let caller set up sliding window for delta preimage\n vcs-svn: Implement text-delta handling\n\nJonathan Nieder (6):\n svn-fe: Prepare for strbuf use\n vcs-svn: Internal fast_export_save_blob helper\n vcs-svn: Introduce repo_read_path to check the content at a path\n vcs-svn: Introduce fd_buffer library\n vcs-svn: Read delta preimage from file descriptor\n vcs-svn: Teach line_buffer about temporary files\n\n[1] Doesn't seem to be available on gmane.\nThe important part (patch 3) is, though:\nhttp://thread.gmane.org/gmane.comp.version-control.git/161730\nShould I re-send?\n\n[2] http://thread.gmane.org/gmane.comp.version-control.git/151086/focus=158913\n"},{"id":"156255","messageId":"20101120192247.GB17823@burratino","threadId":"25776","inReplyTo":"20101120192153.GA17823@burratino","subject":"[PATCH 1/8] svn-fe: Prepare for strbuf use","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-20T19:22:47Z","receivedAt":"2010-11-20T19:22:47Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Linking svn-fe strbuf.o will require sha1_name.o and wrapper.o, hence\nsha1_file.o, hence libz, pthreads, and most of libgit. Luckily there\nis a separate patch series in flight that would trim down those\ndependencies again.\n\nSigned-off-by: Jonathan Nieder \n---\n contrib/svn-fe/Makefile | 15 +++++++--------\n 1 files changed, 7 insertions(+), 8 deletions(-)\n\ndiff --git a/contrib/svn-fe/Makefile b/contrib/svn-fe/Makefile\nindex 360d8da..9732b03 100644\n--- a/contrib/svn-fe/Makefile\n+++ b/contrib/svn-fe/Makefile\n@@ -8,11 +8,13 @@ CFLAGS = -g -O2 -Wall\n LDFLAGS =\n ALL_CFLAGS = $(CFLAGS)\n ALL_LDFLAGS = $(LDFLAGS)\n-EXTLIBS =\n+PTHREAD_LIBS = -lpthread\n+EXTLIBS = -lz $(PTHREAD_LIBS)\n \n GIT_LIB = ../../libgit.a\n VCSSVN_LIB = ../../vcs-svn/lib.a\n-LIBS = $(VCSSVN_LIB) $(GIT_LIB) $(EXTLIBS)\n+XDIFF_LIB = ../../xdiff/lib.a\n+LIBS = $(VCSSVN_LIB) $(GIT_LIB) $(XDIFF_LIB) $(EXTLIBS)\n \n QUIET_SUBDIR0 = +$(MAKE) -C # space to separate -C and subdir\n QUIET_SUBDIR1 =\n@@ -33,7 +35,7 @@ ifndef V\n endif\n endif\n \n-svn-fe$X: svn-fe.o $(VCSSVN_LIB) $(GIT_LIB)\n+svn-fe$X: svn-fe.o $(VCSSVN_LIB) $(GIT_LIB) $(XDIFF_LIB)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ svn-fe.o \\\n \t\t$(ALL_LDFLAGS) $(LIBS)\n \n@@ -51,11 +53,8 @@ svn-fe.1: svn-fe.txt\n \t\t../contrib/svn-fe/$@\n \t$(MV) ../../Documentation/svn-fe.1 .\n \n-../../vcs-svn/lib.a: FORCE\n-\t$(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) vcs-svn/lib.a\n-\n-../../libgit.a: FORCE\n-\t$(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) libgit.a\n+../../%.a: FORCE\n+\t$(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) $*.a\n \n clean:\n \t$(RM) svn-fe$X svn-fe.o svn-fe.html svn-fe.xml svn-fe.1\n-- \n1.7.2.3\n"},{"id":"156257","messageId":"20101120192509.GC17823@burratino","threadId":"25776","inReplyTo":"20101120192153.GA17823@burratino","subject":"[PATCH 2/8] vcs-svn: Internal fast_export_save_blob helper","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-20T19:25:09Z","receivedAt":"2010-11-20T19:25:09Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Introduce fast_export_save_blob, meant to be used after printing\n\n\tcat-blob :n\n\nto the fast-import stream. It reads a response from fd 3 in cat-file\n--batch format. To avoid deadlock, it uses file descriptor-level\ncalls (no stdio) and reads only one character at a time (though the\nlatter restriction could and should be relaxed somehow --- O_NONBLOCK,\nperhaps).\n\nSigned-off-by: Jonathan Nieder \n---\nThe effect of this one is almost completely undone by a later patch. :)\n\nProbably it would be better to use stdio with O_NONBLOCK, after all\n(David, sorry I forgot about our conversations on this before). This\nseries uses file descriptors to be conservative, because I do not have\nmuch of a desire to test for proper O_NONBLOCK support on the relevant\nplatforms yet.\n\n vcs-svn/fast_export.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 87 insertions(+), 0 deletions(-)\n\ndiff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c\nindex 6cfa256..3feef66 100644\n--- a/vcs-svn/fast_export.c\n+++ b/vcs-svn/fast_export.c\n@@ -8,8 +8,10 @@\n #include \"line_buffer.h\"\n #include \"repo_tree.h\"\n #include \"string_pool.h\"\n+#include \"strbuf.h\"\n \n #define MAX_GITSVN_LINE_LEN 4096\n+#define REPORT_FILENO 3\n \n static uint32_t first_commit_done;\n \n@@ -63,6 +65,91 @@ void fast_export_commit(uint32_t revision, uint32_t author, char *log,\n \tprintf(\"progress Imported commit %\"PRIu32\".\\n\\n\", revision);\n }\n \n+static int ends_with(const char *s, size_t len, const char *suffix)\n+{\n+\tconst size_t suffixlen = strlen(suffix);\n+\tif (len < suffixlen)\n+\t\treturn 0;\n+\treturn !memcmp(s + len - suffixlen, suffix, suffixlen);\n+}\n+\n+static int parse_cat_response_line(const char *header, size_t *len)\n+{\n+\tsize_t headerlen = strlen(header);\n+\tconst char *type;\n+\tconst char *end;\n+\n+\tif (ends_with(header, headerlen, \" missing\"))\n+\t\treturn error(\"cat-blob reports missing blob: %s\", header);\n+\ttype = memmem(header, headerlen, \" blob \", strlen(\" blob \"));\n+\tif (!type)\n+\t\treturn error(\"cat-blob header has wrong object type: %s\", header);\n+\t*len = strtoumax(type + strlen(\" blob \"), (char **) &end, 10);\n+\tif (end == type + strlen(\" blob \"))\n+\t\treturn error(\"cat-blob header does not contain length: %s\", header);\n+\tif (*end)\n+\t\treturn error(\"cat-blob header contains garbage after length: %s\", header);\n+\treturn 0;\n+}\n+\n+static struct strbuf response_line = STRBUF_INIT;\n+static const char *get_response_line(void)\n+{\n+\t/*\n+\t * NEEDSWORK: Does not actually need to read one byte at a time.\n+\t * Some platforms have O_NONBLOCK. On others we could read 8 chars\n+\t * at a time until a potential appearance of \" blob \".\n+\t */\n+\tstrbuf_reset(&response_line);\n+\tfor (;;) {\n+\t\tchar buf[1];\n+\t\tif (xread(REPORT_FILENO, buf, 1) < 0) {\n+\t\t\terror(\"cannot read cat-blob result: %s\", strerror(errno));\n+\t\t\treturn NULL;\n+\t\t}\n+\t\tif (*buf == '\\n')\n+\t\t\treturn response_line.buf;\n+\t\tstrbuf_addch(&response_line, *buf);\n+\t}\n+}\n+\n+static int copy_bytes(FILE *out, size_t len)\n+{\n+\tchar buf[4096];\n+\tssize_t nread;\n+\tfor (; len; len -= nread) {\n+\t\tnread = xread(REPORT_FILENO, buf,\n+\t\t\t\t\tlen < sizeof(buf) ? len : sizeof(buf));\n+\t\tif (nread < 0)\n+\t\t\treturn error(\"cannot copy cat-blob result: %s\",\n+\t\t\t\t\tstrerror(errno));\n+\t\tif (!nread)\n+\t\t\treturn error(\"0-length read...\");\n+\t\tif (fwrite(buf, 1, nread, out) != nread)\n+\t\t\treturn error(\"cannot write cat-blob results: %s\",\n+\t\t\t\t\tstrerror(errno));\n+\t}\n+}\n+\n+static int fast_export_save_blob(FILE *out)\n+{\n+\tsize_t len = len;\n+\tconst char *header, *tail;\n+\n+\theader = get_response_line();\n+\tif (!header || parse_cat_response_line(header, &len))\n+\t\treturn -1;\n+\tcopy_bytes(out, len);\n+\n+\t/* Discard trailing newline. */\n+\ttail = get_response_line();\n+\tif (!tail)\n+\t\treturn -1;\n+\tif (*tail)\n+\t\treturn error(\"line following cat-blob response contains garbage: %s\", tail);\n+\treturn 0;\n+}\n+\n void fast_export_blob(uint32_t mode, uint32_t mark, uint32_t len)\n {\n \tif (mode == REPO_MODE_LNK) {\n-- \n1.7.2.3\n"},{"id":"156258","messageId":"20101120192528.GD17823@burratino","threadId":"25776","inReplyTo":"20101120192153.GA17823@burratino","subject":"[PATCH 3/8] vcs-svn: Introduce repo_read_path to check the content at a path","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-20T19:25:28Z","receivedAt":"2010-11-20T19:25:28Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"repo_modify_path returns the mode.\n\nSigned-off-by: Jonathan Nieder \n---\n vcs-svn/repo_tree.c | 9 +++++++++\n vcs-svn/repo_tree.h | 1 +\n 2 files changed, 10 insertions(+), 0 deletions(-)\n\ndiff --git a/vcs-svn/repo_tree.c b/vcs-svn/repo_tree.c\nindex 7214ac8..eb55636 100644\n--- a/vcs-svn/repo_tree.c\n+++ b/vcs-svn/repo_tree.c\n@@ -157,6 +157,15 @@ static void repo_write_dirent(uint32_t *path, uint32_t mode,\n \t\tdent_remove(&dir_pointer(parent_dir_o)->entries, dent);\n }\n \n+uint32_t repo_read_path(uint32_t *path)\n+{\n+\tuint32_t content_offset = 0;\n+\tstruct repo_dirent *dent = repo_read_dirent(active_commit, path);\n+\tif (dent != NULL)\n+\t\tcontent_offset = dent->content_offset;\n+\treturn content_offset;\n+}\n+\n uint32_t repo_copy(uint32_t revision, uint32_t *src, uint32_t *dst)\n {\n \tuint32_t mode = 0, content_offset = 0;\ndiff --git a/vcs-svn/repo_tree.h b/vcs-svn/repo_tree.h\nindex 68baeb5..7070839 100644\n--- a/vcs-svn/repo_tree.h\n+++ b/vcs-svn/repo_tree.h\n@@ -15,6 +15,7 @@ uint32_t next_blob_mark(void);\n uint32_t repo_copy(uint32_t revision, uint32_t *src, uint32_t *dst);\n void repo_add(uint32_t *path, uint32_t mode, uint32_t blob_mark);\n uint32_t repo_modify_path(uint32_t *path, uint32_t mode, uint32_t blob_mark);\n+uint32_t repo_read_path(uint32_t *path);\n void repo_delete(uint32_t *path);\n void repo_commit(uint32_t revision, uint32_t author, char *log, uint32_t uuid,\n \t\t uint32_t url, long unsigned timestamp);\n-- \n1.7.2.3\n"},{"id":"156259","messageId":"20101120192648.GE17823@burratino","threadId":"25776","inReplyTo":"20101120192153.GA17823@burratino","subject":"[PATCH 4/8] vcs-svn: Introduce fd_buffer routines","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-20T19:26:48Z","receivedAt":"2010-11-20T19:26:48Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The fd_buffer library is perhaps poorly named, because it does not\nmanage its own buffer. These functions provide the relevant\nfunctionality from line_buffer for low-level, unbuffered (file\ndescriptor) I/O.\n\nThe purpose is to preserve convenience while avoiding deadlock that\ncould occur from too greedily reading ahead in the cat-blob-fd pipe.\n(An alternative approach to achieve the same effect would be to\nset the O_NONBLOCK flag on the pipe fd.)\n\nSigned-off-by: Jonathan Nieder \n---\n Makefile | 5 ++-\n vcs-svn/fast_export.c | 45 ++++++++++------------------------\n vcs-svn/fd_buffer.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++\n vcs-svn/fd_buffer.h | 19 ++++++++++++++\n 4 files changed, 100 insertions(+), 34 deletions(-)\n create mode 100644 vcs-svn/fd_buffer.c\n create mode 100644 vcs-svn/fd_buffer.h\n\ndiff --git a/Makefile b/Makefile\nindex aa10288..844e3b4 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1760,7 +1760,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/string_pool.o vcs-svn/line_buffer.o \\\n+VCSSVN_OBJS = vcs-svn/string_pool.o vcs-svn/line_buffer.o vcs-svn/fd_buffer.o \\\n \tvcs-svn/repo_tree.o vcs-svn/fast_export.o vcs-svn/svndump.o \\\n \tvcs-svn/sliding_window.o vcs-svn/svndiff.o\n OBJECTS := $(GIT_OBJS) $(XDIFF_OBJS) $(VCSSVN_OBJS)\n@@ -1889,7 +1889,8 @@ xdiff-interface.o $(XDIFF_OBJS): \\\n $(VCSSVN_OBJS): \\\n \tvcs-svn/obj_pool.h vcs-svn/trp.h vcs-svn/string_pool.h \\\n \tvcs-svn/line_buffer.h vcs-svn/repo_tree.h vcs-svn/fast_export.h \\\n-\tvcs-svn/sliding_window.h vcs-svn/svndump.h vcs-svn/svndiff.h\n+\tvcs-svn/sliding_window.h vcs-svn/svndump.h vcs-svn/svndiff.h \\\n+\tvcs-svn/fd_buffer.h\n endif\n \n exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \\\ndiff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c\nindex 0de498b..ebaab72 100644\n--- a/vcs-svn/fast_export.c\n+++ b/vcs-svn/fast_export.c\n@@ -6,6 +6,7 @@\n #include \"git-compat-util.h\"\n #include \"fast_export.h\"\n #include \"line_buffer.h\"\n+#include \"fd_buffer.h\"\n #include \"repo_tree.h\"\n #include \"string_pool.h\"\n #include \"strbuf.h\"\n@@ -73,7 +74,7 @@ static int ends_with(const char *s, size_t len, const char *suffix)\n \treturn !memcmp(s + len - suffixlen, suffix, suffixlen);\n }\n \n-static int parse_cat_response_line(const char *header, size_t *len)\n+static int parse_cat_response_line(const char *header, off_t *len)\n {\n \tsize_t headerlen = strlen(header);\n \tconst char *type;\n@@ -95,45 +96,25 @@ static int parse_cat_response_line(const char *header, size_t *len)\n static struct strbuf response_line = STRBUF_INIT;\n static const char *get_response_line(void)\n {\n-\t/*\n-\t * NEEDSWORK: Does not actually need to read one byte at a time.\n-\t * Some platforms have O_NONBLOCK. On others we could read 8 chars\n-\t * at a time until a potential appearance of \" blob \".\n-\t */\n \tstrbuf_reset(&response_line);\n-\tfor (;;) {\n-\t\tchar buf[1];\n-\t\tif (xread(REPORT_FILENO, buf, 1) < 0) {\n-\t\t\terror(\"cannot read cat-blob result: %s\", strerror(errno));\n-\t\t\treturn NULL;\n-\t\t}\n-\t\tif (*buf == '\\n')\n-\t\t\treturn response_line.buf;\n-\t\tstrbuf_addch(&response_line, *buf);\n-\t}\n+\tif (fd_read_line(&response_line, REPORT_FILENO))\n+\t\treturn NULL;\n+\treturn response_line.buf;\n }\n \n-static int copy_bytes(FILE *out, size_t len)\n+static int copy_bytes(FILE *out, off_t len)\n {\n-\tchar buf[4096];\n-\tssize_t nread;\n-\tfor (; len; len -= nread) {\n-\t\tnread = xread(REPORT_FILENO, buf,\n-\t\t\t\t\tlen < sizeof(buf) ? len : sizeof(buf));\n-\t\tif (nread < 0)\n-\t\t\treturn error(\"cannot copy cat-blob result: %s\",\n-\t\t\t\t\tstrerror(errno));\n-\t\tif (!nread)\n-\t\t\treturn error(\"0-length read...\");\n-\t\tif (fwrite(buf, 1, nread, out) != nread)\n-\t\t\treturn error(\"cannot write cat-blob results: %s\",\n-\t\t\t\t\tstrerror(errno));\n-\t}\n+\toff_t ret = fd_copy_bytes(out, REPORT_FILENO, len);\n+\tif (!ret)\n+\t\treturn error(\"read error: file ends early\");\n+\tif (ret <= 0)\n+\t\treturn error(\"cannot copy cat-blob result\");\n+\treturn 0;\n }\n \n static int fast_export_save_blob(FILE *out)\n {\n-\tsize_t len = len;\n+\toff_t len = len;\n \tconst char *header, *tail;\n \n \theader = get_response_line();\ndiff --git a/vcs-svn/fd_buffer.c b/vcs-svn/fd_buffer.c\nnew file mode 100644\nindex 0000000..3d2c1a1\n--- /dev/null\n+++ b/vcs-svn/fd_buffer.c\n@@ -0,0 +1,65 @@\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 \"fd_buffer.h\"\n+#include \"strbuf.h\"\n+\n+/* Read a line without trailing newline. */\n+int fd_read_line(struct strbuf *line, int fd)\n+{\n+\t/*\n+\t * NEEDSWORK: Does not actually need to read one byte at a time.\n+\t * Some platforms have O_NONBLOCK. On others we could read\n+\t * several chars at a time until an appearance of a string known\n+\t * to belong in the line (e.g., \" blob \").\n+\t */\n+\tfor (;;) {\n+\t\tchar buf[1];\n+\t\tif (xread(fd, buf, 1) < 0)\n+\t\t\treturn error(\"cannot read line: %s\", strerror(errno));\n+\t\tif (*buf == '\\n')\n+\t\t\treturn 0;\n+\t\tstrbuf_addch(line, *buf);\n+\t}\n+}\n+\n+int fd_read_binary(struct strbuf *buf, size_t len, int fd)\n+{\n+\tchar *p, *end;\n+\tstrbuf_grow(buf, len);\n+\tp = buf->buf + buf->len;\n+\tend = p + len;\n+\n+\twhile (p != end) {\n+\t\tssize_t nread = xread(fd, p, end - p);\n+\t\tif (nread < 0)\n+\t\t\treturn error(\"read error: %s\", strerror(errno));\n+\t\tif (!nread)\t/* end of file */\n+\t\t\tbreak;\n+\t\tp += nread;\n+\t}\n+\tstrbuf_setlen(buf, p - buf->buf);\n+\treturn 0;\n+}\n+\n+off_t fd_copy_bytes(FILE *out, int fd, off_t len)\n+{\n+\toff_t total = 0;\n+\twhile (len) {\n+\t\tchar buf[4096];\n+\t\tssize_t nread = xread(fd, buf,\n+\t\t\t\t\tlen < sizeof(buf) ? len : sizeof(buf));\n+\t\tif (nread < 0)\n+\t\t\treturn error(\"read error: %s\", strerror(errno));\n+\t\tif (!nread)\n+\t\t\treturn 0;\n+\t\tif (out && fwrite(buf, 1, nread, out) != nread)\n+\t\t\treturn error(\"write error: %s\", strerror(errno));\n+\t\ttotal += nread;\n+\t\tlen -= nread;\n+\t}\n+\treturn total;\n+}\ndiff --git a/vcs-svn/fd_buffer.h b/vcs-svn/fd_buffer.h\nnew file mode 100644\nindex 0000000..9434ac3\n--- /dev/null\n+++ b/vcs-svn/fd_buffer.h\n@@ -0,0 +1,19 @@\n+#ifndef FD_BUFFER_H\n+#define FD_BUFFER_H\n+\n+#include \"strbuf.h\"\n+\n+/* Low-level input helpers. Usually line_buffer is a better choice. */\n+\n+extern int fd_read_line(struct strbuf *line, int fd);\n+extern int fd_read_binary(struct strbuf *buf, size_t len, int fd);\n+\n+/* returns 0 for end of file */\n+extern off_t fd_copy_bytes(FILE *out, int fd, off_t len);\n+\n+static inline off_t fd_skip_bytes(int fd, off_t len)\n+{\n+\treturn fd_copy_bytes(NULL, fd, len);\n+}\n+\n+#endif\n-- \n1.7.2.3\n"},{"id":"156260","messageId":"20101120192713.GF17823@burratino","threadId":"25776","inReplyTo":"20101120192153.GA17823@burratino","subject":"[PATCH 5/8] vcs-svn: Read delta preimage from file descriptor","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-20T19:27:13Z","receivedAt":"2010-11-20T19:27:13Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Teach the svndiff routines to read the preimage for a delta from a\nfile descriptor without reading ahead. This way, the delta applier\ncan stream its input directly from fast-import's cat-blob-fd pipe.\n\nSigned-off-by: Jonathan Nieder \n---\n test-svn-fe.c | 10 +++++-----\n vcs-svn/sliding_window.c | 21 +++++++++------------\n vcs-svn/sliding_window.h | 2 +-\n vcs-svn/svndiff.c | 6 +++---\n vcs-svn/svndiff.h | 2 +-\n 5 files changed, 19 insertions(+), 22 deletions(-)\n\ndiff --git a/test-svn-fe.c b/test-svn-fe.c\nindex 4799e4c..4ea0cd6 100644\n--- a/test-svn-fe.c\n+++ b/test-svn-fe.c\n@@ -22,20 +22,20 @@ int main(int argc, char *argv[])\n \t\treturn 0;\n \t}\n \tif (argc == 5 && !strcmp(argv[1], \"-d\")) {\n-\t\tstruct line_buffer preimage = LINE_BUFFER_INIT;\n+\t\tint preimage_fd = -1;\n \t\tstruct line_buffer delta = LINE_BUFFER_INIT;\n-\t\tif (buffer_init(&preimage, argv[2]))\n+\t\tpreimage_fd = open(argv[2], O_RDONLY);\n+\t\tif (preimage_fd < 0)\n \t\t\tdie_errno(\"cannot open preimage\");\n \t\tif (buffer_init(&delta, argv[3]))\n \t\t\tdie_errno(\"cannot open delta\");\n \t\tif (svndiff0_apply(&delta, (off_t) strtoull(argv[4], NULL, 0),\n-\t\t\t\t &preimage, stdout))\n+\t\t\t\t preimage_fd, stdout))\n \t\t\treturn 1;\n-\t\tif (buffer_deinit(&preimage))\n+\t\tif (close(preimage_fd))\n \t\t\tdie_errno(\"cannot close preimage\");\n \t\tif (buffer_deinit(&delta))\n \t\t\tdie_errno(\"cannot close delta\");\n-\t\tbuffer_reset(&preimage);\n \t\tbuffer_reset(&delta);\n \t\treturn 0;\n \t}\ndiff --git a/vcs-svn/sliding_window.c b/vcs-svn/sliding_window.c\nindex 5c08828..6d4261a 100644\n--- a/vcs-svn/sliding_window.c\n+++ b/vcs-svn/sliding_window.c\n@@ -5,7 +5,7 @@\n \n #include \"git-compat-util.h\"\n #include \"sliding_window.h\"\n-#include \"line_buffer.h\"\n+#include \"fd_buffer.h\"\n #include \"strbuf.h\"\n \n static void strbuf_remove_from_left(struct strbuf *sb, size_t nbytes)\n@@ -30,7 +30,7 @@ static int check_overflow(off_t a, size_t b)\n int move_window(struct view *view, off_t off, size_t len)\n {\n \toff_t file_offset;\n-\tassert(view && view->file);\n+\tassert(view && view->fd >= 0);\n \tassert(!check_overflow(view->off, view->buf.len));\n \n \tif (check_overflow(off, len))\n@@ -47,21 +47,18 @@ int move_window(struct view *view, off_t off, size_t len)\n \tif (off > file_offset) {\n \t\t/* Seek ahead to skip the gap. */\n \t\tconst off_t gap = off - file_offset;\n-\t\tconst off_t nread = buffer_skip_bytes(view->file, gap);\n+\t\tconst off_t nread = fd_skip_bytes(view->fd, gap);\n \t\tif (nread != gap) {\n-\t\t\tif (!buffer_ferror(view->file))\n+\t\t\tif (!nread)\n \t\t\t\treturn error(\"Preimage ends early\");\n-\t\t\treturn error(\"Cannot seek forward in input: %s\",\n-\t\t\t\t strerror(errno));\n+\t\t\treturn error(\"Cannot seek forward in input\");\n \t\t}\n \t\tfile_offset += gap;\n \t}\n-\tbuffer_read_binary(&view->buf, len - view->buf.len, view->file);\n-\tif (view->buf.len != len) {\n-\t\tif (!buffer_ferror(view->file))\n-\t\t\treturn error(\"Preimage ends early\");\n-\t\treturn error(\"Cannot read preimage: %s\", strerror(errno));\n-\t}\n+\tif (fd_read_binary(&view->buf, len - view->buf.len, view->fd))\n+\t\treturn error(\"Cannot read preimage\");\n+\tif (view->buf.len != len)\n+\t\treturn error(\"Preimage ends early\");\n \tview->off = off;\n \treturn 0;\n }\ndiff --git a/vcs-svn/sliding_window.h b/vcs-svn/sliding_window.h\nindex b9f0552..532dc93 100644\n--- a/vcs-svn/sliding_window.h\n+++ b/vcs-svn/sliding_window.h\n@@ -4,7 +4,7 @@\n #include \"strbuf.h\"\n \n struct view {\n-\tstruct line_buffer *file;\n+\tint fd;\n \toff_t off;\n \tstruct strbuf buf;\n };\ndiff --git a/vcs-svn/svndiff.c b/vcs-svn/svndiff.c\nindex 9ee7411..ef3a921 100644\n--- a/vcs-svn/svndiff.c\n+++ b/vcs-svn/svndiff.c\n@@ -283,10 +283,10 @@ static int apply_one_window(struct line_buffer *delta, off_t *delta_len,\n }\n \n int svndiff0_apply(struct line_buffer *delta, off_t delta_len,\n-\t\t struct line_buffer *preimage, FILE *postimage)\n+\t\t int preimage_fd, FILE *postimage)\n {\n-\tstruct view preimage_view = {preimage, 0, STRBUF_INIT};\n-\tassert(delta && preimage && postimage);\n+\tstruct view preimage_view = {preimage_fd, 0, STRBUF_INIT};\n+\tassert(delta && preimage_fd >= 0 && postimage);\n \n \tif (read_magic(delta, &delta_len))\n \t\tgoto fail;\ndiff --git a/vcs-svn/svndiff.h b/vcs-svn/svndiff.h\nindex a986099..9003d6e 100644\n--- a/vcs-svn/svndiff.h\n+++ b/vcs-svn/svndiff.h\n@@ -4,6 +4,6 @@\n #include \"line_buffer.h\"\n \n extern int svndiff0_apply(struct line_buffer *delta, off_t delta_len,\n-\t\t\t struct line_buffer *preimage, FILE *postimage);\n+\t\t\t\tint preimage_fd, FILE *postimage);\n \n #endif\n-- \n1.7.2.3\n"},{"id":"156261","messageId":"20101120192845.GG17823@burratino","threadId":"25776","inReplyTo":"20101120192153.GA17823@burratino","subject":"[PATCH 6/8] vcs-svn: Let caller set up sliding window for delta preimage","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-20T19:28:45Z","receivedAt":"2010-11-20T19:28:45Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The content of symlinks starts with the word \"link \" in SVN's\nworldview, so we need to prepend that text the sake of delta\napplication. Move responsibility for setting up the input state from\nsvndiff0_apply to the calling program, so programs have a chance to\nseed the sliding window with text of their choice.\n\n[jn: extracted from the patch \"svn-fe: Use the --report-fd feature\"]\n\nSigned-off-by: David Barr \nSigned-off-by: Jonathan Nieder \n---\nI'm a big fan of this trick.\n\n test-svn-fe.c | 8 +++++---\n vcs-svn/svndiff.c | 23 ++++++++---------------\n vcs-svn/svndiff.h | 3 ++-\n 3 files changed, 15 insertions(+), 19 deletions(-)\n\ndiff --git a/test-svn-fe.c b/test-svn-fe.c\nindex 4ea0cd6..64f63cf 100644\n--- a/test-svn-fe.c\n+++ b/test-svn-fe.c\n@@ -5,6 +5,7 @@\n #include \"git-compat-util.h\"\n #include \"vcs-svn/svndump.h\"\n #include \"vcs-svn/svndiff.h\"\n+#include \"vcs-svn/sliding_window.h\"\n #include \"vcs-svn/line_buffer.h\"\n \n int main(int argc, char *argv[])\n@@ -22,20 +23,21 @@ int main(int argc, char *argv[])\n \t\treturn 0;\n \t}\n \tif (argc == 5 && !strcmp(argv[1], \"-d\")) {\n-\t\tint preimage_fd = -1;\n \t\tstruct line_buffer delta = LINE_BUFFER_INIT;\n-\t\tpreimage_fd = open(argv[2], O_RDONLY);\n+\t\tint preimage_fd = open(argv[2], O_RDONLY);\n+\t\tstruct view preimage_view = {preimage_fd, 0, STRBUF_INIT};\n \t\tif (preimage_fd < 0)\n \t\t\tdie_errno(\"cannot open preimage\");\n \t\tif (buffer_init(&delta, argv[3]))\n \t\t\tdie_errno(\"cannot open delta\");\n \t\tif (svndiff0_apply(&delta, (off_t) strtoull(argv[4], NULL, 0),\n-\t\t\t\t preimage_fd, stdout))\n+\t\t\t\t &preimage_view, stdout))\n \t\t\treturn 1;\n \t\tif (close(preimage_fd))\n \t\t\tdie_errno(\"cannot close preimage\");\n \t\tif (buffer_deinit(&delta))\n \t\t\tdie_errno(\"cannot close delta\");\n+\t\tstrbuf_release(&preimage_view.buf);\n \t\tbuffer_reset(&delta);\n \t\treturn 0;\n \t}\ndiff --git a/vcs-svn/svndiff.c b/vcs-svn/svndiff.c\nindex ef3a921..308c734 100644\n--- a/vcs-svn/svndiff.c\n+++ b/vcs-svn/svndiff.c\n@@ -283,31 +283,24 @@ static int apply_one_window(struct line_buffer *delta, off_t *delta_len,\n }\n \n int svndiff0_apply(struct line_buffer *delta, off_t delta_len,\n-\t\t int preimage_fd, FILE *postimage)\n+\t\t\tstruct view *preimage_view, FILE *postimage)\n {\n-\tstruct view preimage_view = {preimage_fd, 0, STRBUF_INIT};\n-\tassert(delta && preimage_fd >= 0 && postimage);\n+\tassert(delta && preimage_view && postimage);\n \n \tif (read_magic(delta, &delta_len))\n-\t\tgoto fail;\n+\t\treturn -1;\n \twhile (delta_len > 0) {\t/* For each window: */\n \t\toff_t pre_off = pre_off;\n \t\tsize_t pre_len;\n \t\tif (read_offset(delta, &pre_off, &delta_len) ||\n \t\t read_length(delta, &pre_len, &delta_len) ||\n-\t\t move_window(&preimage_view, pre_off, pre_len) ||\n+\t\t move_window(preimage_view, pre_off, pre_len) ||\n \t\t apply_one_window(delta, &delta_len,\n-\t\t\t\t &preimage_view, postimage))\n-\t\t\tgoto fail;\n-\t\tif (delta_len && buffer_at_eof(delta)) {\n-\t\t\terror(\"Delta ends early! (%\"PRIu64\" bytes remaining)\",\n+\t\t\t\t preimage_view, postimage))\n+\t\t\treturn -1;\n+\t\tif (delta_len && buffer_at_eof(delta))\n+\t\t\treturn error(\"Delta ends early! (%\"PRIu64\" bytes remaining)\",\n \t\t\t (uint64_t) delta_len);\n-\t\t\tgoto fail;\n-\t\t}\n \t}\n-\tstrbuf_release(&preimage_view.buf);\n \treturn 0;\n- fail:\n-\tstrbuf_release(&preimage_view.buf);\n-\treturn -1;\n }\ndiff --git a/vcs-svn/svndiff.h b/vcs-svn/svndiff.h\nindex 9003d6e..bb5afd0 100644\n--- a/vcs-svn/svndiff.h\n+++ b/vcs-svn/svndiff.h\n@@ -2,8 +2,9 @@\n #define SVNDIFF_H_\n \n #include \"line_buffer.h\"\n+#include \"sliding_window.h\"\n \n extern int svndiff0_apply(struct line_buffer *delta, off_t delta_len,\n-\t\t\t\tint preimage_fd, FILE *postimage);\n+\t\t\t\tstruct view *preimage_view, FILE *postimage);\n \n #endif\n-- \n1.7.2.3\n"},{"id":"156262","messageId":"20101120192904.GH17823@burratino","threadId":"25776","inReplyTo":"20101120192153.GA17823@burratino","subject":"[PATCH 7/8] vcs-svn: Teach line_buffer about temporary files","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-20T19:29:04Z","receivedAt":"2010-11-20T19:29:04Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"It can sometimes be useful to write information temporarily to file,\nto read back later. These functions allow you to do so.\n\nThe dump file importer would use this to save a postimage from delta\napplication until the length is known and fast-import is ready to read\nit.\n\nSigned-off-by: Jonathan Nieder \n---\n vcs-svn/line_buffer.c | 24 ++++++++++++++++++++++++\n vcs-svn/line_buffer.h | 7 ++++++-\n vcs-svn/line_buffer.txt | 22 ++++++++++++++++++++++\n 3 files changed, 52 insertions(+), 1 deletions(-)\n\ndiff --git a/vcs-svn/line_buffer.c b/vcs-svn/line_buffer.c\nindex c54031b..f583623 100644\n--- a/vcs-svn/line_buffer.c\n+++ b/vcs-svn/line_buffer.c\n@@ -42,6 +42,30 @@ int buffer_at_eof(struct line_buffer *buf)\n \treturn 0;\n }\n \n+int buffer_tmpfile_init(struct line_buffer *buf)\n+{\n+\tbuf->infile = tmpfile();\n+\tif (!buf->infile)\n+\t\treturn error(\"cannot open temporary file: %s\", strerror(errno));\n+\treturn 0;\n+}\n+\n+FILE *buffer_tmpfile_rewind(struct line_buffer *buf)\n+{\n+\trewind(buf->infile);\n+\treturn buf->infile;\n+}\n+\n+long buffer_tmpfile_prepare_to_read(struct line_buffer *buf)\n+{\n+\tlong pos = ftell(buf->infile);\n+\tif (pos < 0)\n+\t\treturn error(\"ftell error: %s\", strerror(errno));\n+\tif (fseek(buf->infile, 0, SEEK_SET))\n+\t\treturn error(\"seek error: %s\", strerror(errno));\n+\treturn pos;\n+}\n+\n int buffer_read_char(struct line_buffer *buf)\n {\n \treturn fgetc(buf->infile);\ndiff --git a/vcs-svn/line_buffer.h b/vcs-svn/line_buffer.h\nindex 2375ee1..1db7434 100644\n--- a/vcs-svn/line_buffer.h\n+++ b/vcs-svn/line_buffer.h\n@@ -16,12 +16,17 @@ int buffer_init(struct line_buffer *buf, const char *filename);\n int buffer_deinit(struct line_buffer *buf);\n int buffer_ferror(struct line_buffer *buf);\n int buffer_at_eof(struct line_buffer *buf);\n+void buffer_reset(struct line_buffer *buf);\n+\n+int buffer_tmpfile_init(struct line_buffer *buf);\n+FILE *buffer_tmpfile_rewind(struct line_buffer *buf);\t/* prepare to write. */\n+long buffer_tmpfile_prepare_to_read(struct line_buffer *buf);\n+\n char *buffer_read_line(struct line_buffer *buf);\n char *buffer_read_string(struct line_buffer *buf, uint32_t len);\n int buffer_read_char(struct line_buffer *buf);\n void buffer_read_binary(struct strbuf *sb, uint32_t len, struct line_buffer *f);\n void buffer_copy_bytes(struct line_buffer *buf, off_t len);\n off_t buffer_skip_bytes(struct line_buffer *buf, off_t len);\n-void buffer_reset(struct line_buffer *buf);\n \n #endif\ndiff --git a/vcs-svn/line_buffer.txt b/vcs-svn/line_buffer.txt\nindex d06db24..344efc3 100644\n--- a/vcs-svn/line_buffer.txt\n+++ b/vcs-svn/line_buffer.txt\n@@ -24,6 +24,28 @@ The calling program:\n When finished, the caller can use `buffer_reset` to deallocate\n resources.\n \n+Using temporary files\n+---------------------\n+\n+Sometimes a file is just a convenient place to stash data for\n+later. A program\n+\n+ - initializes a `struct line_buffer` to LINE_BUFFER_INIT\n+ - allocates a temporary file with `buffer_tmpfile_init`\n+ - acquires an output handle by calling `buffer_tmpfile_rewind`\n+ - uses standard I/O functions like `fprintf` and `fwrite` to fill\n+ the temporary file\n+ - declares writing is over with `buffer_tmpfile_prepare_to_read`\n+ - can re-read what was written with `buffer_read_line`,\n+ `buffer_read_string`, and so on\n+ - can reuse the temporary file by calling `buffer_tmpfile_rewind`\n+ again\n+ - cleans up the temporary file with `buffer_deinit`, perhaps to\n+ reuse the line_buffer for some other file.\n+\n+When finished, the program should use `buffer_reset` to deallocate\n+resources.\n+\n Functions\n ---------\n \n-- \n1.7.2.3\n"},{"id":"156263","messageId":"20101120192928.GI17823@burratino","threadId":"25776","inReplyTo":"20101120192153.GA17823@burratino","subject":"[PATCH 8/8] vcs-svn: Implement text-delta handling","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-20T19:29:28Z","receivedAt":"2010-11-20T19:29:28Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: David Barr \n\nHandle input in Subversion's dumpfile format, version 3. This is the\nformat produced by \"svnrdump dump\" and \"svnadmin dump --deltas\", and\nthe main difference between v3 dumpfiles and the dumpfiles already\nhandled is that these can include nodes whose properties and text are\nexpressed relative to some other node.\n\nTo handle such nodes, we find which node the text and properties are\nbased on, handle its property changes, use the cat-blob command to\nrequest the basis blob from the fast-import backend, use the\nsvndiff0_apply() helper to apply the text delta on the fly, writing\noutput to a temporary file, and then measure that postimage file's\nlength and write its content to the fast-import stream.\n\nThe temporary postimage file is shared between delta-using nodes to\navoid some file system overhead.\n\nThe svn-fe interface needs to be more complicated to accomodate the\nbackward flow of information from the fast-import backend to svn-fe.\nThe interface for parsing streams without deltas is unchanged,\nthough.\n\nNEEDSWORK: generalize interface so caller sets the backflow fd, close\ntemporary file before exiting\n\nSigned-off-by: David Barr \nSigned-off-by: Jonathan Nieder \n---\n contrib/svn-fe/svn-fe.txt | 11 ++++--\n t/t9010-svn-fe.sh | 16 +++++++-\n vcs-svn/fast_export.c | 88 +++++++++++++++++++++++++++++++-------------\n vcs-svn/fast_export.h | 3 ++\n vcs-svn/svndump.c | 19 +++++++---\n 5 files changed, 99 insertions(+), 38 deletions(-)\n\ndiff --git a/contrib/svn-fe/svn-fe.txt b/contrib/svn-fe/svn-fe.txt\nindex 35f84bd..6af989f 100644\n--- a/contrib/svn-fe/svn-fe.txt\n+++ b/contrib/svn-fe/svn-fe.txt\n@@ -7,7 +7,13 @@ svn-fe - convert an SVN \"dumpfile\" to a fast-import stream\n \n SYNOPSIS\n --------\n-svnadmin dump --incremental REPO | svn-fe [url] | git fast-import\n+svnadmin dump REPO | svn-fe [url] | git fast-import\n+\n+[verse]\n+mkfifo backchannel &&\n+svnadmin dump --deltas REPO |\n+\tsvn-fe [url] 3backchannel\n \n DESCRIPTION\n -----------\n@@ -25,9 +31,6 @@ Subversion's repository dump format is documented in full in\n Files in this format can be generated using the 'svnadmin dump' or\n 'svk admin dump' command.\n \n-Dumps produced with 'svnadmin dump --deltas' (dumpfile format v3)\n-are not supported.\n-\n OUTPUT FORMAT\n -------------\n The fast-import format is documented by the git-fast-import(1)\ndiff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh\nindex 7dc0670..6c8b803 100755\n--- a/t/t9010-svn-fe.sh\n+++ b/t/t9010-svn-fe.sh\n@@ -35,6 +35,14 @@ text_no_props () {\n \n >empty\n \n+test_expect_success 'setup: have pipes?' '\n+\trm -f frob &&\n+\tif mkfifo frob\n+\tthen\n+\t\ttest_set_prereq PIPE\n+\tfi\n+'\n+\n test_expect_success 'empty dump' '\n \treinit_git &&\n \techo \"SVN-fs-dump-format-version: 2\" >input &&\n@@ -451,7 +459,9 @@ test_expect_success 'change file mode and reiterate content' '\n \ttest_cmp hello actual.target\n '\n \n-test_expect_success 'deltas not supported' '\n+test_expect_success PIPE 'deltas supported' '\n+\treinit_git &&\n+\trm -f backflow &&\n \t{\n \t\t# (old) h + (inline) ello + (old) \\n\n \t\tprintf \"SVNQ%b%b%s\" \"Q\\003\\006\\005\\004\" \"\\001Q\\0204\\001\\002\" \"ello\" |\n@@ -511,7 +521,9 @@ test_expect_success 'deltas not supported' '\n \t\techo PROPS-END &&\n \t\tcat delta\n \t} >delta.dump &&\n-\ttest_must_fail test-svn-fe delta.dump\n+\tmkfifo backflow &&\n+\ttest_must_fail test-svn-fe delta.dump 3backflow\n '\n \n test_expect_success 'property deltas supported' '\ndiff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c\nindex ebaab72..ceb1fc5 100644\n--- a/vcs-svn/fast_export.c\n+++ b/vcs-svn/fast_export.c\n@@ -5,9 +5,11 @@\n \n #include \"git-compat-util.h\"\n #include \"fast_export.h\"\n+#include \"repo_tree.h\"\n+#include \"svndiff.h\"\n+#include \"sliding_window.h\"\n #include \"line_buffer.h\"\n #include \"fd_buffer.h\"\n-#include \"repo_tree.h\"\n #include \"string_pool.h\"\n #include \"strbuf.h\"\n \n@@ -15,6 +17,17 @@\n #define REPORT_FILENO 3\n \n static uint32_t first_commit_done;\n+static struct line_buffer postimage = LINE_BUFFER_INIT;\n+\n+/* NEEDSWORK: move to fast_export_init() */\n+static int init_postimage(void)\n+{\n+\tstatic int postimage_initialized;\n+\tif (postimage_initialized)\n+\t\treturn 0;\n+\tpostimage_initialized = 1;\n+\treturn buffer_tmpfile_init(&postimage);\n+}\n \n void fast_export_delete(uint32_t depth, uint32_t *path)\n {\n@@ -102,33 +115,39 @@ static const char *get_response_line(void)\n \treturn response_line.buf;\n }\n \n-static int copy_bytes(FILE *out, off_t len)\n+static long apply_delta(uint32_t mark, off_t len, struct line_buffer *input,\n+\t\t\tuint32_t old_mark, uint32_t old_mode)\n {\n-\toff_t ret = fd_copy_bytes(out, REPORT_FILENO, len);\n-\tif (!ret)\n-\t\treturn error(\"read error: file ends early\");\n-\tif (ret <= 0)\n-\t\treturn error(\"cannot copy cat-blob result\");\n-\treturn 0;\n-}\n+\tlong ret;\n+\tstruct view preimage = {REPORT_FILENO, 0, STRBUF_INIT};\n+\tFILE *out;\n \n-static int fast_export_save_blob(FILE *out)\n-{\n-\toff_t len = len;\n-\tconst char *header, *tail;\n-\n-\theader = get_response_line();\n-\tif (!header || parse_cat_response_line(header, &len))\n-\t\treturn -1;\n-\tcopy_bytes(out, len);\n-\n-\t/* Discard trailing newline. */\n-\ttail = get_response_line();\n-\tif (!tail)\n-\t\treturn -1;\n-\tif (*tail)\n-\t\treturn error(\"line following cat-blob response contains garbage: %s\", tail);\n-\treturn 0;\n+\tif (init_postimage() || !(out = buffer_tmpfile_rewind(&postimage)))\n+\t\tdie(\"cannot open temporary file for blob retrieval\");\n+\tif (old_mark) {\n+\t\tconst char *response;\n+\t\toff_t dummy;\n+\t\tprintf(\"cat-blob :%\"PRIu32\"\\n\", old_mark);\n+\t\tfflush(stdout);\n+\t\tresponse = get_response_line();\n+\t\t/* Not necessary, just for robustness */\n+\t\tif (parse_cat_response_line(response, &dummy))\n+\t\t\tdie(\"invalid cat-blob response: %s\", response);\n+\t}\n+\tif (old_mode == REPO_MODE_LNK)\n+\t\tstrbuf_addstr(&preimage.buf, \"link \");\n+\tif (svndiff0_apply(input, len, &preimage, out))\n+\t\tdie(\"cannot apply delta\");\n+\tif (old_mark) {\n+\t\t/* Discard trailing newline from cat-blob-fd. */\n+\t\tconst char *tail = get_response_line();\n+\t\tif (!tail || *tail)\n+\t\t\tdie(\"missing newline after cat-blob response\");\n+\t}\n+\tret = buffer_tmpfile_prepare_to_read(&postimage);\n+\tif (ret < 0)\n+\t\tdie(\"cannot read temporary file for blob retrieval\");\n+\treturn ret;\n }\n \n void fast_export_blob(uint32_t mode, uint32_t mark, uint32_t len, struct line_buffer *input)\n@@ -142,3 +161,20 @@ void fast_export_blob(uint32_t mode, uint32_t mark, uint32_t len, struct line_bu\n \tbuffer_copy_bytes(input, len);\n \tfputc('\\n', stdout);\n }\n+\n+void fast_export_blob_delta(uint32_t mode, uint32_t mark,\n+\t\t\t\tuint32_t old_mode, uint32_t old_mark,\n+\t\t\t\tuint32_t len, struct line_buffer *input)\n+{\n+\tlong postimage_len;\n+\tif (len > maximum_signed_value_of_type(off_t))\n+\t\tdie(\"enormous delta\");\n+\tpostimage_len = apply_delta(mark, (off_t) len, input, old_mark, old_mode);\n+\tif (mode == REPO_MODE_LNK) {\n+\t\tbuffer_skip_bytes(&postimage, strlen(\"link \"));\n+\t\tpostimage_len -= strlen(\"link \");\n+\t}\n+\tprintf(\"blob\\nmark :%\"PRIu32\"\\ndata %ld\\n\", mark, postimage_len);\n+\tbuffer_copy_bytes(&postimage, postimage_len);\n+\tfputc('\\n', stdout);\n+}\ndiff --git a/vcs-svn/fast_export.h b/vcs-svn/fast_export.h\nindex 054e7d5..6f77c3b 100644\n--- a/vcs-svn/fast_export.h\n+++ b/vcs-svn/fast_export.h\n@@ -10,5 +10,8 @@ void fast_export_commit(uint32_t revision, uint32_t author, char *log,\n \t\t\tuint32_t uuid, uint32_t url, unsigned long timestamp);\n void fast_export_blob(uint32_t mode, uint32_t mark, uint32_t len,\n \t\t struct line_buffer *input);\n+void fast_export_blob_delta(uint32_t mode, uint32_t mark,\n+\t\t\tuint32_t old_mode, uint32_t old_mark,\n+\t\t\tuint32_t len, struct line_buffer *input);\n \n #endif\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex 3e7fb9b..a6c849e 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -198,12 +198,10 @@ static void read_props(void)\n \n static void handle_node(void)\n {\n-\tuint32_t mark = 0;\n+\tuint32_t mark = 0, old_mode, old_mark;\n \tconst uint32_t type = node_ctx.type;\n \tconst int have_props = node_ctx.propLength != LENGTH_UNKNOWN;\n \n-\tif (node_ctx.text_delta)\n-\t\tdie(\"text deltas not supported\");\n \tif (node_ctx.textLength != LENGTH_UNKNOWN)\n \t\tmark = next_blob_mark();\n \tif (node_ctx.action == NODEACT_DELETE) {\n@@ -224,7 +222,9 @@ static void handle_node(void)\n \tif (mark && type == REPO_MODE_DIR)\n \t\tdie(\"invalid dump: directories cannot have text attached\");\n \tif (node_ctx.action == NODEACT_CHANGE) {\n-\t\tuint32_t mode = repo_modify_path(node_ctx.dst, 0, mark);\n+\t\tuint32_t mode;\n+\t\told_mark = repo_read_path(node_ctx.dst);\n+\t\tmode = repo_modify_path(node_ctx.dst, 0, mark);\n \t\tif (!mode)\n \t\t\tdie(\"invalid dump: path to be modified is missing\");\n \t\tif (mode == REPO_MODE_DIR && type != REPO_MODE_DIR)\n@@ -236,11 +236,12 @@ static void handle_node(void)\n \t\tif (!mark && type != REPO_MODE_DIR)\n \t\t\tdie(\"invalid dump: adds node without text\");\n \t\trepo_add(node_ctx.dst, type, mark);\n+\t\told_mark = 0;\n \t} else {\n \t\tdie(\"invalid dump: Node-path block lacks Node-action\");\n \t}\n+\told_mode = node_ctx.type;\n \tif (have_props) {\n-\t\tconst uint32_t old_mode = node_ctx.type;\n \t\tif (!node_ctx.prop_delta)\n \t\t\tnode_ctx.type = type;\n \t\tif (node_ctx.propLength)\n@@ -248,8 +249,14 @@ static void handle_node(void)\n \t\tif (node_ctx.type != old_mode)\n \t\t\trepo_modify_path(node_ctx.dst, node_ctx.type, mark);\n \t}\n-\tif (mark)\n+\tif (!mark)\n+\t\treturn;\n+\tif (!node_ctx.text_delta) {\n \t\tfast_export_blob(node_ctx.type, mark, node_ctx.textLength, &input);\n+\t\treturn;\n+\t}\n+\tfast_export_blob_delta(node_ctx.type, mark, old_mode, old_mark,\n+\t\t\t\tnode_ctx.textLength, &input);\n }\n \n static void handle_revision(void)\n-- \n1.7.2.3\n"},{"id":"156264","messageId":"20101120193016.GJ17823@burratino","threadId":"25776","inReplyTo":"20101120192153.GA17823@burratino","subject":"[PATCH 9/8] svn-fe: Test script for handling of dumps with --deltas","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-20T19:30:16Z","receivedAt":"2010-11-20T19:30:16Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Signed-off-by: Jonathan Nieder \n---\n contrib/svn-fe/testme.sh | 16 ++++++++++++++++\n 1 files changed, 16 insertions(+), 0 deletions(-)\n create mode 100755 contrib/svn-fe/testme.sh\n\ndiff --git a/contrib/svn-fe/testme.sh b/contrib/svn-fe/testme.sh\nnew file mode 100755\nindex 0000000..29e181f\n--- /dev/null\n+++ b/contrib/svn-fe/testme.sh\n@@ -0,0 +1,16 @@\n+#!/bin/sh\n+# Usage:\n+#\tPATH=${git_src}/contrib/svn-fe:${git_src}/bin-wrappers:$PATH\n+#\ttestme.sh http://cvs2svn.tigris.org/svn/cvs2svn 10 test.git\n+set -e\n+: ${1?\"URL?\"}\n+: ${2?\"How many revisions?\"}\n+: ${3?\"Git directory?\"}\n+\n+git init --bare \"$3\"\n+rm -f \"$3/backflow\"\n+mkfifo \"$3/backflow\"\n+\n+svnrdump dump -r1:\"$2\" \"$1\" |\n+svn-fe 3backflow\n-- \n1.7.2.3\n"},{"id":"156265","messageId":"20101120193152.GK17823@burratino","threadId":"25776","inReplyTo":"20101120192845.GG17823@burratino","subject":"Re: [PATCH 6/8] vcs-svn: Let caller set up sliding window for delta preimage","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-20T19:31:52Z","receivedAt":"2010-11-20T19:31:52Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n\n> The content of symlinks starts with the word \"link \" in SVN's\n> worldview, so we need to prepend that text the sake of delta\n> application. Move responsibility for setting up the input state from\n> svndiff0_apply to the calling program, so programs have a chance to\n> seed the sliding window with text of their choice.\n> \n> [jn: extracted from the patch \"svn-fe: Use the --report-fd feature\"]\n> \n> Signed-off-by: David Barr \n> Signed-off-by: Jonathan Nieder \n\nThis one is\n\n From: David Barr \n\nSorry, I keep on forgetting to preserve pseudo-header (is there a\nformat-patch option to move it to the body?).\n"},{"id":"156899","messageId":"20101130194833.GA9064@burratino","threadId":"25776","inReplyTo":"20101120005334.GL17445@burratino","subject":"Re: [PATCH 11/15] vcs-svn: More dump format sanity checks","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-30T19:48:33Z","receivedAt":"2010-11-30T19:48:33Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n\n> --- a/vcs-svn/svndump.c\n> +++ b/vcs-svn/svndump.c\n> @@ -181,12 +181,22 @@ static void handle_node(void)\n[...]\n> +\t\tif (mode == REPO_MODE_DIR && type != REPO_MODE_DIR)\n> +\t\t\tdie(\"invalid dump: cannot modify a directory into a file\");\n\nThis is tripping for me in a revision like so:\n\n Revision-number: 692\n Prop-content-length: 110\n Content-length: 110\n\n [revprops snipped]\n\n Node-path: lxanew/DCNsnetwork.cpp\n Node-kind: file\n Node-action: add\n Node-copyfrom-rev: 691\n Node-copyfrom-path: DCNsnetwork.cpp\n\n\n Node-path: DCNsnetwork.cpp\n Node-action: delete\n\nThe import up to and including rev 691 works fine. Rev 691:\n\n Revision-number: 691\n Prop-content-length: 110\n Content-length: 110\n\n[props snipped]\n\n Node-path: lxanew/CorpusWordCollection.cpp\n Node-kind: file\n Node-action: add\n Node-copyfrom-rev: 690\n Node-copyfrom-path: CorpusWordCollection.cpp\n\n\n Node-path: CorpusWordCollection.cpp\n Node-action: delete\n\nPretty similar, right? In the target repository:\n\n $ git ls-tree HEAD -- DCNsnetwork.cpp\n 100644 blob f92d77cc88cc50e9ca333604d22db83f0668828a DCNsnetwork.cpp\n\nDCNsnetwork.cpp was added in r688, which looks like so:\n\n Revision-number: 688\n Prop-content-length: 186\n Content-length: 186\n\n[props and many new files snipped]\n\n Node-path: DCNsnetwork.cpp\n Node-kind: file\n Node-action: add\n Prop-delta: true\n Prop-content-length: 10\n Text-delta: true\n Text-content-length: 2999\n Text-content-md5: 48fd4d31d16f90ab449b4d223fc3d7f8\n Content-length: 3009\n\n PROPS-END\n SVN^@^@^@<97>)^C<97>)<80><97>)// snetwork.cpp: implementation of the snetwork class.\n [...]\n\nSo apparently repo_add failed. Hints?\n\nHere's an unrelated simplification, noticed while debugging.\n\nSigned-off-by: Jonathan Nieder \n---\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex d0d3de6..d1ec9a5 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -219,6 +219,7 @@ static void handle_node(void)\n \t\t\tdie(\"invalid dump: adds node without text\");\n \t\trepo_add(node_ctx.dst, type, mark);\n \t\told_mark = 0;\n+\t\tnode_ctx.type = type;\n \t} else {\n \t\tdie(\"invalid dump: Node-path block lacks Node-action\");\n \t}\n"},{"id":"157307","messageId":"20101204172902.GA28299@burratino","threadId":"25776","inReplyTo":"20101120193016.GJ17823@burratino","subject":"Re: [PATCH 9/8] svn-fe: Test script for handling of dumps with --deltas","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-04T17:29:02Z","receivedAt":"2010-12-04T17:29:02Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n\n> +++ b/contrib/svn-fe/testme.sh\n> @@ -0,0 +1,16 @@\n> +#!/bin/sh\n> +# Usage:\n> +#\tPATH=${git_src}/contrib/svn-fe:${git_src}/bin-wrappers:$PATH\n> +#\ttestme.sh http://cvs2svn.tigris.org/svn/cvs2svn 10 test.git\n> +set -e\n> +: ${1?\"URL?\"}\n> +: ${2?\"How many revisions?\"}\n> +: ${3?\"Git directory?\"}\n> +\n> +git init --bare \"$3\"\n> +rm -f \"$3/backflow\"\n> +mkfifo \"$3/backflow\"\n> +\n> +svnrdump dump -r1:\"$2\" \"$1\" |\n> +svn-fe 3 +GIT_DIR=$3 git fast-import --cat-blob-fd=3 3>backflow\n\nIn case someone was puzzled: backflow should be \"$3/backflow\" on these\nlast two lines.\n\nI didn't notice at first because there was already a fifo available in\nthe cwd during testing.\n"},{"id":"157308","messageId":"20101204173449.GB28299@burratino","threadId":"25776","inReplyTo":"20101120192928.GI17823@burratino","subject":"[PATCH 10/8] vcs-svn: Consume whole preimage when applying deltas","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-04T17:34:49Z","receivedAt":"2010-12-04T17:34:49Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Date: Sun Nov 21 20:24:12 2010 -0600\n\nSome deltas do not consume the entire preimage, so we have to\nconsume it explicitly.\n\nNoticed during imports from the ASF repo.\n\nSigned-off-by: Jonathan Nieder \n---\nJonathan Nieder wrote:\n\n> +++ b/vcs-svn/fast_export.c\n> @@ -102,33 +115,39 @@ static const char *get_response_line(void)\n[...]\n> +\tif (svndiff0_apply(input, len, &preimage, out))\n> +\t\tdie(\"cannot apply delta\");\n> +\tif (old_mark) {\n> +\t\t/* Discard trailing newline from cat-blob-fd. */\n> +\t\tconst char *tail = get_response_line();\n> +\t\tif (!tail || *tail)\n> +\t\t\tdie(\"missing newline after cat-blob response\");\n\nAs mentioned before, this error\n\n\tfatal: missing newline after cat-blob response\n\nand\n\n\terror: Preimage ends early\n\nwere sometimes triggering. Here's a fix for squashing. Sorry for\nthe nonsensical code.\n\n t/t9010-svn-fe.sh | 108 +++++++++++++++++++++++++++++++++++++++++++++++++\n vcs-svn/fast_export.c | 16 ++++---\n 2 files changed, 117 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh\nindex 6c8b803..1776a38 100755\n--- a/t/t9010-svn-fe.sh\n+++ b/t/t9010-svn-fe.sh\n@@ -678,6 +678,114 @@ test_expect_success 'deltas for typechange' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success PIPE 'deltas need not consume the whole preimage' '\n+\treinit_git &&\n+\tcat >expect <<-\\EOF &&\n+\tOBJID\n+\t:120000 100644 OBJID OBJID T\tpostimage\n+\tOBJID\n+\t:100644 120000 OBJID OBJID T\tpostimage\n+\tOBJID\n+\t:000000 100644 OBJID OBJID A\tpostimage\n+\tEOF\n+\techo \"first preimage\" >expect.1 &&\n+\tprintf target >expect.2 &&\n+\tprintf lnk >expect.3 &&\n+\trm -f backflow &&\n+\t{\n+\t\tprintf \"SVNQ%b%b%b\" \"QQ\\017\\001\\017\" \"\\0217\" \"first preimage\\n\" |\n+\t\tq_to_nul\n+\t} >delta.1 &&\n+\t{\n+\t\tproperties svn:special \"*\" &&\n+\t\techo PROPS-END\n+\t} >symlink.props &&\n+\t{\n+\t\tprintf \"SVNQ%b%b%b\" \"Q\\002\\013\\004\\012\" \"\\0201\\001\\001\\0211\" \"lnk target\" |\n+\t\tq_to_nul\n+\t} >delta.2 &&\n+\t{\n+\t\tprintf \"SVNQ%b%b\" \"Q\\004\\003\\004Q\" \"\\001Q\\002\\002\" |\n+\t\tq_to_nul\n+\t} >delta.3 &&\n+\t{\n+\t\tcat <<-\\EOF &&\n+\t\tSVN-fs-dump-format-version: 3\n+\n+\t\tRevision-number: 1\n+\t\tProp-content-length: 10\n+\t\tContent-length: 10\n+\n+\t\tPROPS-END\n+\n+\t\tNode-path: postimage\n+\t\tNode-kind: file\n+\t\tNode-action: add\n+\t\tText-delta: true\n+\t\tProp-content-length: 10\n+\t\tEOF\n+\t\techo Text-content-length: $(wc -c deltapartial.dump &&\n+\tmkfifo backflow &&\n+\ttest-svn-fe deltapartial.dump 3backflow &&\n+\t{\n+\t\tgit rev-list HEAD |\n+\t\tgit diff-tree --root --stdin |\n+\t\tsed \"s/$_x40/OBJID/g\"\n+\t} >actual &&\n+\ttest_cmp expect actual &&\n+\tgit show HEAD:postimage >actual.3 &&\n+\tgit show HEAD^:postimage >actual.2 &&\n+\tgit show HEAD^^:postimage >actual.1 &&\n+\ttest_cmp expect.1 actual.1 &&\n+\ttest_cmp expect.2 actual.2 &&\n+\ttest_cmp expect.3 actual.3\n+'\n+\n test_expect_success 't9135/svn.dump' '\n \tsvnadmin create simple-svn &&\n \tsvnadmin load simple-svn <\"$TEST_DIRECTORY/t9135/svn.dump\" &&\ndiff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c\nindex ceb1fc5..f8a41e7 100644\n--- a/vcs-svn/fast_export.c\n+++ b/vcs-svn/fast_export.c\n@@ -119,6 +119,7 @@ static long apply_delta(uint32_t mark, off_t len, struct line_buffer *input,\n \t\t\tuint32_t old_mark, uint32_t old_mode)\n {\n \tlong ret;\n+\toff_t preimage_len = 0;\n \tstruct view preimage = {REPORT_FILENO, 0, STRBUF_INIT};\n \tFILE *out;\n \n@@ -126,22 +127,23 @@ static long apply_delta(uint32_t mark, off_t len, struct line_buffer *input,\n \t\tdie(\"cannot open temporary file for blob retrieval\");\n \tif (old_mark) {\n \t\tconst char *response;\n-\t\toff_t dummy;\n \t\tprintf(\"cat-blob :%\"PRIu32\"\\n\", old_mark);\n \t\tfflush(stdout);\n \t\tresponse = get_response_line();\n-\t\t/* Not necessary, just for robustness */\n-\t\tif (parse_cat_response_line(response, &dummy))\n+\t\tif (parse_cat_response_line(response, &preimage_len))\n \t\t\tdie(\"invalid cat-blob response: %s\", response);\n \t}\n-\tif (old_mode == REPO_MODE_LNK)\n+\tif (old_mode == REPO_MODE_LNK) {\n \t\tstrbuf_addstr(&preimage.buf, \"link \");\n+\t\tpreimage_len += strlen(\"link \");\n+\t}\n \tif (svndiff0_apply(input, len, &preimage, out))\n \t\tdie(\"cannot apply delta\");\n \tif (old_mark) {\n-\t\t/* Discard trailing newline from cat-blob-fd. */\n-\t\tconst char *tail = get_response_line();\n-\t\tif (!tail || *tail)\n+\t\t/* Read the remainder of preimage and trailing newline. */\n+\t\tif (move_window(&preimage, preimage_len, 1))\n+\t\t\tdie(\"cannot seek to end of input\");\n+\t\tif (preimage.buf.buf[0] != '\\n')\n \t\t\tdie(\"missing newline after cat-blob response\");\n \t}\n \tret = buffer_tmpfile_prepare_to_read(&postimage);\n-- \n1.7.2.4\n"},{"id":"157339","messageId":"20101205093253.GC4332@burratino","threadId":"25776","inReplyTo":"20101205091605.GA4332@burratino","subject":"[PATCH 2/2] vcs-svn: fix intermittent repo_tree corruption","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-05T09:32:53Z","receivedAt":"2010-12-05T09:32:53Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Pointers to directory entries do not remain valid after a call to\ndent_insert.\n\nNoticed in the course of importing a small Subversion repository\n(~1000 revs); after setting up a dirent for a certain path as a\nplaceholder, by luck dent_insert would trigger a realloc that\nshifted around addresses, resulting in an import with that file\nreplaced by a directory.\n\nSigned-off-by: Jonathan Nieder \n---\nTested using the aforementioned small svn repository. Any ideas for a\nmore reliable test to include in the test suite? Maybe some hack to\nforce realloc to always move the memory it controls, like\n\n\tin git-compat-util.h:\n\n\t#ifdef WANT_SKITTISH_REALLOC\n\t#define realloc skittish_realloc\n\t#endif\n\n\tin compat/realloc.c:\n\n\t#undef realloc\n\tvoid *skittish_realloc(void *ptr, size_t size)\n\t{\n\t\tvoid *result, *tmp;\n\t\tdest = malloc(size);\n\t\tptr = realloc(ptr, size);\n\t\tif (!ptr || !dest);\n\t\t\treturn NULL;\n\t\tmemcpy(dest, ptr, size);\n\t\treturn dest;\n\t}\n\n?\n\n vcs-svn/repo_tree.c | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/vcs-svn/repo_tree.c b/vcs-svn/repo_tree.c\nindex e94d91d..e3d1fa3 100644\n--- a/vcs-svn/repo_tree.c\n+++ b/vcs-svn/repo_tree.c\n@@ -131,7 +131,7 @@ static void repo_write_dirent(uint32_t *path, uint32_t mode,\n \t\tif (dent == key) {\n \t\t\tdent->mode = REPO_MODE_DIR;\n \t\t\tdent->content_offset = 0;\n-\t\t\tdent_insert(&dir->entries, dent);\n+\t\t\tdent = dent_insert(&dir->entries, dent);\n \t\t}\n \n \t\tif (dent_offset(dent) < dent_pool.committed) {\n@@ -142,7 +142,7 @@ static void repo_write_dirent(uint32_t *path, uint32_t mode,\n \t\t\tdent->name_offset = name;\n \t\t\tdent->mode = REPO_MODE_DIR;\n \t\t\tdent->content_offset = dir_o;\n-\t\t\tdent_insert(&dir->entries, dent);\n+\t\t\tdent = dent_insert(&dir->entries, dent);\n \t\t}\n \n \t\tdir = repo_dir_from_dirent(dent);\n-- \n1.7.2.4\n"},{"id":"157340","messageId":"20101205093357.GD4332@burratino","threadId":"25776","inReplyTo":"20101130194833.GA9064@burratino","subject":"[PATCH jn/svn-fe-maint 0/2] intermittent repo_tree corruption","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-05T09:33:57Z","receivedAt":"2010-12-05T09:33:57Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"[mailer usage fail. resending to git@vger; sorry for the noise]\n\nHi David,\n\nJonathan Nieder wrote:\n> Jonathan Nieder wrote:\n\n>> +++ b/vcs-svn/svndump.c\n>> @@ -181,12 +181,22 @@ static void handle_node(void)\n> [...]\n>> +\t\tif (mode == REPO_MODE_DIR && type != REPO_MODE_DIR)\n>> +\t\t\tdie(\"invalid dump: cannot modify a directory into a file\");\n>\n> This is tripping for me in a revision like so:\n[...] \n> Node-path: lxanew/DCNsnetwork.cpp\n> Node-kind: file\n> Node-action: add\n[...]\n> DCNsnetwork.cpp was added in r688, which looks like so:\n[...]\n> Node-path: DCNsnetwork.cpp\n> Node-kind: file\n> Node-action: add\n\nThis is a bad one. Currently, repo_copy builds new nodes for all\ndirectories containing the destination directory, like so:\n\n\tfor each path component:\n\t\tfind that node, creating if missing\n\t\tmake sure it is in the active commit (cloning if not)\n\t\tset mode to MODE_DIR and set up the next iteration to look there.\n\tset mode and content based on source path\n\nUnfortunately the \"create if missing\" and \"make sure it is the active\ncommit\" steps call realloc without updating the corresponding\npointers.\n\nsvn-fe2 (as contained in git 1.7.3) and svn-fe3 are both affected,\npresumably. Probably existing testing missed this because the\nrepo_tree pool quickly grows large enough that glibc allocates with\nmmap(), with little need to change addresses within a huge 64-bit\naddress space.\n\nJonathan Nieder (2):\n treap: make treap_insert return inserted node\n vcs-svn: fix intermittent repo_tree corruption\n\n test-treap.c | 11 ++++++++---\n vcs-svn/repo_tree.c | 4 ++--\n vcs-svn/trp.h | 3 ++-\n vcs-svn/trp.txt | 10 ++++++++--\n 4 files changed, 20 insertions(+), 8 deletions(-)\n\n-- \n1.7.2.4\n"},{"id":"157341","messageId":"20101205093517.GE4332@burratino","threadId":"25776","inReplyTo":"20101205093357.GD4332@burratino","subject":"[PATCH 1/2] treap: make treap_insert return inserted node","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-05T09:35:17Z","receivedAt":"2010-12-05T09:35:17Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Suppose I try the following:\n\n\tstruct int_node *node = node_pointer(node_alloc(1));\n\tnode->n = 5;\n\ttreap_insert(&root, node);\n\tprintf(\"%d\\n\", node->n);\n\nUsually the result will be 5. But since treap_insert draws memory\nfrom the node pool, if the caller is unlucky then (1) the node pool\nwill be full and (2) realloc will be forced to move the node pool to\nfind room, so the node address becomes invalid and the result of\ndereferencing it is undefined.\n\nSo we ought to use offsets in preference to pointers for references\nthat would remain valid after a call to treap_insert. Tweak the\nsignature to hint at a certain special case: since the inserted node\ncan change address (though not offset), as a convenience teach\ntreap_insert to return its new address.\n\nSo the motivational example could be fixed by adding \"node =\".\n\n\tstruct int_node *node = node_pointer(node_alloc(1));\n\tnode->n = 5;\n\tnode = treap_insert(&root, node);\n\tprintf(\"%d\\n\", node->n);\n\nBased on a true story.\n\nSigned-off-by: Jonathan Nieder \n---\n[resending to list --- sorry for the mess.]\n\n test-treap.c | 11 ++++++++---\n vcs-svn/trp.h | 3 ++-\n vcs-svn/trp.txt | 10 ++++++++--\n 3 files changed, 18 insertions(+), 6 deletions(-)\n\ndiff --git a/test-treap.c b/test-treap.c\nindex cdba511..ab8c951 100644\n--- a/test-treap.c\n+++ b/test-treap.c\n@@ -38,9 +38,14 @@ int main(int argc, char *argv[])\n \t\tusage(\"test-treap < ints\");\n \n \twhile (strbuf_getline(&sb, stdin, '\\n') != EOF) {\n-\t\titem = node_alloc(1);\n-\t\tstrtonode(node_pointer(item), sb.buf);\n-\t\ttreap_insert(&root, node_pointer(item));\n+\t\tstruct int_node *node = node_pointer(node_alloc(1));\n+\n+\t\titem = node_offset(node);\n+\t\tstrtonode(node, sb.buf);\n+\t\tnode = treap_insert(&root, node_pointer(item));\n+\t\tif (node_offset(node) != item)\n+\t\t\tdie(\"inserted %\"PRIu32\" in place of %\"PRIu32\"\",\n+\t\t\t\tnode_offset(node), item);\n \t}\n \n \titem = node_offset(treap_first(&root));\ndiff --git a/vcs-svn/trp.h b/vcs-svn/trp.h\nindex ee35c68..c32b918 100644\n--- a/vcs-svn/trp.h\n+++ b/vcs-svn/trp.h\n@@ -188,11 +188,12 @@ a_attr uint32_t MAYBE_UNUSED a_pre##insert_recurse(uint32_t cur_node, uint32_t i\n \t\treturn ret; \\\n \t} \\\n } \\\n-a_attr void MAYBE_UNUSED a_pre##insert(struct trp_root *treap, a_type *node) \\\n+a_attr a_type *MAYBE_UNUSED a_pre##insert(struct trp_root *treap, a_type *node) \\\n { \\\n \tuint32_t offset = trpn_offset(a_base, node); \\\n \ttrp_node_new(a_base, a_field, offset); \\\n \ttreap->trp_root = a_pre##insert_recurse(treap->trp_root, offset); \\\n+\treturn trpn_pointer(a_base, offset); \\\n } \\\n a_attr uint32_t MAYBE_UNUSED a_pre##remove_recurse(uint32_t cur_node, uint32_t rem_node) \\\n { \\\ndiff --git a/vcs-svn/trp.txt b/vcs-svn/trp.txt\nindex eb4c191..5ca6b42 100644\n--- a/vcs-svn/trp.txt\n+++ b/vcs-svn/trp.txt\n@@ -21,7 +21,9 @@ The caller:\n \n . Allocates a `struct trp_root` variable and sets it to {~0}.\n \n-. Adds new nodes to the set using `foo_insert`.\n+. Adds new nodes to the set using `foo_insert`. Any pointers\n+ to existing nodes cannot be relied upon any more, so the caller\n+ might retrieve them anew with `foo_pointer`.\n \n . Can find a specific item in the set using `foo_search`.\n \n@@ -73,10 +75,14 @@ int (*cmp)(node_type \\*a, node_type \\*b)\n and returning a value less than, equal to, or greater than zero\n according to the result of comparison.\n \n-void foo_insert(struct trp_root *treap, node_type \\*node)::\n+node_type {asterisk}foo_insert(struct trp_root *treap, node_type \\*node)::\n \n \tInsert node into treap. If inserted multiple times,\n \ta node will appear in the treap multiple times.\n++\n+The return value is the address of the node within the treap,\n+which might differ from `node` if `pool_alloc` had to call\n+`realloc` to expand the pool.\n \n void foo_remove(struct trp_root *treap, node_type \\*node)::\n \n-- \n1.7.2.4\n"},{"id":"157441","messageId":"20101206221931.GA13401@burratino","threadId":"25776","inReplyTo":"20101120005334.GL17445@burratino","subject":"[PATCH jn/svn-fe] vcs-svn: Allow change nodes for root of tree (/)","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-06T22:19:32Z","receivedAt":"2010-12-06T22:19:32Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"It is not uncommon for a svn repository to include change records for\nproperties at the top level of the tracked tree:\n\n\tNode-path: \n\tNode-kind: dir\n\tNode-action: change\n\tProp-delta: true\n\tProp-content-length: 43\n\tContent-length: 43\n\n\tK 10\n\tsvn:ignore\n\tV 11\n\tbuild-area\n\n\tPROPS-END\n\nUnfortunately a recent svn-fe change (vcs-svn: More dump format sanity\nchecks, 2010-11-19) causes such nodes to be rejected with the error\nmessage\n\n\tfatal: invalid dump: path to be modified is missing\n\nThe repo_tree module does not keep a dirent for the root of the tree.\nAdd a block to the dump parser to take care of this case.\n\nSigned-off-by: Jonathan Nieder \n---\n t/t9010-svn-fe.sh | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++++\n vcs-svn/svndump.c | 5 +++-\n 2 files changed, 77 insertions(+), 1 deletions(-)\n\ndiff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh\nindex 7dc0670..04ce97d 100755\n--- a/t/t9010-svn-fe.sh\n+++ b/t/t9010-svn-fe.sh\n@@ -580,6 +580,79 @@ test_expect_success 'property deltas supported' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'properties on /' '\n+\treinit_git &&\n+\tcat <<-\\EOF >expect &&\n+\tOBJID\n+\tOBJID\n+\t:000000 100644 OBJID OBJID A\tgreeting\n+\tEOF\n+\tsed -e \"s/X$//\" <<-\\EOF >changeroot.dump &&\n+\tSVN-fs-dump-format-version: 3\n+\n+\tRevision-number: 1\n+\tProp-content-length: 10\n+\tContent-length: 10\n+\n+\tPROPS-END\n+\n+\tNode-path: greeting\n+\tNode-kind: file\n+\tNode-action: add\n+\tText-content-length: 0\n+\tProp-content-length: 10\n+\tContent-length: 10\n+\n+\tPROPS-END\n+\n+\tRevision-number: 2\n+\tProp-content-length: 10\n+\tContent-length: 10\n+\n+\tPROPS-END\n+\n+\tNode-path: X\n+\tNode-kind: dir\n+\tNode-action: change\n+\tProp-delta: true\n+\tProp-content-length: 43\n+\tContent-length: 43\n+\n+\tK 10\n+\tsvn:ignore\n+\tV 11\n+\tbuild-area\n+\n+\tPROPS-END\n+\tEOF\n+\techo Revision-number: 2 &&\n+\techo Prop-content-length: $(wc -c stream &&\n+\tgit fast-import actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'deltas for typechange' '\n \treinit_git &&\n \tcat >expect <<-\\EOF &&\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex d2a5347..6543b51 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -203,7 +203,10 @@ static void handle_node(void)\n \t}\n \tif (mark && type == REPO_MODE_DIR)\n \t\tdie(\"invalid dump: directories cannot have text attached\");\n-\tif (node_ctx.action == NODEACT_CHANGE) {\n+\tif (node_ctx.action == NODEACT_CHANGE && !~*node_ctx.dst) {\n+\t\tif (type != REPO_MODE_DIR)\n+\t\t\tdie(\"invalid dump: root of tree is not a regular file\");\n+\t} else if (node_ctx.action == NODEACT_CHANGE) {\n \t\tuint32_t mode = repo_modify_path(node_ctx.dst, 0, mark);\n \t\tif (!mode)\n \t\t\tdie(\"invalid dump: path to be modified is missing\");\n-- \n1.7.2.4\n"},{"id":"157455","messageId":"20101206231242.GA16804@burratino","threadId":"25776","inReplyTo":"20101206221931.GA13401@burratino","subject":"Re: [PATCH jn/svn-fe] vcs-svn: Allow change nodes for root of tree (/)","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-06T23:12:43Z","receivedAt":"2010-12-06T23:12:43Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n\n> ---\n> t/t9010-svn-fe.sh | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++++\n> vcs-svn/svndump.c | 5 +++-\n> 2 files changed, 77 insertions(+), 1 deletions(-)\n\nHmph, some extra (but harmless) text snuck into the test. :/\n\n-- 8< --\nSubject: fixup! vcs-svn: Allow change nodes for root of tree (/)\n\nSome pointless, unrelated code stowed away while resolving conflicts\nduring a cherry-pick. Remove it.\n\nSigned-off-by: Jonathan Nieder \n---\n t/t9010-svn-fe.sh | 18 ------------------\n 1 files changed, 0 insertions(+), 18 deletions(-)\n\ndiff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh\nindex 04ce97d..8794507 100755\n--- a/t/t9010-svn-fe.sh\n+++ b/t/t9010-svn-fe.sh\n@@ -625,24 +625,6 @@ test_expect_success 'properties on /' '\n \n \tPROPS-END\n \tEOF\n-\techo Revision-number: 2 &&\n-\techo Prop-content-length: $(wc -c stream &&\n \tgit fast-import repo_modify_path returns the mode.\n>\n> Signed-off-by: Jonathan Nieder \n\nAnother cryptic commit message.\n\nA long-winded explanation follows. Please feel free to ignore it, or\neven better, come up with a summary to use as change description that\npeople can refer to for this patch in the future. :)\n\nIt is currently possible to _modify_ both the mode and the text at a\npath but there is only public functionality to retrieve the mode. In\nits original context, this patch added analogous functionality to\nretrieve the text (indexed by a blob name) as preparation for applying\na delta to it.\n\nIn the vcs-svn-incremental series, it will allow us to restructure\nhandle_node() a little: instead of making little modifications to an\nin-memory cache managed by the repo-tree library (saying, \"change the\nmode this way but keep the content; now keep the mode but change the\ncontent\"), handle_node can take responsibility for figuring out what\nwill be the final mode and content for the path in question.\n\nIn other words, a later patch will change handle_node (in the\nNode-action: change case) from looking like\n\n\t/* Change text at path. */\n\tmode = repo_modify_path(path, 0, have_text ? text : 0);\n\n\t/* Change mode at path. */\n\tif (have_props) {\n\t\tread_props(&new_mode);\n\t\tif (new_mode != mode)\n\t\t\trepo_modify_path(path, new_mode, have_text ? text : 0);\n\t}\n\nto\n\n\t/* Determine desired text and mode at path. */\n\told_text = repo_read_path(path);\n\told_mode = repo_modify_path(path, 0, 0);\n\tif (have_props)\n\t\tread_props(old_mode, &mode);\n\tif (!have_text)\n\t\ttext = old_text;\n\n\t/* Make it so. */\n\trepo_add(path, mode, text);\n\nIt is also just nice to have a function like this, to make the API\nless surprising.\n"}]}