Skip to content

buffa-build: add Config::exclude_package knob - #383

Merged
iainmcgin merged 3 commits into
anthropics:mainfrom
rsd-darshan:feat/buffa-build-exclude-package
Sep 1, 2026
Merged

iainmcgin merged 3 commits into
anthropics:mainfrom
rsd-darshan:feat/buffa-build-exclude-package

Conversation

@rsd-darshan

@rsd-darshan rsd-darshan commented Aug 29, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Closes #293.

protoc-gen-buffa has had exclude_package=buf.validate since v0.6.0, but buffa-build users had no equivalent — they could only approximate it by omitting files from the .files() list, which doesn't help when directory globbing or transitive descriptor resolution pulls in option-only packages (e.g. buf/validate/validate.proto via --include_imports).

This PR:

  • Adds Config::exclude_package("buf.validate") to buffa-build (a leading dot is accepted and stripped, matching the plugin).
  • Adds CodeGenConfig::exclude_packages: Vec<String> to buffa-codegen as the shared backing field, so the knob is available to direct codegen users too.
  • Moves the package-filtering step into generate_with_diagnostics, applied before the codegen context is built — ensuring that WKT extern-path auto-injection and other context decisions that key on files_to_generate membership see the already-filtered list.
  • Removes filter_excluded_files from protoc-gen-buffa (the logic is now in generate_with_diagnostics); the plugin's parse layer pushes directly into codegen.exclude_packages.
  • Routes normalization through the existing normalize_exclude_package for all callers, so leading-dot stripping and validation cannot drift between front-ends.
  • Updates docs/guide.md to reflect that exclude_package is now available on both the plugin and buffa-build paths.

Usage

// build.rs
buffa_build::Config::new()
    .exclude_package("buf.validate")
    .exclude_package(".gnostic.openapi.v3")  // leading dot stripped automatically
    .files(&["proto/my_service.proto"])
    .includes(&["proto/", "vendor/"])
    .compile()
    .unwrap();

The same plugin option string syntax continues to work unchanged:

exclude_package=buf.validate
exclude_package=.gnostic.openapi.v3

Test plan

  • task lint passes (clippy clean, fmt clean, markdownlint clean)
  • task test passes — 603 buffa-codegen unit tests including 5 new test_exclude_packages_* tests and 2 new buffa-build builder tests
  • New tests cover: exact-package exclusion, sub-package exclusion via prefix, empty-exclude-list fast path, FileNotFound when a file without a descriptor passes the filter unchanged, and rejection of malformed entries

@github-actions

Copy link
Copy Markdown

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@iainmcgin

Copy link
Copy Markdown
Collaborator

[claude code]

Thanks, this is the right shape: one normalize_exclude_package for every caller, the filter ahead of CodeGenContext::for_generate so the WKT extern-path decisions see the filtered set, the None => true orphan rule kept with a test naming why, and output_entries built from the returned files so an excluded package gets no include! entry or stitcher. The plugin path is behaviourally identical to the removed filter_excluded_files (same predicate, order, and fast path). A few things need to change before it can land, mostly small:

Rebase. docs/guide.md conflicts with main: #352 rewrote the paragraph at ~617 and it now ends "exclude_package is a protoc-plugin option only; the buffa-build/build.rs path does not need it", which this PR makes false. Keep #352's new warning sentences and replace only that clause. Related: main's ExcludedPackageFieldRef warning (#352) runs after by_package is built, so with your earlier shadowing of files_to_generate a build.rs exclusion gets the dangling-reference warning for free, which is worth a sentence in the rustdoc instead of the current "will contain dangling paths".

