diff --git a/lib/util/cache/repository/types.ts b/lib/util/cache/repository/types.ts index 2512773be5924fef77cd46d9cc43bda0532a29c4..7bd11bda8371e1ad7c671bea5477e9c6428f35ad 100644 --- a/lib/util/cache/repository/types.ts +++ b/lib/util/cache/repository/types.ts @@ -51,6 +51,10 @@ export interface BranchCache { * Whether the update branch is behind base branh */ isBehindBase?: boolean; + /** + * Whether the update branch is in conflict with base branch + */ + isConflicted?: boolean; /** * Whether a person not listed in gitIgnoredAuthors updated the branch. */ diff --git a/lib/util/git/conflicts-cache.spec.ts b/lib/util/git/conflicts-cache.spec.ts index b4eea0d7d1888b723023b76ba4f93b97e4b3cd76..745d49405557e2b075155af3ae7d4d01e46609df 100644 --- a/lib/util/git/conflicts-cache.spec.ts +++ b/lib/util/git/conflicts-cache.spec.ts @@ -1,6 +1,6 @@ -import { mocked } from '../../../test/util'; +import { mocked, partial } from '../../../test/util'; import * as _repositoryCache from '../cache/repository'; -import type { RepoCacheData } from '../cache/repository/types'; +import type { BranchCache, RepoCacheData } from '../cache/repository/types'; import { getCachedConflictResult, setCachedConflictResult, @@ -19,122 +19,165 @@ describe('util/git/conflicts-cache', () => { describe('getCachedConflictResult', () => { it('returns null if cache is not populated', () => { - expect(getCachedConflictResult('foo', '111', 'bar', '222')).toBeNull(); + expect( + getCachedConflictResult('foo', 'sha', 'bar', 'base_sha') + ).toBeNull(); }); - it('returns null if target key not found', () => { - expect(getCachedConflictResult('foo', '111', 'bar', '222')).toBeNull(); + it('returns null if branch cache not found', () => { + repoCache.branches = [ + partial<BranchCache>({ + branchName: 'foo', + sha: 'sha', + baseBranch: 'bar', + baseBranchSha: 'base_sha', + isConflicted: true, + }), + ]; + expect( + getCachedConflictResult('not_foo', 'sha', 'bar', 'base_sha') + ).toBeNull(); }); - it('returns null if target SHA has changed', () => { - repoCache.gitConflicts = { - foo: { targetBranchSha: 'aaa', sourceBranches: {} }, - }; - expect(getCachedConflictResult('foo', '111', 'bar', '222')).toBeNull(); + it('returns null if base branch SHA has changed', () => { + repoCache.branches = [ + partial<BranchCache>({ + branchName: 'foo', + sha: 'sha', + baseBranch: 'bar', + baseBranchSha: 'base_sha', + isConflicted: true, + }), + ]; + expect( + getCachedConflictResult('foo', 'sha', 'bar', 'not_base_sha') + ).toBeNull(); }); - it('returns null if source key not found', () => { - repoCache.gitConflicts = { - foo: { targetBranchSha: '111', sourceBranches: {} }, - }; - expect(getCachedConflictResult('foo', '111', 'bar', '222')).toBeNull(); + it('returns null if branch SHA has changed', () => { + repoCache.branches = [ + partial<BranchCache>({ + branchName: 'foo', + sha: 'sha', + baseBranch: 'bar', + baseBranchSha: 'base_sha', + isConflicted: true, + }), + ]; + expect( + getCachedConflictResult('foo', 'not_sha', 'bar', 'base_sha') + ).toBeNull(); }); - it('returns null if source key has changed', () => { - repoCache.gitConflicts = { - foo: { - targetBranchSha: '111', - sourceBranches: { - bar: { sourceBranchSha: 'bbb', isConflicted: true }, - }, - }, - }; - expect(getCachedConflictResult('foo', '111', 'bar', '222')).toBeNull(); + it('returns null if isConfliced is undefined', () => { + repoCache.branches = [ + partial<BranchCache>({ + branchName: 'foo', + sha: 'sha', + baseBranch: 'bar', + baseBranchSha: 'base_sha', + }), + ]; + expect( + getCachedConflictResult('foo', 'sha', 'bar', 'base_sha') + ).toBeNull(); }); it('returns true', () => { - repoCache.gitConflicts = { - foo: { - targetBranchSha: '111', - sourceBranches: { - bar: { sourceBranchSha: '222', isConflicted: true }, - }, - }, - }; - expect(getCachedConflictResult('foo', '111', 'bar', '222')).toBeTrue(); + repoCache.branches = [ + partial<BranchCache>({ + branchName: 'foo', + sha: 'sha', + baseBranch: 'bar', + baseBranchSha: 'base_sha', + isConflicted: true, + }), + ]; + expect( + getCachedConflictResult('foo', 'sha', 'bar', 'base_sha') + ).toBeTrue(); }); - it('returns false', () => { + it('deletes old cache', () => { repoCache.gitConflicts = { foo: { targetBranchSha: '111', sourceBranches: { - bar: { sourceBranchSha: '222', isConflicted: false }, + bar: { sourceBranchSha: '222', isConflicted: true }, }, }, }; - expect(getCachedConflictResult('foo', '111', 'bar', '222')).toBeFalse(); + getCachedConflictResult('foo', '111', 'bar', '222'); + expect(repoCache.gitConflicts).toBeUndefined(); }); }); describe('setCachedConflictResult', () => { - it('sets value for unpopulated cache', () => { - setCachedConflictResult('foo', '111', 'bar', '222', true); - expect(repoCache).toEqual({ - gitConflicts: { - foo: { - targetBranchSha: '111', - sourceBranches: { - bar: { sourceBranchSha: '222', isConflicted: true }, - }, - }, - }, - }); + it('return without updating value for unpopulated cache', () => { + setCachedConflictResult('foo', false); + expect(repoCache).toEqual({}); }); - it('replaces value when source SHA has changed', () => { - setCachedConflictResult('foo', '111', 'bar', '222', false); - setCachedConflictResult('foo', '111', 'bar', '333', false); - setCachedConflictResult('foo', '111', 'bar', '444', true); + it('updates value', () => { + repoCache.branches = [ + partial<BranchCache>({ + branchName: 'foo', + sha: 'sha', + baseBranch: 'bar', + baseBranchSha: 'base_sha', + isConflicted: true, + }), + ]; + setCachedConflictResult('foo', false); expect(repoCache).toEqual({ - gitConflicts: { - foo: { - targetBranchSha: '111', - sourceBranches: { - bar: { sourceBranchSha: '444', isConflicted: true }, - }, + branches: [ + { + branchName: 'foo', + sha: 'sha', + baseBranch: 'bar', + baseBranchSha: 'base_sha', + isConflicted: false, }, - }, + ], }); }); - it('replaces value when target SHA has changed', () => { - setCachedConflictResult('foo', '111', 'bar', '222', false); - setCachedConflictResult('foo', 'aaa', 'bar', '222', true); + it('handles multiple branches', () => { + repoCache.branches = [ + partial<BranchCache>({ + branchName: 'foo-1', + sha: 'sha', + baseBranch: 'bar', + baseBranchSha: 'base_sha', + isConflicted: true, + }), + partial<BranchCache>({ + branchName: 'foo-2', + sha: 'sha', + baseBranch: 'bar', + baseBranchSha: 'base_sha', + isConflicted: false, + }), + ]; + setCachedConflictResult('foo-1', false); + setCachedConflictResult('foo-2', true); expect(repoCache).toEqual({ - gitConflicts: { - foo: { - targetBranchSha: 'aaa', - sourceBranches: { - bar: { sourceBranchSha: '222', isConflicted: true }, - }, + branches: [ + { + branchName: 'foo-1', + sha: 'sha', + baseBranch: 'bar', + baseBranchSha: 'base_sha', + isConflicted: false, }, - }, - }); - }); - - it('replaces value when both target and source SHA have changed', () => { - setCachedConflictResult('foo', '111', 'bar', '222', true); - setCachedConflictResult('foo', 'aaa', 'bar', 'bbb', false); - expect(repoCache).toEqual({ - gitConflicts: { - foo: { - targetBranchSha: 'aaa', - sourceBranches: { - bar: { sourceBranchSha: 'bbb', isConflicted: false }, - }, + { + branchName: 'foo-2', + sha: 'sha', + baseBranch: 'bar', + baseBranchSha: 'base_sha', + isConflicted: true, }, - }, + ], }); }); }); diff --git a/lib/util/git/conflicts-cache.ts b/lib/util/git/conflicts-cache.ts index 35e56d63255e717a71120e64d5fea8214d8c3655..89644332a30fbdde79031a7bb19fa05f5463deee 100644 --- a/lib/util/git/conflicts-cache.ts +++ b/lib/util/git/conflicts-cache.ts @@ -1,56 +1,42 @@ +import { logger } from '../../logger'; import { getCache } from '../cache/repository'; export function getCachedConflictResult( - targetBranchName: string, - targetBranchSha: string, - sourceBranchName: string, - sourceBranchSha: string + branchName: string, + branchSha: string, + baseBranch: string, + baseBranchSha: string ): boolean | null { - const { gitConflicts } = getCache(); - if (!gitConflicts) { - return null; - } - - const targetBranchConflicts = gitConflicts[targetBranchName]; - if (targetBranchConflicts?.targetBranchSha !== targetBranchSha) { - return null; + const cache = getCache(); + if (cache.gitConflicts) { + delete cache.gitConflicts; } - const sourceBranchConflict = - targetBranchConflicts.sourceBranches[sourceBranchName]; - if (sourceBranchConflict?.sourceBranchSha !== sourceBranchSha) { - return null; + const branch = cache?.branches?.find((br) => br.branchName === branchName); + if ( + branch && + branch.baseBranch === baseBranch && + branch.baseBranchSha === baseBranchSha && + branch.sha === branchSha && + branch.isConflicted !== undefined + ) { + return branch.isConflicted; } - return sourceBranchConflict.isConflicted; + return null; } export function setCachedConflictResult( - targetBranchName: string, - targetBranchSha: string, - sourceBranchName: string, - sourceBranchSha: string, + branchName: string, isConflicted: boolean ): void { const cache = getCache(); - cache.gitConflicts ??= {}; - const { gitConflicts } = cache; + const branch = cache?.branches?.find((br) => br.branchName === branchName); - let targetBranchConflicts = gitConflicts[targetBranchName]; - if (targetBranchConflicts?.targetBranchSha !== targetBranchSha) { - gitConflicts[targetBranchName] = { - targetBranchSha, - sourceBranches: {}, - }; - targetBranchConflicts = gitConflicts[targetBranchName]; + if (!branch) { + logger.debug(`setCachedConflictResult(): Branch cache not present`); + return; } - const sourceBranchConflict = - targetBranchConflicts.sourceBranches[sourceBranchName]; - if (sourceBranchConflict?.sourceBranchSha !== sourceBranchSha) { - targetBranchConflicts.sourceBranches[sourceBranchName] = { - sourceBranchSha, - isConflicted, - }; - } + branch.isConflicted = isConflicted; } diff --git a/lib/util/git/index.spec.ts b/lib/util/git/index.spec.ts index 46bf23e45190898dd355fc7c26ce51f104731e98..d5548cbd4a566bbbb418bca393d0fb2f9c6f5e23 100644 --- a/lib/util/git/index.spec.ts +++ b/lib/util/git/index.spec.ts @@ -856,7 +856,7 @@ describe('util/git/index', () => { expect(status.isClean()).toBeTrue(); }); - describe('cache', () => { + describe('cachedConflictResult', () => { beforeEach(() => { jest.resetAllMocks(); }); @@ -872,10 +872,10 @@ describe('util/git/index', () => { expect(res).toBeTrue(); expect(conflictsCache.getCachedConflictResult.mock.calls).toEqual([ [ - defaultBranch, - expect.any(String), 'renovate/conflicted_branch', - expect.any(String), + git.getBranchCommit('renovate/conflicted_branch'), + defaultBranch, + git.getBranchCommit(defaultBranch), ], ]); expect(conflictsCache.setCachedConflictResult).not.toHaveBeenCalled(); @@ -891,13 +891,7 @@ describe('util/git/index', () => { expect(res).toBeTrue(); expect(conflictsCache.setCachedConflictResult.mock.calls).toEqual([ - [ - defaultBranch, - expect.any(String), - 'renovate/conflicted_branch', - expect.any(String), - true, - ], + ['renovate/conflicted_branch', true], ]); }); @@ -911,13 +905,7 @@ describe('util/git/index', () => { expect(res).toBeFalse(); expect(conflictsCache.setCachedConflictResult.mock.calls).toEqual([ - [ - defaultBranch, - expect.any(String), - 'renovate/non_conflicted_branch', - expect.any(String), - false, - ], + ['renovate/non_conflicted_branch', false], ]); }); }); diff --git a/lib/util/git/index.ts b/lib/util/git/index.ts index 860e1be2fc7d0415d212babd48a03c6f2f2a25f1..2a6b95d9b31f5f3e84ed184732615f2c30b066ab 100644 --- a/lib/util/git/index.ts +++ b/lib/util/git/index.ts @@ -684,10 +684,10 @@ export async function isBranchConflicted( } const isConflicted = getCachedConflictResult( - baseBranch, - baseBranchSha, branch, - branchSha + branchSha, + baseBranch, + baseBranchSha ); if (is.boolean(isConflicted)) { logger.debug( @@ -732,7 +732,7 @@ export async function isBranchConflicted( } } - setCachedConflictResult(baseBranch, baseBranchSha, branch, branchSha, result); + setCachedConflictResult(branch, result); logger.debug(`branch.isConflicted(): ${result}`); return result; } diff --git a/lib/util/git/set-branch-commit.spec.ts b/lib/util/git/set-branch-commit.spec.ts index 373fcb6480d50b74100de5e11180f7a40da8979c..9518163e21cf57f38ddfd0c89aebbd2a2761089f 100644 --- a/lib/util/git/set-branch-commit.spec.ts +++ b/lib/util/git/set-branch-commit.spec.ts @@ -16,7 +16,7 @@ describe('util/git/set-branch-commit', () => { }); describe('setBranchCommit', () => { - it('sets new branch in cache if it doesn not exist', () => { + it('sets new branch in cache', () => { git.getBranchCommit.mockReturnValueOnce('base_SHA'); setBranchNewCommit('branch_name', 'base_branch', 'SHA'); expect(logger.logger.debug).toHaveBeenCalledWith( @@ -29,6 +29,7 @@ describe('util/git/set-branch-commit', () => { sha: 'SHA', baseBranchSha: 'base_SHA', isBehindBase: false, + isConflicted: false, isModified: false, parentSha: 'base_SHA', }, @@ -43,8 +44,9 @@ describe('util/git/set-branch-commit', () => { baseBranch: 'base_branch', sha: 'SHA', baseBranchSha: 'base_SHA', - isBehindBase: false, - isModified: false, + isBehindBase: true, + isModified: true, + isConflicted: true, parentSha: 'base_SHA', }), ], @@ -60,6 +62,7 @@ describe('util/git/set-branch-commit', () => { baseBranchSha: 'base_SHA', isBehindBase: false, isModified: false, + isConflicted: false, parentSha: 'base_SHA', }, ]); diff --git a/lib/util/git/set-branch-commit.ts b/lib/util/git/set-branch-commit.ts index 8c042ea965b979bfc3abe0f5d2949036d94e43b5..c891469366667f0415e474fa9c812b72963d9b1e 100644 --- a/lib/util/git/set-branch-commit.ts +++ b/lib/util/git/set-branch-commit.ts @@ -28,9 +28,10 @@ export function setBranchNewCommit( const baseBranchSha = getBranchCommit(baseBranch); - branch.sha = commitSha; branch.baseBranchSha = baseBranchSha; branch.isBehindBase = false; + branch.isConflicted = false; branch.isModified = false; branch.parentSha = baseBranchSha; + branch.sha = commitSha; } diff --git a/lib/workers/repository/cache.ts b/lib/workers/repository/cache.ts index 90ee99d101737e0c4e69005d1922ad64612f4961..6de35aa19d4f80d8428349d419705cf11035833a 100644 --- a/lib/workers/repository/cache.ts +++ b/lib/workers/repository/cache.ts @@ -10,6 +10,7 @@ import type { import { getBranchCommit, isBranchBehindBase, + isBranchConflicted, isBranchModified, } from '../../util/git'; import { getCachedBranchParentShaResult } from '../../util/git/parent-sha-cache'; @@ -56,6 +57,7 @@ async function generateBranchCache( let parentSha = null; let isModified = false; let isBehindBase = false; + let isConflicted = false; if (sha) { parentSha = getCachedBranchParentShaResult(branchName, sha); const branchPr = await platform.getBranchPr(branchName); @@ -64,6 +66,7 @@ async function generateBranchCache( } isModified = await isBranchModified(branchName); isBehindBase = await isBranchBehindBase(branchName, baseBranch); + isConflicted = await isBranchConflicted(baseBranch, branchName); } const automerge = !!branch.automerge; const upgrades: BranchUpgradeCache[] = branch.upgrades @@ -77,6 +80,7 @@ async function generateBranchCache( branchFingerprint, branchName, isBehindBase, + isConflicted, isModified, parentSha, prNo, diff --git a/lib/workers/repository/process/write.spec.ts b/lib/workers/repository/process/write.spec.ts index 9eb5a624b1ef904a5ac1012f4e37e69d1f8c15c0..3534fdac11c3491eb294f1f7e92b5c837dda7292 100644 --- a/lib/workers/repository/process/write.spec.ts +++ b/lib/workers/repository/process/write.spec.ts @@ -204,6 +204,51 @@ describe('workers/repository/process/write', () => { expect(branch.branchFingerprint).toBe(branchFingerprint); }); + it('caches same fingerprint when no commit is made and branch cache existed', async () => { + const branches: BranchConfig[] = [ + { + branchName: 'new/some-branch', + baseBranch: 'base_branch', + manager: 'npm', + upgrades: [ + { + manager: 'unknown-manager', + } as BranchUpgradeConfig, + ], + }, + ]; + const branch = branches[0]; + const managers = [ + ...new Set( + branch.upgrades + .map((upgrade) => hashMap.get(upgrade.manager) ?? upgrade.manager) + .filter(is.string) + ), + ].sort(); + + const branchFingerprint = fingerprint({ + branch, + managers, + }); + repoCache.getCache.mockReturnValueOnce({ + branches: [ + { + branchName: 'new/some-branch', + baseBranch: 'base_branch', + branchFingerprint, + } as BranchCache, + ], + }); + branchWorker.processBranch.mockResolvedValueOnce({ + branchExists: true, + result: BranchResult.Done, + }); + git.branchExists.mockReturnValue(true); + config.repositoryCache = 'enabled'; + expect(await writeUpdates(config, branches)).toBe('done'); + expect(branch.branchFingerprint).toBe(branchFingerprint); + }); + it('caches same fingerprint when no commit is made', async () => { const branches: BranchConfig[] = [ { @@ -217,15 +262,16 @@ describe('workers/repository/process/write', () => { ], }, ]; + const branch = branches[0]; const managers = [ ...new Set( - branches[0].upgrades + branch.upgrades .map((upgrade) => hashMap.get(upgrade.manager) ?? upgrade.manager) .filter(is.string) ), ].sort(); const branchFingerprint = fingerprint({ - branches: JSON.stringify(branches[0]), + branch, managers, }); repoCache.getCache.mockReturnValueOnce({ @@ -242,7 +288,7 @@ describe('workers/repository/process/write', () => { result: BranchResult.Done, }); expect(await writeUpdates(config, branches)).toBe('done'); - expect(branches[0].branchFingerprint).toBe(branchFingerprint); + expect(branch.branchFingerprint).toBe(branchFingerprint); }); it('creates new branchCache when cache is not enabled', async () => { @@ -413,6 +459,7 @@ describe('workers/repository/process/write', () => { baseBranchSha: 'base_sha', isBehindBase: true, isModified: true, + isConflicted: true, branchFingerprint: '123', upgrades: [], automerge: false, @@ -435,5 +482,43 @@ describe('workers/repository/process/write', () => { parentSha: null, }); }); + + it('no change if all parameters are same', () => { + const repoCacheObj: RepoCacheData = { + branches: [ + { + branchName: 'branch_name', + sha: 'sha', + baseBranch: 'base_branch', + baseBranchSha: 'base_sha', + isBehindBase: true, + isModified: true, + isConflicted: true, + branchFingerprint: '123', + upgrades: [], + automerge: false, + prNo: null, + parentSha: null, + }, + ], + }; + repoCache.getCache.mockReturnValue(repoCacheObj); + git.getBranchCommit.mockReturnValueOnce('sha'); + git.getBranchCommit.mockReturnValueOnce('base_sha'); + expect(syncBranchState('branch_name', 'base_branch')).toEqual({ + branchName: 'branch_name', + sha: 'sha', + baseBranch: 'base_branch', + baseBranchSha: 'base_sha', + isBehindBase: true, + isModified: true, + isConflicted: true, + branchFingerprint: '123', + upgrades: [], + automerge: false, + prNo: null, + parentSha: null, + }); + }); }); }); diff --git a/lib/workers/repository/process/write.ts b/lib/workers/repository/process/write.ts index 66bca98b1e3e8c1270a2a251126bfbc8d2f8b03c..2cad7d5af46efe51806d4506a90ea9ceef49e9a3 100644 --- a/lib/workers/repository/process/write.ts +++ b/lib/workers/repository/process/write.ts @@ -69,6 +69,7 @@ export function syncBranchState( if (baseBranchSha !== branchState.baseBranchSha) { logger.debug('syncBranchState(): update baseBranchSha'); delete branchState.isBehindBase; + delete branchState.isConflicted; // update cached branchSha branchState.baseBranchSha = baseBranchSha; @@ -78,6 +79,7 @@ export function syncBranchState( if (branchSha !== branchState.sha) { logger.debug('syncBranchState(): update branchSha'); delete branchState.isBehindBase; + delete branchState.isConflicted; delete branchState.isModified; delete branchState.branchFingerprint; diff --git a/lib/workers/repository/update/branch/index.ts b/lib/workers/repository/update/branch/index.ts index ac2e342096a283a02da7db33829dd910b5798ae8..0241a46d4f089d1d16055f83ce9064f46c05977d 100644 --- a/lib/workers/repository/update/branch/index.ts +++ b/lib/workers/repository/update/branch/index.ts @@ -34,7 +34,6 @@ import { isBranchConflicted, isBranchModified, } from '../../../../util/git'; -import { setCachedConflictResult } from '../../../../util/git/conflicts-cache'; import { getMergeConfidenceLevel, isActiveConfidenceLevel, @@ -541,14 +540,6 @@ export async function processBranch( if (commitSha) { const action = branchExists ? 'updated' : 'created'; logger.info({ commitSha }, `Branch ${action}`); - // TODO #7154 - setCachedConflictResult( - config.baseBranch, - getBranchCommit(config.baseBranch)!, - config.branchName, - commitSha, - false - ); } // Set branch statuses await setArtifactErrorStatus(config);