Fix cygwin install issues

PATCH In review Jonas Fonseca 3 participants 9 messages Jun 15, 2016 Kbuild

lore

Share & tools

lore

Jonas Fonseca

author

2016-06-15 22:42 UTC #0 lore raw

PATCH 1/1 Fix cygwin install issues

Support installing to paths including spaces.
Remove any old .exe files so ln will succeed.

Signed-off-by Jonas Fonseca <[email protected]>

---

Makefile +6 -6

index ac384c7..9ae0dfd 100644
@@ -397,13 +397,13 @@ check:
 ### Installation rules
 
 install: $(PROGRAMS) $(SCRIPTS)
-	$(INSTALL) -d -m755 $(DESTDIR)$(bindir)
-	$(INSTALL) $(PROGRAMS) $(SCRIPTS) $(DESTDIR)$(bindir)
-	$(INSTALL) git-revert $(DESTDIR)$(bindir)/git-cherry-pick
-	sh ./cmd-rename.sh $(DESTDIR)$(bindir)
+	$(INSTALL) -d -m755 "$(DESTDIR)$(bindir)"
+	$(INSTALL) $(PROGRAMS) $(SCRIPTS) "$(DESTDIR)$(bindir)"
+	$(INSTALL) git-revert "$(DESTDIR)$(bindir)/git-cherry-pick"
+	sh ./cmd-rename.sh "$(DESTDIR)$(bindir)" "$X"
 	$(MAKE) -C templates install
-	$(INSTALL) -d -m755 $(DESTDIR)$(GIT_PYTHON_DIR)
-	$(INSTALL) $(PYMODULES) $(DESTDIR)$(GIT_PYTHON_DIR)
+	$(INSTALL) -d -m755 "$(DESTDIR)$(GIT_PYTHON_DIR)"
+	$(INSTALL) $(PYMODULES) "$(DESTDIR)$(GIT_PYTHON_DIR)"
 
 install-doc:
 	$(MAKE) -C Documentation install

cmd-rename.sh +2 -1

index 34e7f49..c9d3171 100755
@@ -1,9 +1,10 @@
 #!/bin/sh
 d="$1"
+X="$2"
 test -d "$d" || exit
 while read old new
 do
-	rm -f "$d/$old"
+	rm -f "$d/$old" "$d/$old$X"
 	if test -f "$d/$new"
 	then
 		ln -s "$new" "$d/$old" || exit

Signature · 1 line

Jonas Fonseca

Jonas Fonseca

author

2016-06-15 22:42 UTC #1 lore raw

Jonas Fonseca <[email protected]> wrote Mon, Oct 10, 2005:

Quoted 2 lines

Support installing to paths including spaces.
Remove any old .exe files so ln will succeed.

I see that there are problems in the git porcelain commands that needs to also be fixed so please ignore this for now.

Instead, is this something that should be supported? (It is quite absurd, although it is required for 'make install' to work out of the box on some systems).

Signature · 1 line

Jonas Fonseca

Junio C Hamano

2016-06-15 22:42 UTC #2 lore raw

In reply to Jonas Fonseca

Quoted 3 lines · Jonas Fonseca

Support installing to paths including spaces.
Remove any old .exe files so ln will succeed.

This is not a complaint but I am just wondering if:

Makefile check: · line 400

Quoted 2 lines

Makefile +1 -1

-	$(INSTALL) -d -m755 $(DESTDIR)$(bindir)
+	$(INSTALL) -d -m755 "$(DESTDIR)$(bindir)"

this is the right way to quote things. I suspect it might be the responsibility of the user to quote them if she chooses to set bindir or DESTDIR to a funky value, like this:

    $ make bindir="'My Documents\Programs'"

Because depending on how funky the values of bindir and DESTDIR are, we cannot say double-quote you are giving them is even the right quoting (think double-quote itself as part of the name).

The other "$X" change to cmd-renames is a good change (I thought I heard HPA talking about that; maybe he sent one to me and I dropped it on the floor by mistake, I dunno). Thanks.

Jonas Fonseca

author

2016-06-15 22:42 UTC #3 lore raw

PATCH 1/1 git.sh: quote all paths

This makes it handle spaces in paths.

Signed-off-by Jonas Fonseca <[email protected]>

---

Junio C Hamano <[email protected]> wrote Mon, Oct 10, 2005:

Quoted 15 lines

Quoted 3 lines · Jonas Fonseca
Support installing to paths including spaces.
Remove any old .exe files so ln will succeed.
This is not a complaint but I am just wondering if:
Review
Quoted 2 lines
(quoted diff) +1 -1
-	$(INSTALL) -d -m755 $(DESTDIR)$(bindir)
+	$(INSTALL) -d -m755 "$(DESTDIR)$(bindir)"
this is the right way to quote things. I suspect it might be the responsibility of the user to quote them if she chooses to set bindir or DESTDIR to a funky value, like this:

