diff --git a/docs/assets/overview.md b/docs/assets/overview.md index 0fd9c3e632d..499fa404b4d 100644 --- a/docs/assets/overview.md +++ b/docs/assets/overview.md @@ -97,6 +97,8 @@ def my_component_with_image(): # Shared assets are linked to your app via symlinks. When using `shared=True`, the asset is symlinked from its original location to your app's external assets directory. This allows you to keep assets alongside their related code. + +On Windows, symlinks require Developer Mode or admin rights. Without them, Reflex falls back to a hardlink on the same drive, or to a copy that is refreshed on the next compile when hardlinking is unavailable, such as across drives or on filesystems without hardlink support, so shared assets work without any extra setup. ``` ## Favicon diff --git a/news/+shared-asset-windows-fallback.bugfix.md b/news/+shared-asset-windows-fallback.bugfix.md new file mode 100644 index 00000000000..405fec86460 --- /dev/null +++ b/news/+shared-asset-windows-fallback.bugfix.md @@ -0,0 +1 @@ +`rx.asset(shared=True)` no longer crashes on Windows without Developer Mode: when the platform denies symlink creation, the shared asset falls back to a hardlink on the same drive, or to a writable copy that is refreshed on the next compile when hardlinking is unavailable, such as across drives or on filesystems without hardlink support. Read-only sources are copied writable, so later compiles can still replace the asset. diff --git a/reflex/assets.py b/reflex/assets.py index 83b3d033477..af008ebc37c 100644 --- a/reflex/assets.py +++ b/reflex/assets.py @@ -1,8 +1,12 @@ """Helper functions for adding assets to the app.""" +import contextlib import hashlib import inspect import logging +import os +import shutil +import stat import time import uuid from pathlib import Path @@ -205,19 +209,31 @@ def remove_stale_external_asset_symlinks(): def _links_to(dst_file: Path, src_file: Path) -> bool: - """Check whether dst_file is already a symlink to src_file. + """Check whether dst_file already presents the src_file content. Args: dst_file: The path to inspect. - src_file: The asset the symlink should point at. + src_file: The asset the link should point at. Returns: - Whether dst_file is a symlink resolving to src_file. + Whether dst_file is a symlink resolving to src_file, a hardlink + sharing its inode, or a copy with the same content. """ try: - if not dst_file.is_symlink(): + if dst_file.is_symlink(): + resolved = dst_file.resolve() + elif dst_file.is_file() and src_file.is_file(): + # A hardlink shares the inode with the source; a copy matches by + # content. The size check keeps the edited-source case on stats + # alone; equal sizes are compared by digest, since a same-size + # rewrite that preserves the mtime must not pass as unchanged. + if dst_file.samefile(src_file): + return True + if dst_file.stat().st_size != src_file.stat().st_size: + return False + return _content_digest(dst_file) == _content_digest(src_file) + else: return False - resolved = dst_file.resolve() except (OSError, RuntimeError): # An unresolvable destination, such as the symlink loop a crashed or # racing writer can leave behind, which Python below 3.13 reports as @@ -230,27 +246,34 @@ def _links_to(dst_file: Path, src_file: Path) -> bool: def _link_shared_asset(dst_file: Path, src_file: Path) -> None: - """Point dst_file at src_file with a symlink, regardless of what is there. + """Point dst_file at src_file with a link, regardless of what is there. Several processes routinely compile into the same assets/external/ directory at once: pytest-xdist workers, parallel builds, or containers - sharing a bind mount. Linking in place would be check-then-act, so the link - is built under a unique temporary name in the destination directory and - renamed over dst_file instead, which is one atomic replace on POSIX and + sharing a bind mount. Linking in place would be check-then-act, so the + link is built under a unique temporary name in the destination directory + and renamed over dst_file instead, which is one atomic replace on POSIX and leaves no window to interleave with. Whichever process wins the race, - dst_file is a symlink to src_file once this returns. - - Windows is the exception: replacing a destination that another process is - itself replacing is denied rather than serialised, so the rename is retried - there, conceding as soon as the other process turns out to have linked the - same asset. + dst_file presents the src_file content once this returns. + + Windows is the exception twice over: replacing a destination that another + process is itself replacing is denied rather than serialised, so the rename + is retried there, conceding as soon as the other process turns out to have + linked the same asset. And without Developer Mode (or admin rights), + creating a symlink is denied outright, so the link falls back to a + hardlink, which shares the content on the same drive — unless the source + is read-only, whose read-only bit a hardlink would share and make the + destination irreplaceable — and to a writable copy when hardlinking is + unavailable at all, such as across drives or on filesystems like FAT32 + that have no hardlinks. Args: - dst_file: The symlink to create in the app's external assets directory. - src_file: The asset file the symlink should point at. + dst_file: The link to create in the app's external assets directory. + src_file: The asset file the link should point at. Raises: PermissionError: If the destination could not be replaced. + OSError: If the source could not be linked or copied at all. """ if _links_to(dst_file, src_file): # Already correct: leave it alone so file watchers see no change. @@ -262,7 +285,24 @@ def _link_shared_asset(dst_file: Path, src_file: Path) -> None: # character is more machinery than a transient name is worth. tmp_file = dst_file.with_name(f".{uuid.uuid4().hex}.tmp") try: - tmp_file.symlink_to(src_file) + try: + tmp_file.symlink_to(src_file) + except OSError: + # Symlink creation was denied (Windows without Developer Mode + # raises WinError 1314). Fall back to a hardlink, which shares the + # source content on the same drive — but not for a read-only + # source, whose read-only bit the hardlink would share and make + # the destination impossible to replace later. Copy otherwise: + # across drives, and on filesystems without hardlinks (FAT32). + source_writable = bool(src_file.stat().st_mode & stat.S_IWRITE) + if source_writable: + with contextlib.suppress(OSError): + os.link(src_file, tmp_file) + if not tmp_file.exists(): + shutil.copy2(src_file, tmp_file) + # copy2 mirrors a read-only source's mode; a read-only + # destination could never be replaced by a later compile. + tmp_file.chmod(tmp_file.stat().st_mode | stat.S_IWRITE) for attempt in range(_MAX_LINK_ATTEMPTS): try: tmp_file.replace(dst_file) diff --git a/tests/units/assets/test_assets.py b/tests/units/assets/test_assets.py index 27fc1e5003f..1ced29dd068 100644 --- a/tests/units/assets/test_assets.py +++ b/tests/units/assets/test_assets.py @@ -6,6 +6,7 @@ import os import pickle import shutil +import stat import threading from collections.abc import Callable, Generator from pathlib import Path @@ -283,6 +284,177 @@ def compile_once() -> None: assert [p.name for p in dst_file.parent.iterdir()] == ["custom_script.js"] +def _deny_symlinks(monkeypatch: pytest.MonkeyPatch) -> None: + """Simulate a platform that denies symlink creation. + + Windows without Developer Mode denies `os.symlink` outright with + WinError 1314, so the link has to fall back to a hardlink or a copy. + + Args: + monkeypatch: A pytest fixture for patching. + """ + + def denied_symlink(target, path, *args, **kwargs): + msg = f"A required privilege is not held by the client: {path!r} -> {target!r}" + raise OSError(msg) + + monkeypatch.setattr(os, "symlink", denied_symlink) + + +def test_link_shared_asset_hardlink_fallback( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Without symlink privilege, a hardlink keeps the content shared. + + Regression test: `rx.asset(shared=True)` crashed with WinError 1314 on + Windows without Developer Mode instead of linking the asset another way. + + Args: + tmp_path: A temporary directory provided by pytest. + monkeypatch: A pytest fixture for patching. + """ + src_file, dst_file = _staged_link_fixture(tmp_path) + _deny_symlinks(monkeypatch) + + _link_shared_asset(dst_file, src_file) + + assert dst_file.is_file() + assert not dst_file.is_symlink() + assert dst_file.samefile(src_file) + assert dst_file.read_text() == "script" + + # A second link is a no-op so file watchers see no change. + inode = dst_file.stat().st_ino + _link_shared_asset(dst_file, src_file) + assert dst_file.stat().st_ino == inode + + +def test_link_shared_asset_copy_fallback( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """When neither symlink nor hardlink exists, a copy keeps the app working. + + Args: + tmp_path: A temporary directory provided by pytest. + monkeypatch: A pytest fixture for patching. + """ + src_file, dst_file = _staged_link_fixture(tmp_path) + _deny_symlinks(monkeypatch) + + def denied_link(*args, **kwargs): + msg = "Invalid cross-device link" + raise OSError(msg) + + monkeypatch.setattr(os, "link", denied_link) + + _link_shared_asset(dst_file, src_file) + + assert dst_file.is_file() + assert not dst_file.is_symlink() + assert not dst_file.samefile(src_file) + assert dst_file.read_text() == "script" + + # A second link with unchanged content is a no-op: the copy is kept. + inode = dst_file.stat().st_ino + _link_shared_asset(dst_file, src_file) + assert dst_file.stat().st_ino == inode + + +def test_link_shared_asset_read_only_source_copy_is_replaceable( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A read-only source is copied writable, so later compiles can replace it. + + Regression test: copy2 mirrors the source's read-only mode, and Windows + refuses to replace a read-only destination, so the next compile would + fail. A hardlink would share the same read-only bit, so the fallback goes + straight to a writable copy. + + Args: + tmp_path: A temporary directory provided by pytest. + monkeypatch: A pytest fixture for patching. + """ + src_file, dst_file = _staged_link_fixture(tmp_path) + _deny_symlinks(monkeypatch) + src_file.chmod(src_file.stat().st_mode & ~stat.S_IWRITE) + + _link_shared_asset(dst_file, src_file) + + assert dst_file.is_file() + assert dst_file.read_text() == "script" + # The destination must be replaceable by a later compile. + assert dst_file.stat().st_mode & stat.S_IWRITE + + # An edited source is re-copied on the next compile, and the fresh copy + # is writable again even though the source stayed read-only. + src_file.chmod(src_file.stat().st_mode | stat.S_IWRITE) + src_file.write_text("updated") + src_file.chmod(src_file.stat().st_mode & ~stat.S_IWRITE) + _link_shared_asset(dst_file, src_file) + assert dst_file.read_text() == "updated" + assert dst_file.stat().st_mode & stat.S_IWRITE + + +def test_link_shared_asset_recopies_same_size_same_mtime( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A same-size rewrite that preserves the mtime still re-copies. + + Regression test: size and mtime are not content identity - restoring an + asset from an archive can rewrite the bytes while keeping the timestamp. + + Args: + tmp_path: A temporary directory provided by pytest. + monkeypatch: A pytest fixture for patching. + """ + src_file, dst_file = _staged_link_fixture(tmp_path) + _deny_symlinks(monkeypatch) + + def denied_link(*args, **kwargs): + msg = "Invalid cross-device link" + raise OSError(msg) + + monkeypatch.setattr(os, "link", denied_link) + + _link_shared_asset(dst_file, src_file) + + before = src_file.stat() + src_file.write_text("updat3", encoding="utf-8") # same length as "script" + os.utime(src_file, ns=(before.st_atime_ns, before.st_mtime_ns)) + + _link_shared_asset(dst_file, src_file) + + assert dst_file.read_text() == "updat3" + + +def test_link_shared_asset_copy_refreshes_edited_source( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """An edited source re-copies on the next compile. + + The content check in `_links_to` treats a stale copy as missing the + source's content, so the copy is replaced rather than left behind. + + Args: + tmp_path: A temporary directory provided by pytest. + monkeypatch: A pytest fixture for patching. + """ + src_file, dst_file = _staged_link_fixture(tmp_path) + _deny_symlinks(monkeypatch) + + def denied_link(*args, **kwargs): + msg = "Invalid cross-device link" + raise OSError(msg) + + monkeypatch.setattr(os, "link", denied_link) + + _link_shared_asset(dst_file, src_file) + src_file.write_text("updated") + _link_shared_asset(dst_file, src_file) + + assert dst_file.read_text() == "updated" + + @pytest.mark.parametrize( "stem", [