* Re: [PATCH] Enable highlight executable path as a configuration option [not found] <4C96938C.5050505@cdwilson.us> @ 2010-09-20 9:10 ` Jakub Narebski 2010-09-21 7:25 ` Christopher Wilson 0 siblings, 1 reply; 4+ messages in thread From: Jakub Narebski @ 2010-09-20 9:10 UTC (permalink / raw) To: Christopher Wilson; +Cc: Junio C Hamano, Alejandro R. Sedeño, git On Mon, 20 Sep 2010, Christopher Wilson wrote: > Allow build-time/run-time configuration of the highlight executable. Defaults > to previous behavior which assumes that highlight is available on the server > PATH. However, if this is not the case, the path to the highlight executable > can be configured at build time as a configuration variable > > HIGHLIGHT_BIN = /path/to/highlight > > or at runtime by configuring GITWEB_CONFIG > > $highlight_bin = /path/to/highlight > > Signed-off-by: Christopher Wilson <cwilson@cdwilson.us> Good idea... but I am not sure about shell quoting and the problem with spaces in pathnames. See comments below. > --- > gitweb/Makefile | 4 +++- > gitweb/README | 7 ++++++- > gitweb/gitweb.perl | 6 +++++- > 3 files changed, 14 insertions(+), 3 deletions(-) > diff --git a/gitweb/README b/gitweb/README > index d481198..69f9860 100644 > --- a/gitweb/README > +++ b/gitweb/README > @@ -114,6 +114,9 @@ You can specify the following configuration variables when building GIT: > when gitweb.cgi is executed, then the file specified in the environment > variable will be loaded instead of the file specified when gitweb.cgi was > created. [Default: /etc/gitweb.conf] > + * HIGHLIGHT_BIN > + Path to the highlight executable to use. Useful if highlight is not > + installed on your webserver's PATH. [Default: highlight] I think it needs to be said that this 'highlight' executable must be the one from http://www.andre-simon.de (assumptions about parameters and output). > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl > index a85e2f6..e808485 100755 > --- a/gitweb/gitweb.perl > +++ b/gitweb/gitweb.perl [...] > @@ -3360,7 +3364,7 @@ sub run_highlighter { > close $fd > or die_error(404, "Reading blob failed"); > open $fd, quote_command(git_cmd(), "cat-file", "blob", $hash)." | ". > - "highlight --xhtml --fragment --syntax $syntax |" > + "$highlight_bin --xhtml --fragment --syntax $syntax |" I think you need + quote_command($highlight_bin)." --xhtml --fragment --syntax $syntax |" here > or die_error(500, "Couldn't open file or run syntax highlighter"); > return $fd; > } > -- > 1.7.2.3 > > -- Jakub Narebski Poland ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Enable highlight executable path as a configuration option 2010-09-20 9:10 ` [PATCH] Enable highlight executable path as a configuration option Jakub Narebski @ 2010-09-21 7:25 ` Christopher Wilson 2010-09-21 19:09 ` Jakub Narebski 0 siblings, 1 reply; 4+ messages in thread From: Christopher Wilson @ 2010-09-21 7:25 UTC (permalink / raw) To: Jakub Narebski; +Cc: Junio C Hamano, "Alejandro R. Sedeño", git On 9/20/10 2:10 AM, Jakub Narebski wrote: > On Mon, 20 Sep 2010, Christopher Wilson wrote: > >> Allow build-time/run-time configuration of the highlight executable. Defaults >> to previous behavior which assumes that highlight is available on the server >> PATH. However, if this is not the case, the path to the highlight executable >> can be configured at build time as a configuration variable >> >> HIGHLIGHT_BIN = /path/to/highlight >> >> or at runtime by configuring GITWEB_CONFIG >> >> $highlight_bin = /path/to/highlight >> >> Signed-off-by: Christopher Wilson<cwilson@cdwilson.us> > > Good idea... but I am not sure about shell quoting and the problem > with spaces in pathnames. See comments below. > >> --- >> gitweb/Makefile | 4 +++- >> gitweb/README | 7 ++++++- >> gitweb/gitweb.perl | 6 +++++- >> 3 files changed, 14 insertions(+), 3 deletions(-) > >> diff --git a/gitweb/README b/gitweb/README >> index d481198..69f9860 100644 >> --- a/gitweb/README >> +++ b/gitweb/README >> @@ -114,6 +114,9 @@ You can specify the following configuration variables when building GIT: >> when gitweb.cgi is executed, then the file specified in the environment >> variable will be loaded instead of the file specified when gitweb.cgi was >> created. [Default: /etc/gitweb.conf] >> + * HIGHLIGHT_BIN >> + Path to the highlight executable to use. Useful if highlight is not >> + installed on your webserver's PATH. [Default: highlight] > > I think it needs to be said that this 'highlight' executable must be > the one from http://www.andre-simon.de (assumptions about parameters and > output). > >> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl >> index a85e2f6..e808485 100755 >> --- a/gitweb/gitweb.perl >> +++ b/gitweb/gitweb.perl > [...] >> @@ -3360,7 +3364,7 @@ sub run_highlighter { >> close $fd >> or die_error(404, "Reading blob failed"); >> open $fd, quote_command(git_cmd(), "cat-file", "blob", $hash)." | ". >> - "highlight --xhtml --fragment --syntax $syntax |" >> + "$highlight_bin --xhtml --fragment --syntax $syntax |" > > I think you need > > + quote_command($highlight_bin)." --xhtml --fragment --syntax $syntax |" > > here > >> or die_error(500, "Couldn't open file or run syntax highlighter"); >> return $fd; >> } >> -- >> 1.7.2.3 >> >> > Jakub, thanks for the helpful feedback. I included the updated patch (see below) which includes the changes you suggested. Allow build-time/run-time configuration of the highlight executable (must be the one from http://www.andre-simon.de due to assumptions about parameters and output). Defaults to previous behavior which assumes that highlight is available on the server PATH. However, if this is not the case, the path to the highlight executable can be configured at build time as a configuration variable HIGHLIGHT_BIN = /path/to/highlight or at runtime by configuring GITWEB_CONFIG $highlight_bin = /path/to/highlight Signed-off-by: Christopher Wilson <cwilson@cdwilson.us> --- gitweb/Makefile | 4 +++- gitweb/README | 11 ++++++++++- gitweb/gitweb.perl | 9 ++++++++- 3 files changed, 21 insertions(+), 3 deletions(-) mode change 100644 => 100755 gitweb/README diff --git a/gitweb/Makefile b/gitweb/Makefile index 2fb7c2d..e32ee76 100644 --- a/gitweb/Makefile +++ b/gitweb/Makefile @@ -35,6 +35,7 @@ GITWEB_FAVICON = static/git-favicon.png GITWEB_JS = static/gitweb.js GITWEB_SITE_HEADER = GITWEB_SITE_FOOTER = +HIGHLIGHT_BIN = highlight # include user config -include ../config.mak.autogen @@ -129,7 +130,8 @@ GITWEB_REPLACE = \ -e 's|++GITWEB_FAVICON++|$(GITWEB_FAVICON)|g' \ -e 's|++GITWEB_JS++|$(GITWEB_JS)|g' \ -e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \ - -e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' + -e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \ + -e 's|++HIGHLIGHT_BIN++|$(HIGHLIGHT_BIN)|g' GITWEB-BUILD-OPTIONS: FORCE @rm -f $@+ diff --git a/gitweb/README b/gitweb/README old mode 100644 new mode 100755 index d481198..bf3664f --- a/gitweb/README +++ b/gitweb/README @@ -114,6 +114,11 @@ You can specify the following configuration variables when building GIT: when gitweb.cgi is executed, then the file specified in the environment variable will be loaded instead of the file specified when gitweb.cgi was created. [Default: /etc/gitweb.conf] + * HIGHLIGHT_BIN + Path to the highlight executable to use (must be the one from + http://www.andre-simon.de due to assumptions about parameters and output). + Useful if highlight is not installed on your webserver's PATH. + [Default: highlight] Runtime gitweb configuration @@ -236,7 +241,11 @@ not include variables usually directly set during build): If server load exceed this value then return "503 Service Unavailable" error. Server load is taken to be 0 if gitweb cannot determine its value. Set it to undefined value to turn it off. The default is 300. - + * $highlight_bin + Path to the highlight executable to use (must be the one from + http://www.andre-simon.de due to assumptions about parameters and output). + Useful if highlight is not installed on your webserver's PATH. + [Default: highlight] Projects list file format ~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index a85e2f6..e5910ce 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -165,6 +165,12 @@ our @diff_opts = ('-M'); # taken from git_commit # the gitweb domain. our $prevent_xss = 0; +# Path to the highlight executable to use (must be the one from +# http://www.andre-simon.de due to assumptions about parameters and output). +# Useful if highlight is not installed on your webserver's PATH. +# [Default: highlight] +our $highlight_bin = "++HIGHLIGHT_BIN++"; + # information about snapshot formats that gitweb is capable of serving our %known_snapshot_formats = ( # name => { @@ -3360,7 +3366,8 @@ sub run_highlighter { close $fd or die_error(404, "Reading blob failed"); open $fd, quote_command(git_cmd(), "cat-file", "blob", $hash)." | ". - "highlight --xhtml --fragment --syntax $syntax |" + quote_command($highlight_bin). + " --xhtml --fragment --syntax $syntax |" or die_error(500, "Couldn't open file or run syntax highlighter"); return $fd; } -- 1.7.2.3 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] Enable highlight executable path as a configuration option 2010-09-21 7:25 ` Christopher Wilson @ 2010-09-21 19:09 ` Jakub Narebski 0 siblings, 0 replies; 4+ messages in thread From: Jakub Narebski @ 2010-09-21 19:09 UTC (permalink / raw) To: Christopher Wilson; +Cc: Junio C Hamano, Alejandro R. Sedeño, git On Tue, 21 Sep 2010, Christopher Wilson wrote: > Jakub, thanks for the helpful feedback. I included the updated patch > (see below) which includes the changes you suggested. > -- >8 -- > Subject: [PATCH] Enable highlight executable path as a configuration option > > Allow build-time/run-time configuration of the highlight executable > (must be the one from http://www.andre-simon.de due to assumptions > about parameters and output). Defaults to previous behavior which > assumes that highlight is available on the server PATH. However, if > this is not the case, the path to the highlight executable can be > configured at build time as a configuration variable > > HIGHLIGHT_BIN = /path/to/highlight > > or at runtime by configuring GITWEB_CONFIG > > $highlight_bin = /path/to/highlight > > Signed-off-by: Christopher Wilson <cwilson@cdwilson.us> For what it is worth it: Acked-by: Jakub Narebski <jnareb@gmail.com> It would certainly be nice to have tests for $highlight_bin, but even assuming that HIGHLIGHT prerequisite is met, I don't know how such test could be written. It is not a show-stopper. -- Jakub Narebski Poland ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] Enable highlight executable path as a configuration option @ 2010-09-19 22:29 Christopher Wilson 0 siblings, 0 replies; 4+ messages in thread From: Christopher Wilson @ 2010-09-19 22:29 UTC (permalink / raw) To: git Allow build-time/run-time configuration of the highlight executable. Defaults to previous behavior which assumes that highlight is available on the server PATH. However, if this is not the case, the path to the highlight executable can be configured at build time as a configuration variable HIGHLIGHT_BIN = /path/to/highlight or at runtime by configuring GITWEB_CONFIG $highlight_bin = /path/to/highlight Signed-off-by: Christopher Wilson <cwilson@cdwilson.us> --- gitweb/Makefile | 4 +++- gitweb/README | 7 ++++++- gitweb/gitweb.perl | 6 +++++- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/gitweb/Makefile b/gitweb/Makefile index 2fb7c2d..e32ee76 100644 --- a/gitweb/Makefile +++ b/gitweb/Makefile @@ -35,6 +35,7 @@ GITWEB_FAVICON = static/git-favicon.png GITWEB_JS = static/gitweb.js GITWEB_SITE_HEADER = GITWEB_SITE_FOOTER = +HIGHLIGHT_BIN = highlight # include user config -include ../config.mak.autogen @@ -129,7 +130,8 @@ GITWEB_REPLACE = \ -e 's|++GITWEB_FAVICON++|$(GITWEB_FAVICON)|g' \ -e 's|++GITWEB_JS++|$(GITWEB_JS)|g' \ -e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \ - -e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' + -e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \ + -e 's|++HIGHLIGHT_BIN++|$(HIGHLIGHT_BIN)|g' GITWEB-BUILD-OPTIONS: FORCE @rm -f $@+ diff --git a/gitweb/README b/gitweb/README index d481198..69f9860 100644 --- a/gitweb/README +++ b/gitweb/README @@ -114,6 +114,9 @@ You can specify the following configuration variables when building GIT: when gitweb.cgi is executed, then the file specified in the environment variable will be loaded instead of the file specified when gitweb.cgi was created. [Default: /etc/gitweb.conf] + * HIGHLIGHT_BIN + Path to the highlight executable to use. Useful if highlight is not + installed on your webserver's PATH. [Default: highlight] Runtime gitweb configuration @@ -236,7 +239,9 @@ not include variables usually directly set during build): If server load exceed this value then return "503 Service Unavailable" error. Server load is taken to be 0 if gitweb cannot determine its value. Set it to undefined value to turn it off. The default is 300. - + * $highlight_bin + Path to the highlight executable to use. Useful if highlight is not + installed on your webserver's PATH. [Default: highlight] Projects list file format ~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index a85e2f6..e808485 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -165,6 +165,10 @@ our @diff_opts = ('-M'); # taken from git_commit # the gitweb domain. our $prevent_xss = 0; +# highlight executable to use +# defaults to "highlight" +our $highlight_bin = "++HIGHLIGHT_BIN++"; + # information about snapshot formats that gitweb is capable of serving our %known_snapshot_formats = ( # name => { @@ -3360,7 +3364,7 @@ sub run_highlighter { close $fd or die_error(404, "Reading blob failed"); open $fd, quote_command(git_cmd(), "cat-file", "blob", $hash)." | ". - "highlight --xhtml --fragment --syntax $syntax |" + "$highlight_bin --xhtml --fragment --syntax $syntax |" or die_error(500, "Couldn't open file or run syntax highlighter"); return $fd; } -- 1.7.2.3 ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-09-21 19:10 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <4C96938C.5050505@cdwilson.us> 2010-09-20 9:10 ` [PATCH] Enable highlight executable path as a configuration option Jakub Narebski 2010-09-21 7:25 ` Christopher Wilson 2010-09-21 19:09 ` Jakub Narebski 2010-09-19 22:29 Christopher Wilson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).