[PATCH] Demonstrate failure of 'core.ignorecase = true'

[PATCH] Demonstrate failure of 'core.ignorecase = true'

[Peter J. Weisberg]

From: "Peter J. Weisberg" <pj@irregularexpressions.net>

On a filesystem that *is* case-sensitive, renaming a file to a name
that would be equivalent on a case-insensitive filesystem makes Git
think the original file was deleted.  Add a test that demonstrates
this as a known error.
---
I have a repository that contains files that I sync from a place where
names are case-insensitive.  When I sync a file that has a change in
the case of the file name, I want Git to ignore that non-change.  I
would think core.ignorecase would accomplish this, but it does not.
---
 t/t2000-ignorecase-config.sh |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)
 create mode 100755 t/t2000-ignorecase-config.sh

diff --git a/t/t2000-ignorecase-config.sh b/t/t2000-ignorecase-config.sh
new file mode 100755
index 0000000..9d05cee
--- /dev/null
+++ b/t/t2000-ignorecase-config.sh
@@ -0,0 +1,21 @@
+#!/bin/sh
+#
+# Copyright (c) 2012 Peter J Weisberg
+#
+
+test_description='core.ignorecase'
+
+. ./test-lib.sh
+
+test_expect_failure "diff-files doesn't show case change when ignorecase=true" '
+	git config core.ignorecase true &&
+
+	touch foo &&
+	git add foo &&
+	git commit -m "foo" &&
+	mv foo FOO &&
+
+	test -z "$(git diff-files)"
+'
+
+test_done
-- 
1.7.9.1

Re: [PATCH] Demonstrate failure of 'core.ignorecase = true'

[Junio C Hamano]

"Peter J. Weisberg" <pj@irregularexpressions.net> writes:

> From: "Peter J. Weisberg" <pj@irregularexpressions.net>
>
> On a filesystem that *is* case-sensitive, renaming a file to a name
> that would be equivalent on a case-insensitive filesystem makes Git
> think the original file was deleted.  Add a test that demonstrates
> this as a known error.
> ---

Thanks, Needs sign-off.

> I have a repository that contains files that I sync from a place where
> names are case-insensitive.  When I sync a file that has a change in
> the case of the file name, I want Git to ignore that non-change.  I
> would think core.ignorecase would accomplish this, but it does not.
> ---

No need for the second "---"

>  t/t2000-ignorecase-config.sh |   21 +++++++++++++++++++++

We'd rather not waste a new test number for a single test like this.

>  1 files changed, 21 insertions(+), 0 deletions(-)
>  create mode 100755 t/t2000-ignorecase-config.sh
>
> diff --git a/t/t2000-ignorecase-config.sh b/t/t2000-ignorecase-config.sh
> new file mode 100755
> index 0000000..9d05cee
> --- /dev/null
> +++ b/t/t2000-ignorecase-config.sh
> @@ -0,0 +1,21 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2012 Peter J Weisberg
> +#
> +
> +test_description='core.ignorecase'
> +
> +. ./test-lib.sh
> +
> +test_expect_failure "diff-files doesn't show case change when ignorecase=true" '

This needs to be protected by test prerequisite to make sure that the test
is run on a case insensitive filesystem.  Even if you declare that the
filesystem is case insensitive by setting core.ignorecase to true, the
underlying system calls like open("foo") will *not* magically start
returning a file descriptor opened for "FOO" if your filesystem is not
case insensitive.

Perhaps something as simple as the following would do:

	# on case insensitive filesystems, "mv" would fail
        if >testfile && ! mv testfile TESTFILE >/dev/null 2>/dev/null
        then
                test_set_prereq CASE_INSENSITIVE_FS
        fi
        rm -f testfile TESTFILE

	test_expect_failure CASE_INSENSITIVE_FS "diff-files doesn't..." '
        	... test body comes here ...


> +	git config core.ignorecase true &&
> +
> +	touch foo &&
> +	git add foo &&
> +	git commit -m "foo" &&
> +	mv foo FOO &&
> +
> +	test -z "$(git diff-files)"
> +'
> +
> +test_done

Re: [PATCH] Demonstrate failure of 'core.ignorecase = true'

[Johannes Sixt]