Quoted 5 lines

    $ make bindir="'My Documents\Programs'"
Because depending on how funky the values of bindir and DESTDIR are, we cannot say double-quote you are giving them is even the right quoting (think double-quote itself as part of the name).

Yeah, ok, it could end up very wrong. I hope that this patch is acceptable. I've tested it lightly, and the core commands seems to work, there might be other git scripts that needs to be changed.

Quoted 3 lines

The other "$X" change to cmd-renames is a good change (I thought I heard HPA talking about that; maybe he sent one to me and I dropped it on the floor by mistake, I dunno). Thanks.

I also sent a 'disguised' patch in <[email protected]> ...

---

git.sh +3 -3

index 7400c16..19f89d8 100755
@@ -1,7 +1,7 @@
 #!/bin/sh
 
 cmd=
-path=$(dirname $0)
+path=$(dirname "$0")
 case "$#" in
 0)	;;
 *)	cmd="$1"

@@ -12,13 +12,13 @@ case "$#" in
 		exit 0 ;;
 	esac
 	
-	test -x $path/git-$cmd && exec $path/git-$cmd "$@"
+	test -x "$path/git-$cmd" && exec "$path/git-$cmd" "$@"
 	
 	case '@@X@@' in
 	    '')
 		;;
 	    *)
-		test -x $path/git-$cmd@@X@@ && exec $path/git-$cmd@@X@@ "$@"
+		test -x "$path/git-$cmd.exe" && exec "$path/git-$cmd.exe" "$@"
 		;;
 	esac
 	;;

Signature · 1 line

Jonas Fonseca

Jonas Fonseca

author

2016-06-15 22:42 UTC #4 lore raw

PATCH 1/1 git.sh: quote all paths

This makes it handle spaces in paths.

Signed-off-by Jonas Fonseca <[email protected]>

---

Sorry I am so sloppy: s/.exe/@@X@@/ ...

git.sh +3 -3

index 7400c16..b424055 100755
@@ -1,7 +1,7 @@
 #!/bin/sh
 
 cmd=
-path=$(dirname $0)
+path=$(dirname "$0")
 case "$#" in
 0)	;;
 *)	cmd="$1"

@@ -12,13 +12,13 @@ case "$#" in
 		exit 0 ;;
 	esac
 	
-	test -x $path/git-$cmd && exec $path/git-$cmd "$@"
+	test -x "$path/git-$cmd" && exec "$path/git-$cmd" "$@"
 	
 	case '@@X@@' in
 	    '')
 		;;
 	    *)
-		test -x $path/git-$cmd@@X@@ && exec $path/git-$cmd@@X@@ "$@"
+		test -x "$path/git-$cmd.exe" && exec "$path/git-$cmd@@X@@" "$@"
 		;;
 	esac
 	;;

Signature · 1 line

Jonas Fonseca

H. Peter Anvin

2016-06-15 22:42 UTC #5 lore raw

In reply to Junio C Hamano

Quoted 10 lines · Junio C Hamano

Quoted 3 lines · Jonas Fonseca
Support installing to paths including spaces.
Remove any old .exe files so ln will succeed.
This is not a complaint but I am just wondering if:

Quoted 8 lines

Makefile check: · line 400
Quoted 2 lines
Makefile +1 -1
-	$(INSTALL) -d -m755 $(DESTDIR)$(bindir)
+	$(INSTALL) -d -m755 "$(DESTDIR)$(bindir)"
this is the right way to quote things. I suspect it might be the responsibility of the user to quote them if she chooses to set bindir or DESTDIR to a funky value, like this:
    $ make bindir="'My Documents\Programs'"

I don't think that's the right approach.

	$(INSTALL) -d -m755 '$(DESTDIR)$(bindir)'

... at least handles everything except embedded single quotes. Note that the single quote is not a special character for make.

Quoted 7 lines

Because depending on how funky the values of bindir and DESTDIR are, we cannot say double-quote you are giving them is even the right quoting (think double-quote itself as part of the name).
The other "$X" change to cmd-renames is a good change (I thought I heard HPA talking about that; maybe he sent one to me and I dropped it on the floor by mistake, I dunno). Thanks.

If I dropped it on the floor, it was mentally :-/

-hpa

Junio C Hamano

2016-06-15 22:42 UTC #6 lore raw

Quoted 10 lines · H. Peter Anvin

Quoted 4 lines
... I suspect it might be
the responsibility of the user to quote them if she chooses to set bindir or DESTDIR to a funky value, like this:
    $ make bindir="'My Documents\Programs'"
