* [PATCH 0/1] windows: avoid illegal filenames during a build @ 2019-07-04 9:14 Johannes Schindelin via GitGitGadget 2019-07-04 9:14 ` [PATCH 1/1] Avoid illegal filenames when building Documentation on NTFS Johannes Schindelin via GitGitGadget 0 siblings, 1 reply; 8+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-04 9:14 UTC (permalink / raw) To: git; +Cc: Junio C Hamano 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 -- gitgitgadget ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/1] Avoid illegal filenames when building Documentation on NTFS 2019-07-04 9:14 [PATCH 0/1] windows: avoid illegal filenames during a build Johannes Schindelin via GitGitGadget @ 2019-07-04 9:14 ` Johannes Schindelin via GitGitGadget 2019-07-08 19:23 ` Junio C Hamano 0 siblings, 1 reply; 8+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-04 9:14 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Johannes Schindelin 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(-) diff --git a/Documentation/Makefile b/Documentation/Makefile index dbf5a0f276..792ddf872e 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -297,9 +297,9 @@ docdep_prereqs = \ cmd-list.made $(cmds_txt) doc.dep : $(docdep_prereqs) $(wildcard *.txt) $(wildcard config/*.txt) build-docdep.perl - $(QUIET_GEN)$(RM) $@+ $@ && \ - $(PERL_PATH) ./build-docdep.perl >$@+ $(QUIET_STDERR) && \ - mv $@+ $@ + $(QUIET_GEN)$(RM) $@.new $@ && \ + $(PERL_PATH) ./build-docdep.perl >$@.new $(QUIET_STDERR) && \ + mv $@.new $@ -include doc.dep @@ -344,8 +344,8 @@ GIT-ASCIIDOCFLAGS: FORCE fi clean: - $(RM) *.xml *.xml+ *.html *.html+ *.1 *.5 *.7 - $(RM) *.texi *.texi+ *.texi++ git.info gitman.info + $(RM) *.xml *.xml.new *.html *.html.new *.1 *.5 *.7 + $(RM) *.texi *.texi.new *.texi.new.new git.info gitman.info $(RM) *.pdf $(RM) howto-index.txt howto/*.html doc.dep $(RM) technical/*.html technical/api-index.txt @@ -355,14 +355,14 @@ clean: $(RM) GIT-ASCIIDOCFLAGS $(MAN_HTML): %.html : %.txt asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS - $(QUIET_ASCIIDOC)$(RM) $@+ $@ && \ - $(TXT_TO_HTML) -d manpage -o $@+ $< && \ - mv $@+ $@ + $(QUIET_ASCIIDOC)$(RM) $@.new $@ && \ + $(TXT_TO_HTML) -d manpage -o $@.new $< && \ + mv $@.new $@ $(OBSOLETE_HTML): %.html : %.txto asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS - $(QUIET_ASCIIDOC)$(RM) $@+ $@ && \ - $(TXT_TO_HTML) -o $@+ $< && \ - mv $@+ $@ + $(QUIET_ASCIIDOC)$(RM) $@.new $@ && \ + $(TXT_TO_HTML) -o $@.new $< && \ + mv $@.new $@ manpage-base-url.xsl: manpage-base-url.xsl.in $(QUIET_GEN)sed "s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|" $< > $@ @@ -372,14 +372,14 @@ manpage-base-url.xsl: manpage-base-url.xsl.in $(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $< %.xml : %.txt asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS - $(QUIET_ASCIIDOC)$(RM) $@+ $@ && \ - $(TXT_TO_XML) -d manpage -o $@+ $< && \ - mv $@+ $@ + $(QUIET_ASCIIDOC)$(RM) $@.new $@ && \ + $(TXT_TO_XML) -d manpage -o $@.new $< && \ + mv $@.new $@ user-manual.xml: user-manual.txt user-manual.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS - $(QUIET_ASCIIDOC)$(RM) $@+ $@ && \ - $(TXT_TO_XML) -d book -o $@+ $< && \ - mv $@+ $@ + $(QUIET_ASCIIDOC)$(RM) $@.new $@ && \ + $(TXT_TO_XML) -d book -o $@.new $< && \ + mv $@.new $@ technical/api-index.txt: technical/api-index-skel.txt \ technical/api-index.sh $(patsubst %,%.txt,$(API_DOCS)) @@ -397,46 +397,46 @@ XSLT = docbook.xsl XSLTOPTS = --xinclude --stringparam html.stylesheet docbook-xsl.css user-manual.html: user-manual.xml $(XSLT) - $(QUIET_XSLTPROC)$(RM) $@+ $@ && \ - xsltproc $(XSLTOPTS) -o $@+ $(XSLT) $< && \ - mv $@+ $@ + $(QUIET_XSLTPROC)$(RM) $@.new $@ && \ + xsltproc $(XSLTOPTS) -o $@.new $(XSLT) $< && \ + mv $@.new $@ git.info: user-manual.texi $(QUIET_MAKEINFO)$(MAKEINFO) --no-split -o $@ user-manual.texi user-manual.texi: user-manual.xml - $(QUIET_DB2TEXI)$(RM) $@+ $@ && \ - $(DOCBOOK2X_TEXI) user-manual.xml --encoding=UTF-8 --to-stdout >$@++ && \ - $(PERL_PATH) fix-texi.perl <$@++ >$@+ && \ - rm $@++ && \ - mv $@+ $@ + $(QUIET_DB2TEXI)$(RM) $@.new $@ && \ + $(DOCBOOK2X_TEXI) user-manual.xml --encoding=UTF-8 --to-stdout >$@.new.new && \ + $(PERL_PATH) fix-texi.perl <$@.new.new >$@.new && \ + rm $@.new.new && \ + mv $@.new $@ user-manual.pdf: user-manual.xml - $(QUIET_DBLATEX)$(RM) $@+ $@ && \ - $(DBLATEX) -o $@+ $(DBLATEX_COMMON) $< && \ - mv $@+ $@ + $(QUIET_DBLATEX)$(RM) $@.new $@ && \ + $(DBLATEX) -o $@.new $(DBLATEX_COMMON) $< && \ + mv $@.new $@ gitman.texi: $(MAN_XML) cat-texi.perl texi.xsl - $(QUIET_DB2TEXI)$(RM) $@+ $@ && \ - ($(foreach xml,$(sort $(MAN_XML)),xsltproc -o $(xml)+ texi.xsl $(xml) && \ - $(DOCBOOK2X_TEXI) --encoding=UTF-8 --to-stdout $(xml)+ && \ - rm $(xml)+ &&) true) > $@++ && \ - $(PERL_PATH) cat-texi.perl $@ <$@++ >$@+ && \ - rm $@++ && \ - mv $@+ $@ + $(QUIET_DB2TEXI)$(RM) $@.new $@ && \ + ($(foreach xml,$(sort $(MAN_XML)),xsltproc -o $(xml).new texi.xsl $(xml) && \ + $(DOCBOOK2X_TEXI) --encoding=UTF-8 --to-stdout $(xml).new && \ + rm $(xml).new &&) true) > $@.new.new && \ + $(PERL_PATH) cat-texi.perl $@ <$@.new.new >$@.new && \ + rm $@.new.new && \ + mv $@.new $@ gitman.info: gitman.texi $(QUIET_MAKEINFO)$(MAKEINFO) --no-split --no-validate $*.texi $(patsubst %.txt,%.texi,$(MAN_TXT)): %.texi : %.xml - $(QUIET_DB2TEXI)$(RM) $@+ $@ && \ - $(DOCBOOK2X_TEXI) --to-stdout $*.xml >$@+ && \ - mv $@+ $@ + $(QUIET_DB2TEXI)$(RM) $@.new $@ && \ + $(DOCBOOK2X_TEXI) --to-stdout $*.xml >$@.new && \ + mv $@.new $@ howto-index.txt: howto-index.sh $(wildcard howto/*.txt) - $(QUIET_GEN)$(RM) $@+ $@ && \ - '$(SHELL_PATH_SQ)' ./howto-index.sh $(sort $(wildcard howto/*.txt)) >$@+ && \ - mv $@+ $@ + $(QUIET_GEN)$(RM) $@.new $@ && \ + '$(SHELL_PATH_SQ)' ./howto-index.sh $(sort $(wildcard howto/*.txt)) >$@.new && \ + mv $@.new $@ $(patsubst %,%.html,$(ARTICLES)) : %.html : %.txt $(QUIET_ASCIIDOC)$(TXT_TO_HTML) $*.txt @@ -445,10 +445,10 @@ WEBDOC_DEST = /pub/software/scm/git/docs howto/%.html: ASCIIDOC_EXTRA += -a git-relative-html-prefix=../ $(patsubst %.txt,%.html,$(wildcard howto/*.txt)): %.html : %.txt GIT-ASCIIDOCFLAGS - $(QUIET_ASCIIDOC)$(RM) $@+ $@ && \ + $(QUIET_ASCIIDOC)$(RM) $@.new $@ && \ sed -e '1,/^$$/d' $< | \ - $(TXT_TO_HTML) - >$@+ && \ - mv $@+ $@ + $(TXT_TO_HTML) - >$@.new && \ + mv $@.new $@ install-webdoc : html '$(SHELL_PATH_SQ)' ./install-webdoc.sh $(WEBDOC_DEST) -- gitgitgadget ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] Avoid illegal filenames when building Documentation on NTFS 2019-07-04 9:14 ` [PATCH 1/1] Avoid illegal filenames when building Documentation on NTFS Johannes Schindelin via GitGitGadget @ 2019-07-08 19:23 ` Junio C Hamano 2019-07-09 13:27 ` Johannes Schindelin 0 siblings, 1 reply; 8+ messages in thread From: Junio C Hamano @ 2019-07-08 19:23 UTC (permalink / raw) To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes: > 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). 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] Avoid illegal filenames when building Documentation on NTFS 2019-07-08 19:23 ` Junio C Hamano @ 2019-07-09 13:27 ` Johannes Schindelin 2019-07-09 14:43 ` Junio C Hamano 0 siblings, 1 reply; 8+ messages in thread From: Johannes Schindelin @ 2019-07-09 13:27 UTC (permalink / raw) To: Junio C Hamano; +Cc: Johannes Schindelin via GitGitGadget, git Hi Junio, On Mon, 8 Jul 2019, Junio C Hamano wrote: > "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> > writes: > > > 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). > > 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. 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] Avoid illegal filenames when building Documentation on NTFS 2019-07-09 13:27 ` Johannes Schindelin @ 2019-07-09 14:43 ` Junio C Hamano 2019-07-10 10:08 ` Johannes Schindelin 2019-07-10 18:26 ` Junio C Hamano 0 siblings, 2 replies; 8+ messages in thread From: Junio C Hamano @ 2019-07-09 14:43 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Johannes Schindelin via GitGitGadget, git Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > 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.) 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. > In addition, your `+` scheme will break on Windows once it uses `git.exe` > or any other non-MSYS2 helper... 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] Avoid illegal filenames when building Documentation on NTFS 2019-07-09 14:43 ` Junio C Hamano @ 2019-07-10 10:08 ` Johannes Schindelin 2019-07-10 18:26 ` Junio C Hamano 1 sibling, 0 replies; 8+ messages in thread From: Johannes Schindelin @ 2019-07-10 10:08 UTC (permalink / raw) To: Junio C Hamano; +Cc: Johannes Schindelin via GitGitGadget, git Hi Junio, On Tue, 9 Jul 2019, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > In addition, your `+` scheme will break on Windows once it uses > > `git.exe` or any other non-MSYS2 helper... > > 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. 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] Avoid illegal filenames when building Documentation on NTFS 2019-07-09 14:43 ` Junio C Hamano 2019-07-10 10:08 ` Johannes Schindelin @ 2019-07-10 18:26 ` Junio C Hamano 2019-07-10 20:35 ` Johannes Schindelin 1 sibling, 1 reply; 8+ messages in thread From: Junio C Hamano @ 2019-07-10 18:26 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Johannes Schindelin via GitGitGadget, git Junio C Hamano <gitster@pobox.com> writes: >> In addition, your `+` scheme will break on Windows once it uses `git.exe` >> or any other non-MSYS2 helper... > > 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. 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] Avoid illegal filenames when building Documentation on NTFS 2019-07-10 18:26 ` Junio C Hamano @ 2019-07-10 20:35 ` Johannes Schindelin 0 siblings, 0 replies; 8+ messages in thread From: Johannes Schindelin @ 2019-07-10 20:35 UTC (permalink / raw) To: Junio C Hamano; +Cc: Johannes Schindelin via GitGitGadget, git Hi Junio, On Wed, 10 Jul 2019, Junio C Hamano wrote: > Junio C Hamano <gitster@pobox.com> writes: > > >> In addition, your `+` scheme will break on Windows once it uses `git.exe` > >> or any other non-MSYS2 helper... > > > > 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"? 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-07-10 20:35 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-07-04 9:14 [PATCH 0/1] windows: avoid illegal filenames during a build Johannes Schindelin via GitGitGadget 2019-07-04 9:14 ` [PATCH 1/1] Avoid illegal filenames when building Documentation on NTFS Johannes Schindelin via GitGitGadget 2019-07-08 19:23 ` Junio C Hamano 2019-07-09 13:27 ` Johannes Schindelin 2019-07-09 14:43 ` Junio C Hamano 2019-07-10 10:08 ` Johannes Schindelin 2019-07-10 18:26 ` Junio C Hamano 2019-07-10 20:35 ` Johannes Schindelin
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git 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 read-only IMAP folder(s) and NNTP newsgroup(s).