fixing bug that could cause to silently overwrite an existing easyconfig when using --try-*

easybuild/framework/easyconfig/easyconfig.py

@@ -790,9 +790,12 @@ def all_dependencies(self):
 
         return self._all_dependencies
 
-    def dump(self, fp):
+    def dump(self, fp, always_overwrite=True, backup=False):
         """
         Dump this easyconfig to file, with the given filename.
+
+        :param overwrite: overwrite existing file at specified location without use of --force
+        :param backup: create backup of existing file before overwriting it
         """
         orig_enable_templating = self.enable_templating
 
@@ -828,7 +831,7 @@ def dump(self, fp):
             ectxt = autopep8.fix_code(ectxt, options=autopep8_opts)
             self.log.debug("Dumped easyconfig after autopep8 reformatting: %s", ectxt)
 
-        write_file(fp, ectxt)
+        write_file(fp, ectxt, always_overwrite=always_overwrite, backup=backup, verbose=backup)
 
         self.enable_templating = orig_enable_templating
 

easybuild/framework/easyconfig/tweak.py

@@ -33,6 +33,7 @@
 :author: Toon Willems (Ghent University)
 :author: Fotis Georgatos (Uni.Lu, NTUA)
 :author: Alan O'Cais (Juelich Supercomputing Centre)
+:author: Maxime Boissonneault (Universite Laval, Calcul Quebec, Compute Canada)
 """
 import copy
 import glob
@@ -357,14 +358,7 @@ def __repr__(self):
         _log.debug("Generated file name for tweaked easyconfig file: %s", tweaked_ec)
 
     # write out tweaked easyconfig file
-    if os.path.exists(tweaked_ec):
-        if build_option('force'):
-            print_warning("Overwriting existing file at %s with tweaked easyconfig file (due to --force)", tweaked_ec)
-        else:
-            raise EasyBuildError("A file already exists at %s where tweaked easyconfig file would be written",
-                                 tweaked_ec)
-
-    write_file(tweaked_ec, ectxt)
+    write_file(tweaked_ec, ectxt, backup=True, always_overwrite=False, verbose=True)
     _log.info("Tweaked easyconfig file written to %s", tweaked_ec)
 
     return tweaked_ec
@@ -489,17 +483,17 @@ def select_or_generate_ec(fp, paths, specs):
     # TOOLCHAIN NAME
 
     # we can't rely on set, because we also need to be able to obtain a list of unique lists
-    def unique(l):
+    def unique(lst):
         """Retain unique elements in a sorted list."""
-        l = sorted(l)
-        if len(l) > 1:
-            l2 = [l[0]]
-            for x in l:
-                if not x == l2[-1]:
-                    l2.append(x)
-            return l2
+        lst = sorted(lst)
+        if len(lst) > 1:
+            res = [lst[0]]
+            for x in lst:
+                if not x == res[-1]:
+                    res.append(x)
+            return res
         else:
-            return l
+            return lst
 
     # determine list of unique toolchain names
     tcnames = unique([x[0]['toolchain']['name'] for x in ecs_and_files])
@@ -856,7 +850,8 @@ def map_easyconfig_to_target_tc_hierarchy(ec_spec, toolchain_mapping, targetdir=
     # Determine the name of the modified easyconfig and dump it to target_dir
     ec_filename = '%s-%s.eb' % (parsed_ec['ec']['name'], det_full_ec_version(parsed_ec['ec']))
     tweaked_spec = os.path.join(targetdir or tempfile.gettempdir(), ec_filename)
-    parsed_ec['ec'].dump(tweaked_spec)
+
+    parsed_ec['ec'].dump(tweaked_spec, always_overwrite=False, backup=True)
     _log.debug("Dumped easyconfig tweaked via --try-toolchain* to %s", tweaked_spec)
 
     return tweaked_spec

easybuild/tools/filetools.py

@@ -193,7 +193,7 @@ def read_file(path, log_error=True):
     return txt
 
 
-def write_file(path, txt, append=False, forced=False, backup=False):
+def write_file(path, txt, append=False, forced=False, backup=False, always_overwrite=True, verbose=False):
     """
     Write given contents to file at given path;
     overwrites current file contents without backup by default!
