Repository navigation
Raise individual pull requests for group members that update-types rejects - #16113
Conversation
…jects A group that sets `update-types` cannot tell whether it claims a dependency until an update checker has resolved the latest version, so `DependencyGroup#contains?` matches on patterns alone. When `GroupUpdateAllVersions` skips such a group -- because it already has an open pull request, or because it produced no change -- it marks every pattern match as handled, which also swallows the dependencies the group is going to reject. Those drop out of `ungrouped_dependencies` and are never raised individually, so a repository whose minor/patch group has an open pull request silently stops receiving the major updates that the group excludes. Defer those dependencies instead of marking them handled. The snapshot records which groups are still waiting on them, they stay in `ungrouped_dependencies`, and the individual update run -- which resolves the latest version anyway -- applies the group's update-types rules to decide between leaving the update to the group and raising a pull request of its own. The semver rules move to `Dependabot::Updater::SemverGroupingRules` so both the grouped and the individual paths share them. The new behaviour is gated behind the `individual_prs_for_semver_excluded_dependencies` experiment.
4d2473e to
42906ec
Compare
|
Heads up on the one red check: The same job fails identically on unrelated branches right now, including The diff is upstream drift in transitive dependencies of That smoke test also has no Every other check is green, including |
| unless version_class.correct?(dependency.version.to_s) && version_class.correct?(checker.latest_version) | ||
| return false |
c96ebb6
into
dependabot:main
What are you trying to accomplish?
Fixes #14202.
A group that restricts
update-typesis documented to leave the updates it rejects to individual pull requests:That is what happens on the first run of a job. It stops happening as soon as the group has an open pull request — which is the steady state for a
patterns: ["*"]minor/patch group — so in practice the rejected updates are never raised at all. The same happens whenever the group itself produces no change, e.g. when every remaining update in it is a major one.GroupUpdateAllVersionsdefers a group that already has an open pull request to its own refresh job and callsDependencySnapshot#mark_group_handled, which marks every dependency matching the group'spatternsas handled.DependencyGroup#contains?cannot takeupdate-typesinto account — the semver level of an update is only known once an update checker has resolved the latest version — so dependencies the group is going to reject are marked handled as well. They drop out ofungrouped_dependenciesand get no pull request from either path.compile_updates_foris careful not to mark those dependencies as handled (group_update_creation.rb#L306, and the spec asserting it atgroup_update_creation_spec.rb#L658), so the intent is already in the code; the blanket marking one level up overrides it.This is the same bug #14475 fixed for NuGet ("if a group matches but the update type isn't allowed in that group, then proceed as an ungrouped update"). This change brings the Ruby updater to parity for every other ecosystem.
The change. When
GroupUpdateAllVersionsskips a group that setsupdate-types, its members are deferred rather than marked handled:DependencySnapshotrecords which groups are still waiting on them and they stay inungrouped_dependencies.UpdateAllVersions— which resolves the latest version for those dependencies anyway — then applies the group'supdate-typesrules: if the group accepts the update, the group's own job will raise it and no individual pull request is opened; if the group rejects it, the dependency is updated individually, exactly as it is on a first run today.The semver rules themselves are unchanged. They moved out of
GroupUpdateCreationintoDependabot::Updater::SemverGroupingRulesso both the grouped and the individual paths apply the same logic;semver_rules_allow_grouping?keeps its signature and delegates.Anything you want to highlight for special attention from reviewers?
individual_prs_for_semver_excluded_dependenciesand off by default, since this changes how many pull requests affected repositories receive. Happy to drop the gate if you would rather ship it directly.mark_group_handledcall sites inGroupUpdateAllVersionsopt in via the newdefer_update_types:keyword — those are the ones that decide whether a dependency gets a pull request at all. The refresh operation's call site is untouched, since a refresh job never runs the individual update path.CreateGroupUpdatePullRequestalready does for them when the group has no open pull request, and it only applies to groups that setupdate-types.semver_rules_allow_grouping?; if that lands first I am happy to rebase the extraction on top of it.How will you know you've accomplished your goal?
silent/tests/testdata/vu-group-semver-existing-pr.txtis a regression test for exactly the reported scenario: aminor/patchgroup with an open pull request, and a dependency whose only available update is a major one. Onmainit produces no pull request at all; with this change it produces the individual major pull request.Also covered by unit specs for the deferral (
dependency_snapshot_spec.rb) and for both outcomes of the re-check in the individual run (update_all_versions_spec.rb). The existingvu-group-semver*integration tests and the grouping specs are unchanged and still pass.Checklist