Re: [PATCH] More flexible URL patterns for gitweb

[Jakub Narebski <jnareb@gmail.com>]

Damien Tournoud <damien@tournoud.net> writes:

> @git_base_url_list hardcodes URLs in the form "$git_base_url/$project",
> which makes impossible to generate proper URLs for SSH access scenarios.

As I understand it, current way of creating project's URLs from
@git_base_url_list and $project (and project name), via joining
with '/' as separator in the form of "$git_base_url/$project"
does not work for a class of scp-like relative "URLs".  You can't
create 'git@git.example.com:path/to/project' URL using current form
of @git_base_url_list support.

Current form of @git_base_url_list support works well both for
absolute scp-like URLs for SSH sccess, for example
'git@git.example.com:/srv/git' + 'path/to/project', and also for
"ssh://" URLs like 'ssh://git.example.com/srv/git' or 
'ssh://git@git.example.com/~' + 'path/to/project'.

> 
> This patch implements a more flexible replacement scheme, using a
> simple placeholder for the project path, and fixes the associated
> documentation.

NAK, at least in this form, for it breaks backwards compatibility.
We do not want to have upgrading gitweb break one's existing gitweb
configuration.  But see below for proposed amendment of this patch.
 
> Signed-off-by: Damien Tournoud <damien@tournoud.net>
> ---
>  gitweb/README      |   13 ++++++-------
>  gitweb/gitweb.perl |    2 +-
>  2 files changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/gitweb/README b/gitweb/README
> index ad6a04c..f16729c 100644
> --- a/gitweb/README
> +++ b/gitweb/README
> @@ -197,13 +197,12 @@ not include variables usually directly set during build):
>     full description is available as 'title' attribute (usually shown on
>     mouseover).  By default set to 25, which might be too small if you
>     use long project descriptions.
> - * @git_base_url_list
> -   List of git base URLs used for URL to where fetch project from, shown
> -   in project summary page.  Full URL is "$git_base_url/$project".
> -   You can setup multiple base URLs (for example one for  git:// protocol
> -   access, and one for http:// "dumb" protocol access).  Note that per
> -   repository configuration in 'cloneurl' file, or as values of gitweb.url
> -   project config.
> + * @git_base_url_patterns
> +   List of git base URLs patterns used to build the URLs displayed in the
> +   project summary page. Examples include "git://git.example.com/%project",
> +   "http://git.example.com/%project" and "git@git.example.com:%project".
> +   Note that per repository configuration in a 'cloneurl' file, or configuration
> +   values of gitweb.url in the project config take precedence over this.

Here it looks like you would be using @git_base_url_patterns for the
new mechanism, while below one can see that you re-use @git_base_url_list...
and break backwards compatibility with existing gitweb configuration using
@git_base_url_list.

Also, you use %project as placeholder... while %project is valid path part
of URLs (although unlikely one).

>   * $default_blob_plain_mimetype
>     Default mimetype for blob_plain (raw) view, if mimetype checking
>     doesn't result in some other type; by default 'text/plain'.
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index c356e95..0fc5957 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -4904,7 +4904,7 @@ sub git_summary {
>  	# or make project git URL from git base URL and project name
>  	my $url_tag = "URL";
>  	my @url_list = git_get_project_url_list($project);
> -	@url_list = map { "$_/$project" } @git_base_url_list unless @url_list;
> +	@url_list = map { my $url = $_; $url =~ s/%project/$project/g; $url } @git_base_url_list unless @url_list;
>  	foreach my $git_url (@url_list) {
>  		next unless $git_url;
>  		print "<tr class=\"metadata_url\"><td>$url_tag</td><td>$git_url</td></tr>\n";

To not break backward compatibility, wouldn't it be better to check if
elements of @git_base_url_list end with ':' or '/', and join base with
project path depending on this condition, i.e.:

+	@url_list = map { m/[/:]$/ ? "$_$project" : "$_/$project" } @git_base_url_list
+               unless @url_list;

This means: if base ends with colon ':' or slash '/', concatenate base
and project path, otherwise join them using '/' as field separator.
-- 
Jakub Narebski
Poland
ShadeHawk on #git