[PATCH 0/3] t/gitweb-lib.sh improvements

[PATCH 0/3] t/gitweb-lib.sh improvements

[Jakub Narebski]

This series of patches is meant as preparation for splitting gitweb;
the most important (but only RFC) is the last patch adding support for
GITWEB_TEST_INSTALLED, similar to GIT_TEST_INSTALLED from
t/test-lib.sh

Comments appreciated.

Jakub Narebski (3):
  t/gitweb-lib.sh: Use GIT_BUILD_DIR
  t/gitweb-lib.sh: Use tabs for indent consistently
  t/gitweb-lib.sh: Add support for GITWEB_TEST_INSTALLED

 t/gitweb-lib.sh |   25 ++++++++++++++++++-------
 1 files changed, 18 insertions(+), 7 deletions(-)
 mode change 100644 => 100755 t/gitweb-lib.sh

-- 
1.7.2.1

[PATCH 1/3] t/gitweb-lib.sh: Use GIT_BUILD_DIR

[Jakub Narebski]

Use "$GIT_BUILD_DIR" instead of "$TEST_DIRECTORY/.." (both defined in
t/test-lib.sh) in t/gitweb-lib.sh.  It better describes the intent.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
I think it makes gitweb_init() and gitweb_install() more clear.

 t/gitweb-lib.sh |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
 mode change 100644 => 100755 t/gitweb-lib.sh

diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh
old mode 100644
new mode 100755
index 81ef2a0..8fad236
--- a/t/gitweb-lib.sh
+++ b/t/gitweb-lib.sh
@@ -19,9 +19,9 @@ our \$site_name = '[localhost]';
 our \$site_header = '';
 our \$site_footer = '';
 our \$home_text = 'indextext.html';
-our @stylesheets = ('file:///$TEST_DIRECTORY/../gitweb/static/gitweb.css');
-our \$logo = 'file:///$TEST_DIRECTORY/../gitweb/static/git-logo.png';
-our \$favicon = 'file:///$TEST_DIRECTORY/../gitweb/static/git-favicon.png';
+our @stylesheets = ('file:///$GIT_BUILD_DIR/gitweb/static/gitweb.css');
+our \$logo = 'file:///$GIT_BUILD_DIR/gitweb/static/git-logo.png';
+our \$favicon = 'file:///$GIT_BUILD_DIR/gitweb/static/git-favicon.png';
 our \$projects_list = '';
 our \$export_ok = '';
 our \$strict_export = '';
