Hotfix - use relative symlinks whenever possible (#1399)

* Hotfix - use relative symlinks whenever possible

* renaming

Author: Wauplin

src/huggingface_hub/file_download.py

@@ -819,14 +819,30 @@ def _normalize_etag(etag: Optional[str]) -> Optional[str]:
     return etag.strip('"')
 
 
+def _create_relative_symlink(src: str, dst: str, new_blob: bool = False) -> None:
+    """Alias method used in `transformers` conversion script."""
+    return _create_symlink(src=src, dst=dst, new_blob=new_blob)
+
+
 def _create_symlink(src: str, dst: str, new_blob: bool = False) -> None:
-    """Create a symbolic link named dst pointing to src as an absolute path.
+    """Create a symbolic link named dst pointing to src.
+
+    By default, it will try to create a symlink using a relative path. Relative paths have 2 advantages:
+    - If the cache_folder is moved (example: back-up on a shared drive), relative paths within the cache folder will
+      not brake.
+    - Relative paths seems to be better handled on Windows. Issue was reported 3 times in less than a week when
+      changing from relative to absolute paths. See https://github.com/huggingface/huggingface_hub/issues/1398,
+      https://github.com/huggingface/diffusers/issues/2729 and https://github.com/huggingface/transformers/pull/22228.
+      NOTE: The issue with absolute paths doesn't happen on admin mode.
+    When creating a symlink from the cache to a local folder, it is possible that a relative path cannot be created.
+    This happens when paths are not on the same volume. In that case, we use absolute paths.
+
 
     The result layout looks something like
         └── [ 128]  snapshots
             ├── [ 128]  2439f60ef33a0d46d85da5001d52aeda5b00ce9f
-            │   ├── [  52]  README.md -> /path/to/cache/blobs/d7edf6bd2a681fb0175f7735299831ee1b22b812
-            │   └── [  76]  pytorch_model.bin -> /path/to/cache/blobs/403450e234d65943a7dcf7e05a771ce3c92faa84dd07db4ac20f592037a1e4bd
+            │   ├── [  52]  README.md -> ../../../blobs/d7edf6bd2a681fb0175f7735299831ee1b22b812
+            │   └── [  76]  pytorch_model.bin -> ../../../blobs/403450e234d65943a7dcf7e05a771ce3c92faa84dd07db4ac20f592037a1e4bd
 
     If symlinks cannot be created on this platform (most likely to be Windows), the workaround is to avoid symlinks by
     having the actual file in `dst`. If it is a new file (`new_blob=True`), we move it to `dst`. If it is not a new file
@@ -844,6 +860,15 @@ def _create_symlink(src: str, dst: str, new_blob: bool = False) -> None:
     abs_src = os.path.abspath(os.path.expanduser(src))
     abs_dst = os.path.abspath(os.path.expanduser(dst))
 
+    # Use relative_dst in priority
+    try:
+        relative_src = os.path.relpath(abs_src, os.path.dirname(abs_dst))
+    except ValueError:
+        # Raised on Windows if src and dst are not on the same volume. This is the case when creating a symlink to a
+        # local_dir instead of within the cache directory.
+        # See https://docs.python.org/3/library/os.path.html#os.path.relpath
+        relative_src = None
+
     try:
         _support_symlinks = are_symlinks_supported(os.path.dirname(os.path.commonpath([abs_src, abs_dst])))
     except PermissionError:
@@ -852,9 +877,10 @@ def _create_symlink(src: str, dst: str, new_blob: bool = False) -> None:
         _support_symlinks = are_symlinks_supported(os.path.dirname(abs_dst))
 
     if _support_symlinks:
-        logger.info(f"Creating pointer from {abs_src} to {abs_dst}")
+        src_rel_or_abs = relative_src or abs_src
+        logger.info(f"Creating pointer from {src_rel_or_abs} to {abs_dst}")
         try:
-            os.symlink(abs_src, abs_dst)
+            os.symlink(src_rel_or_abs, abs_dst)
         except FileExistsError:
             if os.path.islink(abs_dst) and os.path.realpath(abs_dst) == os.path.realpath(abs_src):
                 # `abs_dst` already exists and is a symlink to the `abs_src` blob. It is most likely that the file has

tests/test_file_download.py

@@ -530,7 +530,7 @@ def test_with_local_dir_and_symlinks_and_file_not_cached(self) -> None:
             self.assertTrue(config_file.is_file())
             if os.name != "nt":  # File is symlink (except in Windows CI)
                 self.assertTrue(config_file.is_symlink())
-                blob_path = Path(os.readlink(str(config_file)))  # config_file.readlink() not supported on Python3.7
+                blob_path = config_file.resolve()
                 self.assertTrue(self.cache_dir in blob_path.parents)  # blob is cached!
             else:
                 self.assertFalse(config_file.is_symlink())