Add windows machine for CI (#1112)

* Add windows machine for CI

* add simple windows test

* run only windows tests in CI

* remove apt get in windows ci

* disable symlinks on windows machine in CI

* env variale

* fix disable_symlinks_on_windows_ci fixture

* fix fixture ?

* try to fix permission issues on windows ci

* refacto Offline repository tests

* Update use_tmp_repo behavior

* tmp work

* split between shared vs unique repo tests

* working repository tests

* use credential for repo tests

* refacto dataset repository tests

* run repo test concurrently

* fix github workflow

* fix ci

* fix ci

* echo ci

* fix ci?

* finally fix ci?g

* make test_correct_helper more robust

* delete test_correct_helper

* fix commit context manager

* robust tmp dir deletiong

* Implement more robust TemporaryDirectory

* adapt existing codebase

* cleanup at last

* code quality

* Rename to softTemporaryDirectory

* Skip test on windows + force str path in Repository

* typing

* skip symlink test on windows

* always disable symlink on windows during tests

* fix pytest conf

* xfail_on_windows decorator

* cleaner xfail_on_windows

* deactivate more tests

* another xfail

* skip umask test

* add pending questions

* other xfail

* longer but more robust test_backoff_sleep_time test

* another xfail

* Make windows-specific version of test_scan_cache_on_valid_cache

* fix file size

* 2 cache tests should fail

* resolve path in test missing cache

* test_scan_cache_last_modified_and_last_accessed should fail on Windows

* string instead of path in test_init_from_existing_local_clone

* fix \r\n issue in modelcards on windows

* preserve newlines when saving Repocard

* fix linebreak issue in test_updating_text_updates_content

* add repocard test to ensure linebreaks are kept

* fix for 3.7

* fix flag when saving

* document known issues on windows

* requested change

Author: Wauplin

.github/workflows/python-tests.yml

@@ -16,7 +16,7 @@ env:
   HUGGINGFACE_PRODUCTION_USER_TOKEN: ${{ secrets.HUGGINGFACE_PRODUCTION_USER_TOKEN }}
 
 jobs:
-  build:
+  build-ubuntu:
     runs-on: ubuntu-latest
     strategy:
       fail-fast: false
@@ -123,3 +123,41 @@ jobs:
         with:
           files: ./coverage.xml
           verbose: true
+
+  build-windows:
+    # (almost) Duplicate config compared to `build-ubuntu` but running on Windows.
+    # Please make sure to keep it updated as well.
+    # Lighter version of the tests with only 1 test suite running on Python3.7.
+    runs-on: windows-latest
+    env:
+      DISABLE_SYMLINKS_IN_WINDOWS_TESTS: 1
+    strategy:
+      fail-fast: false
+      matrix:
+        python-version: ["3.7"]
+
+    steps:
+      - uses: actions/checkout@v2
+      - name: Set up Python ${{ matrix.python-version }}
+        uses: actions/setup-python@v2
+        with:
+          python-version: ${{ matrix.python-version }}
+
+      # Install dependencies
+      - name: Configure and install dependencies
+        run: |
+          pip install --upgrade pip
+          pip install .[testing]
+        # sudo apt install -y libsndfile1-dev
+
+      # Run tests
+      - name: Run tests
+        working-directory: ./src # For code coverage to work
+        run: python -m pytest --cov=./huggingface_hub --cov-report=xml:../coverage.xml ../tests
+
+      # Upload code coverage
+      - name: Upload coverage reports to Codecov with GitHub Action
+        uses: codecov/codecov-action@v3
+        with:
+          files: ./coverage.xml
+          verbose: true

codecov.yml

@@ -2,7 +2,7 @@ comment:
   # https://docs.codecov.com/docs/pull-request-comments#requiring-changes
   require_changes: true
   # https://docs.codecov.com/docs/pull-request-comments#after_n_builds
-  after_n_builds: 11
+  after_n_builds: 12
 
 github_checks:
   annotations: false

docs/source/installation.mdx

@@ -131,6 +131,25 @@ Tags: ['pytorch', 'tf', 'jax', 'tflite', 'rust', 'safetensors', 'gpt2', 'text-ge
 Task: text-generation
 ```
 
+## Windows limitations
+
+With our goal of democratizing good ML everywhere, we built `huggingface_hub` to be a
+cross-platform library and in particular to work correctly on both Unix-based and Windows
+systems. However, there are a few cases where `huggingface_hub` has some limitations when
+run on Windows. Here is an exhaustive list of known issues. Please let us know if you
+encounter any undocumented problem by opening [an issue on Github](https://github.com/huggingface/huggingface_hub/issues/new/choose).
+
+- `huggingface_hub`'s cache system relies on symlinks to efficiently cache files downloaded
+from the Hub. On Windows, you must activate developer mode or run your script as admin to
+enable symlinks. If they are not activated, the cache-system still works but in an non-optimized
+manner. Please read [this page](./how-to-cache#limitations) for more details.
+- Filepaths on the Hub can have special characters (e.g. `"path/to?/my/file"`). Windows is
+more restrictive on [special characters](https://learn.microsoft.com/en-us/windows/win32/intl/character-sets-used-in-file-names)
+which makes it impossible to download those files on Windows. Hopefully this is a rare case.
+Please reach out to the repo owner if you think this is a mistake or to us to figure out
+a solution.
+
+
 ## Next steps
 
 Once `huggingface_hub` is properly installed on your machine, you might want

setup.cfg

@@ -60,4 +60,5 @@ max-line-length = 88
 # --durations=0          -> print execution time of each test
 addopts = -Werror::FutureWarning --log-cli-level=INFO -sv --durations=0
 env =
-    HUGGINGFACE_CO_STAGING=1
+    HUGGINGFACE_CO_STAGING=1
+    DISABLE_SYMLINKS_IN_WINDOWS_TESTS=1

src/huggingface_hub/repocard.py

@@ -29,7 +29,8 @@
 )
 
 # exact same regex as in the Hub server. Please keep in sync.
-REGEX_YAML_BLOCK = re.compile(r"^(\s*---[\n\r]+)([\S\s]*?)([\n\r]+---([\n\r]|$))")
+# See https://github.com/huggingface/moon-landing/blob/main/server/lib/ViewMarkdown.ts#L18
+REGEX_YAML_BLOCK = re.compile(r"^(\s*---[\r\n]+)([\S\s]*?)([\r\n]+---(\r\n|\n|$))")
 
 logger = get_logger(__name__)
 
@@ -127,7 +128,9 @@ def save(self, filepath: Union[Path, str]):
         """
         filepath = Path(filepath)
         filepath.parent.mkdir(parents=True, exist_ok=True)
-        filepath.write_text(str(self), encoding="utf-8")
+        # Preserve newlines as in the existing file.
+        with open(filepath, mode="w", newline="", encoding="utf-8") as f:
+            f.write(str(self))
 
     @classmethod
     def load(
@@ -516,11 +519,11 @@ def metadata_save(local_path: Union[str, Path], data: Dict) -> None:
     # try to detect existing newline character
     if os.path.exists(local_path):
         with open(local_path, "r", newline="") as readme:
-            if type(readme.newlines) is tuple:
+            content = readme.read()
+            if isinstance(readme.newlines, tuple):
                 line_break = readme.newlines[0]
-            if type(readme.newlines) is str:
+            elif isinstance(readme.newlines, str):
                 line_break = readme.newlines
-            content = readme.read()
 
     # creates a new file if it not
     with open(local_path, "w", newline="") as readme:

src/huggingface_hub/repository.py

@@ -438,7 +438,7 @@ class Repository:
     @validate_hf_hub_args
     def __init__(
         self,
-        local_dir: str,
+        local_dir: Union[str, Path],
         clone_from: Optional[str] = None,
         repo_type: Optional[str] = None,
         token: Union[bool, str] = True,
@@ -459,7 +459,7 @@ def __init__(
         or the `git_user`/`git_email` pair will be used instead.
 
         Args:
-            local_dir (`str`):
+            local_dir (`str` or `Path`):
                 path (e.g. `'my_trained_model/'`) to the local directory, where
                 the `Repository` will be initialized.
             clone_from (`str`, *optional*):
@@ -494,7 +494,8 @@ def __init__(
             - [`EnvironmentError`](https://docs.python.org/3/library/exceptions.html#EnvironmentError)
               if the remote repository set in `clone_from` does not exist.
         """
-
+        if isinstance(local_dir, Path):
+            local_dir = str(local_dir)
         os.makedirs(local_dir, exist_ok=True)
         self.local_dir = os.path.join(os.getcwd(), local_dir)
         self._repo_type = repo_type

tests/conftest.py

@@ -1,14 +1,17 @@
+import os
+import shutil
 from pathlib import Path
 from typing import Generator
 
 import pytest
 
+import huggingface_hub
 from _pytest.fixtures import SubRequest
 from huggingface_hub import HfApi, HfFolder
 from huggingface_hub.utils import SoftTemporaryDirectory
 
 from .testing_constants import ENDPOINT_PRODUCTION, PRODUCTION_TOKEN
-from .testing_utils import repo_name
+from .testing_utils import repo_name, set_write_permission_and_retry
 
 
 @pytest.fixture
@@ -28,6 +31,9 @@ def test_cache_dir(self) -> None:
     with SoftTemporaryDirectory() as cache_dir:
         request.cls.cache_dir = Path(cache_dir).resolve()
         yield
+        # TemporaryDirectory is not super robust on Windows when a git repository is
+        # cloned in it. See https://www.scivision.dev/python-tempfile-permission-error-windows/.
+        shutil.rmtree(cache_dir, onerror=set_write_permission_and_retry)
 
 
 @pytest.fixture(autouse=True, scope="session")
@@ -47,6 +53,23 @@ def clean_hf_folder_token_for_tests() -> Generator:
         HfFolder().save_token(token)
 
 
+@pytest.fixture(autouse=True)
+def disable_symlinks_on_windows_ci(monkeypatch: pytest.MonkeyPatch) -> None:
+    class FakeSymlinkDict(dict):
+        def __contains__(self, __o: object) -> bool:
+            return True  # consider any `cache_dir` to be already checked
+
+        def __getitem__(self, __key: str) -> bool:
+            return False  # symlinks are never supported
+
+    if os.name == "nt" and os.environ.get("DISABLE_SYMLINKS_IN_WINDOWS_TESTS"):
+        monkeypatch.setattr(
+            huggingface_hub.file_download,
+            "_are_symlinks_supported_in_dir",
+            FakeSymlinkDict(),
+        )
+
+
 @pytest.fixture
 def fx_production_space(request: SubRequest) -> Generator[None, None, None]:
     """Add a `repo_id` attribute referencing a Space repo on the production Hub.

tests/test_cache_layout.py

@@ -8,7 +8,12 @@
 from huggingface_hub.utils._errors import EntryNotFoundError
 
 from .testing_constants import ENDPOINT_STAGING, TOKEN, USER
-from .testing_utils import expect_deprecation, repo_name, with_production_testing
+from .testing_utils import (
+    expect_deprecation,
+    repo_name,
+    with_production_testing,
+    xfail_on_windows,
+)
 
 
 logger = logging.get_logger(__name__)
@@ -24,6 +29,7 @@ def get_file_contents(path):
 
 @with_production_testing
 class CacheFileLayoutHfHubDownload(unittest.TestCase):
+    @xfail_on_windows(reason="Symlinks are deactivated in Windows tests.")
     def test_file_downloaded_in_cache(self):
         for revision, expected_reference in (
             (None, "main"),
@@ -136,6 +142,7 @@ def test_file_download_happens_once(self):
 
             self.assertEqual(creation_time_0, creation_time_1)
 
+    @xfail_on_windows(reason="Symlinks are deactivated in Windows tests.")
     def test_file_download_happens_once_intra_revision(self):
         # Tests that a file is only downloaded once if it's not updated, even across different revisions.
 
@@ -152,6 +159,7 @@ def test_file_download_happens_once_intra_revision(self):
 
             self.assertEqual(creation_time_0, creation_time_1)
 
+    @xfail_on_windows(reason="Symlinks are deactivated in Windows tests.")
     def test_multiple_refs_for_same_file(self):
         with SoftTemporaryDirectory() as cache:
             hf_hub_download(MODEL_IDENTIFIER, "file_0.txt", cache_dir=cache)
@@ -192,6 +200,7 @@ def test_multiple_refs_for_same_file(self):
 
 @with_production_testing
 class CacheFileLayoutSnapshotDownload(unittest.TestCase):
+    @xfail_on_windows(reason="Symlinks are deactivated in Windows tests.")
     def test_file_downloaded_in_cache(self):
         with SoftTemporaryDirectory() as cache:
             snapshot_download(MODEL_IDENTIFIER, cache_dir=cache)
@@ -230,6 +239,7 @@ def test_file_downloaded_in_cache(self):
 
             self.assertTrue(all([os.path.isfile(l) for l in resolved_snapshot_links]))
 
+    @xfail_on_windows(reason="Symlinks are deactivated in Windows tests.")
     def test_file_downloaded_in_cache_several_revisions(self):
         with SoftTemporaryDirectory() as cache:
             snapshot_download(MODEL_IDENTIFIER, cache_dir=cache, revision="file-3")

tests/test_fastai_integration.py

@@ -1,5 +1,4 @@
 import os
-import shutil
 from unittest import TestCase, skip
 
 from huggingface_hub import HfApi
@@ -15,7 +14,7 @@
 )
 
 from .testing_constants import ENDPOINT_STAGING, TOKEN, USER
-from .testing_utils import expect_deprecation, repo_name, set_write_permission_and_retry
+from .testing_utils import expect_deprecation, repo_name, rmtree_with_retry
 
 
 WORKING_REPO_SUBDIR = f"fixtures/working_repo_{__name__.split('.')[-1]}"
@@ -76,7 +75,7 @@ def setUpClass(cls):
 
     def tearDown(self) -> None:
         try:
-            shutil.rmtree(WORKING_REPO_DIR, onerror=set_write_permission_and_retry)
+            rmtree_with_retry(WORKING_REPO_DIR)
         except FileNotFoundError:
             pass
 

tests/test_file_download.py

@@ -57,6 +57,7 @@
     OfflineSimulationMode,
     offline,
     with_production_testing,
+    xfail_on_windows,
 )
 
 
@@ -243,6 +244,7 @@ def test_dataset_lfs_object(self):
             (url, '"95aa6a52d5d6a735563366753ca50492a658031da74f301ac5238b03966972c9"'),
         )
 
+    @xfail_on_windows(reason="umask is UNIX-specific")
     def test_hf_hub_download_custom_cache_permission(self):
         """Checks `hf_hub_download` respect the cache dir permission.
 
@@ -473,13 +475,15 @@ def test_hf_hub_url_on_awful_subfolder_and_filename(self):
             self.expected_url,
         )
 
+    @xfail_on_windows(reason="Windows paths cannot contain a '?'.")
     def test_hf_hub_download_on_awful_filepath(self):
         local_path = hf_hub_download(
             self.repo_id, self.filepath, cache_dir=self.cache_dir
         )
         # Local path is not url-encoded
         self.assertTrue(local_path.endswith(self.filepath))
 
+    @xfail_on_windows(reason="Windows paths cannot contain a '?'.")
     def test_hf_hub_download_on_awful_subfolder_and_filename(self):
         local_path = hf_hub_download(
             self.repo_id,
@@ -492,6 +496,7 @@ def test_hf_hub_download_on_awful_subfolder_and_filename(self):
 
 
 class CreateSymlinkTest(unittest.TestCase):
+    @unittest.skipIf(os.name == "nt", "No symlinks on Windows")
     @patch("huggingface_hub.file_download.are_symlinks_supported")
     def test_create_relative_symlink_concurrent_access(
         self, mock_are_symlinks_supported: Mock