From: Kirill Smelkov <kirr@xxxxxxxxxxxxxxxxxxx>
Instead get the mode from either worktree, index, .git, or origin
entries when blaming and pass it to textconv_object() as context.
The reason to do it is not to run textconv filters on symlinks.
Cc: Axel Bonnet <axel.bonnet@xxxxxxxxxxxxxxx>
Cc: ClÃment Poulain <clement.poulain@xxxxxxxxxxxxxxx>
Cc: Diane Gasselin <diane.gasselin@xxxxxxxxxxxxxxx>
Cc: Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx>
Cc: Jeff King <peff@xxxxxxxx>
Signed-off-by: Kirill Smelkov <kirr@xxxxxxxxxxxxxxxxxxx>
Reviewed-by: Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx>
---
v5:
o No changes
v4:
o Update to resolve conflicts after patch 2 v4 update; no changes otherwise.
v3:
o Reviewed-by: Matthieu
v2:
o Thanks to Matthieu and Jeff got a bit more sure I'm not doing stupid things,
so
o My XXX were removed, and the patch is no longer an RFC
builtin.h | 2 +-
builtin/blame.c | 33 ++++++++++++++++++++++-----------
builtin/cat-file.c | 2 +-
sha1_name.c | 2 ++
t/t8006-blame-textconv.sh | 6 ++----
t/t8007-cat-file-textconv.sh | 6 ++----
6 files changed, 30 insertions(+), 21 deletions(-)
diff --git a/builtin.h b/builtin.h
index 0398d24..9bf69ee 100644
--- a/builtin.h
+++ b/builtin.h
@@ -35,7 +35,7 @@ void finish_copy_notes_for_rewrite(struct notes_rewrite_cfg *c);
extern int check_pager_config(const char *cmd);
-extern int textconv_object(const char *path, const unsigned char *sha1, char **buf, unsigned long *buf_size);
+extern int textconv_object(const char *path, unsigned mode, const unsigned char *sha1, char **buf, unsigned long *buf_size);
extern int cmd_add(int argc, const char **argv, const char *prefix);
extern int cmd_annotate(int argc, const char **argv, const char *prefix);
diff --git a/builtin/blame.c b/builtin/blame.c
index 1015354..f5fccc1 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -83,6 +83,7 @@ struct origin {
struct commit *commit;
mmfile_t file;
unsigned char blob_sha1[20];
+ unsigned mode;
char path[FLEX_ARRAY];
};
@@ -92,6 +93,7 @@ struct origin {
* Return 1 if the conversion succeeds, 0 otherwise.
*/
int textconv_object(const char *path,
+ unsigned mode,
const unsigned char *sha1,
char **buf,
unsigned long *buf_size)
@@ -100,7 +102,7 @@ int textconv_object(const char *path,
struct userdiff_driver *textconv;
df = alloc_filespec(path);
- fill_filespec(df, sha1, S_IFREG | 0664);
+ fill_filespec(df, sha1, mode);
textconv = get_textconv(df);
if (!textconv) {
free_filespec(df);
@@ -125,7 +127,7 @@ static void fill_origin_blob(struct diff_options *opt,
num_read_blob++;
if (DIFF_OPT_TST(opt, ALLOW_TEXTCONV) &&
- textconv_object(o->path, o->blob_sha1, &file->ptr, &file_size))
+ textconv_object(o->path, o->mode, o->blob_sha1, &file->ptr, &file_size))
;
else
file->ptr = read_sha1_file(o->blob_sha1, &type, &file_size);
@@ -313,21 +315,23 @@ static struct origin *get_origin(struct scoreboard *sb,
* for an origin is also used to pass the blame for the entire file to
* the parent to detect the case where a child's blob is identical to
* that of its parent's.
+ *
+ * This also fills origin->mode for corresponding tree path.
*/
-static int fill_blob_sha1(struct origin *origin)
+static int fill_blob_sha1_and_mode(struct origin *origin)
{
- unsigned mode;
if (!is_null_sha1(origin->blob_sha1))
return 0;
if (get_tree_entry(origin->commit->object.sha1,
origin->path,
- origin->blob_sha1, &mode))
+ origin->blob_sha1, &origin->mode))
goto error_out;
if (sha1_object_info(origin->blob_sha1, NULL) != OBJ_BLOB)
goto error_out;
return 0;
error_out:
hashclr(origin->blob_sha1);
+ origin->mode = S_IFINVALID;
return -1;
}
@@ -360,12 +364,14 @@ static struct origin *find_origin(struct scoreboard *sb,
/*
* If the origin was newly created (i.e. get_origin
* would call make_origin if none is found in the
- * scoreboard), it does not know the blob_sha1,
+ * scoreboard), it does not know the blob_sha1/mode,
* so copy it. Otherwise porigin was in the
- * scoreboard and already knows blob_sha1.
+ * scoreboard and already knows blob_sha1/mode.
*/
- if (porigin->refcnt == 1)
+ if (porigin->refcnt == 1) {
hashcpy(porigin->blob_sha1, cached->blob_sha1);
+ porigin->mode = cached->mode;
+ }
return porigin;
}
/* otherwise it was not very useful; free it */
@@ -400,6 +406,7 @@ static struct origin *find_origin(struct scoreboard *sb,
/* The path is the same as parent */
porigin = get_origin(sb, parent, origin->path);
hashcpy(porigin->blob_sha1, origin->blob_sha1);
+ porigin->mode = origin->mode;
} else {
/*
* Since origin->path is a pathspec, if the parent
@@ -425,6 +432,7 @@ static struct origin *find_origin(struct scoreboard *sb,
case 'M':
porigin = get_origin(sb, parent, origin->path);
hashcpy(porigin->blob_sha1, p->one->sha1);
+ porigin->mode = p->one->mode;
break;
case 'A':
case 'T':
@@ -444,6 +452,7 @@ static struct origin *find_origin(struct scoreboard *sb,
cached = make_origin(porigin->commit, porigin->path);
hashcpy(cached->blob_sha1, porigin->blob_sha1);
+ cached->mode = porigin->mode;
parent->util = cached;
}
return porigin;
@@ -486,6 +495,7 @@ static struct origin *find_rename(struct scoreboard *sb,
!strcmp(p->two->path, origin->path)) {
porigin = get_origin(sb, parent, p->one->path);
hashcpy(porigin->blob_sha1, p->one->sha1);
+ porigin->mode = p->one->mode;
break;
}
}
@@ -1099,6 +1109,7 @@ static int find_copy_in_parent(struct scoreboard *sb,
norigin = get_origin(sb, parent, p->one->path);
hashcpy(norigin->blob_sha1, p->one->sha1);
+ norigin->mode = p->one->mode;
fill_origin_blob(&sb->revs->diffopt, norigin, &file_p);
if (!file_p.ptr)
continue;
@@ -2075,7 +2086,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,
switch (st.st_mode & S_IFMT) {
case S_IFREG:
if (DIFF_OPT_TST(opt, ALLOW_TEXTCONV) &&
- textconv_object(read_from, null_sha1, &buf.buf, &buf_len))
+ textconv_object(read_from, mode, null_sha1, &buf.buf, &buf_len))
buf.len = buf_len;
else if (strbuf_read_file(&buf, read_from, st.st_size) != st.st_size)
die_errno("cannot open or read '%s'", read_from);
@@ -2455,11 +2466,11 @@ parse_done:
}
else {
o = get_origin(&sb, sb.final, path);
- if (fill_blob_sha1(o))
+ if (fill_blob_sha1_and_mode(o))
die("no such path %s in %s", path, final_commit_name);
if (DIFF_OPT_TST(&sb.revs->diffopt, ALLOW_TEXTCONV) &&
- textconv_object(path, o->blob_sha1, (char **) &sb.final_buf,
+ textconv_object(path, o->mode, o->blob_sha1, (char **) &sb.final_buf,
&sb.final_buf_size))
;
else
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 76ec3fe..94632db 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -143,7 +143,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
die("git cat-file --textconv %s: <object> must be <sha1:path>",
obj_name);
- if (!textconv_object(obj_context.path, sha1, &buf, &size))
+ if (!textconv_object(obj_context.path, obj_context.mode, sha1, &buf, &size))
die("git cat-file --textconv: unable to run textconv on %s",
obj_name);
break;
diff --git a/sha1_name.c b/sha1_name.c
index 484081d..3e856b8 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1069,6 +1069,7 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,
struct cache_entry *ce;
int pos;
if (namelen > 2 && name[1] == '/')
+ /* don't need mode for commit */
return get_sha1_oneline(name + 2, sha1);
if (namelen < 3 ||
name[2] != ':' ||
@@ -1096,6 +1097,7 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,
break;
if (ce_stage(ce) == stage) {
hashcpy(sha1, ce->sha1);
+ oc->mode = ce->ce_mode;
return 0;
}
pos++;
diff --git a/t/t8006-blame-textconv.sh b/t/t8006-blame-textconv.sh
index 7c35959..dbf623b 100755
--- a/t/t8006-blame-textconv.sh
+++ b/t/t8006-blame-textconv.sh
@@ -94,8 +94,7 @@ test_expect_success SYMLINKS 'blame with --no-textconv (on symlink)' '
test_cmp expected result
'
-# fails with '...symlink.bin is not "binary" file'
-test_expect_failure SYMLINKS 'blame --textconv (on symlink)' '
+test_expect_success SYMLINKS 'blame --textconv (on symlink)' '
git blame --textconv symlink.bin >blame &&
find_blame <blame >result &&
test_cmp expected result
@@ -114,8 +113,7 @@ EOF
GIT_AUTHOR_NAME=Number4 git commit -a -m Fourth --date="2010-01-01 23:00:00"
'
-# fails with '...symlink.bin is not "binary" file'
-test_expect_failure SYMLINKS 'blame on last commit (-C -C, symlink)' '
+test_expect_success SYMLINKS 'blame on last commit (-C -C, symlink)' '
git blame -C -C three.bin >blame &&
find_blame <blame >result &&
cat >expected <<\EOF &&
diff --git a/t/t8007-cat-file-textconv.sh b/t/t8007-cat-file-textconv.sh
index 98a3e1f..78a0085 100755
--- a/t/t8007-cat-file-textconv.sh
+++ b/t/t8007-cat-file-textconv.sh
@@ -79,8 +79,7 @@ test_expect_success SYMLINKS 'cat-file without --textconv (symlink)' '
'
-# fails because cat-file tries to run converter on symlink.bin
-test_expect_failure SYMLINKS 'cat-file --textconv on index (symlink)' '
+test_expect_success SYMLINKS 'cat-file --textconv on index (symlink)' '
! git cat-file --textconv :symlink.bin 2>result &&
cat >expected <<\EOF &&
fatal: git cat-file --textconv: unable to run textconv on :symlink.bin
@@ -88,8 +87,7 @@ EOF
test_cmp expected result
'
-# fails because cat-file tries to run converter on symlink.bin
-test_expect_failure SYMLINKS 'cat-file --textconv on HEAD (symlink)' '
+test_expect_success SYMLINKS 'cat-file --textconv on HEAD (symlink)' '
! git cat-file --textconv HEAD:symlink.bin 2>result &&
cat >expected <<EOF &&
fatal: git cat-file --textconv: unable to run textconv on HEAD:symlink.bin
--
1.7.3.19.g3fe0a
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html