All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] gitweb: readme fixed regarding per user project root repository
@ 2010-03-22 22:15 Sylvain Rabot
  2010-03-22 22:15 ` Sylvain Rabot
  0 siblings, 1 reply; 7+ messages in thread
From: Sylvain Rabot @ 2010-03-22 22:15 UTC (permalink / raw)
  To: git

This is a small modification of the patch I sent (Message-ID: <1267488297-10415-1-git-send-email-sylvain@abstraction.fr>) not yet applied, as far as I have seen.

It includes a fix for a rewrite rule wich was working but not useable, an update to make another one better and some typos fixes.

Regards.

--
Sylvain

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] gitweb: readme fixed regarding per user project root repository
  2010-03-22 22:15 [PATCH] gitweb: readme fixed regarding per user project root repository Sylvain Rabot
@ 2010-03-22 22:15 ` Sylvain Rabot
  2010-03-26 18:52   ` Junio C Hamano
  0 siblings, 1 reply; 7+ messages in thread
From: Sylvain Rabot @ 2010-03-22 22:15 UTC (permalink / raw)
  To: git; +Cc: Sylvain Rabot

+ the RewriteRule '/+<user>' is not working as the '+' character is
  replaced by a space in urls when you click on links. it is replaced by '/u/<user>'
+ the RewriteRule '/user/<user>' updated to allow
  '/user/<user>', '/user/<user>/' and '/user/<user>/gitweb.cgi'
+ some typos fixed

Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
---
 gitweb/README |   22 +++++++++++-----------
 1 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/gitweb/README b/gitweb/README
index ad6a04c..bc90f4d 100644
--- a/gitweb/README
+++ b/gitweb/README
@@ -347,18 +347,18 @@ something like the following in your gitweb.conf (or gitweb_config.perl) file:
   $home_link = "/";
 
 
-Webserver configuration with multiple projects' root
-----------------------------------------------------
+Webserver configuration with multiple project roots
+---------------------------------------------------
 
-If you want to use gitweb with several project roots you can edit your apache
-virtual host and gitweb.conf configuration files like this :
+If you want to use gitweb with several project roots then you can edit your
+apache virtual host and gitweb.conf configuration files like this :
 
 virtual host configuration :
 
 <VirtualHost *:80>
-    ServerName			git.example.org
-    DocumentRoot		/pub/git
-    SetEnv				GITWEB_CONFIG	/etc/gitweb.conf
+    ServerName		git.example.org
+    DocumentRoot	/pub/git
+    SetEnv		GITWEB_CONFIG	/etc/gitweb.conf
 
     # turning on mod rewrite
     RewriteEngine on
@@ -368,13 +368,13 @@ virtual host configuration :
 
     # look for a public_git folder in unix users' home
     # http://git.example.org/~<user>/
-    RewriteRule ^/\~([^\/]+)(/|/gitweb.cgi)?$	/cgi-bin/gitweb.cgi [QSA,E=GITWEB_PROJECTROOT:/home/$1/public_git/,L,PT]
+    RewriteRule ^/\~([^\/]+)(/|/gitweb.cgi)?$		/cgi-bin/gitweb.cgi [QSA,E=GITWEB_PROJECTROOT:/home/$1/public_git/,L,PT]
 
-    # http://git.example.org/+<user>/
-    #RewriteRule ^/\+([^\/]+)(/|/gitweb.cgi)?$	/cgi-bin/gitweb.cgi [QSA,E=GITWEB_PROJECTROOT:/home/$1/public_git/,L,PT]
+    # http://git.example.org/u/<user>/
+    #RewriteRule ^/\+([^\/]+)(/|/gitweb.cgi)?$		/cgi-bin/gitweb.cgi [QSA,E=GITWEB_PROJECTROOT:/home/$1/public_git/,L,PT]
 
     # http://git.example.org/user/<user>/
-    #RewriteRule ^/user/([^\/]+)/(gitweb.cgi)?$	/cgi-bin/gitweb.cgi [QSA,E=GITWEB_PROJECTROOT:/home/$1/public_git/,L,PT]
+    #RewriteRule ^/user/([^\/]+)(/|/gitweb.cgi)?$	/cgi-bin/gitweb.cgi [QSA,E=GITWEB_PROJECTROOT:/home/$1/public_git/,L,PT]
 
     # defined list of project roots
     RewriteRule ^/scm(/|/gitweb.cgi)?$		/cgi-bin/gitweb.cgi [QSA,E=GITWEB_PROJECTROOT:/pub/scm/,L,PT]
-- 
1.7.0.3

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] gitweb: readme fixed regarding per user project root repository
  2010-03-22 22:15 ` Sylvain Rabot
