Skip to content

Commit

Permalink
feat(gerrit): use commit message footers to store source branch name (#…
Browse files Browse the repository at this point in the history
…29802)

Co-authored-by: HonkingGoose <[email protected]>
Co-authored-by: Michael Kriese <[email protected]>
  • Loading branch information
3 people committed Jul 2, 2024
1 parent 7b5809e commit 74aa3d7
Show file tree
Hide file tree
Showing 8 changed files with 136 additions and 28 deletions.
6 changes: 5 additions & 1 deletion lib/modules/platform/gerrit/client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,11 @@ describe('modules/platform/gerrit/client', () => {
['owner:self', { branchName: 'dependency-xyz' }],
['project:repo', { branchName: 'dependency-xyz' }],
['-is:wip', { branchName: 'dependency-xyz' }],
['hashtag:sourceBranch-dependency-xyz', { branchName: 'dependency-xyz' }],
[
'footer:Renovate-Branch=dependency-xyz',
{ branchName: 'dependency-xyz' },
],
['hashtag:sourceBranch-dependency-xyz', { branchName: 'dependency-xyz' }], // for backwards compatibility
['label:Code-Review=-2', { branchName: 'dependency-xyz', label: '-2' }],
[
'branch:otherTarget',
Expand Down
13 changes: 11 additions & 2 deletions lib/modules/platform/gerrit/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,17 @@ class GerritClient {
): string[] {
const filterState = mapPrStateToGerritFilter(searchConfig.state);
const filters = ['owner:self', 'project:' + repository, filterState];
if (searchConfig.branchName !== '') {
filters.push(`hashtag:sourceBranch-${searchConfig.branchName}`);
if (searchConfig.branchName) {
filters.push(
...[
'(',
`footer:Renovate-Branch=${searchConfig.branchName}`,
// for backwards compatibility
'OR',
`hashtag:sourceBranch-${searchConfig.branchName}`,
')',
],
);
}
if (searchConfig.targetBranch) {
filters.push(`branch:${searchConfig.targetBranch}`);
Expand Down
11 changes: 8 additions & 3 deletions lib/modules/platform/gerrit/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,16 @@
## Supported Gerrit versions

Renovate supports all Gerrit 3.x versions.

Support for Gerrit is currently _experimental_, meaning that it _might_ still have some undiscovered bugs or design limitations, and that we _might_ need to change functionality in a non-backwards compatible manner in a non-major release.

The current implementation uses Gerrit's "hashtags" feature.
Therefore you must use a Gerrit version that uses the [NoteDB](https://gerrit-review.googlesource.com/Documentation/note-db.html) backend.
We did not test Gerrit `2.x` with NoteDB (only in `2.15` and `2.16`), but could work.
Renovate stores its metadata in the _commit message footer_.

Previously Renovate stored metadata in Gerrit's _hashtags_.
To keep backwards compatibility, Renovate still reads metadata from hashtags.
But Renovate _always_ puts its metadata in the _commit message footer_!
When the Renovate maintainers mark Gerrit support as stable, the maintainers will remove the "read metadata from hashtags" feature.
This means changes without metadata in the commit message footer will be "forgotten" by Renovate.

## Authentication

Expand Down
21 changes: 16 additions & 5 deletions lib/modules/platform/gerrit/scm.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -300,13 +300,18 @@ describe('modules/platform/gerrit/scm', () => {
baseBranch: 'main',
branchName: 'renovate/dependency-1.x',
files: [],
message: ['commit msg', expect.stringMatching(/Change-Id: I.{32}/)],
message: [
'commit msg',
expect.stringMatching(
/^Renovate-Branch: renovate\/dependency-1\.x\nChange-Id: I[a-z0-9]{40}$/,
),
],
force: true,
});
expect(git.pushCommit).toHaveBeenCalledWith({
files: [],
sourceRef: 'renovate/dependency-1.x',
targetRef: 'refs/for/main%t=sourceBranch-renovate/dependency-1.x',
targetRef: 'refs/for/main',
});
});

Expand Down Expand Up @@ -339,7 +344,10 @@ describe('modules/platform/gerrit/scm', () => {
baseBranch: 'main',
branchName: 'renovate/dependency-1.x',
files: [],
message: ['commit msg', 'Change-Id: ...'],
message: [
'commit msg',
'Renovate-Branch: renovate/dependency-1.x\nChange-Id: ...',
],
force: true,
});
expect(git.fetchRevSpec).toHaveBeenCalledWith('refs/changes/1/2');
Expand Down Expand Up @@ -377,14 +385,17 @@ describe('modules/platform/gerrit/scm', () => {
baseBranch: 'main',
branchName: 'renovate/dependency-1.x',
files: [],
message: ['commit msg', 'Change-Id: ...'],
message: [
'commit msg',
'Renovate-Branch: renovate/dependency-1.x\nChange-Id: ...',
],
force: true,
});
expect(git.fetchRevSpec).toHaveBeenCalledWith('refs/changes/1/2');
expect(git.pushCommit).toHaveBeenCalledWith({
files: [],
sourceRef: 'renovate/dependency-1.x',
targetRef: 'refs/for/main%t=sourceBranch-renovate/dependency-1.x',
targetRef: 'refs/for/main',
});
expect(clientMock.wasApprovedBy).toHaveBeenCalledWith(
existingChange,
Expand Down
6 changes: 2 additions & 4 deletions lib/modules/platform/gerrit/scm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export class GerritScm extends DefaultGitScm {
typeof commit.message === 'string' ? [commit.message] : commit.message;
commit.message = [
...origMsg,
`Change-Id: ${existingChange?.change_id ?? generateChangeId()}`,
`Renovate-Branch: ${commit.branchName}\nChange-Id: ${existingChange?.change_id ?? generateChangeId()}`,
];
const commitResult = await git.prepareCommit({ ...commit, force: true });
if (commitResult) {
Expand All @@ -123,9 +123,7 @@ export class GerritScm extends DefaultGitScm {
if (hasChanges || commit.force) {
const pushResult = await git.pushCommit({
sourceRef: commit.branchName,
targetRef: `refs/for/${commit.baseBranch!}%t=sourceBranch-${
commit.branchName
}`,
targetRef: `refs/for/${commit.baseBranch!}`,
files: commit.files,
});
if (pushResult) {
Expand Down
3 changes: 3 additions & 0 deletions lib/modules/platform/gerrit/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ export type GerritReviewersType = 'REVIEWER' | 'CC' | 'REMOVED';

export interface GerritChange {
branch: string;
/**
* for backwards compatibility
*/
hashtags?: string[];
change_id: string;
subject: string;
Expand Down
82 changes: 72 additions & 10 deletions lib/modules/platform/gerrit/utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import type {
GerritChangeMessageInfo,
GerritChangeStatus,
GerritLabelTypeInfo,
GerritRevisionInfo,
} from './types';
import * as utils from './utils';
import { mapBranchStatusToLabel } from './utils';
Expand Down Expand Up @@ -83,14 +84,22 @@ describe('modules/platform/gerrit/utils', () => {
const change = partial<GerritChange>({
_number: 123456,
status: 'NEW',
hashtags: ['other', 'sourceBranch-renovate/dependency-1.x'],
branch: 'main',
subject: 'Fix for',
reviewers: {
REVIEWER: [partial<GerritAccountInfo>({ username: 'username' })],
REMOVED: [],
CC: [],
},
current_revision: 'abc',
revisions: {
abc: partial<GerritRevisionInfo>({
commit: {
message:
'Some change\n\nRenovate-Branch: renovate/dependency-1.x\nChange-Id: ...',
},
}),
},
messages: [
partial<GerritChangeMessageInfo>({
id: '9d78ac236714cee8c2d86e95d638358925cf6853',
Expand Down Expand Up @@ -122,11 +131,10 @@ describe('modules/platform/gerrit/utils', () => {
});
});

it('map a gerrit change without sourceBranch-tag and reviewers to Pr', () => {
it('map a gerrit change without source branch info and reviewers to Pr', () => {
const change = partial<GerritChange>({
_number: 123456,
status: 'NEW',
hashtags: ['other'],
branch: 'main',
subject: 'Fix for',
});
Expand All @@ -145,26 +153,80 @@ describe('modules/platform/gerrit/utils', () => {
});

describe('extractSourceBranch()', () => {
it('without hashtags', () => {
it('no commit message', () => {
const change = partial<GerritChange>();
expect(utils.extractSourceBranch(change)).toBeUndefined();
});

it('commit message with no footer', () => {
const change = partial<GerritChange>({
hashtags: undefined,
current_revision: 'abc',
revisions: {
abc: partial<GerritRevisionInfo>({
commit: {
message: 'some message...',
},
}),
},
});
expect(utils.extractSourceBranch(change)).toBeUndefined();
});

it('no hashtag with "sourceBranch-" prefix', () => {
it('commit message with footer', () => {
const change = partial<GerritChange>({
hashtags: ['other', 'another'],
current_revision: 'abc',
revisions: {
abc: partial<GerritRevisionInfo>({
commit: {
message:
'Some change\n\nRenovate-Branch: renovate/dependency-1.x\nChange-Id: ...',
},
}),
},
});
expect(utils.extractSourceBranch(change)).toBeUndefined();
expect(utils.extractSourceBranch(change)).toBe('renovate/dependency-1.x');
});

// for backwards compatibility
it('no commit message but with hashtags', () => {
const change = partial<GerritChange>({
hashtags: ['sourceBranch-renovate/dependency-1.x'],
});
expect(utils.extractSourceBranch(change)).toBe('renovate/dependency-1.x');
});

it('hashtag with "sourceBranch-" prefix', () => {
// for backwards compatibility
it('commit message with no footer but with hashtags', () => {
const change = partial<GerritChange>({
hashtags: ['other', 'sourceBranch-renovate/dependency-1.x', 'another'],
hashtags: ['sourceBranch-renovate/dependency-1.x'],
current_revision: 'abc',
revisions: {
abc: partial<GerritRevisionInfo>({
commit: {
message: 'some message...',
},
}),
},
});
expect(utils.extractSourceBranch(change)).toBe('renovate/dependency-1.x');
});

// for backwards compatibility
it('prefers the footer over the hashtags', () => {
const change = partial<GerritChange>({
hashtags: ['sourceBranch-renovate/dependency-1.x'],
current_revision: 'abc',
revisions: {
abc: partial<GerritRevisionInfo>({
commit: {
message:
'Some change\n\nRenovate-Branch: renovate/dependency-2.x\nChange-Id: ...',
},
}),
},
});
expect(utils.extractSourceBranch(change)).toBe('renovate/dependency-2.x');
});
});

describe('findPullRequestBody()', () => {
Expand Down
22 changes: 19 additions & 3 deletions lib/modules/platform/gerrit/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { CONFIG_GIT_URL_UNAVAILABLE } from '../../../constants/error-messages';
import { logger } from '../../../logger';
import type { BranchStatus, PrState } from '../../../types';
import * as hostRules from '../../../util/host-rules';
import { regEx } from '../../../util/regex';
import { joinUrlParts, parseUrl } from '../../../util/url';
import { hashBody } from '../pr-body';
import type { Pr } from '../types';
Expand Down Expand Up @@ -90,9 +91,24 @@ export function mapGerritChangeStateToPrState(
return 'all';
}
export function extractSourceBranch(change: GerritChange): string | undefined {
return change.hashtags
?.find((tag) => tag.startsWith('sourceBranch-'))
?.replace('sourceBranch-', '');
let sourceBranch: string | undefined = undefined;

if (change.current_revision) {
const re = regEx(/^Renovate-Branch: (.+)$/m);
const message = change.revisions[change.current_revision]?.commit?.message;
if (message) {
sourceBranch = re.exec(message)?.[1];
}
}

// for backwards compatibility
if (!sourceBranch) {
sourceBranch = change.hashtags
?.find((tag) => tag.startsWith('sourceBranch-'))
?.replace('sourceBranch-', '');
}

return sourceBranch ?? undefined;
}

export function findPullRequestBody(change: GerritChange): string | undefined {
Expand Down

0 comments on commit 74aa3d7

Please sign in to comment.