enhance log_file_format to fix problem when %(name)s template value is used for log directory

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

easybuild/tools/config.py

@@ -573,25 +573,55 @@ 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')
+    """
+    if ec is None:
+        ec = {}
+
+    name, version = ec.get('name', '%(name)s'), ec.get('version', '%(version)s')
+
+    if date is None:
+        date = '%(date)s'
+    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 (<dir>, <filename>): %s",
+                             logfile_format)
+
     idx = int(not return_directory)
-    return ConfigurationVariables()['logfile_format'][idx]
+    res = ConfigurationVariables()['logfile_format'][idx] % {
+        'date': date,
+        'name': name,
+        'time': timestamp,
+        'version': version,
+    }
+
+    return res
 
 
-def log_format():
+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 +646,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 +662,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)
 

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:

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,95 @@ 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')
+
+        # 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
+        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))
+        self.assertEqual(log_file_format(), 'log.txt')
+
 
 def suite():
     return TestLoaderFiltered().loadTestsFromTestCase(EasyBuildConfigTest, sys.argv[1:])