@ 2010-03-26 18:52   ` Junio C Hamano
  2010-03-29 20:43     ` Sylvain Rabot
  2010-03-29 22:34     ` [PATCH 0/2] gitweb: updates regaring pêr user project root directory Sylvain Rabot
  0 siblings, 2 replies; 7+ messages in thread
From: Junio C Hamano @ 2010-03-26 18:52 UTC (permalink / raw)
  To: Sylvain Rabot; +Cc: git

I was waiting for gitweb people to respond, but nobody seems to be
interested so here is my take on it.

Sylvain Rabot <sylvain@abstraction.fr> writes:

> + the RewriteRule '/+<user>' is not working as the '+' character is
>   replaced by a space in urls when you click on links. it is replaced by '/u/<user>'

I think the _only_ value of having this example, in addition to the next
one that uses "http://host/user/<me>" notation, was to demonstrate that
you do not necessarily have the actual user name and the magic token (be
it "user" or "u") that introduces the per-user hierarchy as separate path
components delimited with a slash.  Changing "+<me>" to "u/<me>" removes
that only additional value from this example.

Anybody moderately intelligent would be able to guess "u/<me>" if she
finds "user/<me>" too long to her taste, so I would suggest updating the
example to allow "http://host/+<user>/" but spell the rewrite rule in such
a way that actually does work.  An alternative is to just remove it.

By the way, does mod-rewrite configuration allow "~<me>" (home-directory
expansion) when setting the environment?  You currently do:

    E=GITWEB_PROJECTROOT:/home/$1/public_git/

but if we somehow could write it like

    E=GITWEB_PROJECTROOT:~$1/public_git/

it would be more generally useful, no?

> + the RewriteRule '/user/<user>' updated to allow
>   '/user/<user>', '/user/<user>/' and '/user/<user>/gitweb.cgi'

Please describe what you added relative to the original, not just what the
final result looks like.  "updated to allow A B C" doesn't tell the reader
"it used to redirect only A and C to gitweb request, but B wasn't
rewritten.", which seems to be the case if I am reading your regexp
correctly.  Describing why it is better to also rewrite B would be a good
idea, too, if it is not obvious.

> + some typos fixed
>
> Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>

> diff --git a/gitweb/README b/gitweb/README
> index ad6a04c..bc90f4d 100644
> --- a/gitweb/README
> +++ b/gitweb/README
> @@ -347,18 +347,18 @@ something like the following in your gitweb.conf (or gitweb_config.perl) file:
>    $home_link = "/";
>  
>  
> -Webserver configuration with multiple projects' root
> -----------------------------------------------------
> +Webserver configuration with multiple project roots
> +---------------------------------------------------

Ok.

> -If you want to use gitweb with several project roots you can edit your apache
> -virtual host and gitweb.conf configuration files like this :
> +If you want to use gitweb with several project roots then you can edit your
> +apache virtual host and gitweb.conf configuration files like this :

Ok (you might want to remove SP before colon, though).

>  virtual host configuration :
>  
>  <VirtualHost *:80>
> -    ServerName			git.example.org
> -    DocumentRoot		/pub/git
> -    SetEnv				GITWEB_CONFIG	/etc/gitweb.conf
> +    ServerName		git.example.org
> +    DocumentRoot	/pub/git
> +    SetEnv		GITWEB_CONFIG	/etc/gitweb.conf

What is this reindentation for?  "Just cosmetic" is an acceptable answer
as long as the change resulted in cosmetic improvement, but it doesn't
seem to be cosmetic improvement, either.

Thanks.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] gitweb: readme fixed regarding per user project root repository
  2010-03-26 18:52   ` Junio C Hamano
