googleapis / googleapis/release-please

node-workspace calculates new candidates incorrectly, resulting in broken or failed PRs

Open
#2,109 1 comment 0 reactions 1 assignee Claimed by @chingor13 View on GitHub
priority: p3 type: bug
Dominant language
TypeScript
Stars
7.5k
Forks
588
Avg merge
12h 16m
Merged PRs (30d)
7

Description

During a manifest release, sometimes new candidate pull requests are made because their dependencies are updating. These are made by calling the workspace's `newCandidate` method.

However, the logic for a new candidate in node-workspace is missing some of the information required to correctly calculate what the PR should look like. For [example](
https://github.com/googleapis/release-please/blob/main/src/plugins/node-workspace.ts#L309), the branch name is calculated from the target branch:

```
headRefName: BranchName.ofTargetBranch(this.targetBranch).toString(),
```

But, it should instead be calculated using the component, as it is in [the base strategy](https://github.com/googleapis/release-please/blob/main/src/strategies/base.ts#L295)

```
const branchComponent = await this.getBranchComponent();
const branchName = branchComponent
? BranchName.ofComponentTargetBranch(branchComponent, this.targetBranch)
: BranchName.ofTargetBranch(this.targetBranch);
```

This is causing PRs to clobber each other, resulting in incorrect behaviour:

```
✔ Updating 4 packages
❯ package: @contract-case/case-boundary
❯ version: 0.9.0 from release-please
❯ package: @contract-case/case-connector
❯ version: 0.6.6 from release-please
❯ package: @contract-case/cli
❯ version: 0.1.7 forced bump
❯ package: @contract-case/contract-case-jest
❯ version: 0.11.1 forced bump
✔ Updating exising candidate pull request for @contract-case/case-boundary, path: packages/case-boundary
✔ Updating @contract-case/case-boundary to 0.9.0
✔ Updating exising candidate pull request for @contract-case/case-connector, path: packages/case-connector
✔ Updating @contract-case/case-connector to 0.6.6
✔ @contract-case/case-connector.@contract-case/case-boundary updated to 0.9.0
✔ appending dependency notes to changelog
✔ appending dependency notes to changelog
✔ Creating new candidate pull request for @contract-case/cli
✔ Updating @contract-case/cli to 0.1.7
✔ @contract-case/cli.@contract-case/case-boundary updated to 0.9.0
✔ Creating new candidate pull request for @contract-case/contract-case-jest
✔ Updating @contract-case/contract-case-jest to 0.11.1
.......
✔ Successfully updated reference release-please--branches--main--components--@contract-case/case-boundary to 5934f1a6e7167bcf4a1173054282104835e47e7d
✔ Successfully updated reference release-please--branches--main to 0cd8a6f3d5484d493dfed9da03a2243283a3aae3
✔ Successfully updated reference release-please--branches--main to 4f900c579037f4a192fa6a8e2117007c52cabacd
✔ Successfully updated reference release-please--branches--main--components--@contract-case/case-connector to e122a074ea807232fa0c4fe755731138e1cb819c
```
^ from two packages updating (@contract-case/case-boundary , and @contract-case/case-connector), and two other packages getting forced version bumps because they depend on the updating packages.

There are four pull requests created, but the PRs from the forced bumps (https://github.com/case-contract-testing/contract-case/pull/264, https://github.com/case-contract-testing/contract-case/pull/263) have:

* Missing component and version information in the titles
* Missing version headers in the changelog and PR bodies
* (incorrectly) identical file changes, because of the collision on branch name.

Sometimes this collision results in a failure instead - here's a snippet from a different run with the same packages updating:

```
✔ Updating reference heads/release-please--branches--main to bcd96fa20258b5599e9e88710cb7b839825aecdf
✔ Successfully created commit. See commit at https://api.github.com/repos/case-contract-testing/contract-case/git/commits/0d874e328fea71a84b80e21353ddc2fc91ee9c08
✔ Updating reference heads/release-please--branches--main to 0d874e328fea71a84b80e21353ddc2fc91ee9c08
✔ Successfully created commit. See commit at https://api.github.com/repos/case-contract-testing/contract-case/git/commits/3f7e1403ed7d20ba6c2929d9bb8ea71f79f44b42
✔ Updating reference heads/release-please--branches--main--components--@contract-case/case-connector to 3f7e1403ed7d20ba6c2929d9bb8ea71f79f44b42
✔ Successfully created commit. See commit at https://api.github.com/repos/case-contract-testing/contract-case/git/commits/fb8044150920468f6f49d99f3f54c9f7f87951e1
✔ Updating reference heads/release-please--branches--main--components--@contract-case/case-boundary to fb8044150920468f6f49d99f3f54c9f7f87951e1
✔ Successfully updated reference release-please--branches--main to bcd96fa20258b5599e9e88710cb7b839825aecdf
Error: release-please failed: Error updating ref heads/release-please--branches--main to 0d874e328fea71a84b80e21353ddc2fc91ee9c08
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.