Am 3/21/2012 23:50, schrieb Peter J. Weisberg:
> +test_expect_failure "diff-files doesn't show case change when ignorecase=true" '
> +	git config core.ignorecase true &&
> +
> +	touch foo &&
> +	git add foo &&
> +	git commit -m "foo" &&
> +	mv foo FOO &&
> +
> +	test -z "$(git diff-files)"
> +'

I tried this in my git.git clone on Windows (NTFS), and it did not produce
the expected failure:

D:\Src\mingw-git>git config core.ignorecase
true

D:\Src\mingw-git>mv git.c GIT.C

D:\Src\mingw-git>git diff-files

D:\Src\mingw-git>echo %ERRORLEVEL%
0

D:\Src\mingw-git>ls -l [Gg][Ii][Tt].[Cc]
-rw-r--r--    1 jsixt    Administ    17166 Mar 22 06:44 GIT.C

What am I missing?

-- Hannes

Re: [PATCH] Demonstrate failure of 'core.ignorecase = true'

[Zbigniew Jędrzejewski-Szmek]

On 03/22/2012 07:49 AM, Johannes Sixt wrote:
> Am 3/21/2012 23:50, schrieb Peter J. Weisberg:
>> +test_expect_failure "diff-files doesn't show case change when ignorecase=true" '
>> +	git config core.ignorecase true&&
>> +
>> +	touch foo&&
>> +	git add foo&&
>> +	git commit -m "foo"&&
>> +	mv foo FOO&&
>> +
>> +	test -z "$(git diff-files)"
>> +'
>
> I tried this in my git.git clone on Windows (NTFS), and it did not produce
> the expected failure:
...
> What am I missing?

The OP meant a case-sensitive fs, not an insensitive one.
"On a filesystem that *is* case-sensitive, ..."

This is a question about core.ignorecase=true. The description in 
git-config(1) is so vague, that it's hard to say what behaviour is expected.

Zbyszek

Re: [PATCH] Demonstrate failure of 'core.ignorecase = true'

[Jeff King]

On Thu, Mar 22, 2012 at 12:25:50PM +0100, Zbigniew Jędrzejewski-Szmek wrote:

> >What am I missing?
> 
> The OP meant a case-sensitive fs, not an insensitive one.
> "On a filesystem that *is* case-sensitive, ..."
> 
> This is a question about core.ignorecase=true. The description in
> git-config(1) is so vague, that it's hard to say what behaviour is
> expected.

I don't know. It says:

  If true, this option enables various workarounds to enable git to work
  better on filesystems that are not case sensitive, like FAT. For
  example, if a directory listing finds "makefile" when git expects
  "Makefile", git will assume it is really the same file, and continue
  to remember it as "Makefile".

which seems pretty clear to me that this is "let git work better on
case-insensitive filesystems", not "make git magically case-insensitive
on case sensitive filesystem". But maybe we could add be more explicit,
like:

-- >8 --
Subject: docs: clarify core.ignorecase on case-sensitive filesystems

core.ignorecase is about handling case-insensitive
filesystems, not making git magically case-insensitive on a
case-sensitive filesystem. That's implied by the current
text, but let's add an explicit note.

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/config.txt |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index c081657..abbab91 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -191,6 +191,10 @@ core.ignorecase::
 The default is false, except linkgit:git-clone[1] or linkgit:git-init[1]
 will probe and set core.ignorecase true if appropriate when the repository
 is created.
++
+Note that this is about making git work well on a case-insensitive
+filesystem. It will not make git case-insensitive when used on a
+case-sensitive filesystem.
 
 core.trustctime::
 	If false, the ctime differences between the index and the
-- 
1.7.10.rc0.9.gdcbe9

Re: [PATCH] Demonstrate failure of 'core.ignorecase = true'

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I don't know. It says:
>
>   If true, this option enables various workarounds to enable git to work
>   better on filesystems that are not case sensitive, like FAT. For
>   example, if a directory listing finds "makefile" when git expects
>   "Makefile", git will assume it is really the same file, and continue
>   to remember it as "Makefile".
>
> which seems pretty clear to me that this is "let git work better on
> case-insensitive filesystems", not "make git magically case-insensitive
> on case sensitive filesystem".

Yes, it says what it needs to say quite clearly.

> But maybe we could add be more explicit,
> like:

Hrm, replacing unclear part with clarified text may make sense, but it
would not help adding new text if the existing description is not clear
enough.

