vcs-svn: Implement Prop-delta handling

The rules for what file is used as delta source for each file are not
documented in dump-load-format.txt.  Luckily, the Apache Software
Foundation repository has rich enough examples to figure out most of
the rules:

Node-action: replace implies the empty property set and empty text as
preimage for deltas.  Otherwise, if a copyfrom source is given, that
node is the preimage for deltas.  Lastly, if none of the above applies
and the node path exists in the current revision, then that version
forms the basis.

[jn: refactored, with tests]

Signed-off-by: David Barr <david.barr@cordelta.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh
index f1e8799bbd5e39ec28829aa30c42010c68b6dbf4..7dc06707a106cd89e3769fdd1b4dfbd5cface42a 100755 (executable)
--- 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_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 \
        {
                properties \
                        svn:author author@example.com \


@@ -565,7 +570,100 @@ test_expect_success 'property deltas not supported' '
                PROPS-END
                EOF
        } >propdelta.dump &&
                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' '
 '
 
 test_expect_success 't9135/svn.dump' '

diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index 576d148e5e8189d36726ffd977328d0125f1f4de..c71a57599ec743813b15b686adf0daaac78a1e18 100644 (file)
--- 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");
 }
 
        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 (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) {
                /* 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))
                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;
 {
        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;
        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 'K':
                        key = pool_intern(val);
                        continue;

+               case 'D':
+                       key = pool_intern(val);
+                       val = NULL;
+                       len = 0;
+                       /* fall through */

                case 'V':
                case 'V':

-                       handle_property(key, val, len);
+                       handle_property(key, val, len, &type_set);

                        key = ~0;
                        continue;
                default:
                        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;
 
        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) {
        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;
        }
        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)
                if (node_ctx.propLength)
                        read_props();
                if (node_ctx.type != old_mode)