@ 2010-03-29 20:43     ` Sylvain Rabot
  2010-03-29 22:34     ` [PATCH 0/2] gitweb: updates regaring pêr user project root directory Sylvain Rabot
  1 sibling, 0 replies; 7+ messages in thread
From: Sylvain Rabot @ 2010-03-29 20:43 UTC (permalink / raw)
  To: git

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

On Fri, 2010-03-26 at 11:52 -0700, Junio C Hamano wrote: 
> I was waiting for gitweb people to respond, but nobody seems to be
> interested so here is my take on it.
> 
> Sylvain Rabot <sylvain@abstraction.fr> writes:
> 
> > + the RewriteRule '/+<user>' is not working as the '+' character is
> >   replaced by a space in urls when you click on links. it is replaced by '/u/<user>'
> 
> I think the _only_ value of having this example, in addition to the next
> one that uses "http://host/user/<me>" notation, was to demonstrate that
> you do not necessarily have the actual user name and the magic token (be
> it "user" or "u") that introduces the per-user hierarchy as separate path
> components delimited with a slash.  Changing "+<me>" to "u/<me>" removes
> that only additional value from this example.
> 
> Anybody moderately intelligent would be able to guess "u/<me>" if she
> finds "user/<me>" too long to her taste, so I would suggest updating the
> example to allow "http://host/+<user>/" but spell the rewrite rule in such
> a way that actually does work.  An alternative is to just remove it.
> 

The problem is http://host/+user works but then, when you click on a
link you will be redirected to :

"http://host/ user?p=git/git.git;a=tree" 
-------------^

I will try to look into gitweb.perl to see if the url encoding can be
updated smoothly without breaking anything to accept the '+' otherwise I
think removing this example would be the right decision like you
suggested.

> By the way, does mod-rewrite configuration allow "~<me>" (home-directory
> expansion) when setting the environment?  You currently do:
> 
>     E=GITWEB_PROJECTROOT:/home/$1/public_git/
> 
> but if we somehow could write it like
> 
>     E=GITWEB_PROJECTROOT:~$1/public_git/
> 
> it would be more generally useful, no?

I looked and I don't think so, ~user/public_git/ is not evaluated by
apache. Maybe it possible to evaluate it in the perl side, I will look
into it also.

> 
> > + the RewriteRule '/user/<user>' updated to allow
> >   '/user/<user>', '/user/<user>/' and '/user/<user>/gitweb.cgi'
> 
> Please describe what you added relative to the original, not just what the
> final result looks like.  "updated to allow A B C" doesn't tell the reader
> "it used to redirect only A and C to gitweb request, but B wasn't
> rewritten.", which seems to be the case if I am reading your regexp
> correctly.  Describing why it is better to also rewrite B would be a good
> idea, too, if it is not obvious.

Will do.

> 
> > + some typos fixed
> >
> > Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
> 
> > diff --git a/gitweb/README b/gitweb/README
> > index ad6a04c..bc90f4d 100644
> > --- a/gitweb/README
> > +++ b/gitweb/README
> > @@ -347,18 +347,18 @@ something like the following in your gitweb.conf (or gitweb_config.perl) file:
> >    $home_link = "/";
> >  
> >  
> > -Webserver configuration with multiple projects' root
> > -----------------------------------------------------
> > +Webserver configuration with multiple project roots
> > +---------------------------------------------------
> 
> Ok.
> 
> > -If you want to use gitweb with several project roots you can edit your apache
> > -virtual host and gitweb.conf configuration files like this :
> > +If you want to use gitweb with several project roots then you can edit your
> > +apache virtual host and gitweb.conf configuration files like this :
> 
> Ok (you might want to remove SP before colon, though).

