ITADN
fal-ai/fal/Issues

clone_repository fails on Windows with path handling errors

#641Openrehan-remade 创建于 2025-10-01
R
rehan-remadecommented
The `clone_repository` function in `fal/toolkit/utils/download_utils.py` fails on Windows with path-related errors. The function uses Unix-style paths and `os.rename()` which has limitations on Windows. ## Environment - OS: Windows 11 - Python: 3.11 - fal version: [current version] ## Error ```terminal [Errno 2] No such file or directory: '\\data\\.fal\\repos/tmp7v7lg3z1Hunyuan3D-Omni-db9489ee.tmp' -> '\\/data/.fal/repos/Hunyuan3D-Omni-db9489ee' Failed to clone repository 'https://github.com/Tencent-Hunyuan/Hunyuan3D-Omni.git' ``` ## Root Causes 1. **Unix-style default paths**: The default `FAL_REPOSITORY_DIR` is hardcoded as `/data/.fal/repos` (line 17), which doesn't translate properly on Windows. 2. **`os.rename()` limitations**: Line 526 uses `os.rename()` which on Windows: - Cannot rename across different drives - Fails if the destination already exists - Has issues with path separators 3. **Mixed path separators**: The error shows paths with both forward slashes and backslashes, indicating improper path normalization. ## Reproduction Steps 1. Run on Windows: ```python repo_path = clone_repository( "https://github.com/Tencent-Hunyuan/Hunyuan3D-Omni.git", commit_hash="db9489ee35589299ac34fa3ea284da50f7f7e305", include_to_path=True, ) ``` 2. The function fails with the path error shown above ## Suggested Fix Replace `os.rename()` with `shutil.move()` and properly handle Windows paths. Here's a working approach based on my workaround: ```python # At line 526, replace: os.rename(temp_dir, local_repo_path) # With: import shutil shutil.move(temp_dir, str(local_repo_path)) ``` Additionally, consider: 1. Using `Path.home() / ".fal" / "repos"` as default on Windows instead of `/data/.fal/repos` 2. Normalizing all paths with `Path()` to handle separators correctly 3. Adding Windows-specific path handling in the module ## Workaround Currently using this custom implementation that works on Windows: ```python def clone_vlm2vec_repo(commit_hash: str = "main") -> str: """Clone VLM2Vec repository with proper path handling""" import subprocess import shutil repo_dir = os.path.join("/data", ".fal", "repos", "VLM2Vec") # Check if repo already exists if os.path.exists(repo_dir) and os.path.exists(os.path.join(repo_dir, ".git")): print(f"VLM2Vec repository already exists at {repo_dir}") # Check out the specific commit try: subprocess.run(["git", "checkout", commit_hash], cwd=repo_dir, check=True, capture_output=True) print(f"Checked out commit {commit_hash}") except subprocess.CalledProcessError: print(f"Failed to checkout commit {commit_hash}, using existing checkout") return repo_dir # Clone the repository print("Cloning VLM2Vec repository...") temp_dir = f"/tmp/VLM2Vec_{uuid.uuid4().hex[:8]}" try: # Clone to temp directory first subprocess.run([ "git", "clone", "https://github.com/TIGER-AI-Lab/VLM2Vec.git", temp_dir ], check=True, capture_output=True) # Checkout specific commit subprocess.run([ "git", "checkout", commit_hash ], cwd=temp_dir, check=True, capture_output=True) # Create parent directory if it doesn't exist os.makedirs(os.path.dirname(repo_dir), exist_ok=True) # Move to final location if os.path.exists(repo_dir): shutil.rmtree(repo_dir) shutil.move(temp_dir, repo_dir) print(f"VLM2Vec repository cloned to {repo_dir}") return repo_dir except Exception as e: print(f"Error cloning repository: {e}") # Clean up temp directory if it exists if os.path.exists(temp_dir): shutil.rmtree(temp_dir) raise ``` Key differences in the workaround: - Uses `shutil.move()` instead of `os.rename()` - Handles existing directories differently - Uses `os.path.join()` for path construction ## Impact This bug prevents Windows users from using the `clone_repository` function, requiring them to implement custom workarounds.
0 条评论