Skip to content

Commit 6528275

Browse files
authored
fix: do not reuse branch if cache fingerprint doesn't match (renovatebot#36616)
1 parent 6b014dc commit 6528275

File tree

5 files changed

+73
-17
lines changed

5 files changed

+73
-17
lines changed

lib/workers/repository/process/write.spec.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import type { BranchConfig, BranchUpgradeConfig } from '../../types';
1515
import * as _branchWorker from '../update/branch';
1616
import * as _limits from './limits';
1717
import {
18-
canSkipBranchUpdateCheck,
18+
compareCacheFingerprint,
1919
generateCommitFingerprintConfig,
2020
syncBranchState,
2121
writeUpdates,
@@ -358,7 +358,9 @@ describe('workers/repository/process/write', () => {
358358
branchName: 'new/some-branch',
359359
sha: '111',
360360
};
361-
expect(canSkipBranchUpdateCheck(branchCache, '222')).toBe(false);
361+
expect(compareCacheFingerprint(branchCache, '222')).toBe(
362+
'no-fingerprint',
363+
);
362364
});
363365

364366
it('returns false when fingerprints are not same', () => {
@@ -368,7 +370,7 @@ describe('workers/repository/process/write', () => {
368370
sha: '111',
369371
commitFingerprint: '211',
370372
};
371-
expect(canSkipBranchUpdateCheck(branchCache, '222')).toBe(false);
373+
expect(compareCacheFingerprint(branchCache, '222')).toBe('no-match');
372374
});
373375

374376
it('returns true', () => {
@@ -378,7 +380,7 @@ describe('workers/repository/process/write', () => {
378380
sha: '111',
379381
commitFingerprint: '222',
380382
};
381-
expect(canSkipBranchUpdateCheck(branchCache, '222')).toBe(true);
383+
expect(compareCacheFingerprint(branchCache, '222')).toBe('matched');
382384
});
383385
});
384386

