Re: [PATCH/RFC] blame: respect "core.ignorecase"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> If we were to do anything, I would think the most sane thing to do
> is a smaller patch to fix fake_working_tree_commit() where it calls
> lstat() and _should_ die with "Cannot lstat MakeFILE" on a sane
> filesystem.  It does not currently make sure the path exists in the
> HEAD exactly as given by the user (i.e. without core.ignorecase
> matching), and die when it is not found.
>
> And that can be done regardless of core.ignorecase.  Currently on a
> case sensitive filesystem without core.ignorecase, this will give a
> useless result:
>
>     $ git rm Nakefile || :;
>     $ git commit --allow-empty -m 'Made sure there is no Nakefile'
>     $ >Nakefile
>     $ git blame -- Nakefile
>     00000000 (Not Committed Yet 2012-09-09 12:21:42 -0700 1) 
>
> and such a change to verify that the path exists in HEAD will give
> us "No such path Nakefile in HEAD" in such a case.
>
> It is a behaviour change, but I think it is a good change,
> regardless of the "What I have is Makefile, but my filesystem lies
> to us saying yes when I ask if MAKEFILE exists" issue.

Perhaps like this (again, totally untested).

A few points to note:

 - If the "Nakefile" is a "new file" with substantial contents, the
   result I labelled as "useless" in the previous message _might_
   have been seen as useful by some user; it might be a regression
   in that sense, but then there is fundamentally no way to give
   sensible behaviour to core.ignorecase users.

 - We used to say get_sha1("HEAD"), but that is not a very good
   practice; even though we know DWIM will find the .git/HEAD, make
   it clear that we are not DWIMming by calling resolve_ref().

 builtin/blame.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git i/builtin/blame.c w/builtin/blame.c
index 0e102bf..395dfbc 100644
--- i/builtin/blame.c
+++ w/builtin/blame.c
@@ -2069,6 +2069,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.
@@ -2087,8 +2100,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));
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]