From: Jonathan Nieder <jrnieder@gmail.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 1/1] mingw: only test index entries for backslashes, not tree entries
Date: Thu, 26 Dec 2019 13:42:45 -0800 [thread overview]
Message-ID: <20191226214245.GA186931@google.com> (raw)
In-Reply-To: <nycvar.QRO.7.76.6.1912262221000.46@tvgsbejvaqbjf.bet>
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
next prev parent reply other threads:[~2019-12-26 21:42 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-26 17:42 [PATCH 0/1] Disallow writing, but not fetching commits with file names containing backslashes Johannes Schindelin via GitGitGadget
2019-12-26 17:42 ` [PATCH 1/1] mingw: only test index entries for backslashes, not tree entries Johannes Schindelin via GitGitGadget
2019-12-26 18:56 ` Junio C Hamano
2019-12-26 21:16 ` Johannes Schindelin
2019-12-30 21:57 ` Junio C Hamano
2020-01-02 19:53 ` Johannes Schindelin
2019-12-26 20:03 ` Jonathan Nieder
2019-12-26 21:23 ` Johannes Schindelin
2019-12-26 21:42 ` Jonathan Nieder [this message]
2019-12-26 22:01 ` Junio C Hamano
2019-12-26 22:25 ` Junio C Hamano
2019-12-31 22:51 ` Johannes Schindelin
2020-01-02 19:58 ` Johannes Schindelin
2020-01-04 1:57 ` Jonathan Nieder
2020-01-04 21:29 ` Johannes Schindelin
2019-12-26 19:22 ` [PATCH 0/1] Disallow writing, but not fetching commits with file names containing backslashes Junio C Hamano
2019-12-26 21:19 ` Johannes Schindelin
2019-12-31 22:53 ` [PATCH v2 " Johannes Schindelin via GitGitGadget
2019-12-31 22:53 ` [PATCH v2 1/1] mingw: only test index entries for backslashes, not tree entries Johannes Schindelin via GitGitGadget
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191226214245.GA186931@google.com \
--to=jrnieder@gmail.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.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.
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 NNTP newsgroup(s).