Re: [PATCH] perl/Makefile: handle paths with spaces in the NO_PERL_MAKEMAKER section
Show 18 quoted lines
> Brandon Casey <casey@nrlssc.navy.mil> writes:
>
>> Junio C Hamano wrote:
>>> Brandon Casey <casey@nrlssc.navy.mil> writes:
>>>
>>>> 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
Show changes to Makefile +6 −6
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))
Brandon Casey