Re: [PATCH] perl/Makefile: handle paths with spaces in the NO_PERL_MAKEMAKER section

Brandon Casey [off-list ref] writes:

There are problems here with spaces, single quotes, and double quotes.
I'll follow up in another email.

I guess we've opened up a large can of worms.  Let's have the minimum fix
that says "We do support whitespace in these paths but no other funnies"
and leave the more intrusive one for post 1.6.0, for now.

Signed-off-by: Brandon Casey <redacted>
---


Junio C Hamano wrote:

Brandon Casey [off-list ref] writes:

quoted

There are problems here with spaces, single quotes, and double quotes.
I'll follow up in another email.

I guess we've opened up a large can of worms.  Let's have the minimum fix
that says "We do support whitespace in these paths but no other funnies"
and leave the more intrusive one for post 1.6.0, for now.

That is this patch.

BUT. It produces some strangeties in the perl.mak file.

PREFIX looks great:

    PREFIX = '/home/casey/opt/test spaces/'

but then down in the $(MAKE_APERL_FILE) section we get something like:

    PREFIX=''/home/casey/opt/test spaces/''

because MakeMaker was already single quoting it.

Everything compiles and installs correctly for me.

In addition to putting a space in the prefix path, I also tried semicolon,
colon, and backslash. semicolon didn't work, but it didn't work before either.
colon and backslash worked. A backslash in the prefix path produced some
complaints when compiling some of the source files caused by the macros that
I mentioned earlier that are used by some of the files. The warning was
'unknown escape sequence ...'. The backslash was interpreted as an escape
sequence and then just dropped, so the path compiled into the object files
was wrong. I guess this doesn't happen on windows?

Summary: with this patch spaces in the prefix path work. It didn't break
         anything that I tested.

Maybe there's a MakeMaker expert in the audience?

-brandon


 perl/Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/perl/Makefile b/perl/Makefile
index b8547db..6d2a200 100644
--- a/perl/Makefile
+++ b/perl/Makefile

@@ -38,7 +38,7 @@ $(makfile): ../GIT-CFLAGS Makefile
 	echo '	echo $(instdir_SQ)' >> $@
 else
 $(makfile): Makefile.PL ../GIT-CFLAGS
-	$(PERL_PATH) $< PREFIX='$(prefix_SQ)'
+	$(PERL_PATH) $< PREFIX=\''$(prefix_SQ)'\'
 endif
 
 # this is just added comfort for calling make directly in perl dir

-- 
1.5.6.2

Junio C Hamano wrote:

Brandon Casey [off-list ref] writes:

quoted

There are problems here with spaces, single quotes, and double quotes.
I'll follow up in another email.

I guess we've opened up a large can of worms.  Let's have the minimum fix
that says "We do support whitespace in these paths but no other funnies"
and leave the more intrusive one for post 1.6.0, for now.

I think those two patches I just sent are enough.

You can apply the double-quote escaping patch I sent earlier if you want
(the one that escapes double quotes in the macros compiled in c programs).
But we'd have a problem installing the perl scripts using MakeMaker, and
we'd have a problem _running_ the perl scripts since the single double
quote causes a syntax error in the perl script. Let's wait till someone
wants to have " in their path shows up (and let them figure out how to
fix it).

-brandon

Brandon Casey wrote:

Junio C Hamano wrote:

quoted

Brandon Casey [off-list ref] writes:

quoted

There are problems here with spaces, single quotes, and double quotes.
I'll follow up in another email.

I guess we've opened up a large can of worms.  Let's have the minimum fix
that says "We do support whitespace in these paths but no other funnies"
and leave the more intrusive one for post 1.6.0, for now.

I think those two patches I just sent are enough.

You can apply the double-quote escaping patch I sent earlier if you want
(the one that escapes double quotes in the macros compiled in c programs).
But we'd have a problem installing the perl scripts using MakeMaker, and
we'd have a problem _running_ the perl scripts since the single double
quote causes a syntax error in the perl script. Let's wait till someone
wants to have " in their path shows up (and let them figure out how to
fix it).

ok, here's a patch that allows me to compile, install using MakeMaker, and
actually use the perl scripts generated by the make file when the installation
prefix contains odd characters like double quote, single quote, space or
semi-colon. The patch below includes that patch I mentioned about escaping
double quotes for c macros.

-brandon


--->8---

diff --git a/Makefile b/Makefile
index 0d373f7..21241b4 100644
--- a/Makefile
+++ b/Makefile

@@ -1031,15 +1031,15 @@ endif
 # Shell quote (do not use $(call) to accommodate ancient setups);
 
 SHA1_HEADER_SQ = $(subst ','\'',$(SHA1_HEADER))
