threads / patch / 5579

patchgitweb: more support for PATH_INFO based URLs

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

## tl;dr

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

replies: 7people: 4as markdown or json

Martin Waitz· Sep 16, 2006, 21:08 UTC · lore
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(-)
Show changes to gitweb/gitweb.perl +27 −7
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
Jakub Narebski· Sep 16, 2006, 21:46 UTC · re: Martin Waitz · lore

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

Martin Waitz wrote:
Show 12 quoted lines
> 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
Matthias Lederhofer· Sep 17, 2006, 12:14 UTC · re: Martin Waitz · lore

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

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(-)
Show changes to gitweb/gitweb.perl +4 −6
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
Junio C Hamano· Sep 17, 2006, 21:34 UTC · re: Matthias Lederhofer · lore

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

Matthias Lederhofer <matled@gmx.net> writes:
> We really need a gitweb test target.
Yes, really.  Any takers who cares truly about gitweb?
Show 7 quoted lines
> 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/([^/]+)$,) {
Jakub Narebski· Sep 17, 2006, 13:18 UTC · re: Martin Waitz · lore

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

Martin Waitz wrote:
Show 8 quoted lines
> 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.
Show 10 quoted lines
> +     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
Jakub Narebski· Sep 17, 2006, 14:20 UTC · re: Jakub Narebski · lore

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

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
Martin Waitz· Sep 19, 2006, 08:19 UTC · re: Jakub Narebski · lore

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

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?

Show 5 quoted lines
> 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
Jakub Narebski· Sep 19, 2006, 09:49 UTC · re: Martin Waitz · lore

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

Martin Waitz wrote:
Show 14 quoted lines
> 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

← back to recent threads