From fec8a6dace001c765c4cf91303c1dc6c18e860f0 Mon Sep 17 00:00:00 2001
From: Gregory Becker <becker33@llnl.gov>
Date: Mon, 3 Apr 2023 11:54:09 -0700
Subject: [PATCH] address review: refactor for clarity

---
 lib/spack/spack/environment/environment.py | 36 ++++++++++------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py
index 09971b996f3..31a60b4d741 100644
--- a/lib/spack/spack/environment/environment.py
+++ b/lib/spack/spack/environment/environment.py
@@ -631,16 +631,13 @@ def valid_update_method_exchange(self, force):
         if not spack.util.atomic_update.renameat2():
             return False
 
-        # This is all to ensure we don't swap symlink -> exchange if we have
-        # symlink as an acceptable method. This is to avoid problems switching
-        # between OSs
-        if os.path.exists(self.root):
-            if os.path.islink(self.root):
-                if force:
-                    os.unlink(self.root)
-                    return True
-                else:
-                    return "symlink" not in self.update_method
+        # Ensure we don't swap symlink -> exchange if we have a symlink and symlink is an
+        # acceptable method. This is to avoid problems switching between OSs.
+        if os.path.islink(self.root):
+            if force:
+                os.unlink(self.root)
+            elif "symlink" in self.update_method:
+                return False
 
         return True
 
@@ -690,16 +687,15 @@ def regenerate(self, concretized_root_specs, force=False):
             tty.debug("View at %s does not need regeneration." % self.root)
             return
 
-        if update_method == "exchange":
-            if os.path.isdir(new_root):
-                # If new_root is the newest thing in its directory, no need to update
-                parent = os.path.dirname(new_root)
-                siblings = [os.path.join(parent, s) for s in os.listdir(parent)]
-                siblings.sort(reverse=True, key=lambda p: os.stat(p).st_mtime)
-                if siblings[0] == new_root:
-                    tty.debug("View at %s does not need regenration." % self.root)
-                    return
-                shutil.rmtree(new_root)
+        if update_method == "exchange" and os.path.isdir(new_root):
+            # If new_root is the newest thing in its directory, no need to update
+            parent = os.path.dirname(new_root)
+            siblings = [os.path.join(parent, s) for s in os.listdir(parent)]
+            siblings.sort(reverse=True, key=lambda p: os.stat(p).st_mtime)
+            if siblings[0] == new_root:
+                tty.debug("View at %s does not need regenration." % self.root)
+                return
+            shutil.rmtree(new_root)
 
         _error_on_nonempty_view_dir(new_root)