From fb34ca96e8870b7352036371fdf54073ae24a8d2 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Sat, 28 Nov 2020 15:45:46 +0100 Subject: [PATCH 1/2] fix regression in apply_regex_substitutions introduced in #3450: also accept list of paths to patch (fixes #3493) --- easybuild/tools/filetools.py | 64 ++++++++++++++++++++---------------- test/framework/filetools.py | 13 ++++++++ 2 files changed, 48 insertions(+), 29 deletions(-) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index 087dada195..31670bc43c 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -1333,22 +1333,26 @@ def apply_patch(patch_file, dest, fn=None, copy=False, level=None, use_git_am=Fa return True -def apply_regex_substitutions(path, regex_subs, backup='.orig.eb'): +def apply_regex_substitutions(paths, regex_subs, backup='.orig.eb'): """ Apply specified list of regex substitutions. - :param path: path to file to patch + :param paths: list of paths to files to patch (or just a single filepath) :param regex_subs: list of substitutions to apply, specified as (, ) :param backup: create backup of original file with specified suffix (no backup if value evaluates to False) """ + if isinstance(paths, string_type): + paths = [paths] + # only report when in 'dry run' mode if build_option('extended_dry_run'): - dry_run_msg("applying regex substitutions to file %s" % path, silent=build_option('silent')) - for regex, subtxt in regex_subs: - dry_run_msg(" * regex pattern '%s', replacement string '%s'" % (regex, subtxt)) + for path in paths: + dry_run_msg("applying regex substitutions to file %s" % path, silent=build_option('silent')) + for regex, subtxt in regex_subs: + dry_run_msg(" * regex pattern '%s', replacement string '%s'" % (regex, subtxt)) else: - _log.info("Applying following regex substitutions to %s: %s", path, regex_subs) + _log.info("Applying following regex substitutions to %s: %s", paths, regex_subs) for i, (regex, subtxt) in enumerate(regex_subs): regex_subs[i] = (re.compile(regex), subtxt) @@ -1359,30 +1363,32 @@ def apply_regex_substitutions(path, regex_subs, backup='.orig.eb'): # no (persistent) backup file is created if empty string value is passed to 'backup' in fileinput.input backup_ext = '' - try: - # make sure that file can be opened in text mode; - # it's possible this fails with UnicodeDecodeError when running EasyBuild with Python 3 + for path in paths: try: - with open(path, 'r') as fp: - _ = fp.read() - except UnicodeDecodeError as err: - _log.info("Encountered UnicodeDecodeError when opening %s in text mode: %s", path, err) - path_backup = back_up_file(path) - _log.info("Editing %s to strip out non-UTF-8 characters (backup at %s)", path, path_backup) - txt = read_file(path, mode='rb') - txt_utf8 = txt.decode(encoding='utf-8', errors='replace') - write_file(path, txt_utf8) - - for line_id, line in enumerate(fileinput.input(path, inplace=1, backup=backup_ext)): - for regex, subtxt in regex_subs: - match = regex.search(line) - if match: - _log.info("Replacing line %d in %s: '%s' -> '%s'", (line_id + 1), path, match.group(0), subtxt) - line = regex.sub(subtxt, line) - sys.stdout.write(line) - - except (IOError, OSError) as err: - raise EasyBuildError("Failed to patch %s: %s", path, err) + # make sure that file can be opened in text mode; + # it's possible this fails with UnicodeDecodeError when running EasyBuild with Python 3 + try: + with open(path, 'r') as fp: + _ = fp.read() + except UnicodeDecodeError as err: + _log.info("Encountered UnicodeDecodeError when opening %s in text mode: %s", path, err) + path_backup = back_up_file(path) + _log.info("Editing %s to strip out non-UTF-8 characters (backup at %s)", path, path_backup) + txt = read_file(path, mode='rb') + txt_utf8 = txt.decode(encoding='utf-8', errors='replace') + write_file(path, txt_utf8) + + for line_id, line in enumerate(fileinput.input(path, inplace=1, backup=backup_ext)): + for regex, subtxt in regex_subs: + match = regex.search(line) + if match: + origtxt = match.group(0) + _log.info("Replacing line %d in %s: '%s' -> '%s'", (line_id + 1), path, origtxt, subtxt) + line = regex.sub(subtxt, line) + sys.stdout.write(line) + + except (IOError, OSError) as err: + raise EasyBuildError("Failed to patch %s: %s", path, err) def modify_env(old, new): diff --git a/test/framework/filetools.py b/test/framework/filetools.py index 3fed4ec0e2..2ae207f793 100644 --- a/test/framework/filetools.py +++ b/test/framework/filetools.py @@ -1219,6 +1219,19 @@ def test_apply_regex_substitutions(self): self.assertTrue(txt.startswith('FOO ')) self.assertTrue(txt.endswith(' bar')) + # also test apply_regex_substitutions with a *list* of paths + # cfr. https://github.com/easybuilders/easybuild-framework/issues/3493 + test_dir = os.path.join(self.test_prefix, 'test_dir') + test_file1 = os.path.join(test_dir, 'one.txt') + test_file2 = os.path.join(test_dir, 'two.txt') + ft.write_file(test_file1, "Donald is an elephant") + ft.write_file(test_file2, "2 + 2 = 5") + regexs = [ + ('Donald', 'Dumbo'), + ('= 5', '= 4'), + ] + ft.apply_regex_substitutions([test_file1, test_file2], regexs) + def test_find_flexlm_license(self): """Test find_flexlm_license function.""" lic_file1 = os.path.join(self.test_prefix, 'one.lic') From f6ce140dce3d398c30d7a4b37eb356de73f13817 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Sat, 28 Nov 2020 16:12:58 +0100 Subject: [PATCH 2/2] better dry run output for apply_regex_substitutions + don't compile regular expressions in list that is passed in --- easybuild/tools/filetools.py | 15 ++++++++------- test/framework/filetools.py | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index 31670bc43c..dc44bfee3d 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -1346,16 +1346,17 @@ def apply_regex_substitutions(paths, regex_subs, backup='.orig.eb'): # only report when in 'dry run' mode if build_option('extended_dry_run'): - for path in paths: - dry_run_msg("applying regex substitutions to file %s" % path, silent=build_option('silent')) - for regex, subtxt in regex_subs: - dry_run_msg(" * regex pattern '%s', replacement string '%s'" % (regex, subtxt)) + paths_str = ', '.join(paths) + dry_run_msg("applying regex substitutions to file(s): %s" % paths_str, silent=build_option('silent')) + for regex, subtxt in regex_subs: + dry_run_msg(" * regex pattern '%s', replacement string '%s'" % (regex, subtxt)) else: _log.info("Applying following regex substitutions to %s: %s", paths, regex_subs) - for i, (regex, subtxt) in enumerate(regex_subs): - regex_subs[i] = (re.compile(regex), subtxt) + compiled_regex_subs = [] + for regex, subtxt in regex_subs: + compiled_regex_subs.append((re.compile(regex), subtxt)) if backup: backup_ext = backup @@ -1379,7 +1380,7 @@ def apply_regex_substitutions(paths, regex_subs, backup='.orig.eb'): write_file(path, txt_utf8) for line_id, line in enumerate(fileinput.input(path, inplace=1, backup=backup_ext)): - for regex, subtxt in regex_subs: + for regex, subtxt in compiled_regex_subs: match = regex.search(line) if match: origtxt = match.group(0) diff --git a/test/framework/filetools.py b/test/framework/filetools.py index 2ae207f793..44ae181c8a 100644 --- a/test/framework/filetools.py +++ b/test/framework/filetools.py @@ -1232,6 +1232,24 @@ def test_apply_regex_substitutions(self): ] ft.apply_regex_substitutions([test_file1, test_file2], regexs) + # also check dry run mode + init_config(build_options={'extended_dry_run': True}) + self.mock_stderr(True) + self.mock_stdout(True) + ft.apply_regex_substitutions([test_file1, test_file2], regexs) + stderr, stdout = self.get_stderr(), self.get_stdout() + self.mock_stderr(False) + self.mock_stdout(False) + + self.assertFalse(stderr) + regex = re.compile('\n'.join([ + r"applying regex substitutions to file\(s\): .*/test_dir/one.txt, .*/test_dir/two.txt", + r" \* regex pattern 'Donald', replacement string 'Dumbo'", + r" \* regex pattern '= 5', replacement string '= 4'", + '', + ])) + self.assertTrue(regex.search(stdout), "Pattern '%s' should be found in: %s" % (regex.pattern, stdout)) + def test_find_flexlm_license(self): """Test find_flexlm_license function.""" lic_file1 = os.path.join(self.test_prefix, 'one.lic')