# [PATCH] vcs-svn: fix broken test 'keep content, but change mode' 2 messages from 2011-08-25 to 2011-08-25. Participants: Dmitry Ivankov. Thread: https://gitlist.dev/t/28218 ## Dmitry Ivankov, 2011-08-25 16:02 Subject: [PATCH] vcs-svn: fix broken test 'keep content, but change mode' Message-ID: <1314288124-16969-1-git-send-email-divanorama@gmail.com> URL: https://gitlist.dev/e/1314288124-16969-1-git-send-email-divanorama%40gmail.com ``` svn symlinks are files with "link destination" content and a "svn:special=*" property set. These are imported as blobs with "destination" content and S_IFLNK mode. When svn copy a file without altering it's content(but maybe altering it's mode), we reuse the blob object thus loosing or not adding the "link " prefix. But we take possible prefix into account when applying svn deltas. And this is the only place we ask fast-import for original blob. So pretend that we want to apply a zero delta to resolve the issue. There is some overhead due to using a temporary file to store such a small blob. But hopefully such node change is too rare to care. Signed-off-by: Dmitry Ivankov --- This could be done better if we just read the cat-blob to memory, added or removed the "link " prefix and wrote it to the stream, because the link destination should be a tiny string. But on the other hand it'd blow up if for some reason it's huge. And taking into account that changing file mode from/to link without a content change should be extremely rare anyway, I think it's ok. Maybe it is redundant to add svndiff0_identity function to just cat blob to a temporary file. The excuse is that svndiff.c is the only user of this temporary file and the cat-blob response, so keep it there. The patch base is svn-fe branch at git://repo.or.cz/git/jrn.git Not backporting it to git master because vcs-svn stuff differs quite much around this change. t/t9010-svn-fe.sh | 2 +- vcs-svn/fast_export.c | 6 +++++- vcs-svn/svndiff.c | 17 +++++++++++++++++ vcs-svn/svndiff.h | 1 + vcs-svn/svndump.c | 16 +++++++++++++++- 5 files changed, 39 insertions(+), 3 deletions(-) diff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh index b7eed24..b6bdfeb 100755 --- a/t/t9010-svn-fe.sh +++ b/t/t9010-svn-fe.sh @@ -413,7 +413,7 @@ test_expect_success PIPE 'action: add node without text' ' try_dump textless.dump must_fail ' -test_expect_failure PIPE 'change file mode but keep old content' ' +test_expect_success PIPE 'change file mode but keep old content' ' reinit_git && cat >expect <<-\EOF && OBJID diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c index 19d7c34..8c7295f 100644 --- a/vcs-svn/fast_export.c +++ b/vcs-svn/fast_export.c @@ -209,7 +209,11 @@ static long apply_delta(off_t len, struct line_buffer *input, preimage.max_off += strlen("link "); check_preimage_overflow(preimage.max_off, 1); } - if (svndiff0_apply(input, len, &preimage, out)) + + if (!input) { + if (svndiff0_identity(&preimage, out)) + die("cannot cat blob"); + } else if (svndiff0_apply(input, len, &preimage, out)) die("cannot apply delta"); if (old_data) { /* Read the remainder of preimage and trailing newline. */ diff --git a/vcs-svn/svndiff.c b/vcs-svn/svndiff.c index 9ee41bb..bf104db 100644 --- a/vcs-svn/svndiff.c +++ b/vcs-svn/svndiff.c @@ -306,3 +306,20 @@ int svndiff0_apply(struct line_buffer *delta, off_t delta_len, } return 0; } +int svndiff0_identity(struct sliding_view *preimage, FILE *postimage) +{ + assert(preimage && postimage); + off_t pre_off = 0; + + while (pre_off != preimage->max_off) { + size_t pre_len = 8192; + if (pre_off + pre_len > preimage->max_off) + pre_len = preimage->max_off - pre_off; + if (move_window(preimage, pre_off, pre_len) || + write_strbuf(&preimage->buf, postimage)) + return -1; + pre_off += pre_len; + } + + return 0; +} diff --git a/vcs-svn/svndiff.h b/vcs-svn/svndiff.h index 74eb464..5afa3f2 100644 --- a/vcs-svn/svndiff.h +++ b/vcs-svn/svndiff.h @@ -6,5 +6,6 @@ struct sliding_view; extern int svndiff0_apply(struct line_buffer *delta, off_t delta_len, struct sliding_view *preimage, FILE *postimage); +extern int svndiff0_identity(struct sliding_view *preimage, FILE *postimage); #endif diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c index b1f4161..1e7ed48 100644 --- a/vcs-svn/svndump.c +++ b/vcs-svn/svndump.c @@ -285,7 +285,21 @@ static void handle_node(void) /* For the fast_export_* functions, NULL means empty. */ old_data = NULL; if (!have_text) { - fast_export_modify(node_ctx.dst.buf, node_ctx.type, old_data); + /* + * This is clean content copy in svn, but we alter the content + * of symlinks (add/remove "link " prefix used by svn). So when + * mode changes from/to symlink specify (recreate) data inline. + */ + if (node_ctx.type != old_mode && (old_mode == REPO_MODE_LNK + || node_ctx.type == REPO_MODE_LNK)) { + + fast_export_modify(node_ctx.dst.buf, + node_ctx.type, "inline"); + fast_export_blob_delta(node_ctx.type, old_mode, + old_data, 0, NULL); + } else + fast_export_modify(node_ctx.dst.buf, + node_ctx.type, old_data); return; } if (!node_ctx.text_delta) { -- 1.7.3.4 ``` ## Dmitry Ivankov, 2011-08-25 16:08 Subject: Re: [PATCH] vcs-svn: fix broken test 'keep content, but change mode' Message-ID: URL: https://gitlist.dev/e/CA%2BgfSn8a7kjbQFP0A2BHfro8MOhpY-TxDr5Fr%2B%3D0qtD9O62GPA%40mail.gmail.com In-Reply-To: <1314288124-16969-1-git-send-email-divanorama@gmail.com> ``` correct David's email (on first attempt I've accidentally used the old one, taken from some git history) On Thu, Aug 25, 2011 at 10:02 PM, Dmitry Ivankov wrote: > svn symlinks are files with "link destination" content and a > "svn:special=*" property set. These are imported as blobs with > "destination" content and S_IFLNK mode. When svn copy a file without > altering it's content(but maybe altering it's mode), we reuse the blob > object thus loosing or not adding the "link " prefix. > > But we take possible prefix into account when applying svn deltas. And > this is the only place we ask fast-import for original blob. So pretend > that we want to apply a zero delta to resolve the issue. > > There is some overhead due to using a temporary file to store such a small > blob. But hopefully such node change is too rare to care. > > Signed-off-by: Dmitry Ivankov > --- > This could be done better if we just read the cat-blob to memory, added or > removed the "link " prefix and wrote it to the stream, because the link > destination should be a tiny string. But on the other hand it'd blow up > if for some reason it's huge. > > And taking into account that changing file mode from/to link without a > content change should be extremely rare anyway, I think it's ok. > > Maybe it is redundant to add svndiff0_identity function to just cat blob > to a temporary file. The excuse is that svndiff.c is the only user of this > temporary file and the cat-blob response, so keep it there. > > The patch base is svn-fe branch at git://repo.or.cz/git/jrn.git > Not backporting it to git master because vcs-svn stuff differs quite much > around this change. > >  t/t9010-svn-fe.sh     |    2 +- >  vcs-svn/fast_export.c |    6 +++++- >  vcs-svn/svndiff.c     |   17 +++++++++++++++++ >  vcs-svn/svndiff.h     |    1 + >  vcs-svn/svndump.c     |   16 +++++++++++++++- >  5 files changed, 39 insertions(+), 3 deletions(-) > > diff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh > index b7eed24..b6bdfeb 100755 > --- a/t/t9010-svn-fe.sh > +++ b/t/t9010-svn-fe.sh > @@ -413,7 +413,7 @@ test_expect_success PIPE 'action: add node without text' ' >        try_dump textless.dump must_fail >  ' > > -test_expect_failure PIPE 'change file mode but keep old content' ' > +test_expect_success PIPE 'change file mode but keep old content' ' >        reinit_git && >        cat >expect <<-\EOF && >        OBJID > diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c > index 19d7c34..8c7295f 100644 > --- a/vcs-svn/fast_export.c > +++ b/vcs-svn/fast_export.c > @@ -209,7 +209,11 @@ static long apply_delta(off_t len, struct line_buffer *input, >                preimage.max_off += strlen("link "); >                check_preimage_overflow(preimage.max_off, 1); >        } > -       if (svndiff0_apply(input, len, &preimage, out)) > + > +       if (!input) { > +               if (svndiff0_identity(&preimage, out)) > +                       die("cannot cat blob"); > +       } else if (svndiff0_apply(input, len, &preimage, out)) >                die("cannot apply delta"); >        if (old_data) { >                /* Read the remainder of preimage and trailing newline. */ > diff --git a/vcs-svn/svndiff.c b/vcs-svn/svndiff.c > index 9ee41bb..bf104db 100644 > --- a/vcs-svn/svndiff.c > +++ b/vcs-svn/svndiff.c > @@ -306,3 +306,20 @@ int svndiff0_apply(struct line_buffer *delta, off_t delta_len, >        } >        return 0; >  } > +int svndiff0_identity(struct sliding_view *preimage, FILE *postimage) > +{ > +       assert(preimage && postimage); > +       off_t pre_off = 0; > + > +       while (pre_off != preimage->max_off) { > +               size_t pre_len = 8192; > +               if (pre_off + pre_len > preimage->max_off) > +                       pre_len = preimage->max_off - pre_off; > +               if (move_window(preimage, pre_off, pre_len) || > +                       write_strbuf(&preimage->buf, postimage)) > +                       return -1; > +               pre_off += pre_len; > +       } > + > +       return 0; > +} > diff --git a/vcs-svn/svndiff.h b/vcs-svn/svndiff.h > index 74eb464..5afa3f2 100644 > --- a/vcs-svn/svndiff.h > +++ b/vcs-svn/svndiff.h > @@ -6,5 +6,6 @@ struct sliding_view; > >  extern int svndiff0_apply(struct line_buffer *delta, off_t delta_len, >                struct sliding_view *preimage, FILE *postimage); > +extern int svndiff0_identity(struct sliding_view *preimage, FILE *postimage); > >  #endif > diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c > index b1f4161..1e7ed48 100644 > --- a/vcs-svn/svndump.c > +++ b/vcs-svn/svndump.c > @@ -285,7 +285,21 @@ static void handle_node(void) >                /* For the fast_export_* functions, NULL means empty. */ >                old_data = NULL; >        if (!have_text) { > -               fast_export_modify(node_ctx.dst.buf, node_ctx.type, old_data); > +               /* > +                * This is clean content copy in svn, but we alter the content > +                * of symlinks (add/remove "link " prefix used by svn). So when > +                * mode changes from/to symlink specify (recreate) data inline. > +                */ > +               if (node_ctx.type != old_mode && (old_mode == REPO_MODE_LNK > +                                       || node_ctx.type == REPO_MODE_LNK)) { > + > +                       fast_export_modify(node_ctx.dst.buf, > +                                               node_ctx.type, "inline"); > +                       fast_export_blob_delta(node_ctx.type, old_mode, > +                                               old_data, 0, NULL); > +               } else > +                       fast_export_modify(node_ctx.dst.buf, > +                                               node_ctx.type, old_data); >                return; >        } >        if (!node_ctx.text_delta) { > -- > 1.7.3.4 > > ```