patchwindows: avoid illegal filenames during a build

The + is not allowed on NTFS filesystems. Even if the MSYS2 make/bash we use to build Git for Windows can work around it, it is not necessary.

Johannes Schindelin (1):
  Avoid illegal filenames when building Documentation on NTFS

 Documentation/Makefile | 88 +++++++++++++++++++++---------------------
 1 file changed, 44 insertions(+), 44 deletions(-)

base-commit: aa25c82427ae70aebf3b8f970f2afd54e9a2a8c6
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-216%2Fdscho%2Fmingw-avoid-illegal-filenames-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-216/dscho/mingw-avoid-illegal-filenames-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/216

From: Johannes Schindelin <johannes.schindelin@gmx.de>

A `+` is not a valid part of a filename with Windows file systems (it is reserved because the `+` operator meant file concatenation back in the DOS days).

When building in Git for Windows' SDK, the `make.exe` that interprets our `Makefile` is an MSYS2 program, which uses the MSYS2/Cygwin trick of mapping such illegal characters into the private Unicode page, so as long as you use programs that are aware of this trick (all MSYS2/Cygwin programs are, however e.g. `git.exe` is not) things are all dandy.

Let's just not use the `+` character.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/Makefile | 88 +++++++++++++++++++++---------------------
 1 file changed, 44 insertions(+), 44 deletions(-)

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:

The title of the cover letter had "windows" in it, so calling '+' illegal was OK, but this patch does not.

I'd rather not to take this patch, because "generate in the target with plus appended, make sure it succeeds, and then rename it to the real target" is quite an established pattern not limited to the Documentation/ directory of this project, if your tooling has been supporting it and can continue to do so (which was the impression I got from the cover letter). Even the top-level .gitignore file knows about it, so does the top-level Makefile and it uses the same pattern.

Hi Junio,

On Mon, 8 Jul 2019, Junio C Hamano wrote:

Using `.lock` is actually an even more established pattern. (I used `.new` because the intention is not to lock files, but I would be prepared to change the patch to that end.)

In addition, your `+` scheme will break on Windows once it uses `git.exe` or any other non-MSYS2 helper, so it is not future-proof, no matter how established you claim it to be.

Ciao, Dscho

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

When I said that the plus convention is established in this project and not limited to the Documentation subdirectory, what I meant was that your patch is insufficient if your goal is to depart from the convention.

The pattern "*+" is known bythe top-level .gitignore file (not just in Documentation/.gitignore) and that is because the build procedure outside Documentation/ also follow the convention, e.g. the toplevel Makefile. Futzing with only one Makefile is not enough.

I think I saw "generate in .tmp, then move to final" pattern used in projects by others. The plus-sign is a lot shorter than anything else and it is cute, but if some filesystems cannot deal with it, changing it to something else may be a plausible workaround, as long as it is done consistently and throughout the codebase.

I am not sure what you mean here. Is your git.exe disabled not to be able to do this: "git.exe add hello+kitty.txt"? I think that is a more grave problem, and not limited to the Makefile in the Documentation/ directory.

Hi Junio,

On Tue, 9 Jul 2019, Junio C Hamano wrote:

A `+` character is not allowed as part of a file name on Windows. Therefore, it is not so much `git.exe`'s problem, as the Git build's problem that it uses such an illegal character.

It _just happens_ to work at the moment, by virtue of Git for Windows' SDK _by happenstance_ using only MSYS2-programs to manipulate those files in question. And MSYS2 programs use a derivative of the Cygwin runtime which uses a dirty trick to pretend _to MSYS2 programs only_ that `+` (and other characters that are equally illegal in file names on Windows) can be used for file names: those characters are mapped into a private page of the Unicode range.

Needless to say, only MSYS2 programs know how to deal with such file names. If you ever use any program that is not based on the MSYS2/Cygwin runtime (such as `git.exe`), things will break.

This strikes me as a fragile setup, hence my attempt to fix it.

Ciao, Dscho

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

The reason why I thought it was a more grave problem is because (1) gets 7648757 out of (2), which gets 2297783817 (that is, among the 2.2 billion paths that appear in various GitHub public repositories, a tad short of 8 million paths have '+' somewhere):

    (1) SELECT count(*) FROM `bigquery-public-data.github_repos.files`
        where path like '%+%';

    (2) SELECT count(*) FROM `bigquery-public-data.github_repos.files`;

If you drop the technique MSYS2 borrowed from Cygwin to support these characters NTFS does not like by mapping them to a private page, wouldn't it mean that these projects cannot be used with Git for Windows? Are we going to bug each of these projects to change their pathnames?

If that is not the case, then that's fine.

FWIW, I am OK with moving away from '+' in *our* project; that is easy enough to do, and such a change does not have to make the end result visually less pleasing, as long as we pick a good substitute suffix. Is '~' (or ',') allowed on NTFS, for example? The point of '+' being short, sweet, and easy to spot, I'd rather not to use '.<anyword>' if we can avoid it.

Whatever suffix we choose, we need to be aware that the pattern is not limited to Documentation/ directory, so at least the top level .gitignore and Makefile in various subdirectories need vetting.

Hi Junio,

On Wed, 10 Jul 2019, Junio C Hamano wrote:

It's not that. The `+` is not allowed on Windows in general.

Mind you, I just read up on the details (see e.g. https://en.wikipedia.org/wiki/Filename#Comparison_of_filename_limitations) and it looks as if NTFS actually _allows_ this. Even VFAT allows it, but FAT32 does not.

So I guess I retract my objections, as there are probably smarter ideas than building Git for Windows on a FAT32 drive.

For aesthetical reasons, I still do not like the convention to append a `+` when appending `.new` would make things clearer, but this is not a hill I am prepared to die on.

Ciao, Dscho