@@ -38,7 +38,7 @@ gitweb_run () {
 	GATEWAY_INTERFACE='CGI/1.1'
 	HTTP_ACCEPT='*/*'
 	REQUEST_METHOD='GET'
-	SCRIPT_NAME="$TEST_DIRECTORY/../gitweb/gitweb.perl"
+	SCRIPT_NAME="$GIT_BUILD_DIR/gitweb/gitweb.perl"
 	QUERY_STRING=""$1""
 	PATH_INFO=""$2""
 	export GATEWAY_INTERFACE HTTP_ACCEPT REQUEST_METHOD \
-- 
1.7.2.1

[PATCH 2/3] t/gitweb-lib.sh: Use tabs for indent consistently

[Jakub Narebski]

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Whitespace cleanup

 t/gitweb-lib.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh
index 8fad236..8c490c8 100755
--- a/t/gitweb-lib.sh
+++ b/t/gitweb-lib.sh
@@ -81,8 +81,8 @@ if ! test_have_prereq PERL; then
 fi
 
 perl -MEncode -e 'decode_utf8("", Encode::FB_CROAK)' >/dev/null 2>&1 || {
-    skip_all='skipping gitweb tests, perl version is too old'
-    test_done
+	skip_all='skipping gitweb tests, perl version is too old'
+	test_done
 }
 
 gitweb_init
-- 
1.7.2.1

[PATCH/RFC 3/3] t/gitweb-lib.sh: Add support for GITWEB_TEST_INSTALLED

[Jakub Narebski]

You can set the GITWEB_TEST_INSTALLED environment variable to the
gitwebdir (the directory where gitweb is installed / deployed to) of
an existing gitweb instalation to test that installation.

This change is intended to make it possible to test that process of
installing gitweb (and the modules it depends on) works correctly
after splitting gitweb.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---

It is an RFC because I am not sure where to put documentation,
i.e. description of GITWEB_TEST_INSTALLED.  Should it be put in
t/README, in gitweb/README, or in gitweb/INSTALL, or in
t/gitweb-lib.sh header?

 t/gitweb-lib.sh |   15 +++++++++++++--
 1 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh
index 8c490c8..b17459c 100755
--- a/t/gitweb-lib.sh
+++ b/t/gitweb-lib.sh
@@ -32,17 +32,28 @@ EOF
 	cat >.git/description <<EOF
 $0 test repository
 EOF
+
+	# You can set the GITWEB_TEST_INSTALLED environment variable to
+	# the gitwebdir (the directory where gitweb is installed / deployed to)
+	# of an existing gitweb instalation to test that installation.
+	if test -n "$GITWEB_TEST_INSTALLED" ; then
+		SCRIPT_NAME="$GITWEB_TEST_INSTALLED/gitweb.cgi"
+		test -f "$SCRIPT_NAME" ||
+		error "Cannot find gitweb.cgi at $GITWEB_TEST_INSTALLED."
+	else # normal case, use source version of gitweb
+		SCRIPT_NAME="$GIT_BUILD_DIR/gitweb/gitweb.perl"
+	fi
+	export SCRIPT_NAME
 }
 
 gitweb_run () {
 	GATEWAY_INTERFACE='CGI/1.1'
 	HTTP_ACCEPT='*/*'
 	REQUEST_METHOD='GET'
-	SCRIPT_NAME="$GIT_BUILD_DIR/gitweb/gitweb.perl"
 	QUERY_STRING=""$1""
 	PATH_INFO=""$2""
 	export GATEWAY_INTERFACE HTTP_ACCEPT REQUEST_METHOD \
-		SCRIPT_NAME QUERY_STRING PATH_INFO
+		QUERY_STRING PATH_INFO
 
 	GITWEB_CONFIG=$(pwd)/gitweb_config.perl
 	export GITWEB_CONFIG
-- 
1.7.2.1

Re: [PATCH/RFC 3/3] t/gitweb-lib.sh: Add support for GITWEB_TEST_INSTALLED

[Ævar Arnfjörð Bjarmason]

On Sun, Sep 12, 2010 at 09:41, Jakub Narebski <jnareb@gmail.com> wrote:
> It is an RFC because I am not sure where to put documentation,
> i.e. description of GITWEB_TEST_INSTALLED.  Should it be put in
> t/README, in gitweb/README, or in gitweb/INSTALL, or in
> t/gitweb-lib.sh header?

That looks like enough documentation as-is, we don't need to document
every test variable like that.

Re: [PATCH/RFC 3/3] t/gitweb-lib.sh: Add support for GITWEB_TEST_INSTALLED

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> It is an RFC because I am not sure where to put documentation,
> i.e. description of GITWEB_TEST_INSTALLED.  Should it be put in
> t/README, in gitweb/README, or in gitweb/INSTALL, or in
> t/gitweb-lib.sh header?

I think the comment in this file is fine.

> +	# You can set the GITWEB_TEST_INSTALLED environment variable to
> +	# the gitwebdir (the directory where gitweb is installed / deployed to)
> +	# of an existing gitweb instalation to test that installation.
> +	if test -n "$GITWEB_TEST_INSTALLED" ; then
> +		SCRIPT_NAME="$GITWEB_TEST_INSTALLED/gitweb.cgi"
> +		test -f "$SCRIPT_NAME" ||
> +		error "Cannot find gitweb.cgi at $GITWEB_TEST_INSTALLED."

I don't know if GIT_WEB_TEST_INSTALLED=/path/to/some/directory (naming the
directory that houses the script which must be named gitweb.cgi) is easier
to use than GIT_WEB_TEST_INSTALLED=/path/to/some/gitweb.perl (naming the
script that is allowed to be renamed).

Re: [PATCH/RFC 3/3] t/gitweb-lib.sh: Add support for GITWEB_TEST_INSTALLED

[Jakub Narebski]

On Sun, 12 Sep 2010, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > It is an RFC because I am not sure where to put documentation,
> > i.e. description of GITWEB_TEST_INSTALLED.  Should it be put in
> > t/README, in gitweb/README, or in gitweb/INSTALL, or in
> > t/gitweb-lib.sh header?
> 
> I think the comment in this file is fine.

O.K.

> > +	# You can set the GITWEB_TEST_INSTALLED environment variable to
> > +	# the gitwebdir (the directory where gitweb is installed / deployed to)
> > +	# of an existing gitweb instalation to test that installation.
> > +	if test -n "$GITWEB_TEST_INSTALLED" ; then
> > +		SCRIPT_NAME="$GITWEB_TEST_INSTALLED/gitweb.cgi"
> > +		test -f "$SCRIPT_NAME" ||
> > +		error "Cannot find gitweb.cgi at $GITWEB_TEST_INSTALLED."
> 
> I don't know if GIT_WEB_TEST_INSTALLED=/path/to/some/directory (naming the
> directory that houses the script which must be named gitweb.cgi) is easier
> to use than GIT_WEB_TEST_INSTALLED=/path/to/some/gitweb.perl (naming the
> script that is allowed to be renamed).

