diff --git a/.changelog.yml b/.changelog.yml index 8bf1d0e2bc92..9c88ae19f1d5 100644 --- a/.changelog.yml +++ b/.changelog.yml @@ -2,7 +2,7 @@ header_template: "## %new_version / %release_date" -entry_template: "* %title. Pull request [#%pull_request_number](%pull_request_url) by %pull_request_author" +entry_template: "* %title. Pull request [#%pull_request_number](%pull_request_url) by %pull_request_authors" extra_entry: template: "* %title." @@ -12,6 +12,19 @@ already_released_template: Changes already released in %released_in are not repe release_date_format: "%Y-%m-%d" +# Commit authors that are tooling rather than people to credit. GitHub app +# accounts, whose login ends in `[bot]`, are dropped without being listed. +excluded_credit_logins: + - claude + - Copilot + +# The accounts of one person: the account they are credited as, followed by +# the ones they have also committed under, so that one person is not listed +# twice. +credit_aliases: + byroot: + - casperisfine + library_headings: rubygems: RubyGems bundler: Bundler diff --git a/CHANGELOG.md b/CHANGELOG.md index 2795893675f6..03137d327e9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,14 +10,14 @@ Changes already released in 4.0.x are not repeated here. #### Features: -* Add content addressable gems support. Pull request [#9773](https://github.com/ruby/rubygems/pull/9773) by Jenny Shen +* Add content addressable gems support. Pull request [#9773](https://github.com/ruby/rubygems/pull/9773) by Jenny Shen, Harriet Oughton and Gira Chawda * Let the gem and bundle cooldown settings cover each other. Pull request [#9852](https://github.com/ruby/rubygems/pull/9852) by Hiroshi SHIBATA -* Rubygems: Add PQC ML-DSA support for cryptographically signed gems workflow. Pull request [#9697](https://github.com/ruby/rubygems/pull/9697) by Jun Aruga +* Rubygems: Add PQC ML-DSA support for cryptographically signed gems workflow. Pull request [#9697](https://github.com/ruby/rubygems/pull/9697) by Jun Aruga, Kazuki Yamaguchi and Hiroshi SHIBATA * Preserve non-UTF-8 legacy gem metadata. Pull request [#9835](https://github.com/ruby/rubygems/pull/9835) by Stanislav (Stas) Katkov * Add an opt-in OS credential store for gem and bundler credentials. Pull request [#9671](https://github.com/ruby/rubygems/pull/9671) by Hiroshi SHIBATA * Add --cooldown to gem install, update and outdated. Pull request [#9734](https://github.com/ruby/rubygems/pull/9734) by Hiroshi SHIBATA * Adopt the compact index for gem commands. Pull request [#9606](https://github.com/ruby/rubygems/pull/9606) by Hiroshi SHIBATA -* Replace Molinillo with PubGrub for dependency resolution. Pull request [#9402](https://github.com/ruby/rubygems/pull/9402) by Matt Larraz +* Replace Molinillo with PubGrub for dependency resolution. Pull request [#9402](https://github.com/ruby/rubygems/pull/9402) by Matt Larraz and Colby Swandale * Replace the rubygems-specific lockfile parser with Bundler's parser. Pull request [#9564](https://github.com/ruby/rubygems/pull/9564) by Hiroshi SHIBATA * Remove pessimistic versioning in gem command output. Pull request [#9550](https://github.com/ruby/rubygems/pull/9550) by Jeremy Evans * Change Gem::Version#approximate_recommendation to be optimistic. Pull request [#9537](https://github.com/ruby/rubygems/pull/9537) by Jeremy Evans @@ -52,7 +52,7 @@ Changes already released in 4.0.x are not repeated here. #### Features: -* Add content addressable gems support. Pull request [#9773](https://github.com/ruby/rubygems/pull/9773) by Jenny Shen +* Add content addressable gems support. Pull request [#9773](https://github.com/ruby/rubygems/pull/9773) by Jenny Shen, Harriet Oughton and Gira Chawda * Let the gem and bundle cooldown settings cover each other. Pull request [#9852](https://github.com/ruby/rubygems/pull/9852) by Hiroshi SHIBATA * Add a `prune` setting to drop rebuildable install artifacts. Pull request [#9815](https://github.com/ruby/rubygems/pull/9815) by Hiroshi SHIBATA * Add an opt-in OS credential store for gem and bundler credentials. Pull request [#9671](https://github.com/ruby/rubygems/pull/9671) by Hiroshi SHIBATA @@ -63,8 +63,8 @@ Changes already released in 4.0.x are not repeated here. * Reuse RubyGems' vendored SecureRandom in Bundler. Pull request [#9651](https://github.com/ruby/rubygems/pull/9651) by Hiroshi SHIBATA * Reuse RubyGems' vendored URI in Bundler. Pull request [#9650](https://github.com/ruby/rubygems/pull/9650) by Hiroshi SHIBATA * Fix two issues by including the ruby platform variants in the lock file. Pull request [#9556](https://github.com/ruby/rubygems/pull/9556) by Randy Stauner -* Fix plugin installation from gemfile. Pull request [#6957](https://github.com/ruby/rubygems/pull/6957) by Cody Cutrer -* Add plugin hooks for Gemfile evaluation and source fetching. Pull request [#9488](https://github.com/ruby/rubygems/pull/9488) by Hiroshi SHIBATA +* Fix plugin installation from gemfile. Pull request [#6957](https://github.com/ruby/rubygems/pull/6957) by Cody Cutrer, Diogo Fernandes and Hiroshi SHIBATA +* Add plugin hooks for Gemfile evaluation and source fetching. Pull request [#9488](https://github.com/ruby/rubygems/pull/9488) by Hiroshi SHIBATA, Cody Cutrer and Marv * Use optimistic version constraints in bundle gem output. Pull request [#9533](https://github.com/ruby/rubygems/pull/9533) by Jeremy Evans * Extend Gemfile `override` DSL with `:all` target and metadata fields (Phase 2). Pull request [#9530](https://github.com/ruby/rubygems/pull/9530) by Hiroshi SHIBATA * Switch bundle add to use optimistic versioning by default. Pull request [#9526](https://github.com/ruby/rubygems/pull/9526) by Jeremy Evans @@ -72,7 +72,7 @@ Changes already released in 4.0.x are not repeated here. * Add `--no-build-extension` and `--no-install-plugin` options to gem install. Pull request [#9473](https://github.com/ruby/rubygems/pull/9473) by Hiroshi SHIBATA * Include detailed dependencies when gemfile and lockfile are conflicts. Pull request [#9332](https://github.com/ruby/rubygems/pull/9332) by Hiroshi SHIBATA * Implement relative path handling for plugin paths. Pull request [#9299](https://github.com/ruby/rubygems/pull/9299) by Hiroshi SHIBATA -* Bundler/inline: perform installation from a forked child. Pull request [#7941](https://github.com/ruby/rubygems/pull/7941) by Jean byroot Boussier +* Bundler/inline: perform installation from a forked child. Pull request [#7941](https://github.com/ruby/rubygems/pull/7941) by Jean Boussier and Hiroshi SHIBATA * Add global gem cache shared by RubyGems and Bundler. Pull request [#9230](https://github.com/ruby/rubygems/pull/9230) by Anthony Panozzo * Update custom errors with Exception#full_message. Pull request [#8488](https://github.com/ruby/rubygems/pull/8488) by neimadTL @@ -92,7 +92,7 @@ Changes already released in 4.0.x are not repeated here. #### Breaking changes: * Remove external tool version checks from `bundle env`. Pull request [#9593](https://github.com/ruby/rubygems/pull/9593) by Hiroshi SHIBATA -* Bundler: ignore patchlevel kwarg in ruby DSL. Pull request [#6023](https://github.com/ruby/rubygems/pull/6023) by Takuya N +* Bundler: ignore patchlevel kwarg in ruby DSL. Pull request [#6023](https://github.com/ruby/rubygems/pull/6023) by Takuya N and Hiroshi SHIBATA * Unify RubyGems and Bundler documentation. Pull request [#9237](https://github.com/ruby/rubygems/pull/9237) by Hiroshi SHIBATA #### Deprecations: diff --git a/test/test_changelog_generator.rb b/test/test_changelog_generator.rb index 8bdf0d07c924..0e7c5b0d69a0 100644 --- a/test/test_changelog_generator.rb +++ b/test/test_changelog_generator.rb @@ -8,7 +8,7 @@ class ChangelogTest < Test::Unit::TestCase Label = Struct.new(:name) User = Struct.new(:name, :login) - PullRequest = Struct.new(:number, :title, :labels, :merged_at, :user, :html_url) + PullRequest = Struct.new(:number, :title, :labels, :merged_at, :authors, :html_url) def setup @changelog = Changelog.for_release("9.9.0") @@ -91,13 +91,67 @@ def test_unreleased_notes_of_a_per_library_changelog_have_no_library_heading def test_entry_leaves_the_author_empty_when_the_pull_request_has_none changed = pull(1, "rubygems: bug fix") - changed.user = User.new(nil, nil) + changed.authors = [User.new(nil, nil)] notes = @changelog.unreleased_notes_for([changed], extra_entry: nil) assert_include notes, "* Change 1. Pull request [#1](https://github.com/ruby/rubygems/pull/1) by " end + def test_entry_credits_everyone_who_committed_to_the_pull_request + changed = pull(1, "rubygems: bug fix") + changed.authors += [User.new("Someone Else", "else"), User.new("A Third", "third")] + + notes = @changelog.unreleased_notes_for([changed], extra_entry: nil) + + assert_include notes, "* Change 1. Pull request [#1](https://github.com/ruby/rubygems/pull/1) by Someone, Someone Else and A Third" + end + + def test_entry_credits_an_author_recorded_under_two_names_once + changed = pull(1, "rubygems: bug fix") + changed.authors += [User.new("Someone Else", "else"), User.new("The Someone", "Someone")] + + notes = @changelog.unreleased_notes_for([changed], extra_entry: nil) + + assert_include notes, "* Change 1. Pull request [#1](https://github.com/ruby/rubygems/pull/1) by Someone and Someone Else" + end + + def test_entry_credits_an_author_committing_under_two_accounts_as_the_main_one + changed = pull(1, "rubygems: bug fix") + changed.authors = [User.new("Jean byroot Boussier", "casperisfine"), User.new("Jean Boussier", "byroot")] + + notes = @changelog.unreleased_notes_for([changed], extra_entry: nil) + + assert_include notes, "* Change 1. Pull request [#1](https://github.com/ruby/rubygems/pull/1) by Jean Boussier" + end + + def test_entry_credits_a_sub_account_under_its_own_name_when_the_main_one_is_absent + changed = pull(1, "rubygems: bug fix") + changed.authors = [User.new("Jean byroot Boussier", "casperisfine")] + + notes = @changelog.unreleased_notes_for([changed], extra_entry: nil) + + assert_include notes, "* Change 1. Pull request [#1](https://github.com/ruby/rubygems/pull/1) by Jean byroot Boussier" + end + + def test_entry_drops_bot_authors_from_the_credits + changed = pull(1, "rubygems: bug fix") + changed.authors += [User.new("Claude Opus 5", "claude"), User.new("dependabot[bot]", "dependabot[bot]")] + + notes = @changelog.unreleased_notes_for([changed], extra_entry: nil) + + assert_include notes, "* Change 1. Pull request [#1](https://github.com/ruby/rubygems/pull/1) by Someone" + end + + def test_entry_of_a_pull_request_only_bots_worked_on_credits_its_author + changed = pull(1, "rubygems: bug fix") + changed.authors = [User.new(nil, "dependabot[bot]"), User.new("Claude Opus 5", "claude")] + + notes = @changelog.unreleased_notes_for([changed], extra_entry: nil) + + assert_include notes, "* Change 1. Pull request [#1](https://github.com/ruby/rubygems/pull/1) by dependabot[bot]" + end + def test_entry_folds_a_newline_in_a_title_into_one_line changed = pull(1, "rubygems: bug fix") changed.title = "Break out\n## 9.8.0 / 2019-01-01" @@ -467,7 +521,7 @@ def pull(number, *labels) "Change #{number}", labels.map {|label| Label.new(label) }, Time.at(number), - User.new("Someone", "someone"), + [User.new("Someone", "someone")], "https://github.com/ruby/rubygems/pull/#{number}" ) end diff --git a/test/test_release.rb b/test/test_release.rb index d4c0288bc3b3..cf6790122537 100644 --- a/test/test_release.rb +++ b/test/test_release.rb @@ -10,22 +10,60 @@ def test_pull_requests_from_maps_a_listing_record_to_the_fields_the_changelog_us pull = release.send(:pull_requests_from, listing([record]), "master since 2026-01-01").first assert_equal 9852, pull.number + assert_equal "PR_kwDOAAlets8AAAABBxIb1Q", pull.node_id assert_equal "Let the gem and bundle cooldown settings cover each other", pull.title assert_equal "https://github.com/ruby/rubygems/pull/9852", pull.html_url assert_equal ["bundler: bug fix"], pull.labels.map(&:name) assert_equal Time.utc(2026, 9, 4, 2, 23, 29), pull.merged_at - assert_equal "Hiroshi SHIBATA", pull.user.name - assert_equal "hsbt", pull.user.login + assert_equal [Release::User.new("Hiroshi SHIBATA", "hsbt")], pull.authors assert_equal "0602168df08a985b635ea24fb80f9048465f9530", pull.merge_commit_sha end def test_pull_requests_from_falls_back_to_the_login_when_the_author_has_no_name - json = listing([record("author" => { "login" => "app/dependabot", "name" => "" })]) + json = listing([record("author" => { "login" => "someone", "name" => "" })]) pull = release.send(:pull_requests_from, json, "master since 2026-01-01").first - assert_nil pull.user.name - assert_equal "app/dependabot", pull.user.login + assert_equal [Release::User.new(nil, "someone")], pull.authors + end + + def test_pull_requests_from_credits_an_app_account_under_the_login_its_commits_carry + json = listing([record("author" => { "login" => "app/dependabot", "name" => "", "is_bot" => true })]) + + pull = release.send(:pull_requests_from, json, "master since 2026-01-01").first + + assert_equal [Release::User.new(nil, "dependabot[bot]")], pull.authors + end + + def test_credit_commit_authors_credits_them_after_the_pull_request_author + pull = release.send(:pull_requests_from, listing([record]), "master since 2026-01-01").first + + credit(pull, [[github_author("Jenny Shen", "jenshenny")], [github_author("Gira Chawda", "girachawda")]]) + + assert_equal [ + Release::User.new("Hiroshi SHIBATA", "hsbt"), + Release::User.new("Jenny Shen", "jenshenny"), + Release::User.new("Gira Chawda", "girachawda"), + ], pull.authors + end + + def test_credit_commit_authors_skips_an_author_with_no_github_account + pull = release.send(:pull_requests_from, listing([record]), "master since 2026-01-01").first + + credit(pull, [[{ "name" => "License Update", "user" => nil }]]) + + assert_equal [Release::User.new("Hiroshi SHIBATA", "hsbt")], pull.authors + end + + def test_credit_commit_authors_names_an_account_with_no_profile_name_from_its_commit + pull = release.send(:pull_requests_from, listing([record]), "master since 2026-01-01").first + + credit(pull, [[{ "name" => " Ali Firas ", "user" => { "login" => "thesmartshadow", "name" => nil } }]]) + + assert_equal [ + Release::User.new("Hiroshi SHIBATA", "hsbt"), + Release::User.new("Ali Firas", "thesmartshadow"), + ], pull.authors end def test_pull_requests_from_refuses_a_truncated_listing @@ -110,9 +148,23 @@ def listing(records) JSON.dump(records) end + def credit(pull, commits) + node = { + "number" => pull.number, + "commits" => { "nodes" => commits.map {|authors| { "commit" => { "authors" => { "nodes" => authors } } } } }, + } + + release.send(:credit_commit_authors, [pull], [node]) + end + + def github_author(name, login) + { "name" => name, "user" => { "login" => login, "name" => name } } + end + def record(overrides = {}) { "number" => 9852, + "id" => "PR_kwDOAAlets8AAAABBxIb1Q", "title" => "Let the gem and bundle cooldown settings cover each other", "url" => "https://github.com/ruby/rubygems/pull/9852", "labels" => [{ "name" => "bundler: bug fix" }], diff --git a/tool/changelog.rb b/tool/changelog.rb index 8892701f5867..778f8fa74d0c 100644 --- a/tool/changelog.rb +++ b/tool/changelog.rb @@ -194,7 +194,7 @@ def format_entry_for(entry) if pull substitutions["%pull_request_number"] = pull.number.to_s substitutions["%pull_request_url"] = pull.html_url - substitutions["%pull_request_author"] = pull.user.name || pull.user.login + substitutions["%pull_request_authors"] = credits_for(pull) end # An entry is one line. A newline in a title or a display name would start @@ -208,6 +208,44 @@ def format_entry_for(entry) new_entry end + # Everyone who committed to the pull request, not only whoever opened it, so + # that work several people shared is not credited to one of them. A pull + # request a bot opened and committed to on its own keeps the bot, since + # dropping it would leave the entry crediting nobody. + def credits_for(pull) + credited = pull.authors.reject {|author| excluded_credit?(author) } + credited = pull.authors.take(1) if credited.empty? + + names = names_of(credited) + last = names.pop + + names.empty? ? last.to_s : "#{names.join(", ")} and #{last}" + end + + # One person can commit under more than one name, and under more than one + # account, so the account they are credited as decides who is on the entry + # already. They keep the place they were first credited in, but are named + # as that account, rather than as whichever one the entry reached first. + def names_of(authors) + authors.group_by {|author| credited_account(author) }.map do |account, recorded| + author = recorded.find {|candidate| candidate.login.to_s.downcase == account } || recorded.first + + author.name || author.login + end + end + + def credited_account(author) + login = author.login.to_s.downcase + + credit_aliases.fetch(login, login) + end + + def excluded_credit?(author) + login = author.login.to_s + + login.end_with?("[bot]") || excluded_credit_logins.any? {|excluded| excluded.casecmp?(login) } + end + def wrap(text, length, indent) result = [] work = text.dup @@ -413,6 +451,16 @@ def already_released_template @config["already_released_template"] end + def excluded_credit_logins + @config["excluded_credit_logins"] + end + + def credit_aliases + @credit_aliases ||= @config["credit_aliases"].flat_map do |account, logins| + logins.map {|login| [login.downcase, account.downcase] } + end.to_h + end + def library_headings @config["library_headings"] end diff --git a/tool/release.rb b/tool/release.rb index acac8d5ae783..7585136b6c4f 100644 --- a/tool/release.rb +++ b/tool/release.rb @@ -10,13 +10,31 @@ class Release # `gh pr list` record so that no per pull request API call is needed. Label = Struct.new(:name) User = Struct.new(:name, :login) - PullRequest = Struct.new(:number, :title, :html_url, :labels, :merged_at, :user, :merge_commit_sha) + PullRequest = Struct.new(:number, :node_id, :title, :html_url, :labels, :merged_at, :authors, :merge_commit_sha) # `gh pr list` refuses to return more than this many results and truncates # silently once a window holds more, so a listing that reaches the cap is # refused rather than cut from a partial one. MERGED_PULL_REQUEST_LIMIT = 1000 + # Pull requests whose commits are read in one query. Every commit and every + # author a query can reach counts against a node budget, and a query over + # that budget is rejected rather than answered partially. + COMMIT_AUTHOR_BATCH_SIZE = 100 + + COMMIT_AUTHORS_QUERY = <<~GRAPHQL + query($ids: [ID!]!) { + nodes(ids: $ids) { + ... on PullRequest { + number + commits(first: 100) { + nodes { commit { authors(first: 10) { nodes { name user { login name } } } } } + } + } + } + } + GRAPHQL + module GithubAPI def gh_client @gh_client ||= begin @@ -408,7 +426,50 @@ def release_pull_request_body end def relevant_pull_requests - @relevant_pull_requests ||= unreleased_pull_requests.select {|pull| @changelog.labelled?(pull) }.sort_by(&:merged_at) + @relevant_pull_requests ||= begin + pulls = unreleased_pull_requests.select {|pull| @changelog.labelled?(pull) }.sort_by(&:merged_at) + add_commit_authors!(pulls) + pulls + end + end + + # A changelog entry credits everyone who committed to the pull request, not + # only whoever opened it. A listing as wide as the one below cannot carry + # commits, so they are read here instead, in batches and only for the pull + # requests that reach the changelog. + def add_commit_authors!(pulls) + pulls.each_slice(COMMIT_AUTHOR_BATCH_SIZE) do |batch| + ids = batch.flat_map {|pull| ["-F", "ids[]=#{pull.node_id}"] } + + json = IO.popen(["gh", "api", "graphql", "-f", "query=#{COMMIT_AUTHORS_QUERY}", *ids], &:read) + raise "Failed to list the commits of #{batch.map(&:number).join(", ")}" unless $?.success? + + credit_commit_authors(batch, JSON.parse(json).dig("data", "nodes")) + end + end + + def credit_commit_authors(pulls, nodes) + by_number = pulls.to_h {|pull| [pull.number, pull] } + + nodes.each do |node| + pull = by_number[node["number"]] + pull.authors += commit_authors_of(node) + end + end + + # Only the authors GitHub resolved to an account. A commit made under an + # address no account carries names whoever configured that clone, which in + # practice is a machine account or a local alias rather than a contributor. + def commit_authors_of(node) + node.dig("commits", "nodes").flat_map {|commit| commit.dig("commit", "authors", "nodes") }.filter_map do |author| + user = author["user"] + next unless user + + # An account carrying no profile name is credited under its commit name. + name = (user["name"] || author["name"]).to_s.strip + + User.new(name.empty? ? nil : name, user["login"]) + end end # Pull requests included in this release. `mergeCommit.oid` is the commit a @@ -454,7 +515,7 @@ def merged_pull_requests(base, since_ref) since = (Time.iso8601(committed_at) - 86_400).utc.strftime("%Y-%m-%d") - json = `gh pr list --repo ruby/rubygems --state merged --base #{base} --search 'merged:>=#{since}' --limit #{MERGED_PULL_REQUEST_LIMIT} --json number,title,labels,mergeCommit,mergedAt,author,url` + json = `gh pr list --repo ruby/rubygems --state merged --base #{base} --search 'merged:>=#{since}' --limit #{MERGED_PULL_REQUEST_LIMIT} --json number,id,title,labels,mergeCommit,mergedAt,author,url` raise "Failed to list pull requests merged into #{base} since #{since}" unless $?.success? pull_requests_from(json, "#{base} since #{since}") @@ -477,14 +538,20 @@ def build_pull_request(record) # `gh` reports a missing author name as an empty string, while the changelog # entry template expects to fall back to the login when there is none. name = author["name"] + # `gh` writes an app account's login as `app/`, while the commits of + # that account carry the `[bot]` form, so the two records of one + # account only meet once this is rewritten. + login = author["login"] + login = "#{login.delete_prefix("app/")}[bot]" if author["is_bot"] PullRequest.new( record["number"], + record["id"], record["title"], record["url"], record["labels"].map {|label| Label.new(label["name"]) }, Time.iso8601(record["mergedAt"]), - User.new(name.to_s.empty? ? nil : name, author["login"]), + [User.new(name.to_s.empty? ? nil : name, login)], record["mergeCommit"]["oid"] ) end