On Scmbug

[Dave Swegen]

On Tue, Jun 01, 2004 at 11:57:04AM -0700, Kristis Makris wrote:
> Hello everyone,
> 
> Thank to David Miller for suggesting I should signup in this mailing
> list. Here are some of my comments:
> 
> > 1) (And this is the one that really annoyed me) If you want people to
> > use a tool that you've written, at least put a descriptive page up. Just
> 
> Sadly, I share the same view. What have I become! I'm sorry, I didn't want this work to go to waste, and I'm awfully busy at the moment. This glue was in use for over 2 months and I just got around to at least announcing it.

Once again apologies for being so grumpy. I'm not normally _that_ bad.
The bz->cvs integration could really do with more people, and scaring
them off by being a miserable Monday morning grumpo is never a good
idea. Welcome aboard :)

> 
> > 2) To be honest, I've pretty much gone off the idea of having the scm
> > system fiddling directly with the bz DB. The main problem is that as bz
> 

> Whatever bz officially offers as an interface, I'll gladly use -- just
> name what that is. Including globals.pl and calling functionality from
> there seemed like the right thing to do. I was planning on emailing
> this list to request for the extra functions I wrote for Bugzilla to
> be included in globals.pl. I noticed there's a WWW::Bugzilla Perl
> module(with which I've experimented for a while), but that requires
> logging in through the web gui with the user credentials, and I'm not
> sure how I should retrieve the user password in an automated way.

The way I was going to this was by having a admin user who owns all
commit messages. It's not as nice as having the comments owned by the
respective commiters, but you get all the sql checking and db
abstraction for free. 

It also solves one of the other big problems - mapping cvs usernames to
bz logins. Your solution is workable, but I think the correct way of
doing that sort of thing is by adding an extra field to the user table.

Just putting the cvs login name, and also whatever username info the
script at the cvs end can obtain from the system into the comment seems
to me at least a reasonable compromise, at least until I feel brave
enough to mess about with the bz schema.

> > 4) (Related to the one above): Having an intermediary step that sits
> > between cvs and bz has two problems: One is that it is one extra
> > step which the user can muck up installing (though your solution is
> > far nicer than ours), and it may make things awkward from a
> > networking POV (think firewalls).
> 
> I find this step required to support a generic scm -> bugtracking
> tool. Problem 2, can be solved through an ssh tunnel, or vpn, which is
> the real solution to the problem.

Do you have a list of other bug trackers that you'd like to see
supported? The only one I can think of that is worth pondering is gnats,
but lets not give gnats users any more reason to stay with it ;)

The other question is how many setups are actually going to have
multiple SCMs talking to one bug tracker (or even more unlikely - the
other way round)? And once you've solved the problem of a) getting the
data from the scm, and b) getting it into the tracker anything in the
middle that doesn't need to be there is simply potential maintainance
and support problems. 

In the case of cvs->bz or svn->bz it gives you nothing. I also feel
quite wary of having a long-running deamon process written in a
scripting language (been there, done that).

> 
> > 6) Potential bug: I believe that the parsing of files,dirs and
> > versions in Glue.pm/prepare_activity_commit_from_CVS doesn't cope
> > with files or dirs with whitespace in them (it won't cope with
> > commas either, but
> 
> A solution to the commas problem was provided in:
> http://www.einval.com/~steve/software/cvs-bugzilla/ CVS was patched to

You might want to use http://bugzilla.mozilla.org/show_bug.cgi?id=199116
instead, which is the canonical location for all work on the cvs->bz
patches that I do.

> handle them.  I will look into this.
> http://bugzilla.mkgnu.net/show_bug.cgi?id=267

That was a hacky solution that was the only thing we could think of at
the time. Again, it simply adds to the complexity of getting the system
set up, and that is something I quite desperatly want to get away from.
Having people patch and recompile CVS is Wrong. It also breaks if there
are quotes in filenames.

By using the {Vsv} in the loginfo script this can be worked around. I've
attached a version of the script I was working on that shows how to get
the necessary information out. I believe it is at least as robust as the
CVS patched version (I believe only a file consistsing of dots, numbers,
and commas will break this method).

To add to my points above about supporting multiple SCMs/bug trackers,
I'd rather have a solution that supports a very limited (but common)
selection setups, but which is as easy as is possibly to set up and
maintain.

> 
> > 7) You might want to think about nicking our bz->viewcvs autolinking
> 
> Thanks, I will. Are there any functions in globals.pl I can use ?
> http://bugzilla.mkgnu.net/show_bug.cgi?id=266

