From ffcabccf5df17f12997feedafefeb5589b8c0511 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 10 Sep 2012 16:30:20 -0700 Subject: [PATCH 1/2] blame $path: avoid getting fooled by case insensitive filesystems "git blame MAKEFILE" run in a history that has "Makefile" but not MAKEFILE can get confused on a case insensitive filesystem, because the check we run to see if there is a corresponding file in the working tree with lstat("MAKEFILE") succeeds. In addition to that check, we have to make sure that the given path also exists in the commit we start digging history from (i.e. "HEAD"). Note that this reveals the breakage in a test added in cd8ae20 (git-blame shouldn't crash if run in an unmerged tree, 2007-10-18), which expects the entire merge-in-progress path to be blamed to the working tree when it did not exist in our tree. As it is clear in the log message of that commit, the old breakage was that it was causing an internal error and the fix was about avoiding it. Just check that the command does not die an uncontrolled death. For this particular case, the blame should fail, as the history for the file in that contents has not been committed yet at the point in the test. Signed-off-by: Junio C Hamano --- builtin/blame.c | 16 ++++++++++++++-- t/t8004-blame-with-conflicts.sh | 4 ++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/builtin/blame.c b/builtin/blame.c index 324d476abf..83683b8287 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -2044,6 +2044,19 @@ static int git_blame_config(const char *var, const char *value, void *cb) return git_default_config(var, value, cb); } +static void verify_working_tree_path(unsigned char *head_sha1, const char *path) +{ + unsigned char blob_sha1[20]; + unsigned mode; + + if (!resolve_ref_unsafe("HEAD", head_sha1, 1, NULL)) + die("no such ref: HEAD"); + if (get_tree_entry(head_sha1, path, blob_sha1, &mode)) + die("no such path '%s' in HEAD", path); + if (sha1_object_info(blob_sha1, NULL) != OBJ_BLOB) + die("path '%s' in HEAD is not a blob", path); +} + /* * Prepare a dummy commit that represents the work tree (or staged) item. * Note that annotating work tree item never works in the reverse. @@ -2062,8 +2075,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt, struct cache_entry *ce; unsigned mode; - if (get_sha1("HEAD", head_sha1)) - die("No such ref: HEAD"); + verify_working_tree_path(head_sha1, path); time(&now); commit = xcalloc(1, sizeof(*commit)); diff --git a/t/t8004-blame-with-conflicts.sh b/t/t8004-blame-with-conflicts.sh index ba19ac127e..b4a260a0fc 100755 --- a/t/t8004-blame-with-conflicts.sh +++ b/t/t8004-blame-with-conflicts.sh @@ -66,8 +66,8 @@ test_expect_success \ git blame file2 ' -test_expect_success 'blame runs on conflicted file in stages 1,3' ' - git blame file1 +test_expect_success 'blame does not crash with conflicted file in stages 1,3' ' + test_must_fail git blame file1 ' test_done From 9aeaab6811dce596b4f6141d76f5300359bfd009 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 11 Sep 2012 14:30:03 -0700 Subject: [PATCH 2/2] blame: allow "blame file" in the middle of a conflicted merge "git blame file" has always meant "find the origin of each line of the file in the history leading to HEAD, oh by the way, blame the lines that are modified locally to the working tree". This teaches "git blame" that during a conflicted merge, some uncommitted changes may have come from the other history that is being merged. The verify_working_tree_path() function introduced in the previous patch to notice a typo in the filename (primarily on case insensitive filesystems) has been updated to allow a filename that does not exist in HEAD (i.e. the tip of our history) as long as it exists one of the commits being merged, so that a "we deleted, the other side modified" case tracks the history of the file in the history of the other side. Signed-off-by: Junio C Hamano --- builtin/blame.c | 93 ++++++++++++++++++++++++--------- t/t8004-blame-with-conflicts.sh | 2 +- 2 files changed, 70 insertions(+), 25 deletions(-) diff --git a/builtin/blame.c b/builtin/blame.c index 83683b8287..449880e802 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -2044,17 +2044,53 @@ static int git_blame_config(const char *var, const char *value, void *cb) return git_default_config(var, value, cb); } -static void verify_working_tree_path(unsigned char *head_sha1, const char *path) +static void verify_working_tree_path(struct commit *work_tree, const char *path) { - unsigned char blob_sha1[20]; - unsigned mode; + struct commit_list *parents; - if (!resolve_ref_unsafe("HEAD", head_sha1, 1, NULL)) - die("no such ref: HEAD"); - if (get_tree_entry(head_sha1, path, blob_sha1, &mode)) - die("no such path '%s' in HEAD", path); - if (sha1_object_info(blob_sha1, NULL) != OBJ_BLOB) - die("path '%s' in HEAD is not a blob", path); + for (parents = work_tree->parents; parents; parents = parents->next) { + const unsigned char *commit_sha1 = parents->item->object.sha1; + unsigned char blob_sha1[20]; + unsigned mode; + + if (!get_tree_entry(commit_sha1, path, blob_sha1, &mode) && + sha1_object_info(blob_sha1, NULL) == OBJ_BLOB) + return; + } + die("no such path '%s' in HEAD", path); +} + +static struct commit_list **append_parent(struct commit_list **tail, const unsigned char *sha1) +{ + struct commit *parent; + + parent = lookup_commit_reference(sha1); + if (!parent) + die("no such commit %s", sha1_to_hex(sha1)); + return &commit_list_insert(parent, tail)->next; +} + +static void append_merge_parents(struct commit_list **tail) +{ + int merge_head; + const char *merge_head_file = git_path("MERGE_HEAD"); + struct strbuf line = STRBUF_INIT; + + merge_head = open(merge_head_file, O_RDONLY); + if (merge_head < 0) { + if (errno == ENOENT) + return; + die("cannot open '%s' for reading", merge_head_file); + } + + while (!strbuf_getwholeline_fd(&line, merge_head, '\n')) { + unsigned char sha1[20]; + if (line.len < 40 || get_sha1_hex(line.buf, sha1)) + die("unknown line in '%s': %s", merge_head_file, line.buf); + tail = append_parent(tail, sha1); + } + close(merge_head); + strbuf_release(&line); } /* @@ -2067,6 +2103,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt, { struct commit *commit; struct origin *origin; + struct commit_list **parent_tail, *parent; unsigned char head_sha1[20]; struct strbuf buf = STRBUF_INIT; const char *ident; @@ -2074,19 +2111,38 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt, int size, len; struct cache_entry *ce; unsigned mode; - - verify_working_tree_path(head_sha1, path); + struct strbuf msg = STRBUF_INIT; time(&now); commit = xcalloc(1, sizeof(*commit)); - commit->parents = xcalloc(1, sizeof(*commit->parents)); - commit->parents->item = lookup_commit_reference(head_sha1); commit->object.parsed = 1; commit->date = now; commit->object.type = OBJ_COMMIT; + parent_tail = &commit->parents; + + if (!resolve_ref_unsafe("HEAD", head_sha1, 1, NULL)) + die("no such ref: HEAD"); + + parent_tail = append_parent(parent_tail, head_sha1); + append_merge_parents(parent_tail); + verify_working_tree_path(commit, path); origin = make_origin(commit, path); + ident = fmt_ident("Not Committed Yet", "not.committed.yet", NULL, 0); + strbuf_addstr(&msg, "tree 0000000000000000000000000000000000000000\n"); + for (parent = commit->parents; parent; parent = parent->next) + strbuf_addf(&msg, "parent %s\n", + sha1_to_hex(parent->item->object.sha1)); + strbuf_addf(&msg, + "author %s\n" + "committer %s\n\n" + "Version of %s from %s\n", + ident, ident, path, + (!contents_from ? path : + (!strcmp(contents_from, "-") ? "standard input" : contents_from))); + commit->buffer = strbuf_detach(&msg, NULL); + if (!contents_from || strcmp("-", contents_from)) { struct stat st; const char *read_from; @@ -2123,7 +2179,6 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt, } else { /* Reading from stdin */ - contents_from = "standard input"; mode = 0; if (strbuf_read(&buf, 0, 0) < 0) die_errno("failed to read from stdin"); @@ -2167,16 +2222,6 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt, */ cache_tree_invalidate_path(active_cache_tree, path); - commit->buffer = xmalloc(400); - ident = fmt_ident("Not Committed Yet", "not.committed.yet", NULL, 0); - snprintf(commit->buffer, 400, - "tree 0000000000000000000000000000000000000000\n" - "parent %s\n" - "author %s\n" - "committer %s\n\n" - "Version of %s from %s\n", - sha1_to_hex(head_sha1), - ident, ident, path, contents_from ? contents_from : path); return commit; } diff --git a/t/t8004-blame-with-conflicts.sh b/t/t8004-blame-with-conflicts.sh index b4a260a0fc..9c353ab222 100755 --- a/t/t8004-blame-with-conflicts.sh +++ b/t/t8004-blame-with-conflicts.sh @@ -67,7 +67,7 @@ test_expect_success \ ' test_expect_success 'blame does not crash with conflicted file in stages 1,3' ' - test_must_fail git blame file1 + git blame file1 ' test_done