From 8aae394506d03b5f6066e0346c98fdbe9928b2ab Mon Sep 17 00:00:00 2001 From: John Zittlau Date: Fri, 12 Jun 2026 10:31:03 -0600 Subject: [PATCH] LIBTRACK-137 Resolve gemspec ownership from owned_globs, not CODEOWNERS The nightly gemfile analysis crashed in add_ownership_from_gemspecs with "undefined method 'raw_hash' for nil", aborting the static-analysis rake task before LibraryTracking.upload ran. That is why the Jobber gemfile project stopped updating. Root cause: code_ownership 2.x made CodeOwnership.for_file default to from_codeowners: true, which resolves ownership from the generated .github/CODEOWNERS file. Jobber hand-curates CODEOWNERS (skip_codeowners_validation: true), so it contains no gems/** entries and for_file returned nil for every gemspec. The code then called team.raw_hash on nil. Pass from_codeowners: false so ownership is resolved from the team owned_globs config (every Jobber gemspec is in fact owned). Also resolve the owner once per gemspec and skip with a warning if a gemspec ever genuinely has no owner, so a single unowned file can no longer crash the whole run. Co-Authored-By: Claude Opus 4.8 Co-Authored-By: Amplify 2.1.3 --- lib/library_version_analysis/gemfile.rb | 13 ++++++- spec/gemfile_spec.rb | 52 +++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/lib/library_version_analysis/gemfile.rb b/lib/library_version_analysis/gemfile.rb index 3845420..cf33ce1 100644 --- a/lib/library_version_analysis/gemfile.rb +++ b/lib/library_version_analysis/gemfile.rb @@ -196,6 +196,17 @@ def add_ownership_from_gemspecs(parsed_results) default_owner = LibraryVersionAnalysis::Configuration.get(:default_owner_name) Dir.glob(File.join("gems", "**", "*.gemspec")) do |gemspec_file| + # from_codeowners: false resolves ownership from the team owned_globs config + # rather than the generated .github/CODEOWNERS file. Repos that hand-curate + # CODEOWNERS (skip_codeowners_validation) otherwise return nil for every gem. + team = CodeOwnership.for_file(gemspec_file, from_codeowners: false) + if team.nil? + warn "No code owner for #{gemspec_file}; skipping gemspec ownership assignment" + next + end + + group = team.raw_hash["group"] + File.foreach(gemspec_file) do |line| scan_result = line.scan(/spec.add_.*dependency\s*"(\S*)"/) @@ -204,8 +215,6 @@ def add_ownership_from_gemspecs(parsed_results) library = scan_result[0][0] next if(parsed_results.has_key?(library) && parsed_results[library].owner != default_owner) - team = CodeOwnership.for_file(gemspec_file) - group = team.raw_hash["group"] parsed_results[library]&.owner = group.start_with?(":") ? group : ":#{group}" end end diff --git a/spec/gemfile_spec.rb b/spec/gemfile_spec.rb index 8ea9d7d..e04b6a6 100644 --- a/spec/gemfile_spec.rb +++ b/spec/gemfile_spec.rb @@ -130,6 +130,58 @@ def do_compare(result:, owner:, current_version:, latest_version:, major:, minor allow(LibraryVersionAnalysis::CheckVersionStatus).to receive(:legacy?).and_return(false) end + describe "#add_ownership_from_gemspecs" do + let(:analyzer) { LibraryVersionAnalysis::Gemfile.new("test") } + let(:gemspec_contents) do + <<~DOC + spec.add_dependency "aasm" + spec.add_development_dependency "packwerk" + DOC + end + + before(:each) do + allow(LibraryVersionAnalysis::Configuration).to receive(:get).and_call_original + allow(LibraryVersionAnalysis::Configuration).to receive(:get) + .with(:default_owner_name).and_return(:unspecified) + allow(Dir).to receive(:glob).and_call_original + allow(Dir).to receive(:glob).with(File.join("gems", "**", "*.gemspec")) + .and_yield("gems/example/example.gemspec") + allow(File).to receive(:foreach).and_call_original + allow(File).to receive(:foreach).with("gems/example/example.gemspec") + .and_yield('spec.add_dependency "aasm"') + .and_yield('spec.add_development_dependency "packwerk"') + end + + it "assigns the gemspec owner's group to each dependency" do + team = double("team", raw_hash: { "group" => "self_serve" }) + allow(CodeOwnership).to receive(:for_file) + .with("gems/example/example.gemspec", from_codeowners: false).and_return(team) + + parsed_results = { + "aasm" => LibraryVersionAnalysis::Versionline.new(owner: :unspecified), + "packwerk" => LibraryVersionAnalysis::Versionline.new(owner: :unspecified), + } + + analyzer.send(:add_ownership_from_gemspecs, parsed_results) + + expect(parsed_results["aasm"].owner).to eq(":self_serve") + expect(parsed_results["packwerk"].owner).to eq(":self_serve") + end + + it "skips the gemspec without raising when it has no code owner" do + allow(CodeOwnership).to receive(:for_file) + .with("gems/example/example.gemspec", from_codeowners: false).and_return(nil) + + parsed_results = { + "aasm" => LibraryVersionAnalysis::Versionline.new(owner: :unspecified), + } + + expect { analyzer.send(:add_ownership_from_gemspecs, parsed_results) } + .not_to raise_error + expect(parsed_results["aasm"].owner).to eq(:unspecified) + end + end + describe "#add_dependency_graph" do it "should reverse simple chain" do c = SpecSetStruct.new(name: "c")