Add include/exclude fields to package manifest by asmello · Pull Request #310 · helsing-ai/buffrs

asmello · 2026-04-17T08:49:01Z

Summary

Adds include and exclude fields to PackageManifest, allowing packages to control which files are included when packaging and populating vendor directories
include starts from a blank slate — only files matching the provided globs are included (any file type, not just .proto)
exclude starts from all files and removes those matching the provided globs
The two fields are mutually exclusive — specifying both produces a validation error at manifest parse time
When neither is set, the existing default behavior applies (all .proto files recursively)
Replaces walkdir with the ignore crate for glob matching, type filtering, and override support in PackageStore::collect()
Introduces Canary13 edition to snapshot current behavior before this change

Test plan

Unit tests for serialization/deserialization with include, exclude, both, and neither
Unit tests for collect() with include patterns, exclude patterns, vendor filtering, file-only results, and invalid globs
Integration tests pass (cargo test)
Manual smoke test: create a package with include / exclude in Proto.toml and confirm correct files are packaged

Introduce `include` and `exclude` fields on `PackageManifest`, allowing users to control which files are packaged. The implementation uses the `ignore` crate (`OverrideBuilder` for include, inverted match semantics for exclude). Key changes: - Bump version to 0.14.0, add `ignore` dependency - Add `include: Option<Vec<String>>` and `exclude: Vec<String>` to `PackageManifest` with serde skip-if-empty semantics - Add `Canary13` edition variant and accept it in the deserializer - Extend `collect()` with `include` and `exclude` parameters, using `OverrideBuilder` for include globs and inverted `filter_entry` matching for exclude globs - Wire include/exclude through `release()`, `populate()`, and `init()` - Filter directory entries from `collect()` results to prevent panics - Activate `TypesBuilder` proto filter with `select("proto")` - Replace `.unwrap()` calls with proper error handling in `release()` and `populate()` - Fix `list()` to not apply root include/exclude to vendored files - Fix `populated_files()` to not redundantly re-apply include/exclude - Handle broken symlinks gracefully in exclude `filter_entry` - Update all test fixtures and tutorial build.rs for new API - Add unit tests for `collect()` and manifest deserialization

Align with Cargo's behavior: only one of `include` or `exclude` may be set on a package manifest. Add validation in manifest parsing, simplify the `collect()` logic now that the two cannot coexist, and update tests accordingly.

- Reject empty `include = []` at parse time with a clear error message - Guard `release()` against missing `manifest.package` - Fix two raw.rs test fixtures that had both include and exclude set - Add test for empty include rejection - Add tests for populate() with include/exclude filtering - Document that Proto.toml is always included regardless of settings

vsiles · 2026-04-20T06:56:00Z

Will have a look today. Before this, can you motivate the choice of include allowing non proto file while the existing behavior filters for .proto only ? A real need or just making implementation more straightforward ?

mara-schulke · 2026-04-27T08:43:41Z