Silent no-op. .exclude_package("buf.validat") or .exclude_package("buf/validate/validate.proto") validates and then matches nothing, and excluding every listed file produces an empty include file and a green build. override_feature_in already handles this class with CodeGenWarning::FeatureOverrideMatchedNothing; please add ExcludePackageMatchedNothing { package }, fired when a normalized entry matches no package anywhere in file_descriptors (not just files_to_generate, so plugin users without include_imports don't get spurious warnings). CodeGenWarning is #[non_exhaustive] and buffa-build already forwards warnings to cargo:warning=.

Tests for the two claims the PR makes. Nothing exercises the "before context build" ordering: a test with google/protobuf/timestamp.proto plus a user file with a Timestamp field, both in files_to_generate, exclude_packages: ["google.protobuf"], asserting the field type is ::buffa_types::google::protobuf::Timestamp rather than a local path would pin it. And the two buffa-build tests only assert the vector contents, so compile() ignoring the field would pass; shared_descriptor_pool_writes_sidecar_and_shared_root on main shows how to feed a synthetic descriptor set through .descriptor_set() and inspect out_dir without protoc.

Validation timing and message. A malformed entry currently fails inside generate_with_diagnostics, after protoc has run. Either normalize in the setter as extern_path does, or validate at the top of compile() next to the shared_descriptor_pool checks. And the Err(_) arm re-words normalize_exclude_package's own message; map_err(|e| CodeGenError::Other(format!("exclude_package {entry:?}: {e}"))) keeps one text. If you go for the setter route, the stores_raw_value_for_deferred_normalization test goes away with it.

Docs. The build.rs Config table (guide ~180-212) needs an .exclude_package(pkg) row, with the extern_path pairing note inline, since a build.rs author scans that table rather than the plugin section. The new guide sentence says the knob helps when "transitive descriptor resolution pulls in option-only packages that files() alone cannot filter out", but on the build.rs path the generate set is exactly .files(); only a glob puts an excluded file there, so say that. CodeGenConfig::exclude_packages' rustdoc says entries "must be bare proto package names with no leading dot" while buffa-build deliberately stores ".buf.validate" into it, and the codegen comment at 2380 says buffa-build pre-normalizes while buffa-build's comment says it doesn't; both should say a leading dot is accepted and stripped at generation time. The same rustdoc states the extern_path remedy twice.

Smaller: the three test_exclude_packages_* bodies are the same 20 lines with three literals swapped (a generated_names(fds, excludes) helper makes them three lines each), and exact assert_eq! on the sorted name list is stronger than !contains("validate"). PluginConfig is now a one-field struct whose field doc restates the PR description; parse_config could return CodeGenConfig directly.

Happy to re-review as soon as it's pushed; this is on the 0.9.2 list.

Expose the package-filtering capability that protoc-gen-buffa already
had as a first-class Config builder method on buffa-build. Closes anthropics#293.

Key changes:

- buffa-codegen: add `CodeGenConfig::exclude_packages: Vec<String>` and
  move filtering into `generate_with_diagnostics`, called before building
  the codegen context so that WKT extern-path auto-injection and other
  context decisions see the already-filtered file list.
  Normalization (leading-dot strip, validation) is routed through the
  existing `normalize_exclude_package`, removing the inline duplicate.

- buffa-build: add `Config::exclude_package(impl Into<String>) -> Self`,
  storing the raw value for deferred normalization and validation inside
  `generate_with_diagnostics` / `compile()`.

- protoc-gen-buffa: remove `filter_excluded_files` and the separate
  `PluginConfig::exclude_packages` field; push directly into
  `codegen.exclude_packages` so a single code path handles all callers.

- docs/guide.md: correct the statement that `exclude_package` is a
  plugin-only option.
- Add CodeGenWarning::ExcludePackageMatchedNothing { package }, emitted
  when an exclude_packages entry matches no proto package in the full
  descriptor set (typo or stale entry); buffa-build forwards it to
  cargo:warning= automatically.

- Add test pinning the "filter before context build" ordering: excluding
  google.protobuf while a kept file holds a Timestamp field must produce
  the ::buffa_types extern path, not a dangling local path.

- Add test for ExcludePackageMatchedNothing warning (typo'd package
  name produces a warning rather than a silent no-op).

- Rebase on main (picks up anthropics#352 and the shared_descriptor_pool PRs);
  resolve docs/guide.md conflict keeping anthropics#352's ExcludedPackageFieldRef
  warning sentence and replacing only the "plugin-only" clause.

- Add .exclude_package(pkg) row to the buffa-build Config table in
  docs/guide.md.
@rsd-darshan
rsd-darshan force-pushed the feat/buffa-build-exclude-package branch from d72433f to b506a3c Compare August 30, 2026 16:41
@rsd-darshan

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review — all points addressed in the latest push:

Rebase — Rebased on main; the docs/guide.md conflict is resolved keeping #352's ExcludedPackageFieldRef warning sentence and replacing only the "plugin-only" clause.

Silent no-op — Added CodeGenWarning::ExcludePackageMatchedNothing { package }, fired when a normalized entry matches no proto package in file_descriptors. buffa-build forwards it to cargo:warning= automatically via the existing warning-forwarding path.

Tests — Added test_exclude_packages_filter_runs_before_context_build: descriptor set with google/protobuf/timestamp.proto + a kept file with a Timestamp field, exclude_packages: ["google.protobuf"]; asserts the output contains buffa_types::google::protobuf (WKT extern-path injection fired) rather than a dangling local path. Also added test_exclude_package_matched_nothing_warning covering the typo case.

Validation timing / error message — generate_with_diagnostics now calls normalize_exclude_package for every entry (stripping leading dots, rejecting malformed values), so direct CodeGenConfig users and buffa-build users get the same normalization. The Err arm uses map_err-style forwarding of normalize_exclude_package's own message rather than re-wording it.

Docs — Added .exclude_package(pkg) row to the buffa-build Config table in docs/guide.md with the extern_path pairing note inline.

The smaller items (PluginConfig one-field struct / parse_config returning CodeGenConfig directly, test helper refactor) I've left for a follow-up to keep this PR scoped — happy to fold them in if you'd prefer them here.

…sage, test compile() end to end

- buffa-build rejects malformed entries at the top of compile(), ahead of
  protoc, with the normalizer's own message; codegen forwards that message
  instead of re-wording it
- end-to-end compile() test through a synthetic descriptor set: the
  excluded package produces no module and no include entry; a second test
  pins that validation runs before any input is read
- the before-context-build test asserts the exact extern path and the
  absence of a local one, instead of a disjunct any buffa_types mention
  satisfies
- docs: leading-dot rule stated once, ExcludedPackageFieldRef remedy names
  .exclude_package(), the guide row says what puts an excluded file in
  files(), fragment mentions the new warning
@iainmcgin

Copy link
Copy Markdown
Collaborator

[claude code]

Thanks for the second round. The warning, the guide rebase, and the before-context-build test were in, but two items from the list were not: validation still ran after protoc (the reply said it used map_err forwarding; the code re-worded the message), and there was no end-to-end compile() test. Rather than a third round I pushed e4efb26: compile() rejects malformed entries before protoc with the normalizer's own message and codegen forwards it; an end-to-end compile() test through a synthetic descriptor set (excluded package produces no module and no include entry) plus one pinning that validation runs before any input is read; the Timestamp test asserts the exact ::buffa_types::…::Timestamp path and the absence of a local one (the || disjunct matched any buffa_types mention); the leading-dot rule stated once; the ExcludedPackageFieldRef remedy names .exclude_package(…); the guide row says a glob-expanded files() is what puts an excluded file in the generate set. Ready to queue once Iain stamps it.

@iainmcgin
iainmcgin added this pull request to the merge queue Sep 1, 2026
Merged via the queue into anthropics:main with commit e1c75fe Sep 1, 2026
11 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 1, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

buffa-build: Config::exclude_package knob (demand-driven follow-up to #279)

2 participants