My bad, French habit, but, according to wikipedia it is also English
"compliant" (http://en.wikipedia.org/wiki/Colon_%28punctuation%
29#Spacing). As you want.

> 
> >  virtual host configuration :
> >  
> >  <VirtualHost *:80>
> > -    ServerName			git.example.org
> > -    DocumentRoot		/pub/git
> > -    SetEnv				GITWEB_CONFIG	/etc/gitweb.conf
> > +    ServerName		git.example.org
> > +    DocumentRoot	/pub/git
> > +    SetEnv		GITWEB_CONFIG	/etc/gitweb.conf
> 
> What is this reindentation for?  "Just cosmetic" is an acceptable answer
> as long as the change resulted in cosmetic improvement, but it doesn't
> seem to be cosmetic improvement, either.

That was the case, it looked better in vim.

> 
> Thanks.


-- 
Sylvain Rabot <sylvain@abstraction.fr>

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 0/2] gitweb: updates regaring pêr user project root directory       
  2010-03-26 18:52   ` Junio C Hamano
  2010-03-29 20:43     ` Sylvain Rabot
@ 2010-03-29 22:34     ` Sylvain Rabot
  2010-03-29 22:34       ` [PATCH 1/2] gitweb: dirty patch to make url rewriting involving '+' working Sylvain Rabot
  1 sibling, 1 reply; 7+ messages in thread
From: Sylvain Rabot @ 2010-03-29 22:34 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

Patches resent to take care Junio's suggestions.

The first patch is quick and dirty. Although after quick tests it seems
to not break anything, gitweb maintainers approval is needed.

Regards

--
Sylvain

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] gitweb: dirty patch to make url rewriting involving '+' working
  2010-03-29 22:34     ` [PATCH 0/2] gitweb: updates regaring pêr user project root directory Sylvain Rabot
@ 2010-03-29 22:34       ` Sylvain Rabot
  2010-03-29 22:34         ` [PATCH 2/2] gitweb: readme fixed regarding per user project root repository Sylvain Rabot
  0 siblings, 1 reply; 7+ messages in thread
From: Sylvain Rabot @ 2010-03-29 22:34 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Sylvain Rabot

CGI::url method replaces '+' character by '%20', this patch reverts
this behavior to make url rewriting involving '+' in the base url working

Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
---
 gitweb/gitweb.perl |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index a2d2283..486996e 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -33,6 +33,10 @@ our $version = "++GIT_VERSION++";
 our $my_url = $cgi->url();
 our $my_uri = $cgi->url(-absolute => 1);
 
+# dirty patch to make url rewriting with '+' character working
+$my_url =~ s/(.*)%20(.*)/$1+$2/g;
+$my_uri =~ s/(.*)%20(.*)/$1+$2/g;
+
 # Base URL for relative URLs in gitweb ($logo, $favicon, ...),
 # needed and used only for URLs with nonempty PATH_INFO
 our $base_url = $my_url;
-- 
1.7.0.3

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] gitweb: readme fixed regarding per user project root repository
  2010-03-29 22:34       ` [PATCH 1/2] gitweb: dirty patch to make url rewriting involving '+' working Sylvain Rabot
