reviewboard: register config options (
bug 1413628); r?glob
Mercurial 4.4 softly requires config options be registered via a central
registrar. If they aren't, a devel warning is issued. The test runner
enables all devel warnings by default (which is a good idea). This
means we get new output and test failures for attempting to access a
config option that isn't registered.
When options are registered, Mercurial also issues a devel warning when
the caller accessing the option specifies a default value. This is
because the default value is defined as part of the option and doesn't
need to be specified at the call site.
Porting an extension to this mechanism is a bit annoying because old
Mercurial versions need to specify a default value (since the option
isn't registered) and new Mercurial versions would issue a warning
if a default argument is specified. The workaround is to set the
default value of "dynamicdefault." This special value means "the
default is dynamic" and ui.config* functions won't issue a devel
warning if a default value is specified.
When support for older versions of Mercurial are dropped, the proper
default value can be registered and the "default" argument can be
dropped from ui.config* callers.
This commit registers all config options used by the reviewboard
extension. By doing so, most test failures in 4.4 due to new
"devel-warn:" lines go away. This leaves a handful of legitimate
test failures.
MozReview-Commit-ID: KjS9MRuc2KD
--- a/hgext/reviewboard/client.py
+++ b/hgext/reviewboard/client.py
@@ -44,16 +44,23 @@ from mercurial import (
obsolete,
phases,
registrar,
scmutil,
sshpeer,
templatekw,
util,
)
+
+# TRACKING hg43
+try:
+ from mercurial import configitems
+except ImportError:
+ configitems = None
+
from mercurial.i18n import _
from mercurial.node import bin, hex
OUR_DIR = os.path.normpath(os.path.dirname(__file__))
execfile(os.path.join(OUR_DIR, '..', 'bootstrap.py'))
with demandimport.deactivated():
try:
@@ -71,16 +78,17 @@ from hgrb.util import (
from mozautomation.commitparser import (
parse_bugs,
parse_commit_id,
)
from mozhg.auth import (
getbugzillaauth,
configureautobmoapikeyauth,
+ register_config_items,
)
from mozhg.rewrite import (
newparents,
replacechangesets,
)
testedwith = '4.1 4.2 4.3'
minimumhgversion = '4.1'
@@ -90,16 +98,38 @@ cmdtable = {}
# Mercurial 4.3 introduced registrar.command as a replacement for
# cmdutil.command.
if util.safehasattr(registrar, 'command'):
command = registrar.command(cmdtable)
else:
command = cmdutil.command(cmdtable)
+# Mercurial 4.3 introduced the config registrar. 4.4 requires config
+# items to be registered to avoid a devel warning.
+if util.safehasattr(registrar, 'configitem'):
+ # Imports get evaluated in block scope. So the if above has no impact.
+
+
+ configtable = {}
+ configitem = registrar.configitem(configtable)
+
+ # Register bugzilla.* options via shared auth module.
+ register_config_items(configitem)
+
+ configitem('mozilla', 'ircnick',
+ default=configitems.dynamicdefault)
+ configitem('reviewboard', 'autopublish',
+ default=configitems.dynamicdefault)
+ configitem('reviewboard', 'deduce-reviewers',
+ default=configitems.dynamicdefault)
+ configitem('reviewboard', 'fakeids',
+ default=configitems.dynamicdefault)
+
+
clientcapabilities = {
'proto1',
'listreviewdata',
'listreviewrepos',
'bzapikeys',
'jsonproto',
'commitid',
}
@@ -681,17 +711,17 @@ def doreview(repo, ui, remote, nodes):
# reviewboard.autopublish, prompt the user. Otherwise, publish
# automatically or not based on this value.
if ui.config('reviewboard', 'autopublish', None) is None:
ui.write('\n')
publish = ui.promptchoice(_('publish these review '
'requests now (Yn)? '
'$$ &Yes $$ &No')) == 0
else:
- publish = ui.configbool('reviewboard', 'autopublish')
+ publish = ui.configbool('reviewboard', 'autopublish', False)
if publish:
publishreviewrequests(ui, remote, bzauth, [newparentid])
else:
ui.status(_('(visit review url to publish these review '
'requests so others can see them)\n'))
--- a/hgext/reviewboard/hgrb/util.py
+++ b/hgext/reviewboard/hgrb/util.py
@@ -83,17 +83,17 @@ def genid(repo=None, fakeidpath=None):
Base62 is used as the encoding mechanism because it is safe for both
URLs and revsets. We could get base66 for URLs, but the characters
-~. could conflict with revsets.
"""
# Provide a backdoor to generate deterministic IDs. This is used for
# testing purposes because tests want constant output. And since
# commit IDs go into the commit and are part of the SHA-1, they need
# to be deterministic.
- if repo and repo.ui.configbool('reviewboard', 'fakeids'):
+ if repo and repo.ui.configbool('reviewboard', 'fakeids', False):
fakeidpath = repo.vfs.join('genid')
if fakeidpath:
try:
with open(fakeidpath, 'rb') as fh:
data = fh.read()
except IOError as e:
if e.errno != errno.ENOENT:
--- a/hgext/reviewboard/server.py
+++ b/hgext/reviewboard/server.py
@@ -27,16 +27,23 @@ from mercurial import (
cmdutil,
demandimport,
extensions,
hg,
registrar,
util,
wireproto,
)
+
+# TRACKING hg43
+try:
+ from mercurial import configitems
+except ImportError:
+ configitems = None
+
from mercurial.i18n import _
from mercurial.node import (
hex,
nullid,
)
from mercurial.hgweb import (
webcommands,
)
@@ -61,23 +68,36 @@ cmdtable = {}
# Mercurial 4.3 introduced registrar.command as a replacement for
# cmdutil.command.
if util.safehasattr(registrar, 'command'):
command = registrar.command(cmdtable)
else:
command = cmdutil.command(cmdtable)
-# Mercurial 4.3 started defining config items using a central registrar.
-# Mercurial 4.4 finished this transition.
-try:
- from mercurial import configitems
- have_config_registrar = True
-except ImportError:
- have_config_registrar = False
+# Mercurial 4.4 requires config items to be registered.
+if util.safehasattr(registrar, 'configitem'):
+ configtable = {}
+ configitem = registrar.configitem(configtable)
+
+ configitem('bugzilla', 'url',
+ default=configitems.dynamicdefault)
+ configitem('reviewboard', 'isdiscoveryrepo',
+ default=configitems.dynamicdefault)
+ configitem('reviewboard', 'password',
+ default=configitems.dynamicdefault)
+ configitem('reviewboard', 'repobasepath',
+ default=configitems.dynamicdefault)
+ configitem('reviewboard', 'repoid',
+ default=configitems.dynamicdefault)
+ configitem('reviewboard', 'url',
+ default=configitems.dynamicdefault)
+ configitem('reviewboard', 'username',
+ default=configitems.dynamicdefault)
+
# Capabilities the server requires in clients.
#
# Add to this and add a corresponding entry in the client extension to force
# the client to pull latest code.
requirecaps = set([
# Client can speak protocol format 1.
'proto1',
@@ -214,17 +234,17 @@ def wrappedwireprotoheads(orig, repo, pr
1) A client will create a bundle before sending it to the server. This will
take minutes on mozilla-central.
2) Bundle2 aware HTTP clients don't like receiving a "pusherr" response from
unbundle - they insist on receiving a bundle2 payload. However, the
server doesn't know if the client supports bundle2 until it reads the
header from the bundle submission!
"""
- if not repo.ui.configbool('reviewboard', 'isdiscoveryrepo'):
+ if not repo.ui.configbool('reviewboard', 'isdiscoveryrepo', False):
return orig(repo, proto, *args, **kwargs)
return wireproto.ooberror(nopushdiscoveryrepos)
def sendjsonresponse(req, obj):
req.respond(HTTP_OK, 'application/json')
return [json.dumps(obj, indent=2, sort_keys=True, encoding='utf-8')]
@@ -379,17 +399,17 @@ def reposetup(ui, repo):
basepath = ui.config('reviewboard', 'repobasepath', None)
if basepath and repo.root.startswith(basepath):
active = True
if not active:
return
if (not ui.configint('reviewboard', 'repoid', None) and
- not ui.configbool('reviewboard', 'isdiscoveryrepo')):
+ not ui.configbool('reviewboard', 'isdiscoveryrepo', False)):
raise util.Abort(_('Please set reviewboard.repoid to the numeric ID '
'of the repository this repo is associated with.'))
if not ui.config('reviewboard', 'url', None):
raise util.Abort(_('Please set reviewboard.url to the URL of the '
'Review Board instance to talk to.'))
if not ui.config('reviewboard', 'username', None):
@@ -400,25 +420,25 @@ def reposetup(ui, repo):
raise util.Abort(_('Please set reviewboard.password to the password '
'for priveleged communications with Review Board.'))
if not ui.config('bugzilla', 'url', None):
raise util.Abort(_('Please set bugzilla.url to the URL of the '
'Bugzilla instance to talk to.'))
# TRACKING hg33+
- if have_config_registrar:
+ if configitems:
publish = ui.configbool('phases', 'publish')
else:
publish = ui.configbool('phases', 'publish', True)
if publish:
raise util.Abort(_('reviewboard server extension is only compatible '
'with non-publishing repositories.'))
ui.setconfig('hooks', 'changegroup.reviewboard', changegrouphook)
# This shouldn't be needed to prevent pushes, as the "heads" wireproto
# wrapping should kill them. However, this is defense in depth and will
# kill all changegroup additions, even if the server operator is dumb.
- if ui.configbool('reviewboard', 'isdiscoveryrepo'):
+ if ui.configbool('reviewboard', 'isdiscoveryrepo', False):
ui.setconfig('hooks', 'pretxnchangegroup.disallowpush',
disallowpushhook)
--- a/pylib/mozhg/mozhg/auth.py
+++ b/pylib/mozhg/mozhg/auth.py
@@ -8,16 +8,41 @@ import platform
import shutil
import tempfile
import urlparse
from mercurial import config, util
from mercurial.i18n import _
+def register_config_items(configitem):
+ """Registers config items with Mercurial's registrar.
+
+ The argument is a ``registrar.configitem`` instance.
+ """
+ from mercurial import configitems
+
+ configitem('bugzilla', 'username',
+ default=configitems.dynamicdefault)
+ configitem('bugzilla', 'apikey',
+ default=configitems.dynamicdefault)
+ configitem('bugzilla', 'password',
+ default=configitems.dynamicdefault)
+ configitem('bugzilla', 'userid',
+ default=configitems.dynamicdefault)
+ configitem('bugzilla', 'cookie',
+ default=configitems.dynamicdefault)
+ configitem('bugzilla', 'firefoxprofile',
+ default=configitems.dynamicdefault)
+ configitem('bugzilla', 'url',
+ default=configitems.dynamicdefault)
+ configitem('mozilla', 'trustedbmoapikeyservices',
+ default=configitems.dynamicdefault)
+
+
class BugzillaAuth(object):
"""Holds Bugzilla authentication credentials."""
def __init__(self, userid=None, cookie=None, username=None, password=None,
apikey=None):
if apikey:
self._type = 'apikey'
elif userid:
@@ -51,21 +76,21 @@ def getbugzillaauth(ui, require=False, p
4) login cookies from Firefox profiles
5) prompt the user
The ``bugzilla.firefoxprofile`` option is interpreted as a list of Firefox
profiles from which data should be read. This overrides the default sort
order.
"""
- username = ui.config('bugzilla', 'username')
- apikey = ui.config('bugzilla', 'apikey')
- password = ui.config('bugzilla', 'password')
- userid = ui.config('bugzilla', 'userid')
- cookie = ui.config('bugzilla', 'cookie')
+ username = ui.config('bugzilla', 'username', None)
+ apikey = ui.config('bugzilla', 'apikey', None)
+ password = ui.config('bugzilla', 'password', None)
+ userid = ui.config('bugzilla', 'userid', None)
+ cookie = ui.config('bugzilla', 'cookie', None)
profileorder = ui.configlist('bugzilla', 'firefoxprofile', [])
if username and apikey:
return BugzillaAuth(username=username, apikey=apikey)
if userid and cookie:
return BugzillaAuth(userid=userid, cookie=cookie)
@@ -293,18 +318,18 @@ def configureautobmoapikeyauth(ui):
carries over the [bugzilla] entries to [auth] entries for trusted services.
"""
services = ui.configlist('mozilla', 'trustedbmoapikeyservices',
TRUSTEDAPIKEYSERVICES)
if not services:
ui.debug('no trusted services to auto define credentials on\n')
return
- username = ui.config('bugzilla', 'username')
- apikey = ui.config('bugzilla', 'apikey')
+ username = ui.config('bugzilla', 'username', None)
+ apikey = ui.config('bugzilla', 'apikey', None)
if not username:
ui.debug('Bugzilla username not defined; cannot define credentials\n')
return
for i, service in enumerate(services):
ui.debug('automatically setting Bugzilla API Key auth %s\n' % service)
key = 'autobmoapikey%d' % i
ui.setconfig('auth', '%s.prefix' % key, service)