From 965702eadd3786b09ec2bd4919001e7b2caab3b5 Mon Sep 17 00:00:00 2001 From: Christophe Combelles Date: Tue, 7 Jul 2026 19:48:38 +0200 Subject: [PATCH] fix(pyvolca): don't mistake a `volca` directory for the engine binary Server._find_binary probed `Path(self.binary).exists()`, which is true for a directory. Running a script from a source checkout that has a `volca/` package directory in the working tree returned that directory as the binary; Popen then spawned an unexecutable path and start() hung until the wait timeout. Check for a file (`is_file()`) at both the explicit-binary and dev-tree fallbacks. Adds tests/test_server.py covering the shadowing case. Cuts pyvolca 0.7.1 (fix only; README compat block regenerated). --- pyvolca/CHANGELOG.md | 9 ++++++ pyvolca/README.md | 2 +- pyvolca/pyproject.toml | 2 +- pyvolca/src/volca/server.py | 10 ++++-- pyvolca/tests/test_server.py | 59 ++++++++++++++++++++++++++++++++++++ 5 files changed, 77 insertions(+), 5 deletions(-) create mode 100644 pyvolca/tests/test_server.py diff --git a/pyvolca/CHANGELOG.md b/pyvolca/CHANGELOG.md index 37f70e90..521d7e57 100644 --- a/pyvolca/CHANGELOG.md +++ b/pyvolca/CHANGELOG.md @@ -18,6 +18,15 @@ git cliff --unreleased --tag pyvolca-v0.X.Y # render as a released section Then paste the rendered block at the top of this file and tighten wording. +## [0.7.1] - 2026-07-07 + +### Fixed + +- `Server` no longer mistakes a directory named `volca` in the working tree for + the engine binary. The lookup now checks for a *file*, so running a script + from a source checkout (which has a `volca/` package directory) starts the + downloaded binary instead of timing out on an unexecutable path. + ## [0.7.0] - 2026-06-24 Co-product allocation shares are now visible from the typed client. diff --git a/pyvolca/README.md b/pyvolca/README.md index dad2a849..ae50dcbb 100644 --- a/pyvolca/README.md +++ b/pyvolca/README.md @@ -26,7 +26,7 @@ pyvolca speaks one revision of the engine's JSON wire format; the engine adverti _Generated from `volca._compat` — run `python scripts/gen_api_md.py` to regenerate._ -This build of **pyvolca 0.7.0** speaks wire format **1** and requires a VoLCA engine **≥ v0.8.0**. +This build of **pyvolca 0.7.1** speaks wire format **1** and requires a VoLCA engine **≥ v0.8.0**. diff --git a/pyvolca/pyproject.toml b/pyvolca/pyproject.toml index e04932b1..0f1c0a05 100644 --- a/pyvolca/pyproject.toml +++ b/pyvolca/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "pyvolca" -version = "0.7.0" +version = "0.7.1" description = "Python client for VoLCA — Life Cycle Assessment engine" requires-python = ">=3.10" license = "Apache-2.0" diff --git a/pyvolca/src/volca/server.py b/pyvolca/src/volca/server.py index 86095194..e46dabfb 100644 --- a/pyvolca/src/volca/server.py +++ b/pyvolca/src/volca/server.py @@ -72,15 +72,19 @@ def _find_binary(self) -> str: """Find the volca binary. Resolution order: - 1. ``self.binary`` if it is an existing path. + 1. ``self.binary`` if it is an existing file. 2. The shared install root (``platformdirs.user_data_dir``) — populated by :func:`volca.download`, ``install.sh``, or ``install.ps1`` interchangeably. 3. ``shutil.which(self.binary)`` — PATH lookup, including the ``~/.local/bin/volca`` shim that ``install.sh`` drops. 4. ``./volca`` / ``./dist/volca`` for ad-hoc dev trees. + + The check is ``is_file``, not ``exists``: a directory named ``volca`` + in the working tree (e.g. a source checkout) must not shadow the real + binary and get handed to ``Popen`` as an unexecutable path. """ - if Path(self.binary).exists(): + if Path(self.binary).is_file(): return self.binary installed = _download.installed_binary() if installed is not None: @@ -89,7 +93,7 @@ def _find_binary(self) -> str: if found: return found for candidate in ["./volca", "./dist/volca"]: - if Path(candidate).exists(): + if Path(candidate).is_file(): return candidate raise FileNotFoundError( f"Cannot find '{self.binary}' binary. " diff --git a/pyvolca/tests/test_server.py b/pyvolca/tests/test_server.py new file mode 100644 index 00000000..f2ba51a0 --- /dev/null +++ b/pyvolca/tests/test_server.py @@ -0,0 +1,59 @@ +"""Offline tests for volca.server.Server binary resolution. + +No engine is spawned — these exercise _find_binary's resolution order, +which must not be fooled by a directory that happens to share the binary +name (the common case: running a script from a source checkout that has a +``volca/`` package directory in the working tree). +""" + +from __future__ import annotations + +from pathlib import Path +from unittest import mock + +import pytest + +from volca import _download +from volca.server import Server + + +def test_find_binary_skips_same_named_directory(tmp_path: Path, monkeypatch): + """A ``volca`` *directory* in cwd must not be returned as the binary.""" + (tmp_path / "volca").mkdir() + monkeypatch.chdir(tmp_path) + installed = tmp_path / "install" / "volca" + installed.parent.mkdir() + installed.write_bytes(b"") + with mock.patch.object(_download, "installed_binary", return_value=installed): + srv = Server(config="absent.toml", binary="volca") + assert srv._find_binary() == str(installed) + + +def test_find_binary_devtree_skips_same_named_directory(tmp_path: Path, monkeypatch): + """Dev-tree fallback: a ``./volca`` directory must not shadow ``./dist/volca``.""" + (tmp_path / "volca").mkdir() + (tmp_path / "dist").mkdir() + (tmp_path / "dist" / "volca").write_bytes(b"") + monkeypatch.chdir(tmp_path) + with mock.patch.object(_download, "installed_binary", return_value=None), \ + mock.patch("volca.server.shutil.which", return_value=None): + srv = Server(config="absent.toml", binary="volca") + assert srv._find_binary() == "./dist/volca" + + +def test_find_binary_accepts_explicit_file(tmp_path: Path): + """An explicit path to a real file is returned as-is.""" + binary = tmp_path / "volca" + binary.write_bytes(b"") + srv = Server(config="absent.toml", binary=str(binary)) + assert srv._find_binary() == str(binary) + + +def test_find_binary_raises_when_nothing_found(tmp_path: Path, monkeypatch): + """No file, no install, no PATH, no dev tree → clear FileNotFoundError.""" + monkeypatch.chdir(tmp_path) + with mock.patch.object(_download, "installed_binary", return_value=None), \ + mock.patch("volca.server.shutil.which", return_value=None): + srv = Server(config="absent.toml", binary="volca") + with pytest.raises(FileNotFoundError): + srv._find_binary()