threads / patch / 30128

patchGitweb: Fix unintended "--no-merges" for regular Atom feed

Subject: [PATCH] Gitweb: Fix unintended "--no-merges" for regular Atom feed

## tl;dr

6 messages between Apr 2, 2012 and Apr 11, 2012. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

Sebastian Pipping· Apr 2, 2012, 16:44 UTC · lore
Hello!

Please excuse that I send the patch as an attachment and consider application. Thanks!

Best,
Sebastian
>From 03bed689671df47040c4a0ad158c0c9b039a50bf Mon Sep 17 00:00:00 2001
From: Sebastian Pipping <sebastian@pipping.org>
Date: Mon, 2 Apr 2012 18:39:48 +0200
Subject: [PATCH] Gitweb: Fix unintended "--no-merges" for regular Atom feed

Before: <link rel="alternate" title="[..] - Atom feed" href="/?p=.git;a=atom;opt=--no-merges" type="application/atom+xml" /> <link rel="alternate" title="[..] - Atom feed (no merges)" href="/?p=.git;a=atom;opt=--no-merges" type="application/atom+xml" />

After:
<link rel="alternate" title="[..] - Atom feed" href="/?p=.git;a=atom;opt=" type="application/atom+xml" />
<link rel="alternate" title="[..] - Atom feed (no merges)" href="/?p=.git;a=atom;opt=--no-merges" type="application/atom+xml" />
---
 gitweb/gitweb.perl |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
Show changes to gitweb/gitweb.perl +1 −0
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index a8b5fad..cc45ae3 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3886,6 +3886,7 @@ sub print_feed_meta {
 				'-type' => "application/$type+xml"
 			);
 
+			$href_params{'extra_options'} = '';
 			$href_params{'action'} = $type;
 			$link_attr{'-href'} = href(%href_params);
 			print "<link ".

-- 
1.7.8.5
Jakub Narebski· Apr 4, 2012, 12:25 UTC · re: Sebastian Pipping · lore

[PATCH (bugfix)] gitweb: Fix unintended "--no-merges" for regular Atom feed

From: Sebastian Pipping <sebastian@pipping.org>

The print_feed_meta() subroutine generates links for feeds with and without merges, in RSS and Atom formats. However because %href_params was not properly reset, it generated links with "--no-merges" for all except the very first link.

Before: <link rel="alternate" title="[..] - Atom feed" href="/?p=.git;a=atom;opt=--no-merges" type="application/atom+xml" /> <link rel="alternate" title="[..] - Atom feed (no merges)" href="/?p=.git;a=atom;opt=--no-merges" type="application/atom+xml" />

After: <link rel="alternate" title="[..] - Atom feed" href="/?p=.git;a=atom" type="application/atom+xml" /> <link rel="alternate" title="[..] - Atom feed (no merges)" href="/?p=.git;a=atom;opt=--no-merges" type="application/atom+xml" />

Signed-off-by: Sebastian Pipping <sebastian@pipping.org>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Sebastian Pipping wrote:
> Please excuse that I send the patch as an attachment and consider
> application.  Thanks!

Beside sending this patch as an attachement instead of putting it inline (what was the cause of this?) it was also lacking Signed-off-by... which I have forged.

I have added explanation of this error in the commit message, and changed from using '' to undef to get rid of 'opt' / 'extra_options' parameter instead of having it empty. It is a better way of doing the reset.

Junio, the bug is very minor, so I don't know if it is worth fixing for 1.7.10.

 gitweb/gitweb.perl |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
Show changes to gitweb/gitweb.perl +1 −0
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index a8b5fad2..ca6f038 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3886,6 +3886,7 @@ sub print_feed_meta {
 				'-type' => "application/$type+xml"
 			);
 
+			$href_params{'extra_options'} = undef;
 			$href_params{'action'} = $type;
 			$link_attr{'-href'} = href(%href_params);
 			print "<link ".

-- 
1.7.9
Junio C Hamano· Apr 4, 2012, 17:47 UTC · re: Jakub Narebski · lore

Re: [PATCH (bugfix)] gitweb: Fix unintended "--no-merges" for regular Atom feed

Jakub Narebski <jnareb@gmail.com> writes:
> Junio, the bug is very minor, so I don't know if it is worth fixing
> for 1.7.10.

