From 1f05d07c456e23c0827efbbb3e738afc9f3152e7 Mon Sep 17 00:00:00 2001 From: David Barr Date: Wed, 17 Nov 2010 23:03:51 -0600 Subject: [PATCH 01/17] vcs-svn: Allow simple v3 dumps (no deltas yet) Since the dumpfile version 1 days, the Subversion dump format gained some new fields: - a unique identifier for the repository (version 2 format) - whether the text and properties for a node should be interpreted as deltas - checksums for a delta's preimage - SHA-1 sums as alternatives to the existing MD5 checksums for copy source and the payload (delta). For now what is relevant to us is the Text-delta and Prop-delta fields, since not noticing these causes a dump file to be misinterpreted (see the previous commit). [jn: with tests] Signed-off-by: David Barr Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t9010-svn-fe.sh | 350 +++++++++++++++++++++++++++++++++++++++++++++- vcs-svn/svndump.c | 21 ++- 2 files changed, 365 insertions(+), 6 deletions(-) diff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh index e9e46ea0fe..be5372ab3b 100755 --- a/t/t9010-svn-fe.sh +++ b/t/t9010-svn-fe.sh @@ -9,6 +9,30 @@ reinit_git () { git init } +properties () { + while test "$#" -ne 0 + do + property="$1" && + value="$2" && + printf "%s\n" "K ${#property}" && + printf "%s\n" "$property" && + printf "%s\n" "V ${#value}" && + printf "%s\n" "$value" && + shift 2 || + return 1 + done +} + +text_no_props () { + text="$1 +" && + printf "%s\n" "Prop-content-length: 10" && + printf "%s\n" "Text-content-length: ${#text}" && + printf "%s\n" "Content-length: $((${#text} + 10))" && + printf "%s\n" "" "PROPS-END" && + printf "%s\n" "$text" +} + >empty test_expect_success 'empty dump' ' @@ -18,13 +42,333 @@ test_expect_success 'empty dump' ' git fast-import input && - test_must_fail test-svn-fe input >stream && + echo "SVN-fs-dump-format-version: 4" >v4.dump && + test_must_fail test-svn-fe v4.dump >stream && test_cmp empty stream ' +test_expect_failure 'empty revision' ' + reinit_git && + printf "rev : %s\n" "" "" >expect && + cat >emptyrev.dump <<-\EOF && + SVN-fs-dump-format-version: 3 + + Revision-number: 1 + Prop-content-length: 0 + Content-length: 0 + + Revision-number: 2 + Prop-content-length: 0 + Content-length: 0 + + EOF + test-svn-fe emptyrev.dump >stream && + git fast-import actual && + test_cmp expect actual +' + +test_expect_success 'empty properties' ' + reinit_git && + printf "rev : %s\n" "" "" >expect && + cat >emptyprop.dump <<-\EOF && + SVN-fs-dump-format-version: 3 + + Revision-number: 1 + Prop-content-length: 10 + Content-length: 10 + + PROPS-END + + Revision-number: 2 + Prop-content-length: 10 + Content-length: 10 + + PROPS-END + EOF + test-svn-fe emptyprop.dump >stream && + git fast-import actual && + test_cmp expect actual +' + +test_expect_success 'author name and commit message' ' + reinit_git && + echo "" >expect.author && + cat >message <<-\EOF && + A concise summary of the change + + A detailed description of the change, why it is needed, what + was broken and why applying this is the best course of action. + + * file.c + Details pertaining to an individual file. + EOF + { + properties \ + svn:author author@example.com \ + svn:log "$(cat message)" && + echo PROPS-END + } >props && + { + echo "SVN-fs-dump-format-version: 3" && + echo && + echo "Revision-number: 1" && + echo Prop-content-length: $(wc -c log.dump && + test-svn-fe log.dump >stream && + git fast-import actual.log && + git log --format="<%an, %ae>" >actual.author && + test_cmp message actual.log && + test_cmp expect.author actual.author +' + +test_expect_success 'unsupported properties are ignored' ' + reinit_git && + echo author >expect && + cat >extraprop.dump <<-\EOF && + SVN-fs-dump-format-version: 3 + + Revision-number: 1 + Prop-content-length: 56 + Content-length: 56 + + K 8 + nonsense + V 1 + y + K 10 + svn:author + V 6 + author + PROPS-END + EOF + test-svn-fe extraprop.dump >stream && + git fast-import actual && + test_cmp expect actual +' + +test_expect_failure 'timestamp and empty file' ' + echo author@example.com >expect.author && + echo 1999-01-01 >expect.date && + echo file >expect.files && + reinit_git && + { + properties \ + svn:author author@example.com \ + svn:date "1999-01-01T00:01:002.000000Z" \ + svn:log "add empty file" && + echo PROPS-END + } >props && + { + cat <<-EOF && + SVN-fs-dump-format-version: 3 + + Revision-number: 1 + EOF + echo Prop-content-length: $(wc -c emptyfile.dump && + test-svn-fe emptyfile.dump >stream && + git fast-import actual.author && + git log --date=short --format=%ad HEAD >actual.date && + git ls-tree -r --name-only HEAD >actual.files && + test_cmp expect.author actual.author && + test_cmp expect.date actual.date && + test_cmp expect.files actual.files && + git checkout HEAD empty-file && + test_cmp empty file +' + +test_expect_success 'directory with files' ' + reinit_git && + printf "%s\n" directory/file1 directory/file2 >expect.files && + echo hi >hi && + echo hello >hello && + { + properties \ + svn:author author@example.com \ + svn:date "1999-02-01T00:01:002.000000Z" \ + svn:log "add directory with some files in it" && + echo PROPS-END + } >props && + { + cat <<-EOF && + SVN-fs-dump-format-version: 3 + + Revision-number: 1 + EOF + echo Prop-content-length: $(wc -c directory.dump && + test-svn-fe directory.dump >stream && + git fast-import actual.files && + git checkout HEAD directory && + test_cmp expect.files actual.files && + test_cmp hello directory/file1 && + test_cmp hi directory/file2 +' + +test_expect_success 'deltas not supported' ' + { + # (old) h + (inline) ello + (old) \n + printf "SVNQ%b%b%s" "Q\003\006\005\004" "\001Q\0204\001\002" "ello" | + q_to_nul + } >delta && + { + properties \ + svn:author author@example.com \ + svn:date "1999-01-05T00:01:002.000000Z" \ + svn:log "add greeting" && + echo PROPS-END + } >props && + { + properties \ + svn:author author@example.com \ + svn:date "1999-01-06T00:01:002.000000Z" \ + svn:log "change it" && + echo PROPS-END + } >props2 && + { + echo SVN-fs-dump-format-version: 3 && + echo && + echo Revision-number: 1 && + echo Prop-content-length: $(wc -c delta.dump && + test_must_fail test-svn-fe delta.dump +' + +test_expect_success 'property deltas not supported' ' + { + properties \ + svn:author author@example.com \ + svn:date "1999-03-06T00:01:002.000000Z" \ + svn:log "make an executable, or chmod -x it" && + echo PROPS-END + } >revprops && + { + echo SVN-fs-dump-format-version: 3 && + echo && + echo Revision-number: 1 && + echo Prop-content-length: $(wc -c propdelta.dump && + test_must_fail test-svn-fe propdelta.dump +' + test_expect_success 't9135/svn.dump' ' svnadmin create simple-svn && svnadmin load simple-svn <"$TEST_DIRECTORY/t9135/svn.dump" && diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c index fa580e62de..6b64c1b857 100644 --- a/vcs-svn/svndump.c +++ b/vcs-svn/svndump.c @@ -42,6 +42,7 @@ static char* log_copy(uint32_t length, char *log) static struct { uint32_t action, propLength, textLength, srcRev, srcMode, mark, type; uint32_t src[REPO_MAX_PATH_DEPTH], dst[REPO_MAX_PATH_DEPTH]; + uint32_t text_delta, prop_delta; } node_ctx; static struct { @@ -58,7 +59,9 @@ static struct { uint32_t svn_log, svn_author, svn_date, svn_executable, svn_special, uuid, revision_number, node_path, node_kind, node_action, node_copyfrom_path, node_copyfrom_rev, text_content_length, - prop_content_length, content_length, svn_fs_dump_format_version; + prop_content_length, content_length, svn_fs_dump_format_version, + /* version 3 format */ + text_delta, prop_delta; } keys; static void reset_node_ctx(char *fname) @@ -72,6 +75,8 @@ static void reset_node_ctx(char *fname) node_ctx.srcMode = 0; pool_tok_seq(REPO_MAX_PATH_DEPTH, node_ctx.dst, "/", fname); node_ctx.mark = 0; + node_ctx.text_delta = 0; + node_ctx.prop_delta = 0; } static void reset_rev_ctx(uint32_t revision) @@ -107,6 +112,9 @@ static void init_keys(void) keys.prop_content_length = pool_intern("Prop-content-length"); keys.content_length = pool_intern("Content-length"); keys.svn_fs_dump_format_version = pool_intern("SVN-fs-dump-format-version"); + /* version 3 format (Subversion 1.1.0) */ + keys.text_delta = pool_intern("Text-delta"); + keys.prop_delta = pool_intern("Prop-delta"); } static void read_props(void) @@ -144,6 +152,9 @@ static void read_props(void) static void handle_node(void) { + if (node_ctx.text_delta || node_ctx.prop_delta) + die("text and property deltas not supported"); + if (node_ctx.propLength != LENGTH_UNKNOWN && node_ctx.propLength) read_props(); @@ -210,8 +221,8 @@ void svndump_read(const char *url) if (key == keys.svn_fs_dump_format_version) { dump_ctx.version = atoi(val); - if (dump_ctx.version > 2) - die("expected svn dump format version <= 2, found %d", + if (dump_ctx.version > 3) + die("expected svn dump format version <= 3, found %d", dump_ctx.version); } else if (key == keys.uuid) { dump_ctx.uuid = pool_intern(val); @@ -255,6 +266,10 @@ void svndump_read(const char *url) node_ctx.textLength = atoi(val); } else if (key == keys.prop_content_length) { node_ctx.propLength = atoi(val); + } else if (key == keys.text_delta) { + node_ctx.text_delta = !strcmp(val, "true"); + } else if (key == keys.prop_delta) { + node_ctx.prop_delta = !strcmp(val, "true"); } else if (key == keys.content_length) { len = atoi(val); buffer_read_line(); From 5c28a8b054cb69a37638b0261fc370422c8fab58 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Fri, 19 Nov 2010 18:46:06 -0600 Subject: [PATCH 02/17] vcs-svn: Check for errors from open() test-svn-fe segfaults when passed a bogus path. Simplify debugging by exiting with a meaningful error message instead. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- contrib/svn-fe/svn-fe.c | 3 ++- test-svn-fe.c | 3 ++- vcs-svn/svndump.c | 6 ++++-- vcs-svn/svndump.h | 2 +- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/contrib/svn-fe/svn-fe.c b/contrib/svn-fe/svn-fe.c index a2677b03e0..35db24f5ea 100644 --- a/contrib/svn-fe/svn-fe.c +++ b/contrib/svn-fe/svn-fe.c @@ -8,7 +8,8 @@ int main(int argc, char **argv) { - svndump_init(NULL); + if (svndump_init(NULL)) + return 1; svndump_read((argc > 1) ? argv[1] : NULL); svndump_deinit(); svndump_reset(); diff --git a/test-svn-fe.c b/test-svn-fe.c index 77cf78abcf..b42ba789b1 100644 --- a/test-svn-fe.c +++ b/test-svn-fe.c @@ -9,7 +9,8 @@ int main(int argc, char *argv[]) { if (argc != 2) usage("test-svn-fe "); - svndump_init(argv[1]); + if (svndump_init(argv[1])) + return 1; svndump_read(NULL); svndump_deinit(); svndump_reset(); diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c index 6b64c1b857..db11851225 100644 --- a/vcs-svn/svndump.c +++ b/vcs-svn/svndump.c @@ -290,14 +290,16 @@ void svndump_read(const char *url) handle_revision(); } -void svndump_init(const char *filename) +int svndump_init(const char *filename) { - buffer_init(filename); + if (buffer_init(filename)) + return error("cannot open %s: %s", filename, strerror(errno)); repo_init(); reset_dump_ctx(~0); reset_rev_ctx(0); reset_node_ctx(NULL); init_keys(); + return 0; } void svndump_deinit(void) diff --git a/vcs-svn/svndump.h b/vcs-svn/svndump.h index 93c412f14a..df9ceb0e8d 100644 --- a/vcs-svn/svndump.h +++ b/vcs-svn/svndump.h @@ -1,7 +1,7 @@ #ifndef SVNDUMP_H_ #define SVNDUMP_H_ -void svndump_init(const char *filename); +int svndump_init(const char *filename); void svndump_read(const char *url); void svndump_deinit(void); void svndump_reset(void); From 1d13e9f600986b7ced8db37a9a9c4967ee7ff9d5 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Fri, 19 Nov 2010 18:46:22 -0600 Subject: [PATCH 03/17] vcs-svn: Eliminate node_ctx.srcRev global The srcRev variable is only used in handle_node(); its purpose is to hold the old mode for a path, to only be used if properties are not being changed. Narrow its scope to make its meaningful lifetime more obvious. No functional change intended. Add some tests as a sanity-check for the simplest case (no renames). Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t9010-svn-fe.sh | 158 ++++++++++++++++++++++++++++++++++++++++++++++ vcs-svn/svndump.c | 15 +++-- 2 files changed, 166 insertions(+), 7 deletions(-) diff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh index be5372ab3b..729e73dddf 100755 --- a/t/t9010-svn-fe.sh +++ b/t/t9010-svn-fe.sh @@ -252,6 +252,164 @@ test_expect_success 'directory with files' ' test_cmp hi directory/file2 ' +test_expect_failure 'change file mode but keep old content' ' + reinit_git && + cat >expect <<-\EOF && + OBJID + :120000 100644 OBJID OBJID T greeting + OBJID + :100644 120000 OBJID OBJID T greeting + OBJID + :000000 100644 OBJID OBJID A greeting + EOF + echo "link hello" >expect.blob && + echo hello >hello && + cat >filemode.dump <<-\EOF && + SVN-fs-dump-format-version: 3 + + Revision-number: 1 + Prop-content-length: 10 + Content-length: 10 + + PROPS-END + + Node-path: greeting + Node-kind: file + Node-action: add + Prop-content-length: 10 + Text-content-length: 11 + Content-length: 21 + + PROPS-END + link hello + + Revision-number: 2 + Prop-content-length: 10 + Content-length: 10 + + PROPS-END + + Node-path: greeting + Node-kind: file + Node-action: change + Prop-content-length: 33 + Content-length: 33 + + K 11 + svn:special + V 1 + * + PROPS-END + + Revision-number: 3 + Prop-content-length: 10 + Content-length: 10 + + PROPS-END + + Node-path: greeting + Node-kind: file + Node-action: change + Prop-content-length: 10 + Content-length: 10 + + PROPS-END + EOF + test-svn-fe filemode.dump >stream && + git fast-import actual && + git show HEAD:greeting >actual.blob && + git show HEAD^:greeting >actual.target && + test_cmp expect actual && + test_cmp expect.blob actual.blob && + test_cmp hello actual.target +' + +test_expect_success 'change file mode and reiterate content' ' + reinit_git && + cat >expect <<-\EOF && + OBJID + :120000 100644 OBJID OBJID T greeting + OBJID + :100644 120000 OBJID OBJID T greeting + OBJID + :000000 100644 OBJID OBJID A greeting + EOF + echo "link hello" >expect.blob && + echo hello >hello && + cat >filemode.dump <<-\EOF && + SVN-fs-dump-format-version: 3 + + Revision-number: 1 + Prop-content-length: 10 + Content-length: 10 + + PROPS-END + + Node-path: greeting + Node-kind: file + Node-action: add + Prop-content-length: 10 + Text-content-length: 11 + Content-length: 21 + + PROPS-END + link hello + + Revision-number: 2 + Prop-content-length: 10 + Content-length: 10 + + PROPS-END + + Node-path: greeting + Node-kind: file + Node-action: change + Prop-content-length: 33 + Text-content-length: 11 + Content-length: 44 + + K 11 + svn:special + V 1 + * + PROPS-END + link hello + + Revision-number: 3 + Prop-content-length: 10 + Content-length: 10 + + PROPS-END + + Node-path: greeting + Node-kind: file + Node-action: change + Prop-content-length: 10 + Text-content-length: 11 + Content-length: 21 + + PROPS-END + link hello + EOF + test-svn-fe filemode.dump >stream && + git fast-import actual && + git show HEAD:greeting >actual.blob && + git show HEAD^:greeting >actual.target && + test_cmp expect actual && + test_cmp expect.blob actual.blob && + test_cmp hello actual.target +' + test_expect_success 'deltas not supported' ' { # (old) h + (inline) ello + (old) \n diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c index db11851225..65bd335aa2 100644 --- a/vcs-svn/svndump.c +++ b/vcs-svn/svndump.c @@ -40,7 +40,7 @@ static char* log_copy(uint32_t length, char *log) } static struct { - uint32_t action, propLength, textLength, srcRev, srcMode, mark, type; + uint32_t action, propLength, textLength, srcRev, mark, type; uint32_t src[REPO_MAX_PATH_DEPTH], dst[REPO_MAX_PATH_DEPTH]; uint32_t text_delta, prop_delta; } node_ctx; @@ -72,7 +72,6 @@ static void reset_node_ctx(char *fname) node_ctx.textLength = LENGTH_UNKNOWN; node_ctx.src[0] = ~0; node_ctx.srcRev = 0; - node_ctx.srcMode = 0; pool_tok_seq(REPO_MAX_PATH_DEPTH, node_ctx.dst, "/", fname); node_ctx.mark = 0; node_ctx.text_delta = 0; @@ -152,6 +151,8 @@ static void read_props(void) static void handle_node(void) { + uint32_t old_mode = 0; + if (node_ctx.text_delta || node_ctx.prop_delta) die("text and property deltas not supported"); @@ -159,7 +160,7 @@ static void handle_node(void) read_props(); if (node_ctx.srcRev) - node_ctx.srcMode = repo_copy(node_ctx.srcRev, node_ctx.src, node_ctx.dst); + old_mode = repo_copy(node_ctx.srcRev, node_ctx.src, node_ctx.dst); if (node_ctx.textLength != LENGTH_UNKNOWN && node_ctx.type != REPO_MODE_DIR) @@ -175,19 +176,19 @@ static void handle_node(void) else if (node_ctx.propLength != LENGTH_UNKNOWN) repo_modify(node_ctx.dst, node_ctx.type, node_ctx.mark); else if (node_ctx.textLength != LENGTH_UNKNOWN) - node_ctx.srcMode = repo_replace(node_ctx.dst, node_ctx.mark); + old_mode = repo_replace(node_ctx.dst, node_ctx.mark); } else if (node_ctx.action == NODEACT_ADD) { if (node_ctx.srcRev && node_ctx.propLength != LENGTH_UNKNOWN) repo_modify(node_ctx.dst, node_ctx.type, node_ctx.mark); else if (node_ctx.srcRev && node_ctx.textLength != LENGTH_UNKNOWN) - node_ctx.srcMode = repo_replace(node_ctx.dst, node_ctx.mark); + old_mode = repo_replace(node_ctx.dst, node_ctx.mark); else if ((node_ctx.type == REPO_MODE_DIR && !node_ctx.srcRev) || node_ctx.textLength != LENGTH_UNKNOWN) repo_add(node_ctx.dst, node_ctx.type, node_ctx.mark); } - if (node_ctx.propLength == LENGTH_UNKNOWN && node_ctx.srcMode) - node_ctx.type = node_ctx.srcMode; + if (node_ctx.propLength == LENGTH_UNKNOWN && old_mode) + node_ctx.type = old_mode; if (node_ctx.mark) fast_export_blob(node_ctx.type, node_ctx.mark, node_ctx.textLength); From da3e217447390d52363989474a5e33bd298ff3ad Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Fri, 19 Nov 2010 18:46:54 -0600 Subject: [PATCH 04/17] vcs-svn: Eliminate node_ctx.mark global The mark variable is only used in handle_node(). Its life is very short and simple: first, a new mark number is allocated if this node has text attached, then that mark is recorded in the in-core tree being built up, and lastly the mark is communicated to fast-import in the stream along with the associated text. A new reader may worry about interaction with other code, especially since mark is not initialized to zero in handle_node() itself. Disperse such worries by making it local. No functional change intended. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- vcs-svn/svndump.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c index 65bd335aa2..1fb7f82bba 100644 --- a/vcs-svn/svndump.c +++ b/vcs-svn/svndump.c @@ -40,7 +40,7 @@ static char* log_copy(uint32_t length, char *log) } static struct { - uint32_t action, propLength, textLength, srcRev, mark, type; + uint32_t action, propLength, textLength, srcRev, type; uint32_t src[REPO_MAX_PATH_DEPTH], dst[REPO_MAX_PATH_DEPTH]; uint32_t text_delta, prop_delta; } node_ctx; @@ -73,7 +73,6 @@ static void reset_node_ctx(char *fname) node_ctx.src[0] = ~0; node_ctx.srcRev = 0; pool_tok_seq(REPO_MAX_PATH_DEPTH, node_ctx.dst, "/", fname); - node_ctx.mark = 0; node_ctx.text_delta = 0; node_ctx.prop_delta = 0; } @@ -151,7 +150,7 @@ static void read_props(void) static void handle_node(void) { - uint32_t old_mode = 0; + uint32_t old_mode = 0, mark = 0; if (node_ctx.text_delta || node_ctx.prop_delta) die("text and property deltas not supported"); @@ -164,7 +163,7 @@ static void handle_node(void) if (node_ctx.textLength != LENGTH_UNKNOWN && node_ctx.type != REPO_MODE_DIR) - node_ctx.mark = next_blob_mark(); + mark = next_blob_mark(); if (node_ctx.action == NODEACT_DELETE) { repo_delete(node_ctx.dst); @@ -172,26 +171,26 @@ static void handle_node(void) node_ctx.action == NODEACT_REPLACE) { if (node_ctx.action == NODEACT_REPLACE && node_ctx.type == REPO_MODE_DIR) - repo_replace(node_ctx.dst, node_ctx.mark); + repo_replace(node_ctx.dst, mark); else if (node_ctx.propLength != LENGTH_UNKNOWN) - repo_modify(node_ctx.dst, node_ctx.type, node_ctx.mark); + repo_modify(node_ctx.dst, node_ctx.type, mark); else if (node_ctx.textLength != LENGTH_UNKNOWN) - old_mode = repo_replace(node_ctx.dst, node_ctx.mark); + old_mode = repo_replace(node_ctx.dst, mark); } else if (node_ctx.action == NODEACT_ADD) { if (node_ctx.srcRev && node_ctx.propLength != LENGTH_UNKNOWN) - repo_modify(node_ctx.dst, node_ctx.type, node_ctx.mark); + repo_modify(node_ctx.dst, node_ctx.type, mark); else if (node_ctx.srcRev && node_ctx.textLength != LENGTH_UNKNOWN) - old_mode = repo_replace(node_ctx.dst, node_ctx.mark); + old_mode = repo_replace(node_ctx.dst, mark); else if ((node_ctx.type == REPO_MODE_DIR && !node_ctx.srcRev) || node_ctx.textLength != LENGTH_UNKNOWN) - repo_add(node_ctx.dst, node_ctx.type, node_ctx.mark); + repo_add(node_ctx.dst, node_ctx.type, mark); } if (node_ctx.propLength == LENGTH_UNKNOWN && old_mode) node_ctx.type = old_mode; - if (node_ctx.mark) - fast_export_blob(node_ctx.type, node_ctx.mark, node_ctx.textLength); + if (mark) + fast_export_blob(node_ctx.type, mark, node_ctx.textLength); else if (node_ctx.textLength != LENGTH_UNKNOWN) buffer_skip_bytes(node_ctx.textLength); } From d6e81a03153810f122f1b8ec3635fd84c5429f69 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Fri, 19 Nov 2010 18:47:41 -0600 Subject: [PATCH 05/17] vcs-svn: Unclutter handle_node by introducing have_props var It is possible for a path node in an SVN-format dump file to leave out the properties section. svn-fe handles this by carrying over the properties (in particular, file type) from the old version of that node. To support this, handle_node tests several times whether a Prop-content-length field is present. Ancient Subversion actually leaves out the Prop-content-length field even for nodes with properties, so that's not quite the right check. Besides, this detail of mechanism is distracting when the question at hand is instead what content the new node should have. So introduce a local have_props variable. The semantics are the same as before; the adaptations to support ancient streams that leave out the prop-content-length can wait until someone needs them. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- vcs-svn/svndump.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c index 1fb7f82bba..45f0e477d7 100644 --- a/vcs-svn/svndump.c +++ b/vcs-svn/svndump.c @@ -151,11 +151,12 @@ static void read_props(void) static void handle_node(void) { uint32_t old_mode = 0, mark = 0; + const int have_props = node_ctx.propLength != LENGTH_UNKNOWN; if (node_ctx.text_delta || node_ctx.prop_delta) die("text and property deltas not supported"); - if (node_ctx.propLength != LENGTH_UNKNOWN && node_ctx.propLength) + if (have_props && node_ctx.propLength) read_props(); if (node_ctx.srcRev) @@ -172,12 +173,12 @@ static void handle_node(void) if (node_ctx.action == NODEACT_REPLACE && node_ctx.type == REPO_MODE_DIR) repo_replace(node_ctx.dst, mark); - else if (node_ctx.propLength != LENGTH_UNKNOWN) + else if (have_props) repo_modify(node_ctx.dst, node_ctx.type, mark); else if (node_ctx.textLength != LENGTH_UNKNOWN) old_mode = repo_replace(node_ctx.dst, mark); } else if (node_ctx.action == NODEACT_ADD) { - if (node_ctx.srcRev && node_ctx.propLength != LENGTH_UNKNOWN) + if (node_ctx.srcRev && have_props) repo_modify(node_ctx.dst, node_ctx.type, mark); else if (node_ctx.srcRev && node_ctx.textLength != LENGTH_UNKNOWN) old_mode = repo_replace(node_ctx.dst, mark); @@ -186,7 +187,7 @@ static void handle_node(void) repo_add(node_ctx.dst, node_ctx.type, mark); } - if (node_ctx.propLength == LENGTH_UNKNOWN && old_mode) + if (!have_props && old_mode) node_ctx.type = old_mode; if (mark) From 462e1f51a5648ce9d7ca26d44ed86327c454889a Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Fri, 19 Nov 2010 18:48:51 -0600 Subject: [PATCH 06/17] vcs-svn: Use mark to indicate nodes with included text Allocate a mark if needed as soon as possible so later code can use "if (mark)" to check if this node has text attached rather than explicitly checking for Text-content-length. While at it, reject directory nodes with text attached; the presence of such a node would indicate a bug in the dump generator or svn-fe's understanding. In the long term, it would be nice to be able to continue parsing and save the error for later, but for now it is simpler to error out right away. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- vcs-svn/svndump.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c index 45f0e477d7..844076b669 100644 --- a/vcs-svn/svndump.c +++ b/vcs-svn/svndump.c @@ -156,15 +156,17 @@ static void handle_node(void) if (node_ctx.text_delta || node_ctx.prop_delta) die("text and property deltas not supported"); + if (node_ctx.textLength != LENGTH_UNKNOWN) + mark = next_blob_mark(); + if (have_props && node_ctx.propLength) read_props(); if (node_ctx.srcRev) old_mode = repo_copy(node_ctx.srcRev, node_ctx.src, node_ctx.dst); - if (node_ctx.textLength != LENGTH_UNKNOWN && - node_ctx.type != REPO_MODE_DIR) - mark = next_blob_mark(); + if (mark && node_ctx.type == REPO_MODE_DIR) + die("invalid dump: directories cannot have text attached"); if (node_ctx.action == NODEACT_DELETE) { repo_delete(node_ctx.dst); @@ -175,15 +177,15 @@ static void handle_node(void) repo_replace(node_ctx.dst, mark); else if (have_props) repo_modify(node_ctx.dst, node_ctx.type, mark); - else if (node_ctx.textLength != LENGTH_UNKNOWN) + else if (mark) old_mode = repo_replace(node_ctx.dst, mark); } else if (node_ctx.action == NODEACT_ADD) { if (node_ctx.srcRev && have_props) repo_modify(node_ctx.dst, node_ctx.type, mark); - else if (node_ctx.srcRev && node_ctx.textLength != LENGTH_UNKNOWN) + else if (node_ctx.srcRev && mark) old_mode = repo_replace(node_ctx.dst, mark); else if ((node_ctx.type == REPO_MODE_DIR && !node_ctx.srcRev) || - node_ctx.textLength != LENGTH_UNKNOWN) + mark) repo_add(node_ctx.dst, node_ctx.type, mark); } @@ -192,8 +194,6 @@ static void handle_node(void) if (mark) fast_export_blob(node_ctx.type, mark, node_ctx.textLength); - else if (node_ctx.textLength != LENGTH_UNKNOWN) - buffer_skip_bytes(node_ctx.textLength); } static void handle_revision(void) From 5af8fae2df03d1888dbf315da29d1cdaa6214f57 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Fri, 19 Nov 2010 18:49:17 -0600 Subject: [PATCH 07/17] vcs-svn: handle_node: Handle deletion case early Take care of "Node-action: delete" as soon as possible, so we can stop worrying about that case in the rest of the function. Functional change: catch deletion nodes with features that would not apply to them (text, properties, or origin data) and error out for those cases. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- vcs-svn/svndump.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c index 844076b669..bc70023073 100644 --- a/vcs-svn/svndump.c +++ b/vcs-svn/svndump.c @@ -159,6 +159,13 @@ static void handle_node(void) if (node_ctx.textLength != LENGTH_UNKNOWN) mark = next_blob_mark(); + if (node_ctx.action == NODEACT_DELETE) { + if (mark || have_props || node_ctx.srcRev) + die("invalid dump: deletion node has " + "copyfrom info, text, or properties"); + return repo_delete(node_ctx.dst); + } + if (have_props && node_ctx.propLength) read_props(); @@ -168,9 +175,7 @@ static void handle_node(void) if (mark && node_ctx.type == REPO_MODE_DIR) die("invalid dump: directories cannot have text attached"); - if (node_ctx.action == NODEACT_DELETE) { - repo_delete(node_ctx.dst); - } else if (node_ctx.action == NODEACT_CHANGE || + if (node_ctx.action == NODEACT_CHANGE || node_ctx.action == NODEACT_REPLACE) { if (node_ctx.action == NODEACT_REPLACE && node_ctx.type == REPO_MODE_DIR) From 6ee4a9be48ee714ddacf313a7073dabdd6c6ee11 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Fri, 19 Nov 2010 18:49:55 -0600 Subject: [PATCH 08/17] vcs-svn: Replace = Delete + Add Simplify by reducing the "Node-action: replace" case to "Node-action: add". This way, the main part of handle_node() only has to deal with "add" and "change" nodes. Functional change: replacing a symlink or executable without setting properties will reset the mode. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- vcs-svn/svndump.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c index bc70023073..6a6aaf92b5 100644 --- a/vcs-svn/svndump.c +++ b/vcs-svn/svndump.c @@ -166,6 +166,11 @@ static void handle_node(void) return repo_delete(node_ctx.dst); } + if (node_ctx.action == NODEACT_REPLACE) { + repo_delete(node_ctx.dst); + node_ctx.action = NODEACT_ADD; + } + if (have_props && node_ctx.propLength) read_props(); @@ -175,12 +180,8 @@ static void handle_node(void) if (mark && node_ctx.type == REPO_MODE_DIR) die("invalid dump: directories cannot have text attached"); - if (node_ctx.action == NODEACT_CHANGE || - node_ctx.action == NODEACT_REPLACE) { - if (node_ctx.action == NODEACT_REPLACE && - node_ctx.type == REPO_MODE_DIR) - repo_replace(node_ctx.dst, mark); - else if (have_props) + if (node_ctx.action == NODEACT_CHANGE) { + if (have_props) repo_modify(node_ctx.dst, node_ctx.type, mark); else if (mark) old_mode = repo_replace(node_ctx.dst, mark); From 08c39b5c44449cb649ac32274e27be8046e373d4 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Fri, 19 Nov 2010 18:51:50 -0600 Subject: [PATCH 09/17] vcs-svn: Combine repo_replace and repo_modify functions There are two functions to change the staged content for a path in the svn importer's active commit: repo_replace, which changes the text and returns the mode, and repo_modify, which changes the text and mode and returns nothing. Worse, there are more subtle differences: - A mark of 0 passed to repo_modify means "use the existing content". repo_replace uses it as mark :0 and produces a corrupt stream. - When passed a path that is not part of the active commit, repo_replace returns without doing anything. repo_modify transparently adds a new directory entry. Get rid of both and introduce a new function with the best features of both: repo_modify_path modifies the mode, content, or both for a path, depending on which arguments are zero. If no such dirent already exists, it does nothing and reports the error by returning 0. Otherwise, the return value is the resulting mode. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- vcs-svn/repo_tree.c | 21 +++++++-------------- vcs-svn/repo_tree.h | 3 +-- vcs-svn/svndump.c | 8 ++++---- 3 files changed, 12 insertions(+), 20 deletions(-) diff --git a/vcs-svn/repo_tree.c b/vcs-svn/repo_tree.c index e94d91d129..7214ac8d0f 100644 --- a/vcs-svn/repo_tree.c +++ b/vcs-svn/repo_tree.c @@ -175,25 +175,18 @@ void repo_add(uint32_t *path, uint32_t mode, uint32_t blob_mark) repo_write_dirent(path, mode, blob_mark, 0); } -uint32_t repo_replace(uint32_t *path, uint32_t blob_mark) -{ - uint32_t mode = 0; - struct repo_dirent *src_dent; - src_dent = repo_read_dirent(active_commit, path); - if (src_dent != NULL) { - mode = src_dent->mode; - repo_write_dirent(path, mode, blob_mark, 0); - } - return mode; -} - -void repo_modify(uint32_t *path, uint32_t mode, uint32_t blob_mark) +uint32_t repo_modify_path(uint32_t *path, uint32_t mode, uint32_t blob_mark) { struct repo_dirent *src_dent; src_dent = repo_read_dirent(active_commit, path); - if (src_dent != NULL && blob_mark == 0) + if (!src_dent) + return 0; + if (!blob_mark) blob_mark = src_dent->content_offset; + if (!mode) + mode = src_dent->mode; repo_write_dirent(path, mode, blob_mark, 0); + return mode; } void repo_delete(uint32_t *path) diff --git a/vcs-svn/repo_tree.h b/vcs-svn/repo_tree.h index 5476175922..68baeb582f 100644 --- a/vcs-svn/repo_tree.h +++ b/vcs-svn/repo_tree.h @@ -14,8 +14,7 @@ uint32_t next_blob_mark(void); uint32_t repo_copy(uint32_t revision, uint32_t *src, uint32_t *dst); void repo_add(uint32_t *path, uint32_t mode, uint32_t blob_mark); -uint32_t repo_replace(uint32_t *path, uint32_t blob_mark); -void repo_modify(uint32_t *path, uint32_t mode, uint32_t blob_mark); +uint32_t repo_modify_path(uint32_t *path, uint32_t mode, uint32_t blob_mark); void repo_delete(uint32_t *path); void repo_commit(uint32_t revision, uint32_t author, char *log, uint32_t uuid, uint32_t url, long unsigned timestamp); diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c index 6a6aaf92b5..e40be580a7 100644 --- a/vcs-svn/svndump.c +++ b/vcs-svn/svndump.c @@ -182,14 +182,14 @@ static void handle_node(void) if (node_ctx.action == NODEACT_CHANGE) { if (have_props) - repo_modify(node_ctx.dst, node_ctx.type, mark); + repo_modify_path(node_ctx.dst, node_ctx.type, mark); else if (mark) - old_mode = repo_replace(node_ctx.dst, mark); + old_mode = repo_modify_path(node_ctx.dst, 0, mark); } else if (node_ctx.action == NODEACT_ADD) { if (node_ctx.srcRev && have_props) - repo_modify(node_ctx.dst, node_ctx.type, mark); + repo_modify_path(node_ctx.dst, node_ctx.type, mark); else if (node_ctx.srcRev && mark) - old_mode = repo_replace(node_ctx.dst, mark); + old_mode = repo_modify_path(node_ctx.dst, 0, mark); else if ((node_ctx.type == REPO_MODE_DIR && !node_ctx.srcRev) || mark) repo_add(node_ctx.dst, node_ctx.type, mark); From 1c7bb316169c700df0d1711555564f86c9cb9366 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Fri, 19 Nov 2010 18:52:28 -0600 Subject: [PATCH 10/17] vcs-svn: Delay read of per-path properties The mode for each file in an svn-format dump is kept in the properties section. The properties section is read as soon as possible to allow the correct mode to be filled in when registering the file with the repo_tree lib. To support nodes with a missing properties section, svn-fe determines the mode in three stages: - The kind (directory or file) of the node is read from the dump and used to make an initial estimate (040000 or 100644). - Properties are read in and allowed to override this for symlinks and executables. - If there is no properties section, the mode from the previous content of the path is left alone, overriding the above considerations. This is a bit of a mess, and worse, it would get even more complicated once we start to support property deltas. If we could only register the file with a provisional value for mode and then change it later when properties say so, the procedure would be much simpler. ... oh, right, we can. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- vcs-svn/svndump.c | 46 +++++++++++++++++++++------------------------- 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c index e40be580a7..4fdfcbbc0d 100644 --- a/vcs-svn/svndump.c +++ b/vcs-svn/svndump.c @@ -150,7 +150,8 @@ static void read_props(void) static void handle_node(void) { - uint32_t old_mode = 0, mark = 0; + uint32_t mark = 0; + const uint32_t type = node_ctx.type; const int have_props = node_ctx.propLength != LENGTH_UNKNOWN; if (node_ctx.text_delta || node_ctx.prop_delta) @@ -171,32 +172,27 @@ static void handle_node(void) node_ctx.action = NODEACT_ADD; } - if (have_props && node_ctx.propLength) - read_props(); - - if (node_ctx.srcRev) - old_mode = repo_copy(node_ctx.srcRev, node_ctx.src, node_ctx.dst); - - if (mark && node_ctx.type == REPO_MODE_DIR) - die("invalid dump: directories cannot have text attached"); - - if (node_ctx.action == NODEACT_CHANGE) { - if (have_props) - repo_modify_path(node_ctx.dst, node_ctx.type, mark); - else if (mark) - old_mode = repo_modify_path(node_ctx.dst, 0, mark); - } else if (node_ctx.action == NODEACT_ADD) { - if (node_ctx.srcRev && have_props) - repo_modify_path(node_ctx.dst, node_ctx.type, mark); - else if (node_ctx.srcRev && mark) - old_mode = repo_modify_path(node_ctx.dst, 0, mark); - else if ((node_ctx.type == REPO_MODE_DIR && !node_ctx.srcRev) || - mark) - repo_add(node_ctx.dst, node_ctx.type, mark); + if (node_ctx.srcRev) { + repo_copy(node_ctx.srcRev, node_ctx.src, node_ctx.dst); + node_ctx.action = NODEACT_CHANGE; } - if (!have_props && old_mode) - node_ctx.type = old_mode; + if (mark && type == REPO_MODE_DIR) + die("invalid dump: directories cannot have text attached"); + + if (node_ctx.action == NODEACT_CHANGE) + node_ctx.type = repo_modify_path(node_ctx.dst, 0, mark); + else /* Node-action: add */ + repo_add(node_ctx.dst, type, mark); + + if (have_props) { + const uint32_t old_mode = node_ctx.type; + node_ctx.type = type; + if (node_ctx.propLength) + read_props(); + if (node_ctx.type != old_mode) + repo_modify_path(node_ctx.dst, node_ctx.type, mark); + } if (mark) fast_export_blob(node_ctx.type, mark, node_ctx.textLength); From 414e569e453a49171b1f3db613f88378324104e8 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Fri, 19 Nov 2010 18:52:59 -0600 Subject: [PATCH 11/17] vcs-svn: Reject path nodes without Node-action It would be better to flag such errors and let the import proceed anyway, but for now it is simpler not to worry about recovery from such weird cases. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t9010-svn-fe.sh | 20 ++++++++++++++++++++ vcs-svn/svndump.c | 7 +++++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh index 729e73dddf..cb9a236245 100755 --- a/t/t9010-svn-fe.sh +++ b/t/t9010-svn-fe.sh @@ -252,6 +252,26 @@ test_expect_success 'directory with files' ' test_cmp hi directory/file2 ' +test_expect_success 'node without action' ' + cat >inaction.dump <<-\EOF && + SVN-fs-dump-format-version: 3 + + Revision-number: 1 + Prop-content-length: 10 + Content-length: 10 + + PROPS-END + + Node-path: directory + Node-kind: dir + Prop-content-length: 10 + Content-length: 10 + + PROPS-END + EOF + test_must_fail test-svn-fe inaction.dump +' + test_expect_failure 'change file mode but keep old content' ' reinit_git && cat >expect <<-\EOF && diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c index 4fdfcbbc0d..0af8ac6807 100644 --- a/vcs-svn/svndump.c +++ b/vcs-svn/svndump.c @@ -174,7 +174,8 @@ static void handle_node(void) if (node_ctx.srcRev) { repo_copy(node_ctx.srcRev, node_ctx.src, node_ctx.dst); - node_ctx.action = NODEACT_CHANGE; + if (node_ctx.action == NODEACT_ADD) + node_ctx.action = NODEACT_CHANGE; } if (mark && type == REPO_MODE_DIR) @@ -182,8 +183,10 @@ static void handle_node(void) if (node_ctx.action == NODEACT_CHANGE) node_ctx.type = repo_modify_path(node_ctx.dst, 0, mark); - else /* Node-action: add */ + else if (node_ctx.action == NODEACT_ADD) repo_add(node_ctx.dst, type, mark); + else + die("invalid dump: Node-path block lacks Node-action"); if (have_props) { const uint32_t old_mode = node_ctx.type; From c7dbf35e91cffbc326078d0c0470662f6422150d Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Fri, 19 Nov 2010 18:53:34 -0600 Subject: [PATCH 12/17] vcs-svn: More dump format sanity checks Node-action: change is not appropriate when switching between file and directory or adding a new file. Current svn-fe silently accepts such nodes and the resulting tree has missing files in the "changed when meant to add" case. Node-action: add requires some content (text or directory); there is no such thing as an "intent to add" node in svn dumps. Current svn-fe accepts such contentless adds but produces an invalid fast-import stream that refers to nonexistent mark :0 in response. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t9010-svn-fe.sh | 21 +++++++++++++++++++++ vcs-svn/svndump.c | 18 ++++++++++++++---- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh index cb9a236245..f1e8799bbd 100755 --- a/t/t9010-svn-fe.sh +++ b/t/t9010-svn-fe.sh @@ -272,6 +272,27 @@ test_expect_success 'node without action' ' test_must_fail test-svn-fe inaction.dump ' +test_expect_success 'action: add node without text' ' + cat >textless.dump <<-\EOF && + SVN-fs-dump-format-version: 3 + + Revision-number: 1 + Prop-content-length: 10 + Content-length: 10 + + PROPS-END + + Node-path: textless + Node-kind: file + Node-action: add + Prop-content-length: 10 + Content-length: 10 + + PROPS-END + EOF + test_must_fail test-svn-fe textless.dump +' + test_expect_failure 'change file mode but keep old content' ' reinit_git && cat >expect <<-\EOF && diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c index 0af8ac6807..ab4ccfc55f 100644 --- a/vcs-svn/svndump.c +++ b/vcs-svn/svndump.c @@ -181,12 +181,22 @@ static void handle_node(void) if (mark && type == REPO_MODE_DIR) die("invalid dump: directories cannot have text attached"); - if (node_ctx.action == NODEACT_CHANGE) - node_ctx.type = repo_modify_path(node_ctx.dst, 0, mark); - else if (node_ctx.action == NODEACT_ADD) + if (node_ctx.action == NODEACT_CHANGE) { + uint32_t mode = repo_modify_path(node_ctx.dst, 0, mark); + if (!mode) + die("invalid dump: path to be modified is missing"); + if (mode == REPO_MODE_DIR && type != REPO_MODE_DIR) + die("invalid dump: cannot modify a directory into a file"); + if (mode != REPO_MODE_DIR && type == REPO_MODE_DIR) + die("invalid dump: cannot modify a file into a directory"); + node_ctx.type = mode; + } else if (node_ctx.action == NODEACT_ADD) { + if (!mark && type != REPO_MODE_DIR) + die("invalid dump: adds node without text"); repo_add(node_ctx.dst, type, mark); - else + } else { die("invalid dump: Node-path block lacks Node-action"); + } if (have_props) { const uint32_t old_mode = node_ctx.type; From 3f3e676d6e6c1d445181107770670368e0ad3160 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Fri, 19 Nov 2010 18:53:54 -0600 Subject: [PATCH 13/17] vcs-svn: Make source easier to read on small screens Remove some newlines from handle_node() that are not needed for clarity. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- vcs-svn/svndump.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c index ab4ccfc55f..153b0c337d 100644 --- a/vcs-svn/svndump.c +++ b/vcs-svn/svndump.c @@ -156,31 +156,25 @@ static void handle_node(void) if (node_ctx.text_delta || node_ctx.prop_delta) die("text and property deltas not supported"); - if (node_ctx.textLength != LENGTH_UNKNOWN) mark = next_blob_mark(); - if (node_ctx.action == NODEACT_DELETE) { if (mark || have_props || node_ctx.srcRev) die("invalid dump: deletion node has " "copyfrom info, text, or properties"); return repo_delete(node_ctx.dst); } - if (node_ctx.action == NODEACT_REPLACE) { repo_delete(node_ctx.dst); node_ctx.action = NODEACT_ADD; } - if (node_ctx.srcRev) { repo_copy(node_ctx.srcRev, node_ctx.src, node_ctx.dst); if (node_ctx.action == NODEACT_ADD) node_ctx.action = NODEACT_CHANGE; } - if (mark && type == REPO_MODE_DIR) die("invalid dump: directories cannot have text attached"); - if (node_ctx.action == NODEACT_CHANGE) { uint32_t mode = repo_modify_path(node_ctx.dst, 0, mark); if (!mode) @@ -197,7 +191,6 @@ static void handle_node(void) } else { die("invalid dump: Node-path block lacks Node-action"); } - if (have_props) { const uint32_t old_mode = node_ctx.type; node_ctx.type = type; @@ -206,7 +199,6 @@ static void handle_node(void) if (node_ctx.type != old_mode) repo_modify_path(node_ctx.dst, node_ctx.type, mark); } - if (mark) fast_export_blob(node_ctx.type, mark, node_ctx.textLength); } From 2a48afe1c256db6273a4ff99eaddc5c18dc46ffd Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Fri, 19 Nov 2010 18:54:20 -0600 Subject: [PATCH 14/17] vcs-svn: Split off function for handling of individual properties The handle_property function is the part of read_props that would be interesting for most people: semantics of properties rather than the algorithm for parsing them. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- vcs-svn/svndump.c | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c index 153b0c337d..5de8dadcdd 100644 --- a/vcs-svn/svndump.c +++ b/vcs-svn/svndump.c @@ -30,7 +30,7 @@ /* Create memory pool for log messages */ obj_pool_gen(log, char, 4096) -static char* log_copy(uint32_t length, char *log) +static char *log_copy(uint32_t length, const char *log) { char *buffer; log_free(log_pool.size); @@ -115,6 +115,23 @@ static void init_keys(void) keys.prop_delta = pool_intern("Prop-delta"); } +static void handle_property(uint32_t key, const char *val, uint32_t len) +{ + if (key == keys.svn_log) { + /* Value length excludes terminating nul. */ + rev_ctx.log = log_copy(len + 1, val); + } else if (key == keys.svn_author) { + rev_ctx.author = pool_intern(val); + } else if (key == keys.svn_date) { + if (parse_date_basic(val, &rev_ctx.timestamp, NULL)) + fprintf(stderr, "Invalid timestamp: %s\n", val); + } else if (key == keys.svn_executable) { + node_ctx.type = REPO_MODE_EXE; + } else if (key == keys.svn_special) { + node_ctx.type = REPO_MODE_LNK; + } +} + static void read_props(void) { uint32_t len; @@ -129,19 +146,7 @@ static void read_props(void) } else if (!strncmp(t, "V ", 2)) { len = atoi(&t[2]); val = buffer_read_string(len); - if (key == keys.svn_log) { - /* Value length excludes terminating nul. */ - rev_ctx.log = log_copy(len + 1, val); - } else if (key == keys.svn_author) { - rev_ctx.author = pool_intern(val); - } else if (key == keys.svn_date) { - if (parse_date_basic(val, &rev_ctx.timestamp, NULL)) - fprintf(stderr, "Invalid timestamp: %s\n", val); - } else if (key == keys.svn_executable) { - node_ctx.type = REPO_MODE_EXE; - } else if (key == keys.svn_special) { - node_ctx.type = REPO_MODE_LNK; - } + handle_property(key, val, len); key = ~0; buffer_read_line(); } From 6263c06d49abdf5e5defdf528c3ff67bf948ac9b Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Fri, 19 Nov 2010 18:54:45 -0600 Subject: [PATCH 15/17] vcs-svn: Sharpen parsing of property lines Prepare to add a new type of property line (the 'D' line) to handle property deltas. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- vcs-svn/svndump.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c index 5de8dadcdd..576d148e5e 100644 --- a/vcs-svn/svndump.c +++ b/vcs-svn/svndump.c @@ -134,21 +134,29 @@ static void handle_property(uint32_t key, const char *val, uint32_t len) static void read_props(void) { - uint32_t len; uint32_t key = ~0; - char *val = NULL; - char *t; + const char *t; while ((t = buffer_read_line()) && strcmp(t, "PROPS-END")) { - if (!strncmp(t, "K ", 2)) { - len = atoi(&t[2]); - key = pool_intern(buffer_read_string(len)); - buffer_read_line(); - } else if (!strncmp(t, "V ", 2)) { - len = atoi(&t[2]); - val = buffer_read_string(len); + uint32_t len; + const char *val; + const char type = t[0]; + + if (!type || t[1] != ' ') + die("invalid property line: %s\n", t); + len = atoi(&t[2]); + val = buffer_read_string(len); + buffer_skip_bytes(1); /* Discard trailing newline. */ + + switch (type) { + case 'K': + key = pool_intern(val); + continue; + case 'V': handle_property(key, val, len); key = ~0; - buffer_read_line(); + continue; + default: + die("invalid property line: %s\n", t); } } } From 6b01b67658e2905b550739f1aee56a00911ca13c Mon Sep 17 00:00:00 2001 From: David Barr Date: Fri, 19 Nov 2010 18:57:46 -0600 Subject: [PATCH 16/17] vcs-svn: Implement Prop-delta handling The rules for what file is used as delta source for each file are not documented in dump-load-format.txt. Luckily, the Apache Software Foundation repository has rich enough examples to figure out most of the rules: Node-action: replace implies the empty property set and empty text as preimage for deltas. Otherwise, if a copyfrom source is given, that node is the preimage for deltas. Lastly, if none of the above applies and the node path exists in the current revision, then that version forms the basis. [jn: refactored, with tests] Signed-off-by: David Barr Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t9010-svn-fe.sh | 102 +++++++++++++++++++++++++++++++++++++++++++++- vcs-svn/svndump.c | 54 +++++++++++++++++++----- 2 files changed, 144 insertions(+), 12 deletions(-) diff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh index f1e8799bbd..7dc06707a1 100755 --- a/t/t9010-svn-fe.sh +++ b/t/t9010-svn-fe.sh @@ -514,7 +514,12 @@ test_expect_success 'deltas not supported' ' test_must_fail test-svn-fe delta.dump ' -test_expect_success 'property deltas not supported' ' +test_expect_success 'property deltas supported' ' + reinit_git && + cat >expect <<-\EOF && + OBJID + :100755 100644 OBJID OBJID M script.sh + EOF { properties \ svn:author author@example.com \ @@ -565,7 +570,100 @@ test_expect_success 'property deltas not supported' ' PROPS-END EOF } >propdelta.dump && - test_must_fail test-svn-fe propdelta.dump + test-svn-fe propdelta.dump >stream && + git fast-import actual && + test_cmp expect actual +' + +test_expect_success 'deltas for typechange' ' + reinit_git && + cat >expect <<-\EOF && + OBJID + :120000 100644 OBJID OBJID T test-file + OBJID + :100755 120000 OBJID OBJID T test-file + OBJID + :000000 100755 OBJID OBJID A test-file + EOF + cat >deleteprop.dump <<-\EOF && + SVN-fs-dump-format-version: 3 + + Revision-number: 1 + Prop-content-length: 10 + Content-length: 10 + + PROPS-END + + Node-path: test-file + Node-kind: file + Node-action: add + Prop-delta: true + Prop-content-length: 35 + Text-content-length: 17 + Content-length: 52 + + K 14 + svn:executable + V 0 + + PROPS-END + link testing 123 + + Revision-number: 2 + Prop-content-length: 10 + Content-length: 10 + + PROPS-END + + Node-path: test-file + Node-kind: file + Node-action: change + Prop-delta: true + Prop-content-length: 53 + Text-content-length: 17 + Content-length: 70 + + K 11 + svn:special + V 1 + * + D 14 + svn:executable + PROPS-END + link testing 231 + + Revision-number: 3 + Prop-content-length: 10 + Content-length: 10 + + PROPS-END + + Node-path: test-file + Node-kind: file + Node-action: change + Prop-delta: true + Prop-content-length: 27 + Text-content-length: 17 + Content-length: 44 + + D 11 + svn:special + PROPS-END + link testing 321 + EOF + test-svn-fe deleteprop.dump >stream && + git fast-import actual && + test_cmp expect actual ' test_expect_success 't9135/svn.dump' ' diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c index 576d148e5e..c71a57599e 100644 --- a/vcs-svn/svndump.c +++ b/vcs-svn/svndump.c @@ -115,20 +115,35 @@ static void init_keys(void) keys.prop_delta = pool_intern("Prop-delta"); } -static void handle_property(uint32_t key, const char *val, uint32_t len) +static void handle_property(uint32_t key, const char *val, uint32_t len, + uint32_t *type_set) { if (key == keys.svn_log) { + if (!val) + die("invalid dump: unsets svn:log"); /* Value length excludes terminating nul. */ rev_ctx.log = log_copy(len + 1, val); } else if (key == keys.svn_author) { rev_ctx.author = pool_intern(val); } else if (key == keys.svn_date) { + if (!val) + die("invalid dump: unsets svn:date"); if (parse_date_basic(val, &rev_ctx.timestamp, NULL)) - fprintf(stderr, "Invalid timestamp: %s\n", val); - } else if (key == keys.svn_executable) { - node_ctx.type = REPO_MODE_EXE; - } else if (key == keys.svn_special) { - node_ctx.type = REPO_MODE_LNK; + warning("invalid timestamp: %s", val); + } else if (key == keys.svn_executable || key == keys.svn_special) { + if (*type_set) { + if (!val) + return; + die("invalid dump: sets type twice"); + } + if (!val) { + node_ctx.type = REPO_MODE_BLB; + return; + } + *type_set = 1; + node_ctx.type = key == keys.svn_executable ? + REPO_MODE_EXE : + REPO_MODE_LNK; } } @@ -136,6 +151,19 @@ static void read_props(void) { uint32_t key = ~0; const char *t; + /* + * NEEDSWORK: to support simple mode changes like + * K 11 + * svn:special + * V 1 + * * + * D 14 + * svn:executable + * we keep track of whether a mode has been set and reset to + * plain file only if not. We should be keeping track of the + * symlink and executable bits separately instead. + */ + uint32_t type_set = 0; while ((t = buffer_read_line()) && strcmp(t, "PROPS-END")) { uint32_t len; const char *val; @@ -151,8 +179,13 @@ static void read_props(void) case 'K': key = pool_intern(val); continue; + case 'D': + key = pool_intern(val); + val = NULL; + len = 0; + /* fall through */ case 'V': - handle_property(key, val, len); + handle_property(key, val, len, &type_set); key = ~0; continue; default: @@ -167,8 +200,8 @@ static void handle_node(void) const uint32_t type = node_ctx.type; const int have_props = node_ctx.propLength != LENGTH_UNKNOWN; - if (node_ctx.text_delta || node_ctx.prop_delta) - die("text and property deltas not supported"); + if (node_ctx.text_delta) + die("text deltas not supported"); if (node_ctx.textLength != LENGTH_UNKNOWN) mark = next_blob_mark(); if (node_ctx.action == NODEACT_DELETE) { @@ -206,7 +239,8 @@ static void handle_node(void) } if (have_props) { const uint32_t old_mode = node_ctx.type; - node_ctx.type = type; + if (!node_ctx.prop_delta) + node_ctx.type = type; if (node_ctx.propLength) read_props(); if (node_ctx.type != old_mode) From 9e8c532108b9078812f23c53a2df3509e7ce71bf Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Mon, 6 Dec 2010 16:19:32 -0600 Subject: [PATCH 17/17] vcs-svn: Allow change nodes for root of tree (/) It is not uncommon for a svn repository to include change records for properties at the top level of the tracked tree: Node-path: Node-kind: dir Node-action: change Prop-delta: true Prop-content-length: 43 Content-length: 43 K 10 svn:ignore V 11 build-area PROPS-END Unfortunately a recent svn-fe change (vcs-svn: More dump format sanity checks, 2010-11-19) causes such nodes to be rejected with the error message fatal: invalid dump: path to be modified is missing The repo_tree module does not keep a dirent for the root of the tree. Add a block to the dump parser to take care of this case. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t9010-svn-fe.sh | 55 +++++++++++++++++++++++++++++++++++++++++++++++ vcs-svn/svndump.c | 5 ++++- 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh index 7dc06707a1..87945073b0 100755 --- a/t/t9010-svn-fe.sh +++ b/t/t9010-svn-fe.sh @@ -580,6 +580,61 @@ test_expect_success 'property deltas supported' ' test_cmp expect actual ' +test_expect_success 'properties on /' ' + reinit_git && + cat <<-\EOF >expect && + OBJID + OBJID + :000000 100644 OBJID OBJID A greeting + EOF + sed -e "s/X$//" <<-\EOF >changeroot.dump && + SVN-fs-dump-format-version: 3 + + Revision-number: 1 + Prop-content-length: 10 + Content-length: 10 + + PROPS-END + + Node-path: greeting + Node-kind: file + Node-action: add + Text-content-length: 0 + Prop-content-length: 10 + Content-length: 10 + + PROPS-END + + Revision-number: 2 + Prop-content-length: 10 + Content-length: 10 + + PROPS-END + + Node-path: X + Node-kind: dir + Node-action: change + Prop-delta: true + Prop-content-length: 43 + Content-length: 43 + + K 10 + svn:ignore + V 11 + build-area + + PROPS-END + EOF + test-svn-fe changeroot.dump >stream && + git fast-import actual && + test_cmp expect actual +' + test_expect_success 'deltas for typechange' ' reinit_git && cat >expect <<-\EOF && diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c index c71a57599e..1669d0fa5e 100644 --- a/vcs-svn/svndump.c +++ b/vcs-svn/svndump.c @@ -221,7 +221,10 @@ static void handle_node(void) } if (mark && type == REPO_MODE_DIR) die("invalid dump: directories cannot have text attached"); - if (node_ctx.action == NODEACT_CHANGE) { + if (node_ctx.action == NODEACT_CHANGE && !~*node_ctx.dst) { + if (type != REPO_MODE_DIR) + die("invalid dump: root of tree is not a regular file"); + } else if (node_ctx.action == NODEACT_CHANGE) { uint32_t mode = repo_modify_path(node_ctx.dst, 0, mark); if (!mode) die("invalid dump: path to be modified is missing");