How about doing it like this?

   Case-insensitive filesystems like FAT and HFS+ have various strange
   behaviours, like reporting that a file "Makefile" already exists when
   the file that actually exists on them is "makefile". By setting this
   variable to `true`, Git employs logic to work around them.

   The default is false, except that git-clone[1] and git-init[1] will
   probe the filesystem and set it to `true` as necessary when a new
   repository is created.

Re: [PATCH] Demonstrate failure of 'core.ignorecase = true'

[Jeff King]

On Thu, Mar 22, 2012 at 09:57:23AM -0700, Junio C Hamano wrote:

> Hrm, replacing unclear part with clarified text may make sense, but it
> would not help adding new text if the existing description is not clear
> enough.
> 
> How about doing it like this?
> 
>    Case-insensitive filesystems like FAT and HFS+ have various strange
>    behaviours, like reporting that a file "Makefile" already exists when
>    the file that actually exists on them is "makefile". By setting this
>    variable to `true`, Git employs logic to work around them.
> 
>    The default is false, except that git-clone[1] and git-init[1] will
>    probe the filesystem and set it to `true` as necessary when a new
>    repository is created.

IMHO, it suffers from the same problem as the original, which is that it
does tells when to use core.ignorecase, but does not specify what
happens when one sets core.ignorecase to true on a case-sensitive
filesystem. Maybe we should be more explicit about what _does_ happen in
that case (to be honest, I am not completely sure). Or just say that it
is not a supported use case.

-Peff

Re: [PATCH] Demonstrate failure of 'core.ignorecase = true'

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Thu, Mar 22, 2012 at 09:57:23AM -0700, Junio C Hamano wrote:
>
>> Hrm, replacing unclear part with clarified text may make sense, but it
>> would not help adding new text if the existing description is not clear
>> enough.
>> 
>> How about doing it like this?
>> 
>>    Case-insensitive filesystems like FAT and HFS+ have various strange
>>    behaviours, like reporting that a file "Makefile" already exists when
>>    the file that actually exists on them is "makefile". By setting this
>>    variable to `true`, Git employs logic to work around them.
>> 
>>    The default is false, except that git-clone[1] and git-init[1] will
>>    probe the filesystem and set it to `true` as necessary when a new
>>    repository is created.
>
> IMHO, it suffers from the same problem as the original, which is that it
> does tells when to use core.ignorecase, but does not specify what

I wanted it to tell *what* happens when core.ignorecase is set.  In other
words, I wanted the description to say that the logic employed is to work
around what case-insensitive filesystems do.  Case sensitive filesystems
obviously do not do what case-insensitive ones do (like reporting a
"Makefile" exists when only "makefile" exists), so I hoped that it was
clear enough that the additional logic would not be suitable there.

> happens when one sets core.ignorecase to true on a case-sensitive
> filesystem. Maybe we should be more explicit about what _does_ happen in
> that case (to be honest, I am not completely sure). Or just say that it
> is not a supported use case.

I guess we really need to make the description foolproof then.

                   ... exists on them is "makefile". By setting this
	variable to `true`, Git employs logic to work around them.
        Setting this to `true` on a case insensitive filesystem does
	not make any sense, because it would not magically make your
	system to treat your filesystem case insensitively.

Re: [PATCH] Demonstrate failure of 'core.ignorecase = true'

[Jeff King]

On Thu, Mar 22, 2012 at 11:44:42AM -0700, Junio C Hamano wrote:

> I wanted it to tell *what* happens when core.ignorecase is set.  In other
> words, I wanted the description to say that the logic employed is to work
> around what case-insensitive filesystems do.  Case sensitive filesystems
> obviously do not do what case-insensitive ones do (like reporting a
> "Makefile" exists when only "makefile" exists), so I hoped that it was
> clear enough that the additional logic would not be suitable there.

Ah. I see now why you made the change you did. But if I missed it,
perhaps it was too subtle (of course, I found the other one perfectly
adequate, so...).

> I guess we really need to make the description foolproof then.
> 
>                    ... exists on them is "makefile". By setting this
> 	variable to `true`, Git employs logic to work around them.
>         Setting this to `true` on a case insensitive filesystem does
> 	not make any sense, because it would not magically make your
> 	system to treat your filesystem case insensitively.

I'm OK with that (modulo s/insensitive/sensitive/ on the third line).
It may be overly explicit, but I would rather err on that side.