@@ -203,15 +203,26 @@ def write_file(path, txt, append=False, forced=False, backup=False):
     :param append: append to existing file rather than overwrite
     :param forced: force actually writing file in (extended) dry run mode
     :param backup: back up existing file before overwriting or modifying it
+    :param always_overwrite: don't require --force to overwrite an existing file
+    :param verbose: be verbose, i.e. inform where backup file was created
     """
     # early exit in 'dry run' mode
     if not forced and build_option('extended_dry_run'):
         dry_run_msg("file written: %s" % path, silent=build_option('silent'))
         return
 
-    if backup and os.path.exists(path):
-        backup = back_up_file(path)
-        _log.info("Existing file %s backed up to %s", path, backup)
+    if os.path.exists(path):
+        if not append:
+            if always_overwrite or build_option('force'):
+                _log.info("Overwriting existing file %s", path)
+            else:
+                raise EasyBuildError("File exists, not overwriting it without --force: %s", path)
+
+        if backup:
+            backed_up_fp = back_up_file(path)
+            _log.info("Existing file %s backed up to %s", path, backed_up_fp)
+            if verbose:
+                print_msg("Backup of %s created at %s" % (path, backed_up_fp))
 
     # note: we can't use try-except-finally, because Python 2.4 doesn't support it as a single block
     try:

test/framework/filetools.py

@@ -547,6 +547,34 @@ def test_read_write_file(self):
         self.assertEqual(ft.read_file(backup1), txt + txt2)
         self.assertEqual(ft.read_file(backup2), 'foo')
 
+        # tese use of 'verbose' to make write_file print location of backed up file
+        self.mock_stdout(True)
+        ft.write_file(fp, 'foo', backup=True, verbose=True)
+        stdout = self.get_stdout()
+        self.mock_stdout(False)
+        regex = re.compile("^== Backup of .*/test.txt created at .*/test.txt.bak_[0-9]*")
+        self.assertTrue(regex.search(stdout), "Pattern '%s' found in: %s" % (regex.pattern, stdout))
+
+        # by default, write_file will just blindly overwrite an already existing file
+        self.assertTrue(os.path.exists(fp))
+        ft.write_file(fp, 'blah')
+        self.assertEqual(ft.read_file(fp), 'blah')
+
+        # blind overwriting can be disabled via 'overwrite'
+        error = "File exists, not overwriting it without --force: %s" % fp
+        self.assertErrorRegex(EasyBuildError, error, ft.write_file, fp, 'blah', always_overwrite=False)
+        self.assertErrorRegex(EasyBuildError, error, ft.write_file, fp, 'blah', always_overwrite=False, backup=True)
+
+        # use of --force ensuring that file gets written regardless of whether or not it exists already
+        build_options = {'force': True}
+        init_config(build_options=build_options)
+
+        ft.write_file(fp, 'overwrittenbyforce', always_overwrite=False)
+        self.assertEqual(ft.read_file(fp), 'overwrittenbyforce')
+
+        ft.write_file(fp, 'overwrittenbyforcewithbackup', always_overwrite=False, backup=True)
+        self.assertEqual(ft.read_file(fp), 'overwrittenbyforcewithbackup')
+
         # also test behaviour of write_file under --dry-run
         build_options = {
             'extended_dry_run': True,
@@ -568,7 +596,6 @@ def test_read_write_file(self):
         self.assertTrue(os.path.exists(foo))
         self.assertEqual(ft.read_file(foo), 'bar')
 
-
     def test_det_patched_files(self):
         """Test det_patched_files function."""
         toy_patch_fn = 'toy-0.0_fix-silly-typo-in-printf-statement.patch'

test/framework/tweak.py

@@ -139,7 +139,7 @@ def test_tweak_one_version(self):
             self.assertEqual(val, tweaked_val, "Different value for %s parameter: %s vs %s" % (key, val, tweaked_val))
 
         # check behaviour if target file already exists
-        error_pattern = "A file already exists at .* where tweaked easyconfig file would be written"
+        error_pattern = "File exists, not overwriting it without --force"
         self.assertErrorRegex(EasyBuildError, error_pattern, tweak_one, toy_ec, tweaked_toy_ec, {'version': '1.2.3'})
 
         # existing file does get overwritten when --force is used