lib/workers/repository/process/write.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,11 @@ import type { BranchCache } from '../../../util/cache/repository/types';
88
import { fingerprint } from '../../../util/fingerprint';
99
import { setBranchNewCommit } from '../../../util/git/set-branch-commit';
1010
import { incCountValue, setCount } from '../../global/limits';
11-
import type { BranchConfig, UpgradeFingerprintConfig } from '../../types';
11+
import type {
12+
BranchConfig,
13+
CacheFingerprintMatchResult,
14+
UpgradeFingerprintConfig,
15+
} from '../../types';
1216
import { processBranch } from '../update/branch';
1317
import { upgradeFingerprintFields } from './fingerprint-fields';
1418
import {
@@ -33,22 +37,22 @@ export function generateCommitFingerprintConfig(
3337
return res;
3438
}
3539

36-
export function canSkipBranchUpdateCheck(
40+
export function compareCacheFingerprint(
3741
branchState: BranchCache,
3842
commitFingerprint: string,
39-
): boolean {
43+
): CacheFingerprintMatchResult {
4044
if (!branchState.commitFingerprint) {
4145
logger.trace('branch.isUpToDate(): no fingerprint');
42-
return false;
46+
return 'no-fingerprint';
4347
}
4448

4549
if (commitFingerprint !== branchState.commitFingerprint) {
4650
logger.debug('branch.isUpToDate(): needs recalculation');
47-
return false;
51+
return 'no-match';
4852
}
4953

5054
logger.debug('branch.isUpToDate(): using cached result "true"');
51-
return true;
55+
return 'matched';
5256
}
5357

5458
export async function syncBranchState(
@@ -156,7 +160,7 @@ export async function writeUpdates(
156160
commitFingerprintConfig: generateCommitFingerprintConfig(branch),
157161
managers,
158162
});
159-
branch.skipBranchUpdate = canSkipBranchUpdateCheck(
163+
branch.cacheFingerprintMatch = compareCacheFingerprint(
160164
branchState,
161165
commitFingerprint,
162166
);

lib/workers/repository/update/branch/index.spec.ts

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,11 @@ import * as _mergeConfidence from '../../../../util/merge-confidence';
3232
import * as _sanitize from '../../../../util/sanitize';
3333
import type { Timestamp } from '../../../../util/timestamp';
3434
import * as _limits from '../../../global/limits';
35-
import type { BranchConfig, BranchUpgradeConfig } from '../../../types';
35+
import type {
36+
BranchConfig,
37+
BranchUpgradeConfig,
38+
CacheFingerprintMatchResult,
39+
} from '../../../types';
3640
import type { ResultWithPr } from '../pr';
3741
import * as _prWorker from '../pr';
3842
import * as _prAutomerge from '../pr/automerge';
@@ -897,6 +901,38 @@ describe('workers/repository/update/branch/index', () => {
897901
expect(prWorker.ensurePr).toHaveBeenCalledTimes(0);
898902
});
899903

904+
it('updates branch when no fingerprint match', async () => {
905+
expect.assertions(3);
906+
getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce(
907+
partial<PackageFilesResult>({
908+
updatedPackageFiles: [partial<FileChange>()],
909+
}),
910+
);
911+
npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({
912+
artifactErrors: [],
913+
updatedArtifacts: [partial<FileChange>()],
914+
});
915+
const inconfig = {
916+
...config,
917+
ignoreTests: true,
918+
prCreation: 'not-pending',
919+
commitBody: '[skip-ci]',
920+
fetchChangeLogs: 'branch',
921+
cacheFingerprintMatch: 'no-match',
922+
} satisfies BranchConfig;
923+
scm.getBranchCommit.mockResolvedValue('123test' as LongCommitSha); //TODO:not needed?
924+
expect(await branchWorker.processBranch(inconfig)).toEqual({
925+
branchExists: true,
926+
updatesVerified: true,
927+
prNo: undefined,
928+
result: 'pending',
929+
commitSha: '123test',
930+
});
931+
932+
expect(automerge.tryBranchAutomerge).toHaveBeenCalledTimes(0);
933+
expect(prWorker.ensurePr).toHaveBeenCalledTimes(0);
934+
});
935+
900936
it('ensures PR and comments notice', async () => {
901937
getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce(
902938
partial<PackageFilesResult>({
@@ -1226,7 +1262,7 @@ describe('workers/repository/update/branch/index', () => {
12261262
await branchWorker.processBranch({
12271263
...config,
12281264
baseBranch: 'new_base',
1229-
skipBranchUpdate: true,
1265+
cacheFingerprintMatch: 'matched',
12301266
});
12311267
expect(logger.debug).toHaveBeenCalledWith(
12321268
'Base branch changed by user, rebasing the branch onto new base',
@@ -1480,7 +1516,7 @@ describe('workers/repository/update/branch/index', () => {
14801516
branchPrefixOld: 'old/',
14811517
commitFingerprint: '111',
14821518
reuseExistingBranch: true,
1483-
skipBranchUpdate: true,
1519+
cacheFingerprintMatch: 'matched' as CacheFingerprintMatchResult,
14841520
};
14851521
expect(await branchWorker.processBranch(inconfig)).toEqual({
14861522
branchExists: true,
@@ -2500,7 +2536,7 @@ describe('workers/repository/update/branch/index', () => {
25002536
}),
25012537
);
25022538
config.reuseExistingBranch = true;
2503-
config.skipBranchUpdate = true;
2539+
config.cacheFingerprintMatch = 'matched';
25042540
const inconfig = {
25052541
...config,
25062542
branchName: 'new/some-branch',

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -475,12 +475,21 @@ export async function processBranch(
475475
'Base branch changed by user, rebasing the branch onto new base',
476476
);
477477
config.reuseExistingBranch = false;
478+
} else if (config.cacheFingerprintMatch === 'no-match') {
479+
logger.debug(
480+
'Cache fingerprint does not match, cannot reuse existing branch',
481+
);
482+
config.reuseExistingBranch = false;
478483
} else {
479484
config = await shouldReuseExistingBranch(config);
480485
}
481486
// TODO: types (#22198)
482487
logger.debug(`Using reuseExistingBranch: ${config.reuseExistingBranch!}`);
483-
if (!(config.reuseExistingBranch && config.skipBranchUpdate)) {
488+
if (
489+
!(
490+
config.reuseExistingBranch && config.cacheFingerprintMatch === 'matched'
491+
)
492+
) {
484493
await scm.checkoutBranch(config.baseBranch);
485494
const res = await getUpdatedPackageFiles(config);
486495
// istanbul ignore if

lib/workers/types.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,11 @@ export type BranchResult =
113113
| 'rebase'
114114
| 'update-not-scheduled';
115115

116+
export type CacheFingerprintMatchResult =
117+
| 'matched'
118+
| 'no-match'
119+
| 'no-fingerprint';
120+
116121
export interface BranchConfig
117122
extends BranchUpgradeConfig,
118123
LegacyAdminConfig,
@@ -135,7 +140,7 @@ export interface BranchConfig
135140
stopUpdating?: boolean;
136141
isConflicted?: boolean;
137142
commitFingerprint?: string;
138-
skipBranchUpdate?: boolean;
143+
cacheFingerprintMatch?: CacheFingerprintMatchResult;
139144
}
140145

141146
export interface BranchMetadata {

0 commit comments

Comments
 (0)