-Peff

Re: [PATCH] Demonstrate failure of 'core.ignorecase = true'

[Zbigniew Jędrzejewski-Szmek]

On 03/22/2012 07:44 PM, Junio C Hamano wrote:
> Jeff King<peff@peff.net>  writes:
>
>> On Thu, Mar 22, 2012 at 09:57:23AM -0700, Junio C Hamano wrote:
>>
>>> Hrm, replacing unclear part with clarified text may make sense, but it
>>> would not help adding new text if the existing description is not clear
>>> enough.
>>>
>>> How about doing it like this?
>>>
>>>     Case-insensitive filesystems like FAT and HFS+ have various strange
>>>     behaviours, like reporting that a file "Makefile" already exists when
>>>     the file that actually exists on them is "makefile". By setting this
>>>     variable to `true`, Git employs logic to work around them.
I think that this paragraph is too judgemental. While case-insensitive 
filesystems may be a pain, they are not "strange" to their users, but 
rather natural, and don't require "working around".

> I guess we really need to make the description foolproof then.
>
>                     ... exists on them is "makefile". By setting this
> 	variable to `true`, Git employs logic to work around them.
>          Setting this to `true` on a case insensitive filesystem does
> 	not make any sense, because it would not magically make your
> 	system to treat your filesystem case insensitively.
Even this updated text does not say _what_ happens when core.ignorecase 
is set on a case-insensitive filesystem. Once that's cleared up, then 
the corner case of core.ignorecase=true on case-sensitive fs can be tackled.

Maybe:
--- 8< ---
When set, case-insensitive comparisons will be used when internally 
comparing file names.

The default is false, but when a new repository is created by 
git-clone[1] or git-init[1], git will probe the filesystem and set it to 
`true` if the filesystem is case-insensitive.

On case-insensitive filesystems like FAT, NTFS and HSF+, names that 
differ only in capitalization, like "Makefile" and "makefile", refer to 
the same file. While such filesystems usually preserve the 
capitalization used during file creation, tools designed for such 
filesystems will often modify capitalization when saving files and when 
displaying filenames. Enabling core.ignorecase causes git to ignore 
case-only differences in file names.

Enabling core.ignorecase on a case insensitive filesystem does
not make sense, because filenames with different capitalization will 
still be treated as different by the filesystem.
--- >8 ---

[+cc Brandon Casey]

zByszek

Re: [PATCH] Demonstrate failure of 'core.ignorecase = true'

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Thu, Mar 22, 2012 at 11:44:42AM -0700, Junio C Hamano wrote:
>
>> I wanted it to tell *what* happens when core.ignorecase is set.  In other
>> words, I wanted the description to say that the logic employed is to work
>> around what case-insensitive filesystems do.  Case sensitive filesystems
>> obviously do not do what case-insensitive ones do (like reporting a
>> "Makefile" exists when only "makefile" exists), so I hoped that it was
>> clear enough that the additional logic would not be suitable there.
>
> Ah. I see now why you made the change you did. But if I missed it,
> perhaps it was too subtle (of course, I found the other one perfectly
> adequate, so...).
>
>> I guess we really need to make the description foolproof then.
>> 
>>                    ... exists on them is "makefile". By setting this
>> 	variable to `true`, Git employs logic to work around them.
>>         Setting this to `true` on a case insensitive filesystem does
>> 	not make any sense, because it would not magically make your
>> 	system to treat your filesystem case insensitively.
>
> I'm OK with that (modulo s/insensitive/sensitive/ on the third line).
> It may be overly explicit, but I would rather err on that side.

Thanks for catching the typo.

Re: [PATCH] Demonstrate failure of 'core.ignorecase = true'

[Junio C Hamano]

Zbigniew Jędrzejewski-Szmek  <zbyszek@in.waw.pl> writes:

> Even this updated text does not say _what_ happens when
> core.ignorecase is set on a case-insensitive filesystem.

That was very much on purpose. We tell users not to do that, because it is
calling for an undefined behaviour. And leaving it undefined gives us a
wiggle room to later do something better if we choose to.

> Maybe:
> --- 8< ---
> When set, case-insensitive comparisons will be used when internally
> comparing file names.

