fix(gerrit): improve commit message vs pr title workaround (#32115)

lib/modules/platform/gerrit/client.spec.ts

@@ -208,39 +208,6 @@ describe('modules/platform/gerrit/client', () => {
     });
   });
 
-  describe('setCommitMessage()', () => {
-    it('setCommitMessage', async () => {
-      const change = partial<GerritChange>({});
-      httpMock
-        .scope(gerritEndpointUrl)
-        .put('/a/changes/123456/message', { message: 'new message' })
-        .reply(200, gerritRestResponse(change), jsonResultHeader);
-      await expect(client.setCommitMessage(123456, 'new message')).toResolve();
-    });
-  });
-
-  describe('updateChangeSubject', () => {
-    it('updateChangeSubject - success', async () => {
-      const change = partial<GerritChange>({});
-      const newSubject = 'new subject';
-      const previousSubject = 'some subject';
-      const previousBody = `some body\n\nChange-Id: some-change-id\n`;
-      httpMock
-        .scope(gerritEndpointUrl)
-        .put('/a/changes/123456/message', {
-          message: `${newSubject}\n\n${previousBody}`,
-        })
-        .reply(200, gerritRestResponse(change), jsonResultHeader);
-      await expect(
-        client.updateChangeSubject(
-          123456,
-          `${previousSubject}\n\n${previousBody}`,
-          'new subject',
-        ),
-      ).toResolve();
-    });
-  });
-
   describe('getMessages()', () => {
     it('no messages', async () => {
       httpMock

lib/modules/platform/gerrit/client.ts

@@ -98,25 +98,6 @@ class GerritClient {
     return change.body;
   }
 
-  async setCommitMessage(changeNumber: number, message: string): Promise<void> {
-    await this.gerritHttp.putJson(`a/changes/${changeNumber}/message`, {
-      body: { message },
-    });
-  }
-
-  async updateChangeSubject(
-    number: number,
-    currentMessage: string,
-    newSubject: string,
-  ): Promise<void> {
-    // Replace first line of the commit message with the new subject
-    const newMessage = currentMessage.replace(
-      new RegExp(`^.*$`, 'm'),
-      newSubject,
-    );
-    await this.setCommitMessage(number, newMessage);
-  }
-
   async getMessages(changeNumber: number): Promise<GerritChangeMessageInfo[]> {
     const messages = await this.gerritHttp.getJson<GerritChangeMessageInfo[]>(
       `a/changes/${changeNumber}/messages`,

lib/modules/platform/gerrit/index.spec.ts

@@ -187,29 +187,6 @@ describe('modules/platform/gerrit/index', () => {
       gerrit.writeToConfig({ labels: {} });
     });
 
-    it('updatePr() - new prTitle => copy to commit msg', async () => {
-      const oldSubject = 'old title';
-      const oldMessage = `${oldSubject}\n\nsome body\n\nChange-Id: ...`;
-      const change = partial<GerritChange>({
-        subject: oldSubject,
-        current_revision: 'some-revision',
-        revisions: {
-          'some-revision': partial<GerritRevisionInfo>({
-            commit: {
-              message: oldMessage,
-            },
-          }),
-        },
-      });
-      clientMock.getChange.mockResolvedValueOnce(change);
-      await gerrit.updatePr({ number: 123456, prTitle: 'new title' });
-      expect(clientMock.updateChangeSubject).toHaveBeenCalledWith(
-        123456,
-        oldMessage,
-        'new title',
-      );
-    });
-
     it('updatePr() - auto approve enabled', async () => {
       const change = partial<GerritChange>({
         current_revision: 'some-revision',
@@ -332,7 +309,7 @@ describe('modules/platform/gerrit/index', () => {
       ]);
     });
 
-    it('createPr() - update body/title WITHOUT approve', async () => {
+    it('createPr() - update body WITHOUT approve', async () => {
       const pr = await gerrit.createPr({
         sourceBranch: 'source',
         targetBranch: 'target',
@@ -349,11 +326,6 @@ describe('modules/platform/gerrit/index', () => {
         TAG_PULL_REQUEST_BODY,
       );
       expect(clientMock.approveChange).not.toHaveBeenCalled();
-      expect(clientMock.updateChangeSubject).toHaveBeenCalledWith(
-        123456,
-        message,
-        'title',
-      );
     });
 
     it('createPr() - update body and approve', async () => {
@@ -373,7 +345,6 @@ describe('modules/platform/gerrit/index', () => {
         TAG_PULL_REQUEST_BODY,
       );
       expect(clientMock.approveChange).toHaveBeenCalledWith(123456);
-      expect(clientMock.setCommitMessage).not.toHaveBeenCalled();
     });
   });
 

lib/modules/platform/gerrit/index.ts

@@ -153,14 +153,6 @@ export async function getPr(number: number): Promise<Pr | null> {
 
 export async function updatePr(prConfig: UpdatePrConfig): Promise<void> {
   logger.debug(`updatePr(${prConfig.number}, ${prConfig.prTitle})`);
-  const change = await client.getChange(prConfig.number);
-  if (change.subject !== prConfig.prTitle) {
-    await client.updateChangeSubject(
-      prConfig.number,
-      change.revisions[change.current_revision].commit.message,
-      prConfig.prTitle,
-    );
-  }
   if (prConfig.prBody) {
     await client.addMessageIfNotAlreadyExists(
       prConfig.number,
@@ -198,14 +190,6 @@ export async function createPr(prConfig: CreatePRConfig): Promise<Pr | null> {
       `the change should be created automatically from previous push to refs/for/${prConfig.sourceBranch}`,
     );
   }
-  //Workaround for "Known Problems.1"
-  if (pr.subject !== prConfig.prTitle) {
-    await client.updateChangeSubject(
-      pr._number,
-      pr.revisions[pr.current_revision].commit.message,
-      prConfig.prTitle,
-    );
-  }
   await client.addMessageIfNotAlreadyExists(
     pr._number,
     prConfig.prBody,

lib/modules/platform/gerrit/readme.md

@@ -61,15 +61,3 @@ For example, if you want to use the [Merge Confidence](../../../merge-confidence
 
 - Creating issues (not a Gerrit concept)
 - Dependency Dashboard (needs issues first)
-
-## Known problems
-
-### PR title is different from first commit message
-
-Sometimes the PR title passed to the Gerrit platform code is different from the first line of the commit message.
-For example:
-
-- Commit-Message=`Update keycloak.version to v21`
-- Pull-Request-Title=`Update keycloak.version to v21 (major)`
-
-In this case the Gerrit-Platform implementation tries to detect this and change the commit-message in a second patch-set.

lib/modules/platform/gerrit/scm.spec.ts

@@ -266,6 +266,7 @@ describe('modules/platform/gerrit/scm', () => {
           baseBranch: 'main',
           message: 'commit msg',
           files: [],
+          prTitle: 'pr title',
         }),
       ).resolves.toBeNull();
       expect(clientMock.findChanges).toHaveBeenCalledWith(
@@ -294,18 +295,20 @@ describe('modules/platform/gerrit/scm', () => {
           baseBranch: 'main',
           message: 'commit msg',
           files: [],
+          prTitle: 'pr title',
         }),
       ).toBe('commitSha');
       expect(git.prepareCommit).toHaveBeenCalledWith({
         baseBranch: 'main',
         branchName: 'renovate/dependency-1.x',
         files: [],
         message: [
-          'commit msg',
+          'pr title',
           expect.stringMatching(
             /^Renovate-Branch: renovate\/dependency-1\.x\nChange-Id: I[a-z0-9]{40}$/,
           ),
         ],
+        prTitle: 'pr title',
         force: true,
       });
       expect(git.pushCommit).toHaveBeenCalledWith({
@@ -338,16 +341,18 @@ describe('modules/platform/gerrit/scm', () => {
           baseBranch: 'main',
           message: ['commit msg'],
           files: [],
+          prTitle: 'pr title',
         }),
       ).toBeNull();
       expect(git.prepareCommit).toHaveBeenCalledWith({
         baseBranch: 'main',
         branchName: 'renovate/dependency-1.x',
         files: [],
         message: [
-          'commit msg',
+          'pr title',
           'Renovate-Branch: renovate/dependency-1.x\nChange-Id: ...',
         ],
+        prTitle: 'pr title',
         force: true,
       });
       expect(git.fetchRevSpec).toHaveBeenCalledWith('refs/changes/1/2');
@@ -379,16 +384,18 @@ describe('modules/platform/gerrit/scm', () => {
           baseBranch: 'main',
           message: 'commit msg',
           files: [],
+          prTitle: 'pr title',
         }),
       ).toBe('commitSha');
       expect(git.prepareCommit).toHaveBeenCalledWith({
         baseBranch: 'main',
         branchName: 'renovate/dependency-1.x',
         files: [],
         message: [
-          'commit msg',
+          'pr title',
           'Renovate-Branch: renovate/dependency-1.x\nChange-Id: ...',
         ],
+        prTitle: 'pr title',
         force: true,
       });
       expect(git.fetchRevSpec).toHaveBeenCalledWith('refs/changes/1/2');

lib/modules/platform/gerrit/scm.ts

@@ -105,10 +105,18 @@ export class GerritScm extends DefaultGitScm {
       .then((res) => res.pop());
 
     let hasChanges = true;
-    const origMsg =
+    const message =
       typeof commit.message === 'string' ? [commit.message] : commit.message;
+
+    // In Gerrit, the change subject/title is the first line of the commit message
+    if (commit.prTitle) {
+      const firstMessageLines = message[0].split('\n');
+      firstMessageLines[0] = commit.prTitle;
+      message[0] = firstMessageLines.join('\n');
+    }
+
     commit.message = [
-      ...origMsg,
+      ...message,
       `Renovate-Branch: ${commit.branchName}\nChange-Id: ${existingChange?.change_id ?? generateChangeId()}`,
     ];
     const commitResult = await git.prepareCommit({ ...commit, force: true });

lib/util/git/types.ts

@@ -83,6 +83,8 @@ export interface CommitFilesConfig {
   message: string | string[];
   force?: boolean;
   platformCommit?: PlatformCommitOptions;
+  /** Only needed by Gerrit platform */
+  prTitle?: string;
 }
 
 export interface PushFilesConfig {

lib/workers/repository/update/branch/commit.ts

@@ -60,5 +60,7 @@ export function commitFilesToBranch(
     message: config.commitMessage!,
     force: !!config.forceCommit,
     platformCommit: config.platformCommit,
+    // Only needed by Gerrit platform
+    prTitle: config.prTitle,
   });
 }