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

[Brad Figg]

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.

> 
>>  			}
>>  			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.

Brad

-- 
Brad Figg brad.figg at canonical.com http://www.canonical.com