To get that working we added one new function to globals.pl, added some
code to another function, and added some code to the parameters page.
The patch should be straightforward enough.

You will obviously have to modify the various regexps to match the
format of your commit comments, but it shouldn't be too hard.

> 
> Thanks for the feedback and your time guys.  Kristis

It's been very useful. I've been wanting to get some discussion on how
to progress the integration bit for some time. Even if we don't end up
working on the same system, it has allowed me to clarify some of my
ideas, so thanks for your time :)

I would love to see this stuff go into 2.20, if for no other reason that
I can stop maintaining a pile of patches at that point (which reminds me
- I really have to get the patches working with 2.18).

I should also perhaps mention that my reason for doing this work in the
first place was so that we could use bz at work: When we were looking at
tracking systems the choice was between the god-awful TestTrack Pro
(which mgmt loved) and bz. Bz won out because of the work we did.

So my interest in handing the sort of serious vote-winning ammo that scm
integration gives to inferior tracking systems is very limited. The next
time I end up having to fight that battle I'd like to have a trump card
up my sleeve, thankyou very much ;)

Cheers
    Dave
-------------- next part --------------
#!/usr/bin/perl -w
# -*- Mode: perl; indent-tabs-mode: nil -*-
#
# The contents of this file are subject to the Mozilla Public
# License Version 1.1 (the "License"); you may not use this file
# except in compliance with the License. You may obtain a copy of
# the License at http://www.mozilla.org/MPL/
#
# Software distributed under the License is distributed on an "AS
# IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
# implied. See the License for the specific language governing
# rights and limitations under the License.
#
# The Original Code is the Bugzilla Bug Tracking System.
#
# The Initial Developer of the Original Code is Plasmon Data Ltd
# Portions created by Plasmon Data Ltd are Copyright (C) 2003 Plasmon 
# Data Ltd. All Rights Reserved.
#
# Contributor(s): Dave Swegen <dswegen at software.plasmon.com>

# Description:
# Grotty script that takes a CVS log entry and mangles it into a comment
# that is appended to the specified bug(s) in bugzilla.
# The entry in CVSROOT/loginfo looks as follows:
# ALL (echo ""; echo "Date: "`date`; cat) | $CVSROOT/CVSROOT/bugzilla-watcher $USER %{Vsv}
# 
# $Id: bugzilla-watcher,v 1.1 2004/01/23 16:01:41 dsw Exp $

use strict;

use DBI;

my $db_host = 'localhost';
my $db_name = 'bugs';
my $db_port = '';
my $db_user = 'bugs';
my $db_pass = 'password';

my %userdict = (    'dsw'=>'dswegen at software.plasmon.com' );
                

# Nicked from Bugzilla/DB.pm
sub connect_main {
    my $dsn = "DBI:mysql:host=$db_host;database=$db_name;port=$db_port";

    #$dsn .= ";mysql_socket=$db_sock" if $db_sock;

    return _connect($dsn);
}

# Nicked from Bugzilla/DB.pm
sub _connect {
    my ($dsn) = @_;

    # connect using our known info to the specified db
    # Apache::DBI will cache this when using mod_perl
    my $dbh = DBI->connect($dsn,
                           $db_user,
                           $db_pass,
                           { RaiseError => 1,
                             PrintError => 0,
                             ShowErrorStatement => 1,
                             HandleError => \&_handle_error,
                             FetchHashKeyName => 'NAME_lc',
                             TaintIn => 1,
                           });

    return $dbh;
}

# Nicked from Bugzilla/DB.pm
sub _handle_error {
    require Carp;

    $_[0] = Carp::longmess($_[0]);
    return 0; # Now let DBI handle raising the error
}

