From 86ab028f9704311d9b99f4f72b2011a43127d8b4 Mon Sep 17 00:00:00 2001 From: Rhys Arkins <rhys@arkins.net> Date: Tue, 5 May 2020 16:02:29 +0200 Subject: [PATCH] fix: Revert "refactor(platform): optimize getFileList and remove branchName param (#6127)" This reverts commit 7d1c9376823b1decee349889e6ea6d8f11e1e151. --- lib/platform/azure/index.ts | 9 +++- lib/platform/bitbucket-server/index.ts | 8 ++- lib/platform/bitbucket/index.ts | 8 ++- lib/platform/bitbucket/utils.ts | 1 + .../git/__snapshots__/storage.spec.ts.snap | 15 +++--- lib/platform/git/storage.spec.ts | 16 +++--- lib/platform/git/storage.ts | 54 +++++++++++-------- lib/platform/gitea/index.ts | 2 +- lib/platform/github/index.ts | 4 +- lib/platform/gitlab/index.ts | 4 +- lib/workers/repository/extract/file-match.ts | 6 ++- 11 files changed, 75 insertions(+), 52 deletions(-) diff --git a/lib/platform/azure/index.ts b/lib/platform/azure/index.ts index 711fd0ba3d..903c723509 100644 --- a/lib/platform/azure/index.ts +++ b/lib/platform/azure/index.ts @@ -48,6 +48,7 @@ interface Config { project: string; azureWorkItemId: string; prList: Pr[]; + fileList: null; repository: string; } @@ -181,8 +182,10 @@ export function getRepoForceRebase(): Promise<boolean> { // Search -export /* istanbul ignore next */ function getFileList(): Promise<string[]> { - return config.storage.getFileList(); +export /* istanbul ignore next */ function getFileList( + branchName?: string +): Promise<string[]> { + return config.storage.getFileList(branchName); } export /* istanbul ignore next */ async function setBaseBranch( @@ -191,7 +194,9 @@ export /* istanbul ignore next */ async function setBaseBranch( logger.debug(`Setting baseBranch to ${branchName}`); config.baseBranch = branchName; delete config.baseCommitSHA; + delete config.fileList; const baseBranchSha = await config.storage.setBaseBranch(branchName); + await getFileList(branchName); return baseBranchSha; } diff --git a/lib/platform/bitbucket-server/index.ts b/lib/platform/bitbucket-server/index.ts index 3cdd5b269b..2d5f726bc5 100644 --- a/lib/platform/bitbucket-server/index.ts +++ b/lib/platform/bitbucket-server/index.ts @@ -49,6 +49,7 @@ interface BbsConfig { baseBranch: string; bbUseDefaultReviewers: boolean; defaultBranch: string; + fileList: any[]; mergeMethod: string; owner: string; prList: Pr[]; @@ -272,8 +273,11 @@ export /* istanbul ignore next */ function setBranchPrefix( // Search // Get full file list -export function getFileList(): Promise<string[]> { - return config.storage.getFileList(); +export function getFileList( + branchName: string = config.baseBranch +): Promise<string[]> { + logger.debug(`getFileList(${branchName})`); + return config.storage.getFileList(branchName); } // Branch diff --git a/lib/platform/bitbucket/index.ts b/lib/platform/bitbucket/index.ts index a7c0f31919..d516a162c3 100644 --- a/lib/platform/bitbucket/index.ts +++ b/lib/platform/bitbucket/index.ts @@ -173,8 +173,8 @@ export function getRepoForceRebase(): Promise<boolean> { // Search // Get full file list -export function getFileList(): Promise<string[]> { - return config.storage.getFileList(); +export function getFileList(branchName?: string): Promise<string[]> { + return config.storage.getFileList(branchName); } export async function setBaseBranch( @@ -182,7 +182,10 @@ export async function setBaseBranch( ): Promise<string> { logger.debug(`Setting baseBranch to ${branchName}`); config.baseBranch = branchName; + delete config.baseCommitSHA; + delete config.fileList; const baseBranchSha = await config.storage.setBaseBranch(branchName); + await getFileList(branchName); return baseBranchSha; } @@ -830,6 +833,7 @@ export async function mergePr( }, } ); + delete config.baseCommitSHA; logger.debug('Automerging succeeded'); } catch (err) /* istanbul ignore next */ { return false; diff --git a/lib/platform/bitbucket/utils.ts b/lib/platform/bitbucket/utils.ts index c4721cbbee..1461bf2860 100644 --- a/lib/platform/bitbucket/utils.ts +++ b/lib/platform/bitbucket/utils.ts @@ -9,6 +9,7 @@ export interface Config { baseBranch: string; baseCommitSHA: string; defaultBranch: string; + fileList: any[]; has_issues: boolean; mergeMethod: string; owner: string; diff --git a/lib/platform/git/__snapshots__/storage.spec.ts.snap b/lib/platform/git/__snapshots__/storage.spec.ts.snap index 84944dea60..28860dd7e4 100644 --- a/lib/platform/git/__snapshots__/storage.spec.ts.snap +++ b/lib/platform/git/__snapshots__/storage.spec.ts.snap @@ -11,24 +11,21 @@ Array [ exports[`platform/git/storage getFile(filePath, branchName) returns null for 404 1`] = `[Error: repository-changed]`; -exports[`platform/git/storage getFileList() returns cached version 1`] = ` +exports[`platform/git/storage getFileList() defaults to master 1`] = `Array []`; + +exports[`platform/git/storage getFileList() should exclude submodules 1`] = ` Array [ + ".gitmodules", "file_to_delete", "master_file", "past_file", ] `; -exports[`platform/git/storage getFileList() returns from other branch 1`] = ` -Array [ - "past_file", -] -`; - -exports[`platform/git/storage getFileList() should exclude submodules 1`] = ` +exports[`platform/git/storage getFileList() should return the correct files 1`] = ` Array [ - ".gitmodules", "file_to_delete", + "future_file", "master_file", "past_file", ] diff --git a/lib/platform/git/storage.spec.ts b/lib/platform/git/storage.spec.ts index 5f116968a6..30c12c7148 100644 --- a/lib/platform/git/storage.spec.ts +++ b/lib/platform/git/storage.spec.ts @@ -82,6 +82,12 @@ describe('platform/git/storage', () => { }); }); describe('getFileList()', () => { + it('returns empty array if error', async () => { + expect(await git.getFileList('not_found')).toEqual([]); + }); + it('should return the correct files', async () => { + expect(await git.getFileList('renovate/future_branch')).toMatchSnapshot(); + }); it('should exclude submodules', async () => { const repo = Git(base.path).silent(true); await repo.submoduleAdd(base.path, 'submodule'); @@ -91,18 +97,12 @@ describe('platform/git/storage', () => { url: base.path, }); expect(await fs.exists(tmpDir.path + '/.gitmodules')).toBeTruthy(); - expect(await git.getFileList()).toMatchSnapshot(); + expect(await git.getFileList('master')).toMatchSnapshot(); await repo.reset(['--hard', 'HEAD^']); }); - it('returns cached version', async () => { + it('defaults to master', async () => { expect(await git.getFileList()).toMatchSnapshot(); }); - it('returns from other branch', async () => { - await git.setBaseBranch('develop'); - const res = await git.getFileList(); - expect(res).toMatchSnapshot(); - expect(res).toHaveLength(1); - }); }); describe('branchExists(branchName)', () => { it('should return true if found', async () => { diff --git a/lib/platform/git/storage.ts b/lib/platform/git/storage.ts index 0856e19cfb..2aca6841e7 100644 --- a/lib/platform/git/storage.ts +++ b/lib/platform/git/storage.ts @@ -32,7 +32,7 @@ interface LocalConfig extends StorageConfig { baseBranchSha: string; branchExists: Record<string, boolean>; branchPrefix: string; - fileList?: string[]; + fileList: Record<string, Promise<string[]>>; } // istanbul ignore next @@ -109,6 +109,7 @@ export class Storage { // eslint-disable-next-line no-multi-assign const config: LocalConfig = (this._config = { ...args, + fileList: {}, } as any); // eslint-disable-next-line no-multi-assign const cwd = (this._cwd = config.localDir); @@ -259,7 +260,6 @@ export class Storage { } logger.debug(`Setting baseBranch to ${branchName}`); this._config.baseBranch = branchName; - delete this._config.fileList; try { if (branchName !== 'master') { this._config.baseBranchSha = ( @@ -305,28 +305,36 @@ export class Storage { } } - async getFileList(): Promise<string[]> { - if (!this._config.fileList) { - const submodules = await this.getSubmodules(); - const files: string = await this._git.raw([ - 'ls-tree', - '-r', - '--name-only', - 'HEAD', - ]); - // istanbul ignore else - if (files) { - this._config.fileList = files - .split('\n') - .filter(Boolean) - .filter((file: string) => - submodules.every((submodule: string) => !file.startsWith(submodule)) - ); - } else { - this._config.fileList = []; - } + async getFileList(branchName?: string): Promise<string[]> { + const branch = branchName || this._config.baseBranch; + if (this._config.fileList[branch] === undefined) { + this._config.fileList[branch] = this.getFileListInner(branchName); + } + return this._config.fileList[branch]; + } + + async getFileListInner(branch: string): Promise<string[]> { + const exists = await this.branchExists(branch); + if (!exists) { + return []; } - return this._config.fileList; + const submodules = await this.getSubmodules(); + const files: string = await this._git.raw([ + 'ls-tree', + '-r', + '--name-only', + 'origin/' + branch, + ]); + // istanbul ignore if + if (!files) { + return []; + } + return files + .split('\n') + .filter(Boolean) + .filter((file: string) => + submodules.every((submodule: string) => !file.startsWith(submodule)) + ); } async getSubmodules(): Promise<string[]> { diff --git a/lib/platform/gitea/index.ts b/lib/platform/gitea/index.ts index e50378ddf0..cbdc65eb0f 100644 --- a/lib/platform/gitea/index.ts +++ b/lib/platform/gitea/index.ts @@ -929,7 +929,7 @@ const platform: Platform = { }, getFileList(): Promise<string[]> { - return config.storage.getFileList(); + return config.storage.getFileList(config.baseBranch); }, getAllRenovateBranches(branchPrefix: string): Promise<string[]> { diff --git a/lib/platform/github/index.ts b/lib/platform/github/index.ts index 9a1e7d5be4..d1080f761c 100644 --- a/lib/platform/github/index.ts +++ b/lib/platform/github/index.ts @@ -535,8 +535,8 @@ export function setBranchPrefix(branchPrefix: string): Promise<void> { // Search // istanbul ignore next -export function getFileList(): Promise<string[]> { - return config.storage.getFileList(); +export function getFileList(branchName = config.baseBranch): Promise<string[]> { + return config.storage.getFileList(branchName); } // Branch diff --git a/lib/platform/gitlab/index.ts b/lib/platform/gitlab/index.ts index e5418ad2a4..e3c620e7b2 100644 --- a/lib/platform/gitlab/index.ts +++ b/lib/platform/gitlab/index.ts @@ -286,8 +286,8 @@ export /* istanbul ignore next */ function setBranchPrefix( // Search // Get full file list -export function getFileList(): Promise<string[]> { - return config.storage.getFileList(); +export function getFileList(branchName = config.baseBranch): Promise<string[]> { + return config.storage.getFileList(branchName); } // Returns true if branch exists, otherwise false diff --git a/lib/workers/repository/extract/file-match.ts b/lib/workers/repository/extract/file-match.ts index b3a976ce0c..04e6ee64f0 100644 --- a/lib/workers/repository/extract/file-match.ts +++ b/lib/workers/repository/extract/file-match.ts @@ -35,6 +35,10 @@ export function filterIgnoredFiles( ); } +export function getFileList(): Promise<string[]> { + return platform.getFileList(); +} + export function getFilteredFileList( config: RenovateConfig, fileList: string[] @@ -48,7 +52,7 @@ export function getFilteredFileList( export async function getMatchingFiles( config: RenovateConfig ): Promise<string[]> { - const allFiles = await platform.getFileList(); + const allFiles = await getFileList(); const fileList = getFilteredFileList(config, allFiles); const { fileMatch, manager } = config; let matchedFiles = []; -- GitLab