Create rename utility function

os.rename() fails on Windows if file already exists.

Create getuid utility function (#21736)

On Windows, replace os.getuid with ctypes.windll.shell32.IsUserAnAdmin().

Tests: Use getuid util function

Co-authored-by: lou.lawrence@kitware.com <lou.lawrence@kitware.com>
Co-authored-by: Betsy McPhail <betsy.mcphail@kitware.com>

11 files changed, 45 insertions, 23 deletions

diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py
index be24f9e53f..9ce56d91fa 100644
--- a/lib/spack/llnl/util/filesystem.py
+++ b/lib/spack/llnl/util/filesystem.py
@@ -6,6 +6,7 @@ import collections
 import errno
 import glob
 import grp
+import ctypes
 import hashlib
 import itertools
 import numbers
@@ -44,6 +45,7 @@ __all__ = [
     'fix_darwin_install_name',
     'force_remove',
     'force_symlink',
+    'getuid',
     'chgrp',
     'chmod_x',
     'copy',
@@ -60,6 +62,7 @@ __all__ = [
     'remove_directory_contents',
     'remove_if_dead_link',
     'remove_linked_tree',
+    'rename',
     'set_executable',
     'set_install_permissions',
     'touch',
@@ -71,6 +74,23 @@ __all__ = [
 ]
 
 
+def getuid():
+    if _platform == "win32":
+        if ctypes.windll.shell32.IsUserAnAdmin() == 0:
+            return 1
+        return 0
+    else:
+        return os.getuid()
+
+
+def rename(src, dst):
+    # On Windows, os.rename will fail if the destination file already exists
+    if is_windows:
+        if os.path.exists(dst):
+            os.remove(dst)
+    os.rename(src, dst)
+
+
 def path_contains_subdirectory(path, root):
     norm_root = os.path.abspath(root).rstrip(os.path.sep) + os.path.sep
     norm_path = os.path.abspath(path).rstrip(os.path.sep) + os.path.sep
@@ -293,7 +313,7 @@ def group_ids(uid=None):
         (list of int): gids of groups the user is a member of
     """
     if uid is None:
-        uid = os.getuid()
+        uid = getuid()
     user = pwd.getpwuid(uid).pw_name
     return [g.gr_gid for g in grp.getgrall() if user in g.gr_mem]
 
diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index f9e3f2125a..4048c3b192 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -45,7 +45,7 @@ from six import iteritems
 
 import llnl.util.lang
 import llnl.util.tty as tty
-from llnl.util.filesystem import mkdirp
+from llnl.util.filesystem import mkdirp, rename
 
 import spack.compilers
 import spack.paths
@@ -292,7 +292,7 @@ class SingleFileScope(ConfigScope):
             with open(tmp, 'w') as f:
                 syaml.dump_config(data_to_write, stream=f,
                                   default_flow_style=False)
-            os.rename(tmp, self.path)
+            rename(tmp, self.path)
         except (yaml.YAMLError, IOError) as e:
             raise ConfigFileError(
                 "Error writing to config file: '%s'" % str(e))
diff --git a/lib/spack/spack/database.py b/lib/spack/spack/database.py
index 3c264c5b0e..7fe1b8fcef 100644
--- a/lib/spack/spack/database.py
+++ b/lib/spack/spack/database.py
@@ -1001,7 +1001,8 @@ class Database(object):
         try:
             with open(temp_file, 'w') as f:
                 self._write_to_file(f)
-            os.rename(temp_file, self._index_path)
+            fs.rename(temp_file, self._index_path)
+
             if _use_uuid:
                 with open(self._verifier_path, 'w') as f:
                     new_verifier = str(uuid.uuid4())
diff --git a/lib/spack/spack/directory_layout.py b/lib/spack/spack/directory_layout.py
index 1351863f68..bfe7077547 100644
--- a/lib/spack/spack/directory_layout.py
+++ b/lib/spack/spack/directory_layout.py
@@ -565,7 +565,7 @@ class YamlViewExtensionsLayout(ExtensionsLayout):
             }, tmp, default_flow_style=False, encoding='utf-8')
 
         # Atomic update by moving tmpfile on top of old one.
-        os.rename(tmp.name, path)
+        fs.rename(tmp.name, path)
 
 
 class DirectoryLayoutError(SpackError):
diff --git a/lib/spack/spack/fetch_strategy.py b/lib/spack/spack/fetch_strategy.py
index bb7d446240..1204932dda 100644
--- a/lib/spack/spack/fetch_strategy.py
+++ b/lib/spack/spack/fetch_strategy.py
@@ -38,6 +38,7 @@ import llnl.util.tty as tty
 from llnl.util.filesystem import (
     get_single_file,
     mkdirp,
+    rename,
     temp_cwd,
     temp_rename,
     working_dir,
@@ -324,7 +325,7 @@ class URLFetchStrategy(FetchStrategy):
             try:
                 partial_file, save_file = self._fetch_from_url(url)
                 if save_file and (partial_file is not None):
-                    os.rename(partial_file, save_file)
+                    rename(partial_file, save_file)
                 break
             except FailedDownloadError as e:
                 errors.append(str(e))
@@ -1399,7 +1400,7 @@ class S3FetchStrategy(URLFetchStrategy):
             warn_content_type_mismatch(self.archive_file or "the archive")
 
         if self.stage.save_filename:
-            os.rename(
+            rename(
                 os.path.join(self.stage.path, basename),
                 self.stage.save_filename)
 
diff --git a/lib/spack/spack/test/install.py b/lib/spack/spack/test/install.py
index a100d23aab..85865be0a3 100644
--- a/lib/spack/spack/test/install.py
+++ b/lib/spack/spack/test/install.py
@@ -456,12 +456,12 @@ def test_pkg_build_paths(install_mockery):
 
         # Now check the newer log filename
         last_log = 'spack-build.txt'
-        os.rename(older_log, last_log)
+        fs.rename(older_log, last_log)
         assert spec.package.log_path.endswith(last_log)
 
         # Check the old environment file
         last_env = 'spack-build.env'
-        os.rename(last_log, last_env)
+        fs.rename(last_log, last_env)
         assert spec.package.env_path.endswith(last_env)
 
     # Cleanup
@@ -492,12 +492,12 @@ def test_pkg_install_paths(install_mockery):
 
         # Now check the newer install log filename
         last_log = 'build.txt'
-        os.rename(older_log, last_log)
+        fs.rename(older_log, last_log)
         assert spec.package.install_log_path.endswith(last_log)
 
         # Check the old install environment file
         last_env = 'build.env'
-        os.rename(last_log, last_env)
+        fs.rename(last_log, last_env)
         assert spec.package.install_env_path.endswith(last_env)
 
     # Cleanup
diff --git a/lib/spack/spack/test/llnl/util/lock.py b/lib/spack/spack/test/llnl/util/lock.py
index 4bb23540f6..a2c54e3968 100644
--- a/lib/spack/spack/test/llnl/util/lock.py
+++ b/lib/spack/spack/test/llnl/util/lock.py
@@ -59,7 +59,7 @@ import pytest
 
 import llnl.util.lock as lk
 import llnl.util.multiproc as mp
-from llnl.util.filesystem import touch
+from llnl.util.filesystem import getuid, touch
 
 #
 # This test can be run with MPI.  MPI is "enabled" if we can import
@@ -580,7 +580,7 @@ def test_write_lock_timeout_with_multiple_readers_3_2_ranges(lock_path):
         TimeoutWrite(lock_path, 5, 1))
 
 
-@pytest.mark.skipif(os.getuid() == 0, reason='user is root')
+@pytest.mark.skipif(getuid() == 0, reason='user is root')
 def test_read_lock_on_read_only_lockfile(lock_dir, lock_path):
     """read-only directory, read-only lockfile."""
     touch(lock_path)
diff --git a/lib/spack/spack/test/stage.py b/lib/spack/spack/test/stage.py
index 9d24bfc266..97462ac4f1 100644
--- a/lib/spack/spack/test/stage.py
+++ b/lib/spack/spack/test/stage.py
@@ -13,7 +13,7 @@ import stat
 
 import pytest
 
-from llnl.util.filesystem import mkdirp, partition_path, touch, working_dir
+from llnl.util.filesystem import getuid, mkdirp, partition_path, touch, working_dir
 
 import spack.paths
 import spack.stage
@@ -357,7 +357,7 @@ def check_stage_dir_perms(prefix, path):
 
     user = getpass.getuser()
     prefix_status = os.stat(prefix)
-    uid = os.getuid()
+    uid = getuid()
 
     # Obtain lists of ancestor and descendant paths of the $user node, if any.
     #
@@ -753,7 +753,7 @@ class TestStage(object):
 
         # The following check depends on the patched os.stat as a poor
         # substitute for confirming the generated warnings.
-        assert os.stat(user_path).st_uid != os.getuid()
+        assert os.stat(user_path).st_uid != getuid()
 
     def test_resolve_paths(self):
         """Test _resolve_paths."""
diff --git a/lib/spack/spack/test/util/spack_lock_wrapper.py b/lib/spack/spack/test/util/spack_lock_wrapper.py
index 005154f3ce..7dd0212279 100644
--- a/lib/spack/spack/test/util/spack_lock_wrapper.py
+++ b/lib/spack/spack/test/util/spack_lock_wrapper.py
@@ -8,7 +8,7 @@ import os
 
 import pytest
 
-from llnl.util.filesystem import group_ids
+from llnl.util.filesystem import getuid, group_ids
 
 import spack.config
 import spack.util.lock as lk
@@ -42,7 +42,7 @@ def test_disable_locking(tmpdir):
 @pytest.mark.nomockstage
 def test_lock_checks_user(tmpdir):
     """Ensure lock checks work with a self-owned, self-group repo."""
-    uid = os.getuid()
+    uid = getuid()
     if uid not in group_ids():
         pytest.skip("user has no group with gid == uid")
 
@@ -76,7 +76,7 @@ def test_lock_checks_user(tmpdir):
 @pytest.mark.nomockstage
 def test_lock_checks_group(tmpdir):
     """Ensure lock checks work with a self-owned, non-self-group repo."""
-    uid = os.getuid()
+    uid = getuid()
     gid = next((g for g in group_ids() if g != uid), None)
     if not gid:
         pytest.skip("user has no group with gid != uid")
diff --git a/lib/spack/spack/util/file_cache.py b/lib/spack/spack/util/file_cache.py
index 3ef46d3b6e..5f68911241 100644
--- a/lib/spack/spack/util/file_cache.py
+++ b/lib/spack/spack/util/file_cache.py
@@ -6,7 +6,7 @@
 import os
 import shutil
 
-from llnl.util.filesystem import mkdirp
+from llnl.util.filesystem import mkdirp, rename
 
 from spack.error import SpackError
 from spack.util.lock import Lock, ReadTransaction, WriteTransaction
@@ -145,7 +145,7 @@ class FileCache(object):
                     shutil.rmtree(cm.tmp_filename, True)
 
                 else:
-                    os.rename(cm.tmp_filename, cm.orig_filename)
+                    rename(cm.tmp_filename, cm.orig_filename)
 
         return WriteTransaction(
             self._get_lock(key), acquire=WriteContextManager)
diff --git a/lib/spack/spack/util/web.py b/lib/spack/spack/util/web.py
index 0f148a88f5..7786bed255 100644
--- a/lib/spack/spack/util/web.py
+++ b/lib/spack/spack/util/web.py
@@ -22,7 +22,7 @@ from six.moves.urllib.request import Request, urlopen
 
 import llnl.util.lang
 import llnl.util.tty as tty
-from llnl.util.filesystem import mkdirp
+from llnl.util.filesystem import mkdirp, rename
 
 import spack.config
 import spack.error
@@ -173,7 +173,7 @@ def push_to_url(
             shutil.copy(local_file_path, remote_file_path)
         else:
             try:
-                os.rename(local_file_path, remote_file_path)
+                rename(local_file_path, remote_file_path)
             except OSError as e:
                 if e.errno == errno.EXDEV:
                     # NOTE(opadron): The above move failed because it crosses