[APP-2111] fork base and add release commit atomically (#215)

* Revert "do not leave comment if custom version has not been set (#212)"

This reverts commit e163d6f.

* fork base and add release commit atomically

* fix

---------

Co-authored-by: meorphis <[email protected]>

Author: meorphis

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<PullRequest> => {
-      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<void> => {
+      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

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<ReleasePullRequest | undefined> {
     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<Record<string, string>>(
+              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],

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<ReleasePullRequest | undefined> {
     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,
     });
   }
 

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, <any>'forkBranch') // eslint-disable-line @typescript-eslint/no-explicit-any
-        .withArgs('release-please--branches--main--changes--next', 'next')
+      const upsertReleaseBranch = sandbox
+        .stub(github, <any>'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, <any>'forkBranch') // eslint-disable-line @typescript-eslint/no-explicit-any
+      const upsertReleaseBranch = sandbox
+        .stub(github, <any>'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();
     });