[PATCH] gitweb: more support for PATH_INFO based URLs

[PATCH] gitweb: more support for PATH_INFO based URLs

[Martin Waitz]

Now three types of path based URLs are supported:
	gitweb.cgi/project.git
	gitweb.cgi/project.git/branch
	gitweb.cgi/project.git/branch/filename

The first one (show project summary) was already supported for a long time
now.  The other two are new: they show the shortlog of a branch or
the plain file contents of some file contained in the repository.

This is especially useful to support project web pages for small
projects: just create an html branch and then use an URL like
gitweb.cgi/project.git/html/index.html.

Signed-off-by: Martin Waitz <tali@admingilde.org>
---
 gitweb/gitweb.perl |   34 +++++++++++++++++++++++++++-------
 1 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 9324d71..2789657 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -171,12 +171,7 @@ if (defined $action) {
 	}
 }
 
-our $project = ($cgi->param('p') || $ENV{'PATH_INFO'});
-if (defined $project) {
-	$project =~ s|^/||;
-	$project =~ s|/$||;
-	$project = undef unless $project;
-}
+our $project = $cgi->param('p');
 if (defined $project) {
 	if (!validate_input($project)) {
 		die_error(undef, "Invalid project parameter");
@@ -187,7 +182,6 @@ if (defined $project) {
 	if (!(-e "$projectroot/$project/HEAD")) {
 		die_error(undef, "No such project");
 	}
-	$git_dir = "$projectroot/$project";
 }
 
 our $file_name = $cgi->param('f');
@@ -247,6 +241,32 @@ if (defined $searchtext) {
 	$searchtext = quotemeta $searchtext;
 }
 
+# now read PATH_INFO and use it as alternative to parameters
+our $path_info = $ENV{"PATH_INFO"};
+$path_info =~ s|^/||;
+$path_info =~ s|/$||;
+if (validate_input($path_info) && !defined $project) {
+	$project = $path_info;
+	while ($project && !-e "$projectroot/$project/HEAD") {
+		$project =~ s,/*[^/]*$,,;
+	}
+	if (defined $project) {
+		$project = undef unless $project;
+	}
+	if ($path_info =~ m,^$project/([^/]+)/(.+)$,) {
+		# we got "project.git/branch/filename"
+		$action    ||= "blob_plain";
+		$hash_base ||= $1;
+		$file_name ||= $2;
+	} elsif ($path_info =~ m,^$project/([^/]+)$,) {
+		# we got "project.git/branch"
+		$action ||= "shortlog";
+		$hash   ||= $1;
+	}
+}
+
+$git_dir = "$projectroot/$project";
+
 # dispatch
 my %actions = (
 	"blame" => \&git_blame2,
-- 
1.4.2.gb8b6b

-- 
Martin Waitz

Re: [PATCH] gitweb: more support for PATH_INFO based URLs

[Jakub Narebski]

Martin Waitz wrote:

> Now three types of path based URLs are supported:
>         gitweb.cgi/project.git
>         gitweb.cgi/project.git/branch
>         gitweb.cgi/project.git/branch/filename
> 
> The first one (show project summary) was already supported for a long time
> now.  The other two are new: they show the shortlog of a branch or
> the plain file contents of some file contained in the repository.
> 
> This is especially useful to support project web pages for small
> projects: just create an html branch and then use an URL like
> gitweb.cgi/project.git/html/index.html.

Very nice.

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

(if it matters)
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

[PATCH] gitweb: fix warnings from dd70235f5a81e (PATH_INFO)

[Matthias Lederhofer]

And removed if in if where one if is enough.
---
Martin Waitz <tali@admingilde.org> wrote:
> +	if (defined $project) {
> +		$project = undef unless $project;
> +	}
This is an if in an if, one if is enough.

> +	if ($path_info =~ m,^$project/([^/]+)/(.+)$,) {
Warning if $project is undef.
> +	} elsif ($path_info =~ m,^$project/([^/]+)$,) {
The same.
> +$git_dir = "$projectroot/$project";
The same.

We really need a gitweb test target.

Something else I noted:
> +	while ($project && !-e "$projectroot/$project/HEAD") {
Evaluating $project boolean value leads to problems if a repository is
named "0" (I dunno if there are other strings than "" and "0" which
evaluate to false in perl).  There are multiple places where this is
used so I did not change it in this patch (even added one more).
Should this be changed?
---
 gitweb/gitweb.perl |   10 ++++------
 1 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 346c15c..4d39525 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -281,22 +281,20 @@ if (validate_input($path_info) && !defin
 	while ($project && !-e "$projectroot/$project/HEAD") {
 		$project =~ s,/*[^/]*$,,;
 	}
-	if (defined $project) {
-		$project = undef unless $project;
-	}
-	if ($path_info =~ m,^$project/([^/]+)/(.+)$,) {
+	$project = undef if !$project;
+	if ($project && $path_info =~ m,^$project/([^/]+)/(.+)$,) {
 		# we got "project.git/branch/filename"
 		$action    ||= "blob_plain";
 		$hash_base ||= $1;
 		$file_name ||= $2;
-	} elsif ($path_info =~ m,^$project/([^/]+)$,) {
+	} elsif ($project && $path_info =~ m,^$project/([^/]+)$,) {
 		# we got "project.git/branch"
 		$action ||= "shortlog";
 		$hash   ||= $1;
 	}
 }
 
-$git_dir = "$projectroot/$project";
+$git_dir = "$projectroot/$project" if $project;
 
 # dispatch
 my %actions = (
-- 
1.4.2.1.ge767

Re: [PATCH] gitweb: more support for PATH_INFO based URLs

[Jakub Narebski]

Martin Waitz wrote:

> Now three types of path based URLs are supported:
>       gitweb.cgi/project.git
>       gitweb.cgi/project.git/branch
>       gitweb.cgi/project.git/branch/filename
> 
> The first one (show project summary) was already supported for a long time
> now.  The other two are new: they show the shortlog of a branch or
> the plain file contents of some file contained in the repository.

> +     if ($path_info =~ m,^$project/([^/]+)/(.+)$,) {
> +             # we got "project.git/branch/filename"
> +             $action    ||= "blob_plain";
> +             $hash_base ||= $1;
> +             $file_name ||= $2;
> +     } elsif ($path_info =~ m,^$project/([^/]+)$,) {
> +             # we got "project.git/branch"
> +             $action ||= "shortlog";
> +             $hash   ||= $1;
> +     }

I'm sorry, but I realized that I didn't think and check this patch through.

First, this patch spews a bunch of warnings: when PATH_INFO is empty, when
we undefine $project etc. The patches by me and by matled try to address
and remove those warnings, but I'm sure we missed some.

Second, the whole concept of third type of path (path_info) based URL is
flawed: branches can also be hierarchical (for example Junio uses
<initals>/<topic> topic branches, although they are not published).
Therefore it is much harder to distinguish where branchname ends and
filename begins. The patch assumes that branches are flat. So for example
for branch with the name like "gitweb/xmms2" the types 2 and 3 wouldn't
work; and type 1 worked before this patch.

Therefore I rescind my Ack.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH] gitweb: more support for PATH_INFO based URLs

[Jakub Narebski]

matled (Matthias Lederhofer) on #git proposed to use ':' as a separator
between branch and filename (as branch doesn't need to be flat, 
e.g. "jc/diff" like branch name), because valid branch name cannot contain
':' (and this limit is only for branch name).

He also said that filename doesn't need to be necessary file (which would be
then present in "blob_plain" view), but it can be also a directory (which
then would be present in "tree" view). We can either check type using
git-cat-file -t via git_get_type subroutine, or assume that if we want for
directory to be shown, it should end with "/".

Let's assume for simplicity that empty branch name equals to HEAD branch,
and empty filename equals to top (root) directory of project.

So we would have the following types of path based URLs:

* project              overview (summary) page of project
* project/branch       shortlog of branch
* project/branch:file  file in branch, blob_plain view
* project/branch:dir/  directory listing of dir in branch, tree view

Possible shortcuts:
* project/branch:      directory listing of branch, main tree view
* project/:file        file in HEAD (raw)
* project/:dir/        directory listing of dir in HEAD
* project/:            directory listing of project's HEAD
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH] gitweb: fix warnings from dd70235f5a81e (PATH_INFO)

[Junio C Hamano]

Matthias Lederhofer <matled@gmx.net> writes:

> We really need a gitweb test target.

Yes, really.  Any takers who cares truly about gitweb?

> Something else I noted:
>> +	while ($project && !-e "$projectroot/$project/HEAD") {
> Evaluating $project boolean value leads to problems if a repository is
> named "0" (I dunno if there are other strings than "" and "0" which
> evaluate to false in perl).  There are multiple places where this is
> used so I did not change it in this patch (even added one more).
> Should this be changed?

Yes, there are tons of places that says if ($foo) and/or while
($bar) and fail miserably when $foo or $bar _can_ be "0".  For
project names "0" is probably not something people would want to
use but there is no inherent reason to forbid it.

How many places, like this, that we should say "defined()"
instead in the current code I wonder?

> -	} elsif ($path_info =~ m,^$project/([^/]+)$,) {
> +	} elsif ($project && $path_info =~ m,^$project/([^/]+)$,) {

Re: [PATCH] gitweb: more support for PATH_INFO based URLs

[Martin Waitz]

[-- Attachment #1: Type: text/plain, Size: 1465 bytes --]

hoi :)

On Sun, Sep 17, 2006 at 04:20:23PM +0200, Jakub Narebski wrote:
> matled (Matthias Lederhofer) on #git proposed to use ':' as a separator
> between branch and filename (as branch doesn't need to be flat, 
> e.g. "jc/diff" like branch name), because valid branch name cannot contain
> ':' (and this limit is only for branch name).

you are right, my patch doesn't work with hierarchical branch names.
However using ":" alone does not work eighter.
My main motivation for this patch was to be able to export .html files
and to have working links between them.
However a <a href="main.html"> link inside "branch:index.html" would
try to get "main.html" and not "branch:main.html".

Perhaps use ":/" as separator?
Or try to add more words to the branch name until we get a valid one.
But I don't know how this plays with Linus' latest ref-pack work
where both "a" and "a/b" are valid branch names (If I understood it
correctly).  Perhaps use a similiar algorith as the one I used
to get the project path?

> He also said that filename doesn't need to be necessary file (which would be
> then present in "blob_plain" view), but it can be also a directory (which
> then would be present in "tree" view). We can either check type using
> git-cat-file -t via git_get_type subroutine, or assume that if we want for
> directory to be shown, it should end with "/".

yes, this was something I wanted to do later, too.

-- 
Martin Waitz

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] gitweb: more support for PATH_INFO based URLs

[Jakub Narebski]

Martin Waitz wrote:

> On Sun, Sep 17, 2006 at 04:20:23PM +0200, Jakub Narebski wrote:
>> matled (Matthias Lederhofer) on #git proposed to use ':' as a separator
>> between branch and filename (as branch doesn't need to be flat, 
>> e.g. "jc/diff" like branch name), because valid branch name cannot contain
>> ':' (and this limit is only for branch name).
> 
> you are right, my patch doesn't work with hierarchical branch names.
> However using ":" alone does not work eighter.
> My main motivation for this patch was to be able to export .html files
> and to have working links between them.
> However a <a href="main.html"> link inside "branch:index.html" would
> try to get "main.html" and not "branch:main.html".
> 
> Perhaps use ":/" as separator?

That would be _very_ easy to add. Just strip leading "/" from pathname,
and we can have

        path/to/project.git/hierarchical/branch:/path/to/filename

By the way, besides hierarchical branches, we might need this if
the repository (project) has the branch (head) and tag with the same name,
and we want to select one or the other.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git