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

Brandon Casey [off-list ref] writes:

Junio C Hamano wrote:

quoted

Brandon Casey [off-list ref] writes:

quoted

Use double quotes to protect against paths which may contain spaces.
...
+	echo '	mkdir -p "$(instdir_SQ)"' >> $@

Is this sufficient?  We seem to apply double-sq when writing shell
scriptlet in GIT-BUILD-OPTIONS from the main Makefile, and I suspect you
would need to do something similar.

It seems to be sufficient. The double quotes survived into my perl.mak file
and the two perl modules were installed correctly when I supplied a prefix
with spaces. Is there something else to be concerned about?

I think the generic way GIT-BUILD-OPTIONS writing is done covers cases
where the installation directory has funnies other than whitespace, e.g. 
double quotes.  Is your 'echo "$(instdir_SQ)"' sufficient?

Junio C Hamano wrote:

Brandon Casey [off-list ref] writes:

quoted

Junio C Hamano wrote:

quoted

Brandon Casey [off-list ref] writes:

quoted

Use double quotes to protect against paths which may contain spaces.
...
+	echo '	mkdir -p "$(instdir_SQ)"' >> $@

Is this sufficient?  We seem to apply double-sq when writing shell
scriptlet in GIT-BUILD-OPTIONS from the main Makefile, and I suspect you
would need to do something similar.

It seems to be sufficient. The double quotes survived into my perl.mak file
and the two perl modules were installed correctly when I supplied a prefix
with spaces. Is there something else to be concerned about?

I think the generic way GIT-BUILD-OPTIONS writing is done covers cases
where the installation directory has funnies other than whitespace, e.g. 
double quotes.  Is your 'echo "$(instdir_SQ)"' sufficient?

DOUBLE QUOTE ISSUE:

I added a double quote to my prefix, and the build fails at compiling config.c
line 589. The failure is caused by the macro ETC_GITCONFIG which is set in the
Makefile and contains the prefix string, which contains the single double quote.
This of course causes a syntax error. So it looks like the cleansing done to
ETC_GITCONFIG doesn't handle this.

Doing this allows me to compile:

    ETC_GITCONFIG_SQ = $(subst ",\",$(subst ','\'',$(ETC_GITCONFIG)))

The patch at the end of this email applies the same treatment to the other
variables I needed to get git to compile. If this is the correct fix, then
the other variables used as macros in git source files would need to be
hunted down... at least SHA1_HEADER_SQ, but maybe others?


SPACE ISSUE:

Also, the installation of the perl modules fails when I have a space in the
path and NO_PERL_MAKEMAKER is _not_ set. IOW the perl makemaker install fails
for me when there is a space in the path. This has nothing to do with the
double quote I was talking about above, I think it would fail with double quote
too.

The line assigning PREFIX in my perl.mak looks like:

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

Shouldn't that argument have quotes around it?

The errors look like:

make -C perl prefix='/home/casey/opt/test spaces/' DESTDIR='' install
make[1]: Entering directory `/home/casey/scratch/git/master/perl'
make[2]: Entering directory `/home/casey/scratch/git/master/perl'
Installing /home/casey/opt/test/private-Error.3pm
Installing /home/casey/opt/test/Git.3pm
Writing /home/casey/opt/test
Can't open file /home/casey/opt/test: Is a directory at /usr/lib/perl5/5.8.5/ExtUtils/Install.pm line 209
make[2]: *** [pure_site_install] Error 255
make[2]: Leaving directory `/home/casey/scratch/git/master/perl'
make[1]: *** [install] Error 2
make[1]: Leaving directory `/home/casey/scratch/git/master/perl'
make: *** [install] Error 2

private-Error.3pm and Git.3pm showed up in /home/casey/opt/test/


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

MakeMaker version 6.17 (Revision: 1.133)
perl v5.8.5

-brandon

diff --git a/Makefile b/Makefile
index 0d373f7..affc288 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))

This adds the double single quote escaping that is performed for
GIT_BUILD_OPTIONS to the paths in the install section to protect
against paths with spaces, quotes or other funny characters in them.

Signed-off-by: Brandon Casey <redacted>
---
 perl/Makefile |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/perl/Makefile b/perl/Makefile
index b8547db..4c6b2a2 100644
--- a/perl/Makefile
+++ b/perl/Makefile

@@ -29,13 +29,14 @@ $(makfile): ../GIT-CFLAGS Makefile
 	'$(PERL_PATH_SQ)' -MError -e 'exit($$Error::VERSION < 0.15009)' || \
 	echo '	cp private-Error.pm blib/lib/Error.pm' >> $@
 	echo install: >> $@
-	echo '	mkdir -p $(instdir_SQ)' >> $@
-	echo '	$(RM) $(instdir_SQ)/Git.pm; cp Git.pm $(instdir_SQ)' >> $@
-	echo '	$(RM) $(instdir_SQ)/Error.pm' >> $@
+	echo "	mkdir -p "\''$(subst ','\'',$(instdir_SQ))'\' >> $@
+	echo "	$(RM) "\''$(subst ','\'',$(instdir_SQ))/Git.pm'\' >> $@
+	echo "	cp Git.pm "\''$(subst ','\'',$(instdir_SQ))'\' >> $@
+	echo "	$(RM) "\''$(subst ','\'',$(instdir_SQ))/Error.pm'\' >> $@
 	'$(PERL_PATH_SQ)' -MError -e 'exit($$Error::VERSION < 0.15009)' || \
-	echo '	cp private-Error.pm $(instdir_SQ)/Error.pm' >> $@
+	echo "	cp private-Error.pm "\''$(subst ','\'',$(instdir_SQ))/Error.pm'\' >> $@
 	echo instlibdir: >> $@
-	echo '	echo $(instdir_SQ)' >> $@
+	echo "	echo "\''$(subst ','\'',$(instdir_SQ))'\' >> $@
 else
 $(makfile): Makefile.PL ../GIT-CFLAGS
 	$(PERL_PATH) $< PREFIX='$(prefix_SQ)'

-- 
1.6.0.rc1.89.g2e7ef.dirty