From 72371cb778e8dec37fc7724dda2dba0392d033bb Mon Sep 17 00:00:00 2001
From: RahulGautamSingh <rahultesnik@gmail.com>
Date: Thu, 6 Oct 2022 10:07:47 +0530
Subject: [PATCH] feat: improve branch cache logic (#17848)
Co-authored-by: Rhys Arkins <rhys@arkins.net>
---
lib/util/cache/repository/types.ts | 14 +-
lib/util/git/behind-base-branch-cache.spec.ts | 271 ++++++++++++++++--
lib/util/git/behind-base-branch-cache.ts | 39 ++-
lib/util/git/index.spec.ts | 36 +--
lib/util/git/index.ts | 63 ++--
lib/util/git/modified-cache.spec.ts | 93 +++---
lib/util/git/modified-cache.ts | 15 +-
lib/util/git/set-branch-commit.spec.ts | 68 +++++
lib/util/git/set-branch-commit.ts | 36 +++
lib/workers/repository/cache.ts | 30 +-
.../repository/onboarding/branch/rebase.ts | 4 +-
lib/workers/repository/process/write.spec.ts | 150 ++++++++--
lib/workers/repository/process/write.ts | 92 ++++--
lib/workers/repository/update/branch/reuse.ts | 3 +-
14 files changed, 709 insertions(+), 205 deletions(-)
create mode 100644 lib/util/git/set-branch-commit.spec.ts
create mode 100644 lib/util/git/set-branch-commit.ts
diff --git a/lib/util/cache/repository/types.ts b/lib/util/cache/repository/types.ts
index 64d8a1d63f..c08081a3e3 100644
--- a/lib/util/cache/repository/types.ts
+++ b/lib/util/cache/repository/types.ts
@@ -31,6 +31,14 @@ export interface BranchCache {
*Whether this branch has automerge enabled
*/
automerge: boolean;
+ /**
+ * Name of base branch
+ */
+ baseBranch: string;
+ /**
+ * The base branch's most recent commit SHA
+ */
+ baseBranchSha: string | null;
/**
* Hash of the manager fingerprints and the update branch config
*/
@@ -39,10 +47,14 @@ export interface BranchCache {
* Branch name
*/
branchName: string;
+ /**
+ * Whether the update branch is behind base branh
+ */
+ isBehindBase?: boolean;
/**
* Whether a person not listed in gitIgnoredAuthors updated the branch.
*/
- isModified: boolean;
+ isModified?: boolean;
/**
* Parent commit of branch sha
*/
diff --git a/lib/util/git/behind-base-branch-cache.spec.ts b/lib/util/git/behind-base-branch-cache.spec.ts
index 30d7fa9a6b..4aaabc6aaa 100644
--- a/lib/util/git/behind-base-branch-cache.spec.ts
+++ b/lib/util/git/behind-base-branch-cache.spec.ts
@@ -1,7 +1,10 @@
-import { mocked, partial } from '../../../test/util';
+import { logger, mocked, partial } from '../../../test/util';
import * as _repositoryCache from '../cache/repository';
import type { BranchCache, RepoCacheData } from '../cache/repository/types';
-import { getCachedBehindBaseResult } from './behind-base-branch-cache';
+import {
+ getCachedBehindBaseResult,
+ setCachedBehindBaseResult,
+} from './behind-base-branch-cache';
jest.mock('../cache/repository');
const repositoryCache = mocked(_repositoryCache);
@@ -16,36 +19,260 @@ describe('util/git/behind-base-branch-cache', () => {
describe('getCachedBehindBaseResult', () => {
it('returns null if cache is not populated', () => {
- expect(getCachedBehindBaseResult('foo', '111')).toBeNull();
+ expect(
+ getCachedBehindBaseResult(
+ 'branch',
+ 'branch_sha',
+ 'base_branch',
+ 'base_branch_sha'
+ )
+ ).toBeNull();
});
it('returns null if branch not found', () => {
- expect(getCachedBehindBaseResult('foo', '111')).toBeNull();
+ repoCache = {
+ branches: [
+ partial<BranchCache>({
+ branchName: 'not_branch',
+ baseBranchSha: 'base_branch_sha',
+ baseBranch: 'base_branch',
+ sha: 'branch_sha',
+ isBehindBase: true,
+ }),
+ ],
+ };
+ repositoryCache.getCache.mockReturnValue(repoCache);
+ expect(
+ getCachedBehindBaseResult(
+ 'branch',
+ 'branch_sha',
+ 'base_branch',
+ 'base_branch_sha'
+ )
+ ).toBeNull();
});
- it('returns null if cache is partially defined', () => {
- const branchName = 'branchName';
- const branchCache = partial<BranchCache>({
- branchName,
- isModified: false,
- });
- const repoCache: RepoCacheData = { branches: [branchCache] };
+ it('returns null if base branch SHA is different', () => {
+ repoCache = {
+ branches: [
+ partial<BranchCache>({
+ branchName: 'branch',
+ baseBranchSha: 'not_base_branch_sha',
+ baseBranch: 'base_branch',
+ sha: 'branch_sha',
+ isBehindBase: true,
+ }),
+ ],
+ };
+ repositoryCache.getCache.mockReturnValue(repoCache);
+ expect(
+ getCachedBehindBaseResult(
+ 'branch',
+ 'branch_sha',
+ 'base_branch',
+ 'base_branch_sha'
+ )
+ ).toBeNull();
+ });
+
+ it('returns null if branch sha is different', () => {
+ repoCache = {
+ branches: [
+ partial<BranchCache>({
+ branchName: 'branch',
+ baseBranchSha: 'base_branch_sha',
+ baseBranch: 'base_branch',
+ sha: 'not_branch_sha',
+ isBehindBase: true,
+ }),
+ ],
+ };
+ repositoryCache.getCache.mockReturnValue(repoCache);
+ expect(
+ getCachedBehindBaseResult(
+ 'branch',
+ 'branch_sha',
+ 'base_branch',
+ 'base_branch_sha'
+ )
+ ).toBeNull();
+ });
+
+ it('returns null if cached value is undefined', () => {
+ repoCache = {
+ branches: [
+ partial<BranchCache>({
+ branchName: 'branch',
+ baseBranchSha: 'base_branch_sha',
+ baseBranch: 'base_branch',
+ sha: 'not_branch_sha',
+ }),
+ ],
+ };
+ repositoryCache.getCache.mockReturnValue(repoCache);
+ expect(
+ getCachedBehindBaseResult(
+ 'branch',
+ 'branch_sha',
+ 'base_branch',
+ 'base_branch_sha'
+ )
+ ).toBeNull();
+ });
+
+ it('returns null if base branch SHA is null', () => {
+ repoCache = {
+ branches: [
+ partial<BranchCache>({
+ branchName: 'branch',
+ baseBranchSha: null,
+ baseBranch: 'base_branch',
+ sha: 'branch_sha',
+ isBehindBase: true,
+ }),
+ ],
+ };
+ repositoryCache.getCache.mockReturnValue(repoCache);
+ expect(
+ getCachedBehindBaseResult(
+ 'branch',
+ 'branch_sha',
+ 'base_branch',
+ 'base_branch_sha'
+ )
+ ).toBeNull();
+ });
+
+ it('returns null if branch SHA is null', () => {
+ repoCache = {
+ branches: [
+ partial<BranchCache>({
+ branchName: 'branch',
+ baseBranchSha: 'base_branch_sha',
+ baseBranch: 'base_branch',
+ sha: null,
+ isBehindBase: true,
+ }),
+ ],
+ };
+ repositoryCache.getCache.mockReturnValue(repoCache);
+ expect(
+ getCachedBehindBaseResult(
+ 'branch',
+ 'branch_sha',
+ 'base_branch',
+ 'base_branch_sha'
+ )
+ ).toBeNull();
+ });
+
+ it('returns cached value', () => {
+ repoCache = {
+ branches: [
+ partial<BranchCache>({
+ branchName: 'branch',
+ baseBranchSha: 'base_branch_sha',
+ baseBranch: 'base_branch',
+ sha: 'branch_sha',
+ isBehindBase: true,
+ }),
+ ],
+ };
repositoryCache.getCache.mockReturnValue(repoCache);
- expect(getCachedBehindBaseResult(branchName, '111')).toBeNull();
+ expect(
+ getCachedBehindBaseResult(
+ 'branch',
+ 'branch_sha',
+ 'base_branch',
+ 'base_branch_sha'
+ )
+ ).toBeTrue();
+ });
+ });
+
+ describe('setCachedBehindBasedResult', () => {
+ it('returns without updating when cache not populated', () => {
+ setCachedBehindBaseResult('foo', false);
+ expect(repoCache).toEqual({});
+ expect(logger.logger.debug).toHaveBeenCalledWith(
+ 'setCachedBehindBaseResult(): Branch cache not present'
+ );
});
- it('returns true if target SHA has changed', () => {
- repoCache.branches = [
- { branchName: 'foo', sha: 'aaa', parentSha: '222' } as BranchCache,
- ];
- expect(getCachedBehindBaseResult('foo', '111')).toBeTrue();
+ it('returns without updating when branch not found', () => {
+ setCachedBehindBaseResult('foo', false);
+ expect(repoCache).toEqual({});
+ expect(logger.logger.debug).toHaveBeenCalledWith(
+ 'setCachedBehindBaseResult(): Branch cache not present'
+ );
});
- it('returns false if target SHA has not changed', () => {
- repoCache.branches = [
- { branchName: 'foo', sha: 'aaa', parentSha: '111' } as BranchCache,
- ];
- expect(getCachedBehindBaseResult('foo', '111')).toBeFalse();
+ it('updates cached value', () => {
+ repoCache = {
+ branches: [
+ partial<BranchCache>({
+ branchName: 'foo',
+ sha: '121',
+ isBehindBase: true,
+ }),
+ ],
+ };
+ repositoryCache.getCache.mockReturnValue(repoCache);
+ setCachedBehindBaseResult('foo', false);
+ expect(repoCache).toEqual({
+ branches: [
+ {
+ branchName: 'foo',
+ sha: '121',
+ isBehindBase: false,
+ },
+ ],
+ });
+ });
+
+ it('handles multiple branches', () => {
+ repoCache = {
+ branches: [
+ partial<BranchCache>({
+ branchName: 'foo-1',
+ sha: '111',
+ isBehindBase: true,
+ }),
+ partial<BranchCache>({
+ branchName: 'foo-2',
+ sha: 'aaa',
+ isBehindBase: false,
+ }),
+ partial<BranchCache>({
+ branchName: 'foo-3',
+ sha: '222',
+ isBehindBase: true,
+ }),
+ ],
+ };
+ repositoryCache.getCache.mockReturnValue(repoCache);
+ setCachedBehindBaseResult('foo-1', false);
+ setCachedBehindBaseResult('foo-2', true);
+ setCachedBehindBaseResult('foo-3', false);
+ expect(repoCache).toEqual({
+ branches: [
+ {
+ branchName: 'foo-1',
+ sha: '111',
+ isBehindBase: false,
+ },
+ {
+ branchName: 'foo-2',
+ sha: 'aaa',
+ isBehindBase: true,
+ },
+ {
+ branchName: 'foo-3',
+ sha: '222',
+ isBehindBase: false,
+ },
+ ],
+ });
});
});
});
diff --git a/lib/util/git/behind-base-branch-cache.ts b/lib/util/git/behind-base-branch-cache.ts
index 6283062df7..929457a8a5 100644
--- a/lib/util/git/behind-base-branch-cache.ts
+++ b/lib/util/git/behind-base-branch-cache.ts
@@ -1,20 +1,43 @@
+import { logger } from '../../logger';
import { getCache } from '../cache/repository';
-// Compare cached parent Sha of a branch to the fetched base-branch sha to determine whether the branch is behind the base
-// Since cache is updated after each run, this will be sufficient to determine whether a branch is behind its parent.
export function getCachedBehindBaseResult(
branchName: string,
- currentBaseBranchSha: string
+ branchSha: string | null,
+ baseBranch: string,
+ baseBranchSha: string | null
): boolean | null {
const cache = getCache();
- const { branches = [] } = cache;
- const cachedBranch = branches?.find(
+ const branch = cache.branches?.find(
(branch) => branch.branchName === branchName
);
- if (!cachedBranch?.parentSha) {
- return null;
+ if (
+ branch &&
+ branch.sha === branchSha &&
+ branch.baseBranch === baseBranch &&
+ branch.baseBranchSha === baseBranchSha &&
+ branch.isBehindBase !== undefined
+ ) {
+ return branch.isBehindBase;
}
- return currentBaseBranchSha !== cachedBranch.parentSha;
+ return null;
+}
+
+export function setCachedBehindBaseResult(
+ branchName: string,
+ isBehindBase: boolean
+): void {
+ const cache = getCache();
+ const branch = cache.branches?.find(
+ (branch) => branch.branchName === branchName
+ );
+
+ if (!branch) {
+ logger.debug(`setCachedBehindBaseResult(): Branch cache not present`);
+ return;
+ }
+
+ branch.isBehindBase = isBehindBase;
}
diff --git a/lib/util/git/index.spec.ts b/lib/util/git/index.spec.ts
index 1a260ad7f3..2bb96cad02 100644
--- a/lib/util/git/index.spec.ts
+++ b/lib/util/git/index.spec.ts
@@ -1,15 +1,14 @@
import fs from 'fs-extra';
import Git from 'simple-git';
import tmp from 'tmp-promise';
-import { mocked, partial } from '../../../test/util';
+import { logger, mocked } from '../../../test/util';
import { GlobalConfig } from '../../config/global';
import {
CONFIG_VALIDATION,
INVALID_PATH,
} from '../../constants/error-messages';
-import * as _repoCache from '../cache/repository';
-import type { BranchCache } from '../cache/repository/types';
import { newlineRegex, regEx } from '../regex';
+import * as _behindBaseCache from './behind-base-branch-cache';
import * as _conflictsCache from './conflicts-cache';
import * as _modifiedCache from './modified-cache';
import * as _parentShaCache from './parent-sha-cache';
@@ -18,11 +17,12 @@ import * as git from '.';
import { setNoVerify } from '.';
jest.mock('./conflicts-cache');
+jest.mock('./behind-base-branch-cache');
jest.mock('./modified-cache');
jest.mock('./parent-sha-cache');
jest.mock('delay');
jest.mock('../cache/repository');
-const repoCache = mocked(_repoCache);
+const behindBaseCache = mocked(_behindBaseCache);
const conflictsCache = mocked(_conflictsCache);
const modifiedCache = mocked(_modifiedCache);
const parentShaCache = mocked(_parentShaCache);
@@ -117,6 +117,7 @@ describe('util/git/index', () => {
const local = Git(tmpDir.path);
await local.addConfig('commit.gpgsign', 'false');
parentShaCache.getCachedBranchParentShaResult.mockReturnValue(null);
+ behindBaseCache.getCachedBehindBaseResult.mockReturnValue(null);
});
afterEach(async () => {
@@ -247,28 +248,27 @@ describe('util/git/index', () => {
describe('isBranchBehindBase()', () => {
it('should return false if same SHA as master', async () => {
- repoCache.getCache.mockReturnValueOnce({});
expect(
- await git.isBranchBehindBase('renovate/future_branch')
+ await git.isBranchBehindBase('renovate/future_branch', defaultBranch)
).toBeFalse();
});
it('should return true if SHA different from master', async () => {
- repoCache.getCache.mockReturnValueOnce({});
- expect(await git.isBranchBehindBase('renovate/past_branch')).toBeTrue();
+ expect(
+ await git.isBranchBehindBase('renovate/past_branch', defaultBranch)
+ ).toBeTrue();
});
it('should return result even if non-default and not under branchPrefix', async () => {
- const parentSha = 'SHA';
- const branchCache = partial<BranchCache>({
- branchName: 'develop',
- parentSha,
- });
- repoCache.getCache.mockReturnValueOnce({}).mockReturnValueOnce({
- branches: [branchCache],
- });
- expect(await git.isBranchBehindBase('develop')).toBeTrue();
- expect(await git.isBranchBehindBase('develop')).toBeTrue(); // cache
+ expect(await git.isBranchBehindBase('develop', defaultBranch)).toBeTrue();
+ });
+
+ it('returns cached value', async () => {
+ behindBaseCache.getCachedBehindBaseResult.mockReturnValue(true);
+ expect(await git.isBranchBehindBase('develop', defaultBranch)).toBeTrue();
+ expect(logger.logger.debug).toHaveBeenCalledWith(
+ 'branch.isBehindBase(): using cached result "true"'
+ );
});
});
diff --git a/lib/util/git/index.ts b/lib/util/git/index.ts
index 98544d3855..76699fa9cf 100644
--- a/lib/util/git/index.ts
+++ b/lib/util/git/index.ts
@@ -493,6 +493,9 @@ export async function getBranchParentSha(
const branchSha = getBranchCommit(branchName);
let parentSha = getCachedBranchParentShaResult(branchName, branchSha);
if (parentSha !== null) {
+ logger.debug(
+ `branch.getBranchParentSha(): using cached result "${parentSha}"`
+ );
return parentSha;
}
@@ -576,14 +579,23 @@ export function getBranchList(): string[] {
return Object.keys(config.branchCommits);
}
-export async function isBranchBehindBase(branchName: string): Promise<boolean> {
- const { currentBranchSha } = config;
-
- let isBehind = getCachedBehindBaseResult(branchName, currentBranchSha);
+export async function isBranchBehindBase(
+ branchName: string,
+ baseBranch: string
+): Promise<boolean> {
+ let isBehind = getCachedBehindBaseResult(
+ branchName,
+ getBranchCommit(branchName), // branch sha
+ baseBranch,
+ getBranchCommit(baseBranch) // base branch sha
+ );
if (isBehind !== null) {
+ logger.debug(`branch.isBehindBase(): using cached result "${isBehind}"`);
return isBehind;
}
+ logger.debug('branch.isBehindBase(): using git to calculate');
+
await syncGit();
try {
const { currentBranchSha, currentBranch } = config;
@@ -595,8 +607,8 @@ export async function isBranchBehindBase(branchName: string): Promise<boolean> {
]);
isBehind = !branches.all.map(localName).includes(branchName);
logger.debug(
- { isBehind, currentBranch, currentBranchSha },
- `isBranchBehindBase=${isBehind}`
+ { currentBranch, currentBranchSha },
+ `branch.isBehindBase(): ${isBehind}`
);
return isBehind;
} catch (err) /* istanbul ignore next */ {
@@ -610,26 +622,26 @@ export async function isBranchBehindBase(branchName: string): Promise<boolean> {
export async function isBranchModified(branchName: string): Promise<boolean> {
if (!branchExists(branchName)) {
- logger.debug(
- { branchName },
- 'Branch does not exist - cannot check isModified'
- );
+ logger.debug('branch.isModified(): no cache');
return false;
}
// First check local config
if (config.branchIsModified[branchName] !== undefined) {
return config.branchIsModified[branchName];
}
- // Second check repoCache
+ // Second check repository cache
const isModified = getCachedModifiedResult(
branchName,
- config.branchCommits[branchName]
+ getBranchCommit(branchName) // branch sha
);
if (isModified !== null) {
- logger.debug('Using cached result for isBranchModified');
- return (config.branchIsModified[branchName] = isModified);
+ logger.debug(`branch.isModified(): using cached result "${isModified}"`);
+ config.branchIsModified[branchName] = isModified;
+ return isModified;
}
+ logger.debug('branch.isModified(): using git to calculate');
+
await syncGit();
// Retrieve the author of the most recent commit
let lastAuthor: string | undefined;
@@ -659,21 +671,17 @@ export async function isBranchModified(branchName: string): Promise<boolean> {
config.ignoredAuthors.some((ignoredAuthor) => lastAuthor === ignoredAuthor)
) {
// author matches - branch has not been modified
- logger.debug({ branchName }, 'Branch has not been modified');
+ logger.debug('branch.isModified() = false');
config.branchIsModified[branchName] = false;
- setCachedModifiedResult(
- branchName,
- config.branchCommits[branchName],
- false
- );
+ setCachedModifiedResult(branchName, false);
return false;
}
logger.debug(
{ branchName, lastAuthor, gitAuthorEmail },
- 'Last commit author does not match git author email - branch has been modified'
+ 'branch.isModified() = true'
);
config.branchIsModified[branchName] = true;
- setCachedModifiedResult(branchName, config.branchCommits[branchName], true);
+ setCachedModifiedResult(branchName, true);
return true;
}
@@ -693,19 +701,21 @@ export async function isBranchConflicted(
return true;
}
- const cachedResult = getCachedConflictResult(
+ const isConflicted = getCachedConflictResult(
baseBranch,
baseBranchSha,
branch,
branchSha
);
- if (is.boolean(cachedResult)) {
+ if (is.boolean(isConflicted)) {
logger.debug(
- `Using cached result ${cachedResult} for isBranchConflicted(${baseBranch}, ${branch})`
+ `branch.isConflicted(): using cached result "${isConflicted}"`
);
- return cachedResult;
+ return isConflicted;
}
+ logger.debug('branch.isConflicted(): using git to calculate');
+
let result = false;
await syncGit();
await writeGitAuthor();
@@ -741,6 +751,7 @@ export async function isBranchConflicted(
}
setCachedConflictResult(baseBranch, baseBranchSha, branch, branchSha, result);
+ logger.debug(`branch.isConflicted(): ${result}`);
return result;
}
diff --git a/lib/util/git/modified-cache.spec.ts b/lib/util/git/modified-cache.spec.ts
index 58c3105c11..75ab80d203 100644
--- a/lib/util/git/modified-cache.spec.ts
+++ b/lib/util/git/modified-cache.spec.ts
@@ -19,28 +19,35 @@ describe('util/git/modified-cache', () => {
describe('getCachedModifiedResult', () => {
it('returns null if cache is not populated', () => {
- expect(getCachedModifiedResult('foo', '111')).toBeNull();
+ expect(getCachedModifiedResult('foo', 'aaa')).toBeNull();
});
- it('returns null if target key not found', () => {
- expect(getCachedModifiedResult('foo', '111')).toBeNull();
+ it('returns null if branch not found', () => {
+ repoCache.branches = [
+ partial<BranchCache>({ branchName: 'not_foo', sha: 'aaa' }),
+ ];
+ expect(getCachedModifiedResult('foo', 'aaa')).toBeNull();
});
- it('returns null if target SHA has changed', () => {
+ it('returns null if branch SHA has changed', () => {
repoCache.branches = [
partial<BranchCache>({ branchName: 'foo', sha: 'aaa' }),
];
- expect(getCachedModifiedResult('foo', '111')).toBeNull();
+ expect(getCachedModifiedResult('foo', 'not_aaa')).toBeNull();
});
- it('returns null if cache is partially defined', () => {
+ it('returns null if cached value is undefined', () => {
repoCache.branches = [
- partial<BranchCache>({
- branchName: 'foo',
- sha: '111',
- }),
+ partial<BranchCache>({ branchName: 'foo', sha: 'aaa' }),
+ ];
+ expect(getCachedModifiedResult('foo', 'aaa')).toBeNull();
+ });
+
+ it('returns null if branch sha is null', () => {
+ repoCache.branches = [
+ partial<BranchCache>({ branchName: 'foo', sha: 'aaa' }),
];
- expect(getCachedModifiedResult('foo', '111')).toBeNull();
+ expect(getCachedModifiedResult('foo', null)).toBeNull();
});
it('returns cached value', () => {
@@ -48,64 +55,28 @@ describe('util/git/modified-cache', () => {
partial<BranchCache>({
branchName: 'foo',
sha: '111',
- isModified: false,
+ isModified: true,
}),
];
- expect(getCachedModifiedResult('foo', '111')).toBeFalse();
+ expect(getCachedModifiedResult('foo', '111')).toBeTrue();
});
});
describe('setCachedModifiedResult', () => {
- it('does not create new branch cache for when cache is empty', () => {
- setCachedModifiedResult('foo', '111', false);
+ it('returns without updating when cache not populated', () => {
+ setCachedModifiedResult('foo', false);
expect(repoCache).toEqual({});
expect(logger.logger.debug).toHaveBeenCalledWith(
- 'Branch cache not present for foo'
+ 'setCachedModifiedResult(): Branch cache not present'
);
});
- it('replaces value when SHA has changed', () => {
- repoCache = {
- branches: [
- partial<BranchCache>({
- branchName: 'foo',
- sha: '121',
- isModified: true,
- }),
- ],
- };
- repositoryCache.getCache.mockReturnValue(repoCache);
- setCachedModifiedResult('foo', '131', false);
- expect(repoCache).toEqual({
- branches: [
- {
- branchName: 'foo',
- sha: '131',
- isModified: false,
- },
- ],
- });
- });
-
- it('adds SHA when it is not found', () => {
- repoCache = {
- branches: [
- partial<BranchCache>({
- branchName: 'foo',
- }),
- ],
- };
- repositoryCache.getCache.mockReturnValue(repoCache);
- setCachedModifiedResult('foo', '131', false);
- expect(repoCache).toEqual({
- branches: [
- {
- branchName: 'foo',
- sha: '131',
- isModified: false,
- },
- ],
- });
+ it('returns without updating when branch not found', () => {
+ setCachedModifiedResult('foo', false);
+ expect(repoCache).toEqual({});
+ expect(logger.logger.debug).toHaveBeenCalledWith(
+ 'setCachedModifiedResult(): Branch cache not present'
+ );
});
it('handles multiple branches', () => {
@@ -129,9 +100,9 @@ describe('util/git/modified-cache', () => {
],
};
repositoryCache.getCache.mockReturnValue(repoCache);
- setCachedModifiedResult('foo-1', '111', false);
- setCachedModifiedResult('foo-2', 'aaa', true);
- setCachedModifiedResult('foo-3', '222', false);
+ setCachedModifiedResult('foo-1', false);
+ setCachedModifiedResult('foo-2', true);
+ setCachedModifiedResult('foo-3', false);
expect(repoCache).toEqual({
branches: [
{
diff --git a/lib/util/git/modified-cache.ts b/lib/util/git/modified-cache.ts
index 2b93bb470e..e25998cdd4 100644
--- a/lib/util/git/modified-cache.ts
+++ b/lib/util/git/modified-cache.ts
@@ -3,10 +3,12 @@ import { getCache } from '../cache/repository';
export function getCachedModifiedResult(
branchName: string,
- branchSha: string
+ branchSha: string | null
): boolean | null {
- const { branches } = getCache();
- const branch = branches?.find((branch) => branch.branchName === branchName);
+ const cache = getCache();
+ const branch = cache.branches?.find(
+ (branch) => branch.branchName === branchName
+ );
if (branch?.sha === branchSha && branch.isModified !== undefined) {
return branch.isModified;
@@ -17,7 +19,6 @@ export function getCachedModifiedResult(
export function setCachedModifiedResult(
branchName: string,
- branchSha: string,
isModified: boolean
): void {
const cache = getCache();
@@ -26,13 +27,9 @@ export function setCachedModifiedResult(
);
if (!branch) {
- logger.debug(`Branch cache not present for ${branchName}`);
+ logger.debug(`setCachedModifiedResult(): Branch cache not present`);
return;
}
- if (!branch.sha || branch.sha !== branchSha) {
- branch.sha = branchSha;
- }
-
branch.isModified = isModified;
}
diff --git a/lib/util/git/set-branch-commit.spec.ts b/lib/util/git/set-branch-commit.spec.ts
new file mode 100644
index 0000000000..373fcb6480
--- /dev/null
+++ b/lib/util/git/set-branch-commit.spec.ts
@@ -0,0 +1,68 @@
+import { git, logger, mocked, partial } from '../../../test/util';
+import * as _repositoryCache from '../cache/repository';
+import type { BranchCache, RepoCacheData } from '../cache/repository/types';
+import { setBranchNewCommit } from './set-branch-commit';
+
+jest.mock('../cache/repository');
+jest.mock('.');
+const repositoryCache = mocked(_repositoryCache);
+
+describe('util/git/set-branch-commit', () => {
+ let repoCache: RepoCacheData = {};
+
+ beforeEach(() => {
+ repoCache = {};
+ repositoryCache.getCache.mockReturnValue(repoCache);
+ });
+
+ describe('setBranchCommit', () => {
+ it('sets new branch in cache if it doesn not exist', () => {
+ git.getBranchCommit.mockReturnValueOnce('base_SHA');
+ setBranchNewCommit('branch_name', 'base_branch', 'SHA');
+ expect(logger.logger.debug).toHaveBeenCalledWith(
+ 'setBranchCommit(): Branch cache not present'
+ );
+ expect(repoCache.branches).toEqual([
+ {
+ branchName: 'branch_name',
+ baseBranch: 'base_branch',
+ sha: 'SHA',
+ baseBranchSha: 'base_SHA',
+ isBehindBase: false,
+ isModified: false,
+ parentSha: 'base_SHA',
+ },
+ ]);
+ });
+
+ it('sets new values in branch when old state exists', () => {
+ repoCache = {
+ branches: [
+ partial<BranchCache>({
+ branchName: 'branch_name',
+ baseBranch: 'base_branch',
+ sha: 'SHA',
+ baseBranchSha: 'base_SHA',
+ isBehindBase: false,
+ isModified: false,
+ parentSha: 'base_SHA',
+ }),
+ ],
+ };
+ git.getBranchCommit.mockReturnValueOnce('base_SHA');
+ repositoryCache.getCache.mockReturnValue(repoCache);
+ setBranchNewCommit('branch_name', 'base_branch', 'SHA');
+ expect(repoCache.branches).toEqual([
+ {
+ branchName: 'branch_name',
+ baseBranch: 'base_branch',
+ sha: 'SHA',
+ baseBranchSha: 'base_SHA',
+ isBehindBase: false,
+ isModified: false,
+ parentSha: 'base_SHA',
+ },
+ ]);
+ });
+ });
+});
diff --git a/lib/util/git/set-branch-commit.ts b/lib/util/git/set-branch-commit.ts
new file mode 100644
index 0000000000..8c042ea965
--- /dev/null
+++ b/lib/util/git/set-branch-commit.ts
@@ -0,0 +1,36 @@
+import { logger } from '../../logger';
+import { getCache } from '../cache/repository';
+import type { BranchCache } from '../cache/repository/types';
+import { getBranchCommit } from '.';
+
+/**
+ * Called when a new commit is added to branch
+ *
+ * ie. when branch is created/updated
+ */
+export function setBranchNewCommit(
+ branchName: string,
+ baseBranch: string,
+ commitSha: string
+): void {
+ logger.debug('setBranchCommit()');
+ const cache = getCache();
+ cache.branches ??= [];
+ let branch = cache.branches.find((br) => br.branchName === branchName);
+ if (!branch) {
+ logger.debug(`setBranchCommit(): Branch cache not present`); // should never be called
+ branch = {
+ branchName,
+ baseBranch,
+ } as BranchCache;
+ cache.branches.push(branch);
+ }
+
+ const baseBranchSha = getBranchCommit(baseBranch);
+
+ branch.sha = commitSha;
+ branch.baseBranchSha = baseBranchSha;
+ branch.isBehindBase = false;
+ branch.isModified = false;
+ branch.parentSha = baseBranchSha;
+}
diff --git a/lib/workers/repository/cache.ts b/lib/workers/repository/cache.ts
index 5211d57668..09ec25a136 100644
--- a/lib/workers/repository/cache.ts
+++ b/lib/workers/repository/cache.ts
@@ -10,6 +10,7 @@ import type {
import {
getBranchCommit,
getBranchParentSha,
+ isBranchBehindBase,
isBranchModified,
} from '../../util/git';
import type { BranchConfig, BranchUpgradeConfig } from '../types';
@@ -47,40 +48,43 @@ function generateBranchUpgradeCache(
async function generateBranchCache(
branch: BranchConfig
): Promise<BranchCache | null> {
- const { branchName } = branch;
+ const { baseBranch, branchName } = branch;
try {
const sha = getBranchCommit(branchName) ?? null;
+ // TODO: fix types (#7154)
+ const baseBranchSha = getBranchCommit(baseBranch!)!;
let prNo = null;
let parentSha = null;
+ let isModified = false;
+ let isBehindBase = false;
if (sha) {
parentSha = await getBranchParentSha(branchName);
const branchPr = await platform.getBranchPr(branchName);
if (branchPr) {
prNo = branchPr.number;
}
+ isModified = await isBranchModified(branchName);
+ // TODO: fix types (#7154)
+ isBehindBase = await isBranchBehindBase(branchName, baseBranch!);
}
const automerge = !!branch.automerge;
- let isModified = false;
- if (sha) {
- try {
- isModified = await isBranchModified(branchName);
- } catch (err) /* istanbul ignore next */ {
- // Do nothing
- }
- }
const upgrades: BranchUpgradeCache[] = branch.upgrades
? branch.upgrades.map(generateBranchUpgradeCache)
: [];
const branchFingerprint = branch.branchFingerprint;
return {
+ automerge,
+ baseBranchSha,
+ // TODO: fix types (#7154)
+ baseBranch: baseBranch!,
+ branchFingerprint,
branchName,
- sha,
+ isBehindBase,
+ isModified,
parentSha,
prNo,
- automerge,
- isModified,
+ sha,
upgrades,
- branchFingerprint,
};
} catch (error) {
const err = error.err || error; // external host error nests err
diff --git a/lib/workers/repository/onboarding/branch/rebase.ts b/lib/workers/repository/onboarding/branch/rebase.ts
index c38748e2ab..94ca8f1798 100644
--- a/lib/workers/repository/onboarding/branch/rebase.ts
+++ b/lib/workers/repository/onboarding/branch/rebase.ts
@@ -29,10 +29,10 @@ export async function rebaseOnboardingBranch(
const configFile = defaultConfigFile(config);
const existingContents = await getFile(configFile, config.onboardingBranch);
const contents = await getOnboardingConfigContents(config, configFile);
- // TODO #7154
+ // TODO: fix types (#7154)
if (
contents === existingContents &&
- !(await isBranchBehindBase(config.onboardingBranch!))
+ !(await isBranchBehindBase(config.onboardingBranch!, config.defaultBranch!))
) {
logger.debug('Onboarding branch is up to date');
return null;
diff --git a/lib/workers/repository/process/write.spec.ts b/lib/workers/repository/process/write.spec.ts
index 01a26e1993..baad316580 100644
--- a/lib/workers/repository/process/write.spec.ts
+++ b/lib/workers/repository/process/write.spec.ts
@@ -20,7 +20,11 @@ import { Limit, isLimitReached } from '../../global/limits';
import { BranchConfig, BranchResult, BranchUpgradeConfig } from '../../types';
import * as _branchWorker from '../update/branch';
import * as _limits from './limits';
-import { canSkipBranchUpdateCheck, writeUpdates } from './write';
+import {
+ canSkipBranchUpdateCheck,
+ syncBranchState,
+ writeUpdates,
+} from './write';
jest.mock('../../../util/git');
jest.mock('../../../util/cache/repository');
@@ -39,6 +43,7 @@ let config: RenovateConfig;
beforeEach(() => {
jest.resetAllMocks();
config = getConfig();
+ repoCache.getCache.mockReturnValue({});
});
describe('workers/repository/process/write', () => {
@@ -57,7 +62,6 @@ describe('workers/repository/process/write', () => {
{ branchName: 'test_branch', manager: 'npm', upgrades: [] },
{ branchName: 'test_branch', manager: 'npm', upgrades: [] },
]);
- repoCache.getCache.mockReturnValue({});
git.branchExists.mockReturnValue(true);
branchWorker.processBranch.mockResolvedValueOnce({
branchExists: true,
@@ -92,8 +96,7 @@ describe('workers/repository/process/write', () => {
branchExists: true,
result: BranchResult.PrCreated,
});
- git.branchExists.mockReturnValueOnce(false);
- git.branchExists.mockReturnValueOnce(true);
+ git.branchExists.mockReturnValueOnce(false).mockReturnValueOnce(true);
limits.getBranchesRemaining.mockResolvedValueOnce(1);
expect(isLimitReached(Limit.Branches)).toBeFalse();
GlobalConfig.set({ dryRun: 'full' });
@@ -135,8 +138,6 @@ describe('workers/repository/process/write', () => {
branchExists: true,
result: BranchResult.NoWork,
});
- git.branchExists.mockReturnValue(true);
- config.repositoryCache = 'enabled';
expect(await writeUpdates(config, branches)).toBe('done');
});
@@ -165,7 +166,6 @@ describe('workers/repository/process/write', () => {
result: BranchResult.Done,
commitSha: 'some-value',
});
- git.branchExists.mockReturnValue(true);
const branchManagersFingerprint = hasha(
[
...new Set(
@@ -179,40 +179,58 @@ describe('workers/repository/process/write', () => {
JSON.stringify(branches[0]),
branchManagersFingerprint,
]);
- config.repositoryCache = 'enabled';
expect(await writeUpdates(config, branches)).toBe('done');
expect(branches[0].branchFingerprint).toBe(fingerprint);
});
- it('shows debug log when the cache is enabled, but branch cache not found', async () => {
+ it('caches same fingerprint when no commit is made', async () => {
const branches = partial<BranchConfig[]>([
{
branchName: 'new/some-branch',
+ baseBranch: 'base_branch',
manager: 'npm',
upgrades: [
{
- manager: 'npm',
+ manager: 'unknown-manager',
} as BranchUpgradeConfig,
],
},
]);
- repoCache.getCache.mockReturnValueOnce({});
+ const branchManagersFingerprint = hasha(
+ [
+ ...new Set(
+ branches[0].upgrades
+ .map((upgrade) => hashMap.get(upgrade.manager) ?? upgrade.manager)
+ .filter(is.string)
+ ),
+ ].sort()
+ );
+ const fingerprint = hasha([
+ JSON.stringify(branches[0]),
+ branchManagersFingerprint,
+ ]);
+ repoCache.getCache.mockReturnValueOnce({
+ branches: [
+ {
+ branchName: 'new/some-branch',
+ baseBranch: 'base_branch',
+ branchFingerprint: fingerprint,
+ } as BranchCache,
+ ],
+ });
branchWorker.processBranch.mockResolvedValueOnce({
branchExists: true,
- result: BranchResult.NoWork,
+ result: BranchResult.Done,
});
- git.branchExists.mockReturnValueOnce(true);
- config.repositoryCache = 'enabled';
- await writeUpdates(config, branches);
- expect(logger.logger.debug).toHaveBeenCalledWith(
- 'No branch cache found for new/some-branch'
- );
+ expect(await writeUpdates(config, branches)).toBe('done');
+ expect(branches[0].branchFingerprint).toBe(fingerprint);
});
- it('adds branch to cache when cache is not enabled', async () => {
+ it('creates new branchCache when cache is not enabled', async () => {
const branches = partial<BranchConfig[]>([
{
branchName: 'new/some-branch',
+ baseBranch: 'base_branch',
manager: 'npm',
upgrades: [
{
@@ -227,7 +245,9 @@ describe('workers/repository/process/write', () => {
branchExists: true,
result: BranchResult.NoWork,
});
- git.getBranchCommit.mockReturnValue('101');
+ git.getBranchCommit
+ .mockReturnValueOnce('sha')
+ .mockReturnValueOnce('base_sha');
git.branchExists.mockReturnValueOnce(true);
await writeUpdates(config, branches);
expect(logger.logger.debug).not.toHaveBeenCalledWith(
@@ -237,7 +257,9 @@ describe('workers/repository/process/write', () => {
branches: [
{
branchName: 'new/some-branch',
- sha: '101',
+ baseBranch: 'base_branch',
+ baseBranchSha: 'base_sha',
+ sha: 'sha',
},
],
});
@@ -273,4 +295,90 @@ describe('workers/repository/process/write', () => {
expect(canSkipBranchUpdateCheck(branchCache, '222')).toBe(true);
});
});
+
+ describe('syncBranchState()', () => {
+ it('creates minimal branch state when cache is not populated', () => {
+ const repoCacheObj = {} as RepoCacheData;
+ 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',
+ });
+ });
+
+ it('when base branch name is different updates it and invalidates isModified value', () => {
+ const repoCacheObj = {
+ branches: [
+ partial<BranchCache>({
+ branchName: 'branch_name',
+ sha: 'sha',
+ baseBranch: 'base_branch',
+ baseBranchSha: 'base_sha',
+ isModified: true,
+ }),
+ ],
+ };
+ repoCache.getCache.mockReturnValue(repoCacheObj);
+ git.getBranchCommit.mockReturnValueOnce('sha');
+ git.getBranchCommit.mockReturnValueOnce('base_sha');
+ expect(syncBranchState('branch_name', 'new_base_branch')).toEqual({
+ branchName: 'branch_name',
+ sha: 'sha',
+ baseBranch: 'new_base_branch',
+ baseBranchSha: 'base_sha',
+ });
+ });
+
+ it('when base branch sha is different updates it and invalidates related values', () => {
+ const repoCacheObj = {
+ branches: [
+ partial<BranchCache>({
+ branchName: 'branch_name',
+ sha: 'sha',
+ baseBranch: 'base_branch',
+ baseBranchSha: 'base_sha',
+ isBehindBase: true,
+ }),
+ ],
+ };
+ repoCache.getCache.mockReturnValue(repoCacheObj);
+ git.getBranchCommit.mockReturnValueOnce('sha');
+ git.getBranchCommit.mockReturnValueOnce('new_base_sha');
+ expect(syncBranchState('branch_name', 'base_branch')).toEqual({
+ branchName: 'branch_name',
+ sha: 'sha',
+ baseBranch: 'base_branch',
+ baseBranchSha: 'new_base_sha',
+ });
+ });
+
+ it('when branch sha is different updates it and invalidates related values', () => {
+ const repoCacheObj = {
+ branches: [
+ partial<BranchCache>({
+ branchName: 'branch_name',
+ sha: 'sha',
+ baseBranch: 'base_branch',
+ baseBranchSha: 'base_sha',
+ isBehindBase: true,
+ isModified: true,
+ branchFingerprint: '123',
+ }),
+ ],
+ };
+ repoCache.getCache.mockReturnValue(repoCacheObj);
+ git.getBranchCommit.mockReturnValueOnce('new_sha');
+ git.getBranchCommit.mockReturnValueOnce('base_sha');
+ expect(syncBranchState('branch_name', 'base_branch')).toEqual({
+ branchName: 'branch_name',
+ sha: 'new_sha',
+ baseBranch: 'base_branch',
+ baseBranchSha: 'base_sha',
+ });
+ });
+ });
});
diff --git a/lib/workers/repository/process/write.ts b/lib/workers/repository/process/write.ts
index 9b442079da..23eb29f87f 100644
--- a/lib/workers/repository/process/write.ts
+++ b/lib/workers/repository/process/write.ts
@@ -7,6 +7,7 @@ import { hashMap } from '../../../modules/manager';
import { getCache } from '../../../util/cache/repository';
import type { BranchCache } from '../../../util/cache/repository/types';
import { branchExists, getBranchCommit } from '../../../util/git';
+import { setBranchNewCommit } from '../../../util/git/set-branch-commit';
import { Limit, incLimitedValue, setMaxLimit } from '../../global/limits';
import { BranchConfig, BranchResult } from '../../types';
import { processBranch } from '../update/branch';
@@ -15,22 +16,79 @@ import { getBranchesRemaining, getPrsRemaining } from './limits';
export type WriteUpdateResult = 'done' | 'automerged';
export function canSkipBranchUpdateCheck(
- branchCache: BranchCache,
+ branchState: BranchCache,
branchFingerprint: string
): boolean {
- if (!branchCache.branchFingerprint) {
+ if (!branchState.branchFingerprint) {
+ logger.trace('branch.isUpToDate(): no fingerprint');
return false;
}
- if (branchFingerprint !== branchCache.branchFingerprint) {
- logger.debug('Branch fingerprint has changed, full check required');
+ if (branchFingerprint !== branchState.branchFingerprint) {
+ logger.debug('branch.isUpToDate(): needs recalculation');
return false;
}
- logger.debug('Branch fingerprint is unchanged, updates check can be skipped');
+ logger.debug('branch.isUpToDate(): using cached result "true"');
return true;
}
+export function syncBranchState(
+ branchName: string,
+ baseBranch: string
+): BranchCache {
+ logger.debug('syncBranchState()');
+ const branchSha = getBranchCommit(branchName)!;
+ const baseBranchSha = getBranchCommit(baseBranch)!;
+
+ const cache = getCache();
+ cache.branches ??= [];
+ const { branches: cachedBranches } = cache;
+ let branchState = cachedBranches.find((br) => br.branchName === branchName);
+ if (!branchState) {
+ logger.debug(
+ 'syncBranchState(): Branch cache not found, creating minimal branchState'
+ );
+ // create a minimal branch state
+ branchState = {
+ branchName,
+ sha: branchSha,
+ baseBranch,
+ baseBranchSha,
+ } as BranchCache;
+ cachedBranches.push(branchState);
+ }
+
+ // if base branch name has changed invalidate cached isModified state
+ if (baseBranch !== branchState.baseBranch) {
+ logger.debug('syncBranchState(): update baseBranch name');
+ branchState.baseBranch = baseBranch!;
+ delete branchState.isModified;
+ }
+
+ // if base branch sha has changed invalidate cached isBehindBase state
+ if (baseBranchSha !== branchState.baseBranchSha) {
+ logger.debug('syncBranchState(): update baseBranchSha');
+ delete branchState.isBehindBase;
+
+ // update cached branchSha
+ branchState.baseBranchSha = baseBranchSha;
+ }
+
+ // if branch sha has changed invalidate all cached states
+ if (branchSha !== branchState.sha) {
+ logger.debug('syncBranchState(): update branchSha');
+ delete branchState.isBehindBase;
+ delete branchState.isModified;
+ delete branchState.branchFingerprint;
+
+ // update cached branchSha
+ branchState.sha = branchSha;
+ }
+
+ return branchState;
+}
+
export async function writeUpdates(
config: RenovateConfig,
allBranches: BranchConfig[]
@@ -44,9 +102,6 @@ export async function writeUpdates(
.sort()
.join(', ')}`
);
- const cache = getCache();
- cache.branches ??= [];
- const { branches: cachedBranches } = cache;
const prsRemaining = await getPrsRemaining(config, branches);
logger.debug({ prsRemaining }, 'Calculated maximum PRs remaining this run');
setMaxLimit(Limit.PullRequests, prsRemaining);
@@ -60,27 +115,14 @@ export async function writeUpdates(
for (const branch of branches) {
const { baseBranch, branchName } = branch;
- const branchSha = getBranchCommit(branchName)!;
const meta: Record<string, string> = { branch: branchName };
if (config.baseBranches?.length && baseBranch) {
meta['baseBranch'] = baseBranch;
}
addMeta(meta);
const branchExisted = branchExists(branchName);
- let branchState = cachedBranches.find((br) => br.branchName === branchName);
-
- if (!branchState) {
- if (branchExisted && config.repositoryCache === 'enabled') {
- logger.debug(`No branch cache found for ${branchName}`);
- }
-
- // create a minimal branch state
- branchState = {
- branchName,
- sha: branchSha,
- } as BranchCache;
- cachedBranches.push(branchState);
- }
+ // TODO: base branch name cannot be undefined - fix optional types (#7154)
+ const branchState = syncBranchState(branchName, baseBranch!);
const branchManagersFingerprint = hasha(
[
@@ -108,6 +150,10 @@ export async function writeUpdates(
? branchFingerprint
: branchState.branchFingerprint;
+ if (res?.commitSha) {
+ // TODO: base branch name cannot be undefined - fix optional types (#7154)
+ setBranchNewCommit(branchName, baseBranch!, res.commitSha);
+ }
if (
branch.result === BranchResult.Automerged &&
branch.automergeType !== 'pr-comment'
diff --git a/lib/workers/repository/update/branch/reuse.ts b/lib/workers/repository/update/branch/reuse.ts
index 386f7c62a0..39e4e55f11 100644
--- a/lib/workers/repository/update/branch/reuse.ts
+++ b/lib/workers/repository/update/branch/reuse.ts
@@ -62,7 +62,8 @@ export async function shouldReuseExistingBranch(
(config.rebaseWhen === 'auto' &&
(config.automerge || (await platform.getRepoForceRebase())))
) {
- if (await isBranchBehindBase(branchName)) {
+ // TODO: fix types (#7154)
+ if (await isBranchBehindBase(branchName, baseBranch!)) {
logger.debug(`Branch is behind base branch and needs rebasing`);
// We can rebase the branch only if no PR or PR can be rebased
if (await isBranchModified(branchName)) {
--
GitLab