This is an archive of the discontinued Mercurial Phabricator instance.

Differential D974

py3: handle keyword arguments correctly in hgext/patchbomb.py
ClosedPublic

Authored by pulkit on Oct 5 2017, 8:20 PM.

Details

Reviewers
yuja
Group Reviewers
hg-reviewers
Commits
rHG71e63fe6b1ab: py3: handle keyword arguments correctly in hgext/patchbomb.py
Summary

The keys of keyword arguments must be str on Python 3 which is unicode. This
patch make sure we pass keyword arguments with keys are str everywhere in this
file and access the keys depending on whether they are bytes or str.

This patch uses pycompat.{byteskwargs|strkwargs} and somewhere it also added r''
to prevent transformer from adding a b'' over there.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

pulkit created this revision.Oct 5 2017, 8:20 PM
yuja requested changes to this revision.Oct 7 2017, 8:49 AM
yuja added a subscriber: yuja.

and missing import of pycompat. Please make sure all tests pass.

hgext/patchbomb.py
631–632

opts is modified here, but stropts isn't. Perhaps we shouldn't
keep the copy for long.

This revision now requires changes to proceed.Oct 7 2017, 8:49 AM
pulkit added a comment.EditedOct 7 2017, 1:27 PM
In D974#16430, @yuja wrote:

and missing import of pycompat. Please make sure all tests pass.

Oops, sorry. I am not sure how this one slipped in without passing tests. I will be more careful from now.

pulkit abandoned this revision.Oct 17 2017, 7:19 PM

Will revisit after the freeze.

yuja requested changes to this revision.Nov 4 2017, 3:39 AM
yuja added inline comments.
hgext/patchbomb.py
631–632

I think it's better to use a single bytes dict instead of managing
two copies carefully.

This revision now requires changes to proceed.Nov 4 2017, 3:39 AM
pulkit updated this revision to Diff 3376.Nov 9 2017, 9:05 AM
yuja accepted this revision.Nov 13 2017, 8:39 AM

Queued this, thanks.

This revision is now accepted and ready to land.Nov 13 2017, 8:39 AM
Closed by commit rHG71e63fe6b1ab: py3: handle keyword arguments correctly in hgext/patchbomb.py (authored by pulkit). · Explain WhyNov 13 2017, 9:21 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext/patchbomb.py (43 lines)

Diff 3231

hgext/patchbomb.py

