git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Derrick Stolee <stolee@gmail.com>
Cc: Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org, szeder.dev@gmail.com, newren@gmail.com,
	Derrick Stolee <dstolee@microsoft.com>
Subject: Re: [PATCH 1/1] sparse-checkout: respect core.ignoreCase in cone mode
Date: Wed, 11 Dec 2019 13:37:27 -0800	[thread overview]
Message-ID: <xmqqa77yo86g.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <3e64b470-93cc-5491-09e1-e499f0a7583d@gmail.com> (Derrick Stolee's message of "Wed, 11 Dec 2019 15:29:38 -0500")

Derrick Stolee <stolee@gmail.com> writes:

> The specific example I have in my mind is that the filesystem can have
> "README.TXT" as what it would report by readdir(), but a user can type
>
> 	git add readme.txt
>
> Without core.ignoreCase, the index will store the path as "readme.txt",
> possibly adding a new entry in the index next to "README.TXT". If
> core.ignoreCase is enabled, then the case reported by readdir() is used.
> (This works both for a tracked and untracked path.)

Yes, but which one is "correct"?  readme.txt read from the index or
README.TXT read from the filesystem?  Presumably, when the paths got
first checked out of the index, it would have been in "readme.txt"
on the filesystem, so the user must have done on purpose something
to cause the file to be named in all uppercase since then.

> This is a case that is difficult to control for. We have no source
> of truth (filesystem or index) at the time of the 'git sparse-checkout
> set' command. I cannot think of a solution to this specific issue
> without doing the more-costly approach on every unpack_trees() call.
>
> I believe this case may be narrow enough to "document and don't-fix",
> but please speak up if anyone thinks this _must_ be fixed.

I do not think it "must be" fixed in the sense that "if it hurts,
don't do so" is a sufficient remedy.  But then for exactly the same
reason, I do not think it is worth doing what you do in this patch.

On the other hand, I think runtime case folding, just like what we
do when "git add" is told to handle a path in different case, would
be the only "right" way to match end-user expectations on a case
insensitive system, even though that is a "nice to do so" and
certainly not a "must do so" thing.

> The "git add" behavior made me think there was sufficient precedent
> in Git to try this change.

Since I view that the "git add" behaviour is a "nice to do so"
runtime conversion, I would actually think the approach you
discarded would be more in line with the precedent.

> I'm happy to follow the community's opinion in this matter, including
> a hard "we will not correct for case in this feature" or "if you want
> this then you'll pay for it in performance." In the latter case I'd
> prefer a new config option to toggle the sparse-checkout case
> insensitivity. That way users could have core.ignoreCase behavior without
> the perf hit if they use clean input to the sparse-checkout feature.

I value correctness more---a system that does incorrect thing
quickly is not all that interesting.

Assuming that your users are sensible and feed good data, how much
"performance hit" are we talking about?  Wouldn't this be something
we can make into a "if you have the paths in the correct case, we'll
see the match just as quickly as in the case sensitive system, so
most of the time there is no penalty, but for correctness we would
also fall back to try different cases"?

  reply	other threads:[~2019-12-11 21:37 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-09 19:42 [PATCH 0/1] sparse-checkout: respect core.ignoreCase in cone mode Derrick Stolee via GitGitGadget
2019-12-09 19:43 ` [PATCH 1/1] " Derrick Stolee via GitGitGadget
2019-12-11 18:44   ` Junio C Hamano
2019-12-11 19:11     ` Derrick Stolee
2019-12-11 20:00       ` Junio C Hamano
2019-12-11 20:29         ` Derrick Stolee
2019-12-11 21:37           ` Junio C Hamano [this message]
2019-12-12  2:51             ` Derrick Stolee
2019-12-12 20:59   ` Derrick Stolee
2019-12-13 19:05     ` Junio C Hamano
2019-12-13 19:40       ` Derrick Stolee
2019-12-13 22:02         ` Junio C Hamano
2019-12-13 18:09 ` [PATCH v2 0/1] " Derrick Stolee via GitGitGadget
2019-12-13 18:09   ` [PATCH v2 1/1] " Derrick Stolee via GitGitGadget
2019-12-13 19:58   ` [PATCH v2 0/1] " Junio C Hamano
2019-12-18 11:41 ` [PATCH " Ed Maste

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqa77yo86g.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=dstolee@microsoft.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=newren@gmail.com \
    --cc=stolee@gmail.com \
    --cc=szeder.dev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).