( )⚙ 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
Lint Skipped
Unit
Unit Tests Skipped

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 7041

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))