{"thread":{"id":"31486","subject":"[PATCH/RFC] blame: respect \"core.ignorecase\"","startedAt":"2012-09-09T17:01:53Z","lastAt":"2012-09-11T21:44:47Z","messageCount":12,"participants":["Ralf Thielow","Johannes Sixt","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"198633","messageId":"1347210113-27435-1-git-send-email-ralf.thielow@gmail.com","threadId":"31486","inReplyTo":null,"subject":"[PATCH/RFC] blame: respect \"core.ignorecase\"","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2012-09-09T17:01:53Z","receivedAt":"2012-09-09T17:01:53Z","isPatch":true,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"If \"core.ignorecase\" is true, \"git blame\" fails\nwhen the given path differs to the real path in case\nsensitivity.\n\nSigned-off-by: Ralf Thielow <ralf.thielow@gmail.com>\n---\n .gitignore             |  1 +\n Makefile               |  3 +++\n builtin/blame.c        | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++\n test-path-ignorecase.c | 27 +++++++++++++++++++++++\n 4 Dateien geändert, 89 Zeilen hinzugefügt(+)\n create mode 100644 test-path-ignorecase.c\n\ndiff --git a/.gitignore b/.gitignore\nindex bb5c91e..65ab9f6 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -195,6 +195,7 @@\n /test-sigchain\n /test-subprocess\n /test-svn-fe\n+/test-path-ignorecase\n /common-cmds.h\n *.tar.gz\n *.dsc\ndiff --git a/Makefile b/Makefile\nindex 6b0c961..dbdd214 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -503,6 +503,7 @@ TEST_PROGRAMS_NEED_X += test-sha1\n TEST_PROGRAMS_NEED_X += test-sigchain\n TEST_PROGRAMS_NEED_X += test-subprocess\n TEST_PROGRAMS_NEED_X += test-svn-fe\n+TEST_PROGRAMS_NEED_X += test-path-ignorecase\n \n TEST_PROGRAMS = $(patsubst %,%$X,$(TEST_PROGRAMS_NEED_X))\n \n@@ -2562,6 +2563,8 @@ test-parse-options$X: parse-options.o parse-options-cb.o\n \n test-svn-fe$X: vcs-svn/lib.a\n \n+test-path-ignorecase$X: builtin/blame.o\n+\n .PRECIOUS: $(TEST_OBJS)\n \n test-%$X: test-%.o GIT-LDFLAGS $(GITLIBS)\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 0d50273..895f665 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -1937,6 +1937,61 @@ static int has_string_in_work_tree(const char *path)\n \treturn !lstat(path, &st);\n }\n \n+const char* get_path_ignorecase(const char *path)\n+{\n+\tstruct strbuf res = STRBUF_INIT;\n+\tstruct strbuf p = STRBUF_INIT;\n+\tint offset = 0;\n+\n+\tif (!ignore_case || has_string_in_work_tree(path))\n+\t\treturn path;\n+\n+\tfor (;;) {\n+\t\tchar c = path[offset++];\n+\n+\t\tif (is_dir_sep(c) || c == '\\0') {\n+\t\t\tDIR *dir;\n+\n+\t\t\tif (res.len)\n+\t\t\t\tdir = opendir(res.buf);\n+\t\t\telse\n+\t\t\t\tdir = opendir(\".\");\n+\n+\t\t\tif (dir != NULL) {\n+\t\t\t\tfor (;;) {\n+\t\t\t\t\tstruct dirent *ent = readdir(dir);\n+\n+\t\t\t\t\tif (ent == NULL )\n+\t\t\t\t\t\tbreak;\n+\n+\t\t\t\t\tif (!strcmp(\".\", ent->d_name) || !strcmp(\"..\", ent->d_name))\n+\t\t\t\t\t\tcontinue;\n+\n+\t\t\t\t\tif (!strcmp(p.buf, ent->d_name))\n+\t\t\t\t\t\tbreak;\n+\n+\t\t\t\t\tif (!strcasecmp(p.buf, ent->d_name)) {\n+\t\t\t\t\t\tstrbuf_release(&p);\n+\t\t\t\t\t\tstrbuf_add(&p, ent->d_name, strlen(ent->d_name));\n+\t\t\t\t\t\tbreak;\n+\t\t\t\t\t}\n+\t\t\t\t}\n+\t\t\t\tclosedir(dir);\n+\t\t\t}\n+\n+\t\t\tstrbuf_addch(&p, c);\n+\t\t\tstrbuf_addbuf(&res, &p);\n+\t\t\tstrbuf_release(&p);\n+\t\t} else {\n+\t\t\tstrbuf_addch(&p, c);\n+\t\t}\n+\n+\t\tif (c == '\\0')\n+\t\t\tbreak;\n+\t}\n+\treturn res.buf;\n+}\n+\n static unsigned parse_score(const char *arg)\n {\n \tchar *end;\n@@ -2448,6 +2503,7 @@ parse_done:\n \t\t\t/* FALLTHROUGH */\n \t\tcase 1: /* (1a) */\n \t\t\tpath = add_prefix(prefix, argv[--argc]);\n+\t\t\tpath = get_path_ignorecase(path);\n \t\t\targv[argc] = NULL;\n \t\t\tbreak;\n \t\tdefault:\n@@ -2457,8 +2513,10 @@ parse_done:\n \t\tif (argc < 2)\n \t\t\tusage_with_options(blame_opt_usage, options);\n \t\tpath = add_prefix(prefix, argv[argc - 1]);\n+\t\tpath = get_path_ignorecase(path);\n \t\tif (argc == 3 && !has_string_in_work_tree(path)) { /* (2b) */\n \t\t\tpath = add_prefix(prefix, argv[1]);\n+\t\t\tpath = get_path_ignorecase(path);\n \t\t\targv[1] = argv[2];\n \t\t}\n \t\targv[argc - 1] = \"--\";\ndiff --git a/test-path-ignorecase.c b/test-path-ignorecase.c\nnew file mode 100644\nindex 0000000..1f17f5c\n--- /dev/null\n+++ b/test-path-ignorecase.c\n@@ -0,0 +1,27 @@\n+#include \"cache.h\"\n+\n+static const char *usage_msg = \"\\n\"\n+\t\t\"test-similar-path <path>...\\n\";\n+const char* get_path_ignorecase(const char *path);\n+\n+int main(int argc, char **argv)\n+{\n+\tchar *p;\n+\tconst char *np;\n+\n+\tif (argc != 2) {\n+\t\tusage(usage_msg);\n+\t\treturn 1;\n+\t}\n+\n+\targv++;\n+\tp = *argv;\n+\n+\tignore_case = 1;\n+\n+\tnp = get_path_ignorecase(p);\n+\n+\tprintf(\"%s\\n\", np);\n+\n+\treturn 0;\n+}\n-- \n1.7.12.1.gfe115d7\n"},{"id":"198636","messageId":"504CE7F2.9010306@kdbg.org","threadId":"31486","inReplyTo":"1347210113-27435-1-git-send-email-ralf.thielow@gmail.com","subject":"Re: [PATCH/RFC] blame: respect \"core.ignorecase\"","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2012-09-09T19:03:14Z","receivedAt":"2012-09-09T19:03:14Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 09.09.2012 19:01, schrieb Ralf Thielow:\n> If \"core.ignorecase\" is true, \"git blame\" fails\n> when the given path differs to the real path in case\n> sensitivity.\n...\n> +\t\t\t\tdir = opendir(res.buf);\n...\n> +\t\t\t\t\tstruct dirent *ent = readdir(dir);\n...\n> +\t\t\t\t\tif (!strcasecmp(p.buf, ent->d_name)) {\n\nI don't think this is how core.ignorecase is intended to work. It does\nnot trigger case-insensitive lookup in the worktree (this is still done\nby the OS), but it does a case-insensitive lookup in the index. That is,\ngiven a worktree entry 'readme', it allows to find the index entry 'README'.\n\nIMO, it is only correct that 'git blame file' operates on 'FILE' if\n'FILE' exists in the index, and in only that case, it does not matter\nhow the 'file' is named in the worktree.\n\n-- Hannes\n"},{"id":"198637","messageId":"7v7gs3q9rp.fsf@alter.siamese.dyndns.org","threadId":"31486","inReplyTo":"1347210113-27435-1-git-send-email-ralf.thielow@gmail.com","subject":"Re: [PATCH/RFC] blame: respect \"core.ignorecase\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-09T19:24:58Z","receivedAt":"2012-09-09T19:24:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ralf Thielow <ralf.thielow@gmail.com> writes:\n\n> If \"core.ignorecase\" is true, \"git blame\" fails\n> when the given path differs to the real path in case\n> sensitivity.\n\nIt is rather hard to respond to this request for comment because you\nare describing the behaviour you perceive as a problem only fuzzily\n(i.e. \"fails\"---what does it do?) without describing the expectation\n(i.e. what should it do instead?).\n\n> +test-path-ignorecase$X: builtin/blame.o\n\nYuck.  If it is a helper function that deserves its own unit test\nhelper executable, shouldn't it live with somewhere more appropriate\nto be shared across commands?\n\n> +const char* get_path_ignorecase(const char *path)\n> +{\n> +\tstruct strbuf res = STRBUF_INIT;\n> +\tstruct strbuf p = STRBUF_INIT;\n> +\tint offset = 0;\n> +\n> +\tif (!ignore_case || has_string_in_work_tree(path))\n> +\t\treturn path;\n\nIf the problem you are trying to solve is that the HEAD and the\nhistory has Makefile but the user said \"git blame MAKEFILE\"\n(i.e. start traversing from the contents in the working tree), then\nhas_string_in_work_tree(\"MAKEFILE\") will return true, and this\nfunction will tell us to find the contents for \"MAKEFILE\" not\n\"Makefile\" in the next revision (i.e. HEAD:MAKEFILE).\n\nAs you didn't describe what you perceive as the problem, I do not\nknow if the above analysis matters for the case you are trying to\nfix, though.\n\nStep back a bit and write down what problem you are trying to solve,\nincluding what existing behaviour you should *not* alter.\n\nSuppose we have this history (time flows from bottom to top), and\nour HEAD is at commit F.  We may or may not have modifications to\nMakefile in the working tree:\n\n    F  Add Makefile again\n    E  Remove Makefile\n    D  Modify Makefile\n    C  Remove MAKEFILE, add Makefile with related or unrelated contents\n    B  Modify MAKEFILE\n    A  Add MAKEFILE\n\nWhat should happen to the following?\n\n    $ git blame Makefile\n    $ git blame MAKEFILE\n    $ git blame MakeFILE\n\n    $ git blame HEAD -- Makefile\n    $ git blame HEAD -- MAKEFILE\n    $ git blame HEAD -- MakeFILE\n\n    $ git blame E -- Makefile\n    $ git blame E -- MAKEFILE\n    $ git blame E -- MakeFILE\n\n    $ git blame B -- Makefile\n    $ git blame B -- MAKEFILE\n    $ git blame B -- MakeFILE\n\nGit should see the pathname that was given by the user literally for\nany of the latter 9 cases (i.e. if we are not starting from the\ncontents of the working tree) regardless of ignorecase.  If the user\ntells us to blame MAKEFILE starting from B, \"fixing\" the path to the\nversion you can read from the working tree that may be similar to\nwhat you have at commit F (i.e. HEAD) and turn it to a blame for\nMakefile is a wrong thing to do, no?\n\nAnother potential problem with the approach your patch takes may be\nthat the case insensitive filesystem you are dealing with might be\nnot just case insensitive, but also be case destroying.  The user\nmay say \"edit Makefile && git add Makefile\", the filesystem may say\nthere is MAKEFILE there, and core.ignorecase is designed to handle\nthis case well by treating that the user really meant Makefile and\nthat it is the filesystem that is lying to us.\n\nNow that we established that the \"fixing\" of the path we got from\nthe user, should _never_ look at the working tree.  Also, if such a\n\"fixing\" ever is useful, it should be done only in the first three\ncases where the user tells us to start blaming from the working\ntree.\n\nSo what should happen to the first three cases?  When doing\n\n    $ git add Makefile\n    $ git add MAKEFILE\n    $ git add MakeFILE\n\nwe infer the case the user really meant by attempting to match the\npath against what is recorded in the index.  If there is no matching\nentry, we use what we got from the user, but if there is a matching\none (and core.ignorecase affects how this matching is done), we fix\nthe path by using the path recorded in the index instead.  Then we\nwill turn the top three cases to \"git blame Makefile\".\n\nThat brings us back to the case where we start blaming from a commit\n(the latter 9 cases).  It might make sense to grab the path in the\ntree of the named commit that matches in the core.ignorecase sense\nto the path given by the user.  Even though commit F does not have\nMAKEFILE nor MakeFILE, we turn the path given by the user to Makefile\nbecause that is the only path that the user _could_ have meant in\nthe context of the command (similarly, MAKEFILE for a blame starting\nat B).  When starting to blame at E that does not have Makefile or\nMAKEFILE, we would use whatever the user threw at us.\n\nI said \"might make sense\" for the last paragraph for a reason,\nthough.  In the history A..F above, if the user is aware of the fact\nthat the history _is_ case sensitive (and setting core.ignorecase is\na signal that she does---it is an admission that the filesystem she\nhappens to have her working tree is incapable of interacting with\nsuch a history natively and needs a bit of help from Git) and that\nsome commit have Makefile and others have MAKEFILE, it will look\ninconsistent if she asked \"git blame D -- MAKEFILE\" and gets the\nresult for \"git blame D -- Makefile\" instead.\n\nHaving said all that, I am not sure if the \"fixing\" is really the\nright approach to begin with.  Contrast these two:\n\n    $ git blame MakeFILE\n    $ git blame HEAD -- MakeFILE\n\nThe latter, regardless of core.ignorecase, should fail, with \"No\nsuch path MakeFILE in HEAD\".  The former is merely an extension to\nthe latter, in the sense that the main traversal is exactly the same\nas the latter, but on top, local modifications are blamed to the\nworking tree.\n\nIf we were to do anything, I would think the most sane thing to do\nis a smaller patch to fix fake_working_tree_commit() where it calls\nlstat() and _should_ die with \"Cannot lstat MakeFILE\" on a sane\nfilesystem.  It does not currently make sure the path exists in the\nHEAD exactly as given by the user (i.e. without core.ignorecase\nmatching), and die when it is not found.\n\nAnd that can be done regardless of core.ignorecase.  Currently on a\ncase sensitive filesystem without core.ignorecase, this will give a\nuseless result:\n\n    $ git rm Nakefile || :;\n    $ git commit --allow-empty -m 'Made sure there is no Nakefile'\n    $ >Nakefile\n    $ git blame -- Nakefile\n    00000000 (Not Committed Yet 2012-09-09 12:21:42 -0700 1) \n\nand such a change to verify that the path exists in HEAD will give\nus \"No such path Nakefile in HEAD\" in such a case.\n\nIt is a behaviour change, but I think it is a good change,\nregardless of the \"What I have is Makefile, but my filesystem lies\nto us saying yes when I ask if MAKEFILE exists\" issue.\n    \n"},{"id":"198638","messageId":"7v1uibq8u9.fsf@alter.siamese.dyndns.org","threadId":"31486","inReplyTo":"7v7gs3q9rp.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] blame: respect \"core.ignorecase\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-09T19:45:02Z","receivedAt":"2012-09-09T19:45:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> If we were to do anything, I would think the most sane thing to do\n> is a smaller patch to fix fake_working_tree_commit() where it calls\n> lstat() and _should_ die with \"Cannot lstat MakeFILE\" on a sane\n> filesystem.  It does not currently make sure the path exists in the\n> HEAD exactly as given by the user (i.e. without core.ignorecase\n> matching), and die when it is not found.\n>\n> And that can be done regardless of core.ignorecase.  Currently on a\n> case sensitive filesystem without core.ignorecase, this will give a\n> useless result:\n>\n>     $ git rm Nakefile || :;\n>     $ git commit --allow-empty -m 'Made sure there is no Nakefile'\n>     $ >Nakefile\n>     $ git blame -- Nakefile\n>     00000000 (Not Committed Yet 2012-09-09 12:21:42 -0700 1) \n>\n> and such a change to verify that the path exists in HEAD will give\n> us \"No such path Nakefile in HEAD\" in such a case.\n>\n> It is a behaviour change, but I think it is a good change,\n> regardless of the \"What I have is Makefile, but my filesystem lies\n> to us saying yes when I ask if MAKEFILE exists\" issue.\n\nPerhaps like this (again, totally untested).\n\nA few points to note:\n\n - If the \"Nakefile\" is a \"new file\" with substantial contents, the\n   result I labelled as \"useless\" in the previous message _might_\n   have been seen as useful by some user; it might be a regression\n   in that sense, but then there is fundamentally no way to give\n   sensible behaviour to core.ignorecase users.\n\n - We used to say get_sha1(\"HEAD\"), but that is not a very good\n   practice; even though we know DWIM will find the .git/HEAD, make\n   it clear that we are not DWIMming by calling resolve_ref().\n\n builtin/blame.c | 16 ++++++++++++++--\n 1 file changed, 14 insertions(+), 2 deletions(-)\n\ndiff --git i/builtin/blame.c w/builtin/blame.c\nindex 0e102bf..395dfbc 100644\n--- i/builtin/blame.c\n+++ w/builtin/blame.c\n@@ -2069,6 +2069,19 @@ static int git_blame_config(const char *var, const char *value, void *cb)\n \treturn git_default_config(var, value, cb);\n }\n \n+static void verify_working_tree_path(unsigned char *head_sha1, const char *path)\n+{\n+\tunsigned char blob_sha1[20];\n+\tunsigned mode;\n+\n+\tif (!resolve_ref_unsafe(\"HEAD\", head_sha1, 1, NULL))\n+\t\tdie(\"no such ref: HEAD\");\n+\tif (get_tree_entry(head_sha1, path, blob_sha1, &mode))\n+\t\tdie(\"no such path '%s' in HEAD\", path);\n+\tif (sha1_object_info(blob_sha1, NULL) != OBJ_BLOB)\n+\t\tdie(\"path '%s' in HEAD is not a blob\", path);\n+}\n+\n /*\n  * Prepare a dummy commit that represents the work tree (or staged) item.\n  * Note that annotating work tree item never works in the reverse.\n@@ -2087,8 +2100,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,\n \tstruct cache_entry *ce;\n \tunsigned mode;\n \n-\tif (get_sha1(\"HEAD\", head_sha1))\n-\t\tdie(\"No such ref: HEAD\");\n+\tverify_working_tree_path(head_sha1, path);\n \n \ttime(&now);\n \tcommit = xcalloc(1, sizeof(*commit));\n"},{"id":"198659","messageId":"CAN0XMOLpQLNk2vSxjxahxEoVxjx8wi4EnVDytEZPhVeYn7XLiw@mail.gmail.com","threadId":"31486","inReplyTo":"7v7gs3q9rp.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] blame: respect \"core.ignorecase\"","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2012-09-10T05:57:26Z","receivedAt":"2012-09-10T05:57:26Z","isPatch":true,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"On Sun, Sep 9, 2012 at 9:24 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Ralf Thielow <ralf.thielow@gmail.com> writes:\n>\n>> If \"core.ignorecase\" is true, \"git blame\" fails\n>> when the given path differs to the real path in case\n>> sensitivity.\n>\n> It is rather hard to respond to this request for comment because you\n> are describing the behaviour you perceive as a problem only fuzzily\n> (i.e. \"fails\"---what does it do?) without describing the expectation\n> (i.e. what should it do instead?).\n>\n>> +test-path-ignorecase$X: builtin/blame.o\n>\n> Yuck.  If it is a helper function that deserves its own unit test\n> helper executable, shouldn't it live with somewhere more appropriate\n> to be shared across commands?\n>\n>> +const char* get_path_ignorecase(const char *path)\n>> +{\n>> +     struct strbuf res = STRBUF_INIT;\n>> +     struct strbuf p = STRBUF_INIT;\n>> +     int offset = 0;\n>> +\n>> +     if (!ignore_case || has_string_in_work_tree(path))\n>> +             return path;\n>\n> If the problem you are trying to solve is that the HEAD and the\n> history has Makefile but the user said \"git blame MAKEFILE\"\n> (i.e. start traversing from the contents in the working tree), then\n> has_string_in_work_tree(\"MAKEFILE\") will return true, and this\n> function will tell us to find the contents for \"MAKEFILE\" not\n> \"Makefile\" in the next revision (i.e. HEAD:MAKEFILE).\n>\n> As you didn't describe what you perceive as the problem, I do not\n> know if the above analysis matters for the case you are trying to\n> fix, though.\n>\n> Step back a bit and write down what problem you are trying to solve,\n> including what existing behaviour you should *not* alter.\n>\n> Suppose we have this history (time flows from bottom to top), and\n> our HEAD is at commit F.  We may or may not have modifications to\n> Makefile in the working tree:\n>\n>     F  Add Makefile again\n>     E  Remove Makefile\n>     D  Modify Makefile\n>     C  Remove MAKEFILE, add Makefile with related or unrelated contents\n>     B  Modify MAKEFILE\n>     A  Add MAKEFILE\n>\n> What should happen to the following?\n>\n>     $ git blame Makefile\n>     $ git blame MAKEFILE\n>     $ git blame MakeFILE\n>\n>     $ git blame HEAD -- Makefile\n>     $ git blame HEAD -- MAKEFILE\n>     $ git blame HEAD -- MakeFILE\n>\n>     $ git blame E -- Makefile\n>     $ git blame E -- MAKEFILE\n>     $ git blame E -- MakeFILE\n>\n>     $ git blame B -- Makefile\n>     $ git blame B -- MAKEFILE\n>     $ git blame B -- MakeFILE\n>\n> Git should see the pathname that was given by the user literally for\n> any of the latter 9 cases (i.e. if we are not starting from the\n> contents of the working tree) regardless of ignorecase.  If the user\n> tells us to blame MAKEFILE starting from B, \"fixing\" the path to the\n> version you can read from the working tree that may be similar to\n> what you have at commit F (i.e. HEAD) and turn it to a blame for\n> Makefile is a wrong thing to do, no?\n>\n> Another potential problem with the approach your patch takes may be\n> that the case insensitive filesystem you are dealing with might be\n> not just case insensitive, but also be case destroying.  The user\n> may say \"edit Makefile && git add Makefile\", the filesystem may say\n> there is MAKEFILE there, and core.ignorecase is designed to handle\n> this case well by treating that the user really meant Makefile and\n> that it is the filesystem that is lying to us.\n>\n> Now that we established that the \"fixing\" of the path we got from\n> the user, should _never_ look at the working tree.  Also, if such a\n> \"fixing\" ever is useful, it should be done only in the first three\n> cases where the user tells us to start blaming from the working\n> tree.\n>\n> So what should happen to the first three cases?  When doing\n>\n>     $ git add Makefile\n>     $ git add MAKEFILE\n>     $ git add MakeFILE\n>\n> we infer the case the user really meant by attempting to match the\n> path against what is recorded in the index.  If there is no matching\n> entry, we use what we got from the user, but if there is a matching\n> one (and core.ignorecase affects how this matching is done), we fix\n> the path by using the path recorded in the index instead.  Then we\n> will turn the top three cases to \"git blame Makefile\".\n>\n> That brings us back to the case where we start blaming from a commit\n> (the latter 9 cases).  It might make sense to grab the path in the\n> tree of the named commit that matches in the core.ignorecase sense\n> to the path given by the user.  Even though commit F does not have\n> MAKEFILE nor MakeFILE, we turn the path given by the user to Makefile\n> because that is the only path that the user _could_ have meant in\n> the context of the command (similarly, MAKEFILE for a blame starting\n> at B).  When starting to blame at E that does not have Makefile or\n> MAKEFILE, we would use whatever the user threw at us.\n>\n> I said \"might make sense\" for the last paragraph for a reason,\n> though.  In the history A..F above, if the user is aware of the fact\n> that the history _is_ case sensitive (and setting core.ignorecase is\n> a signal that she does---it is an admission that the filesystem she\n> happens to have her working tree is incapable of interacting with\n> such a history natively and needs a bit of help from Git) and that\n> some commit have Makefile and others have MAKEFILE, it will look\n> inconsistent if she asked \"git blame D -- MAKEFILE\" and gets the\n> result for \"git blame D -- Makefile\" instead.\n>\n> Having said all that, I am not sure if the \"fixing\" is really the\n> right approach to begin with.  Contrast these two:\n>\n>     $ git blame MakeFILE\n>     $ git blame HEAD -- MakeFILE\n>\n> The latter, regardless of core.ignorecase, should fail, with \"No\n> such path MakeFILE in HEAD\".  The former is merely an extension to\n> the latter, in the sense that the main traversal is exactly the same\n> as the latter, but on top, local modifications are blamed to the\n> working tree.\n>\n> If we were to do anything, I would think the most sane thing to do\n> is a smaller patch to fix fake_working_tree_commit() where it calls\n> lstat() and _should_ die with \"Cannot lstat MakeFILE\" on a sane\n> filesystem.  It does not currently make sure the path exists in the\n> HEAD exactly as given by the user (i.e. without core.ignorecase\n> matching), and die when it is not found.\n>\n> And that can be done regardless of core.ignorecase.  Currently on a\n> case sensitive filesystem without core.ignorecase, this will give a\n> useless result:\n>\n>     $ git rm Nakefile || :;\n>     $ git commit --allow-empty -m 'Made sure there is no Nakefile'\n>     $ >Nakefile\n>     $ git blame -- Nakefile\n>     00000000 (Not Committed Yet 2012-09-09 12:21:42 -0700 1)\n>\n> and such a change to verify that the path exists in HEAD will give\n> us \"No such path Nakefile in HEAD\" in such a case.\n>\n> It is a behaviour change, but I think it is a good change,\n> regardless of the \"What I have is Makefile, but my filesystem lies\n> to us saying yes when I ask if MAKEFILE exists\" issue.\n>\n\nThanks for your long answer! I've learned a lot out of it.\nJust to answer your question, you were right, the problem\nI wanted to solve is the following:\n\necho \"foo\" > README && git add README && git commit -m \"add README\"\ngit config core.ignorecase false\ngit add README; # OK\ngit add readme; # 'fatal: pathspec 'readme' did not match any files' OK\ngit blame README; # has a result, OK\ngit blame readme; # 'fatal: cannot stat path 'readme': No such file or\ndirectory' OK\n\ngit config core.ignorecase true\ngit add readme; # now there's no error here; OK\ngit blame readme; # still an error << NOK\ngit blame HEAD -- readme; # 'fatal: no such path readme in HEAD' << NOK\n\nThanks\n"},{"id":"198680","messageId":"20120910161325.GB9435@sigill.intra.peff.net","threadId":"31486","inReplyTo":"7v7gs3q9rp.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] blame: respect \"core.ignorecase\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-09-10T16:13:25Z","receivedAt":"2012-09-10T16:13:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Sep 09, 2012 at 12:24:58PM -0700, Junio C Hamano wrote:\n\n> Having said all that, I am not sure if the \"fixing\" is really the\n> right approach to begin with.  Contrast these two:\n> \n>     $ git blame MakeFILE\n>     $ git blame HEAD -- MakeFILE\n> \n> The latter, regardless of core.ignorecase, should fail, with \"No\n> such path MakeFILE in HEAD\".  The former is merely an extension to\n> the latter, in the sense that the main traversal is exactly the same\n> as the latter, but on top, local modifications are blamed to the\n> working tree.\n> \n> If we were to do anything, I would think the most sane thing to do\n> is a smaller patch to fix fake_working_tree_commit() where it calls\n> lstat() and _should_ die with \"Cannot lstat MakeFILE\" on a sane\n> filesystem.  It does not currently make sure the path exists in the\n> HEAD exactly as given by the user (i.e. without core.ignorecase\n> matching), and die when it is not found.\n\nYes, I think that is the only sensible thing here. The rest of this\nemail is me essentially me agreeing with you and telling you things you\nalready know, but I had a slightly different line of reasoning, so I\nthought I would share.\n\nAs far as the original patch, if you are going to change blame, then it\nis only logical to change the whole revision parser so that \"git log --\nMAKEFILE\" works. And I do not think that is a direction we want to go.\n\ncore.ignorecase has never been about \"make git case-insensitive\". Git\nrepresents a case-sensitive tree, and will always do so because of the\nsha1 we compute over the tree objects. core.ignorecase is really \"make\ncase-sensitive git work around your case-insensitive filesystem\"[1].\n\nIf the proposal were instead to add a certain type of pathspec that is\ncase-insensitive[2], that would make much more sense to me. It is not\nviolating git's case-sensitivity because it is purely a _query_ issue.\nAnd it is a feature you might use whether or not your filesystem is case\nsensitive.\n\n-Peff\n\n[1] I was going to submit a patch to Documentation/config.txt to make\n    this more clear, but IMHO the current text is already pretty clear.\n\n[2] I did not keep up with Duy's work on pathspec magic-prefixes (and I\n    could not find anything relevant in the code or documentation), but\n    it seems like this would be a logical feature to support there.\n"},{"id":"198723","messageId":"7vvcflmxis.fsf@alter.siamese.dyndns.org","threadId":"31486","inReplyTo":"20120910161325.GB9435@sigill.intra.peff.net","subject":"Re: [PATCH/RFC] blame: respect \"core.ignorecase\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-10T20:30:03Z","receivedAt":"2012-09-10T20:30:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> If the proposal were instead to add a certain type of pathspec that is\n> case-insensitive[2], that would make much more sense to me. It is not\n> violating git's case-sensitivity because it is purely a _query_ issue.\n> And it is a feature you might use whether or not your filesystem is case\n> sensitive.\n> ...\n> [2] I did not keep up with Duy's work on pathspec magic-prefixes (and I\n>     could not find anything relevant in the code or documentation), but\n>     it seems like this would be a logical feature to support there.\n\nI think it mostly is in setup.c (look for \"Magic pathspec\").\n"},{"id":"198725","messageId":"20120910203442.GA968@sigill.intra.peff.net","threadId":"31486","inReplyTo":"7vvcflmxis.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] blame: respect \"core.ignorecase\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-09-10T20:34:42Z","receivedAt":"2012-09-10T20:34:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Sep 10, 2012 at 01:30:03PM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > If the proposal were instead to add a certain type of pathspec that is\n> > case-insensitive[2], that would make much more sense to me. It is not\n> > violating git's case-sensitivity because it is purely a _query_ issue.\n> > And it is a feature you might use whether or not your filesystem is case\n> > sensitive.\n> > ...\n> > [2] I did not keep up with Duy's work on pathspec magic-prefixes (and I\n> >     could not find anything relevant in the code or documentation), but\n> >     it seems like this would be a logical feature to support there.\n> \n> I think it mostly is in setup.c (look for \"Magic pathspec\").\n\nThanks, that helped. I got excited when I saw the \"icase\" in the\ncomments and thought it might already be implemented. But it looks like\nit is still to be done. :)\n\n-Peff\n"},{"id":"198741","messageId":"7vtxv5lfsv.fsf@alter.siamese.dyndns.org","threadId":"31486","inReplyTo":"20120910203442.GA968@sigill.intra.peff.net","subject":"Re: [PATCH/RFC] blame: respect \"core.ignorecase\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-10T21:38:08Z","receivedAt":"2012-09-10T21:38:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Mon, Sep 10, 2012 at 01:30:03PM -0700, Junio C Hamano wrote:\n>\n>> Jeff King <peff@peff.net> writes:\n>> \n>> > If the proposal were instead to add a certain type of pathspec that is\n>> > case-insensitive[2], that would make much more sense to me. It is not\n>> > violating git's case-sensitivity because it is purely a _query_ issue.\n>> > And it is a feature you might use whether or not your filesystem is case\n>> > sensitive.\n>> > ...\n>> > [2] I did not keep up with Duy's work on pathspec magic-prefixes (and I\n>> >     could not find anything relevant in the code or documentation), but\n>> >     it seems like this would be a logical feature to support there.\n>> \n>> I think it mostly is in setup.c (look for \"Magic pathspec\").\n>\n> Thanks, that helped. I got excited when I saw the \"icase\" in the\n> comments and thought it might already be implemented. But it looks like\n> it is still to be done. :)\n\nYeah, some are tongue-in-cheek (e.g. I do not know what \"recursive\npathspec\" even means), but \"noglob\" probably is an urgent need from\ncorrectness point of view for people who are writing Porcelain and\nwant to interact with a history that records funny filenames.\nCurrently you can \"git <cmd> 'foo\\*'\" to match a path that is\nexactly 'foo*' (because it matches) but you also have to hope there\nis no other paths that happens to match that pattern.  A script that\ngrabs paths out of ls-files output and then tries to use them as\npathspec would want to have a way to say \"This is literal. Do not\nhonor globs in it\".\n"},{"id":"198742","messageId":"20120910214152.GA2139@sigill.intra.peff.net","threadId":"31486","inReplyTo":"7vtxv5lfsv.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] blame: respect \"core.ignorecase\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-09-10T21:41:52Z","receivedAt":"2012-09-10T21:41:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Sep 10, 2012 at 02:38:08PM -0700, Junio C Hamano wrote:\n\n> > Thanks, that helped. I got excited when I saw the \"icase\" in the\n> > comments and thought it might already be implemented. But it looks like\n> > it is still to be done. :)\n> \n> Yeah, some are tongue-in-cheek (e.g. I do not know what \"recursive\n> pathspec\" even means), but \"noglob\" probably is an urgent need from\n> correctness point of view for people who are writing Porcelain and\n> want to interact with a history that records funny filenames.\n> Currently you can \"git <cmd> 'foo\\*'\" to match a path that is\n> exactly 'foo*' (because it matches) but you also have to hope there\n> is no other paths that happens to match that pattern.  A script that\n> grabs paths out of ls-files output and then tries to use them as\n> pathspec would want to have a way to say \"This is literal. Do not\n> honor globs in it\".\n\nI agree that the automatic globbing is currently a problem (although one\nthat comes up quite infrequently; I guess people use sane pathnames).\nBut I would think for that particular use case, you would not want to do\na per-glob prefix for that, but would rather use a command-line switch.\nIOW, would you rather do:\n\n  git ls-files |\n  while read fn; do\n    echo \":(noglob)$fn\"\n  done |\n  xargs git log --stdin --\n\nor:\n\n  git ls-files |\n  xargs git log --stdin --no-glob-pathspec --\n\n?\n\n-Peff\n"},{"id":"198752","messageId":"7v4nn5lbom.fsf@alter.siamese.dyndns.org","threadId":"31486","inReplyTo":"20120910214152.GA2139@sigill.intra.peff.net","subject":"Re: [PATCH/RFC] blame: respect \"core.ignorecase\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-10T23:07:05Z","receivedAt":"2012-09-10T23:07:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> But I would think for that particular use case, you would not want to do\n> a per-glob prefix for that, but would rather use a command-line switch.\n\nYes.\n\nEven though it is not listed as possible future semantics, one thing\nthat we may want to have before all others is \":(negate)\", so that\nwe can say something like:\n\n    git log -- '*.c' ':(negate)compat/\"\n\nI do not offhand recall what we decided during the last discussion\non the topic, but IIRC, the concensus was that it will make the code\ntoo complex and general case unusably slow to support 'or', 'and',\nand 'not' operators between pathspec elements (e.g. \"*.c\" or \"*.h\"\nfiles but not in \"compat/\" directory, unless ...) in a general way,\nand it would be sufficient to support negation by:\n\n - first grouping pathspec elements into two bins (i.e. normal\n   patterns and negative patterns);\n\n - doing the usual path limiting using only the normal patterns;\n\n - among the paths that match one of the normal patterns, rejecting\n   the ones that match one of the negative patterns.\n\nor something like that.\n"},{"id":"198814","messageId":"7vsjaofd4g.fsf_-_@alter.siamese.dyndns.org","threadId":"31486","inReplyTo":"7v1uibq8u9.fsf@alter.siamese.dyndns.org","subject":"[PATCH] blame: allow \"blame file\" in the middle of a conflicted merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-11T21:44:47Z","receivedAt":"2012-09-11T21:44:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"git blame file\" has always meant \"find the origin of each line of\nthe file in the history leading to HEAD, oh by the way, blame the\nlines that are modified locally to the working tree\".\n\nThis teaches \"git blame\" that during a conflicted merge, some\nuncommitted changes may have come from the other history that is\nbeing merged.\n\nThe verify_working_tree_path() function introduced in the previous\npatch to notice a typo in the filename (primarily on case insensitive\nfilesystems) has been updated to allow a filename that does not exist\nin HEAD (i.e. the tip of our history) as long as it exists one of the\ncommits being merged, so that a \"we deleted, the other side modified\"\ncase tracks the history of the file in the history of the other side.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * This is a follow-up to the previous one that is on 'pu'.  I was\n   pleasantly surprised that something generally useful came out of\n   a patch to squelch corner case confusion that only happens on a\n   case insensitive filesystem (iow, a case I do not particularly\n   care deeply about ;-).\n\n builtin/blame.c                 | 93 ++++++++++++++++++++++++++++++-----------\n t/t8004-blame-with-conflicts.sh |  2 +-\n 2 files changed, 70 insertions(+), 25 deletions(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 83683b8..449880e 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -2044,17 +2044,53 @@ static int git_blame_config(const char *var, const char *value, void *cb)\n \treturn git_default_config(var, value, cb);\n }\n \n-static void verify_working_tree_path(unsigned char *head_sha1, const char *path)\n+static void verify_working_tree_path(struct commit *work_tree, const char *path)\n {\n-\tunsigned char blob_sha1[20];\n-\tunsigned mode;\n+\tstruct commit_list *parents;\n \n-\tif (!resolve_ref_unsafe(\"HEAD\", head_sha1, 1, NULL))\n-\t\tdie(\"no such ref: HEAD\");\n-\tif (get_tree_entry(head_sha1, path, blob_sha1, &mode))\n-\t\tdie(\"no such path '%s' in HEAD\", path);\n-\tif (sha1_object_info(blob_sha1, NULL) != OBJ_BLOB)\n-\t\tdie(\"path '%s' in HEAD is not a blob\", path);\n+\tfor (parents = work_tree->parents; parents; parents = parents->next) {\n+\t\tconst unsigned char *commit_sha1 = parents->item->object.sha1;\n+\t\tunsigned char blob_sha1[20];\n+\t\tunsigned mode;\n+\n+\t\tif (!get_tree_entry(commit_sha1, path, blob_sha1, &mode) &&\n+\t\t    sha1_object_info(blob_sha1, NULL) == OBJ_BLOB)\n+\t\t\treturn;\n+\t}\n+\tdie(\"no such path '%s' in HEAD\", path);\n+}\n+\n+static struct commit_list **append_parent(struct commit_list **tail, const unsigned char *sha1)\n+{\n+\tstruct commit *parent;\n+\n+\tparent = lookup_commit_reference(sha1);\n+\tif (!parent)\n+\t\tdie(\"no such commit %s\", sha1_to_hex(sha1));\n+\treturn &commit_list_insert(parent, tail)->next;\n+}\n+\n+static void append_merge_parents(struct commit_list **tail)\n+{\n+\tint merge_head;\n+\tconst char *merge_head_file = git_path(\"MERGE_HEAD\");\n+\tstruct strbuf line = STRBUF_INIT;\n+\n+\tmerge_head = open(merge_head_file, O_RDONLY);\n+\tif (merge_head < 0) {\n+\t\tif (errno == ENOENT)\n+\t\t\treturn;\n+\t\tdie(\"cannot open '%s' for reading\", merge_head_file);\n+\t}\n+\n+\twhile (!strbuf_getwholeline_fd(&line, merge_head, '\\n')) {\n+\t\tunsigned char sha1[20];\n+\t\tif (line.len < 40 || get_sha1_hex(line.buf, sha1))\n+\t\t\tdie(\"unknown line in '%s': %s\", merge_head_file, line.buf);\n+\t\ttail = append_parent(tail, sha1);\n+\t}\n+\tclose(merge_head);\n+\tstrbuf_release(&line);\n }\n \n /*\n@@ -2067,6 +2103,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,\n {\n \tstruct commit *commit;\n \tstruct origin *origin;\n+\tstruct commit_list **parent_tail, *parent;\n \tunsigned char head_sha1[20];\n \tstruct strbuf buf = STRBUF_INIT;\n \tconst char *ident;\n@@ -2074,19 +2111,38 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,\n \tint size, len;\n \tstruct cache_entry *ce;\n \tunsigned mode;\n-\n-\tverify_working_tree_path(head_sha1, path);\n+\tstruct strbuf msg = STRBUF_INIT;\n \n \ttime(&now);\n \tcommit = xcalloc(1, sizeof(*commit));\n-\tcommit->parents = xcalloc(1, sizeof(*commit->parents));\n-\tcommit->parents->item = lookup_commit_reference(head_sha1);\n \tcommit->object.parsed = 1;\n \tcommit->date = now;\n \tcommit->object.type = OBJ_COMMIT;\n+\tparent_tail = &commit->parents;\n+\n+\tif (!resolve_ref_unsafe(\"HEAD\", head_sha1, 1, NULL))\n+\t\tdie(\"no such ref: HEAD\");\n+\n+\tparent_tail = append_parent(parent_tail, head_sha1);\n+\tappend_merge_parents(parent_tail);\n+\tverify_working_tree_path(commit, path);\n \n \torigin = make_origin(commit, path);\n \n+\tident = fmt_ident(\"Not Committed Yet\", \"not.committed.yet\", NULL, 0);\n+\tstrbuf_addstr(&msg, \"tree 0000000000000000000000000000000000000000\\n\");\n+\tfor (parent = commit->parents; parent; parent = parent->next)\n+\t\tstrbuf_addf(&msg, \"parent %s\\n\",\n+\t\t\t    sha1_to_hex(parent->item->object.sha1));\n+\tstrbuf_addf(&msg,\n+\t\t    \"author %s\\n\"\n+\t\t    \"committer %s\\n\\n\"\n+\t\t    \"Version of %s from %s\\n\",\n+\t\t    ident, ident, path,\n+\t\t    (!contents_from ? path :\n+\t\t     (!strcmp(contents_from, \"-\") ? \"standard input\" : contents_from)));\n+\tcommit->buffer = strbuf_detach(&msg, NULL);\n+\n \tif (!contents_from || strcmp(\"-\", contents_from)) {\n \t\tstruct stat st;\n \t\tconst char *read_from;\n@@ -2123,7 +2179,6 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,\n \t}\n \telse {\n \t\t/* Reading from stdin */\n-\t\tcontents_from = \"standard input\";\n \t\tmode = 0;\n \t\tif (strbuf_read(&buf, 0, 0) < 0)\n \t\t\tdie_errno(\"failed to read from stdin\");\n@@ -2167,16 +2222,6 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,\n \t */\n \tcache_tree_invalidate_path(active_cache_tree, path);\n \n-\tcommit->buffer = xmalloc(400);\n-\tident = fmt_ident(\"Not Committed Yet\", \"not.committed.yet\", NULL, 0);\n-\tsnprintf(commit->buffer, 400,\n-\t\t\"tree 0000000000000000000000000000000000000000\\n\"\n-\t\t\"parent %s\\n\"\n-\t\t\"author %s\\n\"\n-\t\t\"committer %s\\n\\n\"\n-\t\t\"Version of %s from %s\\n\",\n-\t\tsha1_to_hex(head_sha1),\n-\t\tident, ident, path, contents_from ? contents_from : path);\n \treturn commit;\n }\n \ndiff --git a/t/t8004-blame-with-conflicts.sh b/t/t8004-blame-with-conflicts.sh\nindex b4a260a..9c353ab 100755\n--- a/t/t8004-blame-with-conflicts.sh\n+++ b/t/t8004-blame-with-conflicts.sh\n@@ -67,7 +67,7 @@ test_expect_success \\\n '\n \n test_expect_success 'blame does not crash with conflicted file in stages 1,3' '\n-\ttest_must_fail git blame file1\n+\tgit blame file1\n '\n \n test_done\n-- \n1.7.12.414.g1a62b7a\n"}]}