Skip to content

feat(build): fail with actionable instructions when a C compiler or cargo is missing - #72

Open
rhoadesScholar wants to merge 1 commit into
v2.0from
jeffr/loud-fail-missing-build-tools
Open

feat(build): fail with actionable instructions when a C compiler or cargo is missing#72
rhoadesScholar wants to merge 1 commit into
v2.0from
jeffr/loud-fail-missing-build-tools

Conversation

@rhoadesScholar

Copy link
Copy Markdown
Contributor

First of three, replacing #70 — which bundled this feature with two unrelated bug fixes and announced
all three in its own title. Opened fresh rather than force-pushed so the review history stays readable.
#70 will be closed pointing here.

Why

Building daisy from source compiles daisy-core through maturin. If a C linker or the Rust toolchain
is missing, maturin's error does not say what to install — and since pip install daisy from an sdist
hits this path, it is the first thing a new user sees.

What a user sees now

$ pip install .        # on a machine with no compiler and no cargo

Before — maturin's error, with no indication of the cause or the fix.

After:

  BUILD ERROR: MISSING C COMPILER

A system C compiler / linker ('cc', 'gcc', or 'clang') was not found.
This project compiles native Rust extensions and requires a system toolchain.

Please install a compiler using your system package manager:
  Ubuntu/Debian:  sudo apt update && sudo apt install build-essential
  Fedora/RHEL:    sudo dnf groupinstall "Development Tools"
  Arch Linux:     sudo pacman -S base-devel
  macOS:          xcode-select --install

[exited with code 1]

There is an equivalent message for a missing cargo.

Verified both directions on the same tree — with the toolchain present the build is unaffected:

$ python -c "import build_wrapper; print(build_wrapper.build_wheel('out'))"
   Finished `release` profile [optimized] target(s) in 6.05s
📦 Built wheel for CPython 3.11 to target/wheels/daisy-2.0.0-cp311-cp311-linux_x86_64.whl

How

build_wrapper.py becomes the PEP 517 backend (build-backend = "build_wrapper",
backend-path = ["."]) and wraps maturin's build_wheel / build_editable with a shutil.which
check up front. build_sdist forwards unwrapped — packaging sources needs no toolchain, so gating
it would be a regression.

Everything else is re-exported with from maturin import *. That is safe here specifically because
maturin's __init__.py defines no __all__, so get_requires_for_build_wheel,
get_requires_for_build_sdist and prepare_metadata_for_build_wheel all forward intact — pip will not
fail on a missing hook.

Also in scope

dev, examples and docs extras under [project.optional-dependencies], so pip users get the
dependency groups uv users already had.

That conversion has to update the places that consumed the old [dependency-groups] table, which is
why they are in this PR rather than a follow-up: .github/workflows/docs.yml (two call sites) and
README.md:52 all said uv sync --group docs, which errors with
Group 'docs' is not defined in the project's dependency-groups table once the table is gone.

$ uv sync --extra docs
Resolved 117 packages          # green

(#70 made the conversion but not these updates, so its docs job failed at dependency install. That is
fixed here rather than shipped again.)

rhoadesScholar pushed a commit that referenced this pull request Aug 3, 2026
Pure `ruff --fix` pass over the Python sources with
--select I001,F401,F811,RUF100,UP035,PYI029,PYI041. No behaviour changes:
import sorting, unused-import removals, duplicate-import dedups, redundant
noqa removals, typing.Callable -> collections.abc.Callable, and two .pyi stub
cleanups (redundant __repr__ declarations; float|int -> float).

Rebased from v2.0_patch onto v2.0 now that #70 is closed in favour of #72/#73/#74.
Regenerated rather than cherry-picked, so the two hunks that only existed via #70
(build_wrapper.py, tests/test_worker_serialization.py) are simply absent.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…argo is missing

Building daisy from source compiles the Rust core through maturin. When a C linker or the
Rust toolchain is absent, maturin's own error is cryptic and does not say what to install --
and since `pip install daisy` from an sdist takes this path, it is the first thing a new
user sees.