Does this exist in 1.7.9.x maintenance track? If it is an old bug, I do not think it should go to 1.7.10 proper (perhaps 1.7.10.1) this late, and if it is a bug in a new feature added for 1.7.10, we may want to fix it before the final, as the impact of the patch seems very minor.

Show 15 quoted lines
>  gitweb/gitweb.perl |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index a8b5fad2..ca6f038 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -3886,6 +3886,7 @@ sub print_feed_meta {
>  				'-type' => "application/$type+xml"
>  			);
>  
> +			$href_params{'extra_options'} = undef;
>  			$href_params{'action'} = $type;
>  			$link_attr{'-href'} = href(%href_params);
>  			print "<link ".
Jakub Narebski· Apr 4, 2012, 18:58 UTC · re: Junio C Hamano · lore

Re: [PATCH (bugfix)] gitweb: Fix unintended "--no-merges" for regular Atom feed

Junio C Hamano wrote:
Show 9 quoted lines
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > Junio, the bug is very minor, so I don't know if it is worth fixing
> > for 1.7.10.
> 
> Does this exist in 1.7.9.x maintenance track?  If it is an old bug, I do
> not think it should go to 1.7.10 proper (perhaps 1.7.10.1) this late, and
> if it is a bug in a new feature added for 1.7.10, we may want to fix it
> before the final, as the impact of the patch seems very minor.

It is an old bug, from 05bb5a2 (gitweb: Include links to feeds in HTML header only for '200 OK' response, 2010-12-18) which refactored feed link generation into print_feed_meta(). It is in gitweb since v1.7.4 I think.

So 1.7.10.1 it is...
Show 16 quoted lines
> >  gitweb/gitweb.perl |    1 +
> >  1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> > index a8b5fad2..ca6f038 100755
> > --- a/gitweb/gitweb.perl
> > +++ b/gitweb/gitweb.perl
> > @@ -3886,6 +3886,7 @@ sub print_feed_meta {
> >  				'-type' => "application/$type+xml"
> >  			);
> >  
> > +			$href_params{'extra_options'} = undef;
> >  			$href_params{'action'} = $type;
> >  			$link_attr{'-href'} = href(%href_params);
> >  			print "<link ".
> 
-- 
Jakub Narebski
Poland
Jakub Narebski· Apr 11, 2012, 15:39 UTC · re: Jakub Narebski · lore

Re: [PATCH (bugfix)] gitweb: Fix unintended "--no-merges" for regular Atom feed

On Wed, 4 April 2012, Jakub Narebski wrote:
Show 16 quoted lines
> Junio C Hamano wrote:
> > Jakub Narebski <jnareb@gmail.com> writes:
> > 
> > > Junio, the bug is very minor, so I don't know if it is worth fixing
> > > for 1.7.10.
> > 
> > Does this exist in 1.7.9.x maintenance track?  If it is an old bug, I do
> > not think it should go to 1.7.10 proper (perhaps 1.7.10.1) this late, and
> > if it is a bug in a new feature added for 1.7.10, we may want to fix it
> > before the final, as the impact of the patch seems very minor.
> 
> It is an old bug, from 05bb5a2 (gitweb: Include links to feeds in HTML
> header only for '200 OK' response, 2010-12-18) which refactored feed link
> generation into print_feed_meta().  It is in gitweb since v1.7.4 I think.
> 
> So 1.7.10.1 it is...
Ping!
I don't see this trivial fix (admittedly for obscure bug) in "What's
cooking", and it is not present in 'master'.
 
> > >  gitweb/gitweb.perl |    1 +
> > >  1 files changed, 1 insertions(+), 0 deletions(-)
-- 
Jakub Narebski
Poland
Junio C Hamano· Apr 11, 2012, 16:48 UTC · re: Jakub Narebski · lore

Re: [PATCH (bugfix)] gitweb: Fix unintended "--no-merges" for regular Atom feed

Jakub Narebski <jnareb@gmail.com> writes:
>> So 1.7.10.1 it is...
>
> Ping!

You are part of my distributed memory and the system is nicely working ;-)

> I don't see this trivial fix (admittedly for obscure bug) in "What's
> cooking", and it is not present in 'master'.
Of course not.  Now you reminded me, it will.
Thanks.

← back to recent threads