{"thread":{"id":"51427","subject":"[PATCH 0/1] windows: avoid illegal filenames during a build","startedAt":"2019-07-04T09:14:43Z","lastAt":"2019-07-10T20:35:56Z","messageCount":8,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"378580","messageId":"pull.216.git.gitgitgadget@gmail.com","threadId":"51427","inReplyTo":null,"subject":"[PATCH 0/1] windows: avoid illegal filenames during a build","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-07-04T09:14:40Z","receivedAt":"2019-07-04T09:14:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"The + is not allowed on NTFS filesystems. Even if the MSYS2 make/bash we use\nto build Git for Windows can work around it, it is not necessary.\n\nJohannes Schindelin (1):\n Avoid illegal filenames when building Documentation on NTFS\n\n Documentation/Makefile | 88 +++++++++++++++++++++---------------------\n 1 file changed, 44 insertions(+), 44 deletions(-)\n\n\nbase-commit: aa25c82427ae70aebf3b8f970f2afd54e9a2a8c6\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-216%2Fdscho%2Fmingw-avoid-illegal-filenames-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-216/dscho/mingw-avoid-illegal-filenames-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/216\n-- \ngitgitgadget\n"},{"id":"378581","messageId":"1f0f4fe6fbf0e7a7eea1f3d49db183c70693a895.1562231679.git.gitgitgadget@gmail.com","threadId":"51427","inReplyTo":"pull.216.git.gitgitgadget@gmail.com","subject":"[PATCH 1/1] Avoid illegal filenames when building Documentation on NTFS","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-07-04T09:14:40Z","receivedAt":"2019-07-04T09:14:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nA `+` is not a valid part of a filename with Windows file systems (it is\nreserved because the `+` operator meant file concatenation back in the\nDOS days).\n\nWhen building in Git for Windows' SDK, the `make.exe` that interprets\nour `Makefile` is an MSYS2 program, which uses the MSYS2/Cygwin trick of\nmapping such illegal characters into the private Unicode page, so as\nlong as you use programs that are aware of this trick (all MSYS2/Cygwin\nprograms are, however e.g. `git.exe` is not) things are all dandy.\n\nLet's just not use the `+` character.\n\nSigned-off-by: Johannes Schindelin \n---\n Documentation/Makefile | 88 +++++++++++++++++++++---------------------\n 1 file changed, 44 insertions(+), 44 deletions(-)\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex dbf5a0f276..792ddf872e 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -297,9 +297,9 @@ docdep_prereqs = \\\n \tcmd-list.made $(cmds_txt)\n \n doc.dep : $(docdep_prereqs) $(wildcard *.txt) $(wildcard config/*.txt) build-docdep.perl\n-\t$(QUIET_GEN)$(RM) $@+ $@ && \\\n-\t$(PERL_PATH) ./build-docdep.perl >$@+ $(QUIET_STDERR) && \\\n-\tmv $@+ $@\n+\t$(QUIET_GEN)$(RM) $@.new $@ && \\\n+\t$(PERL_PATH) ./build-docdep.perl >$@.new $(QUIET_STDERR) && \\\n+\tmv $@.new $@\n \n -include doc.dep\n \n@@ -344,8 +344,8 @@ GIT-ASCIIDOCFLAGS: FORCE\n fi\n \n clean:\n-\t$(RM) *.xml *.xml+ *.html *.html+ *.1 *.5 *.7\n-\t$(RM) *.texi *.texi+ *.texi++ git.info gitman.info\n+\t$(RM) *.xml *.xml.new *.html *.html.new *.1 *.5 *.7\n+\t$(RM) *.texi *.texi.new *.texi.new.new git.info gitman.info\n \t$(RM) *.pdf\n \t$(RM) howto-index.txt howto/*.html doc.dep\n \t$(RM) technical/*.html technical/api-index.txt\n@@ -355,14 +355,14 @@ clean:\n \t$(RM) GIT-ASCIIDOCFLAGS\n \n $(MAN_HTML): %.html : %.txt asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS\n-\t$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \\\n-\t$(TXT_TO_HTML) -d manpage -o $@+ $< && \\\n-\tmv $@+ $@\n+\t$(QUIET_ASCIIDOC)$(RM) $@.new $@ && \\\n+\t$(TXT_TO_HTML) -d manpage -o $@.new $< && \\\n+\tmv $@.new $@\n \n $(OBSOLETE_HTML): %.html : %.txto asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS\n-\t$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \\\n-\t$(TXT_TO_HTML) -o $@+ $< && \\\n-\tmv $@+ $@\n+\t$(QUIET_ASCIIDOC)$(RM) $@.new $@ && \\\n+\t$(TXT_TO_HTML) -o $@.new $< && \\\n+\tmv $@.new $@\n \n manpage-base-url.xsl: manpage-base-url.xsl.in\n \t$(QUIET_GEN)sed \"s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|\" $< > $@\n@@ -372,14 +372,14 @@ manpage-base-url.xsl: manpage-base-url.xsl.in\n \t$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<\n \n %.xml : %.txt asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS\n-\t$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \\\n-\t$(TXT_TO_XML) -d manpage -o $@+ $< && \\\n-\tmv $@+ $@\n+\t$(QUIET_ASCIIDOC)$(RM) $@.new $@ && \\\n+\t$(TXT_TO_XML) -d manpage -o $@.new $< && \\\n+\tmv $@.new $@\n \n user-manual.xml: user-manual.txt user-manual.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS\n-\t$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \\\n-\t$(TXT_TO_XML) -d book -o $@+ $< && \\\n-\tmv $@+ $@\n+\t$(QUIET_ASCIIDOC)$(RM) $@.new $@ && \\\n+\t$(TXT_TO_XML) -d book -o $@.new $< && \\\n+\tmv $@.new $@\n \n technical/api-index.txt: technical/api-index-skel.txt \\\n \ttechnical/api-index.sh $(patsubst %,%.txt,$(API_DOCS))\n@@ -397,46 +397,46 @@ XSLT = docbook.xsl\n XSLTOPTS = --xinclude --stringparam html.stylesheet docbook-xsl.css\n \n user-manual.html: user-manual.xml $(XSLT)\n-\t$(QUIET_XSLTPROC)$(RM) $@+ $@ && \\\n-\txsltproc $(XSLTOPTS) -o $@+ $(XSLT) $< && \\\n-\tmv $@+ $@\n+\t$(QUIET_XSLTPROC)$(RM) $@.new $@ && \\\n+\txsltproc $(XSLTOPTS) -o $@.new $(XSLT) $< && \\\n+\tmv $@.new $@\n \n git.info: user-manual.texi\n \t$(QUIET_MAKEINFO)$(MAKEINFO) --no-split -o $@ user-manual.texi\n \n user-manual.texi: user-manual.xml\n-\t$(QUIET_DB2TEXI)$(RM) $@+ $@ && \\\n-\t$(DOCBOOK2X_TEXI) user-manual.xml --encoding=UTF-8 --to-stdout >$@++ && \\\n-\t$(PERL_PATH) fix-texi.perl <$@++ >$@+ && \\\n-\trm $@++ && \\\n-\tmv $@+ $@\n+\t$(QUIET_DB2TEXI)$(RM) $@.new $@ && \\\n+\t$(DOCBOOK2X_TEXI) user-manual.xml --encoding=UTF-8 --to-stdout >$@.new.new && \\\n+\t$(PERL_PATH) fix-texi.perl <$@.new.new >$@.new && \\\n+\trm $@.new.new && \\\n+\tmv $@.new $@\n \n user-manual.pdf: user-manual.xml\n-\t$(QUIET_DBLATEX)$(RM) $@+ $@ && \\\n-\t$(DBLATEX) -o $@+ $(DBLATEX_COMMON) $< && \\\n-\tmv $@+ $@\n+\t$(QUIET_DBLATEX)$(RM) $@.new $@ && \\\n+\t$(DBLATEX) -o $@.new $(DBLATEX_COMMON) $< && \\\n+\tmv $@.new $@\n \n gitman.texi: $(MAN_XML) cat-texi.perl texi.xsl\n-\t$(QUIET_DB2TEXI)$(RM) $@+ $@ && \\\n-\t($(foreach xml,$(sort $(MAN_XML)),xsltproc -o $(xml)+ texi.xsl $(xml) && \\\n-\t\t$(DOCBOOK2X_TEXI) --encoding=UTF-8 --to-stdout $(xml)+ && \\\n-\t\trm $(xml)+ &&) true) > $@++ && \\\n-\t$(PERL_PATH) cat-texi.perl $@ <$@++ >$@+ && \\\n-\trm $@++ && \\\n-\tmv $@+ $@\n+\t$(QUIET_DB2TEXI)$(RM) $@.new $@ && \\\n+\t($(foreach xml,$(sort $(MAN_XML)),xsltproc -o $(xml).new texi.xsl $(xml) && \\\n+\t\t$(DOCBOOK2X_TEXI) --encoding=UTF-8 --to-stdout $(xml).new && \\\n+\t\trm $(xml).new &&) true) > $@.new.new && \\\n+\t$(PERL_PATH) cat-texi.perl $@ <$@.new.new >$@.new && \\\n+\trm $@.new.new && \\\n+\tmv $@.new $@\n \n gitman.info: gitman.texi\n \t$(QUIET_MAKEINFO)$(MAKEINFO) --no-split --no-validate $*.texi\n \n $(patsubst %.txt,%.texi,$(MAN_TXT)): %.texi : %.xml\n-\t$(QUIET_DB2TEXI)$(RM) $@+ $@ && \\\n-\t$(DOCBOOK2X_TEXI) --to-stdout $*.xml >$@+ && \\\n-\tmv $@+ $@\n+\t$(QUIET_DB2TEXI)$(RM) $@.new $@ && \\\n+\t$(DOCBOOK2X_TEXI) --to-stdout $*.xml >$@.new && \\\n+\tmv $@.new $@\n \n howto-index.txt: howto-index.sh $(wildcard howto/*.txt)\n-\t$(QUIET_GEN)$(RM) $@+ $@ && \\\n-\t'$(SHELL_PATH_SQ)' ./howto-index.sh $(sort $(wildcard howto/*.txt)) >$@+ && \\\n-\tmv $@+ $@\n+\t$(QUIET_GEN)$(RM) $@.new $@ && \\\n+\t'$(SHELL_PATH_SQ)' ./howto-index.sh $(sort $(wildcard howto/*.txt)) >$@.new && \\\n+\tmv $@.new $@\n \n $(patsubst %,%.html,$(ARTICLES)) : %.html : %.txt\n \t$(QUIET_ASCIIDOC)$(TXT_TO_HTML) $*.txt\n@@ -445,10 +445,10 @@ WEBDOC_DEST = /pub/software/scm/git/docs\n \n howto/%.html: ASCIIDOC_EXTRA += -a git-relative-html-prefix=../\n $(patsubst %.txt,%.html,$(wildcard howto/*.txt)): %.html : %.txt GIT-ASCIIDOCFLAGS\n-\t$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \\\n+\t$(QUIET_ASCIIDOC)$(RM) $@.new $@ && \\\n \tsed -e '1,/^$$/d' $< | \\\n-\t$(TXT_TO_HTML) - >$@+ && \\\n-\tmv $@+ $@\n+\t$(TXT_TO_HTML) - >$@.new && \\\n+\tmv $@.new $@\n \n install-webdoc : html\n \t'$(SHELL_PATH_SQ)' ./install-webdoc.sh $(WEBDOC_DEST)\n-- \ngitgitgadget\n"},{"id":"378700","messageId":"xmqqv9wcxre3.fsf@gitster-ct.c.googlers.com","threadId":"51427","inReplyTo":"1f0f4fe6fbf0e7a7eea1f3d49db183c70693a895.1562231679.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] Avoid illegal filenames when building Documentation on NTFS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-07-08T19:23:00Z","receivedAt":"2019-07-08T19:23:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> A `+` is not a valid part of a filename with Windows file systems (it is\n> reserved because the `+` operator meant file concatenation back in the\n> DOS days).\n\nThe title of the cover letter had \"windows\" in it, so calling '+'\nillegal was OK, but this patch does not.\n\nI'd rather not to take this patch, because \"generate in the target\nwith plus appended, make sure it succeeds, and then rename it to the\nreal target\" is quite an established pattern not limited to the\nDocumentation/ directory of this project, if your tooling has been\nsupporting it and can continue to do so (which was the impression I\ngot from the cover letter). Even the top-level .gitignore file\nknows about it, so does the top-level Makefile and it uses the same\npattern.\n\n"},{"id":"378722","messageId":"nycvar.QRO.7.76.6.1907091524460.5700@tvgsbejvaqbjf.bet","threadId":"51427","inReplyTo":"xmqqv9wcxre3.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] Avoid illegal filenames when building Documentation on NTFS","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-07-09T13:27:11Z","receivedAt":"2019-07-09T13:27:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 8 Jul 2019, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n>\n> > From: Johannes Schindelin \n> >\n> > A `+` is not a valid part of a filename with Windows file systems (it is\n> > reserved because the `+` operator meant file concatenation back in the\n> > DOS days).\n>\n> I'd rather not to take this patch, because \"generate in the target\n> with plus appended, make sure it succeeds, and then rename it to the\n> real target\" is quite an established pattern not limited to the\n> Documentation/ directory of this project, if your tooling has been\n> supporting it and can continue to do so (which was the impression I\n> got from the cover letter). Even the top-level .gitignore file\n> knows about it, so does the top-level Makefile and it uses the same\n> pattern.\n\nUsing `.lock` is actually an even more established pattern. (I used `.new`\nbecause the intention is not to lock files, but I would be prepared to\nchange the patch to that end.)\n\nIn addition, your `+` scheme will break on Windows once it uses `git.exe`\nor any other non-MSYS2 helper, so it is not future-proof, no matter how\nestablished you claim it to be.\n\nCiao,\nDscho\n"},{"id":"378723","messageId":"xmqqef2zw9op.fsf@gitster-ct.c.googlers.com","threadId":"51427","inReplyTo":"nycvar.QRO.7.76.6.1907091524460.5700@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 1/1] Avoid illegal filenames when building Documentation on NTFS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-07-09T14:43:02Z","receivedAt":"2019-07-09T14:43:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Using `.lock` is actually an even more established pattern. (I used `.new`\n> because the intention is not to lock files, but I would be prepared to\n> change the patch to that end.)\n\nWhen I said that the plus convention is established in this project\nand not limited to the Documentation subdirectory, what I meant was\nthat your patch is insufficient if your goal is to depart from the\nconvention.\n\nThe pattern \"*+\" is known bythe top-level .gitignore file (not just\nin Documentation/.gitignore) and that is because the build procedure\noutside Documentation/ also follow the convention, e.g. the toplevel\nMakefile. Futzing with only one Makefile is not enough.\n\nI think I saw \"generate in .tmp, then move to final\" pattern used in\nprojects by others. The plus-sign is a lot shorter than anything\nelse and it is cute, but if some filesystems cannot deal with it,\nchanging it to something else may be a plausible workaround, as long\nas it is done consistently and throughout the codebase.\n\n> In addition, your `+` scheme will break on Windows once it uses `git.exe`\n> or any other non-MSYS2 helper...\n\nI am not sure what you mean here. Is your git.exe disabled not to\nbe able to do this: \"git.exe add hello+kitty.txt\"? I think that is\na more grave problem, and not limited to the Makefile in the\nDocumentation/ directory.\n"},{"id":"378759","messageId":"nycvar.QRO.7.76.6.1907101202330.46@tvgsbejvaqbjf.bet","threadId":"51427","inReplyTo":"xmqqef2zw9op.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] Avoid illegal filenames when building Documentation on NTFS","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-07-10T10:08:26Z","receivedAt":"2019-07-10T10:08:49Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Tue, 9 Jul 2019, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n>\n> > In addition, your `+` scheme will break on Windows once it uses\n> > `git.exe` or any other non-MSYS2 helper...\n>\n> I am not sure what you mean here. Is your git.exe disabled not to\n> be able to do this: \"git.exe add hello+kitty.txt\"? I think that is\n> a more grave problem, and not limited to the Makefile in the\n> Documentation/ directory.\n\nA `+` character is not allowed as part of a file name on Windows.\nTherefore, it is not so much `git.exe`'s problem, as the Git build's\nproblem that it uses such an illegal character.\n\nIt _just happens_ to work at the moment, by virtue of Git for Windows'\nSDK _by happenstance_ using only MSYS2-programs to manipulate those\nfiles in question. And MSYS2 programs use a derivative of the Cygwin\nruntime which uses a dirty trick to pretend _to MSYS2 programs only_\nthat `+` (and other characters that are equally illegal in file names on\nWindows) can be used for file names: those characters are mapped into\na private page of the Unicode range.\n\nNeedless to say, only MSYS2 programs know how to deal with such file\nnames. If you ever use any program that is not based on the MSYS2/Cygwin\nruntime (such as `git.exe`), things will break.\n\nThis strikes me as a fragile setup, hence my attempt to fix it.\n\nCiao,\nDscho\n"},{"id":"378768","messageId":"xmqqef2xu4oz.fsf@gitster-ct.c.googlers.com","threadId":"51427","inReplyTo":"xmqqef2zw9op.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] Avoid illegal filenames when building Documentation on NTFS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-07-10T18:26:04Z","receivedAt":"2019-07-10T18:26:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n>> In addition, your `+` scheme will break on Windows once it uses `git.exe`\n>> or any other non-MSYS2 helper...\n>\n> I am not sure what you mean here. Is your git.exe disabled not to\n> be able to do this: \"git.exe add hello+kitty.txt\"? I think that is\n> a more grave problem, and not limited to the Makefile in the\n> Documentation/ directory.\n\nThe reason why I thought it was a more grave problem is because (1)\ngets 7648757 out of (2), which gets 2297783817 (that is, among the\n2.2 billion paths that appear in various GitHub public repositories,\na tad short of 8 million paths have '+' somewhere):\n\n (1) SELECT count(*) FROM `bigquery-public-data.github_repos.files`\n where path like '%+%';\n\n (2) SELECT count(*) FROM `bigquery-public-data.github_repos.files`;\n\nIf you drop the technique MSYS2 borrowed from Cygwin to support\nthese characters NTFS does not like by mapping them to a private\npage, wouldn't it mean that these projects cannot be used with Git\nfor Windows? Are we going to bug each of these projects to change\ntheir pathnames?\n\nIf that is not the case, then that's fine.\n\nFWIW, I am OK with moving away from '+' in *our* project; that is\neasy enough to do, and such a change does not have to make the end\nresult visually less pleasing, as long as we pick a good substitute\nsuffix. Is '~' (or ',') allowed on NTFS, for example? The point of\n'+' being short, sweet, and easy to spot, I'd rather not to use\n'.' if we can avoid it.\n\nWhatever suffix we choose, we need to be aware that the pattern is\nnot limited to Documentation/ directory, so at least the top level\n.gitignore and Makefile in various subdirectories need vetting.\n\n"},{"id":"378778","messageId":"nycvar.QRO.7.76.6.1907102137060.46@tvgsbejvaqbjf.bet","threadId":"51427","inReplyTo":"xmqqef2xu4oz.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] Avoid illegal filenames when building Documentation on NTFS","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-07-10T20:35:34Z","receivedAt":"2019-07-10T20:35:56Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 10 Jul 2019, Junio C Hamano wrote:\n\n> Junio C Hamano writes:\n>\n> >> In addition, your `+` scheme will break on Windows once it uses `git.exe`\n> >> or any other non-MSYS2 helper...\n> >\n> > I am not sure what you mean here. Is your git.exe disabled not to\n> > be able to do this: \"git.exe add hello+kitty.txt\"?\n\nIt's not that. The `+` is not allowed on Windows in general.\n\nMind you, I just read up on the details (see e.g.\nhttps://en.wikipedia.org/wiki/Filename#Comparison_of_filename_limitations)\nand it looks as if NTFS actually _allows_ this. Even VFAT allows it, but\nFAT32 does not.\n\nSo I guess I retract my objections, as there are probably smarter ideas\nthan building Git for Windows on a FAT32 drive.\n\nFor aesthetical reasons, I still do not like the convention to append a\n`+` when appending `.new` would make things clearer, but this is not a\nhill I am prepared to die on.\n\nCiao,\nDscho\n"}]}