@ 2010-03-29 22:34         ` Sylvain Rabot
  0 siblings, 0 replies; 7+ messages in thread
From: Sylvain Rabot @ 2010-03-29 22:34 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Sylvain Rabot

- the rewrite rules '/user/<user>' and '/+<user>' have been updated to also allow
  '/user/<user>/', '/user/<user>/gitweb.cgi', '/+<user>/' and '/+<user>/gitweb.cgi'
- some typos fixed

Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
---
 gitweb/README |   26 +++++++++++++-------------
 1 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/gitweb/README b/gitweb/README
index ad6a04c..63e5f2e 100644
--- a/gitweb/README
+++ b/gitweb/README
@@ -347,38 +347,38 @@ something like the following in your gitweb.conf (or gitweb_config.perl) file:
   $home_link = "/";
 
 
-Webserver configuration with multiple projects' root
-----------------------------------------------------
+Webserver configuration with multiple project roots
+---------------------------------------------------
 
-If you want to use gitweb with several project roots you can edit your apache
-virtual host and gitweb.conf configuration files like this :
+If you want to use gitweb with several project roots then you can edit your
+apache virtual host and gitweb.conf configuration files like this :
 
 virtual host configuration :
 
 <VirtualHost *:80>
-    ServerName			git.example.org
-    DocumentRoot		/pub/git
-    SetEnv				GITWEB_CONFIG	/etc/gitweb.conf
+    ServerName		git.example.org
+    DocumentRoot	/pub/git
+    SetEnv		GITWEB_CONFIG	/etc/gitweb.conf
 
     # turning on mod rewrite
     RewriteEngine on
 
     # make the front page an internal rewrite to the gitweb script
-    RewriteRule ^/$ 		/cgi-bin/gitweb.cgi [QSA,L,PT]
+    RewriteRule ^/$					/cgi-bin/gitweb.cgi [QSA,L,PT]
 
     # look for a public_git folder in unix users' home
     # http://git.example.org/~<user>/
-    RewriteRule ^/\~([^\/]+)(/|/gitweb.cgi)?$	/cgi-bin/gitweb.cgi [QSA,E=GITWEB_PROJECTROOT:/home/$1/public_git/,L,PT]
+    RewriteRule ^/\~([^\/]+)(/|/gitweb.cgi)?$		/cgi-bin/gitweb.cgi [QSA,E=GITWEB_PROJECTROOT:/home/$1/public_git/,L,PT]
 
     # http://git.example.org/+<user>/
-    #RewriteRule ^/\+([^\/]+)(/|/gitweb.cgi)?$	/cgi-bin/gitweb.cgi [QSA,E=GITWEB_PROJECTROOT:/home/$1/public_git/,L,PT]
+    #RewriteRule ^/\+([^\/]+)(/|/gitweb.cgi)?$		/cgi-bin/gitweb.cgi [QSA,E=GITWEB_PROJECTROOT:/home/$1/public_git/,L,PT]
 
     # http://git.example.org/user/<user>/
-    #RewriteRule ^/user/([^\/]+)/(gitweb.cgi)?$	/cgi-bin/gitweb.cgi [QSA,E=GITWEB_PROJECTROOT:/home/$1/public_git/,L,PT]
+    #RewriteRule ^/user/([^\/]+)(/|/gitweb.cgi)?$	/cgi-bin/gitweb.cgi [QSA,E=GITWEB_PROJECTROOT:/home/$1/public_git/,L,PT]
 
     # defined list of project roots
-    RewriteRule ^/scm(/|/gitweb.cgi)?$		/cgi-bin/gitweb.cgi [QSA,E=GITWEB_PROJECTROOT:/pub/scm/,L,PT]
-    RewriteRule ^/var(/|/gitweb.cgi)?$		/cgi-bin/gitweb.cgi [QSA,E=GITWEB_PROJECTROOT:/var/git/,L,PT]
+    RewriteRule ^/scm(/|/gitweb.cgi)?$			/cgi-bin/gitweb.cgi [QSA,E=GITWEB_PROJECTROOT:/pub/scm/,L,PT]
+    RewriteRule ^/var(/|/gitweb.cgi)?$			/cgi-bin/gitweb.cgi [QSA,E=GITWEB_PROJECTROOT:/var/git/,L,PT]
 
     # make access for "dumb clients" work
     RewriteRule ^/(.*\.git/(?!/?(HEAD|info|objects|refs)).*)?$ /cgi-bin/gitweb.cgi%{REQUEST_URI}  [L,PT]
-- 
1.7.0.3

^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2010-03-29 22:34 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-03-22 22:15 [PATCH] gitweb: readme fixed regarding per user project root repository Sylvain Rabot
2010-03-22 22:15 ` Sylvain Rabot
2010-03-26 18:52   ` Junio C Hamano
2010-03-29 20:43     ` Sylvain Rabot
2010-03-29 22:34     ` [PATCH 0/2] gitweb: updates regaring pêr user project root directory Sylvain Rabot
2010-03-29 22:34       ` [PATCH 1/2] gitweb: dirty patch to make url rewriting involving '+' working Sylvain Rabot
2010-03-29 22:34         ` [PATCH 2/2] gitweb: readme fixed regarding per user project root repository Sylvain Rabot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.