First, I was following what GIT_TEST_INSTALLED does (it points to bindir,
not to git binary).  Second, I had half of idea to also change paths to
static files... but they are not used anyway, at least not currently
(no check for links).

Anyway we can support both situations, slightly similarly to how
GIT_TRACE can have different kinds of values.
-- 
Jakub Narebski
Poland

[RFC/PATCHv2 3/3] t/gitweb-lib.sh: Add support for GITWEB_TEST_INSTALLED

[Jakub Narebski]

You can set the GITWEB_TEST_INSTALLED environment variable to the
gitwebdir (the directory where gitweb is installed / deployed to) of
an existing gitweb instalation to test that installation.

This change is intended to make it possible to test that process of
installing gitweb and the modules it depends on works correctly (after
splitting gitweb).

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Jakub Narebski wrote:
> On Sun, 12 Sep 2010, Junio C Hamano wrote:
> > Jakub Narebski <jnareb@gmail.com> writes:

> > > +	# You can set the GITWEB_TEST_INSTALLED environment variable to
> > > +	# the gitwebdir (the directory where gitweb is installed / deployed to)
> > > +	# of an existing gitweb instalation to test that installation.
> > > +	if test -n "$GITWEB_TEST_INSTALLED" ; then
> > > +		SCRIPT_NAME="$GITWEB_TEST_INSTALLED/gitweb.cgi"
> > > +		test -f "$SCRIPT_NAME" ||
> > > +		error "Cannot find gitweb.cgi at $GITWEB_TEST_INSTALLED."
> > 
> > I don't know if GIT_WEB_TEST_INSTALLED=/path/to/some/directory (naming the
> > directory that houses the script which must be named gitweb.cgi) is easier
> > to use than GIT_WEB_TEST_INSTALLED=/path/to/some/gitweb.perl (naming the
> > script that is allowed to be renamed).
> 
> First, I was following what GIT_TEST_INSTALLED does (it points to bindir,
> not to git binary).  Second, I had half of idea to also change paths to
> static files... but they are not used anyway, at least not currently
> (no check for links).
> 
> Anyway we can support both situations, slightly similarly to how
> GIT_TRACE can have different kinds of values.

And this implements it.

It is an RFC because it is not well tested... and I have forgot
to update comment about GITWEB_TEST_INSTALLED and the commit message.

 t/gitweb-lib.sh |   19 +++++++++++++++++--
 1 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh
index 8c490c8..b76ec4d 100755
--- a/t/gitweb-lib.sh
+++ b/t/gitweb-lib.sh
@@ -32,17 +32,32 @@ EOF
 	cat >.git/description <<EOF
 $0 test repository
 EOF
+
+	# You can set the GITWEB_TEST_INSTALLED environment variable to
+	# the gitwebdir (the directory where gitweb is installed / deployed to)
+	# of an existing gitweb instalation to test that installation.
+	if test -n "$GITWEB_TEST_INSTALLED" ; then
+		if test -d $GITWEB_TEST_INSTALLED; then
+			SCRIPT_NAME="$GITWEB_TEST_INSTALLED/gitweb.cgi"
+		else
+			SCRIPT_NAME="$GITWEB_TEST_INSTALLED"
+		fi
+		test -f "$SCRIPT_NAME" ||
+		error "Cannot find gitweb at $GITWEB_TEST_INSTALLED."
+	else # normal case, use source version of gitweb
+		SCRIPT_NAME="$GIT_BUILD_DIR/gitweb/gitweb.perl"
+	fi
+	export SCRIPT_NAME
 }
 
 gitweb_run () {
 	GATEWAY_INTERFACE='CGI/1.1'
 	HTTP_ACCEPT='*/*'
 	REQUEST_METHOD='GET'
-	SCRIPT_NAME="$GIT_BUILD_DIR/gitweb/gitweb.perl"
 	QUERY_STRING=""$1""
 	PATH_INFO=""$2""
 	export GATEWAY_INTERFACE HTTP_ACCEPT REQUEST_METHOD \
-		SCRIPT_NAME QUERY_STRING PATH_INFO
+		QUERY_STRING PATH_INFO
 
 	GITWEB_CONFIG=$(pwd)/gitweb_config.perl
 	export GITWEB_CONFIG
-- 
1.7.2.1