-ETC_GITCONFIG_SQ = $(subst ','\'',$(ETC_GITCONFIG))
+ETC_GITCONFIG_SQ = $(subst ",\",$(subst ','\'',$(ETC_GITCONFIG)))
 
 DESTDIR_SQ = $(subst ','\'',$(DESTDIR))
 bindir_SQ = $(subst ','\'',$(bindir))
-mandir_SQ = $(subst ','\'',$(mandir))
-infodir_SQ = $(subst ','\'',$(infodir))
-gitexecdir_SQ = $(subst ','\'',$(gitexecdir))
-template_dir_SQ = $(subst ','\'',$(template_dir))
-htmldir_SQ = $(subst ','\'',$(htmldir))
+mandir_SQ = $(subst ",\",$(subst ','\'',$(mandir)))
+infodir_SQ = $(subst ",\",$(subst ','\'',$(infodir)))
+gitexecdir_SQ = $(subst ",\",$(subst ','\'',$(gitexecdir)))
+template_dir_SQ = $(subst ",\",$(subst ','\'',$(template_dir)))
+htmldir_SQ = $(subst ",\",$(subst ','\'',$(htmldir)))
 prefix_SQ = $(subst ','\'',$(prefix))
 
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))

@@ -1116,11 +1116,11 @@ perl/perl.mak: GIT-CFLAGS perl/Makefile perl/Makefile.PL
 
 $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl
 	$(QUIET_GEN)$(RM) $@ $@+ && \
-	INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \
+	INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir | sed -e 's/"/\\\\\\\"/g'` && \
 	sed -e '1{' \
 	    -e '	s|#!.*perl|#!$(PERL_PATH_SQ)|' \
 	    -e '	h' \
-	    -e '	s=.*=use lib (split(/:/, $$ENV{GITPERLLIB} || "@@INSTLIBDIR@@"));=' \
+	    -e '	s=.*=use lib (split(/:/, $$ENV{GITPERLLIB} || q"@@INSTLIBDIR@@"));=' \
 	    -e '	H' \
 	    -e '	x' \
 	    -e '}' \

@@ -1134,7 +1134,7 @@ gitweb/gitweb.cgi: gitweb/gitweb.perl
 	$(QUIET_GEN)$(RM) $@ $@+ && \
 	sed -e '1s|#!.*perl|#!$(PERL_PATH_SQ)|' \
 	    -e 's|++GIT_VERSION++|$(GIT_VERSION)|g' \
-	    -e 's|++GIT_BINDIR++|$(bindir)|g' \
+	    -e 's|++GIT_BINDIR++|$(subst ",\\",$(bindir_SQ))|g' \
 	    -e 's|++GITWEB_CONFIG++|$(GITWEB_CONFIG)|g' \
 	    -e 's|++GITWEB_CONFIG_SYSTEM++|$(GITWEB_CONFIG_SYSTEM)|g' \
 	    -e 's|++GITWEB_HOME_LINK_STR++|$(GITWEB_HOME_LINK_STR)|g' \

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 90cd99b..155b1ac 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl

@@ -29,7 +29,7 @@ our $my_uri = $cgi->url(-absolute => 1);
 
 # core git executable to use
 # this can just be "git" if your webserver has a sensible PATH
-our $GIT = "++GIT_BINDIR++/git";
+our $GIT = q"++GIT_BINDIR++/git";
 
 # absolute fs-path which will be prepended to the project path
 #our $projectroot = "/pub/scm";

diff --git a/perl/Makefile b/perl/Makefile
index dcbd2dd..2597d60 100644
--- a/perl/Makefile
+++ b/perl/Makefile

@@ -39,7 +39,7 @@ $(makfile): ../GIT-CFLAGS Makefile
 	echo "	echo "\''$(subst ','\'',$(instdir_SQ))'\' >> $@
 else
 $(makfile): Makefile.PL ../GIT-CFLAGS
-	$(PERL_PATH) $< PREFIX=\''$(prefix_SQ)'\'
+	$(PERL_PATH) $< PREFIX=\''$(subst ','\'',$(prefix_SQ))'\'
 endif
 
 # this is just added comfort for calling make directly in perl dir

diff --git a/perl/Makefile.PL b/perl/Makefile.PL
index 320253e..e1e465c 100644
--- a/perl/Makefile.PL
+++ b/perl/Makefile.PL

@@ -3,7 +3,7 @@ use ExtUtils::MakeMaker;
 sub MY::postamble {
 	return <<'MAKE_FRAG';
 instlibdir:
-	@echo '$(INSTALLSITELIB)'
+	@echo $(INSTALLSITELIB)
 
 MAKE_FRAG
 }

-- 
1.5.6.2