Subject: Re: [PATCH 1/1] mingw: only test index entries for backslashes, not tree entries

From: Jonathan Nieder <[email-masked]>
Date: Thu, 26 Dec 2019 13:42:45 -0800

Hi,

Johannes Schindelin wrote:
> On Thu, 26 Dec 2019, Jonathan Nieder wrote:
>> Johannes Schindelin wrote:

>>> Arguably, this is the wrong layer for that error, anyway: As long as the
>>> user never checks out the files whose names contain backslashes, there
>>> should not be any problem in the first place.
[...]
>>          between the lines of this commit messages I sense that there
>> *are* repositories in the wild using these kinds of filenames.
>>
>> Can you say more about that?  What repositories are affected?  Do they
>> contain such filenames at HEAD or only in their history?  If someone
>> wants to check out a revision with such filenames, what should happen?
>
> Yes: https://github.com/zephyrproject-rtos/civetweb contains history where
> at some stage, by mistake, a directory was called `\`. It has been fixed a
> long time ago, but users obviously still want to be able to clone it ;-)

Thanks.

What should and does happen if someone tries to check out an offending
revision?

[...]
> I added this paragraph to the commit message:
>
>     Note: just as before, the check is guarded by `core.protectNTFS` (to
>     allow overriding the check by toggling that config setting), and it
>     is _only_ performed on Windows, as the backslash is not a directory
>     separator elsewhere, even when writing to NTFS-formatted volumes.
>
> Does that clarify the issue enough?

It helps: that tells me why the check is only performed on Windows.

Since this only affects Windows, please only take this review as data
about how someone read the patch.  The patch doesn't make non Windows
specific code any *less* maintainable, and as a general presumption I
assume that the Git for Windows maintainer knows best about what is
needed for maintainability of Windows-specific code.

But the commit message and docs still don't describe what the desired
behavior is.  For example, should I be able to check out a revision
with a backslash in it (e.g. via Git skipping the offending entry), or
is the intended behavior for it to error out and prevent checking out
such versions of code?

Is there anything we can or should do to prevent people checking in
new examples of paths with backslash in them (on all platforms)?

Thanks,
Jonathan