Skip to content

Commit

Permalink
refactor: Move all Git commands behind a PlatformScm interface renova…
Browse files Browse the repository at this point in the history
…tebot#19065

this is a proof of concept only...
  • Loading branch information
NiasSt90 committed Nov 29, 2022
1 parent fd0def1 commit 516762a
Show file tree
Hide file tree
Showing 15 changed files with 191 additions and 79 deletions.
95 changes: 83 additions & 12 deletions lib/modules/platform/gerrit/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,14 +160,14 @@ export async function initRepo({
continue;
}
const currentGerritPatchset = change.revisions![change.current_revision!];
const remoteRefSpec = `${currentGerritPatchset.ref}`;
const localRefSpec = `refs/heads/${branchName}`;
await git.fetchRevSpec(`${remoteRefSpec}:${localRefSpec}`);
//TODO/HACK: we fetch the current changeset with the name "origin/" as prefix too. then most util/git/* cmds should work as expected (like isBranchConflicted, isBranchModified)...
await git.fetchRevSpec(`${remoteRefSpec}:refs/heads/origin/${branchName}`);
//const remoteRefSpec = `${currentGerritPatchset.ref}`;
//const localRefSpec = `refs/heads/${branchName}`;
//await git.fetchRevSpec(`${remoteRefSpec}:${localRefSpec}`);
//TODO: for git.branchExists and git.getBranchCommit
await git.registerBranch(
branchName,
currentGerritPatchset.uploader.username !== gerritUsername
currentGerritPatchset.uploader.username !== gerritUsername,
change.current_revision
);
}