build_wrapper.py becomes the PEP 517 backend (backend-path = ["."]) and wraps maturin's
build_wheel / build_editable with an up-front check for cc/gcc/clang and cargo, printing
per-platform install instructions instead. build_sdist forwards UNWRAPPED -- packaging
sources needs no toolchain, so gating it would be a regression. Everything else is
re-exported from maturin, which defines no __all__, so get_requires_for_build_wheel,
get_requires_for_build_sdist and prepare_metadata_for_build_wheel forward intact.

Rebuilt against current v2.0 and reduced to just this. Dropped from the earlier version:
the [project.optional-dependencies] dev/examples/docs extras (v2.0 kept [dependency-groups]
and added a lint group, so those would now duplicate it -- and their "Your new dev extra"
comments were authoring scaffolding), the matching docs.yml/README --group -> --extra
changes (unnecessary for the same reason, and the cause of the docs-CI failure on the old
branch), and an unrelated .vscode .gitignore line.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@rhoadesScholar
rhoadesScholar force-pushed the jeffr/loud-fail-missing-build-tools branch from 079088c to b31e753 Compare August 4, 2026 02:34
@rhoadesScholar

Copy link
Copy Markdown
Contributor Author

Force-pushed a rebuild against current v2.0, and it shrank a lot: +97/-1 over 2 files, from +208/-69 over 6. Now mergeable.

v2.0 moved substantially since this was opened, and it settled two things the original version got wrong:

1. The extras conversion was the wrong direction. v2.0 kept [dependency-groups] and added a lint group (plus [tool.ruff]). So the [project.optional-dependencies] dev/examples/docs block this PR carried would now simply duplicate it — and its dev extra still pinned dill>=0.3 where v2.0 has moved to cloudpickle>=3. Dropped entirely.

That also removes the cause of this PR's docs-CI failure: the matching docs.yml and README.md changes from uv sync --group docs to --extra docs are unnecessary once [dependency-groups] stays, so those are reverted too. The build job should now pass rather than erroring with Group 'docs' is not defined.

2. An unrelated rider is gone. The old version also added .vscode to .gitignore — nothing to do with build tooling. Dropped.

What is left is only the point of the PR

  • build_wrapper.py (new, 95 lines)
  • two lines in pyproject.toml: build-backend = "build_wrapper", backend-path = ["."]

Re-verified both directions on the rebuilt branch, against current v2.0:

# no toolchain on PATH
  BUILD ERROR: MISSING C COMPILER
A system C compiler / linker ('cc', 'gcc', or 'clang') was not found.
...
  Ubuntu/Debian:  sudo apt update && sudo apt install build-essential
[exit 1]

# toolchain present -- unaffected
   Finished `release` profile [optimized] target(s) in 8.52s
📦 Built wheel for CPython 3.12 to target/wheels/daisy-2.0.0-cp312-cp312-linux_x86_64.whl

And the forwarded hook surface is intact (maturin defines no __all__, so from maturin import * carries them):

  get_requires_for_build_wheel       present
  get_requires_for_build_sdist       present
  prepare_metadata_for_build_wheel   present
  build_wheel / build_editable / build_sdist   present

build_sdist deliberately forwards unwrapped — packaging sources needs no compiler, so gating it would be a regression.


Noticed while resolving, and relevant to my other open work: v2.0's new [tool.ruff] carries the comment "The upstream bundled benchmarks are self-comparing relics of the source anonymization (duplicate import daisy arms, duplicate dict keys) — they need a rewrite, not a lint pass." That is the same defect I was about to file separately, so you have already diagnosed it — I'll aim a benchmark-repair PR at exactly that rather than duplicating the analysis.

@pattonw

pattonw commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

I'm skeptical of this build wrapper. While the error message we get from maturin is quite unhelpful. A fairly long list of "could not compile ..." errors after the one we actually want: "error: linker "cc" not found", I think the error handling along with the provided solutions are very system dependent, and we don't want to try to properly handle all systems. I think a better approach would just be to put a line in the README saying "building from source requires a C toolchain: (build-essentials, xcode-select, etc.)" and leave the error to rust/maturin.
I don't know why this error is so opaque when rust normally has great error messages. Maybe this is an issue that should be posted upstream in maturin or some rust library

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants