From 59622eef74a5a1c698ec71cbf2d48a308c07c1fa Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Wed, 6 Mar 2019 14:00:01 -0700 Subject: Clean up some file opens, regex strings Most recent Python (3.8 alpha) spews warnings aplenty about two subjects: unclosed files and strings which look like they have embedded escapes that Python does not recognize. The latter are usually regexes, and it provides a reminder that regular expressions should normally be specified as raw strings, so Python does not attempt to interpret them. Irritating is that even docstrings are flagged, it's not obvious what the right answer is for a docstring which contains, say, a Windows-style path with backslashes. This converts a bunch of opens that are not closed into context manager usage and regex patterns into raw strings. This eliminate about 4000 warnings spewed by Py3.8 (9200 remain). Signed-off-by: Mats Wichmann --- bin/update-release-info.py | 29 +++++++++++++++++------------ runtest.py | 20 +++++++++++++------- src/CHANGES.txt | 1 + src/engine/SCons/BuilderTests.py | 4 +++- src/engine/SCons/Defaults.py | 4 ++-- src/engine/SCons/Environment.py | 4 ++-- src/engine/SCons/Node/NodeTests.py | 4 ++-- src/engine/SCons/SConsign.py | 3 ++- src/engine/SCons/Scanner/LaTeXTests.py | 6 +++--- src/engine/SCons/Scanner/ScannerTests.py | 6 +++--- src/engine/SCons/Script/SConscript.py | 11 ++++++----- src/engine/SCons/Tool/JavaCommon.py | 4 +++- src/engine/SCons/Tool/msvs.py | 4 ++-- src/engine/SCons/Tool/msvsTests.py | 4 ++-- src/engine/SCons/Tool/swig.py | 6 ++++-- src/engine/SCons/Tool/tex.py | 26 ++++++++++++++++---------- src/engine/SCons/cpp.py | 28 ++++++++++++++-------------- src/engine/SCons/dblite.py | 7 +++++-- 18 files changed, 100 insertions(+), 71 deletions(-) diff --git a/bin/update-release-info.py b/bin/update-release-info.py index 5b871cb..fe5bbcf 100644 --- a/bin/update-release-info.py +++ b/bin/update-release-info.py @@ -81,7 +81,10 @@ else: # Get configuration information config = dict() -exec(open('ReleaseConfig').read(), globals(), config) +with open('ReleaseConfig') as f: + releaseconfig = f.read() +exec(releaseconfig, globals(), config) + try: version_tuple = config['version_tuple'] @@ -152,7 +155,8 @@ class UpdateFile(object): ''' if orig is None: orig = file try: - self.content = open(orig, 'r').read() + with open(orig, 'r') as f: + self.content = f.read() except IOError: # Couldn't open file; don't try to write anything in __del__ self.file = None @@ -180,8 +184,8 @@ class UpdateFile(object): # Determine the pattern to match a version - _rel_types = '(alpha|beta|candidate|final)' - match_pat = '\d+\.\d+\.\d+\.' + _rel_types + '\.(\d+|yyyymmdd)' + _rel_types = r'(alpha|beta|candidate|final)' + match_pat = r'\d+\.\d+\.\d+\.' + _rel_types + r'\.(\d+|yyyymmdd)' match_rel = re.compile(match_pat) def replace_version(self, replacement = version_string, count = 1): @@ -198,14 +202,14 @@ class UpdateFile(object): new_date = 'NEW DATE WILL BE INSERTED HERE' else: min = (time.daylight and time.altzone or time.timezone)//60 - hr = min//60 - min = -(min%60 + hr*100) + hr = min // 60 + min = -(min % 60 + hr * 100) new_date = (time.strftime('%a, %d %b %Y %X', release_date + (0,0,0)) + ' %+.4d' % min) - _days = '(Sun|Mon|Tue|Wed|Thu|Fri|Sat)' - _months = '(Jan|Feb|Mar|Apr|May|Jun|Jul|Aug|Sep|Oce|Nov|Dec)' - match_date = _days+', \d\d '+_months+' \d\d\d\d \d\d:\d\d:\d\d [+-]\d\d\d\d' + _days = r'(Sun|Mon|Tue|Wed|Thu|Fri|Sat)' + _months = r'(Jan|Feb|Mar|Apr|May|Jun|Jul|Aug|Sep|Oce|Nov|Dec)' + match_date = r''.join([_days, r', \d\d ', _months, r' \d\d\d\d \d\d:\d\d:\d\d [+-]\d\d\d\d']) match_date = re.compile(match_date) def replace_date(self, replacement = new_date, count = 1): @@ -220,7 +224,8 @@ class UpdateFile(object): ''' if self.file is not None and self.content != self.orig: print('Updating ' + self.file + '...') - open(self.file, 'w').write(self.content) + with open(self.file, 'w') as f: + f.write(self.content) if mode == 'post': # Set up for the next release series. @@ -309,9 +314,9 @@ t.replace_assign('default_version', repr(version_string)) t = UpdateFile('README.rst') if DEBUG: t.file = '/tmp/README.rst' -t.sub('-' + t.match_pat + '\.', '-' + version_string + '.', count = 0) +t.sub('-' + t.match_pat + r'\.', '-' + version_string + '.', count = 0) for suf in ['tar', 'win32', 'zip', 'rpm', 'exe', 'deb']: - t.sub('-(\d+\.\d+\.\d+)\.%s' % suf, + t.sub(r'-(\d+\.\d+\.\d+)\.%s' % suf, '-%s.%s' % (version_string, suf), count = 0) diff --git a/runtest.py b/runtest.py index ccc170a..2ea1a34 100755 --- a/runtest.py +++ b/runtest.py @@ -345,7 +345,7 @@ sp.append(builddir) sp.append(cwd) # -_ws = re.compile('\s') +_ws = re.compile(r'\s') def escape(s): @@ -672,12 +672,10 @@ def find_py(directory): if 'sconstest.skip' in filenames: continue try: - exclude_fp = open(os.path.join(dirpath, ".exclude_tests")) + with open(os.path.join(dirpath, ".exclude_tests")) as f: + excludes = [e.split("#", 1)[0].strip() for e in f.readlines()] except EnvironmentError: excludes = [] - else: - excludes = [ e.split('#', 1)[0].strip() - for e in exclude_fp.readlines() ] for fname in filenames: if fname.endswith(".py") and fname not in excludes: result.append(os.path.join(dirpath, fname)) @@ -685,7 +683,8 @@ def find_py(directory): if testlistfile: - tests = open(testlistfile, 'r').readlines() + with open(testlistfile, 'r') as f: + tests = f.readlines() tests = [x for x in tests if x[0] != '#'] tests = [x[:-1] for x in tests] tests = [x.strip() for x in tests] @@ -747,7 +746,8 @@ runtest.py: No tests were found. sys.exit(1) if excludelistfile: - excludetests = open(excludelistfile, 'r').readlines() + with open(excludelistfile, 'r') as f: + excludetests = f.readlines() excludetests = [x for x in excludetests if x[0] != '#'] excludetests = [x[:-1] for x in excludetests] excludetests = [x.strip() for x in excludetests] @@ -928,6 +928,12 @@ if options.xml: if options.xml != '-': f.close() +if options.output: + if isinstance(sys.stdout, Tee): + sys.stdout.file.close() + if isinstance(sys.stderr, Tee): + sys.stderr.file.close() + if len(fail): sys.exit(1) elif len(no_result): diff --git a/src/CHANGES.txt b/src/CHANGES.txt index 7d8e8d4..a736ff0 100755 --- a/src/CHANGES.txt +++ b/src/CHANGES.txt @@ -34,6 +34,7 @@ RELEASE 3.0.5.alpha.yyyymmdd - NEW DATE WILL BE INSERTED HERE - Add the textfile tool to the default tool list - Fix syntax on is/is not clauses: should not use with a literal - Properly retrieve exit code when catching SystemExit + - Fix regex patterns that were not specified as raw strings From Bernhard M. Wiedemann: - Do not store build host+user name if reproducible builds are wanted diff --git a/src/engine/SCons/BuilderTests.py b/src/engine/SCons/BuilderTests.py index c9068ed..847e30a 100644 --- a/src/engine/SCons/BuilderTests.py +++ b/src/engine/SCons/BuilderTests.py @@ -680,7 +680,9 @@ class BuilderTestCase(unittest.TestCase): def test_single_source(self): """Test Builder with single_source flag set""" def func(target, source, env): - open(str(target[0]), "w") # TODO: this just a throwaway? + """create the file""" + with open(str(target[0]), "w"): + pass if (len(source) == 1 and len(target) == 1): env['CNT'][0] = env['CNT'][0] + 1 diff --git a/src/engine/SCons/Defaults.py b/src/engine/SCons/Defaults.py index 9ca9ccd..479ef7e 100644 --- a/src/engine/SCons/Defaults.py +++ b/src/engine/SCons/Defaults.py @@ -333,8 +333,8 @@ def touch_func(dest): if os.path.exists(file): atime = os.path.getatime(file) else: - open(file, 'w') - atime = mtime + with open(file, 'w'): + atime = mtime os.utime(file, (atime, mtime)) Touch = ActionFactory(touch_func, diff --git a/src/engine/SCons/Environment.py b/src/engine/SCons/Environment.py index eea8aed..88fbc3b 100644 --- a/src/engine/SCons/Environment.py +++ b/src/engine/SCons/Environment.py @@ -1568,12 +1568,12 @@ class Base(SubstitutionEnvironment): """ filename = self.subst(filename) try: - fp = open(filename, 'r') + with open(filename, 'r') as fp: + lines = SCons.Util.LogicalLines(fp).readlines() except IOError: if must_exist: raise return - lines = SCons.Util.LogicalLines(fp).readlines() lines = [l for l in lines if l[0] != '#'] tdlist = [] for line in lines: diff --git a/src/engine/SCons/Node/NodeTests.py b/src/engine/SCons/Node/NodeTests.py index 7dc5f5d..7d347ac 100644 --- a/src/engine/SCons/Node/NodeTests.py +++ b/src/engine/SCons/Node/NodeTests.py @@ -1335,9 +1335,9 @@ class NodeListTestCase(unittest.TestCase): assert s == "['n3', 'n2', 'n1']", s r = repr(nl) - r = re.sub('at (0[xX])?[0-9a-fA-F]+', 'at 0x', r) + r = re.sub(r'at (0[xX])?[0-9a-fA-F]+', 'at 0x', r) # Don't care about ancestry: just leaf value of MyNode - r = re.sub('<.*?\.MyNode', '= 3: - v_revision = int(re.match('\d+', version[2]).group()) + v_revision = int(re.match(r'\d+', version[2]).group()) else: v_revision = 0 return v_major, v_minor, v_revision diff --git a/src/engine/SCons/Tool/JavaCommon.py b/src/engine/SCons/Tool/JavaCommon.py index 0d4c95a..853f7f2 100644 --- a/src/engine/SCons/Tool/JavaCommon.py +++ b/src/engine/SCons/Tool/JavaCommon.py @@ -358,7 +358,9 @@ if java_parsing: return self.outer_state def parse_java_file(fn, version=default_java_version): - return parse_java(open(fn, 'r').read(), version) + with open(fn, 'r') as f: + data = f.read() + return parse_java(data, version) def parse_java(contents, version=default_java_version, trace=None): """Parse a .java file and return a double of package directory, diff --git a/src/engine/SCons/Tool/msvs.py b/src/engine/SCons/Tool/msvs.py index 60ba278..f4439ba 100644 --- a/src/engine/SCons/Tool/msvs.py +++ b/src/engine/SCons/Tool/msvs.py @@ -520,7 +520,7 @@ class _DSPGenerator(object): config.cmdargs = cmdargs config.runfile = runfile - match = re.match('(.*)\|(.*)', variant) + match = re.match(r'(.*)\|(.*)', variant) if match: config.variant = match.group(1) config.platform = match.group(2) @@ -1428,7 +1428,7 @@ class _GenerateV7DSW(_DSWGenerator): def AddConfig(self, variant, dswfile=dswfile): config = Config() - match = re.match('(.*)\|(.*)', variant) + match = re.match(r'(.*)\|(.*)', variant) if match: config.variant = match.group(1) config.platform = match.group(2) diff --git a/src/engine/SCons/Tool/msvsTests.py b/src/engine/SCons/Tool/msvsTests.py index 6adc598..477694a 100644 --- a/src/engine/SCons/Tool/msvsTests.py +++ b/src/engine/SCons/Tool/msvsTests.py @@ -482,8 +482,8 @@ class DummyRegistry(object): def parse(self, data): parents = [None, None] parents[0] = self.root - keymatch = re.compile('^\[(.*)\]$') - valmatch = re.compile('^(?:"(.*)"|[@])="(.*)"$') + keymatch = re.compile(r'^\[(.*)\]$') + valmatch = re.compile(r'^(?:"(.*)"|[@])="(.*)"$') for line in data: m1 = keymatch.match(line) if m1: diff --git a/src/engine/SCons/Tool/swig.py b/src/engine/SCons/Tool/swig.py index 77cfe1d..c6abefb 100644 --- a/src/engine/SCons/Tool/swig.py +++ b/src/engine/SCons/Tool/swig.py @@ -70,7 +70,9 @@ def _find_modules(src): directors = 0 mnames = [] try: - matches = _reModule.findall(open(src).read()) + with open(src) as f: + data = f.read() + matches = _reModule.findall(data) except IOError: # If the file's not yet generated, guess the module name from the file stem matches = [] @@ -150,7 +152,7 @@ def _get_swig_version(env, swig): with pipe.stdout: out = SCons.Util.to_str(pipe.stdout.read()) - match = re.search('SWIG Version\s+(\S+).*', out, re.MULTILINE) + match = re.search(r'SWIG Version\s+(\S+).*', out, re.MULTILINE) if match: version = match.group(1) if verbose: diff --git a/src/engine/SCons/Tool/tex.py b/src/engine/SCons/Tool/tex.py index 2dfa229..d361dac 100644 --- a/src/engine/SCons/Tool/tex.py +++ b/src/engine/SCons/Tool/tex.py @@ -74,15 +74,15 @@ openout_bcf_re = re.compile(r"OUTPUT *(.*\.bcf)") #printglossary_re = re.compile(r"^[^%]*\\printglossary", re.MULTILINE) # search to find rerun warnings -warning_rerun_str = '(^LaTeX Warning:.*Rerun)|(^Package \w+ Warning:.*Rerun)' +warning_rerun_str = r'(^LaTeX Warning:.*Rerun)|(^Package \w+ Warning:.*Rerun)' warning_rerun_re = re.compile(warning_rerun_str, re.MULTILINE) # search to find citation rerun warnings -rerun_citations_str = "^LaTeX Warning:.*\n.*Rerun to get citations correct" +rerun_citations_str = r"^LaTeX Warning:.*\n.*Rerun to get citations correct" rerun_citations_re = re.compile(rerun_citations_str, re.MULTILINE) # search to find undefined references or citations warnings -undefined_references_str = '(^LaTeX Warning:.*undefined references)|(^Package \w+ Warning:.*undefined citations)' +undefined_references_str = r'(^LaTeX Warning:.*undefined references)|(^Package \w+ Warning:.*undefined citations)' undefined_references_re = re.compile(undefined_references_str, re.MULTILINE) # used by the emitter @@ -297,7 +297,8 @@ def InternalLaTeXAuxAction(XXXLaTeXAction, target = None, source= None, env=None logfilename = targetbase + '.log' logContent = '' if os.path.isfile(logfilename): - logContent = open(logfilename, "r").read() + with open(logfilename, "r") as f: + logContent = f.read() # Read the fls file to find all .aux files @@ -305,7 +306,8 @@ def InternalLaTeXAuxAction(XXXLaTeXAction, target = None, source= None, env=None flsContent = '' auxfiles = [] if os.path.isfile(flsfilename): - flsContent = open(flsfilename, "r").read() + with open(flsfilename, "r") as f: + flsContent = f.read() auxfiles = openout_aux_re.findall(flsContent) # remove duplicates dups = {} @@ -315,7 +317,8 @@ def InternalLaTeXAuxAction(XXXLaTeXAction, target = None, source= None, env=None bcffiles = [] if os.path.isfile(flsfilename): - flsContent = open(flsfilename, "r").read() + with open(flsfilename, "r") as f: + flsContent = f.read() bcffiles = openout_bcf_re.findall(flsContent) # remove duplicates dups = {} @@ -338,7 +341,8 @@ def InternalLaTeXAuxAction(XXXLaTeXAction, target = None, source= None, env=None already_bibtexed.append(auxfilename) target_aux = os.path.join(targetdir, auxfilename) if os.path.isfile(target_aux): - content = open(target_aux, "r").read() + with open(target_aux, "r") as f: + content = f.read() if content.find("bibdata") != -1: if Verbose: print("Need to run bibtex on ",auxfilename) @@ -361,7 +365,8 @@ def InternalLaTeXAuxAction(XXXLaTeXAction, target = None, source= None, env=None already_bibtexed.append(bcffilename) target_bcf = os.path.join(targetdir, bcffilename) if os.path.isfile(target_bcf): - content = open(target_bcf, "r").read() + with open(target_bcf, "r") as f: + content = f.read() if content.find("bibdata") != -1: if Verbose: print("Need to run biber on ",bcffilename) @@ -647,7 +652,7 @@ def ScanFiles(theFile, target, paths, file_tests, file_tests_search, env, graphi if incResult: aux_files.append(os.path.join(targetdir, incResult.group(1))) if Verbose: - print("\include file names : ", aux_files) + print(r"\include file names : ", aux_files) # recursively call this on each of the included files inc_files = [ ] inc_files.extend( include_re.findall(content) ) @@ -813,7 +818,8 @@ def tex_emitter_core(target, source, env, graphics_extensions): # read fls file to get all other files that latex creates and will read on the next pass # remove files from list that we explicitly dealt with above if os.path.isfile(flsfilename): - content = open(flsfilename, "r").read() + with open(flsfilename, "r") as f: + content = f.read() out_files = openout_re.findall(content) myfiles = [auxfilename, logfilename, flsfilename, targetbase+'.dvi',targetbase+'.pdf'] for filename in out_files[:]: diff --git a/src/engine/SCons/cpp.py b/src/engine/SCons/cpp.py index cdd3923..a413690 100644 --- a/src/engine/SCons/cpp.py +++ b/src/engine/SCons/cpp.py @@ -45,16 +45,16 @@ import re cpp_lines_dict = { # Fetch the rest of a #if/#elif as one argument, # with white space optional. - ('if', 'elif') : '\s*(.+)', + ('if', 'elif') : r'\s*(.+)', # Fetch the rest of a #ifdef/#ifndef as one argument, # separated from the keyword by white space. - ('ifdef', 'ifndef',): '\s+(.+)', + ('ifdef', 'ifndef',): r'\s+(.+)', # Fetch the rest of a #import/#include/#include_next line as one # argument, with white space optional. ('import', 'include', 'include_next',) - : '\s*(.+)', + : r'\s*(.+)', # We don't care what comes after a #else or #endif line. ('else', 'endif',) : '', @@ -64,10 +64,10 @@ cpp_lines_dict = { # 2) The optional parentheses and arguments (if it's a function-like # macro, '' if it's not). # 3) The expansion value. - ('define',) : '\s+([_A-Za-z][_A-Za-z0-9_]*)(\([^)]*\))?\s*(.*)', + ('define',) : r'\s+([_A-Za-z][_A-Za-z0-9_]*)(\([^)]*\))?\s*(.*)', # Fetch the #undefed keyword from a #undef line. - ('undef',) : '\s+([_A-Za-z][A-Za-z0-9_]*)', + ('undef',) : r'\s+([_A-Za-z][A-Za-z0-9_]*)', } # Create a table that maps each individual C preprocessor directive to @@ -97,7 +97,7 @@ l = [override.get(x, x) for x in list(Table.keys())] # a list of tuples, one for each preprocessor line. The preprocessor # directive will be the first element in each tuple, and the rest of # the line will be the second element. -e = '^\s*#\s*(' + '|'.join(l) + ')(.*)$' +e = r'^\s*#\s*(' + '|'.join(l) + ')(.*)$' # And last but not least, compile the expression. CPP_Expression = re.compile(e, re.M) @@ -144,12 +144,12 @@ CPP_to_Python_Ops_Expression = re.compile(expr) # A separate list of expressions to be evaluated and substituted # sequentially, not all at once. CPP_to_Python_Eval_List = [ - ['defined\s+(\w+)', '"\\1" in __dict__'], - ['defined\s*\((\w+)\)', '"\\1" in __dict__'], - ['/\*.*\*/', ''], - ['/\*.*', ''], - ['//.*', ''], - ['(0x[0-9A-Fa-f]*)[UL]+', '\\1'], + [r'defined\s+(\w+)', '"\\1" in __dict__'], + [r'defined\s*\((\w+)\)', '"\\1" in __dict__'], + [r'/\*.*\*/', ''], + [r'/\*.*', ''], + [r'//.*', ''], + [r'(0x[0-9A-Fa-f]*)[UL]+', '\\1'], ] # Replace the string representations of the regular expressions in the @@ -225,11 +225,11 @@ line_continuations = re.compile('\\\\\r?\n') # Search for a "function call" macro on an expansion. Returns the # two-tuple of the "function" name itself, and a string containing the # arguments within the call parentheses. -function_name = re.compile('(\S+)\(([^)]*)\)') +function_name = re.compile(r'(\S+)\(([^)]*)\)') # Split a string containing comma-separated function call arguments into # the separate arguments. -function_arg_separator = re.compile(',\s*') +function_arg_separator = re.compile(r',\s*') diff --git a/src/engine/SCons/dblite.py b/src/engine/SCons/dblite.py index 628182b..14bd93d 100644 --- a/src/engine/SCons/dblite.py +++ b/src/engine/SCons/dblite.py @@ -108,16 +108,19 @@ class dblite(object): self._chgrp_to = -1 # don't chgrp if self._flag == "n": - self._open(self._file_name, "wb", self._mode) + with self._open(self._file_name, "wb", self._mode): + pass # just make sure it exists else: try: f = self._open(self._file_name, "rb") except IOError as e: if self._flag != "c": raise e - self._open(self._file_name, "wb", self._mode) + with self._open(self._file_name, "wb", self._mode): + pass # just make sure it exists else: p = f.read() + f.close() if len(p) > 0: try: if bytes is not str: -- cgit v0.12