I don't think that's the right approach.
	$(INSTALL) -d -m755 '$(DESTDIR)$(bindir)'
... at least handles everything except embedded single quotes.

OK. It is better than dq, and the user can still work it around like this if she really wanted to:

	make bindir="Anna'\\''s Home/bin"

I wish we had $(shellquote $(DESTDIR)$(bindir)) in make ;-).

H. Peter Anvin

2016-06-15 22:42 UTC #7 lore raw

Quoted 7 lines · Junio C Hamano

OK. It is better than dq, and the user can still work it around like this if she really wanted to:
	make bindir="Anna'\\''s Home/bin"
I wish we had $(shellquote $(DESTDIR)$(bindir)) in make ;-).

Hmm... let's think about this for a second...

shellquote = '$(subst ','\'',$(1))'

$(call shellquote,$(whatever))

... seems to work just fine.

(No need to worry about ! since Make commands are always /bin/sh.)

-hpa

Junio C Hamano

2016-06-15 22:42 UTC #8 lore raw

Deal with $(bindir) and friends with whitespaces.

... using HPA's shellquote macro.

Signed-off-by Junio C Hamano <[email protected]>

---

I only tested that this passes "Anna's Home" test:

	make	HOME="/tmp/Anna's Home" \
		PERL_PATH="Anna's Home/Program Directory/perl"  \
		PYTHON_PATH="Anna's Home/Program Directory/python" \
		test install

 Makefile               |   34 ++++++++++++++++++++--------------
 git-merge-recursive.py |    2 +-
 t/Makefile             |    8 +++++++-
 templates/Makefile     |   10 ++++++++--
 4 files changed, 36 insertions(+), 18 deletions(-)

applies-to: a35b27b04d18022ea4731941ea2a1bedce998092 8f77be8002b9e254ccb5cc3ed3ad1b9240289f7c

Makefile +20 ~~-14~~

index ac384c7..f7eee47 100644
@@ -163,6 +163,12 @@ LIB_OBJS = \
 LIBS = $(LIB_FILE)
 LIBS += -lz
 
+# Shell quote;
+# Result of this needs to be placed inside ''
+shq = $(subst ','\'',$(1))
+# This has surrounding ''
+shellquote = '$(call shq,$(1))'
+
 #
 # Platform specific tweaks
 #

@@ -235,7 +241,7 @@ ifndef NO_OPENSSL
 		OPENSSL_LINK =
 	endif
 else
-	DEFINES += '-DNO_OPENSSL'
+	DEFINES += -DNO_OPENSSL
 	MOZILLA_SHA1 = 1
 	OPENSSL_LIBSSL =
 endif

@@ -294,7 +300,7 @@ endif
 endif
 endif
 
-DEFINES += '-DSHA1_HEADER=$(SHA1_HEADER)'
+DEFINES += -DSHA1_HEADER=$(call shellquote,$(SHA1_HEADER))
 
 SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH)) \
 	  $(patsubst %.perl,%,$(SCRIPT_PERL)) \

@@ -311,7 +317,7 @@ all:
 
 git: git.sh Makefile
 	rm -f $@+ $@
-	sed -e '1s|#!.*/sh|#!$(SHELL_PATH)|' \
+	sed -e '1s|#!.*/sh|#!$(call shq,$(SHELL_PATH))|' \
 	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
 	    -e 's/@@X@@/$(X)/g' \
 	    $(GIT_LIST_TWEAK) <[email protected] >$@+

@@ -320,22 +326,22 @@ git: git.sh Makefile
 
 $(filter-out git,$(patsubst %.sh,%,$(SCRIPT_SH))) : % : %.sh
 	rm -f $@
-	sed -e '1s|#!.*/sh|#!$(SHELL_PATH)|' \
+	sed -e '1s|#!.*/sh|#!$(call shq,$(SHELL_PATH))|' \
 	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
 	    [email protected] >$@
 	chmod +x $@
 
 $(patsubst %.perl,%,$(SCRIPT_PERL)) : % : %.perl
 	rm -f $@
-	sed -e '1s|#!.*perl|#!$(PERL_PATH)|' \
+	sed -e '1s|#!.*perl|#!$(call shq,$(PERL_PATH))|' \
 	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
 	    [email protected] >$@
 	chmod +x $@
 
 $(patsubst %.py,%,$(SCRIPT_PYTHON)) : % : %.py
 	rm -f $@
