[Buildbot-devel] Changing the behavior of the SVN buildstep defaultBranch argument...

Gareth Armstrong gareth.armstrong at hp.com
Mon Mar 29 13:37:03 UTC 2010


Hello again,

sorry for the delay but my weekend was a little full.  Many thanks for 
the feedback.


On 03/26/2010 07:55 PM, Dustin J. Mitchell wrote:
> We now have source stamps specifying the source code completely:
> repository, project, branch, revision.  Sure, branch can be None, in
> which case you need to supply a default (this was a design mistake,
> IMHO).  And revision can be None, which means "latest" (this is very
> useful).
>    
I am with you on both counts.
> So I think that if we're going to change existing behavior, it should
> be in the direction of just building what you're told, rather than
> (and am I reading this right?!) making the defaultBranch *override*
> the branch in the SourceStamp.
>
> Dustin
>    

I fully agree, I should have been brave enough to propose such a change 
at the start.  So how about this.

All the best,

Gareth

diff --git a/buildbot/steps/source.py b/buildbot/steps/source.py
index 62083db..9341687 100644
--- a/buildbot/steps/source.py
+++ b/buildbot/steps/source.py
@@ -438,43 +438,32 @@ class SVN(Source):

      name = 'svn'

-    def __init__(self, svnurl=None, baseURL=None, defaultBranch=None,
-                 directory=None, username=None, password=None,
+    def __init__(self, baseURL=None, branch='trunk',
+                 username=None, password=None,
                   extra_args=None, keep_on_purge=None, ignore_ignores=None,
                   always_purge=None, depth=None, **kwargs):
          """
-        @type  svnurl: string
-        @param svnurl: the URL which points to the Subversion server,
-                       combining the access method (HTTP, ssh, local file),
-                       the repository host/port, the repository path, the
-                       sub-tree within the repository, and the branch to
-                       check out. Using C{svnurl} does not enable builds of
-                       alternate branches: use C{baseURL} to enable this.
-                       Use exactly one of C{svnurl} and C{baseURL}.
-
-        @param baseURL: if branches are enabled, this is the base URL to
+        @type  baseURL: string
+        @param baseURL: the URL which points to the Subversion server,
+                        combining the access method (HTTP, ssh, local 
file),
+                        the repository host/port, the repository path, the
+                        sub-tree within the repository. This is the 
base URL to
                          which a branch name will be appended. It should
-                        probably end in a slash. Use exactly one of
-                        C{svnurl} and C{baseURL}.
+                        probably end in a slash.

-        @param defaultBranch: if branches are enabled, this is the branch
-                              to use if the Build does not specify one
-                              explicitly. It will simply be appended
-                              to C{baseURL} and the result handed to
-                              the SVN command.
+        @type  branch: string
+        @param branch: This is the branch for a build,  It will simply be
+                       appended to C{baseURL} and the result handed to
+                       the SVN command.

+        @type  username: string
          @param username: username to pass to svn's --username
-        @param password: username to pass to svn's --password
-        """
-
-        if not kwargs.has_key('workdir') and directory is not None:
-            # deal with old configs
-            warn("Please use workdir=, not directory=", DeprecationWarning)
-            kwargs['workdir'] = directory

-        self.svnurl = svnurl
+        @type  password: string
+        @param password: password to pass to svn's --password
+        """
          self.baseURL = baseURL
-        self.branch = defaultBranch
+        self.branch = branch
          self.username = username
          self.password = password
          self.extra_args = extra_args
@@ -484,23 +473,17 @@ class SVN(Source):
          self.depth = depth

          Source.__init__(self, **kwargs)
-        self.addFactoryArguments(svnurl=svnurl,
-                                 baseURL=baseURL,
-                                 defaultBranch=defaultBranch,
-                                 directory=directory,
+        self.addFactoryArguments(baseURL=baseURL,
+                                 branch=branch,
                                   username=username,
                                   password=password,
                                   extra_args=extra_args,
                                   keep_on_purge=keep_on_purge,
                                   ignore_ignores=ignore_ignores,
                                   always_purge=always_purge,
-                 depth=depth,
+                                 depth=depth,
                                   )

-        if not svnurl and not baseURL:
-            raise ValueError("you must use exactly one of svnurl and 
baseURL")
-
-
      def computeSourceRevision(self, changes):
          if not changes or None in [c.revision for c in changes]:
              return None
@@ -508,7 +491,6 @@ class SVN(Source):
          return lastChange

      def startVC(self, branch, revision, patch):
-
          # handle old slaves
          warnings = []
          slavever = self.slaveVersion("svn", "old")
@@ -516,6 +498,14 @@ class SVN(Source):
              m = "slave does not have the 'svn' command"
              raise BuildSlaveTooOldError(m)

+        if slavever == "old":
+            # 0.5.0 compatibility
+            m = ("This buildslave (%s) is too old and probably won't "
+                 "do what is expected of it.  Refusing to build.  "
+                 "Please upgrade the buildslave to buildbot-0.7.0 or "
+                 "newer." % self.build.slavename)
+            raise BuildSlaveTooOldError(m)
+
          if self.slaveVersionIsOlderThan("svn", "1.39"):
              # the slave doesn't know to avoid re-using the same sourcedir
              # when the branch changes. We have no way of knowing which 
branch
@@ -526,43 +516,13 @@ class SVN(Source):
                  and self.args['mode'] in ("update", "copy")):
                  m = ("This buildslave (%s) does not know about multiple "
                       "branches, and using mode=%s would probably build 
the "
-                     "wrong tree. "
+                     "wrong tree.  "
                       "Refusing to build. Please upgrade the buildslave 
to "
                       "buildbot-0.7.0 or newer." % (self.build.slavename,
                                                     self.args['mode']))
                  raise BuildSlaveTooOldError(m)

-        if slavever == "old":
-            # 0.5.0 compatibility
-            if self.args['mode'] in ("clobber", "copy"):
-                # TODO: use some shell commands to make up for the
-                # deficiency, by blowing away the old directory first (thus
-                # forcing a full checkout)
-                warnings.append("WARNING: this slave can only do SVN 
updates"
-                                ", not mode=%s\n" % self.args['mode'])
-                log.msg("WARNING: this slave only does mode=update")
-            if self.args['mode'] == "export":
-                raise BuildSlaveTooOldError("old slave does not have "
-                                            "mode=export")
-            self.args['directory'] = self.args['workdir']
-            if revision is not None:
-                # 0.5.0 can only do HEAD. We have no way of knowing whether
-                # the requested revision is HEAD or not, and for
-                # slowly-changing trees this will probably do the right
-                # thing, so let it pass with a warning
-                m = ("WARNING: old slave can only update to HEAD, not "
-                     "revision=%s" % revision)
-                log.msg(m)
-                warnings.append(m + "\n")
-            revision = "HEAD" # interprets this key differently
-            if patch:
-                raise BuildSlaveTooOldError("old slave can't do patch")
-
-        if self.svnurl:
-            assert not branch # we need baseURL= to use branches
-            self.args['svnurl'] = self.svnurl
-        else:
-            self.args['svnurl'] = self.baseURL + branch
+        self.args['svnurl'] = self.baseURL + self.branch
          self.args['revision'] = revision
          self.args['patch'] = patch

@@ -572,28 +532,30 @@ class SVN(Source):
          if self.depth is not None:
              if self.slaveVersionIsOlderThan("svn","2.9"):
                  m = ("This buildslave (%s) does not support svn depth "
-                 "arguments.  "
-                 "Refusing to build. "
-                 "Please upgrade the buildslave." % (self.build.slavename))
+                     "arguments.  Refusing to build.  Please upgrade the "
+                     "SVN client on the buildslave." % 
self.build.slavename)
                  raise BuildSlaveTooOldError(m)
-            else:
+            else:
                  self.args['depth'] = self.depth

          if self.username is not None or self.password is not None:
              if self.slaveVersionIsOlderThan("svn", "2.8"):
                  m = ("This buildslave (%s) does not support svn 
usernames "
-                     "and passwords.  "
-                     "Refusing to build. Please upgrade the buildslave to "
-                     "buildbot-0.7.10 or newer." % (self.build.slavename,))
+                     "and passwords.  Refusing to build.  Please upgrade "
+                     "the buildslave to buildbot-0.7.10 or newer." %
+                     self.build.slavename)
                  raise BuildSlaveTooOldError(m)
-            if self.username is not None: self.args['username'] = 
self.username
-            if self.password is not None: self.args['password'] = 
self.password
+            if self.username is not None:
+                self.args['username'] = self.username
+            if self.password is not None:
+                self.args['password'] = self.password

          if self.extra_args is not None:
              self.args['extra_args'] = self.extra_args

          revstuff = []
-        if branch is not None and branch != self.branch:
+        #revstuff.append("self.branch=%s" % self.branch)
+        if self.branch.find('trunk') == -1:
              revstuff.append("[branch]")
          if revision is not None:
              revstuff.append("r%s" % revision)


-- 
-------------------------------------------------------------------------------
  Gareth ARMSTRONG
  HP Communications&  Media Solutions
  Email  : gareth.armstrong at hp.com
  Phone  : +33 (0)4.76.14.43.89
-------------------------------------------------------------------------------






More information about the devel mailing list