Merge branch 'master' into nine

Conflicts:
	master/buildbot/db/users.py
	master/docs/developer/utils.rst

master/buildbot/db/users.py

@@ -18,6 +18,7 @@
 from sqlalchemy.sql.expression import and_
 
 from buildbot.db import base
+from buildbot.util import identifiers
 
 
 class UsDict(dict):
@@ -30,7 +31,7 @@ class UsersConnectorComponent(base.DBConnectorComponent):
     def findUserByAttr(self, identifier, attr_type, attr_data, _race_hook=None):
         # note that since this involves two tables, self.findSomethingId is not
         # helpful
-        def thd(conn, no_recurse=False):
+        def thd(conn, no_recurse=False, identifier=identifier):
             tbl = self.db.model.users
             tbl_info = self.db.model.users_info
 
@@ -53,9 +54,11 @@ def thd(conn, no_recurse=False):
             # the new user and the corresponding attributes appear at the same
             # time from the perspective of other masters.
             transaction = conn.begin()
+            inserted_user = False
             try:
                 r = conn.execute(tbl.insert(), dict(identifier=identifier))
                 uid = r.inserted_primary_key[0]
+                inserted_user = True
 
                 conn.execute(tbl_info.insert(),
                              dict(uid=uid, attr_type=attr_type,
@@ -66,10 +69,19 @@ def thd(conn, no_recurse=False):
                 transaction.rollback()
 
                 # try it all over again, in case there was an overlapping,
-                # identical call to findUserByAttr, but only retry once.
+                # identical call to findUserByAttr.  If the identifier
+                # collided, we'll try again indefinitely; otherwise, only once.
                 if no_recurse:
                     raise
-                return thd(conn, no_recurse=True)
+
+                # if we failed to insert the user, then it's because the
+                # identifier wasn't unique
+                if not inserted_user:
+                    identifier = identifiers.incrementIdentifier(256, identifier)
+                else:
+                    no_recurse = True
+
+                return thd(conn, no_recurse=no_recurse, identifier=identifier)
 
             return uid
         d = self.db.pool.do(thd)

master/buildbot/test/unit/test_db_users.py

@@ -13,8 +13,6 @@
 #
 # Copyright Buildbot Team Members
 
-import sqlalchemy as sa
-
 from buildbot.db import users
 from buildbot.test.fake import fakedb
 from buildbot.test.util import connector_component
@@ -199,13 +197,31 @@ def thd(conn):
         return d
 
     def test_addUser_existing_identifier(self):
+        # see http://trac.buildbot.net/ticket/2587
         d = self.insertTestData(self.user1_rows)
         d.addCallback(lambda _: self.db.users.findUserByAttr(
-            identifier='soap',
-            attr_type='telepathIO(tm)',
-            attr_data='hmm,lye'))
-        return self.assertFailure(d, sa.exc.IntegrityError,
-                                  sa.exc.ProgrammingError)
+            identifier='soap',  # same identifier
+            attr_type='IPv9',
+            attr_data='fffffff.ffffff'))  # different attr
+
+        def check_user(uid):
+            # creates a new user
+            self.assertEqual(uid, 2)
+
+            def thd(conn):
+                users_tbl = self.db.model.users
+                users_info_tbl = self.db.model.users_info
+                users = conn.execute(users_tbl.select(order_by=users_tbl.c.identifier)).fetchall()
+                infos = conn.execute(users_info_tbl.select(users_info_tbl.c.uid == uid)).fetchall()
+                self.assertEqual(len(users), 2)
+                self.assertEqual(users[1].uid, uid)
+                self.assertEqual(users[1].identifier, 'soap_2')  # unique'd
+                self.assertEqual(len(infos), 1)
+                self.assertEqual(infos[0].attr_type, 'IPv9')
+                self.assertEqual(infos[0].attr_data, 'fffffff.ffffff')
+            return self.db.pool.do(thd)
+        d.addCallback(check_user)
+        return d
 
     def test_getUser(self):
         d = self.insertTestData(self.user1_rows)

master/contrib/svn_buildbot.py

@@ -240,7 +240,7 @@ def sendChanges(self, opts, changes):
     def sendAllChanges(self, remote, changes):
         dl = [remote.callRemote('addChange', change)
               for change in changes]
-        return defer.DeferredList(dl)
+        return defer.gatherResults(dl, consumeErrors=True)
 
     def run(self):
         opts = Options()