Expand All @@ -179,6 +179,79 @@ export async function initRepo({
return repoConfig;
}

export async function isBranchModified(branchName: string): Promise<boolean> {
const change = await findOwnPr({ branchName, state: 'open' }, true).then(
(res) => res.pop()
);
if (change) {
const currentGerritPatchset = change.revisions![change.current_revision!];
return currentGerritPatchset.uploader.username !== gerritUsername;
}
return false;
}

export async function isBranchBehindBase(
branchName: string,
baseBranch: string
): Promise<boolean> {
const change = await findOwnPr({ branchName, state: 'open' }, true).then(
(res) => res.pop()
);
if (change) {
const currentGerritPatchset = change.revisions![change.current_revision!];
return currentGerritPatchset.actions?.['rebase'].enabled === true;
}
return true;
}

export async function isBranchConflicted(
baseBranch: string,
branch: string
): Promise<boolean> {
//TODO: Gerrit can/would do this for us (seen in UI), but where in the REST-Response we found this information?
//if (!git.branchExists(`origin/${branch}`)) { without registering fake-branches this is always false
const change = await findOwnPr({
branchName: branch,
targetBranch: baseBranch,
state: 'open',
}).then((res) => res.pop());
if (change) {
if (change.mergeable) {
//is this the right property?
return change.mergeable;
}
//the long way...see #change.mergeabilityComputationBehavior documentation
const currentGerritPatchset = change.revisions![change.current_revision!];
const remoteRefSpec = `${currentGerritPatchset.ref}`;
await git.fetchRevSpec(`${remoteRefSpec}:refs/heads/origin/${branch}`);
} else {
throw new Error(
`There is no change with branch=${branch} and baseBranch=${baseBranch}`
);
}
//}
return git.isBranchConflicted(baseBranch, branch);
}

export async function branchExists(branchName: string): Promise<boolean> {
const change = await findOwnPr({ branchName, state: 'open' }).then((res) =>
res.pop()
);
return !!change;
}

export async function getBranchCommit(
branchName: string
): Promise<CommitSha | null> {
const change = await findOwnPr({ branchName, state: 'open' }).then((res) =>
res.pop()
);
if (change) {
return change.current_revision!;
}
return null;
}

/**
* in Gerrit: "Searching Changes"
* /changes/?q=$QUERY
Expand Down Expand Up @@ -206,12 +279,12 @@ async function findOwnPr(
reviewLabel,
];
const requestDetails = [
'SUBMITTABLE',
'CHECK',
'SUBMITTABLE', //include the submittable field in ChangeInfo, which can be used to tell if the change is reviewed and ready for submit.
'CHECK', // include potential problems with the change.
'MESSAGES',
'DETAILED_ACCOUNTS',
'LABELS',
'CURRENT_ACTIONS',
'CURRENT_ACTIONS', //to check if current_revision can be "rebase"
'CURRENT_REVISION', //get RevisionInfo::ref to fetch
];
const changes = await gerritHttp.getJson<GerritChange[]>(
Expand Down Expand Up @@ -500,7 +573,7 @@ export async function getJsonFile(
}

export function getRepoForceRebase(): Promise<boolean> {
return Promise.resolve(true);
return Promise.resolve(false);
}

export async function addReviewers(
Expand Down Expand Up @@ -597,8 +670,6 @@ export async function commitFiles(
files: commit.files,
});
if (pushResult) {
//TODO: check why this was done by original commitAndPush method..
await git.registerBranch(commit.branchName, false, commitSha);
if (change && config.approveAvailable && wasApprovedByMe(change)) {
//change was the old change before commit/push. we need to approve again only if it was previously approved from renovate only
await approveChange(change._number);
Expand Down
19 changes: 19 additions & 0 deletions lib/modules/platform/gerrit/scm.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import type { PlatformScm } from '../types';
import {
isBranchBehindBase,
isBranchConflicted,
isBranchModified,
} from './index';

const gerritScm: Partial<PlatformScm> = {
isBranchModified,
isBranchBehindBase,
isBranchConflicted,

//branchExists: ...TODO: Problem the call needs to be async
//getBranchCommit: ...TODO: Problem the call needs to be async

deleteBranch: (branchName: string) => Promise.resolve(), //no-op for gerrit, there exists no remote-branches for PRs
};

export default gerritScm;
3 changes: 2 additions & 1 deletion lib/modules/platform/gerrit/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export interface GerritChange {
unresolved_comment_count?: number;
_number: number;
owner: GerritAccountInfo;
actions?: GerritActionInfo[];
actions?: { [key: string]: GerritActionInfo };
submit_records: any[]; // SubmitRecordInfo[]
requirements?: any[]; //List of the requirements
submit_requirements?: any[]; //List of the SubmitRequirementResultInfo
Expand Down Expand Up @@ -95,6 +95,7 @@ export interface GerritRevisionInfo {
created: Date; //TODO: map
uploader: GerritAccountInfo;
ref: string; //The Git reference for the patch set.
actions?: { [key: string]: GerritActionInfo };
//... many more...still not necessary
}

Expand Down
2 changes: 2 additions & 0 deletions lib/modules/platform/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type { HostRule } from '../../types';
import { setGitAuthor, setNoVerify, setPrivateKey } from '../../util/git';
import * as hostRules from '../../util/host-rules';
import platforms from './api';
import { setPlatformScmApi } from './scm';
import type { Platform } from './types';

export * from './types';
Expand Down Expand Up @@ -35,6 +36,7 @@ export function setPlatformApi(name: string): void {
);
}
_platform = platforms.get(name);
setPlatformScmApi(name);
}

export async function initPlatform(config: AllConfig): Promise<AllConfig> {
Expand Down
33 changes: 33 additions & 0 deletions lib/modules/platform/scm.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import * as _git from '../../util/git';
import gerritScm from './gerrit/scm';
import type { PlatformScm } from './types';

export const platformScmImpls = new Map<string, Partial<PlatformScm>>();
platformScmImpls.set('gerrit', gerritScm);

export const defaultGitScm: PlatformScm = {
isBranchBehindBase: _git.isBranchBehindBase,
isBranchModified: _git.isBranchModified,
isBranchConflicted: _git.isBranchConflicted,
branchExists: _git.branchExists,
getBranchCommit: _git.getBranchCommit,
deleteBranch: _git.deleteBranch,
};

const scmProxy: ProxyHandler<PlatformScm> = {
get(_target: PlatformScm, prop: keyof PlatformScm, receiver: any) {
if (typeof _scm[prop] !== 'undefined') {
return _scm[prop];
}
return Reflect.get(_target, prop, receiver);
},
};

export const scm = new Proxy<PlatformScm>(defaultGitScm, scmProxy);
let _scm: Partial<PlatformScm> = defaultGitScm;

export function setPlatformScmApi(name: string): void {
_scm = platformScmImpls.has(name)
? platformScmImpls.get(name)!
: defaultGitScm;
}
9 changes: 9 additions & 0 deletions lib/modules/platform/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,3 +212,12 @@ export interface Platform {
filterUnavailableUsers?(users: string[]): Promise<string[]>;
commitFiles?(config: CommitFilesConfig): Promise<CommitSha | null>;
}

export interface PlatformScm {
isBranchBehindBase(branchName: string, baseBranch: string): Promise<boolean>;
isBranchModified(branchName: string): Promise<boolean>;
isBranchConflicted(baseBranch: string, branch: string): Promise<boolean>;
branchExists(branchName: string): boolean;
getBranchCommit(branchName: string): CommitSha | null;
deleteBranch(branchName: string): Promise<void>;
}
12 changes: 6 additions & 6 deletions lib/util/git/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,10 @@ export async function gitRetry<T>(gitFunc: () => Promise<T>): Promise<T> {
throw lastError;
}

function localName(branchName: string): string {
return branchName.replace(regEx(/^origin\//), '');
}

async function isDirectory(dir: string): Promise<boolean> {
try {
return (await fs.stat(dir)).isDirectory();
Expand Down Expand Up @@ -580,7 +584,6 @@ export function getBranchList(): string[] {
return Object.keys(config.branchCommits);
}

//TODO: move to Platform-Interface
export async function isBranchBehindBase(
branchName: string,
baseBranch: string
Expand All @@ -602,13 +605,12 @@ export async function isBranchBehindBase(
try {
const { currentBranchSha, currentBranch } = config;
const branches = await git.branch([
'--all', //for gerrit we check by name, there are no real remote branches. origin/$branchname was faked
'--remotes',
'--verbose',
'--contains',
config.currentBranchSha,
]);
isBehind = !branches.all.filter((b) => b.match(`origin/${branchName}$`))
.length;
isBehind = !branches.all.map(localName).includes(branchName);
logger.debug(
{ currentBranch, currentBranchSha },
`branch.isBehindBase(): ${isBehind}`
Expand All @@ -623,7 +625,6 @@ export async function isBranchBehindBase(
}
}

//TODO: move to Platform-Interface
export async function isBranchModified(branchName: string): Promise<boolean> {
if (!branchExists(branchName)) {
logger.debug('branch.isModified(): no cache');
Expand Down Expand Up @@ -689,7 +690,6 @@ export async function isBranchModified(branchName: string): Promise<boolean> {
return true;
}

//TODO: move to Platform-Interface
export async function isBranchConflicted(
baseBranch: string,
branch: string
Expand Down
14 changes: 5 additions & 9 deletions lib/workers/repository/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,13 @@

import { logger } from '../../logger';
import { platform } from '../../modules/platform';
import { scm } from '../../modules/platform/scm';
import { getCache } from '../../util/cache/repository';
import type {
BranchCache,
BranchUpgradeCache,
} from '../../util/cache/repository/types';
import {
getBranchCommit,
isBranchBehindBase,
isBranchConflicted,
isBranchModified,
} from '../../util/git';
import { getBranchCommit } from '../../util/git';
import { getCachedPristineResult } from '../../util/git/pristine';
import type { BranchConfig, BranchUpgradeConfig } from '../types';

Expand Down Expand Up @@ -63,9 +59,9 @@ async function generateBranchCache(
if (branchPr) {
prNo = branchPr.number;
}
isModified = await isBranchModified(branchName);
isBehindBase = await isBranchBehindBase(branchName, baseBranch);
isConflicted = await isBranchConflicted(baseBranch, branchName);
isModified = await scm.isBranchModified(branchName);
isBehindBase = await scm.isBranchBehindBase(branchName, baseBranch);
isConflicted = await scm.isBranchConflicted(baseBranch, branchName);
}
const automerge = !!branch.automerge;
const upgrades: BranchUpgradeCache[] = branch.upgrades
Expand Down
9 changes: 3 additions & 6 deletions lib/workers/repository/config-migration/branch/rebase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,8 @@ import { GlobalConfig } from '../../../../config/global';
import type { RenovateConfig } from '../../../../config/types';
import { logger } from '../../../../logger';
import { commitAndPush } from '../../../../modules/platform/commit';
import {
checkoutBranch,
getFile,
isBranchModified,
} from '../../../../util/git';
import { scm } from '../../../../modules/platform/scm';
import { checkoutBranch, getFile } from '../../../../util/git';
import { quickStringify } from '../../../../util/stringify';
import { getMigrationBranchName } from '../common';
import { ConfigMigrationCommitMessageFactory } from './commit-message';
Expand All @@ -19,7 +16,7 @@ export async function rebaseMigrationBranch(
): Promise<string | null> {
logger.debug('Checking if migration branch needs rebasing');
const branchName = getMigrationBranchName(config);
if (await isBranchModified(branchName)) {
if (await scm.isBranchModified(branchName)) {
logger.debug('Migration branch has been edited and cannot be rebased');
return null;
}
Expand Down
13 changes: 5 additions & 8 deletions lib/workers/repository/finalise/prune.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,8 @@ import { REPOSITORY_CHANGED } from '../../../constants/error-messages';
import { logger } from '../../../logger';
import { platform } from '../../../modules/platform';
import { ensureComment } from '../../../modules/platform/comment';
import {
deleteBranch,
getBranchList,
isBranchModified,
} from '../../../util/git';
import { scm } from '../../../modules/platform/scm';
import { getBranchList } from '../../../util/git';

async function cleanUpBranches(
{ pruneStaleBranches: enabled }: RenovateConfig,
Expand All @@ -24,7 +21,7 @@ async function cleanUpBranches(
branchName,
state: 'open',
});
const branchIsModified = await isBranchModified(branchName);
const branchIsModified = await scm.isBranchModified(branchName);
if (pr) {
if (branchIsModified) {
logger.debug(
Expand Down Expand Up @@ -69,15 +66,15 @@ async function cleanUpBranches(
prTitle: newPrTitle,
state: 'closed',
});
await deleteBranch(branchName);
await scm.deleteBranch(branchName);
}
} else if (branchIsModified) {
logger.debug('Orphan Branch is modified - skipping branch deletion');
} else if (GlobalConfig.get('dryRun')) {
logger.info(`DRY-RUN: Would delete orphan branch ${branchName}`);
} else {
logger.info({ branch: branchName }, `Deleting orphan branch`);
await deleteBranch(branchName);
await scm.deleteBranch(branchName);
}
} catch (err) /* istanbul ignore next */ {
if (err.message === 'config-validation') {
Expand Down
Loading

0 comments on commit 516762a

Please sign in to comment.