-	sed -e '1s|#!.*python|#!$(PYTHON_PATH)|' \
-	    -e 's|@@GIT_PYTHON_PATH@@|$(GIT_PYTHON_DIR)|g' \
+	sed -e '1s|#!.*python|#!$(call shq,$(PYTHON_PATH))|' \
+	    -e 's|@@GIT_PYTHON_PATH@@|$(call shq,$(GIT_PYTHON_DIR))|g' \
 	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
 	    [email protected] >$@
 	chmod +x $@

@@ -365,7 +371,7 @@ git-rev-list$X: LIBS += $(OPENSSL_LIBSSL
 
 init-db.o: init-db.c
 	$(CC) -c $(ALL_CFLAGS) \
-		-DDEFAULT_GIT_TEMPLATE_DIR='"$(template_dir)"' $*.c
+		-DDEFAULT_GIT_TEMPLATE_DIR=$(call shellquote,"$(template_dir)") $*.c
 
 $(LIB_OBJS): $(LIB_H)
 $(patsubst git-%$X,%.o,$(PROGRAMS)): $(LIB_H)

@@ -397,13 +403,13 @@ check:
 ### Installation rules
 
 install: $(PROGRAMS) $(SCRIPTS)
-	$(INSTALL) -d -m755 $(DESTDIR)$(bindir)
-	$(INSTALL) $(PROGRAMS) $(SCRIPTS) $(DESTDIR)$(bindir)
-	$(INSTALL) git-revert $(DESTDIR)$(bindir)/git-cherry-pick
-	sh ./cmd-rename.sh $(DESTDIR)$(bindir)
+	$(INSTALL) -d -m755 $(call shellquote,$(DESTDIR)$(bindir))
+	$(INSTALL) $(PROGRAMS) $(SCRIPTS) $(call shellquote,$(DESTDIR)$(bindir))
+	$(INSTALL) git-revert $(call shellquote,$(DESTDIR)$(bindir)/git-cherry-pick)
+	sh ./cmd-rename.sh $(call shellquote,$(DESTDIR)$(bindir))
 	$(MAKE) -C templates install
-	$(INSTALL) -d -m755 $(DESTDIR)$(GIT_PYTHON_DIR)
-	$(INSTALL) $(PYMODULES) $(DESTDIR)$(GIT_PYTHON_DIR)
+	$(INSTALL) -d -m755 $(call shellquote,$(DESTDIR)$(GIT_PYTHON_DIR))
+	$(INSTALL) $(PYMODULES) $(call shellquote,$(DESTDIR)$(GIT_PYTHON_DIR))
 
 install-doc:
 	$(MAKE) -C Documentation install

git-merge-recursive.py +1 -1

index b80a860..626d854 100755
@@ -4,7 +4,7 @@ import sys, math, random, os, re, signal
 from heapq import heappush, heappop
 from sets import Set
 
-sys.path.append('@@GIT_PYTHON_PATH@@')
+sys.path.append('''@@GIT_PYTHON_PATH@@''')
 from gitMergeCommon import *
 
 originalIndexFile = os.environ.get('GIT_INDEX_FILE',

t/Makefile +7 -1

index e71da77..5c76aff 100644
@@ -7,10 +7,16 @@
 SHELL_PATH ?= $(SHELL)
 TAR ?= $(TAR)
 
+# Shell quote;
+# Result of this needs to be placed inside ''
+shq = $(subst ','\'',$(1))
+# This has surrounding ''
+shellquote = '$(call shq,$(1))'
+
 T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)
 
 all:
-	@$(foreach t,$T,echo "*** $t ***"; $(SHELL_PATH) $t $(GIT_TEST_OPTS) || exit; )
+	@$(foreach t,$T,echo "*** $t ***"; $(call shellquote,$(SHELL_PATH)) $t $(GIT_TEST_OPTS) || exit; )
 	@rm -fr trash
 
 clean:

templates/Makefile +8 -2

index c23aee8..07e928e 100644
@@ -6,6 +6,12 @@ prefix ?= $(HOME)
 template_dir ?= $(prefix)/share/git-core/templates/
 # DESTDIR=
 
+# Shell quote;
+# Result of this needs to be placed inside ''
+shq = $(subst ','\'',$(1))
+# This has surrounding ''
+shellquote = '$(call shq,$(1))'
+
 all: boilerplates.made custom
 	find blt

@@ -38,6 +44,6 @@ clean:
 	rm -rf blt boilerplates.made
 
 install: all
-	$(INSTALL) -d -m755 $(DESTDIR)$(template_dir)
+	$(INSTALL) -d -m755 $(call shellquote,$(DESTDIR)$(template_dir))
 	(cd blt && $(TAR) cf - .) | \
-	(cd $(DESTDIR)$(template_dir) && $(TAR) xf -)
+	(cd $(call shellquote,$(DESTDIR)$(template_dir)) && $(TAR) xf -)

---
0.99.8.GIT