From 60f735de479e4c52bb24d05b02abdb877d78d47c Mon Sep 17 00:00:00 2001 From: Robin Huang Date: Thu, 29 Aug 2024 21:51:12 -0700 Subject: [PATCH] Adding mac / windows specific tests for standalone command. (#169) * Add test for install and standalone on windows. * Test in venv. * Skip Manager. * Fix activate command. * Update. * Update. * improve platform system determination * `DependencyCompiler`: add `reqs` parameter to `.Download` and `.Wheel` methods * refactored tarball creation/extraction to use `create_tarball`/`extract_tarball` * skip uv wheel when dehydrating standalone python on windows * small fixup to joining python standalone download url * improve parsing of reqs from reqFile * add `tarfile.data_filter` to all tar filters to address secruity audit * revert tar security fix, since `tarfile.data_filter` is busted in many python versions - see: https://github.com/python/cpython/issues/107845 * add numpy<2 override on windows --------- Co-authored-by: telamonian --- .github/workflows/run-on-gpu.yml | 2 + .github/workflows/test-mac.yml | 32 +++++++ .github/workflows/test-windows.yml | 34 +++++++ comfy_cli/standalone.py | 77 ++++------------ comfy_cli/utils.py | 141 ++++++++++++++++++++++++++--- comfy_cli/uv.py | 90 +++++++++++++----- 6 files changed, 282 insertions(+), 94 deletions(-) create mode 100644 .github/workflows/test-mac.yml create mode 100644 .github/workflows/test-windows.yml diff --git a/.github/workflows/run-on-gpu.yml b/.github/workflows/run-on-gpu.yml index fe35995..a87ec3c 100644 --- a/.github/workflows/run-on-gpu.yml +++ b/.github/workflows/run-on-gpu.yml @@ -7,12 +7,14 @@ on: paths: - "comfy_cli/**" - "!comfy_cli/test_**" + - "!.github/**" pull_request: branches: - main paths: - "comfy_cli/**" - "!comfy_cli/test_**" + - "!.github/**" jobs: test-cli-gpu: diff --git a/.github/workflows/test-mac.yml b/.github/workflows/test-mac.yml new file mode 100644 index 0000000..c9f4800 --- /dev/null +++ b/.github/workflows/test-mac.yml @@ -0,0 +1,32 @@ +name: "Mac Specific Commands" +on: + pull_request: + branches: + - main + +jobs: + test: + runs-on: macos-latest + env: + PYTHONIOENCODING: "utf8" + + steps: + - name: Check out code + uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: 3.12 + + - name: Install Dependencies + run: | + python -m venv venv + source venv/bin/activate + python -m pip install --upgrade pip + pip install pytest + pip install -e . + comfy --skip-prompt --workspace ./ComfyUI install --fast-deps --m-series --skip-manager + comfy --workspace ./ComfyUI standalone --platform macos + comfy standalone --rehydrate + ./python/bin/python ComfyUI/main.py --cpu --quick-test-for-ci diff --git a/.github/workflows/test-windows.yml b/.github/workflows/test-windows.yml new file mode 100644 index 0000000..66f0488 --- /dev/null +++ b/.github/workflows/test-windows.yml @@ -0,0 +1,34 @@ +name: "Windows Specific Commands" +on: + pull_request: + branches: + - main + +jobs: + test: + runs-on: windows-latest + env: + PYTHONIOENCODING: "utf8" + + steps: + - name: Check out code + uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: 3.12 + + - name: Install Dependencies + run: | + python -m venv venv + .\venv\Scripts\Activate.ps1 + Get-Command python + python -m pip install --upgrade pip + pip install pytest + pip install -e . + comfy --skip-prompt --workspace ./ComfyUI install --fast-deps --nvidia --cuda-version 12.1 --skip-manager + comfy --workspace ./ComfyUI standalone --platform windows --proc x86_64 + ls + comfy standalone --rehydrate --platform windows --proc x86_64 + ./python/python.exe ComfyUI/main.py --cpu --quick-test-for-ci diff --git a/comfy_cli/standalone.py b/comfy_cli/standalone.py index 414cf9d..c40861e 100644 --- a/comfy_cli/standalone.py +++ b/comfy_cli/standalone.py @@ -1,18 +1,13 @@ -import platform as os_platform import shutil import subprocess -import tarfile from pathlib import Path from typing import Optional import requests -from rich.live import Live -from rich.progress import Progress, TextColumn -from rich.table import Table from comfy_cli.constants import OS, PROC from comfy_cli.typing import PathLike -from comfy_cli.utils import download_progress, get_os, get_proc +from comfy_cli.utils import create_tarball, download_url, extract_tarball, get_os, get_proc from comfy_cli.uv import DependencyCompiler _here = Path(__file__).expanduser().resolve().parent @@ -37,6 +32,7 @@ def download_standalone_python( tag: str = "latest", flavor: str = "install_only", cwd: PathLike = ".", + show_progress: bool = True, ) -> PathLike: """grab a pre-built distro from the python-build-standalone project. See https://gregoryszorc.com/docs/python-build-standalone/main/""" @@ -59,9 +55,9 @@ def download_standalone_python( name = f"cpython-{version}+{tag}-{target}-{flavor}" fname = f"{name}.tar.gz" - url = f"{asset_url_prefix}/{fname}" + url = f"{asset_url_prefix.rstrip('/')}/{fname.lstrip('/')}" - return download_progress(url, fname, cwd=cwd) + return download_url(url, fname, cwd=cwd, show_progress=show_progress) class StandalonePython: @@ -74,7 +70,8 @@ def FromDistro( flavor: str = "install_only", cwd: PathLike = ".", name: PathLike = "python", - ): + show_progress: bool = True, + ) -> "StandalonePython": fpath = download_standalone_python( platform=platform, proc=proc, @@ -82,35 +79,23 @@ def FromDistro( tag=tag, flavor=flavor, cwd=cwd, + show_progress=show_progress, ) return StandalonePython.FromTarball(fpath, name) @staticmethod - def FromTarball(fpath: PathLike, name: PathLike = "python") -> "StandalonePython": + def FromTarball(fpath: PathLike, name: PathLike = "python", show_progress: bool = True) -> "StandalonePython": fpath = Path(fpath) - - with tarfile.open(fpath) as tar: - info = tar.next() - old_name = info.name.split("/")[0] - - old_rpath = fpath.parent / old_name rpath = fpath.parent / name - # clean the tar file expand target and the final target - shutil.rmtree(old_rpath, ignore_errors=True) - shutil.rmtree(rpath, ignore_errors=True) - - with tarfile.open(fpath) as tar: - tar.extractall() - - shutil.move(old_rpath, rpath) + extract_tarball(inPath=fpath, outPath=rpath, show_progress=show_progress) return StandalonePython(rpath=rpath) def __init__(self, rpath: PathLike): self.rpath = Path(rpath) self.name = self.rpath.name - if os_platform.system() == "Windows": + if get_os() == OS.WINDOWS: self.bin = self.rpath self.executable = self.bin / "python.exe" else: @@ -170,7 +155,9 @@ def dehydrate_comfy_deps( extraSpecs=extraSpecs, ) self.dep_comp.compile_deps() - self.dep_comp.fetch_dep_wheels() + + skip_uv = get_os() == OS.WINDOWS + self.dep_comp.fetch_dep_wheels(skip_uv=skip_uv) def rehydrate_comfy_deps(self): self.dep_comp = DependencyCompiler( @@ -178,40 +165,8 @@ def rehydrate_comfy_deps(self): ) self.dep_comp.install_wheels_directly() - def to_tarball(self, outPath: Optional[PathLike] = None, progress: bool = True): - outPath = self.rpath.with_suffix(".tgz") if outPath is None else Path(outPath) - - # do a little clean up prep - outPath.unlink(missing_ok=True) + def to_tarball(self, outPath: Optional[PathLike] = None, show_progress: bool = True): + # remove any __pycache__ before creating archive self.clean() - if progress: - fileSize = sum(f.stat().st_size for f in self.rpath.glob("**/*")) - - barProg = Progress() - addTar = barProg.add_task("[cyan]Creating tarball...", total=fileSize) - pathProg = Progress(TextColumn("{task.description}")) - pathTar = pathProg.add_task("") - - progress_table = Table.grid() - progress_table.add_row(barProg) - progress_table.add_row(pathProg) - - _size = 0 - - def _filter(tinfo: tarfile.TarInfo): - nonlocal _size - pathProg.update(pathTar, description=tinfo.path) - barProg.advance(addTar, _size) - _size = Path(tinfo.path).stat().st_size - return tinfo - else: - _filter = None - - with Live(progress_table, refresh_per_second=10): - with tarfile.open(outPath, "w:gz") as tar: - tar.add(self.rpath.relative_to(Path(".").expanduser().resolve()), filter=_filter) - - if progress: - barProg.advance(addTar, _size) - pathProg.update(pathTar, description="") + create_tarball(inPath=self.rpath, outPath=outPath, show_progress=show_progress) diff --git a/comfy_cli/utils.py b/comfy_cli/utils.py index 586f022..cbd024f 100644 --- a/comfy_cli/utils.py +++ b/comfy_cli/utils.py @@ -6,13 +6,16 @@ import platform import shutil import subprocess -import sys +import tarfile from pathlib import Path +from typing import Optional import psutil import requests import typer from rich import print, progress +from rich.live import Live +from rich.table import Table from comfy_cli.constants import DEFAULT_COMFY_WORKSPACE, OS, PROC from comfy_cli.typing import PathLike @@ -39,12 +42,16 @@ def get_instance(*args, **kwargs): def get_os(): - if sys.platform == "darwin": + platform_system = platform.system().lower() + + if platform_system == "darwin": return OS.MACOS - elif "win" in sys.platform: + elif platform_system == "windows": return OS.WINDOWS - - return OS.LINUX + elif platform_system == "linux": + return OS.LINUX + else: + raise ValueError(f"Running on unsupported os {platform.system()}") def get_proc(): @@ -97,7 +104,13 @@ def f(incomplete: str) -> list[str]: return f -def download_progress(url: str, fname: PathLike, cwd: PathLike = ".", allow_redirects: bool = True) -> PathLike: +def download_url( + url: str, + fname: PathLike, + cwd: PathLike = ".", + allow_redirects: bool = True, + show_progress: bool = True, +) -> PathLike: """download url to local file fname and show a progress bar. See https://stackoverflow.com/q/37573483""" cwd = Path(cwd).expanduser().resolve() @@ -107,12 +120,118 @@ def download_progress(url: str, fname: PathLike, cwd: PathLike = ".", allow_redi if response.status_code != 200: response.raise_for_status() # Will only raise for 4xx codes, so... raise RuntimeError(f"Request to {url} returned status code {response.status_code}") - fsize = int(response.headers.get("Content-Length", 0)) - desc = "(Unknown total file size)" if fsize == 0 else "" response.raw.read = functools.partial(response.raw.read, decode_content=True) # Decompress if needed - with progress.wrap_file(response.raw, total=fsize, description=desc) as response_raw: - with fpath.open("wb") as f: - shutil.copyfileobj(response_raw, f) + with fpath.open("wb") as f: + if show_progress: + fsize = int(response.headers.get("Content-Length", 0)) + desc = f"downloading {fname}..." + ("(Unknown total file size)" if fsize == 0 else "") + + with progress.wrap_file(response.raw, total=fsize, description=desc) as response_raw: + shutil.copyfileobj(response_raw, f) + else: + shutil.copyfileobj(response.raw, f) return fpath + + +def extract_tarball( + inPath: PathLike, + outPath: Optional[PathLike] = None, + show_progress: bool = True, +): + inPath = Path(inPath).expanduser().resolve() + outPath = inPath.with_suffix("") if outPath is None else Path(outPath).expanduser().resolve() + + with tarfile.open(inPath) as tar: + info = tar.next() + old_name = info.name.split("/")[0] + # path to top-level of extraction result + extractPath = inPath.with_name(old_name) + + # clean both the extraction path and the final target path + shutil.rmtree(extractPath, ignore_errors=True) + shutil.rmtree(outPath, ignore_errors=True) + + if show_progress: + fileSize = inPath.stat().st_size + + barProg = progress.Progress() + barTask = barProg.add_task("[cyan]extracting tarball...", total=fileSize) + pathProg = progress.Progress(progress.TextColumn("{task.description}")) + pathTask = pathProg.add_task("") + + progress_table = Table.grid() + progress_table.add_row(barProg) + progress_table.add_row(pathProg) + + _size = 0 + + def _filter(tinfo: tarfile.TarInfo, _path: PathLike): + nonlocal _size + pathProg.update(pathTask, description=tinfo.path) + barProg.advance(barTask, _size) + _size = tinfo.size + + # TODO: ideally we'd use data_filter here, but it's busted: https://github.com/python/cpython/issues/107845 + # return tarfile.data_filter(tinfo, _path) + return tinfo + else: + _filter = None + + with Live(progress_table, refresh_per_second=10): + with tarfile.open(inPath) as tar: + tar.extractall(filter=_filter) + + if show_progress: + barProg.advance(barTask, _size) + pathProg.update(pathTask, description="") + + shutil.move(extractPath, outPath) + + +def create_tarball( + inPath: PathLike, + outPath: Optional[PathLike] = None, + cwd: Optional[PathLike] = None, + show_progress: bool = True, +): + cwd = Path("." if cwd is None else cwd).expanduser().resolve() + inPath = Path(inPath).expanduser().resolve() + outPath = inPath.with_suffix(".tgz") if outPath is None else Path(outPath).expanduser().resolve() + + # clean the archive target path + outPath.unlink(missing_ok=True) + + if show_progress: + fileSize = sum(f.stat().st_size for f in inPath.glob("**/*")) + + barProg = progress.Progress() + barTask = barProg.add_task("[cyan]creating tarball...", total=fileSize) + pathProg = progress.Progress(progress.TextColumn("{task.description}")) + pathTask = pathProg.add_task("") + + progress_table = Table.grid() + progress_table.add_row(barProg) + progress_table.add_row(pathProg) + + _size = 0 + + def _filter(tinfo: tarfile.TarInfo): + nonlocal _size + pathProg.update(pathTask, description=tinfo.path) + barProg.advance(barTask, _size) + _size = Path(tinfo.path).stat().st_size + + return tinfo + else: + _filter = None + + with Live(progress_table, refresh_per_second=10): + with tarfile.open(outPath, "w:gz") as tar: + # don't include parent paths in archive + tar.add(inPath.relative_to(cwd), filter=_filter) + + if show_progress: + barProg.advance(barTask, _size) + pathProg.update(pathTask, description="") diff --git a/comfy_cli/uv.py b/comfy_cli/uv.py index f807b77..8bfcd85 100644 --- a/comfy_cli/uv.py +++ b/comfy_cli/uv.py @@ -7,8 +7,9 @@ from typing import Any, Optional, Union, cast from comfy_cli import ui -from comfy_cli.constants import GPU_OPTION +from comfy_cli.constants import GPU_OPTION, OS from comfy_cli.typing import PathLike +from comfy_cli.utils import get_os def _run(cmd: list[str], cwd: PathLike, check: bool = True) -> subprocess.CompletedProcess[Any]: @@ -43,6 +44,26 @@ def parse_uv_compile_error(err: str) -> tuple[str, list[str]]: return reqName, cast(list[str], reqRe.findall(err)) +def parse_req_file(rf: PathLike, skips: Optional[list[str]] = None): + skips = [] if skips is None else skips + + reqs: list[str] = [] + opts: list[str] = [] + with open(rf) as f: + for line in f: + line = line.strip() + if not line or line.startswith("#"): + continue + elif "==" in line and line.split("==")[0] in skips: + continue + elif line.startswith("--"): + opts.extend(line.split()) + else: + reqs.append(line) + + return opts + reqs + + class DependencyCompiler: rocmPytorchUrl = "https://download.pytorch.org/whl/rocm6.1" nvidiaPytorchUrl = "https://download.pytorch.org/whl/cu121" @@ -52,6 +73,7 @@ class DependencyCompiler: # ensure usage of {gpu} version of pytorch --extra-index-url {gpuUrl} torch + torchaudio torchsde torchvision """ @@ -234,11 +256,12 @@ def Sync( @staticmethod def Download( cwd: PathLike, - reqFile: list[PathLike], executable: PathLike = sys.executable, extraUrl: Optional[str] = None, noDeps: bool = False, out: Optional[PathLike] = None, + reqs: Optional[list[str]] = None, + reqFile: Optional[list[PathLike]] = None, ) -> subprocess.CompletedProcess[Any]: """For now, the `download` cmd has no uv support, so use pip""" cmd = [ @@ -248,12 +271,6 @@ def Download( "download", ] - if isinstance(reqFile, (str, Path)): - cmd.extend(["-r", str(reqFile)]) - elif isinstance(reqFile, list): - for rf in reqFile: - cmd.extend(["-r", str(rf)]) - if extraUrl is not None: cmd.extend(["--extra-index-url", extraUrl]) @@ -263,25 +280,32 @@ def Download( if out is not None: cmd.extend(["-d", str(out)]) + if reqs is not None: + cmd.extend(reqs) + + if reqFile is not None: + for rf in reqFile: + cmd.extend(["--requirement", rf]) + return _check_call(cmd, cwd) @staticmethod def Wheel( cwd: PathLike, - reqFile: list[PathLike], executable: PathLike = sys.executable, extraUrl: Optional[str] = None, noDeps: bool = False, out: Optional[PathLike] = None, + reqs: Optional[list[str]] = None, + reqFile: Optional[list[PathLike]] = None, ) -> subprocess.CompletedProcess[Any]: """For now, the `wheel` cmd has no uv support, so use pip""" - cmd = [str(executable), "-m", "pip", "wheel"] - - if isinstance(reqFile, (str, Path)): - cmd.extend(["-r", str(reqFile)]) - elif isinstance(reqFile, list): - for rf in reqFile: - cmd.extend(["-r", str(rf)]) + cmd = [ + str(executable), + "-m", + "pip", + "wheel", + ] if extraUrl is not None: cmd.extend(["--extra-index-url", extraUrl]) @@ -292,6 +316,13 @@ def Wheel( if out is not None: cmd.extend(["-w", str(out)]) + if reqs is not None: + cmd.extend(reqs) + + if reqFile is not None: + for rf in reqFile: + cmd.extend(["--requirement", rf]) + return _check_call(cmd, cwd) @staticmethod @@ -368,6 +399,11 @@ def make_override(self): f.write(DependencyCompiler.overrideGpu.format(gpu=self.gpu, gpuUrl=self.gpuUrl)) f.write("\n\n") + # TODO: remove numpy<2 override once torch is compatible with numpy>=2 + if get_os() == OS.WINDOWS: + f.write("numpy<2\n") + f.write("\n\n") + completed = DependencyCompiler.Compile( cwd=self.cwd, reqFiles=self.reqFilesCore, @@ -487,22 +523,32 @@ def sync_core_plus_ext(self): extraUrl=self.gpuUrl, ) - def fetch_dep_dists(self): + def fetch_dep_dists(self, skip_uv: bool = False): + skips = ["uv"] if skip_uv else None + reqs = parse_req_file(self.out, skips=skips) + + extraUrl = None if "--extra-index-url" in reqs else self.gpuUrl + DependencyCompiler.Download( cwd=self.cwd, - reqFile=[self.out], executable=self.executable, - extraUrl=self.gpuUrl, + extraUrl=extraUrl, noDeps=True, out=self.outDir / "dists", + reqs=reqs, ) - def fetch_dep_wheels(self): + def fetch_dep_wheels(self, skip_uv: bool = False): + skips = ["uv"] if skip_uv else None + reqs = parse_req_file(self.out, skips=skips) + + extraUrl = None if "--extra-index-url" in reqs else self.gpuUrl + DependencyCompiler.Wheel( cwd=self.cwd, - reqFile=[self.out], executable=self.executable, - extraUrl=self.gpuUrl, + extraUrl=extraUrl, noDeps=True, out=self.outDir / "wheels", + reqs=reqs, )