{"thread":{"id":"25287","subject":"[BUG, PATCH v5 0/3] Fix {blame,cat-file} --textconv for cases with symlinks","startedAt":"2010-09-29T11:35:21Z","lastAt":"2010-10-24T17:40:08Z","messageCount":6,"participants":["Kirill Smelkov","Jeff King"],"isPatch":true,"patchVersion":5,"patchTotal":3},"messages":[{"id":"152039","messageId":"cover.1285758714.git.kirr@mns.spb.ru","threadId":"25287","inReplyTo":null,"subject":"[BUG, PATCH v5 0/3] Fix {blame,cat-file} --textconv for cases with symlinks","fromName":"Kirill Smelkov","fromEmail":"kirr@mns.spb.ru","sentAt":"2010-09-29T11:35:21Z","receivedAt":"2010-09-29T11:35:21Z","isPatch":true,"sender":{"key":"kirr@navytux.spb.ru","avatar":"https://gravatar.com/avatar/cf3445fdad1849941e17ab25bf1ee7c5ea1be2deb444be25ff5f36b0e50a985f?d=mp&s=160"},"body":"Recently I've spot a bug in git blame --textconv, which was wrongly\ncalling pdftotext (my *.pdf conversion program) on a symlink.pdf, and I\nwas getting something like\n\n $ git blame -C -C regular-file.pdf\n Error: May not be a PDF file (continuing anyway)\n Error: PDF file is damaged - attempting to reconstruct xref table...\n Error: Couldn't find trailer dictionary\n Error: Couldn't read xref table\n Warning: program returned non-zero exit code #1\n fatal: unable to read files to diff\n\nThat errors come from pdftotext run on symlink.pdf being extracted to\n/tmp/ with one-line plain-text content pointing to link destination.\n\n\nPlease apply and thanks,\nKirill\n\n\nP.S. I'm sorry if this time there is again some bug on my side...\n\n\nv5:\n\n o Avoid touching t4042 at all\n o Change $@ to $1 in textconv helper directly in patch1\n\nv4:\n\n o add prereq on SYMLINKS in tests\n o Use consistent pattern for detecting and converting binaries (was 'bin:' and\n 'bin: ')\n o avoid using $@ in textconv helper - it gets only one argument\n\nv3:\n\n o Slightly changed patches descriptions as per comment by Matthieu, and added\n Matthieu's Reviewed-by.\n\nv2:\n\n o Incorporated suggestions by Matthieu and Jeff (details in each patch)\n\nKirill Smelkov (3):\n tests: Prepare --textconv tests for correctly-failing conversion\n program\n blame,cat-file: Demonstrate --textconv is wrongly running converter\n on symlinks\n blame,cat-file --textconv: Don't assume mode is ``S_IFREF | 0664''\n\n builtin.h | 2 +-\n builtin/blame.c | 33 +++++++++++++++-------\n builtin/cat-file.c | 2 +-\n sha1_name.c | 2 +\n t/t8006-blame-textconv.sh | 62 +++++++++++++++++++++++++++++++++++++-----\n t/t8007-cat-file-textconv.sh | 38 ++++++++++++++++++++++---\n 6 files changed, 114 insertions(+), 25 deletions(-)\n\n-- \n1.7.3.19.g3fe0a\n"},{"id":"152040","messageId":"6b9535a19651c2354eac6634ba88b6fb3442044b.1285758714.git.kirr@mns.spb.ru","threadId":"25287","inReplyTo":"cover.1285758714.git.kirr@mns.spb.ru","subject":"[PATCH v5 1/3] blame,cat-file: Prepare --textconv tests for correctly-failing conversion program","fromName":"Kirill Smelkov","fromEmail":"kirr@mns.spb.ru","sentAt":"2010-09-29T11:35:22Z","receivedAt":"2010-09-29T11:35:22Z","isPatch":true,"sender":{"key":"kirr@navytux.spb.ru","avatar":"https://gravatar.com/avatar/cf3445fdad1849941e17ab25bf1ee7c5ea1be2deb444be25ff5f36b0e50a985f?d=mp&s=160"},"body":"From: Kirill Smelkov \n\nThe textconv filter is sometimes incorrectly ran on a temporary file\nwhose content is the target of a symbolic link, instead of actual file\ncontent. Prepare to test this by marking the content of the file to\nconvert with \"bin:\", and let the helper die if \"bin:\" is not found in\nthe file content.\n\nNOTE: I've changed $@ to $1 in helper becase textconv program \"should\ntake a single argument\" (see Documentation/gitattributes.txt), so\nmaking this more explicit makes sense and also helps to avoid\nproblems with feeding arguments to echo.\n\n(Description partly by Matthieu Moy)\n\nCc: Axel Bonnet \nCc: Clément Poulain \nCc: Diane Gasselin \nCc: Matthieu Moy \nCc: Jeff King \nSigned-off-by: Kirill Smelkov \nReviewed-by: Matthieu Moy \n---\n\nv5:\n\n o Avoid touching t4042-diff-textconv-caching.sh at all.\n o Use $1 instead of $@ in textconv helper\n\nv4:\n\n o Use consistent pattern for detecting and converting binaries (was 'bin:' and\n 'bin: ')\n\nv3:\n\n o Add Matthieu's Reviewed-by, and move secondary note to the end to avoid\n distracting an intrested reader.\n\nv2:\n\n o Changed patch description as suggested by Matthieu\n\n\n\n t/t8006-blame-textconv.sh | 15 ++++++++-------\n t/t8007-cat-file-textconv.sh | 11 ++++++-----\n 2 files changed, 14 insertions(+), 12 deletions(-)\n\ndiff --git a/t/t8006-blame-textconv.sh b/t/t8006-blame-textconv.sh\nindex 9ad96d4..3619f8a 100755\n--- a/t/t8006-blame-textconv.sh\n+++ b/t/t8006-blame-textconv.sh\n@@ -9,22 +9,23 @@ find_blame() {\n \n cat >helper <<'EOF'\n #!/bin/sh\n-sed 's/^/converted: /' \"$@\"\n+grep -q '^bin: ' \"$1\" || { echo \"E: $1 is not \\\"binary\\\" file\" 1>&2; exit 1; }\n+sed 's/^bin: /converted: /' \"$1\"\n EOF\n chmod +x helper\n \n test_expect_success 'setup ' '\n-\techo test 1 >one.bin &&\n-\techo test number 2 >two.bin &&\n+\techo \"bin: test 1\" >one.bin &&\n+\techo \"bin: test number 2\" >two.bin &&\n \tgit add . &&\n \tGIT_AUTHOR_NAME=Number1 git commit -a -m First --date=\"2010-01-01 18:00:00\" &&\n-\techo test 1 version 2 >one.bin &&\n-\techo test number 2 version 2 >>two.bin &&\n+\techo \"bin: test 1 version 2\" >one.bin &&\n+\techo \"bin: test number 2 version 2\" >>two.bin &&\n \tGIT_AUTHOR_NAME=Number2 git commit -a -m Second --date=\"2010-01-01 20:00:00\"\n '\n \n cat >expected <>two.bin &&\n+\techo \"bin: test number 2 version 3\" >>two.bin &&\n \tGIT_AUTHOR_NAME=Number3 git commit -a -m Third --date=\"2010-01-01 22:00:00\"\n '\n \ndiff --git a/t/t8007-cat-file-textconv.sh b/t/t8007-cat-file-textconv.sh\nindex 38ac05e..71f4145 100755\n--- a/t/t8007-cat-file-textconv.sh\n+++ b/t/t8007-cat-file-textconv.sh\n@@ -5,15 +5,16 @@ test_description='git cat-file textconv support'\n \n cat >helper <<'EOF'\n #!/bin/sh\n-sed 's/^/converted: /' \"$@\"\n+grep -q '^bin: ' \"$1\" || { echo \"E: $1 is not \\\"binary\\\" file\" 1>&2; exit 1; }\n+sed 's/^bin: /converted: /' \"$1\"\n EOF\n chmod +x helper\n \n test_expect_success 'setup ' '\n-\techo test >one.bin &&\n+\techo \"bin: test\" >one.bin &&\n \tgit add . &&\n \tGIT_AUTHOR_NAME=Number1 git commit -a -m First --date=\"2010-01-01 18:00:00\" &&\n-\techo test version 2 >one.bin &&\n+\techo \"bin: test version 2\" >one.bin &&\n \tGIT_AUTHOR_NAME=Number2 git commit -a -m Second --date=\"2010-01-01 20:00:00\"\n '\n \n@@ -33,7 +34,7 @@ test_expect_success 'setup textconv filters' '\n '\n \n cat >expected <expected <\n\ngit blame --textconv is wrongly calling the textconv filter on\nsymlinks: symlinks are stored as blobs whose content is the target of\nthe link, and blame calls the textconv filter on a temporary file\nfilled-in with the content of this blob.\n\nFor example:\n\n $ git blame -C -C regular-file.pdf\n Error: May not be a PDF file (continuing anyway)\n Error: PDF file is damaged - attempting to reconstruct xref table...\n Error: Couldn't find trailer dictionary\n Error: Couldn't read xref table\n Warning: program returned non-zero exit code #1\n fatal: unable to read files to diff\n\nThat errors come from pdftotext run on symlink.pdf being extracted to\n/tmp/ with one-line plain-text content pointing to link destination.\n\nSo several failures are demonstrated here:\n\n - git cat-file --textconv :symlink.bin # also HEAD:symlink.bin\n - git blame --textconv symlink.bin\n - git blame -C -C --textconv regular-file # but also looks on symlink.bin\n\nAt present they all fail with something like.\n\n E: /tmp/j3ELEs_symlink.bin is not \"binary\" file\n\nNOTE: git diff doesn't try to textconv the pathnames, it runs the\ntextual diff without textconv, which is the expected behavior.\n\n(Description partly by Matthieu Moy)\n\nCc: Axel Bonnet \nCc: Clément Poulain \nCc: Diane Gasselin \nCc: Matthieu Moy \nCc: Jeff King \nSigned-off-by: Kirill Smelkov \nReviewed-by: Matthieu Moy \n---\n\nv5:\n\n o No changes\n\nv4:\n\n o As noticed by Junio add prereq on SYMLINKS where appropriate\n\nv3:\n\n o Slight cleanup of the description\n o Reviewed-by: Matthieu\n\nv2:\n\n (As suggested by Matthieu)\n o Changed patch descriptio\n o Moved most of >expected preparation into test_expect_*\n o Changed multiple echo'es into cat <one.bin &&\n \techo \"bin: test number 2\" >two.bin &&\n+\tif test_have_prereq SYMLINKS; then\n+\t\tln -s one.bin symlink.bin\n+\tfi &&\n \tgit add . &&\n \tGIT_AUTHOR_NAME=Number1 git commit -a -m First --date=\"2010-01-01 18:00:00\" &&\n \techo \"bin: test 1 version 2\" >one.bin &&\n \techo \"bin: test number 2 version 2\" >>two.bin &&\n+\tif test_have_prereq SYMLINKS; then\n+\t\tln -sf two.bin symlink.bin\n+\tfi &&\n \tGIT_AUTHOR_NAME=Number2 git commit -a -m Second --date=\"2010-01-01 20:00:00\"\n '\n \n@@ -78,4 +84,47 @@ test_expect_success 'blame from previous revision' '\n \ttest_cmp expected result\n '\n \n+cat >expected <blame &&\n+\tfind_blame result &&\n+\ttest_cmp expected result\n+'\n+\n+# fails with '...symlink.bin is not \"binary\" file'\n+test_expect_failure SYMLINKS 'blame --textconv (on symlink)' '\n+\tgit blame --textconv symlink.bin >blame &&\n+\tfind_blame result &&\n+\ttest_cmp expected result\n+'\n+\n+# cp two.bin three.bin and make small tweak\n+# (this will direct blame -C -C three.bin to consider two.bin and symlink.bin)\n+test_expect_success SYMLINKS 'make another new commit' '\n+\tcat >three.bin <<\\EOF &&\n+bin: test number 2\n+bin: test number 2 version 2\n+bin: test number 2 version 3\n+bin: test number 3\n+EOF\n+\tgit add three.bin &&\n+\tGIT_AUTHOR_NAME=Number4 git commit -a -m Fourth --date=\"2010-01-01 23:00:00\"\n+'\n+\n+# fails with '...symlink.bin is not \"binary\" file'\n+test_expect_failure SYMLINKS 'blame on last commit (-C -C, symlink)' '\n+\tgit blame -C -C three.bin >blame &&\n+\tfind_blame result &&\n+\tcat >expected <<\\EOF &&\n+(Number1 2010-01-01 18:00:00 +0000 1) converted: test number 2\n+(Number2 2010-01-01 20:00:00 +0000 2) converted: test number 2 version 2\n+(Number3 2010-01-01 22:00:00 +0000 3) converted: test number 2 version 3\n+(Number4 2010-01-01 23:00:00 +0000 4) converted: test number 3\n+EOF\n+\ttest_cmp expected result\n+'\n+\n test_done\ndiff --git a/t/t8007-cat-file-textconv.sh b/t/t8007-cat-file-textconv.sh\nindex 71f4145..98a3e1f 100755\n--- a/t/t8007-cat-file-textconv.sh\n+++ b/t/t8007-cat-file-textconv.sh\n@@ -12,6 +12,9 @@ chmod +x helper\n \n test_expect_success 'setup ' '\n \techo \"bin: test\" >one.bin &&\n+\tif test_have_prereq SYMLINKS; then\n+\t\tln -s one.bin symlink.bin\n+\tfi &&\n \tgit add . &&\n \tGIT_AUTHOR_NAME=Number1 git commit -a -m First --date=\"2010-01-01 18:00:00\" &&\n \techo \"bin: test version 2\" >one.bin &&\n@@ -68,4 +71,30 @@ test_expect_success 'cat-file --textconv on previous commit' '\n \tgit cat-file --textconv HEAD^:one.bin >result &&\n \ttest_cmp expected result\n '\n+\n+test_expect_success SYMLINKS 'cat-file without --textconv (symlink)' '\n+\tgit cat-file blob :symlink.bin >result &&\n+\tprintf \"%s\" \"one.bin\" >expected\n+\ttest_cmp expected result\n+'\n+\n+\n+# fails because cat-file tries to run converter on symlink.bin\n+test_expect_failure SYMLINKS 'cat-file --textconv on index (symlink)' '\n+\t! git cat-file --textconv :symlink.bin 2>result &&\n+\tcat >expected <<\\EOF &&\n+fatal: git cat-file --textconv: unable to run textconv on :symlink.bin\n+EOF\n+\ttest_cmp expected result\n+'\n+\n+# fails because cat-file tries to run converter on symlink.bin\n+test_expect_failure SYMLINKS 'cat-file --textconv on HEAD (symlink)' '\n+\t! git cat-file --textconv HEAD:symlink.bin 2>result &&\n+\tcat >expected <\n\nInstead get the mode from either worktree, index, .git, or origin\nentries when blaming and pass it to textconv_object() as context.\n\nThe reason to do it is not to run textconv filters on symlinks.\n\nCc: Axel Bonnet \nCc: Clément Poulain \nCc: Diane Gasselin \nCc: Matthieu Moy \nCc: Jeff King \nSigned-off-by: Kirill Smelkov \nReviewed-by: Matthieu Moy \n---\n\nv5:\n\n o No changes\n\nv4:\n\n o Update to resolve conflicts after patch 2 v4 update; no changes otherwise.\n\nv3:\n\n o Reviewed-by: Matthieu\n\nv2:\n\n o Thanks to Matthieu and Jeff got a bit more sure I'm not doing stupid things,\n so\n o My XXX were removed, and the patch is no longer an RFC\n\n builtin.h | 2 +-\n builtin/blame.c | 33 ++++++++++++++++++++++-----------\n builtin/cat-file.c | 2 +-\n sha1_name.c | 2 ++\n t/t8006-blame-textconv.sh | 6 ++----\n t/t8007-cat-file-textconv.sh | 6 ++----\n 6 files changed, 30 insertions(+), 21 deletions(-)\n\ndiff --git a/builtin.h b/builtin.h\nindex 0398d24..9bf69ee 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -35,7 +35,7 @@ void finish_copy_notes_for_rewrite(struct notes_rewrite_cfg *c);\n \n extern int check_pager_config(const char *cmd);\n \n-extern int textconv_object(const char *path, const unsigned char *sha1, char **buf, unsigned long *buf_size);\n+extern int textconv_object(const char *path, unsigned mode, const unsigned char *sha1, char **buf, unsigned long *buf_size);\n \n extern int cmd_add(int argc, const char **argv, const char *prefix);\n extern int cmd_annotate(int argc, const char **argv, const char *prefix);\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 1015354..f5fccc1 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -83,6 +83,7 @@ struct origin {\n \tstruct commit *commit;\n \tmmfile_t file;\n \tunsigned char blob_sha1[20];\n+\tunsigned mode;\n \tchar path[FLEX_ARRAY];\n };\n \n@@ -92,6 +93,7 @@ struct origin {\n * Return 1 if the conversion succeeds, 0 otherwise.\n */\n int textconv_object(const char *path,\n+\t\t unsigned mode,\n \t\t const unsigned char *sha1,\n \t\t char **buf,\n \t\t unsigned long *buf_size)\n@@ -100,7 +102,7 @@ int textconv_object(const char *path,\n \tstruct userdiff_driver *textconv;\n \n \tdf = alloc_filespec(path);\n-\tfill_filespec(df, sha1, S_IFREG | 0664);\n+\tfill_filespec(df, sha1, mode);\n \ttextconv = get_textconv(df);\n \tif (!textconv) {\n \t\tfree_filespec(df);\n@@ -125,7 +127,7 @@ static void fill_origin_blob(struct diff_options *opt,\n \n \t\tnum_read_blob++;\n \t\tif (DIFF_OPT_TST(opt, ALLOW_TEXTCONV) &&\n-\t\t textconv_object(o->path, o->blob_sha1, &file->ptr, &file_size))\n+\t\t textconv_object(o->path, o->mode, o->blob_sha1, &file->ptr, &file_size))\n \t\t\t;\n \t\telse\n \t\t\tfile->ptr = read_sha1_file(o->blob_sha1, &type, &file_size);\n@@ -313,21 +315,23 @@ static struct origin *get_origin(struct scoreboard *sb,\n * for an origin is also used to pass the blame for the entire file to\n * the parent to detect the case where a child's blob is identical to\n * that of its parent's.\n+ *\n+ * This also fills origin->mode for corresponding tree path.\n */\n-static int fill_blob_sha1(struct origin *origin)\n+static int fill_blob_sha1_and_mode(struct origin *origin)\n {\n-\tunsigned mode;\n \tif (!is_null_sha1(origin->blob_sha1))\n \t\treturn 0;\n \tif (get_tree_entry(origin->commit->object.sha1,\n \t\t\t origin->path,\n-\t\t\t origin->blob_sha1, &mode))\n+\t\t\t origin->blob_sha1, &origin->mode))\n \t\tgoto error_out;\n \tif (sha1_object_info(origin->blob_sha1, NULL) != OBJ_BLOB)\n \t\tgoto error_out;\n \treturn 0;\n error_out:\n \thashclr(origin->blob_sha1);\n+\torigin->mode = S_IFINVALID;\n \treturn -1;\n }\n \n@@ -360,12 +364,14 @@ static struct origin *find_origin(struct scoreboard *sb,\n \t\t\t/*\n \t\t\t * If the origin was newly created (i.e. get_origin\n \t\t\t * would call make_origin if none is found in the\n-\t\t\t * scoreboard), it does not know the blob_sha1,\n+\t\t\t * scoreboard), it does not know the blob_sha1/mode,\n \t\t\t * so copy it. Otherwise porigin was in the\n-\t\t\t * scoreboard and already knows blob_sha1.\n+\t\t\t * scoreboard and already knows blob_sha1/mode.\n \t\t\t */\n-\t\t\tif (porigin->refcnt == 1)\n+\t\t\tif (porigin->refcnt == 1) {\n \t\t\t\thashcpy(porigin->blob_sha1, cached->blob_sha1);\n+\t\t\t\tporigin->mode = cached->mode;\n+\t\t\t}\n \t\t\treturn porigin;\n \t\t}\n \t\t/* otherwise it was not very useful; free it */\n@@ -400,6 +406,7 @@ static struct origin *find_origin(struct scoreboard *sb,\n \t\t/* The path is the same as parent */\n \t\tporigin = get_origin(sb, parent, origin->path);\n \t\thashcpy(porigin->blob_sha1, origin->blob_sha1);\n+\t\tporigin->mode = origin->mode;\n \t} else {\n \t\t/*\n \t\t * Since origin->path is a pathspec, if the parent\n@@ -425,6 +432,7 @@ static struct origin *find_origin(struct scoreboard *sb,\n \t\tcase 'M':\n \t\t\tporigin = get_origin(sb, parent, origin->path);\n \t\t\thashcpy(porigin->blob_sha1, p->one->sha1);\n+\t\t\tporigin->mode = p->one->mode;\n \t\t\tbreak;\n \t\tcase 'A':\n \t\tcase 'T':\n@@ -444,6 +452,7 @@ static struct origin *find_origin(struct scoreboard *sb,\n \n \t\tcached = make_origin(porigin->commit, porigin->path);\n \t\thashcpy(cached->blob_sha1, porigin->blob_sha1);\n+\t\tcached->mode = porigin->mode;\n \t\tparent->util = cached;\n \t}\n \treturn porigin;\n@@ -486,6 +495,7 @@ static struct origin *find_rename(struct scoreboard *sb,\n \t\t !strcmp(p->two->path, origin->path)) {\n \t\t\tporigin = get_origin(sb, parent, p->one->path);\n \t\t\thashcpy(porigin->blob_sha1, p->one->sha1);\n+\t\t\tporigin->mode = p->one->mode;\n \t\t\tbreak;\n \t\t}\n \t}\n@@ -1099,6 +1109,7 @@ static int find_copy_in_parent(struct scoreboard *sb,\n \n \t\t\tnorigin = get_origin(sb, parent, p->one->path);\n \t\t\thashcpy(norigin->blob_sha1, p->one->sha1);\n+\t\t\tnorigin->mode = p->one->mode;\n \t\t\tfill_origin_blob(&sb->revs->diffopt, norigin, &file_p);\n \t\t\tif (!file_p.ptr)\n \t\t\t\tcontinue;\n@@ -2075,7 +2086,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,\n \t\tswitch (st.st_mode & S_IFMT) {\n \t\tcase S_IFREG:\n \t\t\tif (DIFF_OPT_TST(opt, ALLOW_TEXTCONV) &&\n-\t\t\t textconv_object(read_from, null_sha1, &buf.buf, &buf_len))\n+\t\t\t textconv_object(read_from, mode, null_sha1, &buf.buf, &buf_len))\n \t\t\t\tbuf.len = buf_len;\n \t\t\telse if (strbuf_read_file(&buf, read_from, st.st_size) != st.st_size)\n \t\t\t\tdie_errno(\"cannot open or read '%s'\", read_from);\n@@ -2455,11 +2466,11 @@ parse_done:\n \t}\n \telse {\n \t\to = get_origin(&sb, sb.final, path);\n-\t\tif (fill_blob_sha1(o))\n+\t\tif (fill_blob_sha1_and_mode(o))\n \t\t\tdie(\"no such path %s in %s\", path, final_commit_name);\n \n \t\tif (DIFF_OPT_TST(&sb.revs->diffopt, ALLOW_TEXTCONV) &&\n-\t\t textconv_object(path, o->blob_sha1, (char **) &sb.final_buf,\n+\t\t textconv_object(path, o->mode, o->blob_sha1, (char **) &sb.final_buf,\n \t\t\t\t &sb.final_buf_size))\n \t\t\t;\n \t\telse\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 76ec3fe..94632db 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -143,7 +143,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)\n \t\t\tdie(\"git cat-file --textconv %s: must be \",\n \t\t\t obj_name);\n \n-\t\tif (!textconv_object(obj_context.path, sha1, &buf, &size))\n+\t\tif (!textconv_object(obj_context.path, obj_context.mode, sha1, &buf, &size))\n \t\t\tdie(\"git cat-file --textconv: unable to run textconv on %s\",\n \t\t\t obj_name);\n \t\tbreak;\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 484081d..3e856b8 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -1069,6 +1069,7 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,\n \t\tstruct cache_entry *ce;\n \t\tint pos;\n \t\tif (namelen > 2 && name[1] == '/')\n+\t\t\t/* don't need mode for commit */\n \t\t\treturn get_sha1_oneline(name + 2, sha1);\n \t\tif (namelen < 3 ||\n \t\t name[2] != ':' ||\n@@ -1096,6 +1097,7 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,\n \t\t\t\tbreak;\n \t\t\tif (ce_stage(ce) == stage) {\n \t\t\t\thashcpy(sha1, ce->sha1);\n+\t\t\t\toc->mode = ce->ce_mode;\n \t\t\t\treturn 0;\n \t\t\t}\n \t\t\tpos++;\ndiff --git a/t/t8006-blame-textconv.sh b/t/t8006-blame-textconv.sh\nindex 7c35959..dbf623b 100755\n--- a/t/t8006-blame-textconv.sh\n+++ b/t/t8006-blame-textconv.sh\n@@ -94,8 +94,7 @@ test_expect_success SYMLINKS 'blame with --no-textconv (on symlink)' '\n \ttest_cmp expected result\n '\n \n-# fails with '...symlink.bin is not \"binary\" file'\n-test_expect_failure SYMLINKS 'blame --textconv (on symlink)' '\n+test_expect_success SYMLINKS 'blame --textconv (on symlink)' '\n \tgit blame --textconv symlink.bin >blame &&\n \tfind_blame result &&\n \ttest_cmp expected result\n@@ -114,8 +113,7 @@ EOF\n \tGIT_AUTHOR_NAME=Number4 git commit -a -m Fourth --date=\"2010-01-01 23:00:00\"\n '\n \n-# fails with '...symlink.bin is not \"binary\" file'\n-test_expect_failure SYMLINKS 'blame on last commit (-C -C, symlink)' '\n+test_expect_success SYMLINKS 'blame on last commit (-C -C, symlink)' '\n \tgit blame -C -C three.bin >blame &&\n \tfind_blame result &&\n \tcat >expected <<\\EOF &&\ndiff --git a/t/t8007-cat-file-textconv.sh b/t/t8007-cat-file-textconv.sh\nindex 98a3e1f..78a0085 100755\n--- a/t/t8007-cat-file-textconv.sh\n+++ b/t/t8007-cat-file-textconv.sh\n@@ -79,8 +79,7 @@ test_expect_success SYMLINKS 'cat-file without --textconv (symlink)' '\n '\n \n \n-# fails because cat-file tries to run converter on symlink.bin\n-test_expect_failure SYMLINKS 'cat-file --textconv on index (symlink)' '\n+test_expect_success SYMLINKS 'cat-file --textconv on index (symlink)' '\n \t! git cat-file --textconv :symlink.bin 2>result &&\n \tcat >expected <<\\EOF &&\n fatal: git cat-file --textconv: unable to run textconv on :symlink.bin\n@@ -88,8 +87,7 @@ EOF\n \ttest_cmp expected result\n '\n \n-# fails because cat-file tries to run converter on symlink.bin\n-test_expect_failure SYMLINKS 'cat-file --textconv on HEAD (symlink)' '\n+test_expect_success SYMLINKS 'cat-file --textconv on HEAD (symlink)' '\n \t! git cat-file --textconv HEAD:symlink.bin 2>result &&\n \tcat >expected < Kirill Smelkov (3):\n> tests: Prepare --textconv tests for correctly-failing conversion\n> program\n> blame,cat-file: Demonstrate --textconv is wrongly running converter\n> on symlinks\n> blame,cat-file --textconv: Don't assume mode is ``S_IFREF | 0664''\n\nI finally got around to reviewing this series again (thanks for your\npatience, Kirill). This latest version (v5) looks good to me.\n\n-Peff\n"},{"id":"154344","messageId":"20101024174008.GA28961@landau.phys.spbu.ru","threadId":"25287","inReplyTo":"20101022200516.GA13926@sigill.intra.peff.net","subject":"Re: [BUG, PATCH v5 0/3] Fix {blame,cat-file} --textconv for cases with symlinks","fromName":"Kirill Smelkov","fromEmail":"kirr@landau.phys.spbu.ru","sentAt":"2010-10-24T17:40:08Z","receivedAt":"2010-10-24T17:40:08Z","isPatch":true,"sender":{"key":"kirr@navytux.spb.ru","avatar":"https://gravatar.com/avatar/cf3445fdad1849941e17ab25bf1ee7c5ea1be2deb444be25ff5f36b0e50a985f?d=mp&s=160"},"body":"On Fri, Oct 22, 2010 at 04:05:16PM -0400, Jeff King wrote:\n> On Wed, Sep 29, 2010 at 03:35:21PM +0400, Kirill Smelkov wrote:\n> \n> > Kirill Smelkov (3):\n> > tests: Prepare --textconv tests for correctly-failing conversion\n> > program\n> > blame,cat-file: Demonstrate --textconv is wrongly running converter\n> > on symlinks\n> > blame,cat-file --textconv: Don't assume mode is ``S_IFREF | 0664''\n> \n> I finally got around to reviewing this series again (thanks for your\n> patience, Kirill). This latest version (v5) looks good to me.\n\nThanks, Jeff. And thanks for your and everyone's patience with me too.\n\nKirill\n"}]}