threads / patch / 25079

patch, 3 partst/gitweb-lib.sh improvements

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

## tl;dr

8 messages between Sep 12, 2010 and Sep 16, 2010. Diffs are folded; open one to read it.

replies: 7people: 3as markdown or json

Jakub Narebski· Sep 12, 2010, 09:41 UTC · lore

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
Jakub Narebski· Sep 12, 2010, 09:41 UTC · re: Jakub Narebski · lore

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

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
Show changes to t/gitweb-lib.sh +4 −4
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
Jakub Narebski· Sep 12, 2010, 09:41 UTC · re: Jakub Narebski · lore

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

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Whitespace cleanup
 t/gitweb-lib.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Show changes to t/gitweb-lib.sh +2 −2
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
Jakub Narebski· Sep 12, 2010, 09:41 UTC · re: Jakub Narebski · lore

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

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(-)
Show changes to t/gitweb-lib.sh +13 −2
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
Ævar Arnfjörð Bjarmason· Sep 12, 2010, 09:46 UTC · re: Jakub Narebski · lore

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

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.

Junio C Hamano· Sep 12, 2010, 17:37 UTC · re: Jakub Narebski · lore

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

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.
Show 7 quoted lines
> +	# 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).

Jakub Narebski· Sep 12, 2010, 19:33 UTC · re: Junio C Hamano · lore

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

On Sun, 12 Sep 2010, Junio C Hamano wrote:
Show 8 quoted lines
> 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.
Show 12 quoted lines
> > +	# 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
Jakub Narebski· Sep 16, 2010, 19:58 UTC · re: Jakub Narebski · lore

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

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:
Show 20 quoted lines
> > > +	# 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(-)
Show changes to t/gitweb-lib.sh +17 −2
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

← back to recent threads