{"thread":{"id":"18443","subject":"[PATCH] documentation: Makefile accounts for SHELL_PATH setting","startedAt":"2009-03-21T02:40:20Z","lastAt":"2009-03-22T06:19:46Z","messageCount":4,"participants":["Ben Walton","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"108779","messageId":"1237603220-22897-1-git-send-email-bwalton@artsci.utoronto.ca","threadId":"18443","inReplyTo":null,"subject":"[PATCH] documentation: Makefile accounts for SHELL_PATH setting","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2009-03-21T02:40:20Z","receivedAt":"2009-03-21T02:40:20Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"Take SHELL_PATH into account, if set, in Documentation/Makefile.  This\nallows the caller to provide a shell capable of running the install\nscripts on systems where sh is not bash.\n\nSigned-off-by: Ben Walton <bwalton@artsci.utoronto.ca>\n---\n Documentation/Makefile |   17 +++++++++++------\n 1 files changed, 11 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex 144ec32..2eb5a72 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -67,6 +67,11 @@ ASCIIDOC_EXTRA += -a docbook-xsl-172\n MANPAGE_XSL = manpage-1.72.xsl\n endif\n \n+#retain original (but broken) behaviour if SHELL_PATH isn't overridden\n+ifndef SHELL_PATH\n+\tSHELL_PATH = sh\n+endif\n+\n #\n # Please note that there is a minor bug in asciidoc.\n # The version after 6.0.3 _will_ include the patch found here:\n@@ -116,7 +121,7 @@ install-pdf: pdf\n \t$(INSTALL) -m 644 user-manual.pdf $(DESTDIR)$(pdfdir)\n \n install-html: html\n-\tsh ./install-webdoc.sh $(DESTDIR)$(htmldir)\n+\t$(SHELL_PATH) ./install-webdoc.sh $(DESTDIR)$(htmldir)\n \n ../GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE\n \t$(MAKE) -C ../ GIT-VERSION-FILE\n@@ -178,7 +183,7 @@ user-manual.xml: user-manual.txt user-manual.conf\n \n technical/api-index.txt: technical/api-index-skel.txt \\\n \ttechnical/api-index.sh $(patsubst %,%.txt,$(API_DOCS))\n-\tcd technical && sh ./api-index.sh\n+\tcd technical && $(SHELL_PATH) ./api-index.sh\n \n $(patsubst %,%.html,$(API_DOCS) technical/api-index): %.html : %.txt\n \t$(ASCIIDOC) -b xhtml11 -f asciidoc.conf \\\n@@ -220,7 +225,7 @@ $(patsubst %.txt,%.texi,$(MAN_TXT)): %.texi : %.xml\n \n howto-index.txt: howto-index.sh $(wildcard howto/*.txt)\n \t$(RM) $@+ $@\n-\tsh ./howto-index.sh $(wildcard howto/*.txt) >$@+\n+\t$(SHELL_PATH) ./howto-index.sh $(wildcard howto/*.txt) >$@+\n \tmv $@+ $@\n \n $(patsubst %,%.html,$(ARTICLES)) : %.html : %.txt\n@@ -234,14 +239,14 @@ $(patsubst %.txt,%.html,$(wildcard howto/*.txt)): %.html : %.txt\n \tmv $@+ $@\n \n install-webdoc : html\n-\tsh ./install-webdoc.sh $(WEBDOC_DEST)\n+\t$(SHELL_PATH) ./install-webdoc.sh $(WEBDOC_DEST)\n \n quick-install: quick-install-man\n \n quick-install-man:\n-\tsh ./install-doc-quick.sh $(DOC_REF) $(DESTDIR)$(mandir)\n+\t$(SHELL_PATH) ./install-doc-quick.sh $(DOC_REF) $(DESTDIR)$(mandir)\n \n quick-install-html:\n-\tsh ./install-doc-quick.sh $(HTML_REF) $(DESTDIR)$(htmldir)\n+\t$(SHELL_PATH) ./install-doc-quick.sh $(HTML_REF) $(DESTDIR)$(htmldir)\n \n .PHONY: .FORCE-GIT-VERSION-FILE\n-- \n1.6.0.5\n"},{"id":"108781","messageId":"20090321032240.GA31547@coredump.intra.peff.net","threadId":"18443","inReplyTo":"1237603220-22897-1-git-send-email-bwalton@artsci.utoronto.ca","subject":"Re: [PATCH] documentation: Makefile accounts for SHELL_PATH setting","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-03-21T03:22:40Z","receivedAt":"2009-03-21T03:22:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 20, 2009 at 10:40:20PM -0400, Ben Walton wrote:\n\n> Take SHELL_PATH into account, if set, in Documentation/Makefile.  This\n> allows the caller to provide a shell capable of running the install\n> scripts on systems where sh is not bash.\n\nMakes sense.\n\n> +#retain original (but broken) behaviour if SHELL_PATH isn't overridden\n> +ifndef SHELL_PATH\n> +\tSHELL_PATH = sh\n> +endif\n\nThe Makefile in t/Makefile does:\n\n  SHELL_PATH ?= $(SHELL)\n\nwhich I think makes more sense (and yes, yours actually keeps the\nexisting behavior, but it's probably better to inherit from SHELL in\ncase it is set to something more useful).\n\n>  install-html: html\n> -\tsh ./install-webdoc.sh $(DESTDIR)$(htmldir)\n> +\t$(SHELL_PATH) ./install-webdoc.sh $(DESTDIR)$(htmldir)\n\nYou need a SHELL_PATH_SQ to handle paths with spaces; see t/Makefile for\nan example.\n\nI wonder if both subdirs should simply be pulling from\nGIT-BUILD-OPTIONS, though, which would allow this to use the specified\nSHELL_PATH:\n\n  $ make SHELL_PATH=whatever\n  $ cd Documentation && make\n\nbut maybe it is not worth caring about (since it may complicate building\nDocumentation if you _haven't_ build the actual code).\n\n-Peff\n"},{"id":"108791","messageId":"1237635198-sup-2111@ntdws12.chass.utoronto.ca","threadId":"18443","inReplyTo":"20090321032240.GA31547@coredump.intra.peff.net","subject":"Re: [PATCH] documentation: Makefile accounts for SHELL_PATH setting","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2009-03-21T11:38:35Z","receivedAt":"2009-03-21T11:38:35Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"Excerpts from Jeff King's message of Fri Mar 20 23:22:40 -0400 2009:\n> > +#retain original (but broken) behaviour if SHELL_PATH isn't overridden\n> > +ifndef SHELL_PATH\n> > +    SHELL_PATH = sh\n> > +endif\n> \n> The Makefile in t/Makefile does:\n> \n>   SHELL_PATH ?= $(SHELL)\n> \n> which I think makes more sense (and yes, yours actually keeps the\n> existing behavior, but it's probably better to inherit from SHELL in\n> case it is set to something more useful).\n\nI used the ifndef/endif setup becuase that's how the PERL_PATH was set\nand also becuase I think it's slightly more explicit.  I'm ok with ?=\nthough too.  I had considered using $(SHELL), but discarded it because\nit veered from the current behaviour.  I agree that $(SHELL) is likely\nbetter than sh though.\n\n> >  install-html: html\n> > -    sh ./install-webdoc.sh $(DESTDIR)$(htmldir)\n> > +    $(SHELL_PATH) ./install-webdoc.sh $(DESTDIR)$(htmldir)\n> \n> You need a SHELL_PATH_SQ to handle paths with spaces; see t/Makefile for\n> an example.\n\nOk.  I'll look at this and implement the _SQ bits too to make it safer\nand more general.\n\n> I wonder if both subdirs should simply be pulling from\n> GIT-BUILD-OPTIONS, though, which would allow this to use the specified\n> SHELL_PATH:\n> \n>   $ make SHELL_PATH=whatever\n>   $ cd Documentation && make\n> \n> but maybe it is not worth caring about (since it may complicate building\n> Documentation if you _haven't_ build the actual code).\n\nIn my case, I'm using the configure script and then running make,\nwhich sees the Documentation/Makefile source in the ../config.mak\nfiles, so there may be some variance between pure make and make +\nautoconf in this respect.  I hadn't looked at it in that light.\nShould this be reconciled too?\n\nI'm away today, but will try to correct this patch when I get home.\n\nThanks\n-Ben\n-- \nBen Walton\nSystems Programmer - CHASS\nUniversity of Toronto\nC:416.407.5610 | W:416.978.4302\n\nGPG Key Id: 8E89F6D2; Key Server: pgp.mit.edu\nContact me to arrange for a CAcert assurance meeting.\n"},{"id":"108880","messageId":"20090322061946.GC14765@coredump.intra.peff.net","threadId":"18443","inReplyTo":"1237635198-sup-2111@ntdws12.chass.utoronto.ca","subject":"Re: [PATCH] documentation: Makefile accounts for SHELL_PATH setting","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-03-22T06:19:46Z","receivedAt":"2009-03-22T06:19:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Mar 21, 2009 at 07:38:35AM -0400, Ben Walton wrote:\n\n> I used the ifndef/endif setup becuase that's how the PERL_PATH was set\n> and also becuase I think it's slightly more explicit.  I'm ok with ?=\n\nI can't think of any reason why the two would not be equivalent\nfunctionally. I would generally use ?= because it is more portable, but\nwe are inextricably bound to gmake at this point, so I don't think that\nmatters. So I don't have a strong preference.\n\n> > but maybe it is not worth caring about (since it may complicate building\n> > Documentation if you _haven't_ build the actual code).\n> \n> In my case, I'm using the configure script and then running make,\n> which sees the Documentation/Makefile source in the ../config.mak\n> files, so there may be some variance between pure make and make +\n> autoconf in this respect.  I hadn't looked at it in that light.\n> Should this be reconciled too?\n\nOh, right, I forgot that it pulls in config.mak. So it is really a\nnon-issue if you are putting SHELL_PATH in your config.mak (or defining\nit via autoconf). So nevermind my ramblings in that direction.\n\nI think it should be fine to just resend your patch with:\n\n  1. default to $(SHELL)\n\n  2. quote $(SHELL_PATH) as appropriate\n\n-Peff\n"}]}