* [PATCH 01/15] vcs-svn: Check for errors from open()
2010-11-20 0:45 ` [RFC/PATCH 0/15] svn-fe: support for property deltas (but not text " Jonathan Nieder
@ 2010-11-20 0:46 ` Jonathan Nieder
2010-11-20 0:46 ` [PATCH 02/15] vcs-svn: Eliminate node_ctx.srcRev global Jonathan Nieder
` (14 subsequent siblings)
15 siblings, 0 replies; 40+ messages in thread
From: Jonathan Nieder @ 2010-11-20 0:46 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
test-svn-fe segfaults when passed a bogus path. Simplify debugging by
exiting with a meaningful error message instead.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Doesn't belong in this series, just something I noticed while
debugging.
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 a2677b0..35db24f 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 77cf78a..b42ba78 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 <file>");
- 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 6b64c1b..db11851 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 93c412f..df9ceb0 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);
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 02/15] vcs-svn: Eliminate node_ctx.srcRev global
2010-11-20 0:45 ` [RFC/PATCH 0/15] svn-fe: support for property deltas (but not text " Jonathan Nieder
2010-11-20 0:46 ` [PATCH 01/15] vcs-svn: Check for errors from open() Jonathan Nieder
@ 2010-11-20 0:46 ` Jonathan Nieder
2010-11-20 0:46 ` [PATCH 03/15] vcs-svn: Eliminate node_ctx.mark global Jonathan Nieder
` (13 subsequent siblings)
15 siblings, 0 replies; 40+ messages in thread
From: Jonathan Nieder @ 2010-11-20 0:46 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
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 <jrnieder@gmail.com>
---
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 be5372a..729e73d 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 <stream &&
+ {
+ git rev-list HEAD |
+ git diff-tree --root --stdin |
+ sed "s/$_x40/OBJID/g"
+ } >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 <stream &&
+ {
+ git rev-list HEAD |
+ git diff-tree --root --stdin |
+ sed "s/$_x40/OBJID/g"
+ } >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 db11851..65bd335 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);
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 03/15] vcs-svn: Eliminate node_ctx.mark global
2010-11-20 0:45 ` [RFC/PATCH 0/15] svn-fe: support for property deltas (but not text " Jonathan Nieder
2010-11-20 0:46 ` [PATCH 01/15] vcs-svn: Check for errors from open() Jonathan Nieder
2010-11-20 0:46 ` [PATCH 02/15] vcs-svn: Eliminate node_ctx.srcRev global Jonathan Nieder
@ 2010-11-20 0:46 ` Jonathan Nieder
2010-11-20 0:47 ` [PATCH 04/15] vcs-svn: Unclutter handle_node by introducing have_props var Jonathan Nieder
` (12 subsequent siblings)
15 siblings, 0 replies; 40+ messages in thread
From: Jonathan Nieder @ 2010-11-20 0:46 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
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 <jrnieder@gmail.com>
---
vcs-svn/svndump.c | 23 +++++++++++------------
1 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index 65bd335..1fb7f82 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);
}
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 04/15] vcs-svn: Unclutter handle_node by introducing have_props var
2010-11-20 0:45 ` [RFC/PATCH 0/15] svn-fe: support for property deltas (but not text " Jonathan Nieder
` (2 preceding siblings ...)
2010-11-20 0:46 ` [PATCH 03/15] vcs-svn: Eliminate node_ctx.mark global Jonathan Nieder
@ 2010-11-20 0:47 ` Jonathan Nieder
2010-11-20 0:48 ` [PATCH 05/15] vcs-svn: Use mark to indicate nodes with included text Jonathan Nieder
` (11 subsequent siblings)
15 siblings, 0 replies; 40+ messages in thread
From: Jonathan Nieder @ 2010-11-20 0:47 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
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 <jrnieer@gmail.com>
---
vcs-svn/svndump.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index 1fb7f82..45f0e47 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)
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 05/15] vcs-svn: Use mark to indicate nodes with included text
2010-11-20 0:45 ` [RFC/PATCH 0/15] svn-fe: support for property deltas (but not text " Jonathan Nieder
` (3 preceding siblings ...)
2010-11-20 0:47 ` [PATCH 04/15] vcs-svn: Unclutter handle_node by introducing have_props var Jonathan Nieder
@ 2010-11-20 0:48 ` Jonathan Nieder
2010-11-20 0:49 ` [PATCH 06/15] vcs-svn: handle_node: Handle deletion case early Jonathan Nieder
` (10 subsequent siblings)
15 siblings, 0 replies; 40+ messages in thread
From: Jonathan Nieder @ 2010-11-20 0:48 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
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 <jrnieder@gmail.com>
---
vcs-svn/svndump.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index 45f0e47..844076b 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)
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 06/15] vcs-svn: handle_node: Handle deletion case early
2010-11-20 0:45 ` [RFC/PATCH 0/15] svn-fe: support for property deltas (but not text " Jonathan Nieder
` (4 preceding siblings ...)
2010-11-20 0:48 ` [PATCH 05/15] vcs-svn: Use mark to indicate nodes with included text Jonathan Nieder
@ 2010-11-20 0:49 ` Jonathan Nieder
2010-11-20 0:49 ` [PATCH 07/15] vcs-svn: Replace = Delete + Add Jonathan Nieder
` (9 subsequent siblings)
15 siblings, 0 replies; 40+ messages in thread
From: Jonathan Nieder @ 2010-11-20 0:49 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
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 <jrnieder@gmail.com>
---
vcs-svn/svndump.c | 11 ++++++++---
1 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index 844076b..bc70023 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)
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 07/15] vcs-svn: Replace = Delete + Add
2010-11-20 0:45 ` [RFC/PATCH 0/15] svn-fe: support for property deltas (but not text " Jonathan Nieder
` (5 preceding siblings ...)
2010-11-20 0:49 ` [PATCH 06/15] vcs-svn: handle_node: Handle deletion case early Jonathan Nieder
@ 2010-11-20 0:49 ` Jonathan Nieder
2010-11-20 0:51 ` [PATCH 08/15] vcs-svn: Combine repo_replace and repo_modify functions Jonathan Nieder
` (8 subsequent siblings)
15 siblings, 0 replies; 40+ messages in thread
From: Jonathan Nieder @ 2010-11-20 0:49 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
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 <jrnieder@gmail.com>
---
I was happy to discover this.
vcs-svn/svndump.c | 13 +++++++------
1 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index bc70023..6a6aaf9 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);
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 08/15] vcs-svn: Combine repo_replace and repo_modify functions
2010-11-20 0:45 ` [RFC/PATCH 0/15] svn-fe: support for property deltas (but not text " Jonathan Nieder
` (6 preceding siblings ...)
2010-11-20 0:49 ` [PATCH 07/15] vcs-svn: Replace = Delete + Add Jonathan Nieder
@ 2010-11-20 0:51 ` Jonathan Nieder
2010-11-20 0:52 ` [PATCH 09/15] vcs-svn: Delay read of per-path properties Jonathan Nieder
` (7 subsequent siblings)
15 siblings, 0 replies; 40+ messages in thread
From: Jonathan Nieder @ 2010-11-20 0:51 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
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 <jrnieder@gmail.com>
---
David, I think we discussed doing something like this a few months
ago. It seems appropriate because Subversion's Replace FS operation
is not similar to repo_replace after all.
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 e94d91d..7214ac8 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 repo_modify_path(uint32_t *path, uint32_t mode, 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)
-{
- 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 5476175..68baeb5 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 6a6aaf9..e40be58 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);
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 09/15] vcs-svn: Delay read of per-path properties
2010-11-20 0:45 ` [RFC/PATCH 0/15] svn-fe: support for property deltas (but not text " Jonathan Nieder
` (7 preceding siblings ...)
2010-11-20 0:51 ` [PATCH 08/15] vcs-svn: Combine repo_replace and repo_modify functions Jonathan Nieder
@ 2010-11-20 0:52 ` Jonathan Nieder
2010-11-20 0:52 ` [PATCH 10/15] vcs-svn: Reject path nodes without Node-action Jonathan Nieder
` (6 subsequent siblings)
15 siblings, 0 replies; 40+ messages in thread
From: Jonathan Nieder @ 2010-11-20 0:52 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
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 <jrnieder@gmail.com>
---
vcs-svn/svndump.c | 42 +++++++++++++++++++-----------------------
1 files changed, 19 insertions(+), 23 deletions(-)
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index e40be58..4fdfcbb 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) {
+ repo_copy(node_ctx.srcRev, node_ctx.src, node_ctx.dst);
+ node_ctx.action = NODEACT_CHANGE;
+ }
- 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)
+ if (mark && 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.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 && old_mode)
- node_ctx.type = old_mode;
+ 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);
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 10/15] vcs-svn: Reject path nodes without Node-action
2010-11-20 0:45 ` [RFC/PATCH 0/15] svn-fe: support for property deltas (but not text " Jonathan Nieder
` (8 preceding siblings ...)
2010-11-20 0:52 ` [PATCH 09/15] vcs-svn: Delay read of per-path properties Jonathan Nieder
@ 2010-11-20 0:52 ` Jonathan Nieder
2010-11-20 14:53 ` Jonathan Nieder
2010-11-20 0:53 ` [PATCH 11/15] vcs-svn: More dump format sanity checks Jonathan Nieder
` (5 subsequent siblings)
15 siblings, 1 reply; 40+ messages in thread
From: Jonathan Nieder @ 2010-11-20 0:52 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
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 <jrnieder@gmail.com>
---
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 729e73d..cb9a236 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 4fdfcbb..78c2de0 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;
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH 10/15] vcs-svn: Reject path nodes without Node-action
2010-11-20 0:52 ` [PATCH 10/15] vcs-svn: Reject path nodes without Node-action Jonathan Nieder
@ 2010-11-20 14:53 ` Jonathan Nieder
0 siblings, 0 replies; 40+ messages in thread
From: Jonathan Nieder @ 2010-11-20 14:53 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
Jonathan Nieder wrote:
> --- 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;
Whitespace damage. Here's a fixup.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index 78c2de0..0af8ac6 100644
--- a/vcs-svn/svndump.c
+++ b/vcs-svn/svndump.c
@@ -175,7 +175,7 @@ static void handle_node(void)
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;
+ node_ctx.action = NODEACT_CHANGE;
}
if (mark && type == REPO_MODE_DIR)
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH 11/15] vcs-svn: More dump format sanity checks
2010-11-20 0:45 ` [RFC/PATCH 0/15] svn-fe: support for property deltas (but not text " Jonathan Nieder
` (9 preceding siblings ...)
2010-11-20 0:52 ` [PATCH 10/15] vcs-svn: Reject path nodes without Node-action Jonathan Nieder
@ 2010-11-20 0:53 ` Jonathan Nieder
2010-11-30 19:48 ` Jonathan Nieder
2010-12-06 22:19 ` [PATCH jn/svn-fe] vcs-svn: Allow change nodes for root of tree (/) Jonathan Nieder
2010-11-20 0:53 ` [PATCH 12/15] vcs-svn: Make source easier to read on small screens Jonathan Nieder
` (4 subsequent siblings)
15 siblings, 2 replies; 40+ messages in thread
From: Jonathan Nieder @ 2010-11-20 0:53 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
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 <jrnieder@gmail.com>
---
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 cb9a236..f1e8799 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 78c2de0..62bb148 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;
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH 11/15] vcs-svn: More dump format sanity checks
2010-11-20 0:53 ` [PATCH 11/15] vcs-svn: More dump format sanity checks Jonathan Nieder
@ 2010-11-30 19:48 ` Jonathan Nieder
[not found] ` <20101205091605.GA4332@burratino>
2010-12-05 9:33 ` [PATCH jn/svn-fe-maint 0/2] " Jonathan Nieder
2010-12-06 22:19 ` [PATCH jn/svn-fe] vcs-svn: Allow change nodes for root of tree (/) Jonathan Nieder
1 sibling, 2 replies; 40+ messages in thread
From: Jonathan Nieder @ 2010-11-30 19:48 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
Jonathan Nieder wrote:
> --- a/vcs-svn/svndump.c
> +++ b/vcs-svn/svndump.c
> @@ -181,12 +181,22 @@ static void handle_node(void)
[...]
> + if (mode == REPO_MODE_DIR && type != REPO_MODE_DIR)
> + die("invalid dump: cannot modify a directory into a file");
This is tripping for me in a revision like so:
Revision-number: 692
Prop-content-length: 110
Content-length: 110
[revprops snipped]
Node-path: lxanew/DCNsnetwork.cpp
Node-kind: file
Node-action: add
Node-copyfrom-rev: 691
Node-copyfrom-path: DCNsnetwork.cpp
Node-path: DCNsnetwork.cpp
Node-action: delete
The import up to and including rev 691 works fine. Rev 691:
Revision-number: 691
Prop-content-length: 110
Content-length: 110
[props snipped]
Node-path: lxanew/CorpusWordCollection.cpp
Node-kind: file
Node-action: add
Node-copyfrom-rev: 690
Node-copyfrom-path: CorpusWordCollection.cpp
Node-path: CorpusWordCollection.cpp
Node-action: delete
Pretty similar, right? In the target repository:
$ git ls-tree HEAD -- DCNsnetwork.cpp
100644 blob f92d77cc88cc50e9ca333604d22db83f0668828a DCNsnetwork.cpp
DCNsnetwork.cpp was added in r688, which looks like so:
Revision-number: 688
Prop-content-length: 186
Content-length: 186
[props and many new files snipped]
Node-path: DCNsnetwork.cpp
Node-kind: file
Node-action: add
Prop-delta: true
Prop-content-length: 10
Text-delta: true
Text-content-length: 2999
Text-content-md5: 48fd4d31d16f90ab449b4d223fc3d7f8
Content-length: 3009
PROPS-END
SVN^@^@^@<97>)^C<97>)<80><97>)// snetwork.cpp: implementation of the snetwork class.
[...]
So apparently repo_add failed. Hints?
Here's an unrelated simplification, noticed while debugging.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index d0d3de6..d1ec9a5 100644
--- a/vcs-svn/svndump.c
+++ b/vcs-svn/svndump.c
@@ -219,6 +219,7 @@ static void handle_node(void)
die("invalid dump: adds node without text");
repo_add(node_ctx.dst, type, mark);
old_mark = 0;
+ node_ctx.type = type;
} else {
die("invalid dump: Node-path block lacks Node-action");
}
^ permalink raw reply related [flat|nested] 40+ messages in thread[parent not found: <20101205091605.GA4332@burratino>]
* [PATCH 2/2] vcs-svn: fix intermittent repo_tree corruption
[not found] ` <20101205091605.GA4332@burratino>
@ 2010-12-05 9:32 ` Jonathan Nieder
0 siblings, 0 replies; 40+ messages in thread
From: Jonathan Nieder @ 2010-12-05 9:32 UTC (permalink / raw)
To: David Barr; +Cc: git, Ramkumar Ramachandra, Sverre Rabbelier
Pointers to directory entries do not remain valid after a call to
dent_insert.
Noticed in the course of importing a small Subversion repository
(~1000 revs); after setting up a dirent for a certain path as a
placeholder, by luck dent_insert would trigger a realloc that
shifted around addresses, resulting in an import with that file
replaced by a directory.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Tested using the aforementioned small svn repository. Any ideas for a
more reliable test to include in the test suite? Maybe some hack to
force realloc to always move the memory it controls, like
in git-compat-util.h:
#ifdef WANT_SKITTISH_REALLOC
#define realloc skittish_realloc
#endif
in compat/realloc.c:
#undef realloc
void *skittish_realloc(void *ptr, size_t size)
{
void *result, *tmp;
dest = malloc(size);
ptr = realloc(ptr, size);
if (!ptr || !dest);
return NULL;
memcpy(dest, ptr, size);
return dest;
}
?
vcs-svn/repo_tree.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/vcs-svn/repo_tree.c b/vcs-svn/repo_tree.c
index e94d91d..e3d1fa3 100644
--- a/vcs-svn/repo_tree.c
+++ b/vcs-svn/repo_tree.c
@@ -131,7 +131,7 @@ static void repo_write_dirent(uint32_t *path, uint32_t mode,
if (dent == key) {
dent->mode = REPO_MODE_DIR;
dent->content_offset = 0;
- dent_insert(&dir->entries, dent);
+ dent = dent_insert(&dir->entries, dent);
}
if (dent_offset(dent) < dent_pool.committed) {
@@ -142,7 +142,7 @@ static void repo_write_dirent(uint32_t *path, uint32_t mode,
dent->name_offset = name;
dent->mode = REPO_MODE_DIR;
dent->content_offset = dir_o;
- dent_insert(&dir->entries, dent);
+ dent = dent_insert(&dir->entries, dent);
}
dir = repo_dir_from_dirent(dent);
--
1.7.2.4
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH jn/svn-fe-maint 0/2] intermittent repo_tree corruption
2010-11-30 19:48 ` Jonathan Nieder
[not found] ` <20101205091605.GA4332@burratino>
@ 2010-12-05 9:33 ` Jonathan Nieder
2010-12-05 9:35 ` [PATCH 1/2] treap: make treap_insert return inserted node Jonathan Nieder
1 sibling, 1 reply; 40+ messages in thread
From: Jonathan Nieder @ 2010-12-05 9:33 UTC (permalink / raw)
To: David Barr; +Cc: git, Ramkumar Ramachandra, Sverre Rabbelier
[mailer usage fail. resending to git@vger; sorry for the noise]
Hi David,
Jonathan Nieder wrote:
> Jonathan Nieder wrote:
>> +++ b/vcs-svn/svndump.c
>> @@ -181,12 +181,22 @@ static void handle_node(void)
> [...]
>> + if (mode == REPO_MODE_DIR && type != REPO_MODE_DIR)
>> + die("invalid dump: cannot modify a directory into a file");
>
> This is tripping for me in a revision like so:
[...]
> Node-path: lxanew/DCNsnetwork.cpp
> Node-kind: file
> Node-action: add
[...]
> DCNsnetwork.cpp was added in r688, which looks like so:
[...]
> Node-path: DCNsnetwork.cpp
> Node-kind: file
> Node-action: add
This is a bad one. Currently, repo_copy builds new nodes for all
directories containing the destination directory, like so:
for each path component:
find that node, creating if missing
make sure it is in the active commit (cloning if not)
set mode to MODE_DIR and set up the next iteration to look there.
set mode and content based on source path
Unfortunately the "create if missing" and "make sure it is the active
commit" steps call realloc without updating the corresponding
pointers.
svn-fe2 (as contained in git 1.7.3) and svn-fe3 are both affected,
presumably. Probably existing testing missed this because the
repo_tree pool quickly grows large enough that glibc allocates with
mmap(), with little need to change addresses within a huge 64-bit
address space.
Jonathan Nieder (2):
treap: make treap_insert return inserted node
vcs-svn: fix intermittent repo_tree corruption
test-treap.c | 11 ++++++++---
vcs-svn/repo_tree.c | 4 ++--
vcs-svn/trp.h | 3 ++-
vcs-svn/trp.txt | 10 ++++++++--
4 files changed, 20 insertions(+), 8 deletions(-)
--
1.7.2.4
^ permalink raw reply [flat|nested] 40+ messages in thread* [PATCH 1/2] treap: make treap_insert return inserted node
2010-12-05 9:33 ` [PATCH jn/svn-fe-maint 0/2] " Jonathan Nieder
@ 2010-12-05 9:35 ` Jonathan Nieder
0 siblings, 0 replies; 40+ messages in thread
From: Jonathan Nieder @ 2010-12-05 9:35 UTC (permalink / raw)
To: David Barr; +Cc: git, Ramkumar Ramachandra, Sverre Rabbelier
Suppose I try the following:
struct int_node *node = node_pointer(node_alloc(1));
node->n = 5;
treap_insert(&root, node);
printf("%d\n", node->n);
Usually the result will be 5. But since treap_insert draws memory
from the node pool, if the caller is unlucky then (1) the node pool
will be full and (2) realloc will be forced to move the node pool to
find room, so the node address becomes invalid and the result of
dereferencing it is undefined.
So we ought to use offsets in preference to pointers for references
that would remain valid after a call to treap_insert. Tweak the
signature to hint at a certain special case: since the inserted node
can change address (though not offset), as a convenience teach
treap_insert to return its new address.
So the motivational example could be fixed by adding "node =".
struct int_node *node = node_pointer(node_alloc(1));
node->n = 5;
node = treap_insert(&root, node);
printf("%d\n", node->n);
Based on a true story.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
[resending to list --- sorry for the mess.]
test-treap.c | 11 ++++++++---
vcs-svn/trp.h | 3 ++-
vcs-svn/trp.txt | 10 ++++++++--
3 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/test-treap.c b/test-treap.c
index cdba511..ab8c951 100644
--- a/test-treap.c
+++ b/test-treap.c
@@ -38,9 +38,14 @@ int main(int argc, char *argv[])
usage("test-treap < ints");
while (strbuf_getline(&sb, stdin, '\n') != EOF) {
- item = node_alloc(1);
- strtonode(node_pointer(item), sb.buf);
- treap_insert(&root, node_pointer(item));
+ struct int_node *node = node_pointer(node_alloc(1));
+
+ item = node_offset(node);
+ strtonode(node, sb.buf);
+ node = treap_insert(&root, node_pointer(item));
+ if (node_offset(node) != item)
+ die("inserted %"PRIu32" in place of %"PRIu32"",
+ node_offset(node), item);
}
item = node_offset(treap_first(&root));
diff --git a/vcs-svn/trp.h b/vcs-svn/trp.h
index ee35c68..c32b918 100644
--- a/vcs-svn/trp.h
+++ b/vcs-svn/trp.h
@@ -188,11 +188,12 @@ a_attr uint32_t MAYBE_UNUSED a_pre##insert_recurse(uint32_t cur_node, uint32_t i
return ret; \
} \
} \
-a_attr void MAYBE_UNUSED a_pre##insert(struct trp_root *treap, a_type *node) \
+a_attr a_type *MAYBE_UNUSED a_pre##insert(struct trp_root *treap, a_type *node) \
{ \
uint32_t offset = trpn_offset(a_base, node); \
trp_node_new(a_base, a_field, offset); \
treap->trp_root = a_pre##insert_recurse(treap->trp_root, offset); \
+ return trpn_pointer(a_base, offset); \
} \
a_attr uint32_t MAYBE_UNUSED a_pre##remove_recurse(uint32_t cur_node, uint32_t rem_node) \
{ \
diff --git a/vcs-svn/trp.txt b/vcs-svn/trp.txt
index eb4c191..5ca6b42 100644
--- a/vcs-svn/trp.txt
+++ b/vcs-svn/trp.txt
@@ -21,7 +21,9 @@ The caller:
. Allocates a `struct trp_root` variable and sets it to {~0}.
-. Adds new nodes to the set using `foo_insert`.
+. Adds new nodes to the set using `foo_insert`. Any pointers
+ to existing nodes cannot be relied upon any more, so the caller
+ might retrieve them anew with `foo_pointer`.
. Can find a specific item in the set using `foo_search`.
@@ -73,10 +75,14 @@ int (*cmp)(node_type \*a, node_type \*b)
and returning a value less than, equal to, or greater than zero
according to the result of comparison.
-void foo_insert(struct trp_root *treap, node_type \*node)::
+node_type {asterisk}foo_insert(struct trp_root *treap, node_type \*node)::
Insert node into treap. If inserted multiple times,
a node will appear in the treap multiple times.
++
+The return value is the address of the node within the treap,
+which might differ from `node` if `pool_alloc` had to call
+`realloc` to expand the pool.
void foo_remove(struct trp_root *treap, node_type \*node)::
--
1.7.2.4
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH jn/svn-fe] vcs-svn: Allow change nodes for root of tree (/)
2010-11-20 0:53 ` [PATCH 11/15] vcs-svn: More dump format sanity checks Jonathan Nieder
2010-11-30 19:48 ` Jonathan Nieder
@ 2010-12-06 22:19 ` Jonathan Nieder
2010-12-06 23:12 ` Jonathan Nieder
1 sibling, 1 reply; 40+ messages in thread
From: Jonathan Nieder @ 2010-12-06 22:19 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
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 <jrnieder@gmail.com>
---
t/t9010-svn-fe.sh | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++++
vcs-svn/svndump.c | 5 +++-
2 files changed, 77 insertions(+), 1 deletions(-)
diff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh
index 7dc0670..04ce97d 100755
--- a/t/t9010-svn-fe.sh
+++ b/t/t9010-svn-fe.sh
@@ -580,6 +580,79 @@ 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
+ echo Revision-number: 2 &&
+ echo Prop-content-length: $(wc -c <revprops) &&
+ echo Content-length: $(wc -c <revprops) &&
+ echo &&
+ cat revprops &&
+ echo &&
+ cat <<-\EOF
+ Node-path: script.sh
+ Node-kind: file
+ Node-action: change
+ Prop-delta: true
+ Prop-content-length: 30
+ Content-length: 30
+
+ D 14
+ svn:executable
+ PROPS-END
+ EOF
+ test-svn-fe changeroot.dump >stream &&
+ git fast-import <stream &&
+ {
+ git rev-list HEAD |
+ git diff-tree --root --always --stdin |
+ sed "s/$_x40/OBJID/g"
+ } >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 d2a5347..6543b51 100644
--- a/vcs-svn/svndump.c
+++ b/vcs-svn/svndump.c
@@ -203,7 +203,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");
--
1.7.2.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH jn/svn-fe] vcs-svn: Allow change nodes for root of tree (/)
2010-12-06 22:19 ` [PATCH jn/svn-fe] vcs-svn: Allow change nodes for root of tree (/) Jonathan Nieder
@ 2010-12-06 23:12 ` Jonathan Nieder
0 siblings, 0 replies; 40+ messages in thread
From: Jonathan Nieder @ 2010-12-06 23:12 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
Jonathan Nieder wrote:
> ---
> t/t9010-svn-fe.sh | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> vcs-svn/svndump.c | 5 +++-
> 2 files changed, 77 insertions(+), 1 deletions(-)
Hmph, some extra (but harmless) text snuck into the test. :/
-- 8< --
Subject: fixup! vcs-svn: Allow change nodes for root of tree (/)
Some pointless, unrelated code stowed away while resolving conflicts
during a cherry-pick. Remove it.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
t/t9010-svn-fe.sh | 18 ------------------
1 files changed, 0 insertions(+), 18 deletions(-)
diff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh
index 04ce97d..8794507 100755
--- a/t/t9010-svn-fe.sh
+++ b/t/t9010-svn-fe.sh
@@ -625,24 +625,6 @@ test_expect_success 'properties on /' '
PROPS-END
EOF
- echo Revision-number: 2 &&
- echo Prop-content-length: $(wc -c <revprops) &&
- echo Content-length: $(wc -c <revprops) &&
- echo &&
- cat revprops &&
- echo &&
- cat <<-\EOF
- Node-path: script.sh
- Node-kind: file
- Node-action: change
- Prop-delta: true
- Prop-content-length: 30
- Content-length: 30
-
- D 14
- svn:executable
- PROPS-END
- EOF
test-svn-fe changeroot.dump >stream &&
git fast-import <stream &&
{
--
1.7.2.4
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH 12/15] vcs-svn: Make source easier to read on small screens
2010-11-20 0:45 ` [RFC/PATCH 0/15] svn-fe: support for property deltas (but not text " Jonathan Nieder
` (10 preceding siblings ...)
2010-11-20 0:53 ` [PATCH 11/15] vcs-svn: More dump format sanity checks Jonathan Nieder
@ 2010-11-20 0:53 ` Jonathan Nieder
2010-11-20 0:54 ` [PATCH 13/15] vcs-svn: Split off function for handling of individual properties Jonathan Nieder
` (3 subsequent siblings)
15 siblings, 0 replies; 40+ messages in thread
From: Jonathan Nieder @ 2010-11-20 0:53 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
Remove some newlines from handle_node() that are not needed for
clarity.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
vcs-svn/svndump.c | 8 --------
1 files changed, 0 insertions(+), 8 deletions(-)
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index 62bb148..5de32e1 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);
}
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 13/15] vcs-svn: Split off function for handling of individual properties
2010-11-20 0:45 ` [RFC/PATCH 0/15] svn-fe: support for property deltas (but not text " Jonathan Nieder
` (11 preceding siblings ...)
2010-11-20 0:53 ` [PATCH 12/15] vcs-svn: Make source easier to read on small screens Jonathan Nieder
@ 2010-11-20 0:54 ` Jonathan Nieder
2010-11-20 0:54 ` [PATCH 14/15] vcs-svn: Sharpen parsing of property lines Jonathan Nieder
` (2 subsequent siblings)
15 siblings, 0 replies; 40+ messages in thread
From: Jonathan Nieder @ 2010-11-20 0:54 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
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 <jrnieder@gmail.com>
---
vcs-svn/svndump.c | 33 +++++++++++++++++++--------------
1 files changed, 19 insertions(+), 14 deletions(-)
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index 5de32e1..3acc36c 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();
}
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 14/15] vcs-svn: Sharpen parsing of property lines
2010-11-20 0:45 ` [RFC/PATCH 0/15] svn-fe: support for property deltas (but not text " Jonathan Nieder
` (12 preceding siblings ...)
2010-11-20 0:54 ` [PATCH 13/15] vcs-svn: Split off function for handling of individual properties Jonathan Nieder
@ 2010-11-20 0:54 ` Jonathan Nieder
2010-11-20 0:57 ` [PATCH 15/15] vcs-svn: Implement Prop-delta handling Jonathan Nieder
2010-11-20 19:21 ` [WIP/PATCH 0/8] svn-fe: support for text deltas Jonathan Nieder
15 siblings, 0 replies; 40+ messages in thread
From: Jonathan Nieder @ 2010-11-20 0:54 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
Prepare to add a new type of property line (the 'D' line) to
handle property deltas.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
vcs-svn/svndump.c | 30 +++++++++++++++++++-----------
1 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index 3acc36c..c3e1e32 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);
}
}
}
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 15/15] vcs-svn: Implement Prop-delta handling
2010-11-20 0:45 ` [RFC/PATCH 0/15] svn-fe: support for property deltas (but not text " Jonathan Nieder
` (13 preceding siblings ...)
2010-11-20 0:54 ` [PATCH 14/15] vcs-svn: Sharpen parsing of property lines Jonathan Nieder
@ 2010-11-20 0:57 ` Jonathan Nieder
2010-11-20 19:21 ` [WIP/PATCH 0/8] svn-fe: support for text deltas Jonathan Nieder
15 siblings, 0 replies; 40+ messages in thread
From: Jonathan Nieder @ 2010-11-20 0:57 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
From: David Barr <david.barr@cordelta.com>
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 <david.barr@cordelta.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
That's the end of this chapter. Thanks for reading.
In the next chapter, the line_buffer lib will gain the ability to read
from multiple files (based on some patches seen on the list before)
and use this ability to manage the cat-blob-fd and some temporary
files for applying deltas.
Reading from the cat-blob-fd with stdio functions carries a great risk
of deadlock, so we need to find a way to do this without fdopen()
(some routines for doing this are already written and usable, but they
haven't been made into a nice library yet).
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 f1e8799..7dc0670 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 <stream &&
+ {
+ git rev-list HEAD |
+ git diff-tree --stdin |
+ sed "s/$_x40/OBJID/g"
+ } >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 <stream &&
+ {
+ git rev-list HEAD |
+ git diff-tree --root --stdin |
+ sed "s/$_x40/OBJID/g"
+ } >actual &&
+ test_cmp expect actual
'
test_expect_success 't9135/svn.dump' '
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index c3e1e32..8a224ac 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)
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* [WIP/PATCH 0/8] svn-fe: support for text deltas
2010-11-20 0:45 ` [RFC/PATCH 0/15] svn-fe: support for property deltas (but not text " Jonathan Nieder
` (14 preceding siblings ...)
2010-11-20 0:57 ` [PATCH 15/15] vcs-svn: Implement Prop-delta handling Jonathan Nieder
@ 2010-11-20 19:21 ` Jonathan Nieder
2010-11-20 19:22 ` [PATCH 1/8] svn-fe: Prepare for strbuf use Jonathan Nieder
` (8 more replies)
15 siblings, 9 replies; 40+ messages in thread
From: Jonathan Nieder @ 2010-11-20 19:21 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
Jonathan Nieder wrote:
> Jonathan Nieder wrote:
>> This mini-series is part of an effort to get David and Ram's svn
>> import work integrated into mainline git[1].
The next chapter. This adds handling of text deltas. I broke it
in the process, though, as you can see with the attached test script:
$ testme.sh http://svn.apache.org/repos/asf 100 asf.git
[...]
* Dumped revision 35.
error: Preimage ends early
fatal: cannot apply delta
Perhaps you can take this as a puzzle. Where does the patch series
go wrong?
I like to think some of the cleanup will make this more maintainable
in the future, even if it is broken now.
Builds on the db/fast-import-cat-blob[1] and jn/svndiff0[2] topics.
Thoughts, improvements, complaints welcome.
David Barr (2):
vcs-svn: Let caller set up sliding window for delta preimage
vcs-svn: Implement text-delta handling
Jonathan Nieder (6):
svn-fe: Prepare for strbuf use
vcs-svn: Internal fast_export_save_blob helper
vcs-svn: Introduce repo_read_path to check the content at a path
vcs-svn: Introduce fd_buffer library
vcs-svn: Read delta preimage from file descriptor
vcs-svn: Teach line_buffer about temporary files
[1] Doesn't seem to be available on gmane.
The important part (patch 3) is, though:
http://thread.gmane.org/gmane.comp.version-control.git/161730
Should I re-send?
[2] http://thread.gmane.org/gmane.comp.version-control.git/151086/focus=158913
^ permalink raw reply [flat|nested] 40+ messages in thread* [PATCH 1/8] svn-fe: Prepare for strbuf use
2010-11-20 19:21 ` [WIP/PATCH 0/8] svn-fe: support for text deltas Jonathan Nieder
@ 2010-11-20 19:22 ` Jonathan Nieder
2010-11-20 19:25 ` [PATCH 2/8] vcs-svn: Internal fast_export_save_blob helper Jonathan Nieder
` (7 subsequent siblings)
8 siblings, 0 replies; 40+ messages in thread
From: Jonathan Nieder @ 2010-11-20 19:22 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
Linking svn-fe strbuf.o will require sha1_name.o and wrapper.o, hence
sha1_file.o, hence libz, pthreads, and most of libgit. Luckily there
is a separate patch series in flight that would trim down those
dependencies again.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
contrib/svn-fe/Makefile | 15 +++++++--------
1 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/contrib/svn-fe/Makefile b/contrib/svn-fe/Makefile
index 360d8da..9732b03 100644
--- a/contrib/svn-fe/Makefile
+++ b/contrib/svn-fe/Makefile
@@ -8,11 +8,13 @@ CFLAGS = -g -O2 -Wall
LDFLAGS =
ALL_CFLAGS = $(CFLAGS)
ALL_LDFLAGS = $(LDFLAGS)
-EXTLIBS =
+PTHREAD_LIBS = -lpthread
+EXTLIBS = -lz $(PTHREAD_LIBS)
GIT_LIB = ../../libgit.a
VCSSVN_LIB = ../../vcs-svn/lib.a
-LIBS = $(VCSSVN_LIB) $(GIT_LIB) $(EXTLIBS)
+XDIFF_LIB = ../../xdiff/lib.a
+LIBS = $(VCSSVN_LIB) $(GIT_LIB) $(XDIFF_LIB) $(EXTLIBS)
QUIET_SUBDIR0 = +$(MAKE) -C # space to separate -C and subdir
QUIET_SUBDIR1 =
@@ -33,7 +35,7 @@ ifndef V
endif
endif
-svn-fe$X: svn-fe.o $(VCSSVN_LIB) $(GIT_LIB)
+svn-fe$X: svn-fe.o $(VCSSVN_LIB) $(GIT_LIB) $(XDIFF_LIB)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ svn-fe.o \
$(ALL_LDFLAGS) $(LIBS)
@@ -51,11 +53,8 @@ svn-fe.1: svn-fe.txt
../contrib/svn-fe/$@
$(MV) ../../Documentation/svn-fe.1 .
-../../vcs-svn/lib.a: FORCE
- $(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) vcs-svn/lib.a
-
-../../libgit.a: FORCE
- $(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) libgit.a
+../../%.a: FORCE
+ $(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) $*.a
clean:
$(RM) svn-fe$X svn-fe.o svn-fe.html svn-fe.xml svn-fe.1
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 2/8] vcs-svn: Internal fast_export_save_blob helper
2010-11-20 19:21 ` [WIP/PATCH 0/8] svn-fe: support for text deltas Jonathan Nieder
2010-11-20 19:22 ` [PATCH 1/8] svn-fe: Prepare for strbuf use Jonathan Nieder
@ 2010-11-20 19:25 ` Jonathan Nieder
2010-11-20 19:25 ` [PATCH 3/8] vcs-svn: Introduce repo_read_path to check the content at a path Jonathan Nieder
` (6 subsequent siblings)
8 siblings, 0 replies; 40+ messages in thread
From: Jonathan Nieder @ 2010-11-20 19:25 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
Introduce fast_export_save_blob, meant to be used after printing
cat-blob :n
to the fast-import stream. It reads a response from fd 3 in cat-file
--batch format. To avoid deadlock, it uses file descriptor-level
calls (no stdio) and reads only one character at a time (though the
latter restriction could and should be relaxed somehow --- O_NONBLOCK,
perhaps).
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
The effect of this one is almost completely undone by a later patch. :)
Probably it would be better to use stdio with O_NONBLOCK, after all
(David, sorry I forgot about our conversations on this before). This
series uses file descriptors to be conservative, because I do not have
much of a desire to test for proper O_NONBLOCK support on the relevant
platforms yet.
vcs-svn/fast_export.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 87 insertions(+), 0 deletions(-)
diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c
index 6cfa256..3feef66 100644
--- a/vcs-svn/fast_export.c
+++ b/vcs-svn/fast_export.c
@@ -8,8 +8,10 @@
#include "line_buffer.h"
#include "repo_tree.h"
#include "string_pool.h"
+#include "strbuf.h"
#define MAX_GITSVN_LINE_LEN 4096
+#define REPORT_FILENO 3
static uint32_t first_commit_done;
@@ -63,6 +65,91 @@ void fast_export_commit(uint32_t revision, uint32_t author, char *log,
printf("progress Imported commit %"PRIu32".\n\n", revision);
}
+static int ends_with(const char *s, size_t len, const char *suffix)
+{
+ const size_t suffixlen = strlen(suffix);
+ if (len < suffixlen)
+ return 0;
+ return !memcmp(s + len - suffixlen, suffix, suffixlen);
+}
+
+static int parse_cat_response_line(const char *header, size_t *len)
+{
+ size_t headerlen = strlen(header);
+ const char *type;
+ const char *end;
+
+ if (ends_with(header, headerlen, " missing"))
+ return error("cat-blob reports missing blob: %s", header);
+ type = memmem(header, headerlen, " blob ", strlen(" blob "));
+ if (!type)
+ return error("cat-blob header has wrong object type: %s", header);
+ *len = strtoumax(type + strlen(" blob "), (char **) &end, 10);
+ if (end == type + strlen(" blob "))
+ return error("cat-blob header does not contain length: %s", header);
+ if (*end)
+ return error("cat-blob header contains garbage after length: %s", header);
+ return 0;
+}
+
+static struct strbuf response_line = STRBUF_INIT;
+static const char *get_response_line(void)
+{
+ /*
+ * NEEDSWORK: Does not actually need to read one byte at a time.
+ * Some platforms have O_NONBLOCK. On others we could read 8 chars
+ * at a time until a potential appearance of " blob ".
+ */
+ strbuf_reset(&response_line);
+ for (;;) {
+ char buf[1];
+ if (xread(REPORT_FILENO, buf, 1) < 0) {
+ error("cannot read cat-blob result: %s", strerror(errno));
+ return NULL;
+ }
+ if (*buf == '\n')
+ return response_line.buf;
+ strbuf_addch(&response_line, *buf);
+ }
+}
+
+static int copy_bytes(FILE *out, size_t len)
+{
+ char buf[4096];
+ ssize_t nread;
+ for (; len; len -= nread) {
+ nread = xread(REPORT_FILENO, buf,
+ len < sizeof(buf) ? len : sizeof(buf));
+ if (nread < 0)
+ return error("cannot copy cat-blob result: %s",
+ strerror(errno));
+ if (!nread)
+ return error("0-length read...");
+ if (fwrite(buf, 1, nread, out) != nread)
+ return error("cannot write cat-blob results: %s",
+ strerror(errno));
+ }
+}
+
+static int fast_export_save_blob(FILE *out)
+{
+ size_t len = len;
+ const char *header, *tail;
+
+ header = get_response_line();
+ if (!header || parse_cat_response_line(header, &len))
+ return -1;
+ copy_bytes(out, len);
+
+ /* Discard trailing newline. */
+ tail = get_response_line();
+ if (!tail)
+ return -1;
+ if (*tail)
+ return error("line following cat-blob response contains garbage: %s", tail);
+ return 0;
+}
+
void fast_export_blob(uint32_t mode, uint32_t mark, uint32_t len)
{
if (mode == REPO_MODE_LNK) {
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 3/8] vcs-svn: Introduce repo_read_path to check the content at a path
2010-11-20 19:21 ` [WIP/PATCH 0/8] svn-fe: support for text deltas Jonathan Nieder
2010-11-20 19:22 ` [PATCH 1/8] svn-fe: Prepare for strbuf use Jonathan Nieder
2010-11-20 19:25 ` [PATCH 2/8] vcs-svn: Internal fast_export_save_blob helper Jonathan Nieder
@ 2010-11-20 19:25 ` Jonathan Nieder
2011-03-06 12:29 ` Jonathan Nieder
2010-11-20 19:26 ` [PATCH 4/8] vcs-svn: Introduce fd_buffer routines Jonathan Nieder
` (5 subsequent siblings)
8 siblings, 1 reply; 40+ messages in thread
From: Jonathan Nieder @ 2010-11-20 19:25 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
repo_modify_path returns the mode.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
vcs-svn/repo_tree.c | 9 +++++++++
vcs-svn/repo_tree.h | 1 +
2 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/vcs-svn/repo_tree.c b/vcs-svn/repo_tree.c
index 7214ac8..eb55636 100644
--- a/vcs-svn/repo_tree.c
+++ b/vcs-svn/repo_tree.c
@@ -157,6 +157,15 @@ static void repo_write_dirent(uint32_t *path, uint32_t mode,
dent_remove(&dir_pointer(parent_dir_o)->entries, dent);
}
+uint32_t repo_read_path(uint32_t *path)
+{
+ uint32_t content_offset = 0;
+ struct repo_dirent *dent = repo_read_dirent(active_commit, path);
+ if (dent != NULL)
+ content_offset = dent->content_offset;
+ return content_offset;
+}
+
uint32_t repo_copy(uint32_t revision, uint32_t *src, uint32_t *dst)
{
uint32_t mode = 0, content_offset = 0;
diff --git a/vcs-svn/repo_tree.h b/vcs-svn/repo_tree.h
index 68baeb5..7070839 100644
--- a/vcs-svn/repo_tree.h
+++ b/vcs-svn/repo_tree.h
@@ -15,6 +15,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_modify_path(uint32_t *path, uint32_t mode, uint32_t blob_mark);
+uint32_t repo_read_path(uint32_t *path);
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);
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH 3/8] vcs-svn: Introduce repo_read_path to check the content at a path
2010-11-20 19:25 ` [PATCH 3/8] vcs-svn: Introduce repo_read_path to check the content at a path Jonathan Nieder
@ 2011-03-06 12:29 ` Jonathan Nieder
0 siblings, 0 replies; 40+ messages in thread
From: Jonathan Nieder @ 2011-03-06 12:29 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
Hi,
Jonathan Nieder wrote:
> repo_modify_path returns the mode.
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Another cryptic commit message.
A long-winded explanation follows. Please feel free to ignore it, or
even better, come up with a summary to use as change description that
people can refer to for this patch in the future. :)
It is currently possible to _modify_ both the mode and the text at a
path but there is only public functionality to retrieve the mode. In
its original context, this patch added analogous functionality to
retrieve the text (indexed by a blob name) as preparation for applying
a delta to it.
In the vcs-svn-incremental series, it will allow us to restructure
handle_node() a little: instead of making little modifications to an
in-memory cache managed by the repo-tree library (saying, "change the
mode this way but keep the content; now keep the mode but change the
content"), handle_node can take responsibility for figuring out what
will be the final mode and content for the path in question.
In other words, a later patch will change handle_node (in the
Node-action: change case) from looking like
/* Change text at path. */
mode = repo_modify_path(path, 0, have_text ? text : 0);
/* Change mode at path. */
if (have_props) {
read_props(&new_mode);
if (new_mode != mode)
repo_modify_path(path, new_mode, have_text ? text : 0);
}
to
/* Determine desired text and mode at path. */
old_text = repo_read_path(path);
old_mode = repo_modify_path(path, 0, 0);
if (have_props)
read_props(old_mode, &mode);
if (!have_text)
text = old_text;
/* Make it so. */
repo_add(path, mode, text);
It is also just nice to have a function like this, to make the API
less surprising.
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 4/8] vcs-svn: Introduce fd_buffer routines
2010-11-20 19:21 ` [WIP/PATCH 0/8] svn-fe: support for text deltas Jonathan Nieder
` (2 preceding siblings ...)
2010-11-20 19:25 ` [PATCH 3/8] vcs-svn: Introduce repo_read_path to check the content at a path Jonathan Nieder
@ 2010-11-20 19:26 ` Jonathan Nieder
2010-11-20 19:27 ` [PATCH 5/8] vcs-svn: Read delta preimage from file descriptor Jonathan Nieder
` (4 subsequent siblings)
8 siblings, 0 replies; 40+ messages in thread
From: Jonathan Nieder @ 2010-11-20 19:26 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
The fd_buffer library is perhaps poorly named, because it does not
manage its own buffer. These functions provide the relevant
functionality from line_buffer for low-level, unbuffered (file
descriptor) I/O.
The purpose is to preserve convenience while avoiding deadlock that
could occur from too greedily reading ahead in the cat-blob-fd pipe.
(An alternative approach to achieve the same effect would be to
set the O_NONBLOCK flag on the pipe fd.)
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Makefile | 5 ++-
vcs-svn/fast_export.c | 45 ++++++++++------------------------
vcs-svn/fd_buffer.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++
vcs-svn/fd_buffer.h | 19 ++++++++++++++
4 files changed, 100 insertions(+), 34 deletions(-)
create mode 100644 vcs-svn/fd_buffer.c
create mode 100644 vcs-svn/fd_buffer.h
diff --git a/Makefile b/Makefile
index aa10288..844e3b4 100644
--- a/Makefile
+++ b/Makefile
@@ -1760,7 +1760,7 @@ ifndef NO_CURL
endif
XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \
xdiff/xmerge.o xdiff/xpatience.o
-VCSSVN_OBJS = vcs-svn/string_pool.o vcs-svn/line_buffer.o \
+VCSSVN_OBJS = vcs-svn/string_pool.o vcs-svn/line_buffer.o vcs-svn/fd_buffer.o \
vcs-svn/repo_tree.o vcs-svn/fast_export.o vcs-svn/svndump.o \
vcs-svn/sliding_window.o vcs-svn/svndiff.o
OBJECTS := $(GIT_OBJS) $(XDIFF_OBJS) $(VCSSVN_OBJS)
@@ -1889,7 +1889,8 @@ xdiff-interface.o $(XDIFF_OBJS): \
$(VCSSVN_OBJS): \
vcs-svn/obj_pool.h vcs-svn/trp.h vcs-svn/string_pool.h \
vcs-svn/line_buffer.h vcs-svn/repo_tree.h vcs-svn/fast_export.h \
- vcs-svn/sliding_window.h vcs-svn/svndump.h vcs-svn/svndiff.h
+ vcs-svn/sliding_window.h vcs-svn/svndump.h vcs-svn/svndiff.h \
+ vcs-svn/fd_buffer.h
endif
exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \
diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c
index 0de498b..ebaab72 100644
--- a/vcs-svn/fast_export.c
+++ b/vcs-svn/fast_export.c
@@ -6,6 +6,7 @@
#include "git-compat-util.h"
#include "fast_export.h"
#include "line_buffer.h"
+#include "fd_buffer.h"
#include "repo_tree.h"
#include "string_pool.h"
#include "strbuf.h"
@@ -73,7 +74,7 @@ static int ends_with(const char *s, size_t len, const char *suffix)
return !memcmp(s + len - suffixlen, suffix, suffixlen);
}
-static int parse_cat_response_line(const char *header, size_t *len)
+static int parse_cat_response_line(const char *header, off_t *len)
{
size_t headerlen = strlen(header);
const char *type;
@@ -95,45 +96,25 @@ static int parse_cat_response_line(const char *header, size_t *len)
static struct strbuf response_line = STRBUF_INIT;
static const char *get_response_line(void)
{
- /*
- * NEEDSWORK: Does not actually need to read one byte at a time.
- * Some platforms have O_NONBLOCK. On others we could read 8 chars
- * at a time until a potential appearance of " blob ".
- */
strbuf_reset(&response_line);
- for (;;) {
- char buf[1];
- if (xread(REPORT_FILENO, buf, 1) < 0) {
- error("cannot read cat-blob result: %s", strerror(errno));
- return NULL;
- }
- if (*buf == '\n')
- return response_line.buf;
- strbuf_addch(&response_line, *buf);
- }
+ if (fd_read_line(&response_line, REPORT_FILENO))
+ return NULL;
+ return response_line.buf;
}
-static int copy_bytes(FILE *out, size_t len)
+static int copy_bytes(FILE *out, off_t len)
{
- char buf[4096];
- ssize_t nread;
- for (; len; len -= nread) {
- nread = xread(REPORT_FILENO, buf,
- len < sizeof(buf) ? len : sizeof(buf));
- if (nread < 0)
- return error("cannot copy cat-blob result: %s",
- strerror(errno));
- if (!nread)
- return error("0-length read...");
- if (fwrite(buf, 1, nread, out) != nread)
- return error("cannot write cat-blob results: %s",
- strerror(errno));
- }
+ off_t ret = fd_copy_bytes(out, REPORT_FILENO, len);
+ if (!ret)
+ return error("read error: file ends early");
+ if (ret <= 0)
+ return error("cannot copy cat-blob result");
+ return 0;
}
static int fast_export_save_blob(FILE *out)
{
- size_t len = len;
+ off_t len = len;
const char *header, *tail;
header = get_response_line();
diff --git a/vcs-svn/fd_buffer.c b/vcs-svn/fd_buffer.c
new file mode 100644
index 0000000..3d2c1a1
--- /dev/null
+++ b/vcs-svn/fd_buffer.c
@@ -0,0 +1,65 @@
+/*
+ * Licensed under a two-clause BSD-style license.
+ * See LICENSE for details.
+ */
+
+#include "git-compat-util.h"
+#include "fd_buffer.h"
+#include "strbuf.h"
+
+/* Read a line without trailing newline. */
+int fd_read_line(struct strbuf *line, int fd)
+{
+ /*
+ * NEEDSWORK: Does not actually need to read one byte at a time.
+ * Some platforms have O_NONBLOCK. On others we could read
+ * several chars at a time until an appearance of a string known
+ * to belong in the line (e.g., " blob ").
+ */
+ for (;;) {
+ char buf[1];
+ if (xread(fd, buf, 1) < 0)
+ return error("cannot read line: %s", strerror(errno));
+ if (*buf == '\n')
+ return 0;
+ strbuf_addch(line, *buf);
+ }
+}
+
+int fd_read_binary(struct strbuf *buf, size_t len, int fd)
+{
+ char *p, *end;
+ strbuf_grow(buf, len);
+ p = buf->buf + buf->len;
+ end = p + len;
+
+ while (p != end) {
+ ssize_t nread = xread(fd, p, end - p);
+ if (nread < 0)
+ return error("read error: %s", strerror(errno));
+ if (!nread) /* end of file */
+ break;
+ p += nread;
+ }
+ strbuf_setlen(buf, p - buf->buf);
+ return 0;
+}
+
+off_t fd_copy_bytes(FILE *out, int fd, off_t len)
+{
+ off_t total = 0;
+ while (len) {
+ char buf[4096];
+ ssize_t nread = xread(fd, buf,
+ len < sizeof(buf) ? len : sizeof(buf));
+ if (nread < 0)
+ return error("read error: %s", strerror(errno));
+ if (!nread)
+ return 0;
+ if (out && fwrite(buf, 1, nread, out) != nread)
+ return error("write error: %s", strerror(errno));
+ total += nread;
+ len -= nread;
+ }
+ return total;
+}
diff --git a/vcs-svn/fd_buffer.h b/vcs-svn/fd_buffer.h
new file mode 100644
index 0000000..9434ac3
--- /dev/null
+++ b/vcs-svn/fd_buffer.h
@@ -0,0 +1,19 @@
+#ifndef FD_BUFFER_H
+#define FD_BUFFER_H
+
+#include "strbuf.h"
+
+/* Low-level input helpers. Usually line_buffer is a better choice. */
+
+extern int fd_read_line(struct strbuf *line, int fd);
+extern int fd_read_binary(struct strbuf *buf, size_t len, int fd);
+
+/* returns 0 for end of file */
+extern off_t fd_copy_bytes(FILE *out, int fd, off_t len);
+
+static inline off_t fd_skip_bytes(int fd, off_t len)
+{
+ return fd_copy_bytes(NULL, fd, len);
+}
+
+#endif
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 5/8] vcs-svn: Read delta preimage from file descriptor
2010-11-20 19:21 ` [WIP/PATCH 0/8] svn-fe: support for text deltas Jonathan Nieder
` (3 preceding siblings ...)
2010-11-20 19:26 ` [PATCH 4/8] vcs-svn: Introduce fd_buffer routines Jonathan Nieder
@ 2010-11-20 19:27 ` Jonathan Nieder
2010-11-20 19:28 ` [PATCH 6/8] vcs-svn: Let caller set up sliding window for delta preimage Jonathan Nieder
` (3 subsequent siblings)
8 siblings, 0 replies; 40+ messages in thread
From: Jonathan Nieder @ 2010-11-20 19:27 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
Teach the svndiff routines to read the preimage for a delta from a
file descriptor without reading ahead. This way, the delta applier
can stream its input directly from fast-import's cat-blob-fd pipe.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
test-svn-fe.c | 10 +++++-----
vcs-svn/sliding_window.c | 21 +++++++++------------
vcs-svn/sliding_window.h | 2 +-
vcs-svn/svndiff.c | 6 +++---
vcs-svn/svndiff.h | 2 +-
5 files changed, 19 insertions(+), 22 deletions(-)
diff --git a/test-svn-fe.c b/test-svn-fe.c
index 4799e4c..4ea0cd6 100644
--- a/test-svn-fe.c
+++ b/test-svn-fe.c
@@ -22,20 +22,20 @@ int main(int argc, char *argv[])
return 0;
}
if (argc == 5 && !strcmp(argv[1], "-d")) {
- struct line_buffer preimage = LINE_BUFFER_INIT;
+ int preimage_fd = -1;
struct line_buffer delta = LINE_BUFFER_INIT;
- if (buffer_init(&preimage, argv[2]))
+ preimage_fd = open(argv[2], O_RDONLY);
+ if (preimage_fd < 0)
die_errno("cannot open preimage");
if (buffer_init(&delta, argv[3]))
die_errno("cannot open delta");
if (svndiff0_apply(&delta, (off_t) strtoull(argv[4], NULL, 0),
- &preimage, stdout))
+ preimage_fd, stdout))
return 1;
- if (buffer_deinit(&preimage))
+ if (close(preimage_fd))
die_errno("cannot close preimage");
if (buffer_deinit(&delta))
die_errno("cannot close delta");
- buffer_reset(&preimage);
buffer_reset(&delta);
return 0;
}
diff --git a/vcs-svn/sliding_window.c b/vcs-svn/sliding_window.c
index 5c08828..6d4261a 100644
--- a/vcs-svn/sliding_window.c
+++ b/vcs-svn/sliding_window.c
@@ -5,7 +5,7 @@
#include "git-compat-util.h"
#include "sliding_window.h"
-#include "line_buffer.h"
+#include "fd_buffer.h"
#include "strbuf.h"
static void strbuf_remove_from_left(struct strbuf *sb, size_t nbytes)
@@ -30,7 +30,7 @@ static int check_overflow(off_t a, size_t b)
int move_window(struct view *view, off_t off, size_t len)
{
off_t file_offset;
- assert(view && view->file);
+ assert(view && view->fd >= 0);
assert(!check_overflow(view->off, view->buf.len));
if (check_overflow(off, len))
@@ -47,21 +47,18 @@ int move_window(struct view *view, off_t off, size_t len)
if (off > file_offset) {
/* Seek ahead to skip the gap. */
const off_t gap = off - file_offset;
- const off_t nread = buffer_skip_bytes(view->file, gap);
+ const off_t nread = fd_skip_bytes(view->fd, gap);
if (nread != gap) {
- if (!buffer_ferror(view->file))
+ if (!nread)
return error("Preimage ends early");
- return error("Cannot seek forward in input: %s",
- strerror(errno));
+ return error("Cannot seek forward in input");
}
file_offset += gap;
}
- buffer_read_binary(&view->buf, len - view->buf.len, view->file);
- if (view->buf.len != len) {
- if (!buffer_ferror(view->file))
- return error("Preimage ends early");
- return error("Cannot read preimage: %s", strerror(errno));
- }
+ if (fd_read_binary(&view->buf, len - view->buf.len, view->fd))
+ return error("Cannot read preimage");
+ if (view->buf.len != len)
+ return error("Preimage ends early");
view->off = off;
return 0;
}
diff --git a/vcs-svn/sliding_window.h b/vcs-svn/sliding_window.h
index b9f0552..532dc93 100644
--- a/vcs-svn/sliding_window.h
+++ b/vcs-svn/sliding_window.h
@@ -4,7 +4,7 @@
#include "strbuf.h"
struct view {
- struct line_buffer *file;
+ int fd;
off_t off;
struct strbuf buf;
};
diff --git a/vcs-svn/svndiff.c b/vcs-svn/svndiff.c
index 9ee7411..ef3a921 100644
--- a/vcs-svn/svndiff.c
+++ b/vcs-svn/svndiff.c
@@ -283,10 +283,10 @@ static int apply_one_window(struct line_buffer *delta, off_t *delta_len,
}
int svndiff0_apply(struct line_buffer *delta, off_t delta_len,
- struct line_buffer *preimage, FILE *postimage)
+ int preimage_fd, FILE *postimage)
{
- struct view preimage_view = {preimage, 0, STRBUF_INIT};
- assert(delta && preimage && postimage);
+ struct view preimage_view = {preimage_fd, 0, STRBUF_INIT};
+ assert(delta && preimage_fd >= 0 && postimage);
if (read_magic(delta, &delta_len))
goto fail;
diff --git a/vcs-svn/svndiff.h b/vcs-svn/svndiff.h
index a986099..9003d6e 100644
--- a/vcs-svn/svndiff.h
+++ b/vcs-svn/svndiff.h
@@ -4,6 +4,6 @@
#include "line_buffer.h"
extern int svndiff0_apply(struct line_buffer *delta, off_t delta_len,
- struct line_buffer *preimage, FILE *postimage);
+ int preimage_fd, FILE *postimage);
#endif
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 6/8] vcs-svn: Let caller set up sliding window for delta preimage
2010-11-20 19:21 ` [WIP/PATCH 0/8] svn-fe: support for text deltas Jonathan Nieder
` (4 preceding siblings ...)
2010-11-20 19:27 ` [PATCH 5/8] vcs-svn: Read delta preimage from file descriptor Jonathan Nieder
@ 2010-11-20 19:28 ` Jonathan Nieder
2010-11-20 19:31 ` Jonathan Nieder
2010-11-20 19:29 ` [PATCH 7/8] vcs-svn: Teach line_buffer about temporary files Jonathan Nieder
` (2 subsequent siblings)
8 siblings, 1 reply; 40+ messages in thread
From: Jonathan Nieder @ 2010-11-20 19:28 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
The content of symlinks starts with the word "link " in SVN's
worldview, so we need to prepend that text the sake of delta
application. Move responsibility for setting up the input state from
svndiff0_apply to the calling program, so programs have a chance to
seed the sliding window with text of their choice.
[jn: extracted from the patch "svn-fe: Use the --report-fd feature"]
Signed-off-by: David Barr <david.barr@cordelta.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
I'm a big fan of this trick.
test-svn-fe.c | 8 +++++---
vcs-svn/svndiff.c | 23 ++++++++---------------
vcs-svn/svndiff.h | 3 ++-
3 files changed, 15 insertions(+), 19 deletions(-)
diff --git a/test-svn-fe.c b/test-svn-fe.c
index 4ea0cd6..64f63cf 100644
--- a/test-svn-fe.c
+++ b/test-svn-fe.c
@@ -5,6 +5,7 @@
#include "git-compat-util.h"
#include "vcs-svn/svndump.h"
#include "vcs-svn/svndiff.h"
+#include "vcs-svn/sliding_window.h"
#include "vcs-svn/line_buffer.h"
int main(int argc, char *argv[])
@@ -22,20 +23,21 @@ int main(int argc, char *argv[])
return 0;
}
if (argc == 5 && !strcmp(argv[1], "-d")) {
- int preimage_fd = -1;
struct line_buffer delta = LINE_BUFFER_INIT;
- preimage_fd = open(argv[2], O_RDONLY);
+ int preimage_fd = open(argv[2], O_RDONLY);
+ struct view preimage_view = {preimage_fd, 0, STRBUF_INIT};
if (preimage_fd < 0)
die_errno("cannot open preimage");
if (buffer_init(&delta, argv[3]))
die_errno("cannot open delta");
if (svndiff0_apply(&delta, (off_t) strtoull(argv[4], NULL, 0),
- preimage_fd, stdout))
+ &preimage_view, stdout))
return 1;
if (close(preimage_fd))
die_errno("cannot close preimage");
if (buffer_deinit(&delta))
die_errno("cannot close delta");
+ strbuf_release(&preimage_view.buf);
buffer_reset(&delta);
return 0;
}
diff --git a/vcs-svn/svndiff.c b/vcs-svn/svndiff.c
index ef3a921..308c734 100644
--- a/vcs-svn/svndiff.c
+++ b/vcs-svn/svndiff.c
@@ -283,31 +283,24 @@ static int apply_one_window(struct line_buffer *delta, off_t *delta_len,
}
int svndiff0_apply(struct line_buffer *delta, off_t delta_len,
- int preimage_fd, FILE *postimage)
+ struct view *preimage_view, FILE *postimage)
{
- struct view preimage_view = {preimage_fd, 0, STRBUF_INIT};
- assert(delta && preimage_fd >= 0 && postimage);
+ assert(delta && preimage_view && postimage);
if (read_magic(delta, &delta_len))
- goto fail;
+ return -1;
while (delta_len > 0) { /* For each window: */
off_t pre_off = pre_off;
size_t pre_len;
if (read_offset(delta, &pre_off, &delta_len) ||
read_length(delta, &pre_len, &delta_len) ||
- move_window(&preimage_view, pre_off, pre_len) ||
+ move_window(preimage_view, pre_off, pre_len) ||
apply_one_window(delta, &delta_len,
- &preimage_view, postimage))
- goto fail;
- if (delta_len && buffer_at_eof(delta)) {
- error("Delta ends early! (%"PRIu64" bytes remaining)",
+ preimage_view, postimage))
+ return -1;
+ if (delta_len && buffer_at_eof(delta))
+ return error("Delta ends early! (%"PRIu64" bytes remaining)",
(uint64_t) delta_len);
- goto fail;
- }
}
- strbuf_release(&preimage_view.buf);
return 0;
- fail:
- strbuf_release(&preimage_view.buf);
- return -1;
}
diff --git a/vcs-svn/svndiff.h b/vcs-svn/svndiff.h
index 9003d6e..bb5afd0 100644
--- a/vcs-svn/svndiff.h
+++ b/vcs-svn/svndiff.h
@@ -2,8 +2,9 @@
#define SVNDIFF_H_
#include "line_buffer.h"
+#include "sliding_window.h"
extern int svndiff0_apply(struct line_buffer *delta, off_t delta_len,
- int preimage_fd, FILE *postimage);
+ struct view *preimage_view, FILE *postimage);
#endif
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH 6/8] vcs-svn: Let caller set up sliding window for delta preimage
2010-11-20 19:28 ` [PATCH 6/8] vcs-svn: Let caller set up sliding window for delta preimage Jonathan Nieder
@ 2010-11-20 19:31 ` Jonathan Nieder
0 siblings, 0 replies; 40+ messages in thread
From: Jonathan Nieder @ 2010-11-20 19:31 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
Jonathan Nieder wrote:
> The content of symlinks starts with the word "link " in SVN's
> worldview, so we need to prepend that text the sake of delta
> application. Move responsibility for setting up the input state from
> svndiff0_apply to the calling program, so programs have a chance to
> seed the sliding window with text of their choice.
>
> [jn: extracted from the patch "svn-fe: Use the --report-fd feature"]
>
> Signed-off-by: David Barr <david.barr@cordelta.com>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
This one is
From: David Barr <david.barr@cordelta.com>
Sorry, I keep on forgetting to preserve pseudo-header (is there a
format-patch option to move it to the body?).
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 7/8] vcs-svn: Teach line_buffer about temporary files
2010-11-20 19:21 ` [WIP/PATCH 0/8] svn-fe: support for text deltas Jonathan Nieder
` (5 preceding siblings ...)
2010-11-20 19:28 ` [PATCH 6/8] vcs-svn: Let caller set up sliding window for delta preimage Jonathan Nieder
@ 2010-11-20 19:29 ` Jonathan Nieder
2010-11-20 19:29 ` [PATCH 8/8] vcs-svn: Implement text-delta handling Jonathan Nieder
2010-11-20 19:30 ` [PATCH 9/8] svn-fe: Test script for handling of dumps with --deltas Jonathan Nieder
8 siblings, 0 replies; 40+ messages in thread
From: Jonathan Nieder @ 2010-11-20 19:29 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
It can sometimes be useful to write information temporarily to file,
to read back later. These functions allow you to do so.
The dump file importer would use this to save a postimage from delta
application until the length is known and fast-import is ready to read
it.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
vcs-svn/line_buffer.c | 24 ++++++++++++++++++++++++
vcs-svn/line_buffer.h | 7 ++++++-
vcs-svn/line_buffer.txt | 22 ++++++++++++++++++++++
3 files changed, 52 insertions(+), 1 deletions(-)
diff --git a/vcs-svn/line_buffer.c b/vcs-svn/line_buffer.c
index c54031b..f583623 100644
--- a/vcs-svn/line_buffer.c
+++ b/vcs-svn/line_buffer.c
@@ -42,6 +42,30 @@ int buffer_at_eof(struct line_buffer *buf)
return 0;
}
+int buffer_tmpfile_init(struct line_buffer *buf)
+{
+ buf->infile = tmpfile();
+ if (!buf->infile)
+ return error("cannot open temporary file: %s", strerror(errno));
+ return 0;
+}
+
+FILE *buffer_tmpfile_rewind(struct line_buffer *buf)
+{
+ rewind(buf->infile);
+ return buf->infile;
+}
+
+long buffer_tmpfile_prepare_to_read(struct line_buffer *buf)
+{
+ long pos = ftell(buf->infile);
+ if (pos < 0)
+ return error("ftell error: %s", strerror(errno));
+ if (fseek(buf->infile, 0, SEEK_SET))
+ return error("seek error: %s", strerror(errno));
+ return pos;
+}
+
int buffer_read_char(struct line_buffer *buf)
{
return fgetc(buf->infile);
diff --git a/vcs-svn/line_buffer.h b/vcs-svn/line_buffer.h
index 2375ee1..1db7434 100644
--- a/vcs-svn/line_buffer.h
+++ b/vcs-svn/line_buffer.h
@@ -16,12 +16,17 @@ int buffer_init(struct line_buffer *buf, const char *filename);
int buffer_deinit(struct line_buffer *buf);
int buffer_ferror(struct line_buffer *buf);
int buffer_at_eof(struct line_buffer *buf);
+void buffer_reset(struct line_buffer *buf);
+
+int buffer_tmpfile_init(struct line_buffer *buf);
+FILE *buffer_tmpfile_rewind(struct line_buffer *buf); /* prepare to write. */
+long buffer_tmpfile_prepare_to_read(struct line_buffer *buf);
+
char *buffer_read_line(struct line_buffer *buf);
char *buffer_read_string(struct line_buffer *buf, uint32_t len);
int buffer_read_char(struct line_buffer *buf);
void buffer_read_binary(struct strbuf *sb, uint32_t len, struct line_buffer *f);
void buffer_copy_bytes(struct line_buffer *buf, off_t len);
off_t buffer_skip_bytes(struct line_buffer *buf, off_t len);
-void buffer_reset(struct line_buffer *buf);
#endif
diff --git a/vcs-svn/line_buffer.txt b/vcs-svn/line_buffer.txt
index d06db24..344efc3 100644
--- a/vcs-svn/line_buffer.txt
+++ b/vcs-svn/line_buffer.txt
@@ -24,6 +24,28 @@ The calling program:
When finished, the caller can use `buffer_reset` to deallocate
resources.
+Using temporary files
+---------------------
+
+Sometimes a file is just a convenient place to stash data for
+later. A program
+
+ - initializes a `struct line_buffer` to LINE_BUFFER_INIT
+ - allocates a temporary file with `buffer_tmpfile_init`
+ - acquires an output handle by calling `buffer_tmpfile_rewind`
+ - uses standard I/O functions like `fprintf` and `fwrite` to fill
+ the temporary file
+ - declares writing is over with `buffer_tmpfile_prepare_to_read`
+ - can re-read what was written with `buffer_read_line`,
+ `buffer_read_string`, and so on
+ - can reuse the temporary file by calling `buffer_tmpfile_rewind`
+ again
+ - cleans up the temporary file with `buffer_deinit`, perhaps to
+ reuse the line_buffer for some other file.
+
+When finished, the program should use `buffer_reset` to deallocate
+resources.
+
Functions
---------
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 8/8] vcs-svn: Implement text-delta handling
2010-11-20 19:21 ` [WIP/PATCH 0/8] svn-fe: support for text deltas Jonathan Nieder
` (6 preceding siblings ...)
2010-11-20 19:29 ` [PATCH 7/8] vcs-svn: Teach line_buffer about temporary files Jonathan Nieder
@ 2010-11-20 19:29 ` Jonathan Nieder
2010-12-04 17:34 ` [PATCH 10/8] vcs-svn: Consume whole preimage when applying deltas Jonathan Nieder
2010-11-20 19:30 ` [PATCH 9/8] svn-fe: Test script for handling of dumps with --deltas Jonathan Nieder
8 siblings, 1 reply; 40+ messages in thread
From: Jonathan Nieder @ 2010-11-20 19:29 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
From: David Barr <david.barr@cordelta.com>
Handle input in Subversion's dumpfile format, version 3. This is the
format produced by "svnrdump dump" and "svnadmin dump --deltas", and
the main difference between v3 dumpfiles and the dumpfiles already
handled is that these can include nodes whose properties and text are
expressed relative to some other node.
To handle such nodes, we find which node the text and properties are
based on, handle its property changes, use the cat-blob command to
request the basis blob from the fast-import backend, use the
svndiff0_apply() helper to apply the text delta on the fly, writing
output to a temporary file, and then measure that postimage file's
length and write its content to the fast-import stream.
The temporary postimage file is shared between delta-using nodes to
avoid some file system overhead.
The svn-fe interface needs to be more complicated to accomodate the
backward flow of information from the fast-import backend to svn-fe.
The interface for parsing streams without deltas is unchanged,
though.
NEEDSWORK: generalize interface so caller sets the backflow fd, close
temporary file before exiting
Signed-off-by: David Barr <david.barr@cordelta.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
contrib/svn-fe/svn-fe.txt | 11 ++++--
t/t9010-svn-fe.sh | 16 +++++++-
vcs-svn/fast_export.c | 88 +++++++++++++++++++++++++++++++-------------
vcs-svn/fast_export.h | 3 ++
vcs-svn/svndump.c | 19 +++++++---
5 files changed, 99 insertions(+), 38 deletions(-)
diff --git a/contrib/svn-fe/svn-fe.txt b/contrib/svn-fe/svn-fe.txt
index 35f84bd..6af989f 100644
--- a/contrib/svn-fe/svn-fe.txt
+++ b/contrib/svn-fe/svn-fe.txt
@@ -7,7 +7,13 @@ svn-fe - convert an SVN "dumpfile" to a fast-import stream
SYNOPSIS
--------
-svnadmin dump --incremental REPO | svn-fe [url] | git fast-import
+svnadmin dump REPO | svn-fe [url] | git fast-import
+
+[verse]
+mkfifo backchannel &&
+svnadmin dump --deltas REPO |
+ svn-fe [url] 3<backchannel |
+ git fast-import --cat-blob-fd=3 3>backchannel
DESCRIPTION
-----------
@@ -25,9 +31,6 @@ Subversion's repository dump format is documented in full in
Files in this format can be generated using the 'svnadmin dump' or
'svk admin dump' command.
-Dumps produced with 'svnadmin dump --deltas' (dumpfile format v3)
-are not supported.
-
OUTPUT FORMAT
-------------
The fast-import format is documented by the git-fast-import(1)
diff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh
index 7dc0670..6c8b803 100755
--- a/t/t9010-svn-fe.sh
+++ b/t/t9010-svn-fe.sh
@@ -35,6 +35,14 @@ text_no_props () {
>empty
+test_expect_success 'setup: have pipes?' '
+ rm -f frob &&
+ if mkfifo frob
+ then
+ test_set_prereq PIPE
+ fi
+'
+
test_expect_success 'empty dump' '
reinit_git &&
echo "SVN-fs-dump-format-version: 2" >input &&
@@ -451,7 +459,9 @@ test_expect_success 'change file mode and reiterate content' '
test_cmp hello actual.target
'
-test_expect_success 'deltas not supported' '
+test_expect_success PIPE 'deltas supported' '
+ reinit_git &&
+ rm -f backflow &&
{
# (old) h + (inline) ello + (old) \n
printf "SVNQ%b%b%s" "Q\003\006\005\004" "\001Q\0204\001\002" "ello" |
@@ -511,7 +521,9 @@ test_expect_success 'deltas not supported' '
echo PROPS-END &&
cat delta
} >delta.dump &&
- test_must_fail test-svn-fe delta.dump
+ mkfifo backflow &&
+ test_must_fail test-svn-fe delta.dump 3<backflow |
+ git fast-import --cat-blob-fd=3 3>backflow
'
test_expect_success 'property deltas supported' '
diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c
index ebaab72..ceb1fc5 100644
--- a/vcs-svn/fast_export.c
+++ b/vcs-svn/fast_export.c
@@ -5,9 +5,11 @@
#include "git-compat-util.h"
#include "fast_export.h"
+#include "repo_tree.h"
+#include "svndiff.h"
+#include "sliding_window.h"
#include "line_buffer.h"
#include "fd_buffer.h"
-#include "repo_tree.h"
#include "string_pool.h"
#include "strbuf.h"
@@ -15,6 +17,17 @@
#define REPORT_FILENO 3
static uint32_t first_commit_done;
+static struct line_buffer postimage = LINE_BUFFER_INIT;
+
+/* NEEDSWORK: move to fast_export_init() */
+static int init_postimage(void)
+{
+ static int postimage_initialized;
+ if (postimage_initialized)
+ return 0;
+ postimage_initialized = 1;
+ return buffer_tmpfile_init(&postimage);
+}
void fast_export_delete(uint32_t depth, uint32_t *path)
{
@@ -102,33 +115,39 @@ static const char *get_response_line(void)
return response_line.buf;
}
-static int copy_bytes(FILE *out, off_t len)
+static long apply_delta(uint32_t mark, off_t len, struct line_buffer *input,
+ uint32_t old_mark, uint32_t old_mode)
{
- off_t ret = fd_copy_bytes(out, REPORT_FILENO, len);
- if (!ret)
- return error("read error: file ends early");
- if (ret <= 0)
- return error("cannot copy cat-blob result");
- return 0;
-}
+ long ret;
+ struct view preimage = {REPORT_FILENO, 0, STRBUF_INIT};
+ FILE *out;
-static int fast_export_save_blob(FILE *out)
-{
- off_t len = len;
- const char *header, *tail;
-
- header = get_response_line();
- if (!header || parse_cat_response_line(header, &len))
- return -1;
- copy_bytes(out, len);
-
- /* Discard trailing newline. */
- tail = get_response_line();
- if (!tail)
- return -1;
- if (*tail)
- return error("line following cat-blob response contains garbage: %s", tail);
- return 0;
+ if (init_postimage() || !(out = buffer_tmpfile_rewind(&postimage)))
+ die("cannot open temporary file for blob retrieval");
+ if (old_mark) {
+ const char *response;
+ off_t dummy;
+ printf("cat-blob :%"PRIu32"\n", old_mark);
+ fflush(stdout);
+ response = get_response_line();
+ /* Not necessary, just for robustness */
+ if (parse_cat_response_line(response, &dummy))
+ die("invalid cat-blob response: %s", response);
+ }
+ if (old_mode == REPO_MODE_LNK)
+ strbuf_addstr(&preimage.buf, "link ");
+ if (svndiff0_apply(input, len, &preimage, out))
+ die("cannot apply delta");
+ if (old_mark) {
+ /* Discard trailing newline from cat-blob-fd. */
+ const char *tail = get_response_line();
+ if (!tail || *tail)
+ die("missing newline after cat-blob response");
+ }
+ ret = buffer_tmpfile_prepare_to_read(&postimage);
+ if (ret < 0)
+ die("cannot read temporary file for blob retrieval");
+ return ret;
}
void fast_export_blob(uint32_t mode, uint32_t mark, uint32_t len, struct line_buffer *input)
@@ -142,3 +161,20 @@ void fast_export_blob(uint32_t mode, uint32_t mark, uint32_t len, struct line_bu
buffer_copy_bytes(input, len);
fputc('\n', stdout);
}
+
+void fast_export_blob_delta(uint32_t mode, uint32_t mark,
+ uint32_t old_mode, uint32_t old_mark,
+ uint32_t len, struct line_buffer *input)
+{
+ long postimage_len;
+ if (len > maximum_signed_value_of_type(off_t))
+ die("enormous delta");
+ postimage_len = apply_delta(mark, (off_t) len, input, old_mark, old_mode);
+ if (mode == REPO_MODE_LNK) {
+ buffer_skip_bytes(&postimage, strlen("link "));
+ postimage_len -= strlen("link ");
+ }
+ printf("blob\nmark :%"PRIu32"\ndata %ld\n", mark, postimage_len);
+ buffer_copy_bytes(&postimage, postimage_len);
+ fputc('\n', stdout);
+}
diff --git a/vcs-svn/fast_export.h b/vcs-svn/fast_export.h
index 054e7d5..6f77c3b 100644
--- a/vcs-svn/fast_export.h
+++ b/vcs-svn/fast_export.h
@@ -10,5 +10,8 @@ void fast_export_commit(uint32_t revision, uint32_t author, char *log,
uint32_t uuid, uint32_t url, unsigned long timestamp);
void fast_export_blob(uint32_t mode, uint32_t mark, uint32_t len,
struct line_buffer *input);
+void fast_export_blob_delta(uint32_t mode, uint32_t mark,
+ uint32_t old_mode, uint32_t old_mark,
+ uint32_t len, struct line_buffer *input);
#endif
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index 3e7fb9b..a6c849e 100644
--- a/vcs-svn/svndump.c
+++ b/vcs-svn/svndump.c
@@ -198,12 +198,10 @@ static void read_props(void)
static void handle_node(void)
{
- uint32_t mark = 0;
+ uint32_t mark = 0, old_mode, old_mark;
const uint32_t type = node_ctx.type;
const int have_props = node_ctx.propLength != LENGTH_UNKNOWN;
- 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) {
@@ -224,7 +222,9 @@ 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) {
- uint32_t mode = repo_modify_path(node_ctx.dst, 0, mark);
+ uint32_t mode;
+ old_mark = repo_read_path(node_ctx.dst);
+ 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)
@@ -236,11 +236,12 @@ static void handle_node(void)
if (!mark && type != REPO_MODE_DIR)
die("invalid dump: adds node without text");
repo_add(node_ctx.dst, type, mark);
+ old_mark = 0;
} else {
die("invalid dump: Node-path block lacks Node-action");
}
+ old_mode = node_ctx.type;
if (have_props) {
- const uint32_t old_mode = node_ctx.type;
if (!node_ctx.prop_delta)
node_ctx.type = type;
if (node_ctx.propLength)
@@ -248,8 +249,14 @@ static void handle_node(void)
if (node_ctx.type != old_mode)
repo_modify_path(node_ctx.dst, node_ctx.type, mark);
}
- if (mark)
+ if (!mark)
+ return;
+ if (!node_ctx.text_delta) {
fast_export_blob(node_ctx.type, mark, node_ctx.textLength, &input);
+ return;
+ }
+ fast_export_blob_delta(node_ctx.type, mark, old_mode, old_mark,
+ node_ctx.textLength, &input);
}
static void handle_revision(void)
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 10/8] vcs-svn: Consume whole preimage when applying deltas
2010-11-20 19:29 ` [PATCH 8/8] vcs-svn: Implement text-delta handling Jonathan Nieder
@ 2010-12-04 17:34 ` Jonathan Nieder
0 siblings, 0 replies; 40+ messages in thread
From: Jonathan Nieder @ 2010-12-04 17:34 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
Date: Sun Nov 21 20:24:12 2010 -0600
Some deltas do not consume the entire preimage, so we have to
consume it explicitly.
Noticed during imports from the ASF repo.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Jonathan Nieder wrote:
> +++ b/vcs-svn/fast_export.c
> @@ -102,33 +115,39 @@ static const char *get_response_line(void)
[...]
> + if (svndiff0_apply(input, len, &preimage, out))
> + die("cannot apply delta");
> + if (old_mark) {
> + /* Discard trailing newline from cat-blob-fd. */
> + const char *tail = get_response_line();
> + if (!tail || *tail)
> + die("missing newline after cat-blob response");
As mentioned before, this error
fatal: missing newline after cat-blob response
and
error: Preimage ends early
were sometimes triggering. Here's a fix for squashing. Sorry for
the nonsensical code.
t/t9010-svn-fe.sh | 108 +++++++++++++++++++++++++++++++++++++++++++++++++
vcs-svn/fast_export.c | 16 ++++---
2 files changed, 117 insertions(+), 7 deletions(-)
diff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh
index 6c8b803..1776a38 100755
--- a/t/t9010-svn-fe.sh
+++ b/t/t9010-svn-fe.sh
@@ -678,6 +678,114 @@ test_expect_success 'deltas for typechange' '
test_cmp expect actual
'
+test_expect_success PIPE 'deltas need not consume the whole preimage' '
+ reinit_git &&
+ cat >expect <<-\EOF &&
+ OBJID
+ :120000 100644 OBJID OBJID T postimage
+ OBJID
+ :100644 120000 OBJID OBJID T postimage
+ OBJID
+ :000000 100644 OBJID OBJID A postimage
+ EOF
+ echo "first preimage" >expect.1 &&
+ printf target >expect.2 &&
+ printf lnk >expect.3 &&
+ rm -f backflow &&
+ {
+ printf "SVNQ%b%b%b" "QQ\017\001\017" "\0217" "first preimage\n" |
+ q_to_nul
+ } >delta.1 &&
+ {
+ properties svn:special "*" &&
+ echo PROPS-END
+ } >symlink.props &&
+ {
+ printf "SVNQ%b%b%b" "Q\002\013\004\012" "\0201\001\001\0211" "lnk target" |
+ q_to_nul
+ } >delta.2 &&
+ {
+ printf "SVNQ%b%b" "Q\004\003\004Q" "\001Q\002\002" |
+ q_to_nul
+ } >delta.3 &&
+ {
+ cat <<-\EOF &&
+ SVN-fs-dump-format-version: 3
+
+ Revision-number: 1
+ Prop-content-length: 10
+ Content-length: 10
+
+ PROPS-END
+
+ Node-path: postimage
+ Node-kind: file
+ Node-action: add
+ Text-delta: true
+ Prop-content-length: 10
+ EOF
+ echo Text-content-length: $(wc -c <delta.1) &&
+ echo Content-length: $((10 + $(wc -c <delta.1))) &&
+ echo &&
+ echo PROPS-END &&
+ cat delta.1 &&
+ cat <<-\EOF &&
+
+ Revision-number: 2
+ Prop-content-length: 10
+ Content-length: 10
+
+ PROPS-END
+
+ Node-path: postimage
+ Node-kind: file
+ Node-action: change
+ Text-delta: true
+ EOF
+ echo Prop-content-length: $(wc -c <symlink.props) &&
+ echo Text-content-length: $(wc -c <delta.2) &&
+ echo Content-length: $(($(wc -c <symlink.props) + $(wc -c <delta.2))) &&
+ echo &&
+ cat symlink.props &&
+ cat delta.2 &&
+ cat <<-\EOF &&
+
+ Revision-number: 3
+ Prop-content-length: 10
+ Content-length: 10
+
+ PROPS-END
+
+ Node-path: postimage
+ Node-kind: file
+ Node-action: change
+ Text-delta: true
+ Prop-content-length: 10
+ EOF
+ echo Text-content-length: $(wc -c <delta.3) &&
+ echo Content-length: $((10 + $(wc -c <delta.3))) &&
+ echo &&
+ echo PROPS-END &&
+ cat delta.3 &&
+ echo
+ } >deltapartial.dump &&
+ mkfifo backflow &&
+ test-svn-fe deltapartial.dump 3<backflow |
+ git fast-import --cat-blob-fd=3 3>backflow &&
+ {
+ git rev-list HEAD |
+ git diff-tree --root --stdin |
+ sed "s/$_x40/OBJID/g"
+ } >actual &&
+ test_cmp expect actual &&
+ git show HEAD:postimage >actual.3 &&
+ git show HEAD^:postimage >actual.2 &&
+ git show HEAD^^:postimage >actual.1 &&
+ test_cmp expect.1 actual.1 &&
+ test_cmp expect.2 actual.2 &&
+ test_cmp expect.3 actual.3
+'
+
test_expect_success 't9135/svn.dump' '
svnadmin create simple-svn &&
svnadmin load simple-svn <"$TEST_DIRECTORY/t9135/svn.dump" &&
diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c
index ceb1fc5..f8a41e7 100644
--- a/vcs-svn/fast_export.c
+++ b/vcs-svn/fast_export.c
@@ -119,6 +119,7 @@ static long apply_delta(uint32_t mark, off_t len, struct line_buffer *input,
uint32_t old_mark, uint32_t old_mode)
{
long ret;
+ off_t preimage_len = 0;
struct view preimage = {REPORT_FILENO, 0, STRBUF_INIT};
FILE *out;
@@ -126,22 +127,23 @@ static long apply_delta(uint32_t mark, off_t len, struct line_buffer *input,
die("cannot open temporary file for blob retrieval");
if (old_mark) {
const char *response;
- off_t dummy;
printf("cat-blob :%"PRIu32"\n", old_mark);
fflush(stdout);
response = get_response_line();
- /* Not necessary, just for robustness */
- if (parse_cat_response_line(response, &dummy))
+ if (parse_cat_response_line(response, &preimage_len))
die("invalid cat-blob response: %s", response);
}
- if (old_mode == REPO_MODE_LNK)
+ if (old_mode == REPO_MODE_LNK) {
strbuf_addstr(&preimage.buf, "link ");
+ preimage_len += strlen("link ");
+ }
if (svndiff0_apply(input, len, &preimage, out))
die("cannot apply delta");
if (old_mark) {
- /* Discard trailing newline from cat-blob-fd. */
- const char *tail = get_response_line();
- if (!tail || *tail)
+ /* Read the remainder of preimage and trailing newline. */
+ if (move_window(&preimage, preimage_len, 1))
+ die("cannot seek to end of input");
+ if (preimage.buf.buf[0] != '\n')
die("missing newline after cat-blob response");
}
ret = buffer_tmpfile_prepare_to_read(&postimage);
--
1.7.2.4
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH 9/8] svn-fe: Test script for handling of dumps with --deltas
2010-11-20 19:21 ` [WIP/PATCH 0/8] svn-fe: support for text deltas Jonathan Nieder
` (7 preceding siblings ...)
2010-11-20 19:29 ` [PATCH 8/8] vcs-svn: Implement text-delta handling Jonathan Nieder
@ 2010-11-20 19:30 ` Jonathan Nieder
2010-12-04 17:29 ` Jonathan Nieder
8 siblings, 1 reply; 40+ messages in thread
From: Jonathan Nieder @ 2010-11-20 19:30 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
contrib/svn-fe/testme.sh | 16 ++++++++++++++++
1 files changed, 16 insertions(+), 0 deletions(-)
create mode 100755 contrib/svn-fe/testme.sh
diff --git a/contrib/svn-fe/testme.sh b/contrib/svn-fe/testme.sh
new file mode 100755
index 0000000..29e181f
--- /dev/null
+++ b/contrib/svn-fe/testme.sh
@@ -0,0 +1,16 @@
+#!/bin/sh
+# Usage:
+# PATH=${git_src}/contrib/svn-fe:${git_src}/bin-wrappers:$PATH
+# testme.sh http://cvs2svn.tigris.org/svn/cvs2svn 10 test.git
+set -e
+: ${1?"URL?"}
+: ${2?"How many revisions?"}
+: ${3?"Git directory?"}
+
+git init --bare "$3"
+rm -f "$3/backflow"
+mkfifo "$3/backflow"
+
+svnrdump dump -r1:"$2" "$1" |
+svn-fe 3<backflow |
+GIT_DIR=$3 git fast-import --cat-blob-fd=3 3>backflow
--
1.7.2.3
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH 9/8] svn-fe: Test script for handling of dumps with --deltas
2010-11-20 19:30 ` [PATCH 9/8] svn-fe: Test script for handling of dumps with --deltas Jonathan Nieder
@ 2010-12-04 17:29 ` Jonathan Nieder
0 siblings, 0 replies; 40+ messages in thread
From: Jonathan Nieder @ 2010-12-04 17:29 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr
Jonathan Nieder wrote:
> +++ b/contrib/svn-fe/testme.sh
> @@ -0,0 +1,16 @@
> +#!/bin/sh
> +# Usage:
> +# PATH=${git_src}/contrib/svn-fe:${git_src}/bin-wrappers:$PATH
> +# testme.sh http://cvs2svn.tigris.org/svn/cvs2svn 10 test.git
> +set -e
> +: ${1?"URL?"}
> +: ${2?"How many revisions?"}
> +: ${3?"Git directory?"}
> +
> +git init --bare "$3"
> +rm -f "$3/backflow"
> +mkfifo "$3/backflow"
> +
> +svnrdump dump -r1:"$2" "$1" |
> +svn-fe 3<backflow |
> +GIT_DIR=$3 git fast-import --cat-blob-fd=3 3>backflow
In case someone was puzzled: backflow should be "$3/backflow" on these
last two lines.
I didn't notice at first because there was already a fifo available in
the cwd during testing.
^ permalink raw reply [flat|nested] 40+ messages in thread