[PATCH 1/1] UBUNTU: SAUCE: Adopt the use of "BugLink:" lines in git commit messages.

[Andy Whitcroft]

On Fri, May 01, 2009 at 01:49:53PM -0700, Brad Figg wrote:
> Andy Whitcroft wrote:
> > On Fri, May 01, 2009 at 11:12:12AM -0700, Brad Figg wrote:
> >> From rtg:
> >>
> >>   I think there are a couple of good reasons for doing this.
> >>
> >>   1) I'm a lazy typist. I find it quite convenient to simply click on the
> >>   Buglink URL and be presented with the bug page.
> >>
> >>   2) The 'Bug:' field implies a certain context, which is useless when
> >>   pushing our patches upstream. Rather then having to cleanse patches of
> >>   irrelevant information, lets put it in the commit in an interesting and
> >>   useful form to begin with. I suspect, given the feedback that Amit has
> >>   already received from Andrew Morton, that the 'Bug:' field will not be
> >>   well received in general.
> >>
> >>   3) Finally, if we start developing for the upstream kernel (as we will
> >>   if I get my way), we can take advantage of BugLink goodness
> >>   automatically as our upstream patches show up in stable and other
> >>   places.
> >>
> >> BugLink: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/370475
> >>
> >> Signed-off-by: Brad Figg <brad.figg at canonical.com>
> >> ---
> >>  debian/scripts/misc/git-ubuntu-log |   29 +++++++++++++++++++++++++----
> >>  1 files changed, 25 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/debian/scripts/misc/git-ubuntu-log b/debian/scripts/misc/git-ubuntu-log
> >> index 860990c..b880ce6 100755
> >> --- a/debian/scripts/misc/git-ubuntu-log
> >> +++ b/debian/scripts/misc/git-ubuntu-log
> >> @@ -60,7 +60,7 @@ sub add_entry($) {
> >>  }
> >>  
> >>  sub shortlog_entry($$$$$) {
> >> -	my ($name, $desc, $bug, $cve, $commit) = @_;
> >> +	my ($name, $desc, $bug, $buglink, $cve, $commit) = @_;
> >>  	my $entry;
> >>  
> >>  	$desc =~ s#/pub/scm/linux/kernel/git/#/.../#g;
> >> @@ -74,6 +74,7 @@ sub shortlog_entry($$$$$) {
> >>  	$entry->{'cve'} = $cve;
> >>  	$entry->{'commit'} = $commit;
> >>  	$entry->{'author'} = $name;
> >> +	$entry->{'buglink'} = $buglink;
> >>  
> >>  	if ($desc =~ /^Revert "/) {
> >>  		push(@reverts, $entry);
> >> @@ -117,6 +118,23 @@ sub shortlog_output {
> >>  			}
> >>  			if (defined($entry->{'bugno'})) {
> >>  				print "    - LP: #" .  $entry->{'bugno'} . "\n";
> >> +				# If there isn't a buglink, assume the bug number is to a bug
> >> +				# in launchpad and generate the appropriate url
> >> +				#
> >> +				if (!defined($entry->{'buglink'})) {
> >> +					print "    - BugLink: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/" .  $entry->{'bugno'} . "\n";
> >> +				}
> >> +			}
> >> +			if (defined($entry->{'buglink'})) {
> >> +				# If there isn't a bugno line, and if the buglink is into
> >> +				# launchpad, use the buglink to create the launchpad bug number
> >> +				#
> >> +				if (!defined($entry->{'bugno'})) {
> >> +					if ($entry->{'buglink'} =~ /https:\/\/.*.launchpad.net\/.*\/([0-9]+)\S*$/) {
> >> +						print "    - LP: #" .  $1 . "\n";
> >> +					}
> >> +				}
> >> +				print "    - BugLink: " .  $entry->{'buglink'} . "\n";
> > 
> > I don't think I was expecting to find the buglink going into the debian
> > changelog.  Tim were you?
> 
> I guess I don't know how the output from this script is used but how do the
> above lines indicate that it's going to the debian changelog? It looks to
> me that everything goes to stdout. It's kicked off from the printchanges
> rule of debian/rules.

All of the output ends up in the debian changelog when we run fdr
insertchanges, as I understand things.

> > 
> >>  			}
> >>  			if (defined($entry->{'cve'})) {
> >>  				print "    - " . $entry->{'cve'} . "\n";
> >> @@ -167,6 +185,7 @@ sub changelog_input {
> >>  			next unless /^\s*?(.*)/;
> >>  			my $ignore = 0;
> >>  			my $bug = undef;
> >> +			my $buglink = undef;
> >>  
> >>  			# skip lines that are obviously not
> >>  			# a 1-line cset description
> >> @@ -178,7 +197,8 @@ sub changelog_input {
> >>  			if ($desc =~ /^ *(Revert "|)UBUNTU:/) {
> >>  				while (<STDIN>) {
> >>  					$ignore = 1 if /^ *Ignore: yes/i;
> >> -					$bug = $2 if /^ *Bug: *(#|)(.*)/;
> >> +					$bug = $2 if /^ *Bug: *(#|)(.*)/i;
> >> +					$buglink = $1 if /^ *BugLink: *(http.*)/i;
> >>  					$cve = $1 if /^ *(CVE-.*)/;
> >>  					last if /^commit /;
> >>  				}
> >> @@ -186,14 +206,15 @@ sub changelog_input {
> >>  				$author = $kernel_auth;
> >>  				$ignore = 1 if $desc =~ /Merge /;
> >>  				while (<STDIN>) {
> >> -					$bug = $2 if /^ *Bug: *(#|)(.*)/;
> >> +					$bug = $2 if /^ *Bug: *(#|)(.*)/i;
> >> +					$buglink = $1 if /^ *BugLink: *(http.*)/i;
> > 
> > This form only allows us to put one BugLink: in the change.  We rarely
> > but sometimes do do the following if more than one bug is fixed by the
> > same change:
> > 
> >    Bug: #123456, #234567
> > 
> > If we don't need the BugLink: in the debian/changelog then I think we
> > could make multiple BugLink: <foo> headers work and simplify the patch
> > a lot by simply adding them to $bug, something like this:
> > 
> > 		$bug = ''
> > 		while (<STDIN>) {
> > 			$bug .= $2 . "," if /^ *Bug: *(#|)(.*)/i;
> > 			$bug .= $1 . ","  if /^ *BugLink: *http.*\/([0-9]+)/i;
> > 			$cve = $1 if /^ *(CVE-.*)/;
> > 			last if /^commit /;
> > 		}
> > 		chop($bug);
> > 
> >>  				}
> >>  			}
> >>  
> >>  			if (!$ignore) {
> > 
> >> -				&shortlog_entry($author, $desc, $bug,
> >> +				&shortlog_entry($author, $desc, $bug, $buglink,
> >>  						$cve, $commit, 0);
> >>  			}
> >>  
> >> -- 
> > 
> > -apw
> 
> I understand that by adding the BugLink info to the regular $bug you've
> simplified things a bit but how does this change things from going to
> the debian changelog?
> 
> Also, there are a couple of cases where if bug# is present bug buglink
> is not and vice versa that I was handling and now your change doesn't.
> 
> I do agree, that I should handle multiple buglinks and that I don't.

Well my change makes sense only if we are not intending on putting
the BugLinks into the debian changelog ie. that we only care about
extracting the bug numbers whichever way they are specified.  I think
my form would see bug numbers either on bug: nnnn lines or on buglink:
..../nnnnn lines and treat them as the same thing, ie. producing - LP:
NNNN in the debian changelog which is key internally for the bug janitor
to close bugs as they packages release.

The key question is whether we expect to be putting buglinks into the
debian/changelog.  I am expecting the answer to that to be no.  We are
looking only to change from using Bug: # to BugLink: in the git
changelog so that the bug means something when it goes upstream.  I
think we need clarification from Tim.

-apw