Conversation
Dependencies that are only needed to work on a package itself, such as testbench infrastructure, currently have no way to be kept out of dependent projects. The `target` field on a dependency only filters it out of source listings; it is still resolved and inherited by everyone depending on the package. Add a `dev_dependencies` section (alias `dev-dependencies`) whose entries are resolved only when the package is the root package, and are never propagated to packages depending on it. Entries accept exactly the same fields as `dependencies`, so `remote` shorthands, `target` and `pass_targets` all keep working. Listing a package in both sections is an error. Deliberately a single section rather than named groups: target expressions already provide the selection axis, and a second grouping mechanism would overlap with them. `Manifest::root_dependencies` and `root_dependency` mark the sites where dev-dependencies enter the picture, so the root-only rule stays visible in the code. This also fixes `bender fusesoc --single` disagreeing with `bender fusesoc` about the root package's `depend` list, since the former reads the manifest directly while the latter derives it from the shared source tree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
dev_dependencies manifest sectiondev_dependencies manifest section
dev_dependencies manifest sectiondev_dependencies manifest section
…on.rs` The manifest-level cases (section separation, the `dev-dependencies` alias, field parity with `dependencies`, and the duplicate-section error) are pure `PartialManifest::validate` behavior and do not need a subprocess. Move them to `#[cfg(test)] mod tests` in `config.rs`, alongside the existing unit tests in `progress.rs` and `diagnostic.rs`. What remains genuinely needs an end-to-end run against a multi-package graph, so keep it as an integration test but name the file for what it covers rather than for one feature, so later resolution and source-tree tests have a home. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every key in the manifest format is snake_case, and the alias was the
only serde alias or rename in `config.rs`, so it would have been the
sole kebab-case spelling accepted anywhere in a `Bender.yml`. The
aliases bender does carry are all CLI backward-compatibility shims;
there is nothing to stay compatible with for a brand-new section.
The kebab-case spelling now falls through to the unknown-field path,
which names the offending key:
warning[W03]: Ignoring unknown field dev-dependencies in package X.
╰─› help: Check for typos in dev-dependencies or remove it [...]
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds a
dev_dependenciessection toBender.ymlfor dependencies that are only needed to work on a package itself. They are resolved when the package is the root package and are not propagated to packages that depend on it — Cargo's[dev-dependencies]semantics.Entries accept exactly the same fields as
dependencies— Git/path/revision sources,remoteshorthands,target,pass_targets. Listing a package in both sections is an error. The section is spelleddev_dependenciesonly: every key in the manifest format is snake_case, so Cargo's kebab-casedev-dependenciesis deliberately not aliased and falls through to theW03unknown-field warning, which names the offending key.Why
Today the only knob is
targeton a dependency, and as the book already notes, it only filters that dependency out of source listings and generated scripts — it does not affect resolution. Every declared dependency is still resolved, locked, and inherited by everyone downstream. There is currently no way to say "this one is mine, don't inherit it".Design note: one section, not named groups
Deliberately a single section rather than Python-style named dependency groups. Bender already has a full target-expression language with CLI selection (
-t), package scoping, and negation — a second grouping mechanism would overlap with it and raise questions like "what if grouptestandtarget: testdisagree?". Target expressions give grouping for free:Implementation
config.rs— newdev_dependenciesfield onManifest/PartialManifest. Dependency validation is factored into a shared closure so both sections behave identically.Manifest::root_dependencies()/root_dependency()mark every site where dev-deps legitimately enter the picture, keeping the root-only rule greppable rather than implicit.resolver.rs— the root manifest's dev-deps are registered alongside its regular deps and their version constraints chained intomark(). Transitive manifests are untouched, so a dependency's dev-deps are never read.sess.rs(lockfile validation, topological roots, root package source group),cmd/sources.rs(pass_targets),cmd/parents.rs,cmd/fusesoc.rs.Drive-by fix
bender fusesocandbender fusesoc --singledisagreed on the root package'sdependlist: the full run derives it from the shared source tree,--singlereads the manifest directly. Aligned so both emit the same list.Reviewer notes
The source-leakage contract. Bender emits one flat file list for the whole graph, so a dependency's source groups stay visible downstream even though its dev-deps are not. A package whose sources reference a dev-dependency must keep those sources behind a target consumers don't enable, or the generated file list references modules no longer in the graph.
This maps cleanly onto the convention added in #327:
target: test— reusable VIP that consumers enable → its deps belong independenciestarget: tb— non-reusable testbench code consumers never enable → its deps belong indev_dependenciesThis is documented under Keeping Dev-Only Sources Consistent, but it is currently a documented contract, not enforced. Making it structural would mean dropping
tb-gated sources for non-root packages — a separate, more invasive change, deliberately left out of scope here.Behavioral consequence. As in Cargo, simulating a sub-IP's testbench from a top-level repo no longer pulls that sub-IP's dev-deps, since it isn't the root. That may warrant a
--with-dev-deps <pkg>escape hatch later; happy to add it if reviewers think the PULP workflow needs it.Backward compatibility. Older Bender versions route the unknown
dev_dependencieskey to the extra-field path, emitW03, and ignore it — which is exactly the correct behavior for a consumer, so this degrades gracefully.Testing
Split by nature rather than by feature:
dependencies, and the duplicate-section error are purePartialManifest::validatebehavior, so they are unit tests insrc/config.rs, alongside the existing ones inprogress.rsanddiagnostic.rs. No subprocess, no fixture.tests/resolution.rsruns the binary against a multi-package fixture built undertests/tmp/, using only path dependencies so it needs no network. Covers root-only resolution, lockfile contents,targetcomposition,bender parents, and a dev-dep that is also a transitive dep of a regular dep (unifies to one lockfile entry, still ranks correctly in the file list). Named for what it covers rather than for this feature, so later resolution tests have a home.Note that
tests/cli_regressiondeliberately does not carry this: it diffs the current binary against one built frommasterand asserts stdout is byte-identical, so a new manifest section fails there by construction (golden emitsW03and ignores the field; new resolves it). Its fixture stays untouched, which also means it keeps proving this PR changes nothing for manifests withoutdev_dependencies.All local tests pass — 20 lib, 5 resolution, 8 pickle, 18 script.
cargo fmt --checkclean; clippy warning count unchanged at 16, all pre-existing.🤖 Generated with Claude Code