From eafce83889b3a968a0684d283ed5adb02e9b37bd Mon Sep 17 00:00:00 2001 From: Michael Kriese <michael.kriese@visualon.de> Date: Thu, 9 Apr 2020 07:43:47 +0200 Subject: [PATCH] fix: binary file handling (#5916) --- lib/manager/common.ts | 6 ++- lib/manager/gradle-wrapper/artifacts.spec.ts | 39 ++++++-------------- lib/manager/gradle-wrapper/artifacts.ts | 34 ++++++----------- lib/platform/azure/index.ts | 3 +- lib/platform/bitbucket-server/index.ts | 3 +- lib/platform/bitbucket/index.ts | 3 +- lib/platform/common.ts | 20 +++++++++- lib/platform/git/storage.ts | 23 +----------- lib/platform/gitea/index.spec.ts | 3 +- lib/platform/gitea/index.ts | 3 +- lib/platform/github/index.ts | 3 +- lib/platform/gitlab/index.ts | 3 +- lib/workers/branch/get-updated.ts | 8 ++-- lib/workers/branch/index.ts | 9 ++--- lib/workers/common.ts | 6 +-- 15 files changed, 69 insertions(+), 97 deletions(-) diff --git a/lib/manager/common.ts b/lib/manager/common.ts index eba78df889..6869a3da2e 100644 --- a/lib/manager/common.ts +++ b/lib/manager/common.ts @@ -1,7 +1,7 @@ import { ReleaseType } from 'semver'; import { RangeStrategy, SkipReason } from '../types'; import { ValidationMessage, GlobalConfig, UpdateType } from '../config/common'; -import { FileData } from '../platform/common'; +import { File } from '../platform/common'; export type Result<T> = T | Promise<T>; @@ -41,6 +41,8 @@ export interface UpdateArtifactsConfig extends ManagerConfig { cacheDir?: string; postUpdateOptions?: string[]; ignoreScripts?: boolean; + + toVersion?: string; } export interface PackageUpdateConfig { @@ -188,7 +190,7 @@ export interface ArtifactError { export interface UpdateArtifactsResult { artifactError?: ArtifactError; - file?: FileData; + file?: File; } export interface UpdateArtifact { diff --git a/lib/manager/gradle-wrapper/artifacts.spec.ts b/lib/manager/gradle-wrapper/artifacts.spec.ts index 93d3be3094..f54c718190 100644 --- a/lib/manager/gradle-wrapper/artifacts.spec.ts +++ b/lib/manager/gradle-wrapper/artifacts.spec.ts @@ -3,12 +3,14 @@ import { resolve } from 'path'; import Git from 'simple-git/promise'; import * as dcUpdate from '.'; import { platform as _platform } from '../../platform'; -import { mocked } from '../../../test/util'; +import { mocked, getName } from '../../../test/util'; import { ifSystemSupportsGradle } from '../gradle/__testutil__/gradle'; +import { setUtilConfig } from '../../util'; const platform = mocked(_platform); const config = { localDir: resolve(__dirname, './__fixtures__/testFiles'), + toVersion: '5.6.4', }; jest.mock('../../util/got'); @@ -23,14 +25,14 @@ async function resetTestFiles() { }); } -describe('manager/gradle-wrapper/update', () => { +describe(getName(__filename), () => { + beforeAll(() => { + setUtilConfig(config); + }); + beforeEach(() => { + jest.resetAllMocks(); + }); describe('updateArtifacts - replaces existing value', () => { - beforeEach(() => { - jest.resetModules(); - jest.resetAllMocks(); - jest.clearAllMocks(); - }); - ifSystemSupportsGradle(6).it('replaces existing value', async () => { try { jest.setTimeout(5 * 60 * 1000); @@ -53,7 +55,7 @@ describe('manager/gradle-wrapper/update', () => { ), 'utf8' ), - config, + config: { ...config, toVersion: '6.3' }, }); expect(res).toEqual( @@ -64,7 +66,6 @@ describe('manager/gradle-wrapper/update', () => { 'gradlew.bat', ].map(fileProjectPath => { return { - artifactError: null, file: { name: fileProjectPath, contents: readFileSync( @@ -98,12 +99,6 @@ describe('manager/gradle-wrapper/update', () => { }); }); describe('updateArtifacts - up to date', () => { - beforeEach(() => { - jest.resetModules(); - jest.resetAllMocks(); - jest.clearAllMocks(); - }); - ifSystemSupportsGradle(6).it('up to date', async () => { try { jest.setTimeout(5 * 60 * 1000); @@ -148,12 +143,6 @@ describe('manager/gradle-wrapper/update', () => { }); }); describe('updateArtifacts - error handling - getRepoStatus', () => { - beforeEach(() => { - jest.resetModules(); - jest.resetAllMocks(); - jest.clearAllMocks(); - }); - ifSystemSupportsGradle(6).it('error handling - getRepoStatus', async () => { try { jest.setTimeout(5 * 60 * 1000); @@ -201,12 +190,6 @@ describe('manager/gradle-wrapper/update', () => { }); describe('updateArtifacts - error handling - command execution', () => { - beforeEach(() => { - jest.resetModules(); - jest.resetAllMocks(); - jest.clearAllMocks(); - }); - ifSystemSupportsGradle(6).it( 'error handling - command execution', async () => { diff --git a/lib/manager/gradle-wrapper/artifacts.ts b/lib/manager/gradle-wrapper/artifacts.ts index 322826ec1b..0e3b8f5270 100644 --- a/lib/manager/gradle-wrapper/artifacts.ts +++ b/lib/manager/gradle-wrapper/artifacts.ts @@ -1,4 +1,3 @@ -/* istanbul ignore file */ import Git from 'simple-git/promise'; import { resolve } from 'path'; import * as fs from 'fs-extra'; @@ -7,21 +6,17 @@ import { UpdateArtifact, UpdateArtifactsResult } from '../common'; import { exec, ExecOptions } from '../../util/exec'; import { readLocalFile } from '../../util/fs'; import { platform } from '../../platform'; -import { VERSION_REGEX } from './search'; import { gradleWrapperFileName, prepareGradleCommand } from '../gradle/index'; async function addIfUpdated( status: Git.StatusResult, - projectDir: string, fileProjectPath: string ): Promise<UpdateArtifactsResult | null> { if (status.modified.includes(fileProjectPath)) { - const filePath = resolve(projectDir, `./${fileProjectPath}`); return { - artifactError: null, file: { name: fileProjectPath, - contents: await readLocalFile(filePath), + contents: await readLocalFile(fileProjectPath), }, }; } @@ -31,34 +26,28 @@ async function addIfUpdated( export async function updateArtifacts({ packageFileName, updatedDeps, - newPackageFileContent, config, }: UpdateArtifact): Promise<UpdateArtifactsResult[] | null> { try { const projectDir = config.localDir; - logger.debug(updatedDeps, 'gradle-wrapper.updateArtifacts()'); - const gradlewPath = resolve( - projectDir, - `./${gradleWrapperFileName(config)}` - ); - const version = VERSION_REGEX.exec(newPackageFileContent).groups.version; - await prepareGradleCommand( - gradleWrapperFileName(config), + logger.debug({ updatedDeps }, 'gradle-wrapper.updateArtifacts()'); + const gradlew = gradleWrapperFileName(config); + const gradlewPath = resolve(projectDir, `./${gradlew}`); + const cmd = await prepareGradleCommand( + gradlew, projectDir, await fs.stat(gradlewPath).catch(() => null), - null + `wrapper --gradle-version ${config.toVersion}` ); - const cmd = `${gradlewPath} wrapper --gradle-version ${version} --project-dir ${projectDir}`; logger.debug(`Updating gradle wrapper: "${cmd}"`); const execOptions: ExecOptions = { - cwd: config.localDir, docker: { image: 'renovate/gradle', }, }; try { await exec(cmd, execOptions); - } catch (err) /* istanbul ignore next */ { + } catch (err) { logger.warn( { err }, 'Error executing gradle wrapper update command. It can be not a critical one though.' @@ -72,13 +61,12 @@ export async function updateArtifacts({ 'gradle/wrapper/gradle-wrapper.jar', 'gradlew', 'gradlew.bat', - ].map(async fileProjectPath => - addIfUpdated(status, projectDir, fileProjectPath) - ) + ].map(async fileProjectPath => addIfUpdated(status, fileProjectPath)) ) ).filter(e => e != null); logger.debug( - `Returning updated gradle-wrapper files: ${updateArtifactsResult}` + { files: updateArtifactsResult.map(r => r.file.name) }, + `Returning updated gradle-wrapper files` ); return updateArtifactsResult; } catch (err) { diff --git a/lib/platform/azure/index.ts b/lib/platform/azure/index.ts index 2f36c5d847..cb5debc846 100644 --- a/lib/platform/azure/index.ts +++ b/lib/platform/azure/index.ts @@ -6,7 +6,7 @@ import { import * as azureHelper from './azure-helper'; import * as azureApi from './azure-got-wrapper'; import * as hostRules from '../../util/host-rules'; -import GitStorage, { StatusResult, CommitFilesConfig } from '../git/storage'; +import GitStorage, { StatusResult } from '../git/storage'; import { logger } from '../../logger'; import { PlatformConfig, @@ -20,6 +20,7 @@ import { FindPRConfig, EnsureCommentConfig, EnsureIssueResult, + CommitFilesConfig, } from '../common'; import { sanitize } from '../../util/sanitize'; import { smartTruncate } from '../utils/pr-body'; diff --git a/lib/platform/bitbucket-server/index.ts b/lib/platform/bitbucket-server/index.ts index f91389d6d3..eda4791916 100644 --- a/lib/platform/bitbucket-server/index.ts +++ b/lib/platform/bitbucket-server/index.ts @@ -4,7 +4,7 @@ import delay from 'delay'; import { api } from './bb-got-wrapper'; import * as utils from './utils'; import * as hostRules from '../../util/host-rules'; -import GitStorage, { StatusResult, CommitFilesConfig } from '../git/storage'; +import GitStorage, { StatusResult } from '../git/storage'; import { logger } from '../../logger'; import { PlatformConfig, @@ -20,6 +20,7 @@ import { EnsureCommentConfig, EnsureIssueResult, EnsureIssueConfig, + CommitFilesConfig, } from '../common'; import { sanitize } from '../../util/sanitize'; import { smartTruncate } from '../utils/pr-body'; diff --git a/lib/platform/bitbucket/index.ts b/lib/platform/bitbucket/index.ts index 1fcda29f1c..d7369de70e 100644 --- a/lib/platform/bitbucket/index.ts +++ b/lib/platform/bitbucket/index.ts @@ -5,7 +5,7 @@ import { api } from './bb-got-wrapper'; import * as utils from './utils'; import * as hostRules from '../../util/host-rules'; import { logger } from '../../logger'; -import GitStorage, { StatusResult, CommitFilesConfig } from '../git/storage'; +import GitStorage, { StatusResult } from '../git/storage'; import { readOnlyIssueBody } from '../utils/read-only-issue-body'; import * as comments from './comments'; import { @@ -21,6 +21,7 @@ import { FindPRConfig, EnsureCommentConfig, EnsureIssueResult, + CommitFilesConfig, } from '../common'; import { sanitize } from '../../util/sanitize'; import { smartTruncate } from '../utils/pr-body'; diff --git a/lib/platform/common.ts b/lib/platform/common.ts index b234d58c41..2217d14a0b 100644 --- a/lib/platform/common.ts +++ b/lib/platform/common.ts @@ -1,14 +1,30 @@ import got from 'got'; import Git from 'simple-git/promise'; import { RenovateConfig } from '../config/common'; -import { CommitFilesConfig } from './git/storage'; import { BranchStatus } from '../types'; -export interface FileData { +/** + * File to commit to branch + */ +export interface File { + /** + * Relative file path + */ name: string; + + /** + * file contents + */ contents: string | Buffer; } +export type CommitFilesConfig = { + branchName: string; + files: File[]; + message: string; + parentBranch?: string; +}; + export interface GotApiOptions { useCache?: boolean; hostType?: string; diff --git a/lib/platform/git/storage.ts b/lib/platform/git/storage.ts index eebd206a56..4c25006bf7 100644 --- a/lib/platform/git/storage.ts +++ b/lib/platform/git/storage.ts @@ -12,6 +12,7 @@ import { REPOSITORY_EMPTY, REPOSITORY_TEMPORARY_ERROR, } from '../../constants/error-messages'; +import { CommitFilesConfig } from '../common'; declare module 'fs-extra' { export function exists(pathLike: string): Promise<boolean>; @@ -19,21 +20,6 @@ declare module 'fs-extra' { export type StatusResult = Git.StatusResult; -/** - * File to commit to branch - */ -export interface File { - /** - * Relative file path - */ - name: string; - - /** - * file contents - */ - contents: string | Buffer; -} - interface StorageConfig { localDir: string; baseBranch?: string; @@ -49,13 +35,6 @@ interface LocalConfig extends StorageConfig { branchPrefix: string; } -export type CommitFilesConfig = { - branchName: string; - files: File[]; - message: string; - parentBranch?: string; -}; - // istanbul ignore next function checkForPlatformFailure(err: Error): void { if (process.env.NODE_ENV === 'test') { diff --git a/lib/platform/gitea/index.spec.ts b/lib/platform/gitea/index.spec.ts index d325809190..ff9c0e10be 100644 --- a/lib/platform/gitea/index.spec.ts +++ b/lib/platform/gitea/index.spec.ts @@ -15,11 +15,12 @@ import { RepoConfig, RepoParams, Platform, + CommitFilesConfig, + File, } from '..'; import { logger as _logger } from '../../logger'; import { BranchStatus } from '../../types'; import { GiteaGotApi } from './gitea-got-wrapper'; -import { CommitFilesConfig, File } from '../git/storage'; describe('platform/gitea', () => { let gitea: Platform; diff --git a/lib/platform/gitea/index.ts b/lib/platform/gitea/index.ts index 4ba3119ecb..4a12c91926 100644 --- a/lib/platform/gitea/index.ts +++ b/lib/platform/gitea/index.ts @@ -1,5 +1,5 @@ import URL from 'url'; -import GitStorage, { CommitFilesConfig, StatusResult } from '../git/storage'; +import GitStorage, { StatusResult } from '../git/storage'; import * as hostRules from '../../util/host-rules'; import { BranchStatusConfig, @@ -14,6 +14,7 @@ import { RepoConfig, RepoParams, VulnerabilityAlert, + CommitFilesConfig, } from '../common'; import { api } from './gitea-got-wrapper'; import { PLATFORM_TYPE_GITEA } from '../../constants/platforms'; diff --git a/lib/platform/github/index.ts b/lib/platform/github/index.ts index 1fef0446d7..d1ed07e7dc 100644 --- a/lib/platform/github/index.ts +++ b/lib/platform/github/index.ts @@ -5,7 +5,7 @@ import URL from 'url'; import { logger } from '../../logger'; import { api } from './gh-got-wrapper'; import * as hostRules from '../../util/host-rules'; -import GitStorage, { StatusResult, CommitFilesConfig } from '../git/storage'; +import GitStorage, { StatusResult } from '../git/storage'; import { PlatformConfig, RepoParams, @@ -18,6 +18,7 @@ import { FindPRConfig, EnsureCommentConfig, EnsureIssueResult, + CommitFilesConfig, } from '../common'; import { configFileNames } from '../../config/app-strings'; diff --git a/lib/platform/gitlab/index.ts b/lib/platform/gitlab/index.ts index f30100236a..cc445eda37 100644 --- a/lib/platform/gitlab/index.ts +++ b/lib/platform/gitlab/index.ts @@ -3,7 +3,7 @@ import is from '@sindresorhus/is'; import { api } from './gl-got-wrapper'; import * as hostRules from '../../util/host-rules'; -import GitStorage, { StatusResult, CommitFilesConfig } from '../git/storage'; +import GitStorage, { StatusResult } from '../git/storage'; import { PlatformConfig, RepoParams, @@ -17,6 +17,7 @@ import { BranchStatusConfig, FindPRConfig, EnsureCommentConfig, + CommitFilesConfig, } from '../common'; import { configFileNames } from '../../config/app-strings'; import { logger } from '../../logger'; diff --git a/lib/workers/branch/get-updated.ts b/lib/workers/branch/get-updated.ts index 26274f7d9e..69f227f9fb 100644 --- a/lib/workers/branch/get-updated.ts +++ b/lib/workers/branch/get-updated.ts @@ -1,5 +1,5 @@ import is from '@sindresorhus/is'; -import { FileData, platform } from '../../platform'; +import { File, platform } from '../../platform'; import { logger } from '../../logger'; import { get } from '../../manager'; import { ArtifactError } from '../../manager/common'; @@ -11,8 +11,8 @@ import { BranchConfig } from '../common'; export interface PackageFilesResult { artifactErrors: ArtifactError[]; parentBranch?: string; - updatedPackageFiles: FileData[]; - updatedArtifacts: FileData[]; + updatedPackageFiles: File[]; + updatedArtifacts: File[]; } export async function getUpdatedPackageFiles( @@ -108,7 +108,7 @@ export async function getUpdatedPackageFiles( name, contents: updatedFileContents[name], })); - const updatedArtifacts: FileData[] = []; + const updatedArtifacts: File[] = []; const artifactErrors: ArtifactError[] = []; for (const packageFile of updatedPackageFiles) { const manager = packageFileManagers[packageFile.name]; diff --git a/lib/workers/branch/index.ts b/lib/workers/branch/index.ts index f703fb9076..c2e9d522cb 100644 --- a/lib/workers/branch/index.ts +++ b/lib/workers/branch/index.ts @@ -1,8 +1,6 @@ import { DateTime } from 'luxon'; -import { readFile } from 'fs-extra'; import is from '@sindresorhus/is'; import minimatch from 'minimatch'; -import { join } from 'upath'; import { logger } from '../../logger'; import { isScheduledNow } from './schedule'; import { getUpdatedPackageFiles } from './get-updated'; @@ -40,6 +38,7 @@ import { import { BranchStatus } from '../../types'; import { exec } from '../../util/exec'; import { regEx } from '../../util/regex'; +import { readLocalFile } from '../../util/fs'; // TODO: proper typings function rebaseCheck(config: RenovateConfig, branchPr: any): boolean { @@ -367,12 +366,10 @@ export async function processBranch( { file: relativePath, pattern }, 'Post-upgrade file saved' ); - const existingContent = await readFile( - join(config.localDir, relativePath) - ); + const existingContent = await readLocalFile(relativePath); config.updatedArtifacts.push({ name: relativePath, - contents: existingContent.toString(), + contents: existingContent, }); } } diff --git a/lib/workers/common.ts b/lib/workers/common.ts index da26efba81..af470e93ab 100644 --- a/lib/workers/common.ts +++ b/lib/workers/common.ts @@ -8,7 +8,7 @@ import { ValidationMessage, } from '../config'; import { LookupUpdate } from './repository/process/lookup/common'; -import { FileData, PlatformPrOptions } from '../platform'; +import { File, PlatformPrOptions } from '../platform'; import { Release } from '../datasource'; export interface BranchUpgradeConfig @@ -44,8 +44,8 @@ export interface BranchUpgradeConfig releaseTimestamp?: string; sourceDirectory?: string; - updatedPackageFiles?: FileData[]; - updatedArtifacts?: FileData[]; + updatedPackageFiles?: File[]; + updatedArtifacts?: File[]; } export enum PrResult { -- GitLab