When we try to create a new file with open("./Makefile", O_CREAT) system
call, we do not opendir(".")  and readdir() to see if "makefile" exists
ourselves at all, but the above makes it sound as if we would do such
things to make sure we compare filenames ignoring there case.

That is *not* what happens, and that is not what we want to say in the
documentation.

Re: [PATCH] Demonstrate failure of 'core.ignorecase = true'

[PJ Weisberg]

As you've probably deduced, I simply failed to RTFM.  I'm not sure
you'll gain much by changing the description on the man page, since I
thought the name 'ignorecase' was self-explanatory and barely even
looked at it.  :-/

On Wed, Mar 21, 2012 at 4:58 PM, Junio C Hamano <gitster@pobox.com> wrote:

> underlying system calls like open("foo") will *not* magically start
> returning a file descriptor opened for "FOO" if your filesystem is not
> case insensitive.

No, but magic_open("foo") might, if someone had put forth the effort
to write a function called magic_open.  But the more I think about it,
the more it seems that doing everything you would need to do to make
ignorecase work the way I thought it did is almost certainly not worth
the effort.


-PJ

Gehm's Corollary to Clark's Law: Any technology distinguishable from
magic is insufficiently advanced.

Re: [PATCH] Demonstrate failure of 'core.ignorecase = true'

[Zbigniew Jędrzejewski-Szmek]

On 03/22/2012 09:37 PM, Junio C Hamano wrote:
> Zbigniew Jędrzejewski-Szmek<zbyszek@in.waw.pl>  writes:
>
>> Even this updated text does not say _what_ happens when
>> core.ignorecase is set on a case-insensitive filesystem.
>
> That was very much on purpose. We tell users not to do that, because it is
> calling for an undefined behaviour. And leaving it undefined gives us a
> wiggle room to later do something better if we choose to.
>
>> Maybe:
>> --- 8<  ---
>> When set, case-insensitive comparisons will be used when internally
>> comparing file names.
>
> When we try to create a new file with open("./Makefile", O_CREAT) system
> call, we do not opendir(".")  and readdir() to see if "makefile" exists
> ourselves at all, but the above makes it sound as if we would do such
> things to make sure we compare filenames ignoring there case.

Hence "internally" -- in the sense that filesystem calls are executed
by the OS, so they can be said to be external to git. Maybe this could
be worded differently.

> That is *not* what happens, and that is not what we want to say in the
> documentation.
Yeah, but we should say *something*, to let the reader understand
the behaviour.

For example, the reader should understand, that "work arounds" 
implemented by git do not include the normalization of filenames,
and if files are added with bad capitalization, they will stay that way 
on case sensitive filesystems.

-
Zbyszek

Re: [PATCH] Demonstrate failure of 'core.ignorecase = true'

[PJ Weisberg]

2012/3/22 Junio C Hamano <gitster@pobox.com>:
> Zbigniew Jędrzejewski-Szmek  <zbyszek@in.waw.pl> writes:
>
>> Even this updated text does not say _what_ happens when
>> core.ignorecase is set on a case-insensitive filesystem.
>
> That was very much on purpose. We tell users not to do that, because it is
> calling for an undefined behaviour. And leaving it undefined gives us a
> wiggle room to later do something better if we choose to.

Where do you tell users not to do that?  Nothing on the man page
actually says that the behavior is undefined, but it does seem to be
weird.  Rename default.asp to Default.asp, and Git reports that
default.asp was deleted, but doesn't mention Default.asp.  (Presumably
it sees Default.asp and decides that it's the same as default.asp,
which it already determined was deleted.  Like I said, weird.)

-PJ

Gehm's Corollary to Clark's Law: Any technology distinguishable from
magic is insufficiently advanced.

Re: [PATCH] Demonstrate failure of 'core.ignorecase = true'

[Junio C Hamano]

PJ Weisberg <pj@irregularexpressions.net> writes:

> On Wed, Mar 21, 2012 at 4:58 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
>> underlying system calls like open("foo") will *not* magically start
>> returning a file descriptor opened for "FOO" if your filesystem is not
>> case insensitive.
>
> No, but magic_open("foo") might, if someone had put forth the effort
> to write a function called magic_open.

Exactly.

That is why we avoid describing what happens when you set it on a case
sensitive filesystem, to leave the door open for such a cleverness.

It may still be a mistake in the manual that we did not explicitly say
that setting core.ignorecase on a case sensitive system will give you an
undefined behaviour.

