diff --git a/README.md b/README.md index 60e144fb..6ba4b855 100644 --- a/README.md +++ b/README.md @@ -14,6 +14,8 @@ curl -LsSf https://raw.githubusercontent.com/keboola/cli/main/install.sh | sh This installs a **prebuilt wheel** from the latest GitHub release -- a few-seconds download, no source build. Building from `git+` instead recompiles the bundled React SPA via npm on every install, which takes minutes on WSL ([#353](https://github.com/keboola/cli/issues/353)). The script bundles the `[server]` extras by default (set `KBAGENT_NO_SERVER=1` for a CLI-only install) and needs only `curl` + [`uv`](https://docs.astral.sh/uv/). +Requires **Python >=3.12**. `uv` normally fetches a matching interpreter on its own even if your default Python is older, but if it doesn't (offline, or Python downloads disabled) the install fails with `does not satisfy Python>=3.12` -- set `UV_PYTHON` (a standard `uv` env var) to a 3.12-or-newer interpreter you actually have -- `UV_PYTHON=3.12`, `UV_PYTHON=3.13`, or a full path. It has to reach `sh`, not `curl` -- `curl -LsSf https://raw.githubusercontent.com/keboola/cli/main/install.sh | UV_PYTHON=3.12 sh`, or `export UV_PYTHON=3.12` beforehand. Prefixing the whole pipeline (`UV_PYTHON=3.12 curl ... | sh`) assigns it to `curl`, where it has no effect. + Prefer to build from source, or pin a specific ref? ```bash diff --git a/pyproject.toml b/pyproject.toml index 45dff823..88be469d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -47,9 +47,26 @@ packages = ["src/keboola_agent_cli"] # ``uv tool install git+...`` or ``pip install`` from PyPI without the # user needing the ``web/`` source tree. # -# ``force-include`` overrides the default ``.gitignore`` exclusion so the -# generated dist actually lands in the wheel. Key on the LHS is the path -# on disk; value on the RHS is the path inside the wheel. +# ``force-include`` (below) is meant to be the ONLY way ``_ui_dist/`` enters +# the wheel. Without this ``exclude``, the ``packages`` collection above picks +# the dir up as well and the build aborts with "A second file is being added to +# the wheel archive at the same path". +# +# Do NOT count on hatchling's .gitignore-based exclusion to prevent that; it is +# version-dependent and it does not save this repo. Bisected on a clean +# checkout WITH both ``.git`` and ``.gitignore`` present and a prebuilt SPA on +# disk: hatchling <= 1.29.0 builds, >= 1.30.0 fails (1.30 / 1.31 / 1.32 all +# reproduce), and ``requires = ["hatchling"]`` above is unpinned, so every +# build takes the latest. Whether that exclusion fires at all also depends on +# where the ``.gitignore`` sits relative to the project root and the ``.git`` +# boundary hatchling stops its upward search at -- an ancestor ``.gitignore`` +# suppressed the duplicate in a minimal fixture, this repo's own root +# ``.gitignore`` does not. +# +# ``exclude`` is a plain glob, evaluated unconditionally, so it holds across +# both VCS state and hatchling version: verified shipping exactly one +# ``_ui_dist/index.html`` on hatchling 1.27, 1.29, 1.30 and 1.32. +exclude = ["src/keboola_agent_cli/_ui_dist"] [tool.hatch.build.targets.wheel.force-include] "src/keboola_agent_cli/_ui_dist" = "keboola_agent_cli/_ui_dist" @@ -83,6 +100,14 @@ exclude = [ "web/frontend/dist", "web/frontend/tsconfig.tsbuildinfo", "web/backend", + # Generated SPA output must never ship as *source*. ``include`` above has + # ``src/``, so with a ``_ui_dist/`` already on disk (an earlier editable + # install) and hatchling's .gitignore exclusion not applying -- as + # unreliable here as it is for the wheel target above -- the sdist carries + # a prebuilt SPA. Harmless downstream, since ``_bundle_ui`` rmtree's the + # dir before every build and a stale copy can never reach a wheel, but it + # ships bytes that are not source. + "src/keboola_agent_cli/_ui_dist", ] [tool.pytest.ini_options] diff --git a/scripts/hatch_build.py b/scripts/hatch_build.py index 49fd648f..67f2f2cf 100644 --- a/scripts/hatch_build.py +++ b/scripts/hatch_build.py @@ -15,9 +15,14 @@ 2. **Building it on the fly** if missing AND ``npm`` is available -- covers the ``uv tool install git+...`` happy path on machines that already have Node 20+ for other reasons. -3. **Copying** the dist into ``src/keboola_agent_cli/_ui_dist/`` so - hatchling's normal package collection picks it up (the dir is in - ``.gitignore`` to avoid checking in generated assets). +3. **Copying** the dist into ``src/keboola_agent_cli/_ui_dist/``, which + reaches the wheel *only* through ``force-include``. The wheel target + deliberately ``exclude``s that (gitignored) dir from hatchling's normal + package collection, because having both paths pick it up aborts the build + with "A second file is being added to the wheel archive at the same path". + Dropping either half breaks a build: no ``force-include`` ships a UI-less + wheel, no ``exclude`` reintroduces the duplicate. See the comment on + ``[tool.hatch.build.targets.wheel].exclude`` in ``pyproject.toml``. If neither a prebuilt dist nor ``npm`` is available, the hook logs a warning and lets the wheel build proceed without the UI. The CLI will diff --git a/tests/test_build_hook.py b/tests/test_build_hook.py index 2cdcb889..4db755c7 100644 --- a/tests/test_build_hook.py +++ b/tests/test_build_hook.py @@ -12,14 +12,22 @@ any OS. - **Bug 2** -- an early ``return`` left ``_ui_dist/`` missing, and hatchling's ``force-include`` then failed the whole build. We assert every code path - leaves ``_ui_dist/`` existing on disk. (The force-include interaction itself - is OS-independent and is additionally exercised end-to-end by the CI wheel - build.) + leaves ``_ui_dist/`` existing on disk. + +Also covers the duplicate-path failure that the wheel ``exclude`` in +``pyproject.toml`` guards against: with ``_ui_dist/`` populated at collection +time, hatchling's ``packages`` selection picks it up alongside the +``force-include`` and the build aborts with "A second file is being added to +the wheel archive at the same path". That reproduces on an ordinary checkout +(``.git`` and ``.gitignore`` both present) from hatchling 1.30.0 onwards -- +1.29.0 and earlier build fine -- so it is not tied to any VCS layout. See +``TestForceIncludeNoDuplicate`` below for the end-to-end regression test. """ from __future__ import annotations import importlib.util +import shutil import subprocess import sys import zipfile @@ -40,6 +48,22 @@ sys.modules["hatch_build"] = hatch_build _spec.loader.exec_module(hatch_build) + +@pytest.fixture(autouse=True) +def _neutralize_skip_ui_build(monkeypatch: pytest.MonkeyPatch) -> None: + """Clear the ambient ``KBAGENT_SKIP_UI_BUILD`` knob for every test here. + + Not a CI concern: ``ci.yml`` scopes that flag to a single step's ``env:``, + so it never reaches the pytest step. This guards the developer who + exported it in their shell. Left set, ``_bundle_ui`` ships an EMPTY + ``_ui_dist/``, and 5 tests in this module then fail with messages about + bundle contents instead of naming the env var -- including the end-to-end + wheel build, which inherits this environment through ``uv build``. Tests + that exercise the skip path opt in explicitly with ``monkeypatch.setenv``. + """ + monkeypatch.delenv(hatch_build.SKIP_UI_BUILD_ENV, raising=False) + + # The CI wheel-content assertion helper lives in ``scripts/`` -- load it the # same way so its logic is regression-tested in normal (ubuntu) CI, not only by # the Windows wheel-build job that calls it as a subprocess. @@ -289,3 +313,62 @@ def test_no_ui_fails_on_ui_wheel(self, tmp_path: Path) -> None: def test_missing_wheel_is_an_error(self, tmp_path: Path) -> None: assert check_wheel_ui.main(["--expect-ui", "--dist", str(tmp_path)]) == 1 + + +class TestForceIncludeNoDuplicate: + """A populated ``_ui_dist/`` must not land in the wheel twice. + + Reproduces the real failure end-to-end (not mocked): a minimal project + laid out with the actual ``pyproject.toml`` / ``hatch_build.py`` and a + prebuilt SPA dist on disk, so the hook populates ``_ui_dist/`` with a real + file. + + The fixture also puts hatchling's .gitignore-based exclusion deliberately + out of reach. That exclusion is *not* what ``exclude`` stands in for -- + the duplicate reproduces in a normal checkout too on hatchling >= 1.30 -- + but pinning it here stops the assertion from passing for an incidental + reason, e.g. under a ``TMPDIR`` that happens to sit below some other + ``.gitignore``. + """ + + def test_wheel_build_does_not_duplicate_ui_dist(self, tmp_path: Path) -> None: + if shutil.which("uv") is None: + pytest.skip("uv not on PATH") + + repo_root = Path(__file__).resolve().parents[1] + project = tmp_path / "project" + (project / "src" / "keboola_agent_cli").mkdir(parents=True) + (project / "src" / "keboola_agent_cli" / "__init__.py").write_text("", encoding="utf-8") + (project / "src" / "keboola_agent_cli" / "py.typed").write_text("", encoding="utf-8") + (project / "scripts").mkdir() + shutil.copy( + repo_root / "scripts" / "hatch_build.py", project / "scripts" / "hatch_build.py" + ) + shutil.copy(repo_root / "pyproject.toml", project / "pyproject.toml") + (project / "README.md").write_text("test project", encoding="utf-8") + + dist = project / "web" / "frontend" / "dist" + dist.mkdir(parents=True) + (dist / "index.html").write_text("app", encoding="utf-8") + + # An empty ``.git`` dir is the boundary that halts hatchling's upward + # ``.gitignore`` search, so no ancestor ``.gitignore`` can quietly + # supply an exclusion. Verified load-bearing: without it, a ``TMPDIR`` + # below any tree carrying a matching ``.gitignore`` makes this test + # pass even with the ``exclude`` fix reverted. + (project / ".git").mkdir() + assert not (project / ".gitignore").exists() + + result = subprocess.run( + ["uv", "build", "--wheel", "-o", str(tmp_path / "out")], + cwd=project, + capture_output=True, + text=True, + timeout=300, + ) + assert result.returncode == 0, result.stderr + + (wheel,) = list((tmp_path / "out").glob("*.whl")) + with zipfile.ZipFile(wheel) as zf: + ui_entries = [n for n in zf.namelist() if n.endswith("_ui_dist/index.html")] + assert ui_entries == ["keboola_agent_cli/_ui_dist/index.html"]