Skip to content

Add allow_unowned_globs to permit files with no owner - #135

Open
dduugg wants to merge 7 commits into
mainfrom
allow-unowned-files
Open

dduugg wants to merge 7 commits into
mainfrom
allow-unowned-files

Conversation

@dduugg

@dduugg dduugg commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Adds an allow_unowned_globs option to config/code_ownership.yml, for rubyatscale/code_ownership#140, and bumps the version to 0.6.0.

A repo that assigns owners to only some of its files had one way to get validate to pass: unowned_globs: ['**/*']. But files matched by unowned_globs are left out of the project, so that also switched off their # @team annotations.

A file in owned_globs that matches allow_unowned_globs doesn't need an owner. validate / gv no longer reports it under "Some files are missing ownership", and validate <files> no longer reports it under "Unowned files detected". It is otherwise treated like any owned file. It's still read, its annotations still count, and every other check still runs, so it still fails if it has more than one owner or names an invalid team.

  • ['**/*'] covers #140's case. This includes files at the project root, which a test checks.
  • A scoped list such as ['**/deprecated/**/*'] exempts only those paths. Files with no owner outside it still fail.
  • The default is [], and nothing else changes. In particular, unowned_globs files are still left out, and unowned_globs takes precedence over allow_unowned_globs. The README now spells out the difference between the two options, and how to move off unowned_globs: ['**/*'].

Why not make annotations work inside unowned_globs

That was the first approach I tried, and I benchmarked it. It means opening every file that unowned_globs matches, and unowned_globs is often a large third-party or generated tree. On a synthetic repo with git-tracked vendored files matching both globs:

  • a cold generate was about 7x slower at 100k files and 11x slower at 300k;
  • warm runs were 2.3–2.8x slower;
  • the cache grew 20–60x.

The cost is the per-file open, so reading only the first line didn't help. A gitignored node_modules wouldn't have been affected, but tracked vendored code would have been, and so would any checkout where git ls-files fails (non-git checkouts, or CI containers with "dubious ownership"). allow_unowned_globs only adds a glob check for files that already have no owner, using the same glob_match that owned_globs runs on every walked file.

It would also have changed existing behavior: every repo with annotations inside unowned_globs would see its CODEOWNERS change, and could start getting multiple-owner errors. An opt-in key avoids that.

Changes

  • config.rs: allow_unowned_globs: Vec<String>, defaulting to empty.
  • project.rs / project_builder.rs: carry it through to Project, the same way executable_name is carried.
  • validator.rs: skip FileWithoutOwner for files that match self.project.allow_unowned_globs. The multiple-owner and invalid-team checks are unchanged.
  • runner.rs: validate <files> skips unowned files that match allow_unowned_globs. It now makes each path project-relative once, with path_utils::relative_to_buf, and uses that path for every glob check.
  • path_utils.rs: one crate-private matches_globs, which replaces identical private copies in runner.rs, project_builder.rs and the validator. Two &str variants in file_owner_resolver.rs and the perf binary predate this PR and are left alone.
  • README.md: documents the option and contrasts it with unowned_globs, says which one wins and how to migrate, updates validation rule 3, and moves the Getting Started example's hand-picked app file from unowned_globs to allow_unowned_globs.
  • Cargo.toml / Cargo.lock: version 0.6.0. It's a minor bump because the new pub field on Config breaks downstream code that builds a Config with a struct literal. No known consumer does; code_ownership only uses codeowners::runner.

Test plan

  • A new allow_unowned_globs fixture, allowing only app/deprecated/**/*, with 12 integration tests:
    • validate, gv and validate app/deprecated/old.rb pass, and validate with an absolute path to that file passes too.
    • for-file reports the annotated file's team inside the allowed glob and Unowned for the unannotated one.
    • A file with no owner outside the glob still fails, in both validate and validate <files>.
    • Inside the glob, a file with two owners still fails, and so does an annotation naming an unknown team.
    • Without the option, the allowed file is reported.
    • ['**/*'] allows a root-level file, in both validate and validate <files>.
  • Unit tests for parsing the option and its default, and for **/* matching root-level paths.
  • Each production change breaks at least one test when reverted, including matching validate <files> against the raw argument instead of the project-relative path.
  • The main binary fails the new fixture's validate. It ignores the unknown key.
  • code_ownership's main builds against this branch and its specs pass. Through the gem, #140's setup passes validate! and validate!(files:), and annotated files still resolve and appear in CODEOWNERS.
  • cargo test: 192 passed, 3 ignored (also ignored on main). fmt and clippy are clean.
  • CI passes.

Follow-ups

code_ownership needs a release that picks up v0.6.0 before #140 can be closed:

  • Bump the codeowners tag in ext/code_ownership/Cargo.toml, update Cargo.lock, rebuild the checked-in bundle, and bump version.rb.
  • README: the two places that mention only unowned_globs should cover allow_unowned_globs too. Its description of unowned_globs as a TODO list of files to add owners to now fits allow_unowned_globs better.
  • The YARD docs on validate! mention only unowned_globs.
  • Say which version added the key. Older versions ignore it, and validate then still lists the files.
  • Add a spec for #140. write_configuration(allow_unowned_globs: [...]) writes a symbol key that serde ignores, so pass 'allow_unowned_globs' => [...].

code_ownership#140: a repo that assigns owners to only some files had no way
to pass `validate` except `unowned_globs: ['**/*']`, and files matched by
unowned_globs are never read, so that also switched off their `@team`
annotations.