Re: [PATCH] Demonstrate failure of 'core.ignorecase = true'

[Junio C Hamano]

PJ Weisberg <pj@irregularexpressions.net> writes:

> 2012/3/22 Junio C Hamano <gitster@pobox.com>:
>> Zbigniew Jędrzejewski-Szmek  <zbyszek@in.waw.pl> writes:
>>
>>> Even this updated text does not say _what_ happens when
>>> core.ignorecase is set on a case-insensitive filesystem.
>>
>> That was very much on purpose. We tell users not to do that, because it is
>> calling for an undefined behaviour. And leaving it undefined gives us a
>> wiggle room to later do something better if we choose to.
>
> Where do you tell users not to do that?  Nothing on the man page
> actually says that the behavior is undefined,...

Read the messages in the thread you are responding to. It is a discussion
about how we update the documentation to say it.

Re: [PATCH] Demonstrate failure of 'core.ignorecase = true'

[Jeff King]

On Thu, Mar 22, 2012 at 09:00:31PM +0100, Zbigniew Jędrzejewski-Szmek wrote:

> Maybe:
> --- 8< ---
> When set, case-insensitive comparisons will be used when internally
> comparing file names.
> 
> The default is false, but when a new repository is created by
> git-clone[1] or git-init[1], git will probe the filesystem and set it
> to `true` if the filesystem is case-insensitive.
> 
> On case-insensitive filesystems like FAT, NTFS and HSF+, names that
> differ only in capitalization, like "Makefile" and "makefile", refer
> to the same file. While such filesystems usually preserve the
> capitalization used during file creation, tools designed for such
> filesystems will often modify capitalization when saving files and
> when displaying filenames. Enabling core.ignorecase causes git to
> ignore case-only differences in file names.
> 
> Enabling core.ignorecase on a case insensitive filesystem does
> not make sense, because filenames with different capitalization will
> still be treated as different by the filesystem.
> --- >8 ---

From his response, I guess Junio does not agree, but this is my favorite
of the texts proposed so far.

-Peff

PS If we do use it, it needs s/HSF/HFS/.

Re: [PATCH] Demonstrate failure of 'core.ignorecase = true'

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Thu, Mar 22, 2012 at 09:00:31PM +0100, Zbigniew Jędrzejewski-Szmek wrote:
>
>> Maybe:
>> --- 8< ---
>> When set, case-insensitive comparisons will be used when internally
>> comparing file names.
>> 
>> The default is false, but when a new repository is created by
>> git-clone[1] or git-init[1], git will probe the filesystem and set it
>> to `true` if the filesystem is case-insensitive.
>> 
>> On case-insensitive filesystems like FAT, NTFS and HSF+, names that
>> differ only in capitalization, like "Makefile" and "makefile", refer
>> to the same file. While such filesystems usually preserve the
>> capitalization used during file creation, tools designed for such
>> filesystems will often modify capitalization when saving files and
>> when displaying filenames. Enabling core.ignorecase causes git to
>> ignore case-only differences in file names.
>> 
>> Enabling core.ignorecase on a case insensitive filesystem does
>> not make sense, because filenames with different capitalization will
>> still be treated as different by the filesystem.
>> --- >8 ---
>
> From his response, I guess Junio does not agree, but this is my favorite
> of the texts proposed so far.

I do not care too deeply, as long as we do not paint ourselves in a corner
by saying things that we do not have to say and end up sounding as if we
are defining what the undefined behaviour should be.

If the change of the presentation order seen above is reverted (in other
words, drop the first paragraph, move the second paragraph to the very
end), I wouldn't mind the above too much.

> PS If we do use it, it needs s/HSF/HFS/.

That too.

Re: [PATCH] Demonstrate failure of 'core.ignorecase = true'

[Thomas Rast]

Junio C Hamano <gitster@pobox.com> writes:

> PJ Weisberg <pj@irregularexpressions.net> writes:
>
>> On Wed, Mar 21, 2012 at 4:58 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>>> underlying system calls like open("foo") will *not* magically start
>>> returning a file descriptor opened for "FOO" if your filesystem is not
>>> case insensitive.
>>
>> No, but magic_open("foo") might, if someone had put forth the effort
>> to write a function called magic_open.
>
> Exactly.
>
> That is why we avoid describing what happens when you set it on a case
> sensitive filesystem, to leave the door open for such a cleverness.
>
> It may still be a mistake in the manual that we did not explicitly say
> that setting core.ignorecase on a case sensitive system will give you an
> undefined behaviour.