# Checks to see if user is known, and if so returns login to use
sub validate_userid {
    my $user = $_[0];

    if (! exists $userdict{$user}) {
        print "No matching login for user id $user.\n";
        exit(0);
    }

    my $sql = 'SELECT userid FROM profiles WHERE login_name=?';
    if (undef ($dbh->selectrow_array($sql, undef, $userdict{$user})) {
        print "Login for user id $userdict{$user} not found\n";
        exit(0);
    }
    return $userdict{$user};
}

sub validate_bugs {
    foreach my $bug (@bugslist) {



    #"SELECT userid FROM profiles WHERE login_name = \'$SenderShort\';")
# Handle input from CVS. The command line looks something like this:
# user module/dir1/dir2  1.1,file1,1.2  <file>,<oldversion>,<newversion> ...

my $cvsuser = shift @ARGV;

my ($moddir, $rest) = ($ARGV[0] =~ /
    ^(.*?)         # Directory name
    \s+
    ([\d\.]+,.*)    # Rest of input
    $
    /x);

# DEBUG
print "argv = $ARGV[0]\n";
# If it simply a new directory being added we can quit out
# Of course we're buggered if someone decides to call a file
# '- New directory' ;)
if ($rest =~ /- New directory/) {
    exit (0)
}
#DEBUG
print "moddir= $moddir rest = $rest\n";
# Filearr is a list of 'oldversion,filename,newversion'
my @filearr = ($rest =~ /([\d\.]+,.*?,[\d\.]+)/g);

my (@lines) = readline(*STDIN);
chomp @lines;

my ($bug, $tag, $i, @rawlog);
my $done = 0;
# Process everything before actual log
while (! $done) {
    if ($lines[0] =~ /Log Message/) {
        $done = 1;
        shift @lines;
    }
    elsif ($lines[0] =~ s/^\s*Tag:\s*(.*)$/$1/) {
        $tag = shift @lines;
        chomp $tag;
    }
    else {
        shift @lines;
    }
}
if (! $tag) {
    $tag = "HEAD";
}
my $pad = '  ';
while (@lines) {
    my (%bugdict);
    # Grab and handle the bug id
    if ($lines[0] =~ s/
        ^BugID      # This is a bug id line
        :?          # Optional ':'
        \s*         # Optional whitespace
        \#?         # Optional '#'
        ([\d,\s]+)  # One or more bug ids, separated by commas and or whitespace
        \s*/$1/xi) {
        my @bugids = ($lines[0] =~ /(\d+)/g);
        # Make sure there are no dupes in there
        foreach my $i (@bugids) {
            $bugdict{$i} = 1;
        }
        $bug = join(' ', keys %bugdict);
        print "Appending log to bug(s) $bug\n";
        shift @lines;
    }
    elsif ($lines[0] =~ /^BugID:?\s*.*$/i) {
        print "BugID line found, but invalid entry. Ignoring.\n";
        push @rawlog, $pad . shift @lines;
    }
    else {
        push @rawlog, $pad . shift @lines;
    }
}
if (! $bug) {
    print "No bugID found. Not appending log message.";
    exit 0;
}
# Put all the lines in the logmessage into one string
my $log = join("\n", at rawlog);
my $filelines = "";
my $filename = "";
my (@line, $file, @files);
# Process filenames and versions
foreach my $arg (@filearr) {
    print "arg = $arg\n";
    @line = split(",", $arg);   # Commas in filenames screw things up:(
    $file = "$moddir/$line[1]\t$line[0]    $line[2]";
    # Get rid of whitespace-in-filename preserving quotes
    $file =~ s/\"//g;
    push @files, $file;
}
while (@files) {
    $filelines = $filelines . $pad . (shift @files) . "\n";
}

# Remove trailing empty lines
$/="";
# Remove various forms of whitespace
$log =~ s/^\s+//s;
$log =~ s/\s+$//s;
$filelines =~ s/^\s+//s;
$filelines =~ s/\s+$//s;

$log = SqlQuote($log);

my $clogmesg =  "CVS COMMIT\nLOG MESSAGE:\n$pad$log\nBRANCH: $tag\nFILES CHECKED IN:\n$pad$filelines";

# Must work to here before proceeding
# DEBUG
print $clogmesg;

# Try to connect
my $dbh;
$dbh = connect_main();
if (! defined $dbh) {
    print "Connection to DB failed\n";
    exit 0;
}

# Check to see if user is known
my $login = validate_user($cvsuser);
my @bugs = validate_bugs();