diff --git a/src/github.ts b/src/github.ts index 32535268b..f995501d4 100644 --- a/src/github.ts +++ b/src/github.ts @@ -12,7 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -import {commitAndPush} from 'code-suggester/build/src/github/commit-and-push'; +import { + createTree, + generateTreeObjects, +} from 'code-suggester/build/src/github/commit-and-push'; import {addLabels} from 'code-suggester/build/src/github/labels'; import {setupLogger} from 'code-suggester/build/src/logger'; @@ -1275,27 +1278,7 @@ export class GitHub { updates: Update[], options?: CreatePullRequestOptions ): Promise => { - const changes = await this.buildChangeSet(updates, refBranch); - - // create release branch - const pullRequestBranchSha = await this.forkBranch( - pullRequest.headBranchName, - refBranch - ); - - // commit and push changeset - await commitAndPush( - this.octokit, - pullRequestBranchSha, - changes, - { - branch: pullRequest.headBranchName, - repo: this.repository.repo, - owner: this.repository.owner, - }, - message, - true - ); + await this.upsertReleaseBranch(pullRequest, refBranch, message, updates); // create pull request, unless one already exists let pullRequestNumber: number; @@ -1334,6 +1317,131 @@ export class GitHub { } ); + upsertReleaseBranch = wrapAsync( + async ( + pullRequest: PullRequest, + refBranch: string, + message: string, + updates: Update[] + ): Promise => { + this.logger.debug( + { + pullRequest: pullRequest.headBranchName, + refBranch: refBranch, + message: message, + updates: updates.length, + }, + 'upserting release branch' + ); + + const changes = await this.buildChangeSet(updates, refBranch); + + const refSHA = await this.getBranchSha(refBranch); + + this.logger.debug( + { + refBranch: refBranch, + refSHA: refSHA, + changes: changes.size, + }, + 'found ref branch SHA' + ); + + if (!refSHA) { + throw new Error( + `could not find branch ${refBranch} in repository ${this.repository.owner}/${this.repository.repo}` + ); + } + + const tree = generateTreeObjects(changes); + + const treeSha = await createTree( + this.octokit, + this.repository, + refSHA, + tree + ); + + this.logger.debug( + { + treeSha: treeSha, + }, + 'created tree' + ); + + const {data: newCommit} = await this.octokit.git.createCommit({ + ...this.repository, + message, + tree: treeSha, + parents: [refSHA], + }); + + this.logger.debug( + { + newCommit: newCommit.sha, + }, + 'created commit' + ); + + const ref = `heads/${pullRequest.headBranchName}`; + try { + // try to update existing branch + await this.octokit.git.updateRef({ + ...this.repository, + ref, + sha: newCommit.sha, + force: true, + }); + + this.logger.debug( + { + ref: ref, + newCommit: newCommit.sha, + }, + 'updated existing branch' + ); + } catch (error) { + if (this.isRefDoesNotExistError(error)) { + this.logger.debug( + { + ref: ref, + }, + 'branch does not exist, creating it' + ); + + // branch doesn't exist, create it + await this.octokit.git.createRef({ + ...this.repository, + ref: `refs/${ref}`, + sha: newCommit.sha, + }); + + this.logger.debug( + { + ref: ref, + newCommit: newCommit.sha, + }, + 'created new branch' + ); + } else { + throw error; + } + } + } + ); + + isRefDoesNotExistError = (error: unknown): boolean => { + return ( + (error && + typeof error === 'object' && + 'status' in error && + error.status === 422 && + 'message' in error && + typeof error.message === 'string' && + error.message.includes('does not exist')) === true + ); + }; + /** * Fetch a pull request given the pull number * @param {number} number The pull request number diff --git a/src/strategies/base.ts b/src/strategies/base.ts index 4af681837..b169f99f5 100644 --- a/src/strategies/base.ts +++ b/src/strategies/base.ts @@ -22,6 +22,7 @@ import { MANIFEST_PULL_REQUEST_TITLE_PATTERN, ExtraFile, DEFAULT_CUSTOM_VERSION_LABEL, + DEFAULT_RELEASE_PLEASE_MANIFEST, } from '../manifest'; import {DefaultVersioningStrategy} from '../versioning-strategies/default'; import {DefaultChangelogNotes} from '../changelog-notes/default'; @@ -274,12 +275,14 @@ export abstract class BaseStrategy implements Strategy { labels = [], latestRelease, draft, + manifestPath, }: { commits: Commit[]; latestRelease?: Release; draft?: boolean; labels?: string[]; existingPullRequest?: PullRequest; + manifestPath?: string; }): Promise { this.logger.info(`Considering: ${commits.length} raw commits`); @@ -388,7 +391,13 @@ To set a custom version be sure to use the [semantic versioning format](https:// } else { // look at the manifest from release branch and compare against version from PR title try { - if (newVersion.toString() !== existingPRTitleVersion.toString()) { + const manifest = + (await this.github.getFileJson>( + manifestPath || DEFAULT_RELEASE_PLEASE_MANIFEST, + existingPullRequest.headBranchName + )) || {}; + const componentVersion = manifest[component || '.']; + if (componentVersion !== existingPRTitleVersion?.toString()) { // version from title has been edited, add custom version label, a comment, and use the title version this.github.addIssueLabels( [DEFAULT_CUSTOM_VERSION_LABEL], diff --git a/src/strategies/java.ts b/src/strategies/java.ts index 06f1d623c..b114b8cdb 100644 --- a/src/strategies/java.ts +++ b/src/strategies/java.ts @@ -79,12 +79,14 @@ export class Java extends BaseStrategy { labels = [], latestRelease, draft, + manifestPath, }: { commits: ConventionalCommit[]; latestRelease?: Release; draft?: boolean; labels?: string[]; existingPullRequest?: PullRequest; + manifestPath?: string; }): Promise { if (await this.needsSnapshot(commits, latestRelease)) { this.logger.info('Repository needs a snapshot bump.'); @@ -100,6 +102,7 @@ export class Java extends BaseStrategy { latestRelease, draft, labels, + manifestPath, }); } diff --git a/test/github.ts b/test/github.ts index b810be6ae..06968107e 100644 --- a/test/github.ts +++ b/test/github.ts @@ -35,7 +35,6 @@ import { import {fail} from 'assert'; import {PullRequestBody} from '../src/util/pull-request-body'; import {PullRequestTitle} from '../src/util/pull-request-title'; -import * as codeSuggesterCommitAndPush from 'code-suggester/build/src/github/commit-and-push'; import {HttpsProxyAgent} from 'https-proxy-agent'; import {HttpProxyAgent} from 'http-proxy-agent'; import {Commit} from '../src/commit'; @@ -1112,26 +1111,10 @@ describe('GitHub', () => { describe('updatePullRequest', () => { it('handles a ref branch different from the base branch', async () => { - const forkBranchStub = sandbox - .stub(github, 'forkBranch') // eslint-disable-line @typescript-eslint/no-explicit-any - .withArgs('release-please--branches--main--changes--next', 'next') + const upsertReleaseBranch = sandbox + .stub(github, 'upsertReleaseBranch') // eslint-disable-line @typescript-eslint/no-explicit-any .resolves('the-pull-request-branch-sha'); - const commitAndPushStub = sandbox - .stub(codeSuggesterCommitAndPush, 'commitAndPush') - .withArgs( - sinon.match.any, - 'the-pull-request-branch-sha', - sinon.match.any, - sinon.match.has( - 'branch', - 'release-please--branches--main--changes--next' - ), - sinon.match.string, - true - ) - .resolves(); - const getPullRequestStub = sandbox .stub(github, 'getPullRequest') .withArgs(123) @@ -1171,19 +1154,16 @@ describe('GitHub', () => { }; await github.updatePullRequest(123, pullRequest, 'main', 'next'); - sinon.assert.calledOnce(forkBranchStub); - sinon.assert.calledOnce(commitAndPushStub); + sinon.assert.calledOnce(upsertReleaseBranch); sinon.assert.calledOnce(getPullRequestStub); req.done(); }); it('handles a PR body that is too big', async () => { - const commitAndPushStub = sandbox - .stub(codeSuggesterCommitAndPush, 'commitAndPush') - .resolves(); - const forkBranchStub = sandbox - .stub(github, 'forkBranch') // eslint-disable-line @typescript-eslint/no-explicit-any + const upsertReleaseBranch = sandbox + .stub(github, 'upsertReleaseBranch') // eslint-disable-line @typescript-eslint/no-explicit-any .resolves('the-pull-request-branch-sha'); + req = req.patch('/repos/fake/fake/pulls/123').reply(200, { number: 123, title: 'updated-title', @@ -1225,8 +1205,7 @@ describe('GitHub', () => { pullRequestOverflowHandler, }); sinon.assert.calledOnce(handleOverflowStub); - sinon.assert.calledOnce(commitAndPushStub); - sinon.assert.calledOnce(forkBranchStub); + sinon.assert.calledOnce(upsertReleaseBranch); sinon.assert.calledOnce(getPullRequestStub); req.done(); }); diff --git a/test/manifest.ts b/test/manifest.ts index 69dd05c18..59ac2d176 100644 --- a/test/manifest.ts +++ b/test/manifest.ts @@ -2494,6 +2494,28 @@ describe('Manifest', () => { }) ) ) + .withArgs( + '.release-please-manifest.json', + 'release-please--branches--main--changes--next--components--pkg1' + ) + .resolves( + buildGitHubFileRaw( + JSON.stringify({ + 'path/a': '1.0.1', + }) + ) + ) + .withArgs( + '.release-please-manifest.json', + 'release-please--branches--main--changes--next--components--pkg2' + ) + .resolves( + buildGitHubFileRaw( + JSON.stringify({ + 'path/b': '2.0.1', + }) + ) + ) .withArgs('path/b/package.json', 'next') .resolves( buildGitHubFileRaw( @@ -2501,6 +2523,38 @@ describe('Manifest', () => { name: 'pkg2', }) ) + ) + .withArgs( + '.release-please-manifest.json', + 'release-please--branches--main--changes--next--components--pkg3' + ) + .resolves( + buildGitHubFileRaw( + JSON.stringify({ + 'path/c': '3.0.1', + }) + ) + ) + .withArgs('path/c/setup.py', 'next') + .resolves( + buildGitHubFileRaw( + ` +name = "pkg3" +description = "Something" +version = "3.0.0" +` + ) + ) + .withArgs( + '.release-please-manifest.json', + 'release-please--branches--main--changes--next--components--pkg4' + ) + .resolves( + buildGitHubFileRaw( + JSON.stringify({ + 'path/d': '4.0.1', + }) + ) ); const findFilesByFilenameAndRefStub = sandbox @@ -2535,7 +2589,7 @@ describe('Manifest', () => { const pullRequests = await manifest.buildPullRequests( [ { - title: 'chore(main): release v6.7.9-alpha.1', // version from title differs from expected 4.0.1 + title: 'chore(main): release v6.7.9-alpha.1', // version from title differs from PR manifest body: 'some content', headBranchName: 'release-please--branches--main--changes--next--components--pkg1', @@ -2545,7 +2599,7 @@ describe('Manifest', () => { files: [], }, { - title: 'chore(main): release v7.8.9', // version from title differs from expected 4.0.1 + title: 'chore(main): release v7.8.9', // version from title differs from PR manifest body: 'some content', headBranchName: 'release-please--branches--main--changes--next--components--pkg2', @@ -2555,7 +2609,7 @@ describe('Manifest', () => { files: [], }, { - title: 'chore(main): release 8.9.0', // version from title differs from expected 4.0.1 + title: 'chore(main): release 8.9.0', // version from title differs from PR manifest body: 'some content', headBranchName: 'release-please--branches--main--changes--next--components--pkg3', @@ -2565,7 +2619,7 @@ describe('Manifest', () => { files: [], }, { - title: 'chore(main): release v9.0.1', // version from title differs from expected 4.0.1 + title: 'chore(main): release v9.0.1', // version from title differs from PR manifest body: 'some content', headBranchName: 'release-please--branches--main--changes--next--components--pkg4', @@ -2582,9 +2636,9 @@ describe('Manifest', () => { expect(pullRequests[1].version?.toString()).to.eql('7.8.9'); expect(pullRequests[2].version?.toString()).to.eql('8.9.0'); expect(pullRequests[3].version?.toString()).to.eql('9.0.1'); + sinon.assert.called(getFileContentsOnBranchStub); sinon.assert.called(addIssueLabelsStub); sinon.assert.called(findFilesByFilenameAndRefStub); - sinon.assert.called(getFileContentsOnBranchStub); expect(commentCount).to.eql(4); });