diff --git a/Cargo.lock b/Cargo.lock index 3388824..5327792 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,17 +2,6 @@ # It is not intended for manual editing. version = 4 -[[package]] -name = "ahash" -version = "0.7.8" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "891477e0c6a8957309ee5c45a6368af3ae14bb510732d2684ffa19af310920f9" -dependencies = [ - "getrandom 0.2.16", - "once_cell", - "version_check", -] - [[package]] name = "aho-corasick" version = "1.1.3" @@ -200,8 +189,8 @@ dependencies = [ [[package]] name = "codeowners" -version = "0.3.3" -source = "git+https://github.com/rubyatscale/codeowners-rs.git?tag=v0.3.3#437a527fe02b620e77723cc4e3090bdb0a42bdb3" +version = "0.5.0" +source = "git+https://github.com/rubyatscale/codeowners-rs.git?tag=v0.5.0#699075bc607c8c6020761e5d962084f88c6f2fb4" dependencies = [ "clap", "clap_derive", @@ -302,9 +291,9 @@ dependencies = [ [[package]] name = "error-stack" -version = "0.5.0" +version = "0.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fe413319145d1063f080f27556fd30b1d70b01e2ba10c2a6e40d4be982ffc5d1" +checksum = "d01a8d4d427153bae0c38ca68d7912cbf9903fdcc97ecac18801738b3656a46d" dependencies = [ "anyhow", "rustc_version", @@ -325,17 +314,6 @@ version = "2.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "37909eebbb50d72f9059c3b6d82c0463f2ff062c9e95845c43a6c9c0355411be" -[[package]] -name = "getrandom" -version = "0.2.16" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "335ff9f135e4384c8150d6f27c6daed433577f86b4750418338c01a1a2528592" -dependencies = [ - "cfg-if", - "libc", - "wasi", -] - [[package]] name = "getrandom" version = "0.3.4" @@ -367,15 +345,6 @@ dependencies = [ "regex-syntax", ] -[[package]] -name = "hashbrown" -version = "0.12.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8a9ee70c43aaf417c914396645a0fa852624801b24ebb7ae78fe8272889ac888" -dependencies = [ - "ahash", -] - [[package]] name = "hashbrown" version = "0.16.0" @@ -411,7 +380,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4b0f83760fb341a774ed326568e19f5a863af4a952def8c39f9ab92fd95b88e5" dependencies = [ "equivalent", - "hashbrown 0.16.0", + "hashbrown", ] [[package]] @@ -484,15 +453,6 @@ version = "0.4.28" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "34080505efa8e45a4b816c349525ebe327ceaa8559756f0356cba97ef3bf7432" -[[package]] -name = "lru" -version = "0.7.8" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e999beba7b6e8345721bd280141ed958096a2e4abdf74f67ff4ce49b4b54e47a" -dependencies = [ - "hashbrown 0.12.3", -] - [[package]] name = "magnus" version = "0.8.2" @@ -538,7 +498,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f8d1d5792299bab3f8b5d88d1b7a7cb50ad7ef039a8c4d45a6b84880a6526276" dependencies = [ "lazy_static", - "lru", "memoize-inner", ] @@ -902,7 +861,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2d31c77bdf42a745371d260a26ca7163f1e0924b64afa0b688e61b5a9fa02f16" dependencies = [ "fastrand", - "getrandom 0.3.4", + "getrandom", "once_cell", "rustix", "windows-sys 0.61.2", @@ -1002,12 +961,6 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ba73ea9cf16a25df0c8caa16c51acb937d5712a8429db78a3ee29d5dcacd3a65" -[[package]] -name = "version_check" -version = "0.9.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0b928f33d975fc6ad9f86c8f283853ad26bdd5b10b7f1542aa2fa15e2289105a" - [[package]] name = "walkdir" version = "2.5.0" @@ -1018,12 +971,6 @@ dependencies = [ "winapi-util", ] -[[package]] -name = "wasi" -version = "0.11.1+wasi-snapshot-preview1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ccf3ec651a847eb01de73ccad15eb7d99f80485de043efb2f370cd654f4ea44b" - [[package]] name = "wasip2" version = "1.0.1+wasi-0.2.4" diff --git a/ext/code_ownership/Cargo.toml b/ext/code_ownership/Cargo.toml index 3f29bef..809ded3 100644 --- a/ext/code_ownership/Cargo.toml +++ b/ext/code_ownership/Cargo.toml @@ -17,7 +17,7 @@ rb-sys = { version = "0.9.111", features = [ magnus = { version = "0.8" } serde = { version = "1.0.219", features = ["derive"] } serde_magnus = "0.10" -codeowners = { git = "https://github.com/rubyatscale/codeowners-rs.git", tag = "v0.3.3" } +codeowners = { git = "https://github.com/rubyatscale/codeowners-rs.git", tag = "v0.5.0" } [dev-dependencies] rb-sys = { version = "0.9.117", features = [ diff --git a/ext/code_ownership/src/lib.rs b/ext/code_ownership/src/lib.rs index 626c438..95e160b 100644 --- a/ext/code_ownership/src/lib.rs +++ b/ext/code_ownership/src/lib.rs @@ -74,6 +74,10 @@ fn version() -> String { runner::version() } +fn clear_team_cache() { + runner::clear_team_cache(); +} + fn validate(ruby: &Ruby, files: Option>) -> Result { let run_config = build_run_config(); let files_vec = files.unwrap_or_default(); @@ -90,9 +94,16 @@ fn generate_and_validate(ruby: &Ruby, files: Option>, skip_stage: bo fn validate_result(ruby: &Ruby, run_result: &runner::RunResult) -> Result { if !run_result.validation_errors.is_empty() { + // codeowners-rs reports the stale-CODEOWNERS diff via info_messages; the raise is our only channel for it. + let messages: Vec<&str> = run_result + .validation_errors + .iter() + .chain(&run_result.info_messages) + .map(String::as_str) + .collect(); Err(Error::new( ruby.exception_runtime_error(), - run_result.validation_errors.join("\n"), + messages.join("\n"), )) } else if !run_result.io_errors.is_empty() { Err(Error::new( @@ -129,6 +140,7 @@ fn init(ruby: &Ruby) -> Result<(), Error> { module.define_singleton_method("for_team", function!(for_team, 1))?; module.define_singleton_method("version", function!(version, 0))?; module.define_singleton_method("teams_for_files", function!(teams_for_files, 1))?; + module.define_singleton_method("clear_team_cache", function!(clear_team_cache, 0))?; Ok(()) } diff --git a/lib/code_ownership.rb b/lib/code_ownership.rb index 67f666b..80bc414 100644 --- a/lib/code_ownership.rb +++ b/lib/code_ownership.rb @@ -317,10 +317,13 @@ def self.for_package(package) # Namely, the set of files, packages, and directories which are tracked for ownership should not change. # The primary reason this is helpful is for clients of CodeOwnership who want to test their code, and each test context # has different ownership and tracked files. + # It also clears the team files that codeowners-rs caches for `for_file(..., from_codeowners: false)`, + # so call it after adding or editing team files in a long-lived process. sig { void } def self.bust_caches! Private::FilePathTeamCache.bust_cache! Private::FilePathFinder.instance_variable_set(:@pwd, nil) Private::FilePathFinder.instance_variable_set(:@pwd_prefix, nil) + ::RustCodeOwners.clear_team_cache end end diff --git a/lib/code_ownership/code_ownership.bundle b/lib/code_ownership/code_ownership.bundle index 1b42d3e..84c7a80 100755 Binary files a/lib/code_ownership/code_ownership.bundle and b/lib/code_ownership/code_ownership.bundle differ diff --git a/lib/code_ownership/version.rb b/lib/code_ownership/version.rb index 7815856..667e740 100644 --- a/lib/code_ownership/version.rb +++ b/lib/code_ownership/version.rb @@ -2,5 +2,5 @@ # frozen_string_literal: true module CodeOwnership - VERSION = '2.1.4' + VERSION = '2.2.0' end diff --git a/sorbet/rbi/manual.rbi b/sorbet/rbi/manual.rbi index fe77997..feba3a1 100644 --- a/sorbet/rbi/manual.rbi +++ b/sorbet/rbi/manual.rbi @@ -1,3 +1,5 @@ +# typed: false + class Hash def to_json(*_args); end end @@ -21,5 +23,9 @@ module RustCodeOwners def teams_for_files(files) end + + sig { void } + def clear_team_cache + end end end diff --git a/spec/lib/code_ownership_spec.rb b/spec/lib/code_ownership_spec.rb index 0ec059c..f1b46d7 100644 --- a/spec/lib/code_ownership_spec.rb +++ b/spec/lib/code_ownership_spec.rb @@ -223,6 +223,34 @@ end end + describe '.for_file with from_codeowners: false and package ownership' do + subject { CodeOwnership.for_file(file_path, from_codeowners: false) } + + let(:file_path) { 'packs/outer/inner/file.rb' } + + before do + create_non_empty_application + write_file('packs/outer/package.yml', "owner: Foo\n") + write_file(file_path) + end + + context 'when the package sets metadata.owner' do + before { write_file('packs/outer/inner/package.yml', { 'metadata' => { 'owner' => 'Bar' } }.to_yaml) } + + it 'returns the metadata owner' do + expect(subject).to eq CodeTeams.find('Bar') + end + end + + context 'when the package sets conflicting owners' do + before { write_file('packs/outer/inner/package.yml', { 'owner' => 'Foo', 'metadata' => { 'owner' => 'Bar' } }.to_yaml) } + + it 'does not fall through to the enclosing package' do + expect(subject).to be_nil + end + end + end + describe '.for_class' do subject { described_class.for_class(klass) } @@ -578,6 +606,45 @@ expect(error.message).not_to include('`codeowners generate`') end end + + it 'includes the required CODEOWNERS changes after the headline' do + expect { CodeOwnership.validate!(autocorrect: false) }.to raise_error(RuntimeError) do |error| + expect(error.message).to match(/CODEOWNERS out of date.*The following changes are required \(- current, \+ expected\):/m) + expect(error.message).to include("\n+/packs/my_pack/new_file.rb @MyOrg/bar-team") + end + end + + it 'regenerates the CODEOWNERS file when autocorrecting' do + expect { CodeOwnership.validate!(stage_changes: false) }.not_to raise_error + expect(codeowners_path.read).to include('/packs/my_pack/new_file.rb @MyOrg/bar-team') + expect { CodeOwnership.validate!(autocorrect: false) }.not_to raise_error + end + end + end + + describe '.bust_caches!' do + let(:file_path) { 'app/services/thing.rb' } + + def write_team(name, owned_globs: []) + config = { 'name' => name, 'github' => { 'team' => "@MyOrg/#{name.downcase}-team" }, 'owned_globs' => owned_globs } + write_file("config/teams/#{name.downcase}.yml", config.to_yaml) + end + + before do + write_configuration + write_file(file_path) + write_team('Foo', owned_globs: ['app/services/**']) + write_team('Bar') + end + + it 'makes a team file change visible to for_file within one process' do + expect(CodeOwnership.for_file(file_path, from_codeowners: false)).to eq CodeTeams.find('Foo') + + write_team('Foo') + write_team('Bar', owned_globs: ['app/services/**']) + CodeOwnership.bust_caches! + + expect(CodeOwnership.for_file(file_path, from_codeowners: false)).to eq CodeTeams.find('Bar') end end end