How about trying to read "HEAD" as "head" instead when core.ignorecase
is true?  That would allow us to catch such misconfiguration (which I
imagine can also happen accidentally if you mv a repository across FS
boundaries) and tell the user about it.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH] Demonstrate failure of 'core.ignorecase = true'

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

> How about trying to read "HEAD" as "head" instead when core.ignorecase
> is true?  That would allow us to catch such misconfiguration (which I
> imagine can also happen accidentally if you mv a repository across FS
> boundaries) and tell the user about it.

Do you mean something like this?

I do not like it.  It essentially amounts to checking with the FS every
time we run Git.

 config.c |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/config.c b/config.c
index 68d3294..8783937 100644
--- a/config.c
+++ b/config.c
@@ -575,7 +575,16 @@ static int git_default_core_config(const char *var, const char *value)
 	}
 
 	if (!strcmp(var, "core.ignorecase")) {
+		static int true_case; /* 0: unknown, 1: sensitive, 2: fat */
 		ignore_case = git_config_bool(var, value);
+		if (ignore_case) {
+			if (!true_case) {
+				true_case = fs_is_case_sensitive() ? 1 : 2;
+				if (true_case == 2)
+					warn("Whoa");
+			}
+			ignore_case = true_case >> 1;
+		}
 		return 0;
 	}
 

Re: [PATCH] Demonstrate failure of 'core.ignorecase = true'

[Jeff King]

On Fri, Mar 23, 2012 at 10:47:48AM -0700, Junio C Hamano wrote:

> Thomas Rast <trast@student.ethz.ch> writes:
> 
> > How about trying to read "HEAD" as "head" instead when core.ignorecase
> > is true?  That would allow us to catch such misconfiguration (which I
> > imagine can also happen accidentally if you mv a repository across FS
> > boundaries) and tell the user about it.
> 
> Do you mean something like this?
> 
> I do not like it.  It essentially amounts to checking with the FS every
> time we run Git.

I think Thomas's suggestion is to piggy-back it onto an existing file
lookup ("head" instead of "HEAD"), so you aren't doing any extra work.
However, I'm not sure that would be sufficient. If I copy a repo from a
case-insensitive filesystem to a case-sensitive one, what will the case
of "HEAD" be on the new filesystem?

If the original filesystem was case-preserving, I would expect "HEAD".
But on a true caseless filesystem, it could be either. Of course,
current git would already blow up if the file was copied as "head",
which makes me think this is probably a rare case. So maybe that is not
worth worrying about.

I dunno. I think Thomas's idea is clever, but is this actually a problem
in practice? The current discussion seems more like a documentation bug,
and I don't remember seeing anybody reporting issues moving a repo
across filesystems (presumably most people use clone or push, which
handle this properly).

-Peff

Re: [PATCH] Demonstrate failure of 'core.ignorecase = true'

[Jeff King]

On Fri, Mar 23, 2012 at 02:48:44PM -0400, Jeff King wrote:

> I think Thomas's suggestion is to piggy-back it onto an existing file
> lookup ("head" instead of "HEAD"), so you aren't doing any extra work.
> However, I'm not sure that would be sufficient. If I copy a repo from a
> case-insensitive filesystem to a case-sensitive one, what will the case
> of "HEAD" be on the new filesystem?
> 
> If the original filesystem was case-preserving, I would expect "HEAD".
> But on a true caseless filesystem, it could be either. Of course,
> current git would already blow up if the file was copied as "head",
> which makes me think this is probably a rare case. So maybe that is not
> worth worrying about.

As soon as I sent this, I had two additional thoughts:

  1. You could probably just use "HeAd", which is unlikely to work
     anywhere except on a case-insensitive filesystem, and gets around
     my objection above.

  2. This still isn't a good test, because it is checking case
     sensitivity of the repo directory, not the working tree, and
     core.ignorecase is about the latter. It's possible to have the two
     on different filesystems with different capabilities.

     Though I think the initial test in "git init" suffers from the same
     problem (it checks that "config" is accessible as "CoNfIg"), and I
     have no heard anybody complaining about that.

-Peff