Re: [PATCH 1/1] sparse-checkout: respect core.ignoreCase in cone mode
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 11, 2019, 18:44 UTC
- Message-ID
- <xmqqtv66og63.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <23705845ce73992bf7ab645d28febebe0a698d49.1575920580.git.gitgitgadget@gmail.com>
"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 5 quoted lines
> Another option would be to do case-insensitive checks while > updating the skip-worktree bits during unpack_trees(). Outside of > the potential performance loss on a more expensive code path, that > also breaks compatibility with older versions of Git as using the > same sparse-checkout file would change the paths that are included.
I haven't thought things through, but that sounds as if saying that we cannot fix old bugs. We use the entries in the index as a hint for "correcting" a path to the right case on a case-insensitive filesystem to avoid corrupting the case in the index, which is case sensitive and is a way to convey the intended case (by writing them out to a tree object) to those who use systems that can reliably reproduce cases in pathnames. But that still does not mean on a case-insensitive filesystem, "hello.c" in the "right" case recorded in the index will always be spelled like so---somebody can make readdir() or equivalent to yield "Hello.c" for it, and if the user tells us to say they want to do X (e.g. ignore, skip checkout, etc.) to "hello.c", we should do the same to "Hello.c" on such a system, I would think.
If the runtime needs to deal with the case insensitive filesystems anyway (and I suspect it does), there isn't much point matching and forcing the case to the paths in the index like this patch does, I think. So...
Show 116 quoted lines
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
> Documentation/git-sparse-checkout.txt | 4 ++++
> builtin/sparse-checkout.c | 19 +++++++++++++++++--
> cache.h | 1 +
> name-hash.c | 10 ++++++++++
> t/t1091-sparse-checkout-builtin.sh | 13 +++++++++++++
> 5 files changed, 45 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt
> index b975285673..849efa0f0b 100644
> --- a/Documentation/git-sparse-checkout.txt
> +++ b/Documentation/git-sparse-checkout.txt
> @@ -150,6 +150,10 @@ expecting patterns of these types. Git will warn if the patterns do not match.
> If the patterns do match the expected format, then Git will use faster hash-
> based algorithms to compute inclusion in the sparse-checkout.
>
> +If `core.ignoreCase=true`, then the 'git sparse-checkout set' command will
> +correct for incorrect case when assigning patterns to the sparse-checkout
> +file.
> +
> SEE ALSO
> --------
>
> diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
> index a542d617a5..0de426384e 100644
> --- a/builtin/sparse-checkout.c
> +++ b/builtin/sparse-checkout.c
> @@ -336,6 +336,22 @@ static void insert_recursive_pattern(struct pattern_list *pl, struct strbuf *pat
> }
> }
>
> +static void sanitize_cone_input(struct strbuf *line)
> +{
> + if (ignore_case) {
> + struct index_state *istate = the_repository->index;
> + const char *name = index_dir_matching_name(istate, line->buf, line->len);
> +
> + if (name) {
> + strbuf_setlen(line, 0);
> + strbuf_addstr(line, name);
> + }
> + }
> +
> + if (line->buf[0] != '/')
> + strbuf_insert(line, 0, "/", 1);
> +}
> +
> static void strbuf_to_cone_pattern(struct strbuf *line, struct pattern_list *pl)
> {
> strbuf_trim(line);
> @@ -345,8 +361,7 @@ static void strbuf_to_cone_pattern(struct strbuf *line, struct pattern_list *pl)
> if (!line->len)
> return;
>
> - if (line->buf[0] != '/')
> - strbuf_insert(line, 0, "/", 1);
> + sanitize_cone_input(line);
>
> insert_recursive_pattern(pl, line);
> }
> diff --git a/cache.h b/cache.h
> index d3c89e7a53..a2d9d437f0 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -728,6 +728,7 @@ int repo_index_has_changes(struct repository *repo,
> int verify_path(const char *path, unsigned mode);
> int strcmp_offset(const char *s1, const char *s2, size_t *first_change);
> int index_dir_exists(struct index_state *istate, const char *name, int namelen);
> +const char *index_dir_matching_name(struct index_state *istate, const char *name, int namelen);
> void adjust_dirname_case(struct index_state *istate, char *name);
> struct cache_entry *index_file_exists(struct index_state *istate, const char *name, int namelen, int igncase);
>
> diff --git a/name-hash.c b/name-hash.c
> index ceb1d7bd6f..46898b6571 100644
> --- a/name-hash.c
> +++ b/name-hash.c
> @@ -681,6 +681,16 @@ int index_dir_exists(struct index_state *istate, const char *name, int namelen)
> return dir && dir->nr;
> }
>
> +const char *index_dir_matching_name(struct index_state *istate, const char *name, int namelen)
> +{
> + struct dir_entry *dir;
> +
> + lazy_init_name_hash(istate);
> + dir = find_dir_entry(istate, name, namelen);
> +
> + return dir ? dir->name : NULL;
> +}
> +
> void adjust_dirname_case(struct index_state *istate, char *name)
> {
> const char *startPtr = name;
> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
> index d5e2892526..d0ce48869f 100755
> --- a/t/t1091-sparse-checkout-builtin.sh
> +++ b/t/t1091-sparse-checkout-builtin.sh
> @@ -304,4 +304,17 @@ test_expect_success 'sparse-checkout (init|set|disable) fails with dirty status'
> git -C dirty sparse-checkout disable
> '
>
> +test_expect_success 'cone mode: set with core.ignoreCase=true' '
> + test_when_finished git -C repo config --unset core.ignoreCase &&
> + git -C repo sparse-checkout init --cone &&
> + git -C repo config core.ignoreCase true &&
> + git -C repo sparse-checkout set Folder1 &&
> + cat >expect <<-EOF &&
> + /*
> + !/*/
> + /folder1/
> + EOF
> + test_cmp expect repo/.git/info/sparse-checkout
> +'
> +
> test_done