clone_repository fails on Windows with path handling errors
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 条评论