From 269ccee7eb648c3d05d576950d05d35553fd2a42 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Fri, 22 Mar 2019 10:18:37 +0100 Subject: [PATCH 1/4] enhance log_file_format to fix problem when %(name)s template value is used for log directory --- easybuild/framework/easyblock.py | 8 ++--- easybuild/tools/config.py | 54 ++++++++++++++++++++-------- easybuild/tools/package/utilities.py | 5 ++- 3 files changed, 45 insertions(+), 22 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 0884c03ce7..426f06d511 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -995,7 +995,7 @@ def make_devel_module(self, create_in_builddir=False): if create_in_builddir: output_dir = self.builddir else: - output_dir = os.path.join(self.installdir, log_path()) + output_dir = os.path.join(self.installdir, log_path(ec=self.cfg)) mkdir(output_dir, parents=True) filename = os.path.join(output_dir, ActiveMNS().det_devel_module_filename(self.cfg)) @@ -1126,7 +1126,7 @@ def make_module_extra(self, altroot=None, altversion=None): lines.append(self.module_generator.set_environment(version_envvar, altversion or self.version)) # $EBDEVEL - devel_path = os.path.join(log_path(), ActiveMNS().det_devel_module_filename(self.cfg)) + devel_path = os.path.join(log_path(ec=self.cfg), ActiveMNS().det_devel_module_filename(self.cfg)) devel_path_envvar = DEVEL_ENV_VAR_NAME_PREFIX + env_name lines.append(self.module_generator.set_environment(devel_path_envvar, devel_path, relpath=True)) @@ -2909,11 +2909,11 @@ def build_and_install_one(ecdict, init_env): if app.cfg['stop']: ended = 'STOPPED' if app.builddir is not None: - new_log_dir = os.path.join(app.builddir, config.log_path()) + new_log_dir = os.path.join(app.builddir, config.log_path(ec=app.cfg)) else: new_log_dir = os.path.dirname(app.logfile) else: - new_log_dir = os.path.join(app.installdir, config.log_path()) + new_log_dir = os.path.join(app.installdir, config.log_path(ec=app.cfg)) if build_option('read_only_installdir'): # temporarily re-enable write permissions for copying log/easyconfig to install dir adjust_permissions(new_log_dir, stat.S_IWUSR, add=True, recursive=False) diff --git a/easybuild/tools/config.py b/easybuild/tools/config.py index 67f10f7fa7..dfb49f22de 100644 --- a/easybuild/tools/config.py +++ b/easybuild/tools/config.py @@ -573,25 +573,53 @@ def get_module_syntax(): return ConfigurationVariables()['module_syntax'] -def log_file_format(return_directory=False): - """Return the format for the logfile or the directory""" +def log_file_format(return_directory=False, ec=None, date=None, timestamp=None): + """ + Return the format for the logfile or the directory + + :param ec: dict-like value that provides values for %(name)s and %(version)s template values + :param date: string representation of date to use ('%(date)s') + :param timestamp: timestamp to use ('%(time)s') + """ idx = int(not return_directory) - return ConfigurationVariables()['logfile_format'][idx] + res = ConfigurationVariables()['logfile_format'][idx] + + if ec is None: + ec = {} + + name, version = ec.get('name', '%(name)s'), ec.get('version', '%(version)s') -def log_format(): + if date is None: + date = '%(date)s' + if timestamp is None: + timestamp = '%(time)s' + + res = res % { + 'date': date, + 'name': name, + 'time': timestamp, + 'version': version, + } + + return res + + +def log_format(ec=None): """ Return the logfilename format """ # TODO needs renaming, is actually a formatter for the logfilename - return log_file_format(return_directory=False) + return log_file_format(return_directory=False, ec=ec) -def log_path(): +def log_path(ec=None): """ Return the log path """ - return log_file_format(return_directory=True) + date = time.strftime("%Y%m%d") + timestamp = time.strftime("%H%M%S") + return log_file_format(return_directory=True, ec=ec, date=date, timestamp=timestamp) def get_build_log_path(): @@ -616,17 +644,13 @@ def get_log_filename(name, version, add_salt=False, date=None, timestamp=None): :param date: string representation of date to use ('%(date)s') :param timestamp: timestamp to use ('%(time)s') """ + if date is None: date = time.strftime("%Y%m%d") if timestamp is None: timestamp = time.strftime("%H%M%S") - filename = log_file_format() % { - 'name': name, - 'version': version, - 'date': date, - 'time': timestamp, - } + filename = log_file_format(ec={'name': name, 'version': version}, date=date, timestamp=timestamp) if add_salt: salt = ''.join(random.choice(string.letters) for i in range(5)) @@ -636,8 +660,8 @@ def get_log_filename(name, version, add_salt=False, date=None, timestamp=None): filepath = os.path.join(get_build_log_path(), filename) # Append numbers if the log file already exist - counter = 1 - while os.path.isfile(filepath): + counter = 0 + while os.path.exists(filepath): counter += 1 filepath = "%s.%d" % (filepath, counter) diff --git a/easybuild/tools/package/utilities.py b/easybuild/tools/package/utilities.py index a946f6cbfc..44759b3baa 100644 --- a/easybuild/tools/package/utilities.py +++ b/easybuild/tools/package/utilities.py @@ -50,7 +50,6 @@ _log = fancylogger.getLogger('tools.package') # pylint: disable=C0103 - def avail_package_naming_schemes(): """ Returns the list of valed naming schemes @@ -131,8 +130,8 @@ def package_with_fpm(easyblock): # Excluding the EasyBuild logs and test reports that might be in the installdir exclude_files_globs = [ - os.path.join(log_path(), "*.log"), - os.path.join(log_path(), "*.md"), + os.path.join(log_path(ec=easyblock.cfg), "*.log"), + os.path.join(log_path(ec=easyblock.cfg), "*.md"), ] # stripping off leading / to match expected glob in fpm for exclude_files_glob in exclude_files_globs: From bad19f09b36a92f1b9acdb8675edc48c1de68fd9 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Fri, 22 Mar 2019 10:21:29 +0100 Subject: [PATCH 2/4] add tests for log_file_format, get_log_filename and log_path functions --- test/framework/config.py | 87 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 86 insertions(+), 1 deletion(-) diff --git a/test/framework/config.py b/test/framework/config.py index bc357e732d..0e8d3a33e2 100644 --- a/test/framework/config.py +++ b/test/framework/config.py @@ -29,6 +29,7 @@ @author: Stijn De Weirdt (Ghent University) """ import os +import re import shutil import sys import tempfile @@ -38,7 +39,8 @@ import easybuild.tools.options as eboptions from easybuild.tools import run from easybuild.tools.build_log import EasyBuildError -from easybuild.tools.config import build_option, build_path, source_paths, install_path, get_repositorypath +from easybuild.tools.config import build_option, build_path, get_log_filename, get_repositorypath +from easybuild.tools.config import install_path, log_file_format, log_path, source_paths from easybuild.tools.config import BuildOptions, ConfigurationVariables from easybuild.tools.config import DEFAULT_PATH_SUBDIRS, init_build_options from easybuild.tools.filetools import copy_dir, mkdir, write_file @@ -570,6 +572,89 @@ def test_strict(self): init_config(build_options={'strict': options.strict}) self.assertEqual(build_option('strict'), strict_val) + def test_get_log_filename(self): + """Test for get_log_filename().""" + + tmpdir = tempfile.gettempdir() + + res = get_log_filename('foo', '1.2.3') + regex = re.compile(os.path.join(tmpdir, r'easybuild-foo-1\.2\.3-[0-9]{8}\.[0-9]{6}\.log$')) + self.assertTrue(regex.match(res), "Pattern '%s' matches '%s'" % (regex.pattern, res)) + + res = get_log_filename('foo', '1.2.3', date='19700101') + regex = re.compile(os.path.join(tmpdir, r'easybuild-foo-1\.2\.3-19700101\.[0-9]{6}\.log$')) + self.assertTrue(regex.match(res), "Pattern '%s' matches '%s'" % (regex.pattern, res)) + + res = get_log_filename('foo', '1.2.3', timestamp='094651') + regex = re.compile(os.path.join(tmpdir, r'easybuild-foo-1\.2\.3-[0-9]{8}\.094651\.log$')) + self.assertTrue(regex.match(res), "Pattern '%s' matches '%s'" % (regex.pattern, res)) + + res = get_log_filename('foo', '1.2.3', date='19700101', timestamp='094651') + regex = re.compile(os.path.join(tmpdir, r'easybuild-foo-1\.2\.3-19700101\.094651\.log$')) + self.assertTrue(regex.match(res), "Pattern '%s' matches '%s'" % (regex.pattern, res)) + + # if log file already exists, numbers are added to the filename to obtain a new file path + write_file(res, '') + res = get_log_filename('foo', '1.2.3', date='19700101', timestamp='094651') + regex = re.compile(os.path.join(tmpdir, r'easybuild-foo-1\.2\.3-19700101\.094651\.log\.1$')) + self.assertTrue(regex.match(res), "Pattern '%s' matches '%s'" % (regex.pattern, res)) + + # adding salt ensures a unique filename (pretty much) + prev_log_filenames = [] + for i in range(10): + res = get_log_filename('foo', '1.2.3', date='19700101', timestamp='094651', add_salt=True) + regex = re.compile(os.path.join(tmpdir, r'easybuild-foo-1\.2\.3-19700101\.094651\.[a-zA-Z]{5}\.log$')) + self.assertTrue(regex.match(res), "Pattern '%s' matches '%s'" % (regex.pattern, res)) + self.assertTrue(res not in prev_log_filenames) + prev_log_filenames.append(res) + + def test_log_file_format(self): + """Test for log_file_format().""" + + # first test defaults -> no templating when no values are provided + self.assertEqual(log_file_format(), 'easybuild-%(name)s-%(version)s-%(date)s.%(time)s.log') + self.assertEqual(log_file_format(return_directory=True), 'easybuild') + + # test whether provided values are used to complete template + ec = {'name': 'foo', 'version': '1.2.3'} + res = log_file_format(ec=ec, date='20190322', timestamp='094356') + self.assertEqual(res, 'easybuild-foo-1.2.3-20190322.094356.log') + + res = log_file_format(return_directory=True, ec=ec, date='20190322', timestamp='094356') + self.assertEqual(res, 'easybuild') + + # partial templating is done when only some values are provided... + self.assertEqual(log_file_format(ec=ec), 'easybuild-foo-1.2.3-%(date)s.%(time)s.log') + res = log_file_format(date='20190322', timestamp='094356') + self.assertEqual(res, 'easybuild-%(name)s-%(version)s-20190322.094356.log') + + # also try with a custom setting + init_config(args=['--logfile-format=eb-%(name)s-%(date)s,log-%(version)s-%(date)s-%(time)s.out']) + self.assertEqual(log_file_format(), 'log-%(version)s-%(date)s-%(time)s.out') + self.assertEqual(log_file_format(return_directory=True), 'eb-%(name)s-%(date)s') + + res = log_file_format(ec=ec, date='20190322', timestamp='094356') + self.assertEqual(res, 'log-1.2.3-20190322-094356.out') + + res = log_file_format(return_directory=True, ec=ec, date='20190322', timestamp='094356') + self.assertEqual(res, 'eb-foo-20190322') + + def test_log_path(self): + """Test for log_path().""" + # default + self.assertEqual(log_path(), 'easybuild') + + # providing template values doesn't affect the default + ec = {'name': 'foo', 'version': '1.2.3'} + res = log_path(ec=ec) + self.assertEqual(res, 'easybuild') + + # reconfigure with value for log directory that includes templates + init_config(args=['--logfile-format=easybuild-%(name)s-%(version)s-%(date)s-%(time)s,log,txt']) + regex = re.compile(r'^easybuild-foo-1\.2\.3-[0-9-]{8}-[0-9]{6}$') + res = log_path(ec=ec) + self.assertTrue(regex.match(res), "Pattern '%s' matches '%s'" % (regex.pattern, res)) + def suite(): return TestLoaderFiltered().loadTestsFromTestCase(EasyBuildConfigTest, sys.argv[1:]) From 1f58af94587884da322afe5e078ba2df5287d112 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Fri, 22 Mar 2019 10:52:58 +0100 Subject: [PATCH 3/4] minor code cleanup in log_file_format --- easybuild/tools/config.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/easybuild/tools/config.py b/easybuild/tools/config.py index dfb49f22de..6490a22e83 100644 --- a/easybuild/tools/config.py +++ b/easybuild/tools/config.py @@ -581,10 +581,6 @@ def log_file_format(return_directory=False, ec=None, date=None, timestamp=None): :param date: string representation of date to use ('%(date)s') :param timestamp: timestamp to use ('%(time)s') """ - idx = int(not return_directory) - - res = ConfigurationVariables()['logfile_format'][idx] - if ec is None: ec = {} @@ -595,7 +591,8 @@ def log_file_format(return_directory=False, ec=None, date=None, timestamp=None): if timestamp is None: timestamp = '%(time)s' - res = res % { + idx = int(not return_directory) + res = ConfigurationVariables()['logfile_format'][idx] % { 'date': date, 'name': name, 'time': timestamp, From 884413b1c63dae8182c46a7e0fcf79d8b53b6336 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Fri, 22 Mar 2019 11:25:06 +0100 Subject: [PATCH 4/4] make sure --logfile-format setting is correct, should be a 2-tuple --- easybuild/tools/config.py | 5 +++++ test/framework/config.py | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/easybuild/tools/config.py b/easybuild/tools/config.py index 6490a22e83..35c256097a 100644 --- a/easybuild/tools/config.py +++ b/easybuild/tools/config.py @@ -591,6 +591,11 @@ def log_file_format(return_directory=False, ec=None, date=None, timestamp=None): if timestamp is None: timestamp = '%(time)s' + logfile_format = ConfigurationVariables()['logfile_format'] + if not isinstance(logfile_format, tuple) or len(logfile_format) != 2: + raise EasyBuildError("Incorrect log file format specification, should be 2-tuple (, ): %s", + logfile_format) + idx = int(not return_directory) res = ConfigurationVariables()['logfile_format'][idx] % { 'date': date, diff --git a/test/framework/config.py b/test/framework/config.py index 0e8d3a33e2..7d653b5338 100644 --- a/test/framework/config.py +++ b/test/framework/config.py @@ -639,6 +639,11 @@ def test_log_file_format(self): res = log_file_format(return_directory=True, ec=ec, date='20190322', timestamp='094356') self.assertEqual(res, 'eb-foo-20190322') + # test handling of incorrect setting for --logfile-format + init_config(args=['--logfile-format=easybuild,log.txt,thisiswrong']) + error_pattern = "Incorrect log file format specification, should be 2-tuple" + self.assertErrorRegex(EasyBuildError, error_pattern, log_file_format) + def test_log_path(self): """Test for log_path().""" # default @@ -650,10 +655,11 @@ def test_log_path(self): self.assertEqual(res, 'easybuild') # reconfigure with value for log directory that includes templates - init_config(args=['--logfile-format=easybuild-%(name)s-%(version)s-%(date)s-%(time)s,log,txt']) + init_config(args=['--logfile-format=easybuild-%(name)s-%(version)s-%(date)s-%(time)s,log.txt']) regex = re.compile(r'^easybuild-foo-1\.2\.3-[0-9-]{8}-[0-9]{6}$') res = log_path(ec=ec) self.assertTrue(regex.match(res), "Pattern '%s' matches '%s'" % (regex.pattern, res)) + self.assertEqual(log_file_format(), 'log.txt') def suite():