( )⚙ D10181 verify: convert an exception to bytes before logging

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10181

verify: convert an exception to bytes before logging
ClosedPublic

Authored by mharbison72 on Mar 12 2021, 1:09 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHGed0899e01628: verify: convert an exception to bytes before logging
Summary

I'm not entirely sure why this code appears to be trying to convert twice, but
it was flagged by pytype:

File "/mnt/c/Users/Matt/hg/mercurial/verify.py", line 84, in _exc: Function _bytestr.__init__ was called with the wrong arguments [wrong-arg-types]
         Expected: (self, ints: Iterable[int])
  Actually passed: (self, ints: Exception)
  The following methods aren't implemented on Exception:
  __iter__

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mharbison72 created this revision.Mar 12 2021, 1:09 PM
mharbison72 added inline comments.Mar 12 2021, 1:14 PM
mercurial/verify.py
88–89

Was this a logic error? I've not seen this pattern in other places. Everything I've seen assumes pycompat.bytestr() returns something.

pulkit added a subscriber: pulkit.Mar 12 2021, 1:21 PM
pulkit added inline comments.
mercurial/verify.py
88–89

Does seems like so.

mharbison72 added inline comments.Mar 16 2021, 6:05 PM
mercurial/verify.py
88–89

Are you waiting for me to fix this? I figured I'd wait until this lands on stable, and then fix it on default since it never runs.

mharbison72 added a commit: rHGed0899e01628: verify: convert an exception to bytes before logging.Mar 17 2021, 6:12 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHGed0899e01628: verify: convert an exception to bytes before logging (authored by mharbison72). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/verify.py (5 lines)

Diff 26471

mercurial/verify.py

# verify.py - repository integrity checking for Mercurial # verify.py - repository integrity checking for Mercurial
# #
# Copyright 2006, 2007 Matt Mackall <mpm@selenic.com> # Copyright 2006, 2007 Matt Mackall <mpm@selenic.com>
# #
# This software may be used and distributed according to the terms of the # This software may be used and distributed according to the terms of the
# GNU General Public License version 2 or any later version. # GNU General Public License version 2 or any later version.
from __future__ import absolute_import from __future__ import absolute_import
import os import os
from .i18n import _ from .i18n import _
from .node import ( from .node import (
nullid, nullid,
short, short,
) )
from .utils import (
stringutil,
)
from . import ( from . import (
error, error,
pycompat, pycompat,
revlog, revlog,
util, util,
) )
msg = b"%s: %s" % (linkrev, msg) msg = b"%s: %s" % (linkrev, msg)
if filename: if filename:
msg = b"%s@%s" % (filename, msg) msg = b"%s@%s" % (filename, msg)
self.ui.warn(b" " + msg + b"\n") self.ui.warn(b" " + msg + b"\n")
self.errors += 1 self.errors += 1
def _exc(self, linkrev, msg, inst, filename=None): def _exc(self, linkrev, msg, inst, filename=None):
"""record exception raised during the verify process""" """record exception raised during the verify process"""
fmsg = pycompat.bytestr(inst) fmsg = stringutil.forcebytestr(inst)
if not fmsg: if not fmsg:
fmsg = pycompat.byterepr(inst) fmsg = pycompat.byterepr(inst)
mharbison72AuthorUnsubmitted
Done

Was this a logic error? I've not seen this pattern in other places. Everything I've seen assumes pycompat.bytestr() returns something.

mharbison72: Was this a logic error? I've not seen this pattern in other places. Everything I've seen…
pulkitUnsubmitted
Not Done

Does seems like so.

pulkit: Does seems like so.
mharbison72AuthorUnsubmitted
Done

Are you waiting for me to fix this? I figured I'd wait until this lands on stable, and then fix it on default since it never runs.

mharbison72: Are you waiting for me to fix this? I figured I'd wait until this lands on stable, and then…
self._err(linkrev, b"%s: %s" % (msg, fmsg), filename) self._err(linkrev, b"%s: %s" % (msg, fmsg), filename)
def _checkrevlog(self, obj, name, linkrev): def _checkrevlog(self, obj, name, linkrev):
"""verify high level property of a revlog """verify high level property of a revlog
- revlog is present, - revlog is present,
- revlog is non-empty, - revlog is non-empty,
- sizes (index and data) are correct, - sizes (index and data) are correct,