Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(gerrit): use commit message footers to store source branch name #29802

Merged
merged 19 commits into from
Jul 2, 2024
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
eeaf981
feat(gerrit): use commit message footers instead of hashtags
felipecrs Jun 21, 2024
8bf41bc
chore(gerrit): avoid an empty line between footers
felipecrs Jun 21, 2024
8465fbd
chore(gerrit): fix tests after lastavoiding empty line between footers
felipecrs Jun 21, 2024
0e4de2f
chore(gerrit): fix lint error after last commit
felipecrs Jun 21, 2024
2b7a3da
chore(gerrit): delete spurious orig files
felipecrs Jun 21, 2024
252b288
chore(gerrit): restore backwards compatibility with hashtags
felipecrs Jun 24, 2024
69a01b1
chore(gerrit): rename Renovate-Source-Branch to Renovate-Branch
felipecrs Jun 24, 2024
bf20b2d
chore(gerrit): update documentation
felipecrs Jun 24, 2024
9fa72ce
Merge branch 'main' of https://github.com/renovatebot/renovate into u…
felipecrs Jun 24, 2024
c4b3934
chore(gerrit): improve some conditionals
felipecrs Jun 24, 2024
ce48e4d
chore(gerrit): avoid multiple array push calls
felipecrs Jun 24, 2024
af0284b
Apply suggestions from code review
felipecrs Jun 26, 2024
a1d714e
Merge branch 'main' into use-footers-gerrit
felipecrs Jun 26, 2024
2503509
Apply suggestions from code review
felipecrs Jun 28, 2024
b3725ca
Merge branch 'main' of https://github.com/renovatebot/renovate into u…
felipecrs Jun 28, 2024
9470849
Fix review changes
felipecrs Jun 28, 2024
a514cd7
Try to fix coverage
felipecrs Jun 28, 2024
e1c8215
Fix Gerrit filter missing braces
felipecrs Jul 2, 2024
19bc4d9
Merge branch 'main' of https://github.com/renovatebot/renovate into u…
felipecrs Jul 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
11 changes: 9 additions & 2 deletions lib/modules/platform/gerrit/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,15 @@ 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}`,
felipecrs marked this conversation as resolved.
Show resolved Hide resolved
],
);
}
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