+    ) -> miette::Result<Vec<PathBuf>> {
+        debug_assert!(
+            include.is_none() || exclude.is_empty(),
+            "include and exclude are mutually exclusive"


Why is this?

In my naive understanding i would have assumed that we have:

Base = ./proto/**.proto Include = <some-globs> Exclude = <some-globs> Final = (Base & Include) - Exclude

Yeah, that's also possible (and in fact at first I implemented it that way). But that's not how Cargo works, and apparently the reason is that you never really need both, and there's more than one possible interpretation if both are provided (although I agree what you described is the most intuitive), so we avoid ambiguity and potential mistakes by disallowing this case. With only one list, you can directly infer the resulting filter, while with both it gets trickier to reason about what ends up being included.

mara-schulke · 2026-04-27T08:47:52Z

        }

-        for entry in self.collect(&source_path, false).await {
+        let include = manifest.include.as_deref();


Why is the include defined using let but the exclude one is passed as an arg directly?

This is because exclude is Vec<_> while include is Option<Vec<_>> — we need to distinguish include not being set from it being set as an empty vec because those have different semantics. But now that I think about it, the same goes for exclude, because an empty exclude list actually includes more files than the default. Will update.

mara-schulke · 2026-04-27T08:50:07Z

Overall i think this is a sane feature, and a non controversial implementation, but im not yet 100% sure on the semantics of the include and exclude fields. How is this implemented in prevalent package managers? I would go with the most common semantics here; if this is already the case, ignore my comments:)

mara-schulke · 2026-04-27T08:52:45Z

This seems related

mara-schulke · 2026-04-27T08:54:52Z

Also, can you in any case, please extend the book for this feature?

wngr · 2026-04-27T10:25:15Z

@vsiles

Before this, can you motivate the choice of include allowing non proto file while the existing behavior filters for .proto only ? A real need or just making implementation more straightforward ?

We have a use case where we want to package an additional manifest file alongside the proto files. This additional metadata is related to the proto definitions, but inlining it as comments seems brittle and cumbersome.

asmello · 2026-05-01T12:35:23Z

This seems related

Entirely! I implemented the same semantics except that when omitted only *.proto files are included (for backward compatibility).

`exclude: Vec<String>` collapsed an omitted field and `exclude = []` into the same empty vec, both falling through to the default `*.proto`-only collection path. Switch to `Option<Vec<String>>` so an explicit empty list takes the exclude branch (start from all files, remove nothing) — the only way to express "bundle every file" without listing every glob.

`include = []` produces a package containing only the manifest (always bundled by `Package::create`). That's a valid if unusual buffrs package, so there's no concrete reason to reject it at parse time. Drop the bail, swap the rejection test for a distinction-preserving one mirroring the empty-exclude case, and short-circuit in `collect()` so empty include actually yields no files — `OverrideBuilder` with no patterns is a no-op filter that would otherwise let everything through, the opposite of the intended "match nothing" semantic.

Pulls in the fix for RUSTSEC-2026-0104: a reachable panic when parsing certificate revocation lists with a syntactically valid empty BIT STRING in `IssuingDistributionPoint.onlySomeReasons`. Buffrs doesn't use CRLs directly, but the advisory still trips `cargo deny check` in CI.

The default-proto branch in `collect()` panicked via `.expect()` if `TypesBuilder` ever rejected the registration or build steps. Both are infallible in practice, but the rest of `collect()` already wraps errors as miette diagnostics, so propagate these too for consistent pretty printing. Addresses review comment on PR #310.

asmello force-pushed the asm/include-exclude branch from 57743f3 to a010572 Compare April 17, 2026 09:00

asmello force-pushed the asm/include-exclude branch from a010572 to e7e3f4a Compare April 17, 2026 09:03

Make include and exclude mutually exclusive

fbcd143

Align with Cargo's behavior: only one of `include` or `exclude` may be set on a package manifest. Add validation in manifest parsing, simplify the `collect()` logic now that the two cannot coexist, and update tests accordingly.

asmello force-pushed the asm/include-exclude branch from e7e3f4a to fbcd143 Compare April 17, 2026 09:20

asmello commented Apr 17, 2026

View reviewed changes

Comment thread src/package/store.rs

asmello marked this pull request as ready for review April 17, 2026 13:35

asmello requested review from mara-schulke and vsiles April 17, 2026 13:37

mara-schulke reviewed Apr 27, 2026

View reviewed changes

Comment thread src/package/store.rs Outdated

mara-schulke reviewed Apr 27, 2026

View reviewed changes

mara-schulke assigned asmello Apr 27, 2026

asmello added 2 commits May 1, 2026 13:57

asmello force-pushed the asm/include-exclude branch from efb789b to 706ca6a Compare May 1, 2026 13:41

asmello added 3 commits May 1, 2026 14:47

fixup! Distinguish unset and empty exclude in PackageManifest

80999da

Uh oh!

Conversation

asmello commented Apr 17, 2026

Summary

Test plan

Uh oh!

Uh oh!

vsiles commented Apr 20, 2026

Uh oh!

mara-schulke Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

mara-schulke Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asmello May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mara-schulke Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

asmello May 1, 2026

Choose a reason for hiding this comment

Uh oh!

asmello May 1, 2026

Choose a reason for hiding this comment

Uh oh!

mara-schulke commented Apr 27, 2026

Uh oh!

mara-schulke commented Apr 27, 2026

Uh oh!

mara-schulke commented Apr 27, 2026

Uh oh!

wngr commented Apr 27, 2026

Uh oh!

asmello commented May 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mara-schulke Apr 27, 2026 •

edited

Loading