Skip to content
Snippets Groups Projects
Unverified Commit d3b774e8 authored by Sergei Zharinov's avatar Sergei Zharinov Committed by GitHub
Browse files

feat(git): Cache for local conflict detection (#13764)

parent deae1b07
No related branches found
No related tags found
No related merge requests found
import type { PackageFile } from '../../../manager/types';
import type { RepoInitConfig } from '../../../workers/repository/init/types';
import type { GitConflictsCache } from '../../git/types';
export interface BaseBranchCache {
sha: string; // branch commit sha
......@@ -50,4 +51,5 @@ export interface Cache {
graphqlPageCache?: Record<string, GithubGraphqlPageCache>;
};
};
gitConflicts?: GitConflictsCache;
}
import { mocked } from '../../../test/util';
import * as _repositoryCache from '../../util/cache/repository';
import type { Cache } from '../cache/repository/types';
import {
getCachedConflictResult,
setCachedConflictResult,
} from './conflicts-cache';
jest.mock('../../util/cache/repository');
const repositoryCache = mocked(_repositoryCache);
describe('util/git/conflicts-cache', () => {
let repoCache: Cache = {};
beforeEach(() => {
repoCache = {};
repositoryCache.getCache.mockReturnValue(repoCache);
});
describe('getCachedConflictResult', () => {
it('returns null if cache is not populated', () => {
expect(getCachedConflictResult('foo', '111', 'bar', '222')).toBeNull();
});
it('returns null if target key not found', () => {
expect(getCachedConflictResult('foo', '111', 'bar', '222')).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 source key not found', () => {
repoCache.gitConflicts = {
foo: { targetBranchSha: '111', sourceBranches: {} },
};
expect(getCachedConflictResult('foo', '111', 'bar', '222')).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 true', () => {
repoCache.gitConflicts = {
foo: {
targetBranchSha: '111',
sourceBranches: {
bar: { sourceBranchSha: '222', isConflicted: true },
},
},
};
expect(getCachedConflictResult('foo', '111', 'bar', '222')).toBeTrue();
});
it('returns false', () => {
repoCache.gitConflicts = {
foo: {
targetBranchSha: '111',
sourceBranches: {
bar: { sourceBranchSha: '222', isConflicted: false },
},
},
};
expect(getCachedConflictResult('foo', '111', 'bar', '222')).toBeFalse();
});
});
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('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);
expect(repoCache).toEqual({
gitConflicts: {
foo: {
targetBranchSha: '111',
sourceBranches: {
bar: { sourceBranchSha: '444', isConflicted: true },
},
},
},
});
});
it('replaces value when target SHA has changed', () => {
setCachedConflictResult('foo', '111', 'bar', '222', false);
setCachedConflictResult('foo', 'aaa', 'bar', '222', true);
expect(repoCache).toEqual({
gitConflicts: {
foo: {
targetBranchSha: 'aaa',
sourceBranches: {
bar: { sourceBranchSha: '222', isConflicted: true },
},
},
},
});
});
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 },
},
},
},
});
});
});
});
import { getCache } from '../cache/repository';
export function getCachedConflictResult(
targetBranchName: string,
targetBranchSha: string,
sourceBranchName: string,
sourceBranchSha: string
): boolean | null {
const { gitConflicts } = getCache();
if (!gitConflicts) {
return null;
}
const targetBranchConflicts = gitConflicts[targetBranchName];
if (targetBranchConflicts?.targetBranchSha !== targetBranchSha) {
return null;
}
const sourceBranchConflict =
targetBranchConflicts.sourceBranches[sourceBranchName];
if (sourceBranchConflict?.sourceBranchSha !== sourceBranchSha) {
return null;
}
return sourceBranchConflict.isConflicted;
}
export function setCachedConflictResult(
targetBranchName: string,
targetBranchSha: string,
sourceBranchName: string,
sourceBranchSha: string,
isConflicted: boolean
): void {
const cache = getCache();
cache.gitConflicts ??= {};
const { gitConflicts } = cache;
let targetBranchConflicts = gitConflicts[targetBranchName];
if (targetBranchConflicts?.targetBranchSha !== targetBranchSha) {
gitConflicts[targetBranchName] = {
targetBranchSha,
sourceBranches: {},
};
targetBranchConflicts = gitConflicts[targetBranchName];
}
const sourceBranchConflict =
targetBranchConflicts.sourceBranches[sourceBranchName];
if (sourceBranchConflict?.sourceBranchSha !== sourceBranchSha) {
targetBranchConflicts.sourceBranches[sourceBranchName] = {
sourceBranchSha,
isConflicted,
};
}
}
import fs from 'fs-extra';
import Git from 'simple-git';
import tmp from 'tmp-promise';
import { mocked } from '../../../test/util';
import { GlobalConfig } from '../../config/global';
import { CONFIG_VALIDATION } from '../../constants/error-messages';
import * as _conflictsCache from './conflicts-cache';
import type { FileChange } from './types';
import * as git from '.';
import { setNoVerify } from '.';
jest.mock('./conflicts-cache');
const conflictsCache = mocked(_conflictsCache);
// Class is no longer exported
const SimpleGit = Git().constructor as { prototype: ReturnType<typeof Git> };
......@@ -631,6 +636,8 @@ describe('util/git/index', () => {
await repo.commit('other (updated branch) message');
await repo.checkout(defaultBranch);
conflictsCache.getCachedConflictResult.mockReturnValue(null);
});
it('returns true for non-existing source branch', async () => {
......@@ -680,5 +687,70 @@ describe('util/git/index', () => {
expect(status.current).toEqual(branchBefore);
expect(status.isClean()).toBeTrue();
});
describe('cache', () => {
beforeEach(() => {
jest.resetAllMocks();
});
it('returns cached values', async () => {
conflictsCache.getCachedConflictResult.mockReturnValue(true);
const res = await git.isBranchConflicted(
defaultBranch,
'renovate/conflicted_branch'
);
expect(res).toBeTrue();
expect(conflictsCache.getCachedConflictResult.mock.calls).toEqual([
[
defaultBranch,
expect.any(String),
'renovate/conflicted_branch',
expect.any(String),
],
]);
expect(conflictsCache.setCachedConflictResult).not.toHaveBeenCalled();
});
it('caches truthy return value', async () => {
conflictsCache.getCachedConflictResult.mockReturnValue(null);
const res = await git.isBranchConflicted(
defaultBranch,
'renovate/conflicted_branch'
);
expect(res).toBeTrue();
expect(conflictsCache.setCachedConflictResult.mock.calls).toEqual([
[
defaultBranch,
expect.any(String),
'renovate/conflicted_branch',
expect.any(String),
true,
],
]);
});
it('caches falsy return value', async () => {
conflictsCache.getCachedConflictResult.mockReturnValue(null);
const res = await git.isBranchConflicted(
defaultBranch,
'renovate/non_conflicted_branch'
);
expect(res).toBeFalse();
expect(conflictsCache.setCachedConflictResult.mock.calls).toEqual([
[
defaultBranch,
expect.any(String),
'renovate/non_conflicted_branch',
expect.any(String),
false,
],
]);
});
});
});
});
import URL from 'url';
import is from '@sindresorhus/is';
import fs from 'fs-extra';
import simpleGit, {
Options,
......@@ -26,6 +27,10 @@ import { Limit, incLimitedValue } from '../../workers/global/limits';
import { regEx } from '../regex';
import { parseGitAuthor } from './author';
import { getNoVerify, simpleGitConfig } from './config';
import {
getCachedConflictResult,
setCachedConflictResult,
} from './conflicts-cache';
import { checkForPlatformFailure, handleCommitError } from './error';
import { configSigningKey, writePrivateKey } from './private-key';
import type {
......@@ -518,7 +523,10 @@ export async function isBranchConflicted(
logger.debug(`isBranchConflicted(${baseBranch}, ${branch})`);
await syncGit();
await writeGitAuthor();
if (!branchExists(baseBranch) || !branchExists(branch)) {
const baseBranchSha = getBranchCommit(baseBranch);
const branchSha = getBranchCommit(branch);
if (!baseBranchSha || !branchSha) {
logger.warn(
{ baseBranch, branch },
'isBranchConflicted: branch does not exist'
......@@ -526,6 +534,19 @@ export async function isBranchConflicted(
return true;
}
const cachedResult = getCachedConflictResult(
baseBranch,
baseBranchSha,
branch,
branchSha
);
if (is.boolean(cachedResult)) {
logger.debug(
`Using cached result ${cachedResult} for isBranchConflicted(${baseBranch}, ${branch})`
);
return cachedResult;
}
let result = false;
const origBranch = config.currentBranch;
......@@ -558,6 +579,7 @@ export async function isBranchConflicted(
}
}
setCachedConflictResult(baseBranch, baseBranchSha, branch, branchSha, result);
return result;
}
......
......@@ -75,6 +75,22 @@ export interface CommitFilesConfig {
force?: boolean;
}
export type BranchName = string;
export type TargetBranchName = BranchName;
export type SourceBranchName = BranchName;
export type GitConflictsCache = Record<TargetBranchName, TargetBranchConflicts>;
export interface TargetBranchConflicts {
targetBranchSha: CommitSha;
sourceBranches: Record<SourceBranchName, SourceBranchConflict>;
}
export interface SourceBranchConflict {
sourceBranchSha: CommitSha;
isConflicted: boolean;
}
export interface CommitResult {
sha: string;
files: FileChange[];
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please to comment