[PATCH 1/1] UBUNTU: SAUCE: Adopt the use of "BugLink:" lines in git commit messages.
Brad Figg
brad.figg at canonical.com
Fri May 1 23:21:51 UTC 2009
Andy Whitcroft wrote:
> 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
Why wouldn't your changes also show up in the output and thus in then
changelog?
--
Brad Figg brad.figg at canonical.com http://www.canonical.com
More information about the kernel-team
mailing list