[Buildbot-devel] SVN.parseGotRevision() patch, v. 2

[Aaron Maxwell]

An update: The master.cfg example I have below is not quite right.  I managed 
to mix up buildbot.slave.commands.SVN and buildbot.steps.source.SVN there.

Originally I thought of this patch as generally valuable. Now I'm not so 
certain: I did not find a good way to subclass buildbot.slave.commands.SVN, 
and have it registered, all in master.cfg. It's actually easy if you don't 
mind adding the subclass in buildbot/slave/commands.py directly, which is 
what I ended up doing. But clearly that is not ideal.

I still think the patch has value and would like to see it integrated, because 
if nothing else it encapsulates the svn version command, and IMO makes the 
whole class more extensible and flexible.  (There's also selfishness, in that 
I personally find it useful, but I'm trying not to let that influence me :) 

Okay, I guess that's all.  Brian (or other committer - sorry, I don't know 
everyone here yet), if this is something you'd like to check in, let me know 
if I can help.  Regardless, thanks for buildbot.

-Aaron

On Tuesday 06 November 2007 14:59:52 Aaron Maxwell wrote:
> Hi all,
>
> Based on the feedback and further experiments, here is what I think is a
> much better patch:
> http://redsymbol.net/buildbot/commands.py_2.patch
>
> (I've pasted it below as well.)  Basically, it introduces a method
> SVN.getSvnVersionCommand() that returns the shell command used to determine
> the svn version.  By default, it just returns ["svnversion", "."], in line
> with what's in 0.7.6.  Someone like myself who needs a different
> calculation can just subclass it.  To illustrate, here's what we now have
> in our master.cfg:
>
> {{{
> from buildbot.steps.source import SVN
> class SnapSVN(SVN):
>     def getSvnVersionCommand(self):
>         # We use a current working directory of '..' because we want
>         # to get the most recent checkin revision under trunk/ (or
>         # branch/foo/), rather than trunk/SnapLogic/ .
>         return [getCommand("svnversion"), "-c", ".."]
>
> primary_factory.addStep(SnapSVN,
>                        name='Checking out',
>                        haltOnFailure=True,
> <and so on>
> }}}
>
> This is a lot cleaner and more extensible that the first patch, I think.
> Brian, is this a patch you might consider integrating in trunk?  Does
> anything need to be changed?
>
> Feedback appreciated.  For convenience, here's the patch contents:  (sorry,
> it's laid out confusingly - basically I added the getSvnVersionCommand()
> method, then made a few smaller changes to parseGotRevision())
>
> --- buildbot/slave/commands.py.orig     2007-10-29 20:55:59.000000000 -0700
> +++ buildbot/slave/commands.py  2007-11-06 14:26:14.000000000 -0800
> @@ -1494,15 +1494,24 @@
>          self.command = c
>          return c.start()
>
> -    def parseGotRevision(self):
> +    def getSvnVersionCommand(self):
> +        """
> +        Get the (shell) command used to determine SVN revision number
> +        of checked-out code
> +
> +        return: list of strings, passable as the command argument to
> ShellCommand
> +        """
>          # svn checkout operations finish with 'Checked out revision
> 16657.' # svn update operations finish the line 'At revision 16654.' # But
> we don't use those. Instead, run 'svnversion'.
>          svnversion_command = getCommand("svnversion")
>          # older versions of 'svnversion' (1.1.4) require the WC_PATH
>          # argument, newer ones (1.3.1) do not.
> -        command = [svnversion_command, "."]
> -        c = ShellCommand(self.builder, command,
> +        return [svnversion_command, "."]
> +
> +    def parseGotRevision(self):
> +        c = ShellCommand(self.builder,
> +                         self.getSvnVersionCommand(),
>                           os.path.join(self.builder.basedir, self.srcdir),
>                           environ=self.env,
>                           sendStdout=False, sendStderr=False, sendRC=False,
> @@ -1510,16 +1519,16 @@
>          c.usePTY = False
>          d = c.start()
>          def _parse(res):
> -            r = c.stdout.strip()
> -            # Support for removing svnversion indicator for 'modified'
> -            if r[-1] == 'M':
> -                r = r[:-1]
> +            r_raw = c.stdout.strip()
> +            # Extract revision from the version "number" string
> +            r = r_raw.rstrip('MS')
> +            r = r.split(':')[-1]
>              got_version = None
>              try:
>                  got_version = int(r)
>              except ValueError:
>                  msg =("SVN.parseGotRevision unable to parse output "
> -                      "of svnversion: '%s'" % r)
> +                      "of svnversion: '%s'" % r_raw)
>                  log.msg(msg)
>                  self.sendStatus({'header': msg + "\n"})
>              return got_version



-- 
Aaron Maxwell .:. amax at snaplogic.org .:. http://snaplogic.org
SnapLogic, Inc. - Data Integration for the Last Mile