cmdutil, cmdutil,
commands, commands,
error, error,
formatter, formatter,
hg, hg,
mail, mail,
node as nodemod, node as nodemod,
patch, patch,
pycompat,
registrar, registrar,
repair, repair,
scmutil, scmutil,
templater, templater,
util, util,
) )
stringio = util.stringio stringio = util.stringio
The bundle is a returned as a single in-memory binary blob. The bundle is a returned as a single in-memory binary blob.
""" """
ui = repo.ui ui = repo.ui
tmpdir = tempfile.mkdtemp(prefix='hg-email-bundle-') tmpdir = tempfile.mkdtemp(prefix='hg-email-bundle-')
tmpfn = os.path.join(tmpdir, 'bundle') tmpfn = os.path.join(tmpdir, 'bundle')
btype = ui.config('patchbomb', 'bundletype') btype = ui.config('patchbomb', 'bundletype')
if btype: if btype:
opts['type'] = btype opts[r'type'] = btype
try: try:
commands.bundle(ui, repo, tmpfn, dest, **opts) commands.bundle(ui, repo, tmpfn, dest, **opts)
return util.readfile(tmpfn) return util.readfile(tmpfn)
finally: finally:
try: try:
os.unlink(tmpfn) os.unlink(tmpfn)
except OSError: except OSError:
pass pass
os.rmdir(tmpdir) os.rmdir(tmpdir)
def _getdescription(repo, defaultbody, sender, **opts): def _getdescription(repo, defaultbody, sender, **opts):
"""obtain the body of the introduction message and return it """obtain the body of the introduction message and return it
This is also used for the body of email with an attached bundle. This is also used for the body of email with an attached bundle.
The body can be obtained either from the command line option or entered by The body can be obtained either from the command line option or entered by
the user through the editor. the user through the editor.
""" """
ui = repo.ui ui = repo.ui
if opts.get('desc'): if opts.get(r'desc'):
body = open(opts.get('desc')).read() body = open(opts.get(r'desc')).read()
else: else:
ui.write(_('\nWrite the introductory message for the ' ui.write(_('\nWrite the introductory message for the '
'patch series.\n\n')) 'patch series.\n\n'))
body = ui.edit(defaultbody, sender, repopath=repo.path, body = ui.edit(defaultbody, sender, repopath=repo.path,
action='patchbombbody') action='patchbombbody')
# Save series description in case sendmail fails # Save series description in case sendmail fails
msgfile = repo.vfs('last-email.txt', 'wb') msgfile = repo.vfs('last-email.txt', 'wb')
msgfile.write(body) msgfile.write(body)
msgfile.close() msgfile.close()
return body return body
def _getbundlemsgs(repo, sender, bundle, **opts): def _getbundlemsgs(repo, sender, bundle, **opts):
"""Get the full email for sending a given bundle """Get the full email for sending a given bundle
This function returns a list of "email" tuples (subject, content, None). This function returns a list of "email" tuples (subject, content, None).
The list is always one message long in that case. The list is always one message long in that case.
""" """
ui = repo.ui ui = repo.ui
_charsets = mail._charsets(ui) _charsets = mail._charsets(ui)
subj = (opts.get('subject') subj = (opts.get(r'subject')
or prompt(ui, 'Subject:', 'A bundle for your repository')) or prompt(ui, 'Subject:', 'A bundle for your repository'))
body = _getdescription(repo, '', sender, **opts) body = _getdescription(repo, '', sender, **opts)
msg = emailmod.MIMEMultipart.MIMEMultipart() msg = emailmod.MIMEMultipart.MIMEMultipart()
if body: if body:
msg.attach(mail.mimeencode(ui, body, _charsets, opts.get('test'))) msg.attach(mail.mimeencode(ui, body, _charsets, opts.get(r'test')))
datapart = emailmod.MIMEBase.MIMEBase('application', 'x-mercurial-bundle') datapart = emailmod.MIMEBase.MIMEBase('application', 'x-mercurial-bundle')
datapart.set_payload(bundle) datapart.set_payload(bundle)
bundlename = '%s.hg' % opts.get('bundlename', 'bundle') bundlename = '%s.hg' % opts.get(r'bundlename', 'bundle')
datapart.add_header('Content-Disposition', 'attachment', datapart.add_header('Content-Disposition', 'attachment',
filename=bundlename) filename=bundlename)
emailmod.Encoders.encode_base64(datapart) emailmod.Encoders.encode_base64(datapart)
msg.attach(datapart) msg.attach(datapart)
msg['Subject'] = mail.headencode(ui, subj, _charsets, opts.get('test')) msg['Subject'] = mail.headencode(ui, subj, _charsets, opts.get(r'test'))
return [(msg, subj, None)] return [(msg, subj, None)]
def _makeintro(repo, sender, revs, patches, **opts): def _makeintro(repo, sender, revs, patches, **opts):
"""make an introduction email, asking the user for content if needed """make an introduction email, asking the user for content if needed
email is returned as (subject, body, cumulative-diffstat)""" email is returned as (subject, body, cumulative-diffstat)"""
ui = repo.ui ui = repo.ui
_charsets = mail._charsets(ui) _charsets = mail._charsets(ui)
# use the last revision which is likely to be a bookmarked head # use the last revision which is likely to be a bookmarked head
prefix = _formatprefix(ui, repo, revs.last(), opts.get('flag'), prefix = _formatprefix(ui, repo, revs.last(), opts.get(r'flag'),
0, len(patches), numbered=True) 0, len(patches), numbered=True)
subj = (opts.get('subject') or subj = (opts.get(r'subject') or
prompt(ui, '(optional) Subject: ', rest=prefix, default='')) prompt(ui, '(optional) Subject: ', rest=prefix, default=''))
if not subj: if not subj:
return None # skip intro if the user doesn't bother return None # skip intro if the user doesn't bother
subj = prefix + ' ' + subj subj = prefix + ' ' + subj
body = '' body = ''
if opts.get('diffstat'): if opts.get(r'diffstat'):
# generate a cumulative diffstat of the whole patch series # generate a cumulative diffstat of the whole patch series
diffstat = patch.diffstat(sum(patches, [])) diffstat = patch.diffstat(sum(patches, []))
body = '\n' + diffstat body = '\n' + diffstat
else: else:
diffstat = None diffstat = None
body = _getdescription(repo, body, sender, **opts) body = _getdescription(repo, body, sender, **opts)
msg = mail.mimeencode(ui, body, _charsets, opts.get('test')) msg = mail.mimeencode(ui, body, _charsets, opts.get(r'test'))
msg['Subject'] = mail.headencode(ui, subj, _charsets, msg['Subject'] = mail.headencode(ui, subj, _charsets,
opts.get('test')) opts.get(r'test'))
return (msg, subj, diffstat) return (msg, subj, diffstat)
def _getpatchmsgs(repo, sender, revs, patchnames=None, **opts): def _getpatchmsgs(repo, sender, revs, patchnames=None, **opts):
"""return a list of emails from a list of patches """return a list of emails from a list of patches
This involves introduction message creation if necessary. This involves introduction message creation if necessary.
This function returns a list of "email" tuples (subject, content, None). This function returns a list of "email" tuples (subject, content, None).
""" """
bytesopts = pycompat.byteskwargs(opts)
ui = repo.ui ui = repo.ui
_charsets = mail._charsets(ui) _charsets = mail._charsets(ui)
patches = list(_getpatches(repo, revs, **opts)) patches = list(_getpatches(repo, revs, **opts))
msgs = [] msgs = []
ui.write(_('this patch series consists of %d patches.\n\n') ui.write(_('this patch series consists of %d patches.\n\n')
% len(patches)) % len(patches))
# build the intro message, or skip it if the user declines # build the intro message, or skip it if the user declines
if introwanted(ui, opts, len(patches)): if introwanted(ui, bytesopts, len(patches)):
msg = _makeintro(repo, sender, revs, patches, **opts) msg = _makeintro(repo, sender, revs, patches, **opts)
if msg: if msg:
msgs.append(msg) msgs.append(msg)
# are we going to send more than one message? # are we going to send more than one message?
numbered = len(msgs) + len(patches) > 1 numbered = len(msgs) + len(patches) > 1
# now generate the actual patch messages # now generate the actual patch messages
name = None name = None
assert len(revs) == len(patches) assert len(revs) == len(patches)
for i, (r, p) in enumerate(zip(revs, patches)): for i, (r, p) in enumerate(zip(revs, patches)):
if patchnames: if patchnames:
name = patchnames[i] name = patchnames[i]
msg = makepatch(ui, repo, r, p, opts, _charsets, i + 1, msg = makepatch(ui, repo, r, p, bytesopts, _charsets,
len(patches), numbered, name) i + 1, len(patches), numbered, name)
msgs.append(msg) msgs.append(msg)
return msgs return msgs
def _getoutgoing(repo, dest, revs): def _getoutgoing(repo, dest, revs):
'''Return the revisions present locally but not in dest''' '''Return the revisions present locally but not in dest'''
ui = repo.ui ui = repo.ui
url = ui.expandpath(dest or 'default-push', dest or 'default') url = ui.expandpath(dest or 'default-push', dest or 'default')
mutt -R -f mbox # ... and view it with mutt mutt -R -f mbox # ... and view it with mutt
hg email -o -m mbox && # generate an mbox file ... hg email -o -m mbox && # generate an mbox file ...
formail -s sendmail \\ # ... and use formail to send from the mbox formail -s sendmail \\ # ... and use formail to send from the mbox
-bm -t < mbox # ... using sendmail -bm -t < mbox # ... using sendmail
Before using this command, you will need to enable email in your Before using this command, you will need to enable email in your
hgrc. See the [email] section in hgrc(5) for details. hgrc. See the [email] section in hgrc(5) for details.
''' '''
stropts = opts
opts = pycompat.byteskwargs(opts)
_charsets = mail._charsets(ui) _charsets = mail._charsets(ui)
bundle = opts.get('bundle') bundle = opts.get('bundle')
date = opts.get('date') date = opts.get('date')
mbox = opts.get('mbox') mbox = opts.get('mbox')
outgoing = opts.get('outgoing') outgoing = opts.get('outgoing')
rev = opts.get('rev') rev = opts.get('rev')
if bookmark not in repo._bookmarks: if bookmark not in repo._bookmarks:
raise error.Abort(_("bookmark '%s' not found") % bookmark) raise error.Abort(_("bookmark '%s' not found") % bookmark)
revs = repair.stripbmrevset(repo, bookmark) revs = repair.stripbmrevset(repo, bookmark)
revs = scmutil.revrange(repo, revs) revs = scmutil.revrange(repo, revs)
if outgoing: if outgoing:
revs = _getoutgoing(repo, dest, revs) revs = _getoutgoing(repo, dest, revs)
if bundle: if bundle:
opts['revs'] = [str(r) for r in revs] opts['revs'] = [str(r) for r in revs]
stropts[r'revs'] = [str(r) for r in revs]
yujaUnsubmitted
Not Done