With `allow_unowned_files: true`, a file in owned_globs that no mechanism
assigns an owner is no longer an error, in both `validate` and
`validate <files>`. Every mechanism, file annotations included, still applies
to those files, so the annotations keep working. It defaults to false, and
nothing else changes: unowned_globs files are still never read, which is what
keeps large third-party trees cheap.

Making annotations work inside unowned_globs instead would have meant opening
every file those globs match. Measured on a synthetic tree of tracked vendored
files, that made a cold `generate` about 7x slower at 100k files and 11x at
300k.
@dduugg
dduugg requested a review from a team as a code owner September 28, 2026 17:44
A boolean could only exempt every file at once, while a glob list can also
scope the exemption, e.g. `**/deprecated/**/*`, and `['**/*']` still covers
code_ownership#140's case. It matches how owned_globs and unowned_globs are
configured. The difference from unowned_globs is that files here are still
read, so annotations and every other mechanism still apply; they just don't
have to have an owner.

The globs are only checked for files that have no owner, using the same
glob_match used for owned_globs on every walked file.
The validator imported matches_globs from project_builder, which made
project_builder's copy crate-public just to share a one-line helper. It now
has its own private copy, like runner.rs and file_owner_resolver.rs, and
project_builder's copy is private again. validate_files' relative-path
closure now calls path_utils::relative_to_buf instead of hand-rolling it.

Also pin that `allow_unowned_globs: ['**/*']`, the README's way to allow
every unowned file, covers files at the project root, for both `validate`
and `validate <files>`.
runner.rs, project_builder.rs and the validator each had an identical
private matches_globs. They now all use path_utils::matches_globs, next to
relative_to and relative_to_buf, with a unit test that `**/*` matches both
root-level and nested paths.
@dduugg dduugg changed the title Add allow_unowned_files to permit files with no owner Add allow_unowned_globs to permit files with no owner Sep 28, 2026
Nothing tested that `validate <files>` matched allow_unowned_globs against the
project-relative path. Matching the raw argument instead passed the whole
suite, because the only test passed a path that was already relative. A test
with an absolute path now covers it. validate_files also relativizes each path
once, carrying it through the owned_globs filter, instead of twice per file.

Two more tests pin the README's claim that only the missing-owner check is
waived: a file in an allowed glob still fails when it has more than one owner
or names an invalid team.

The validator now reads allow_unowned_globs from its project instead of
keeping a copy, matches_globs is pub(crate) so it doesn't become public API,
and its imports sit with the other crate imports.
The README said unowned_globs files are never read, but `for-file` reads the
top of the file before ignoring its annotation. It also implied that no
ownership mechanism applies to unowned_globs files, when directory, package
and team-glob rules still cover them through CODEOWNERS. The real differences
are that annotations count and that the other checks still run.

It now also says that unowned_globs takes precedence, and how to move off
`unowned_globs: ['**/*']`, the setup code_ownership#140 started from, where
adding the new key alone changes nothing. The Getting Started example moves a
hand-picked app file from unowned_globs to allow_unowned_globs, since
unowned_globs is meant for third-party and generated code.
A minor bump. Since 0.5.0:

  - `allow_unowned_globs` (#135) is a new config key. It defaults to `[]`, so
    existing configs behave as before, but it adds a `pub` field to `Config`,
    which breaks downstream code that builds a `Config` with a struct literal.
  - Test git no longer follows a hook's GIT_DIR into this repo (#133). This
    only affects the test suite.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

1 participant