Hi Junio,

On Thu, 26 Dec 2019, Junio C Hamano wrote:

"Johannes Schindelin via GitGitGadget" [off-list ref]
writes:

quoted

From: Johannes Schindelin <redacted>

During a clone of a repository that contained a file with a backslash in
its name in the past, as of v2.24.1(2), Git for Windows prints errors
like this:

	error: filename in tree entry contains backslash: '\'

While the clone still succeeds, a similar error prevents the equivalent
`git fetch` operation, which is inconsistent.

Yes, inconsistent is bad and it is puzzling.  I would have expected,
if this gate on the transport layer is desirable, such a check would
be implemented as a part of transfer.fsckObjects and covered equally
by fetch and clone codepaths.  What went wrong to allow "clone" to
go through while stopping "fetch"?  Can you describe the root cause
of the difference in the log message?

My bad, I should have root-caused this better.

Turns out that this inconsistency is only in Git for Windows v2.24.1(2)
but not in current `master` of Git, so I simply struck that part from the
commit message.

quoted

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.

I do agree that rejecting these tree objects that has a slash in its
path component is probably wrong.  A GIT_WINDOWS_NATIVE box should
be able to host a bare repository on it, and users on machines that
are OK with paths that Windows may not like should be able to
interact with it, by pushing to it, fetching from it, and updating
the repository on that Windows box by going there and fetching from
elsewhere.  Rejecting these names at the object validity level means
Git on Windows would be incompatible with Git elsewhere.

And It hink the same logic apply to those names like prn, con, nul,
etc.  How are the users protected from them?  We should prevent
these names from entering the index the same way, shouldn't we?

quoted

So let's instead prevent such files to be added to the index.

... and loosen the check that (incorrectly) gets triggered from what
codepaths in "git fetch" (but not from "git clone")?

I rephrased it to:

    So let's loosen the requirements: we now leave tree entries with
    backslashes in their file names alone, but we do require any entries
    that are added to the Git index to contain no backslashes on Windows.

quoted

This addresses https://github.com/git-for-windows/git/issues/2435

Signed-off-by: Johannes Schindelin <redacted>
---
 read-cache.c               | 5 +++++
 t/t7415-submodule-names.sh | 7 ++++---
 tree-walk.c                | 6 ------
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index ad0b48c84d..737916ebd9 100644
--- a/read-cache.c
+++ b/read-cache.c

@@ -1278,6 +1278,11 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e
 	int skip_df_check = option & ADD_CACHE_SKIP_DFCHECK;
 	int new_only = option & ADD_CACHE_NEW_ONLY;

+#ifdef GIT_WINDOWS_NATIVE
+	if (protect_ntfs && strchr(ce->name, '\\'))

As I wondered above, names that must not enter the index may not be
limited to those with backslashes in them.  Perhaps you'd want a
separate helper function so that you can extend the logic more
easily, i.e.

	if (protect_ntfs && invalid_name_on_windows(ce->name))

or something like that.

I decided to perform those checks at yet another layer: when trying to
create new files. My idea was that I would want to catch even things like
`git config -f LPT1 ...` (`LPT1` is a reserved name on Windows, you cannot
create a file with that name).

Obviously, I cannot handle the backslash in the same code path, as e.g.
`git config -f C:\Users\me\.gitconfig ...` is totally valid.

Ciao,
Dscho

quoted

diff --git a/tree-walk.c b/tree-walk.c
index b3d162051f..d5a8e096a6 100644
--- a/tree-walk.c
+++ b/tree-walk.c

@@ -43,12 +43,6 @@ static int decode_tree_entry(struct tree_desc *desc, const char *buf, unsigned l
 		strbuf_addstr(err, _("empty filename in tree entry"));
 		return -1;
 	}
-#ifdef GIT_WINDOWS_NATIVE
-	if (protect_ntfs && strchr(path, '\\')) {
-		strbuf_addf(err, _("filename in tree entry contains backslash: '%s'"), path);
-		return -1;
-	}
-#endif
 	len = strlen(path) + 1;

 	/* Initialize the descriptor entry */