opts is modified here, but stropts isn't. Perhaps we shouldn't
keep the copy for long.

yuja: opts is modified here, but stropts isn't. Perhaps we shouldn't keep the copy for long.
yujaUnsubmitted
Not Done

I think it's better to use a single bytes dict instead of managing
two copies carefully.

yuja: I think it's better to use a single bytes dict instead of managing two copies carefully.
# check if revision exist on the public destination # check if revision exist on the public destination
publicurl = repo.ui.config('patchbomb', 'publicurl') publicurl = repo.ui.config('patchbomb', 'publicurl')
if publicurl: if publicurl:
repo.ui.debug('checking that revision exist in the public repo') repo.ui.debug('checking that revision exist in the public repo')
try: try:
publicpeer = hg.peer(repo, {}, publicurl) publicpeer = hg.peer(repo, {}, publicurl)
except error.RepoError: except error.RepoError:
return '<%s.%s@%s>' % (id[:20], int(start_time[0]), socket.getfqdn()) return '<%s.%s@%s>' % (id[:20], int(start_time[0]), socket.getfqdn())
# deprecated config: patchbomb.from # deprecated config: patchbomb.from
sender = (opts.get('from') or ui.config('email', 'from') or sender = (opts.get('from') or ui.config('email', 'from') or
ui.config('patchbomb', 'from') or ui.config('patchbomb', 'from') or
prompt(ui, 'From', ui.username())) prompt(ui, 'From', ui.username()))
if bundle: if bundle:
bundledata = _getbundle(repo, dest, **opts) bundledata = _getbundle(repo, dest, **stropts)
bundleopts = opts.copy() bundleopts = stropts.copy()
bundleopts.pop('bundle', None) # already processed bundleopts.pop(r'bundle', None) # already processed
msgs = _getbundlemsgs(repo, sender, bundledata, **bundleopts) msgs = _getbundlemsgs(repo, sender, bundledata, **bundleopts)
else: else:
msgs = _getpatchmsgs(repo, sender, revs, **opts) msgs = _getpatchmsgs(repo, sender, revs, **stropts)
showaddrs = [] showaddrs = []
def getaddrs(header, ask=False, default=None): def getaddrs(header, ask=False, default=None):
configkey = header.lower() configkey = header.lower()
opt = header.replace('-', '_').lower() opt = header.replace('-', '_').lower()
addrs = opts.get(opt) addrs = opts.get(opt)
if addrs: if addrs: