Skip to content

Commit e7dd3d3

Browse files
Merge PR #17697: coq_makefile: put file to install list in file instead of shell script
Reviewed-by: gares Co-authored-by: gares <[email protected]>
2 parents 3154ae5 + d5dcd0e commit e7dd3d3

File tree

2 files changed

+26
-6
lines changed

2 files changed

+26
-6
lines changed
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
- **Fixed:**
2+
`coq_makefile` avoids generating a command containing all files to install in a make rule,
3+
which could surpass the maximum single argument size in some developments
4+
(`#17697 <https://github.com/coq/coq/pull/17697>`_,
5+
fixes `#17721 <https://github.com/coq/coq/issues/17721>`_,
6+
by Gaëtan Gilbert).

tools/CoqMakefile.in

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -592,13 +592,27 @@ beautify: $(BEAUTYFILES)
592592
# There rules can be extended in @LOCAL_FILE@
593593
# Extensions can't assume when they run.
594594

595+
# We use $(file) to avoid generating a very long command string to pass to the shell
596+
# (cf https://coq.zulipchat.com/#narrow/stream/250632-Coq-Platform-devs-.26-users/topic/Strange.20command.20length.20limit.20on.20Linux)
597+
# However Apple ships old make which doesn't have $(file) so we need a fallback
598+
$(file >.hasfile,1)
599+
HASFILE:=$(shell if [ -e .hasfile ]; then echo 1; rm .hasfile; fi)
600+
601+
.filestoinstall:
602+
@:$(if $(HASFILE),$(file >$@,$(FILESTOINSTALL)),\
603+
$(shell rm -f $@) \
604+
$(foreach x,$(FILESTOINSTALL),$(shell printf '%s\n' "$x" >> $@)))
605+
606+
607+
.PHONY: .filestoinstall
608+
595609
# findlib needs the package to not be installed, so we remove it before
596610
# installing it (see the call to findlib_remove)
597-
install: META
598-
$(HIDE)code=0; for f in $(FILESTOINSTALL); do\
611+
install: META .filestoinstall
612+
$(HIDE)code=0; for f in $$(cat .filestoinstall); do\
599613
if ! [ -f "$$f" ]; then >&2 echo $$f does not exist; code=1; fi \
600614
done; exit $$code
601-
$(HIDE)for f in $(FILESTOINSTALL); do\
615+
$(HIDE)for f in $$(cat .filestoinstall); do\
602616
df="`$(COQMKFILE) -destination-of "$$f" $(COQLIBS)`";\
603617
if [ "$$?" != "0" -o -z "$$df" ]; then\
604618
echo SKIP "$$f" since it has no logical path;\
@@ -640,16 +654,16 @@ install-doc:: html mlihtml
640654
done
641655
.PHONY: install-doc
642656

643-
uninstall::
657+
uninstall:: .filestoinstall
644658
@# Extension point
645659
$(call findlib_remove)
646-
$(HIDE)for f in $(FILESTOINSTALL); do \
660+
$(HIDE)for f in $$(cat .filestoinstall); do \
647661
df="`$(COQMKFILE) -destination-of "$$f" $(COQLIBS)`" &&\
648662
instf="$(COQLIBINSTALL)/$$df/`basename $$f`" &&\
649663
rm -f "$$instf" &&\
650664
echo RM "$$instf" ;\
651665
done
652-
$(HIDE)for f in $(FILESTOINSTALL); do \
666+
$(HIDE)for f in $$(cat .filestoinstall); do \
653667
df="`$(COQMKFILE) -destination-of "$$f" $(COQLIBS)`" &&\
654668
echo RMDIR "$(COQLIBINSTALL)/$$df/" &&\
655669
(rmdir "$(COQLIBINSTALL)/$$df/" 2>/dev/null || true); \

0 commit comments

Comments
 (0)