archive: fix subst file generation

Before the strbuf conversion, result was a char pointer.  The if
statement checked for it being not NULL, which meant that no
"$Format:...$" string had been found and no replacement had to be
made.  format_subst() returned NULL in that case -- the caller
then simply kept the original file content, as it was unaffected
by the expansion.

The length of the string being 0 is not the same as the string
being NULL (expansion to an empty string vs. no expansion at all),
so checking result.len != 0 is not a full replacement for the old
NULL check.

However, I doubt the subtle optimization explained above resulted
in a notable speed-up anyway.  Simplify the code and add the tail
of the file to the expanded string unconditionally.

[jc: added a test to expose the breakage this fixes]

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin-archive.c b/builtin-archive.c
index b50d5ad19654153129e5b24a4aa585f78cef1d54..6fa424d3869c051fe7f5907f15e83bb5367a8ac4 100644 (file)
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -111,9 +111,7 @@ static void *format_subst(const struct commit *commit, const char *format,
                a = c + 1;
        }
 
                a = c + 1;
        }
 

-       if (result.len && len) {
-               strbuf_add(&result, a, len);
-       }
+       strbuf_add(&result, a, len);

 
        *sizep = result.len;
 
 
        *sizep = result.len;
 

diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index 42e28ab758358500183ee8ade35d7052c69ce9e1..dca2067b2d0bcd4423d843561b9275be50fe0da3 100755 (executable)
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -36,7 +36,8 @@ test_expect_success \
      echo simple textfile >a/a &&
      mkdir a/bin &&
      cp /bin/sh a/bin &&
      echo simple textfile >a/a &&
      mkdir a/bin &&
      cp /bin/sh a/bin &&

-     printf "A\$Format:%s\$O" "$SUBSTFORMAT" >a/substfile &&
+     printf "A\$Format:%s\$O" "$SUBSTFORMAT" >a/substfile1 &&
+     printf "A not substituted O" >a/substfile2 &&

      ln -s a a/l1 &&
      (p=long_path_to_a_file && cd a &&
       for depth in 1 2 3 4 5; do mkdir $p && cd $p; done &&
      ln -s a a/l1 &&
      (p=long_path_to_a_file && cd a &&
       for depth in 1 2 3 4 5; do mkdir $p && cd $p; done &&


@@ -108,20 +109,22 @@ test_expect_success \
     'diff -r a c/prefix/a'
 
 test_expect_success \
     'diff -r a c/prefix/a'
 
 test_expect_success \

-    'create an archive with a substfile' \
-    'echo substfile export-subst >a/.gitattributes &&
+    'create an archive with a substfiles' \
+    'echo "substfile?" export-subst >a/.gitattributes &&

      git archive HEAD >f.tar &&
      rm a/.gitattributes'
 
 test_expect_success \
      git archive HEAD >f.tar &&
      rm a/.gitattributes'
 
 test_expect_success \

-    'extract substfile' \
+    'extract substfiles' \

     '(mkdir f && cd f && $TAR xf -) <f.tar'
 
 test_expect_success \
      'validate substfile contents' \
      'git log --max-count=1 "--pretty=format:A${SUBSTFORMAT}O" HEAD \
     '(mkdir f && cd f && $TAR xf -) <f.tar'
 
 test_expect_success \
      'validate substfile contents' \
      'git log --max-count=1 "--pretty=format:A${SUBSTFORMAT}O" HEAD \

-      >f/a/substfile.expected &&
-      diff f/a/substfile.expected f/a/substfile'
+      >f/a/substfile1.expected &&
+      diff f/a/substfile1.expected f/a/substfile1 &&
+      diff a/substfile2 f/a/substfile2
+'

 
 test_expect_success \
     'git archive --format=zip' \
 
 test_expect_success \
     'git archive --format=zip' \