Fix symlink behavior (#343)

tests/base.py

@@ -186,7 +186,7 @@ def check_strings(self, command, output, expected_present, expected_absent):
             print_in_box(error_message)
             self.stop(error_message)
 
-    def setupDirs(self, test_name):
+    def setupDirs(self, test_name, follow_symlinks=False):
         """
         Set up directories for testing.
         """
@@ -223,7 +223,9 @@ def setupDirs(self, test_name):
             os.symlink("file0.txt", "{}/file0_soft.txt".format(self.test_dir))
 
         # Bad symbolic (soft) link (points to a file name which points to an inode)
-        if not os.path.lexists("{}/file0_soft_bad.txt".format(self.test_dir)):
+        if (not follow_symlinks) and (
+            not os.path.lexists("{}/file0_soft_bad.txt".format(self.test_dir))
+        ):
             # Create symbolic link pointing to test_dir/file0_that_doesnt_exist.txt
             # named test_dir/file0_soft_bad.txt
             os.symlink(
@@ -250,6 +252,7 @@ def create(
         use_hpss,
         zstash_path,
         keep=False,
+        follow_symlinks=False,
         cache=None,
         verbose=False,
         no_tars_md5=False,
@@ -268,11 +271,16 @@ def create(
             cache_option = " --cache={}".format(cache)
         else:
             cache_option = ""
+        if follow_symlinks:
+            follow_symlinks_option = " --follow-symlinks"
+        else:
+            follow_symlinks_option = ""
         v_option = " -v" if verbose else ""
         no_tars_md5_option = " --no_tars_md5" if no_tars_md5 else ""
-        cmd = "{}zstash create{}{}{}{} --hpss={} {}".format(
+        cmd = "{}zstash create{}{}{}{}{} --hpss={} {}".format(
             zstash_path,
             keep_option,
+            follow_symlinks_option,
             cache_option,
             v_option,
             no_tars_md5_option,

tests/scripts/README.md

@@ -0,0 +1 @@
+Scripts for debugging / manually testing zstash

tests/scripts/symlinks.sh

@@ -0,0 +1,44 @@
+# Test symlinks
+# Adjusted from https://github.com/E3SM-Project/zstash/issues/341
+
+follow_symlinks=true
+
+rm -rf workdir workdir2 workdir3
+mkdir workdir workdir2 workdir3
+cd workdir
+mkdir -p src/d1 src/d2
+touch src/d1/large_file.txt
+
+# This creates a symlink in d2 that links to a file in d1
+# Notice absolute path is used for source
+ln -s /home/ac.forsyth2/ez/zstash/tests/scripts/workdir/src/d1/large_file.txt src/d2/large_file.txt
+
+echo ""
+echo "ls -l  src/d2"
+ls -l  src/d2
+# symlink
+
+echo ""
+if [[ "${follow_symlinks,,}" == "true" ]]; then
+  echo "zstash create --hpss=none --follow-symlinks --cache /home/ac.forsyth2/ez/zstash/tests/scripts/workdir2 src/d2"
+  zstash create --hpss=none --follow-symlinks --cache /home/ac.forsyth2/ez/zstash/tests/scripts/workdir2 src/d2
+else
+  echo "zstash create --hpss=none --cache /home/ac.forsyth2/ez/zstash/tests/scripts/workdir2 src/d2"
+  zstash create --hpss=none --cache /home/ac.forsyth2/ez/zstash/tests/scripts/workdir2 src/d2
+fi
+
+echo ""
+echo "ls -l  src/d2"
+ls -l  src/d2
+# symlink (src is unaffected)
+
+cd ../workdir3
+echo ""
+echo "zstash extract --hpss=none --cache /home/ac.forsyth2/ez/zstash/tests/scripts/workdir2"
+zstash extract --hpss=none --cache /home/ac.forsyth2/ez/zstash/tests/scripts/workdir2
+
+cd ..
+echo ""
+echo "ls workdir3"
+ls workdir3
+# large_file.txt

tests/test_create.py

@@ -19,13 +19,14 @@ class TestCreate(TestZstash):
     """
 
     # x = on, no mark = off, b = both on and off tested
-    # option | CreateVerbose | CreateIncludeDir | CreateIncludeFile | CreateExcludeDir | CreateExcludeFile | CreateKeep | CreateCache | TestZstash.create (used in multiple tests) | TestCheckParallel.testKeepTarsWithPreviouslySetHPSS |
-    # --exclude | | | |x|x| | | | |
-    # --include | |x|x| | | | | | |
-    # --maxsize | | | | | | | | |x|
-    # --keep    | | | | | |x| |b| |
-    # --cache   | | | | | | |x| | |
-    # -v        |x| | | | | | | | |
+    # option | CreateVerbose | CreateIncludeDir | CreateIncludeFile | CreateExcludeDir | CreateExcludeFile | CreateKeep | CreateCache | CreateFollowSymlinks | TestZstash.create (used in multiple tests) | TestCheckParallel.testKeepTarsWithPreviouslySetHPSS |
+    # --exclude         | | | |x|x| | | | | |
+    # --follow-symlinks | | | | | | | |x| | |
+    # --include         | |x|x| | | | | | | |
+    # --maxsize         | | | | | | | | | |x|
+    # --keep            | | | | | |x| | |b| |
+    # --cache           | | | | | | |x|x| | |
+    # -v                |x| | | | | | | | | |
 
     def helperCreateVerbose(self, test_name, hpss_path: str, zstash_path=ZSTASH_PATH):
         """
@@ -209,6 +210,17 @@ def helperCreateCache(self, test_name, hpss_path, zstash_path=ZSTASH_PATH):
             )
             self.stop(error_message)
 
+    def helperCreateFollowSymlinks(self, test_name, zstash_path=ZSTASH_PATH):
+        """
+        Test `zstash create --hpss=none --follow-symlinks --cache=my_cache`
+        """
+        self.hpss_path = "none"
+        self.cache = "my_cache"
+        use_hpss = self.setupDirs(test_name, follow_symlinks=True)
+        self.create(use_hpss, zstash_path, follow_symlinks=True, cache=self.cache)
+        # Test that the link in the src directory remains a link (i.e., is not a copied file)
+        self.assertTrue(os.path.islink(f"{self.test_dir}/file0_soft.txt"))
+
     def testCreateVerbose(self):
         self.helperCreateVerbose("testCreateVerbose", "none")
 
@@ -252,6 +264,9 @@ def testCreateCacheHPSS(self):
         self.conditional_hpss_skip()
         self.helperCreateCache("testCreateCacheHPSS", HPSS_ARCHIVE)
 
+    def testCreateFollowSymlinks(self):
+        self.helperCreateFollowSymlinks("testCreateFollowSymlinks")
+
 
 if __name__ == "__main__":
     unittest.main()

zstash/hpss_utils.py

@@ -3,7 +3,6 @@
 import hashlib
 import os
 import os.path
-import shutil
 import sqlite3
 import tarfile
 import traceback
@@ -98,7 +97,7 @@ def add_files(
                 do_hash = False
             tarFileObject = HashIO(os.path.join(cache, tfname), "wb", do_hash)
             # FIXME: error: Argument "fileobj" to "open" has incompatible type "HashIO"; expected "Optional[IO[bytes]]"
-            tar = tarfile.open(mode="w", fileobj=tarFileObject)  # type: ignore
+            tar = tarfile.open(mode="w", fileobj=tarFileObject, dereference=follow_symlinks)  # type: ignore
 
         # Add current file to tar archive
         current_file: str = files[i]
@@ -179,10 +178,6 @@ def add_file(
 
     # FIXME: error: "TarFile" has no attribute "offset"
     offset: int = tar.offset  # type: ignore
-    if follow_symlinks and os.path.islink(file_name):
-        linked_file_name = os.path.realpath(file_name)
-        os.remove(file_name)  # Remove symbolic link and create a hard copy
-        shutil.copy(linked_file_name, file_name)
     tarinfo: tarfile.TarInfo = tar.gettarinfo(file_name)
     # Change the size of any hardlinks from 0 to the size of the actual file
     if tarinfo.islnk():