From c300b6150eab9877dcfcbf45e8cbfe3391c0cd10 Mon Sep 17 00:00:00 2001 From: Lucain Date: Mon, 20 Mar 2023 13:17:27 +0100 Subject: [PATCH 1/3] Hotfix - use relative symlinks whenever possible (#1399) * Hotfix - use relative symlinks whenever possible * renaming --- src/huggingface_hub/file_download.py | 36 ++++++++++++++++++++++++---- tests/test_file_download.py | 2 +- 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/huggingface_hub/file_download.py b/src/huggingface_hub/file_download.py index acc0f8fb1a..dae2c8d3fd 100644 --- a/src/huggingface_hub/file_download.py +++ b/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 diff --git a/tests/test_file_download.py b/tests/test_file_download.py index 8ee71f656d..16f7798e2a 100644 --- a/tests/test_file_download.py +++ b/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()) From 1e3c73156b77a7371075d5b057eff071f85bbf49 Mon Sep 17 00:00:00 2001 From: Nima Boscarino Date: Wed, 22 Mar 2023 04:27:17 -0700 Subject: [PATCH 2/3] Model card template: Move model usage instructions out of Bias section (#1400) --- src/huggingface_hub/templates/modelcard_template.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/huggingface_hub/templates/modelcard_template.md b/src/huggingface_hub/templates/modelcard_template.md index f5cca406b1..ec2d18d427 100644 --- a/src/huggingface_hub/templates/modelcard_template.md +++ b/src/huggingface_hub/templates/modelcard_template.md @@ -67,7 +67,7 @@ {{ bias_recommendations | default("Users (both direct and downstream) should be made aware of the risks, biases and limitations of the model. More information needed for further recommendations.", true)}} -### How to Get Started with the Model +## How to Get Started with the Model Use the code below to get started with the model. From 8e7365da159e1f2654bdbe57641ecde0b049f405 Mon Sep 17 00:00:00 2001 From: Lucain Pouget Date: Wed, 22 Mar 2023 14:33:14 +0100 Subject: [PATCH 3/3] PR should not fail if codecov is bad --- codecov.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/codecov.yml b/codecov.yml index 3bddeacadc..8179c2170b 100644 --- a/codecov.yml +++ b/codecov.yml @@ -4,5 +4,10 @@ comment: # https://docs.codecov.com/docs/pull-request-comments#after_n_builds after_n_builds: 12 +coverage: + status: + # not in PRs + patch: false + github_checks: annotations: false