( )⚙ D2863 cleanup: fix some latent open(path).read() et al calls we previously missed

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2863

cleanup: fix some latent open(path).read() et al calls we previously missed
ClosedPublic

Authored by durin42 on Mar 14 2018, 3:53 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHG0585337ea787: cleanup: fix some latent open(path).read() et al calls we previously missed
rHG57eb879feb77: cleanup: fix some latent open(path).read() et al calls we previously missed
rHG76c594af8ec8: cleanup: fix some latent open(path).read() et al calls we previously missed
Summary

This pattern was banned by check-code way back in 1b4b82063ce2 (may of
2011), but due to a regular expression rewriting bug in check-code
these particular callsites were never detected. Python 3.7 caught the
bug, which then exposed these errors.

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.

Revision Contents
Changeset List

PathPackages
Mi18n/hggettext (6 lines)
Mmercurial/cffi/bdiffbuild.py (7 lines)
Mmercurial/cffi/mpatchbuild.py (5 lines)
Mmercurial/posix.py (3 lines)
Mtests/f (3 lines)

Diff 7058

i18n/hggettext

def docstrings(path): def docstrings(path):
"""Extract docstrings from path. """Extract docstrings from path.
This respects the Mercurial cmdtable/table convention and will This respects the Mercurial cmdtable/table convention and will
only extract docstrings from functions mentioned in these tables. only extract docstrings from functions mentioned in these tables.
""" """
mod = importpath(path) mod = importpath(path)
if not path.startswith('mercurial/') and mod.__doc__: if not path.startswith('mercurial/') and mod.__doc__:
src = open(path).read() with open(path) as fobj:
src = fobj.read()
lineno = 1 + offset(src, mod.__doc__, path, 7) lineno = 1 + offset(src, mod.__doc__, path, 7)
print(poentry(path, lineno, mod.__doc__)) print(poentry(path, lineno, mod.__doc__))
functions = list(getattr(mod, 'i18nfunctions', [])) functions = list(getattr(mod, 'i18nfunctions', []))
functions = [(f, True) for f in functions] functions = [(f, True) for f in functions]
cmdtable = getattr(mod, 'cmdtable', {}) cmdtable = getattr(mod, 'cmdtable', {})
if not cmdtable: if not cmdtable:
if origdoc: if origdoc:
lineno += offset(src, origdoc, name, 1) lineno += offset(src, origdoc, name, 1)
else: else:
lineno += offset(src, doc, name, 1) lineno += offset(src, doc, name, 1)
print(poentry(actualpath, lineno, doc)) print(poentry(actualpath, lineno, doc))
def rawtext(path): def rawtext(path):
src = open(path).read() with open(path) as f:
src = f.read()
print(poentry(path, 1, src)) print(poentry(path, 1, src))
if __name__ == "__main__": if __name__ == "__main__":
# It is very important that we import the Mercurial modules from # It is very important that we import the Mercurial modules from
# the source tree where hggettext is executed. Otherwise we might # the source tree where hggettext is executed. Otherwise we might
# accidentally import and extract strings from a Mercurial # accidentally import and extract strings from a Mercurial
# installation mentioned in PYTHONPATH. # installation mentioned in PYTHONPATH.
sys.path.insert(0, os.getcwd()) sys.path.insert(0, os.getcwd())
from mercurial import demandimport; demandimport.enable() from mercurial import demandimport; demandimport.enable()
for path in sys.argv[1:]: for path in sys.argv[1:]:
if path.endswith('.txt'): if path.endswith('.txt'):
rawtext(path) rawtext(path)
else: else:
docstrings(path) docstrings(path)

mercurial/cffi/bdiffbuild.py

from __future__ import absolute_import from __future__ import absolute_import
import cffi import cffi
import os import os
ffi = cffi.FFI() ffi = cffi.FFI()
with open(os.path.join(os.path.join(os.path.dirname(__file__), '..'),
'bdiff.c')) as f:
ffi.set_source("mercurial.cffi._bdiff", ffi.set_source("mercurial.cffi._bdiff",
open(os.path.join(os.path.join(os.path.dirname(__file__), '..'), f.read(), include_dirs=['mercurial'])
'bdiff.c')).read(), include_dirs=['mercurial'])
ffi.cdef(""" ffi.cdef("""
struct bdiff_line { struct bdiff_line {
int hash, n, e; int hash, n, e;
ssize_t len; ssize_t len;
const char *l; const char *l;
}; };
struct bdiff_hunk; struct bdiff_hunk;

mercurial/cffi/mpatchbuild.py

from __future__ import absolute_import from __future__ import absolute_import
import cffi import cffi
import os import os
ffi = cffi.FFI() ffi = cffi.FFI()
mpatch_c = os.path.join(os.path.join(os.path.dirname(__file__), '..', mpatch_c = os.path.join(os.path.join(os.path.dirname(__file__), '..',
'mpatch.c')) 'mpatch.c'))
ffi.set_source("mercurial.cffi._mpatch", open(mpatch_c).read(), with open(mpatch_c) as f:
ffi.set_source("mercurial.cffi._mpatch", f.read(),
include_dirs=["mercurial"]) include_dirs=["mercurial"])
ffi.cdef(""" ffi.cdef("""
struct mpatch_frag { struct mpatch_frag {
int start, end, len; int start, end, len;
const char *data; const char *data;
}; };
struct mpatch_flist { struct mpatch_flist {

mercurial/posix.py

fd = tempfile.NamedTemporaryFile(dir=fscheckdir, fd = tempfile.NamedTemporaryFile(dir=fscheckdir,
prefix=r'hg-checklink-') prefix=r'hg-checklink-')
target = pycompat.fsencode(os.path.basename(fd.name)) target = pycompat.fsencode(os.path.basename(fd.name))
else: else:
# create a fixed file to link to; doesn't matter if it # create a fixed file to link to; doesn't matter if it
# already exists. # already exists.
target = 'checklink-target' target = 'checklink-target'
try: try:
open(os.path.join(cachedir, target), 'w').close() fullpath = os.path.join(cachedir, target)
open(fullpath, 'w').close()
except IOError as inst: except IOError as inst:
if inst[0] == errno.EACCES: if inst[0] == errno.EACCES:
# If we can't write to cachedir, just pretend # If we can't write to cachedir, just pretend
# that the fs is readonly and by association # that the fs is readonly and by association
# that the fs won't support symlinks. This # that the fs won't support symlinks. This
# seems like the least dangerous way to avoid # seems like the least dangerous way to avoid
# data loss. # data loss.
return False return False

tests/f

isfile = os.path.isfile(f) and not islink isfile = os.path.isfile(f) and not islink
dirfiles = None dirfiles = None
content = None content = None
facts = [] facts = []
if isfile: if isfile:
if opts.type: if opts.type:
facts.append(b'file') facts.append(b'file')
if any((opts.hexdump, opts.dump, opts.md5, opts.sha1, opts.sha256)): if any((opts.hexdump, opts.dump, opts.md5, opts.sha1, opts.sha256)):
content = open(f, 'rb').read() with open(f, 'rb') as fobj:
content = fobj.read()
elif islink: elif islink:
if opts.type: if opts.type:
facts.append(b'link') facts.append(b'link')
content = os.readlink(f) content = os.readlink(f)
elif isstdin: elif isstdin:
content = getattr(sys.stdin, 'buffer', sys.stdin).read() content = getattr(sys.stdin, 'buffer', sys.stdin).read()
if opts.size: if opts.size:
facts.append(b'size=%d' % len(content)) facts.append(b'size=%d' % len(content))