{"thread":{"id":"52418","subject":"[PATCH 0/1] sparse-checkout: respect core.ignoreCase in cone mode","startedAt":"2019-12-09T19:43:04Z","lastAt":"2019-12-18T19:16:43Z","messageCount":16,"participants":["Derrick Stolee via GitGitGadget","Junio C Hamano","Derrick Stolee","Ed Maste"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"387825","messageId":"pull.488.git.1575920580.gitgitgadget@gmail.com","threadId":"52418","inReplyTo":null,"subject":"[PATCH 0/1] sparse-checkout: respect core.ignoreCase in cone mode","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-09T19:42:59Z","receivedAt":"2019-12-09T19:43:04Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"This is the first of several cleanups to the sparse-checkout feature that\nhad a large overhaul in ds/sparse-cone.\n\nWe have an internal customer that plans to use sparse-checkout as a core\nfeature of their version control experience. They use a case-insensitive\nfilesystem, so as they created their directory dependency graph they did not\nkeep consistent casing. This data is what they plan to feed to \"git\nsparse-checkout set\".\n\nWhile I would certainly prefer that the data is cleaned up (and that process\nis ongoing), I could not argue with the logic that git add does the \"right\nthing\" when core.ignoreCase is enabled.\n\nThanks, -Stolee\n\nDerrick Stolee (1):\n sparse-checkout: respect core.ignoreCase in cone mode\n\n Documentation/git-sparse-checkout.txt | 4 ++++\n builtin/sparse-checkout.c | 19 +++++++++++++++++--\n cache.h | 1 +\n name-hash.c | 10 ++++++++++\n t/t1091-sparse-checkout-builtin.sh | 13 +++++++++++++\n 5 files changed, 45 insertions(+), 2 deletions(-)\n\n\nbase-commit: cff4e9138d8df45e3b6199171092ee781cdadaeb\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-488%2Fderrickstolee%2Fsparse-case-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-488/derrickstolee/sparse-case-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/488\n-- \ngitgitgadget\n"},{"id":"387826","messageId":"23705845ce73992bf7ab645d28febebe0a698d49.1575920580.git.gitgitgadget@gmail.com","threadId":"52418","inReplyTo":"pull.488.git.1575920580.gitgitgadget@gmail.com","subject":"[PATCH 1/1] sparse-checkout: respect core.ignoreCase in cone mode","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-09T19:43:00Z","receivedAt":"2019-12-09T19:43:06Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhen a user uses the sparse-checkout feature in cone mode, they\nadd patterns using \"git sparse-checkout set ...\"\nor by using \"--stdin\" to provide the directories line-by-line over\nstdin. This behaviour naturally looks a lot like the way a user\nwould type \"git add ...\"\n\nIf core.ignoreCase is enabled, then \"git add\" will match the input\nusing a case-insensitive match. Do the same for the sparse-checkout\nfeature.\n\nThe sanitize_cone_input() method is named to imply that other checks\nmay be added. In fact, such checks are planned including looking for\nwildcards that make the paths invalid cone patterns or must be\nescaped.\n\nSpecifically, if the path has a match in the index, then use that\npath instead. If there is no match, still add that path to the\npatterns, as the user may expect the directory to appear after a\ncheckout to another ref. However, we have no matching path to\ncorrect for a case conflict, and must assume that the user provided\nthe correct case.\n\nAnother option would be to do case-insensitive checks while\nupdating the skip-worktree bits during unpack_trees(). Outside of\nthe potential performance loss on a more expensive code path, that\nalso breaks compatibility with older versions of Git as using the\nsame sparse-checkout file would change the paths that are included.\n\nSigned-off-by: Derrick Stolee \n---\n Documentation/git-sparse-checkout.txt | 4 ++++\n builtin/sparse-checkout.c | 19 +++++++++++++++++--\n cache.h | 1 +\n name-hash.c | 10 ++++++++++\n t/t1091-sparse-checkout-builtin.sh | 13 +++++++++++++\n 5 files changed, 45 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt\nindex b975285673..849efa0f0b 100644\n--- a/Documentation/git-sparse-checkout.txt\n+++ b/Documentation/git-sparse-checkout.txt\n@@ -150,6 +150,10 @@ expecting patterns of these types. Git will warn if the patterns do not match.\n If the patterns do match the expected format, then Git will use faster hash-\n based algorithms to compute inclusion in the sparse-checkout.\n \n+If `core.ignoreCase=true`, then the 'git sparse-checkout set' command will\n+correct for incorrect case when assigning patterns to the sparse-checkout\n+file.\n+\n SEE ALSO\n --------\n \ndiff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c\nindex a542d617a5..0de426384e 100644\n--- a/builtin/sparse-checkout.c\n+++ b/builtin/sparse-checkout.c\n@@ -336,6 +336,22 @@ static void insert_recursive_pattern(struct pattern_list *pl, struct strbuf *pat\n \t}\n }\n \n+static void sanitize_cone_input(struct strbuf *line)\n+{\n+\tif (ignore_case) {\n+\t\tstruct index_state *istate = the_repository->index;\n+\t\tconst char *name = index_dir_matching_name(istate, line->buf, line->len);\n+\n+\t\tif (name) {\n+\t\t\tstrbuf_setlen(line, 0);\n+\t\t\tstrbuf_addstr(line, name);\n+\t\t}\n+\t}\n+\n+\tif (line->buf[0] != '/')\n+\t\tstrbuf_insert(line, 0, \"/\", 1);\n+}\n+\n static void strbuf_to_cone_pattern(struct strbuf *line, struct pattern_list *pl)\n {\n \tstrbuf_trim(line);\n@@ -345,8 +361,7 @@ static void strbuf_to_cone_pattern(struct strbuf *line, struct pattern_list *pl)\n \tif (!line->len)\n \t\treturn;\n \n-\tif (line->buf[0] != '/')\n-\t\tstrbuf_insert(line, 0, \"/\", 1);\n+\tsanitize_cone_input(line);\n \n \tinsert_recursive_pattern(pl, line);\n }\ndiff --git a/cache.h b/cache.h\nindex d3c89e7a53..a2d9d437f0 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -728,6 +728,7 @@ int repo_index_has_changes(struct repository *repo,\n int verify_path(const char *path, unsigned mode);\n int strcmp_offset(const char *s1, const char *s2, size_t *first_change);\n int index_dir_exists(struct index_state *istate, const char *name, int namelen);\n+const char *index_dir_matching_name(struct index_state *istate, const char *name, int namelen);\n void adjust_dirname_case(struct index_state *istate, char *name);\n struct cache_entry *index_file_exists(struct index_state *istate, const char *name, int namelen, int igncase);\n \ndiff --git a/name-hash.c b/name-hash.c\nindex ceb1d7bd6f..46898b6571 100644\n--- a/name-hash.c\n+++ b/name-hash.c\n@@ -681,6 +681,16 @@ int index_dir_exists(struct index_state *istate, const char *name, int namelen)\n \treturn dir && dir->nr;\n }\n \n+const char *index_dir_matching_name(struct index_state *istate, const char *name, int namelen)\n+{\n+\tstruct dir_entry *dir;\n+\n+\tlazy_init_name_hash(istate);\n+\tdir = find_dir_entry(istate, name, namelen);\n+\n+\treturn dir ? dir->name : NULL;\n+}\n+\n void adjust_dirname_case(struct index_state *istate, char *name)\n {\n \tconst char *startPtr = name;\ndiff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh\nindex d5e2892526..d0ce48869f 100755\n--- a/t/t1091-sparse-checkout-builtin.sh\n+++ b/t/t1091-sparse-checkout-builtin.sh\n@@ -304,4 +304,17 @@ test_expect_success 'sparse-checkout (init|set|disable) fails with dirty status'\n \tgit -C dirty sparse-checkout disable\n '\n \n+test_expect_success 'cone mode: set with core.ignoreCase=true' '\n+\ttest_when_finished git -C repo config --unset core.ignoreCase &&\n+\tgit -C repo sparse-checkout init --cone &&\n+\tgit -C repo config core.ignoreCase true &&\n+\tgit -C repo sparse-checkout set Folder1 &&\n+\tcat >expect <<-EOF &&\n+\t\t/*\n+\t\t!/*/\n+\t\t/folder1/\n+\tEOF\n+\ttest_cmp expect repo/.git/info/sparse-checkout\n+'\n+\n test_done\n-- \ngitgitgadget\n"},{"id":"387960","messageId":"xmqqtv66og63.fsf@gitster-ct.c.googlers.com","threadId":"52418","inReplyTo":"23705845ce73992bf7ab645d28febebe0a698d49.1575920580.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] sparse-checkout: respect core.ignoreCase in cone mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-11T18:44:52Z","receivedAt":"2019-12-11T18:44:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> Another option would be to do case-insensitive checks while\n> updating the skip-worktree bits during unpack_trees(). Outside of\n> the potential performance loss on a more expensive code path, that\n> also breaks compatibility with older versions of Git as using the\n> same sparse-checkout file would change the paths that are included.\n\nI haven't thought things through, but that sounds as if saying that\nwe cannot fix old bugs. We use the entries in the index as a hint\nfor \"correcting\" a path to the right case on a case-insensitive\nfilesystem to avoid corrupting the case in the index, which is case\nsensitive and is a way to convey the intended case (by writing them\nout to a tree object) to those who use systems that can reliably\nreproduce cases in pathnames. But that still does not mean on a\ncase-insensitive filesystem, \"hello.c\" in the \"right\" case recorded\nin the index will always be spelled like so---somebody can make\nreaddir() or equivalent to yield \"Hello.c\" for it, and if the user\ntells us to say they want to do X (e.g. ignore, skip checkout, etc.)\nto \"hello.c\", we should do the same to \"Hello.c\" on such a system, I\nwould think.\n\nIf the runtime needs to deal with the case insensitive filesystems\nanyway (and I suspect it does), there isn't much point matching and\nforcing the case to the paths in the index like this patch does, I\nthink. So...\n\n\n> Signed-off-by: Derrick Stolee \n> ---\n> Documentation/git-sparse-checkout.txt | 4 ++++\n> builtin/sparse-checkout.c | 19 +++++++++++++++++--\n> cache.h | 1 +\n> name-hash.c | 10 ++++++++++\n> t/t1091-sparse-checkout-builtin.sh | 13 +++++++++++++\n> 5 files changed, 45 insertions(+), 2 deletions(-)\n>\n> diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt\n> index b975285673..849efa0f0b 100644\n> --- a/Documentation/git-sparse-checkout.txt\n> +++ b/Documentation/git-sparse-checkout.txt\n> @@ -150,6 +150,10 @@ expecting patterns of these types. Git will warn if the patterns do not match.\n> If the patterns do match the expected format, then Git will use faster hash-\n> based algorithms to compute inclusion in the sparse-checkout.\n> \n> +If `core.ignoreCase=true`, then the 'git sparse-checkout set' command will\n> +correct for incorrect case when assigning patterns to the sparse-checkout\n> +file.\n> +\n> SEE ALSO\n> --------\n> \n> diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c\n> index a542d617a5..0de426384e 100644\n> --- a/builtin/sparse-checkout.c\n> +++ b/builtin/sparse-checkout.c\n> @@ -336,6 +336,22 @@ static void insert_recursive_pattern(struct pattern_list *pl, struct strbuf *pat\n> \t}\n> }\n> \n> +static void sanitize_cone_input(struct strbuf *line)\n> +{\n> +\tif (ignore_case) {\n> +\t\tstruct index_state *istate = the_repository->index;\n> +\t\tconst char *name = index_dir_matching_name(istate, line->buf, line->len);\n> +\n> +\t\tif (name) {\n> +\t\t\tstrbuf_setlen(line, 0);\n> +\t\t\tstrbuf_addstr(line, name);\n> +\t\t}\n> +\t}\n> +\n> +\tif (line->buf[0] != '/')\n> +\t\tstrbuf_insert(line, 0, \"/\", 1);\n> +}\n> +\n> static void strbuf_to_cone_pattern(struct strbuf *line, struct pattern_list *pl)\n> {\n> \tstrbuf_trim(line);\n> @@ -345,8 +361,7 @@ static void strbuf_to_cone_pattern(struct strbuf *line, struct pattern_list *pl)\n> \tif (!line->len)\n> \t\treturn;\n> \n> -\tif (line->buf[0] != '/')\n> -\t\tstrbuf_insert(line, 0, \"/\", 1);\n> +\tsanitize_cone_input(line);\n> \n> \tinsert_recursive_pattern(pl, line);\n> }\n> diff --git a/cache.h b/cache.h\n> index d3c89e7a53..a2d9d437f0 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -728,6 +728,7 @@ int repo_index_has_changes(struct repository *repo,\n> int verify_path(const char *path, unsigned mode);\n> int strcmp_offset(const char *s1, const char *s2, size_t *first_change);\n> int index_dir_exists(struct index_state *istate, const char *name, int namelen);\n> +const char *index_dir_matching_name(struct index_state *istate, const char *name, int namelen);\n> void adjust_dirname_case(struct index_state *istate, char *name);\n> struct cache_entry *index_file_exists(struct index_state *istate, const char *name, int namelen, int igncase);\n> \n> diff --git a/name-hash.c b/name-hash.c\n> index ceb1d7bd6f..46898b6571 100644\n> --- a/name-hash.c\n> +++ b/name-hash.c\n> @@ -681,6 +681,16 @@ int index_dir_exists(struct index_state *istate, const char *name, int namelen)\n> \treturn dir && dir->nr;\n> }\n> \n> +const char *index_dir_matching_name(struct index_state *istate, const char *name, int namelen)\n> +{\n> +\tstruct dir_entry *dir;\n> +\n> +\tlazy_init_name_hash(istate);\n> +\tdir = find_dir_entry(istate, name, namelen);\n> +\n> +\treturn dir ? dir->name : NULL;\n> +}\n> +\n> void adjust_dirname_case(struct index_state *istate, char *name)\n> {\n> \tconst char *startPtr = name;\n> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh\n> index d5e2892526..d0ce48869f 100755\n> --- a/t/t1091-sparse-checkout-builtin.sh\n> +++ b/t/t1091-sparse-checkout-builtin.sh\n> @@ -304,4 +304,17 @@ test_expect_success 'sparse-checkout (init|set|disable) fails with dirty status'\n> \tgit -C dirty sparse-checkout disable\n> '\n> \n> +test_expect_success 'cone mode: set with core.ignoreCase=true' '\n> +\ttest_when_finished git -C repo config --unset core.ignoreCase &&\n> +\tgit -C repo sparse-checkout init --cone &&\n> +\tgit -C repo config core.ignoreCase true &&\n> +\tgit -C repo sparse-checkout set Folder1 &&\n> +\tcat >expect <<-EOF &&\n> +\t\t/*\n> +\t\t!/*/\n> +\t\t/folder1/\n> +\tEOF\n> +\ttest_cmp expect repo/.git/info/sparse-checkout\n> +'\n> +\n> test_done\n"},{"id":"387967","messageId":"9dbf6d43-ac1e-4790-84e5-4829d21f5fdb@gmail.com","threadId":"52418","inReplyTo":"xmqqtv66og63.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] sparse-checkout: respect core.ignoreCase in cone mode","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-12-11T19:11:32Z","receivedAt":"2019-12-11T19:11:36Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 12/11/2019 1:44 PM, Junio C Hamano wrote:\n> \"Derrick Stolee via GitGitGadget\" writes:\n> \n>> Another option would be to do case-insensitive checks while\n>> updating the skip-worktree bits during unpack_trees(). Outside of\n>> the potential performance loss on a more expensive code path, that\n>> also breaks compatibility with older versions of Git as using the\n>> same sparse-checkout file would change the paths that are included.\n> \n> I haven't thought things through, but that sounds as if saying that\n> we cannot fix old bugs. We use the entries in the index as a hint\n> for \"correcting\" a path to the right case on a case-insensitive\n> filesystem to avoid corrupting the case in the index, which is case\n> sensitive and is a way to convey the intended case (by writing them\n> out to a tree object) to those who use systems that can reliably\n> reproduce cases in pathnames. But that still does not mean on a\n> case-insensitive filesystem, \"hello.c\" in the \"right\" case recorded\n> in the index will always be spelled like so---somebody can make\n> readdir() or equivalent to yield \"Hello.c\" for it, and if the user\n> tells us to say they want to do X (e.g. ignore, skip checkout, etc.)\n> to \"hello.c\", we should do the same to \"Hello.c\" on such a system, I\n> would think.\n> \n> If the runtime needs to deal with the case insensitive filesystems\n> anyway (and I suspect it does), there isn't much point matching and\n> forcing the case to the paths in the index like this patch does, I\n> think. So...\n\nI'm trying to find a way around these two ideas:\n\n1. The index is case-sensitive, and the sparse-checkout patterns are\n case-sensitive.\n\n2. If a filesystem is case-insensitive, most index-change operations\n already succeed with incorrect case, especially with core.ignoreCase\n enabled.\n\nThe approach I have is to allow a user to provide a case that does not\nmatch the index, and then we store the pattern in the sparse-checkout\nthat matches the case in the index.\n\nAnother approach would be to make the sparse-checkout patterns be\ncase-insensitive when core.ignoreCase is enabled. I chose against\nthis due to the compatibility and the performance cost. We would\nlikely pay that performance penalty even if all the patterns have\nthe correct case. It violates the existing \"contract\" with the\nsparse-checkout file, and that is probably what you are talking about\nwith \"we cannot fix old bugs\".\n\nIt sounds like you are preferring this second option, despite the\nperformance costs. It is possible to use a case-insensitive hashing\nalgorithm when in the cone mode, but I'm not sure about how to do\na similar concept for the normal mode.\n\nThanks,\n-Stolee\n"},{"id":"387972","messageId":"xmqqeexaocos.fsf@gitster-ct.c.googlers.com","threadId":"52418","inReplyTo":"9dbf6d43-ac1e-4790-84e5-4829d21f5fdb@gmail.com","subject":"Re: [PATCH 1/1] sparse-checkout: respect core.ignoreCase in cone mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-11T20:00:03Z","receivedAt":"2019-12-11T20:00:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n> I'm trying to find a way around these two ideas:\n>\n> 1. The index is case-sensitive, and the sparse-checkout patterns are\n> case-sensitive.\n\nOK. The latter is local to the repository and not shared to the\nworld where people with case sensitive systems would live, right?\n\n> 2. If a filesystem is case-insensitive, most index-change operations\n> already succeed with incorrect case, especially with core.ignoreCase\n> enabled.\n\nI am not sure about \"incorrect\", though. \n\nMy understanding is that modern case-insensitive systems are not\ninformation-destroying [*1*]. A new file you create as \"ReadMe.txt\"\non your filesystem would be seen in that mixed case spelling via\nreaddir() or equivalent, so adding it to the index as-is would\nlikely be in the \"correct\" case, no? If, after adding that path to\nthe index, you did \"rm ReadMe.txt\" and created \"README.TXT\", surely\nwe won't have both ReadMe.txt and README.TXT in the index with\nignoreCase set, and keep using ReadMe.txt that matches what you\nadded to the index. I am not sure which one is the \"incorrect\" case\nin such a scenario.\n\n> The approach I have is to allow a user to provide a case that does not\n> match the index, and then we store the pattern in the sparse-checkout\n> that matches the case in the index.\n\nYes, I understood that from your proposed log message and cover\nletter. They were very clearly written.\n\nBut imagine that your user writes ABC in the sparse pattern file,\nand there is no abc anything in the index in any case permutations.\n\nWhen you check out a branch that has Abc, shouldn't the pattern ABC\naffect the operation just like a pattern Abc would on a case\ninsensitive system?\n\nOr are end users perfectly aware that the thing in that branch is\nspelled \"Abc\" and not \"ABC\" (the data in Git does---it comes from a\ntree object that is case sensitive)? If so, then the pattern ABC\nshould not affect the subtree whose name is \"Abc\" even on a case\ninsensitive system.\n\nI am not sure what the design of this part of the system expects out\nof the end user. Perhaps keeping the patterns case insensitive and\ntell the end users to spell them correctly is the right way to go, I\nguess, if it is just the filesystem that cannot represente the cases\ncorrectly at times and the users are perfectly capable of telling\nthe right and wrong cases apart.\n\nBut then I am not sure why correcting misspelled names by comparing\nthem with the index entries is a good idea, either.\n\n> It sounds like you are preferring this second option, despite the\n> performance costs. It is possible to use a case-insensitive hashing\n> algorithm when in the cone mode, but I'm not sure about how to do\n> a similar concept for the normal mode.\n\nI have no strong preference, other than that I prefer to see things\nbeing done consistently. Don't we already use case folding hash\nfunction to ensure that we won't add duplicate to the index on\ncase-insensitive system? I suspect that we would need to do the\nsame, whether in cone mode or just a normal sparse-checkout mode\nconsistently.\n\nThanks.\n\n\n[Footnote]\n\n*1* ... unlike HFS+ where everything is forced to NKD and a bit of\ninformation---whether the original was in NKC or NKD---is discarded\nforever.\n"},{"id":"387974","messageId":"3e64b470-93cc-5491-09e1-e499f0a7583d@gmail.com","threadId":"52418","inReplyTo":"xmqqeexaocos.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] sparse-checkout: respect core.ignoreCase in cone mode","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-12-11T20:29:38Z","receivedAt":"2019-12-11T20:29:42Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 12/11/2019 3:00 PM, Junio C Hamano wrote:\n> Derrick Stolee writes:\n> \n>> I'm trying to find a way around these two ideas:\n>>\n>> 1. The index is case-sensitive, and the sparse-checkout patterns are\n>> case-sensitive.\n> \n> OK. The latter is local to the repository and not shared to the\n> world where people with case sensitive systems would live, right?\n\nYes, this information is local to a specific local copy. Even worktrees\nhave distinct sparse-checkout files.\n\n>> 2. If a filesystem is case-insensitive, most index-change operations\n>> already succeed with incorrect case, especially with core.ignoreCase\n>> enabled.\n> \n> I am not sure about \"incorrect\", though. \n> \n> My understanding is that modern case-insensitive systems are not\n> information-destroying [*1*]. A new file you create as \"ReadMe.txt\"\n> on your filesystem would be seen in that mixed case spelling via\n> readdir() or equivalent, so adding it to the index as-is would\n> likely be in the \"correct\" case, no? If, after adding that path to\n> the index, you did \"rm ReadMe.txt\" and created \"README.TXT\", surely\n> we won't have both ReadMe.txt and README.TXT in the index with\n> ignoreCase set, and keep using ReadMe.txt that matches what you\n> added to the index. I am not sure which one is the \"incorrect\" case\n> in such a scenario.\n\nThe specific example I have in my mind is that the filesystem can have\n\"README.TXT\" as what it would report by readdir(), but a user can type\n\n\tgit add readme.txt\n\nWithout core.ignoreCase, the index will store the path as \"readme.txt\",\npossibly adding a new entry in the index next to \"README.TXT\". If\ncore.ignoreCase is enabled, then the case reported by readdir() is used.\n(This works both for a tracked and untracked path.)\n\n>> The approach I have is to allow a user to provide a case that does not\n>> match the index, and then we store the pattern in the sparse-checkout\n>> that matches the case in the index.\n> \n> Yes, I understood that from your proposed log message and cover\n> letter. They were very clearly written.\n> \n> But imagine that your user writes ABC in the sparse pattern file,\n> and there is no abc anything in the index in any case permutations.\n> \n> When you check out a branch that has Abc, shouldn't the pattern ABC\n> affect the operation just like a pattern Abc would on a case\n> insensitive system?\n> \n> Or are end users perfectly aware that the thing in that branch is\n> spelled \"Abc\" and not \"ABC\" (the data in Git does---it comes from a\n> tree object that is case sensitive)? If so, then the pattern ABC\n> should not affect the subtree whose name is \"Abc\" even on a case\n> insensitive system.\n\nThis is a case that is difficult to control for. We have no source\nof truth (filesystem or index) at the time of the 'git sparse-checkout\nset' command. I cannot think of a solution to this specific issue\nwithout doing the more-costly approach on every unpack_trees() call.\n\nI believe this case may be narrow enough to \"document and don't-fix\",\nbut please speak up if anyone thinks this _must_ be fixed.\n\nThe other thing I can say is that my current approach _could_ be\nreplaced in the future by the more invasive check in unpack_trees().\nThe behavior change would be invisible to users who don't inspect\ntheir sparse-checkout files other than this narrow case.\n\nSpecifically, our internal customer is planning to update the\nsparse-checkout cone based on files in the workdir, which means that\nthe paths are expected to be in the index at the time of the 'set'\ncall.\n\n> I am not sure what the design of this part of the system expects out\n> of the end user. Perhaps keeping the patterns case insensitive and\n> tell the end users to spell them correctly is the right way to go, I\n> guess, if it is just the filesystem that cannot represente the cases\n> correctly at times and the users are perfectly capable of telling\n> the right and wrong cases apart.\n\nMy first reaction to this internal request was \"just clean up your\ndata.\" The issue is keeping it clean as users are changing the data\noften and the only current checks are whether the build passes (and\nthe build \"sees\" the files because the filesystem accepts the wrong\ncase).\n\nThe \"git add\" behavior made me think there was sufficient precedent\nin Git to try this change.\n\nI'm happy to follow the community's opinion in this matter, including\na hard \"we will not correct for case in this feature\" or \"if you want\nthis then you'll pay for it in performance.\" In the latter case I'd\nprefer a new config option to toggle the sparse-checkout case\ninsensitivity. That way users could have core.ignoreCase behavior without\nthe perf hit if they use clean input to the sparse-checkout feature.\n\n> But then I am not sure why correcting misspelled names by comparing\n> them with the index entries is a good idea, either.\n\nRight, that seems like a line too far.\n\n>> It sounds like you are preferring this second option, despite the\n>> performance costs. It is possible to use a case-insensitive hashing\n>> algorithm when in the cone mode, but I'm not sure about how to do\n>> a similar concept for the normal mode.\n> \n> I have no strong preference, other than that I prefer to see things\n> being done consistently. Don't we already use case folding hash\n> function to ensure that we won't add duplicate to the index on\n> case-insensitive system? I suspect that we would need to do the\n> same, whether in cone mode or just a normal sparse-checkout mode\n> consistently.\n\nSince the cone mode uses a hashset to match the paths to the patterns,\nthat conversion is possible. I'm not sure how to start making arbitrary\nregexes be case-insensitive in the non-cone-mode case. Suggestions?\n\nThank you for the discussion. I was hoping to get feedback on this\napproach, which is why this patch is isolated to only this issue.\n\nThanks,\n-Stolee\n\n\n"},{"id":"387978","messageId":"xmqqa77yo86g.fsf@gitster-ct.c.googlers.com","threadId":"52418","inReplyTo":"3e64b470-93cc-5491-09e1-e499f0a7583d@gmail.com","subject":"Re: [PATCH 1/1] sparse-checkout: respect core.ignoreCase in cone mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-11T21:37:27Z","receivedAt":"2019-12-11T21:37:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n> The specific example I have in my mind is that the filesystem can have\n> \"README.TXT\" as what it would report by readdir(), but a user can type\n>\n> \tgit add readme.txt\n>\n> Without core.ignoreCase, the index will store the path as \"readme.txt\",\n> possibly adding a new entry in the index next to \"README.TXT\". If\n> core.ignoreCase is enabled, then the case reported by readdir() is used.\n> (This works both for a tracked and untracked path.)\n\nYes, but which one is \"correct\"? readme.txt read from the index or\nREADME.TXT read from the filesystem? Presumably, when the paths got\nfirst checked out of the index, it would have been in \"readme.txt\"\non the filesystem, so the user must have done on purpose something\nto cause the file to be named in all uppercase since then.\n\n> This is a case that is difficult to control for. We have no source\n> of truth (filesystem or index) at the time of the 'git sparse-checkout\n> set' command. I cannot think of a solution to this specific issue\n> without doing the more-costly approach on every unpack_trees() call.\n>\n> I believe this case may be narrow enough to \"document and don't-fix\",\n> but please speak up if anyone thinks this _must_ be fixed.\n\nI do not think it \"must be\" fixed in the sense that \"if it hurts,\ndon't do so\" is a sufficient remedy. But then for exactly the same\nreason, I do not think it is worth doing what you do in this patch.\n\nOn the other hand, I think runtime case folding, just like what we\ndo when \"git add\" is told to handle a path in different case, would\nbe the only \"right\" way to match end-user expectations on a case\ninsensitive system, even though that is a \"nice to do so\" and\ncertainly not a \"must do so\" thing.\n\n> The \"git add\" behavior made me think there was sufficient precedent\n> in Git to try this change.\n\nSince I view that the \"git add\" behaviour is a \"nice to do so\"\nruntime conversion, I would actually think the approach you\ndiscarded would be more in line with the precedent.\n\n> I'm happy to follow the community's opinion in this matter, including\n> a hard \"we will not correct for case in this feature\" or \"if you want\n> this then you'll pay for it in performance.\" In the latter case I'd\n> prefer a new config option to toggle the sparse-checkout case\n> insensitivity. That way users could have core.ignoreCase behavior without\n> the perf hit if they use clean input to the sparse-checkout feature.\n\nI value correctness more---a system that does incorrect thing\nquickly is not all that interesting.\n\nAssuming that your users are sensible and feed good data, how much\n\"performance hit\" are we talking about? Wouldn't this be something\nwe can make into a \"if you have the paths in the correct case, we'll\nsee the match just as quickly as in the case sensitive system, so\nmost of the time there is no penalty, but for correctness we would\nalso fall back to try different cases\"?\n"},{"id":"387999","messageId":"85f4574b-80fa-6638-3f32-a2e8177ff960@gmail.com","threadId":"52418","inReplyTo":"xmqqa77yo86g.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] sparse-checkout: respect core.ignoreCase in cone mode","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-12-12T02:51:10Z","receivedAt":"2019-12-12T02:51:14Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 12/11/2019 4:37 PM, Junio C Hamano wrote:\n> Derrick Stolee writes:\n> \n>> The specific example I have in my mind is that the filesystem can have\n>> \"README.TXT\" as what it would report by readdir(), but a user can type\n>>\n>> \tgit add readme.txt\n>>\n>> Without core.ignoreCase, the index will store the path as \"readme.txt\",\n>> possibly adding a new entry in the index next to \"README.TXT\". If\n>> core.ignoreCase is enabled, then the case reported by readdir() is used.\n>> (This works both for a tracked and untracked path.)\n> \n> Yes, but which one is \"correct\"? readme.txt read from the index or\n> README.TXT read from the filesystem? Presumably, when the paths got\n> first checked out of the index, it would have been in \"readme.txt\"\n> on the filesystem, so the user must have done on purpose something\n> to cause the file to be named in all uppercase since then.\n\nAh. The issue here is that with core.ignoreCase=true, the index and\nfilesystem agree. It's just the user input that differs.\n\n>> This is a case that is difficult to control for. We have no source\n>> of truth (filesystem or index) at the time of the 'git sparse-checkout\n>> set' command. I cannot think of a solution to this specific issue\n>> without doing the more-costly approach on every unpack_trees() call.\n>>\n>> I believe this case may be narrow enough to \"document and don't-fix\",\n>> but please speak up if anyone thinks this _must_ be fixed.\n> \n> I do not think it \"must be\" fixed in the sense that \"if it hurts,\n> don't do so\" is a sufficient remedy. But then for exactly the same\n> reason, I do not think it is worth doing what you do in this patch.\n> \n> On the other hand, I think runtime case folding, just like what we\n> do when \"git add\" is told to handle a path in different case, would\n> be the only \"right\" way to match end-user expectations on a case\n> insensitive system, even though that is a \"nice to do so\" and\n> certainly not a \"must do so\" thing.\n> \n>> The \"git add\" behavior made me think there was sufficient precedent\n>> in Git to try this change.\n> \n> Since I view that the \"git add\" behaviour is a \"nice to do so\"\n> runtime conversion, I would actually think the approach you\n> discarded would be more in line with the precedent.\n> \n>> I'm happy to follow the community's opinion in this matter, including\n>> a hard \"we will not correct for case in this feature\" or \"if you want\n>> this then you'll pay for it in performance.\" In the latter case I'd\n>> prefer a new config option to toggle the sparse-checkout case\n>> insensitivity. That way users could have core.ignoreCase behavior without\n>> the perf hit if they use clean input to the sparse-checkout feature.\n> \n> I value correctness more---a system that does incorrect thing\n> quickly is not all that interesting.\n> \n> Assuming that your users are sensible and feed good data, how much\n> \"performance hit\" are we talking about?\n\nI'll need to build a prototype to test the performance hit. Maybe\nI'm overestimating the effect of using a case-insensitive hash.\n\n> Wouldn't this be something\n> we can make into a \"if you have the paths in the correct case, we'll\n> see the match just as quickly as in the case sensitive system, so\n> most of the time there is no penalty, but for correctness we would\n> also fall back to try different cases\"?\n\nI think we would want the config option present to say \"my data may\nnot be in the proper case, please do extra work for me\". I can't\nthink of a way to do the fallback in real-time.\n\nI'll try again with the case-insensitive hash algorithm and see\nhow that shakes out.\n\nThanks,\n-Stolee\n\n\n"},{"id":"388048","messageId":"7aeb1589-cc08-654f-1bc0-d48fec1c0729@gmail.com","threadId":"52418","inReplyTo":"23705845ce73992bf7ab645d28febebe0a698d49.1575920580.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] sparse-checkout: respect core.ignoreCase in cone mode","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-12-12T20:59:34Z","receivedAt":"2019-12-12T20:59:38Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 12/9/2019 2:43 PM, Derrick Stolee via GitGitGadget wrote:\n> From: Derrick Stolee \n> \n> When a user uses the sparse-checkout feature in cone mode, they\n> add patterns using \"git sparse-checkout set ...\"\n> or by using \"--stdin\" to provide the directories line-by-line over\n> stdin. This behaviour naturally looks a lot like the way a user\n> would type \"git add ...\"\n\nJunio:\n\nWhile I was updating my GGG PR [1] with new logic, I see that you\nadded this commit to ds/sparse-cone. I didn't intend to these\nto be in the same topic, and I was hoping this discussion doesn't\ndelay the other commits in ds/sparse-cone from merging.\n\nIf you have a plan for that branch and the merge status of those\ncommits, then I'm happy to re-target my PR against 'next' or\nan equivalent branch.\n\nThanks,\n-Stolee\n\n[1] https://github.com/gitgitgadget/git/pull/488\n\n"},{"id":"388136","messageId":"pull.488.v2.git.1576260593.gitgitgadget@gmail.com","threadId":"52418","inReplyTo":"pull.488.git.1575920580.gitgitgadget@gmail.com","subject":"[PATCH v2 0/1] sparse-checkout: respect core.ignoreCase in cone mode","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-13T18:09:52Z","receivedAt":"2019-12-13T20:40:11Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"This is the first of several cleanups to the sparse-checkout feature that\nhad a large overhaul in ds/sparse-cone.\n\nWe have an internal customer that plans to use sparse-checkout as a core\nfeature of their version control experience. They use a case-insensitive\nfilesystem, so as they created their directory dependency graph they did not\nkeep consistent casing. This data is what they plan to feed to \"git\nsparse-checkout set\".\n\nWhile I would certainly prefer that the data is cleaned up (and that process\nis ongoing), I could not argue with the logic that git add does the \"right\nthing\" when core.ignoreCase is enabled.\n\nUpdate in V2:\n\nBased on excellent feedback from Junio, we decided to shore up the case\nissues from the previous implementation. Now, the sparse-checkout file may\nbe stored with the \"wrong\" case but the hash algorithm is replaced with a\ncase-insensitive check when core.ignoreCase is set. Because the performance\nimpact is much lower than I anticipated, I no longer think it is important\nto add a new config option to enable/disable this logic.\n\nThanks, -Stolee\n\nDerrick Stolee (1):\n sparse-checkout: respect core.ignoreCase in cone mode\n\n Documentation/git-sparse-checkout.txt | 5 +++++\n builtin/sparse-checkout.c | 10 ++++++++--\n dir.c | 15 ++++++++++++---\n t/t1091-sparse-checkout-builtin.sh | 17 +++++++++++++++++\n 4 files changed, 42 insertions(+), 5 deletions(-)\n\n\nbase-commit: cff4e9138d8df45e3b6199171092ee781cdadaeb\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-488%2Fderrickstolee%2Fsparse-case-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-488/derrickstolee/sparse-case-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/488\n\nRange-diff vs v1:\n\n 1: 23705845ce ! 1: dc97d5a0d2 sparse-checkout: respect core.ignoreCase in cone mode\n @@ -12,23 +12,42 @@\n using a case-insensitive match. Do the same for the sparse-checkout\n feature.\n \n - The sanitize_cone_input() method is named to imply that other checks\n - may be added. In fact, such checks are planned including looking for\n - wildcards that make the paths invalid cone patterns or must be\n - escaped.\n -\n - Specifically, if the path has a match in the index, then use that\n - path instead. If there is no match, still add that path to the\n - patterns, as the user may expect the directory to appear after a\n - checkout to another ref. However, we have no matching path to\n - correct for a case conflict, and must assume that the user provided\n - the correct case.\n -\n - Another option would be to do case-insensitive checks while\n - updating the skip-worktree bits during unpack_trees(). Outside of\n - the potential performance loss on a more expensive code path, that\n - also breaks compatibility with older versions of Git as using the\n - same sparse-checkout file would change the paths that are included.\n + Perform case-insensitive checks while updating the skip-worktree\n + bits during unpack_trees(). This is done by changing the hash\n + algorithm and hashmap comparison methods to optionally use case-\n + insensitive methods.\n +\n + When this is enabled, there is a small performance cost in the\n + hashing algorithm. To tease out the worst possible case, the\n + following was run on a repo with a deep directory structure:\n +\n + git ls-tree -d -r --name-only HEAD |\n + git sparse-checkout set --stdin\n +\n + The 'set' command was timed with core.ignoreCase disabled or\n + enabled. For the repo with a deep history, the numbers were\n +\n + core.ignoreCase=false: 62s\n + core.ignoreCase=true: 74s (+19.3%)\n +\n + For reproducibility, the equivalent test on the Linux kernel\n + repository had these numbers:\n +\n + core.ignoreCase=false: 3.1s\n + core.ignoreCase=true: 3.6s (+16%)\n +\n + Now, this is not an entirely fair comparison, as most users\n + will define their sparse cone using more shallow directories,\n + and the performance improvement from eb42feca97 (\"unpack-trees:\n + hash less in cone mode\" 2019-11-21) can remove most of the\n + hash cost. For a more realistic test, drop the \"-r\" from the\n + ls-tree command to store only the first-level directories.\n + In that case, the Linux kernel repository takes 0.2-0.25s in\n + each case, and the deep repository takes one second, plus or\n + minus 0.05s, in each case.\n +\n + Thus, we _can_ demonstrate a cost to this change, but it is\n + unlikely to matter to any reasonable sparse-checkout cone.\n \n Signed-off-by: Derrick Stolee \n \n @@ -39,9 +58,10 @@\n If the patterns do match the expected format, then Git will use faster hash-\n based algorithms to compute inclusion in the sparse-checkout.\n \n -+If `core.ignoreCase=true`, then the 'git sparse-checkout set' command will\n -+correct for incorrect case when assigning patterns to the sparse-checkout\n -+file.\n ++If `core.ignoreCase=true`, then the pattern-matching algorithm will use a\n ++case-insensitive check. This corrects for case mismatched filenames in the\n ++'git sparse-checkout set' command to reflect the expected cone in the working\n ++directory.\n +\n SEE ALSO\n --------\n @@ -51,71 +71,76 @@\n --- a/builtin/sparse-checkout.c\n +++ b/builtin/sparse-checkout.c\n @@\n - \t}\n - }\n + \tstruct pattern_entry *e = xmalloc(sizeof(*e));\n + \te->patternlen = path->len;\n + \te->pattern = strbuf_detach(path, NULL);\n +-\thashmap_entry_init(&e->ent, memhash(e->pattern, e->patternlen));\n ++\thashmap_entry_init(&e->ent,\n ++\t\t\t ignore_case ?\n ++\t\t\t strihash(e->pattern) :\n ++\t\t\t strhash(e->pattern));\n + \n + \thashmap_add(&pl->recursive_hashmap, &e->ent);\n \n -+static void sanitize_cone_input(struct strbuf *line)\n -+{\n -+\tif (ignore_case) {\n -+\t\tstruct index_state *istate = the_repository->index;\n -+\t\tconst char *name = index_dir_matching_name(istate, line->buf, line->len);\n -+\n -+\t\tif (name) {\n -+\t\t\tstrbuf_setlen(line, 0);\n -+\t\t\tstrbuf_addstr(line, name);\n -+\t\t}\n -+\t}\n -+\n -+\tif (line->buf[0] != '/')\n -+\t\tstrbuf_insert(line, 0, \"/\", 1);\n -+}\n -+\n - static void strbuf_to_cone_pattern(struct strbuf *line, struct pattern_list *pl)\n - {\n - \tstrbuf_trim(line);\n @@\n - \tif (!line->len)\n - \t\treturn;\n + \t\te = xmalloc(sizeof(struct pattern_entry));\n + \t\te->patternlen = newlen;\n + \t\te->pattern = xstrndup(oldpattern, newlen);\n +-\t\thashmap_entry_init(&e->ent, memhash(e->pattern, e->patternlen));\n ++\t\thashmap_entry_init(&e->ent,\n ++\t\t\t\t ignore_case ?\n ++\t\t\t\t strihash(e->pattern) :\n ++\t\t\t\t strhash(e->pattern));\n \n --\tif (line->buf[0] != '/')\n --\t\tstrbuf_insert(line, 0, \"/\", 1);\n -+\tsanitize_cone_input(line);\n + \t\tif (!hashmap_get_entry(&pl->parent_hashmap, e, ent, NULL))\n + \t\t\thashmap_add(&pl->parent_hashmap, &e->ent);\n +\n + diff --git a/dir.c b/dir.c\n + --- a/dir.c\n + +++ b/dir.c\n +@@\n + \t\t\t ? ee1->patternlen\n + \t\t\t : ee2->patternlen;\n \n - \tinsert_recursive_pattern(pl, line);\n ++\tif (ignore_case)\n ++\t\treturn strncasecmp(ee1->pattern, ee2->pattern, min_len);\n + \treturn strncmp(ee1->pattern, ee2->pattern, min_len);\n }\n -\n - diff --git a/cache.h b/cache.h\n - --- a/cache.h\n - +++ b/cache.h\n + \n @@\n - int verify_path(const char *path, unsigned mode);\n - int strcmp_offset(const char *s1, const char *s2, size_t *first_change);\n - int index_dir_exists(struct index_state *istate, const char *name, int namelen);\n -+const char *index_dir_matching_name(struct index_state *istate, const char *name, int namelen);\n - void adjust_dirname_case(struct index_state *istate, char *name);\n - struct cache_entry *index_file_exists(struct index_state *istate, const char *name, int namelen, int igncase);\n + \t\ttranslated->pattern = truncated;\n + \t\ttranslated->patternlen = given->patternlen - 2;\n + \t\thashmap_entry_init(&translated->ent,\n +-\t\t\t\t memhash(translated->pattern, translated->patternlen));\n ++\t\t\t\t ignore_case ?\n ++\t\t\t\t strihash(translated->pattern) :\n ++\t\t\t\t strhash(translated->pattern));\n \n -\n - diff --git a/name-hash.c b/name-hash.c\n - --- a/name-hash.c\n - +++ b/name-hash.c\n + \t\tif (!hashmap_get_entry(&pl->recursive_hashmap,\n + \t\t\t\t translated, ent, NULL)) {\n @@\n - \treturn dir && dir->nr;\n + \ttranslated->pattern = xstrdup(given->pattern);\n + \ttranslated->patternlen = given->patternlen;\n + \thashmap_entry_init(&translated->ent,\n +-\t\t\t memhash(translated->pattern, translated->patternlen));\n ++\t\t\t ignore_case ?\n ++\t\t\t strihash(translated->pattern) :\n ++\t\t\t strhash(translated->pattern));\n + \n + \thashmap_add(&pl->recursive_hashmap, &translated->ent);\n + \n +@@\n + \t/* Check straight mapping */\n + \tp.pattern = pattern->buf;\n + \tp.patternlen = pattern->len;\n +-\thashmap_entry_init(&p.ent, memhash(p.pattern, p.patternlen));\n ++\thashmap_entry_init(&p.ent,\n ++\t\t\t ignore_case ?\n ++\t\t\t strihash(p.pattern) :\n ++\t\t\t strhash(p.pattern));\n + \treturn !!hashmap_get_entry(map, &p, ent, NULL);\n }\n \n -+const char *index_dir_matching_name(struct index_state *istate, const char *name, int namelen)\n -+{\n -+\tstruct dir_entry *dir;\n -+\n -+\tlazy_init_name_hash(istate);\n -+\tdir = find_dir_entry(istate, name, namelen);\n -+\n -+\treturn dir ? dir->name : NULL;\n -+}\n -+\n - void adjust_dirname_case(struct index_state *istate, char *name)\n - {\n - \tconst char *startPtr = name;\n \n diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh\n --- a/t/t1091-sparse-checkout-builtin.sh\n @@ -125,16 +150,20 @@\n '\n \n +test_expect_success 'cone mode: set with core.ignoreCase=true' '\n -+\ttest_when_finished git -C repo config --unset core.ignoreCase &&\n +\tgit -C repo sparse-checkout init --cone &&\n -+\tgit -C repo config core.ignoreCase true &&\n -+\tgit -C repo sparse-checkout set Folder1 &&\n ++\tgit -C repo -c core.ignoreCase=true sparse-checkout set folder1 &&\n +\tcat >expect <<-EOF &&\n +\t\t/*\n +\t\t!/*/\n +\t\t/folder1/\n +\tEOF\n -+\ttest_cmp expect repo/.git/info/sparse-checkout\n ++\ttest_cmp expect repo/.git/info/sparse-checkout &&\n ++\tls repo >dir &&\n ++\tcat >expect <<-EOF &&\n ++\t\ta\n ++\t\tfolder1\n ++\tEOF\n ++\ttest_cmp expect dir\n +'\n +\n test_done\n\n-- \ngitgitgadget\n"},{"id":"388137","messageId":"dc97d5a0d2a39d3888b75e1154770817e16c3112.1576260593.git.gitgitgadget@gmail.com","threadId":"52418","inReplyTo":"pull.488.v2.git.1576260593.gitgitgadget@gmail.com","subject":"[PATCH v2 1/1] sparse-checkout: respect core.ignoreCase in cone mode","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-13T18:09:53Z","receivedAt":"2019-12-13T20:40:11Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhen a user uses the sparse-checkout feature in cone mode, they\nadd patterns using \"git sparse-checkout set ...\"\nor by using \"--stdin\" to provide the directories line-by-line over\nstdin. This behaviour naturally looks a lot like the way a user\nwould type \"git add ...\"\n\nIf core.ignoreCase is enabled, then \"git add\" will match the input\nusing a case-insensitive match. Do the same for the sparse-checkout\nfeature.\n\nPerform case-insensitive checks while updating the skip-worktree\nbits during unpack_trees(). This is done by changing the hash\nalgorithm and hashmap comparison methods to optionally use case-\ninsensitive methods.\n\nWhen this is enabled, there is a small performance cost in the\nhashing algorithm. To tease out the worst possible case, the\nfollowing was run on a repo with a deep directory structure:\n\n\tgit ls-tree -d -r --name-only HEAD |\n\t\tgit sparse-checkout set --stdin\n\nThe 'set' command was timed with core.ignoreCase disabled or\nenabled. For the repo with a deep history, the numbers were\n\n\tcore.ignoreCase=false: 62s\n\tcore.ignoreCase=true: 74s (+19.3%)\n\nFor reproducibility, the equivalent test on the Linux kernel\nrepository had these numbers:\n\n\tcore.ignoreCase=false: 3.1s\n\tcore.ignoreCase=true: 3.6s (+16%)\n\nNow, this is not an entirely fair comparison, as most users\nwill define their sparse cone using more shallow directories,\nand the performance improvement from eb42feca97 (\"unpack-trees:\nhash less in cone mode\" 2019-11-21) can remove most of the\nhash cost. For a more realistic test, drop the \"-r\" from the\nls-tree command to store only the first-level directories.\nIn that case, the Linux kernel repository takes 0.2-0.25s in\neach case, and the deep repository takes one second, plus or\nminus 0.05s, in each case.\n\nThus, we _can_ demonstrate a cost to this change, but it is\nunlikely to matter to any reasonable sparse-checkout cone.\n\nSigned-off-by: Derrick Stolee \n---\n Documentation/git-sparse-checkout.txt | 5 +++++\n builtin/sparse-checkout.c | 10 ++++++++--\n dir.c | 15 ++++++++++++---\n t/t1091-sparse-checkout-builtin.sh | 17 +++++++++++++++++\n 4 files changed, 42 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt\nindex b975285673..9c3c66cc37 100644\n--- a/Documentation/git-sparse-checkout.txt\n+++ b/Documentation/git-sparse-checkout.txt\n@@ -150,6 +150,11 @@ expecting patterns of these types. Git will warn if the patterns do not match.\n If the patterns do match the expected format, then Git will use faster hash-\n based algorithms to compute inclusion in the sparse-checkout.\n \n+If `core.ignoreCase=true`, then the pattern-matching algorithm will use a\n+case-insensitive check. This corrects for case mismatched filenames in the\n+'git sparse-checkout set' command to reflect the expected cone in the working\n+directory.\n+\n SEE ALSO\n --------\n \ndiff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c\nindex a542d617a5..5d62f7a66d 100644\n--- a/builtin/sparse-checkout.c\n+++ b/builtin/sparse-checkout.c\n@@ -313,7 +313,10 @@ static void insert_recursive_pattern(struct pattern_list *pl, struct strbuf *pat\n \tstruct pattern_entry *e = xmalloc(sizeof(*e));\n \te->patternlen = path->len;\n \te->pattern = strbuf_detach(path, NULL);\n-\thashmap_entry_init(&e->ent, memhash(e->pattern, e->patternlen));\n+\thashmap_entry_init(&e->ent,\n+\t\t\t ignore_case ?\n+\t\t\t strihash(e->pattern) :\n+\t\t\t strhash(e->pattern));\n \n \thashmap_add(&pl->recursive_hashmap, &e->ent);\n \n@@ -329,7 +332,10 @@ static void insert_recursive_pattern(struct pattern_list *pl, struct strbuf *pat\n \t\te = xmalloc(sizeof(struct pattern_entry));\n \t\te->patternlen = newlen;\n \t\te->pattern = xstrndup(oldpattern, newlen);\n-\t\thashmap_entry_init(&e->ent, memhash(e->pattern, e->patternlen));\n+\t\thashmap_entry_init(&e->ent,\n+\t\t\t\t ignore_case ?\n+\t\t\t\t strihash(e->pattern) :\n+\t\t\t\t strhash(e->pattern));\n \n \t\tif (!hashmap_get_entry(&pl->parent_hashmap, e, ent, NULL))\n \t\t\thashmap_add(&pl->parent_hashmap, &e->ent);\ndiff --git a/dir.c b/dir.c\nindex 2ef92a50a0..22d08e61c2 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -625,6 +625,8 @@ int pl_hashmap_cmp(const void *unused_cmp_data,\n \t\t\t ? ee1->patternlen\n \t\t\t : ee2->patternlen;\n \n+\tif (ignore_case)\n+\t\treturn strncasecmp(ee1->pattern, ee2->pattern, min_len);\n \treturn strncmp(ee1->pattern, ee2->pattern, min_len);\n }\n \n@@ -665,7 +667,9 @@ static void add_pattern_to_hashsets(struct pattern_list *pl, struct path_pattern\n \t\ttranslated->pattern = truncated;\n \t\ttranslated->patternlen = given->patternlen - 2;\n \t\thashmap_entry_init(&translated->ent,\n-\t\t\t\t memhash(translated->pattern, translated->patternlen));\n+\t\t\t\t ignore_case ?\n+\t\t\t\t strihash(translated->pattern) :\n+\t\t\t\t strhash(translated->pattern));\n \n \t\tif (!hashmap_get_entry(&pl->recursive_hashmap,\n \t\t\t\t translated, ent, NULL)) {\n@@ -694,7 +698,9 @@ static void add_pattern_to_hashsets(struct pattern_list *pl, struct path_pattern\n \ttranslated->pattern = xstrdup(given->pattern);\n \ttranslated->patternlen = given->patternlen;\n \thashmap_entry_init(&translated->ent,\n-\t\t\t memhash(translated->pattern, translated->patternlen));\n+\t\t\t ignore_case ?\n+\t\t\t strihash(translated->pattern) :\n+\t\t\t strhash(translated->pattern));\n \n \thashmap_add(&pl->recursive_hashmap, &translated->ent);\n \n@@ -724,7 +730,10 @@ static int hashmap_contains_path(struct hashmap *map,\n \t/* Check straight mapping */\n \tp.pattern = pattern->buf;\n \tp.patternlen = pattern->len;\n-\thashmap_entry_init(&p.ent, memhash(p.pattern, p.patternlen));\n+\thashmap_entry_init(&p.ent,\n+\t\t\t ignore_case ?\n+\t\t\t strihash(p.pattern) :\n+\t\t\t strhash(p.pattern));\n \treturn !!hashmap_get_entry(map, &p, ent, NULL);\n }\n \ndiff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh\nindex d5e2892526..cee98a1c8a 100755\n--- a/t/t1091-sparse-checkout-builtin.sh\n+++ b/t/t1091-sparse-checkout-builtin.sh\n@@ -304,4 +304,21 @@ test_expect_success 'sparse-checkout (init|set|disable) fails with dirty status'\n \tgit -C dirty sparse-checkout disable\n '\n \n+test_expect_success 'cone mode: set with core.ignoreCase=true' '\n+\tgit -C repo sparse-checkout init --cone &&\n+\tgit -C repo -c core.ignoreCase=true sparse-checkout set folder1 &&\n+\tcat >expect <<-EOF &&\n+\t\t/*\n+\t\t!/*/\n+\t\t/folder1/\n+\tEOF\n+\ttest_cmp expect repo/.git/info/sparse-checkout &&\n+\tls repo >dir &&\n+\tcat >expect <<-EOF &&\n+\t\ta\n+\t\tfolder1\n+\tEOF\n+\ttest_cmp expect dir\n+'\n+\n test_done\n-- \ngitgitgadget\n"},{"id":"388144","messageId":"xmqqwob0jbba.fsf@gitster-ct.c.googlers.com","threadId":"52418","inReplyTo":"7aeb1589-cc08-654f-1bc0-d48fec1c0729@gmail.com","subject":"Re: [PATCH 1/1] sparse-checkout: respect core.ignoreCase in cone mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-13T19:05:29Z","receivedAt":"2019-12-13T20:40:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n> If you have a plan for that branch and the merge status of those\n> commits, then I'm happy to re-target my PR against 'next' or\n> an equivalent branch.\n\nThis change does not make much sense without the cone-mode topic,\nno?\n"},{"id":"388148","messageId":"d1b11424-db8b-e13c-e896-923d74b3d531@gmail.com","threadId":"52418","inReplyTo":"xmqqwob0jbba.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] sparse-checkout: respect core.ignoreCase in cone mode","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-12-13T19:40:58Z","receivedAt":"2019-12-13T20:41:03Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 12/13/2019 2:05 PM, Junio C Hamano wrote:\n> Derrick Stolee writes:\n> \n>> If you have a plan for that branch and the merge status of those\n>> commits, then I'm happy to re-target my PR against 'next' or\n>> an equivalent branch.\n> \n> This change does not make much sense without the cone-mode topic,\n> no?\n\nI'm saying it should go on top, as a new branch that depends on\nds/sparse-cone. Sorry for not making that clear.\n\nThanks,\n-Stolee\n\n"},{"id":"388158","messageId":"xmqq7e30j8un.fsf@gitster-ct.c.googlers.com","threadId":"52418","inReplyTo":"pull.488.v2.git.1576260593.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/1] sparse-checkout: respect core.ignoreCase in cone mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-13T19:58:40Z","receivedAt":"2019-12-13T20:41:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> case-insensitive check when core.ignoreCase is set. Because the performance\n> impact is much lower than I anticipated, I no longer think it is important\n> to add a new config option to enable/disable this logic.\n\nNice.\n\nThanks.\n"},{"id":"388178","messageId":"xmqqeex7hoj8.fsf@gitster-ct.c.googlers.com","threadId":"52418","inReplyTo":"d1b11424-db8b-e13c-e896-923d74b3d531@gmail.com","subject":"Re: [PATCH 1/1] sparse-checkout: respect core.ignoreCase in cone mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-13T22:02:51Z","receivedAt":"2019-12-13T22:02:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n> On 12/13/2019 2:05 PM, Junio C Hamano wrote:\n>> Derrick Stolee writes:\n>> \n>>> If you have a plan for that branch and the merge status of those\n>>> commits, then I'm happy to re-target my PR against 'next' or\n>>> an equivalent branch.\n>> \n>> This change does not make much sense without the cone-mode topic,\n>> no?\n>\n> I'm saying it should go on top, as a new branch that depends on\n> ds/sparse-cone. Sorry for not making that clear.\n\nI thought that it was a *bug* that ds/sparse-cone topic does not\nmatch the pattern case insensitively on case insensitive systems,\nand from that point of view, letting the earlier part of the topic\ngraduate without the fix would not make sense, I thought.\n\nThanks.\n\n\n"},{"id":"388455","messageId":"CAPyFy2AwJ6TJpNWQOSPia9=5tMd1sUJKvRmNwbqTFkYWixTHjQ@mail.gmail.com","threadId":"52418","inReplyTo":"pull.488.git.1575920580.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/1] sparse-checkout: respect core.ignoreCase in cone mode","fromName":"Ed Maste","fromEmail":"emaste@freebsd.org","sentAt":"2019-12-18T11:41:00Z","receivedAt":"2019-12-18T19:16:43Z","isPatch":true,"sender":{"key":"emaste@freebsd.org","avatar":"https://avatars.githubusercontent.com/u/1034582?v=4"},"body":"On Mon, 9 Dec 2019 at 14:43, Derrick Stolee via GitGitGadget\n wrote:\n>\n> This is the first of several cleanups to the sparse-checkout feature that\n> had a large overhaul in ds/sparse-cone.\n\nAfter this landed in next t1091-sparse-checkout-builtin.sh failed in\nmy Cirrus-CI run on FreeBSD -\nhttps://cirrus-ci.com/task/5085664047005696\n\nThe first failure is:\n\nnot ok 4 - git sparse-checkout init\n#\n# git -C repo sparse-checkout init &&\n# cat >expect <<-EOF &&\n# /*\n# !/*/\n# EOF\n# test_cmp expect repo/.git/info/sparse-checkout &&\n# test_cmp_config -C repo true core.sparsecheckout &&\n# ls repo >dir &&\n# echo a >expect &&\n# test_cmp expect dir\n#\n\nI'm trying to reproduce locally and investigate.\n"}]}