From 572749a3980ccd952699c1baf6f4303f899921ab Mon Sep 17 00:00:00 2001 From: Rhys Arkins Date: Wed, 21 Apr 2021 17:53:57 +0200 Subject: [PATCH] refactor: artifactUpdateApproach -> skipInstalls Roll back the previous refactor. --- docs/usage/self-hosted-configuration.md | 14 +++++----- .../__snapshots__/migration.spec.ts.snap | 2 -- lib/config/definitions.ts | 9 +++--- lib/config/migration.spec.ts | 2 -- lib/config/migration.ts | 7 ----- .../extract/__snapshots__/index.spec.ts.snap | 28 +++++++++---------- lib/manager/npm/extract/index.ts | 19 +++++++++---- lib/manager/npm/post-update/lerna.spec.ts | 8 +++--- lib/manager/npm/post-update/lerna.ts | 6 ++-- lib/manager/npm/post-update/npm.spec.ts | 20 ++++++------- lib/manager/npm/post-update/npm.ts | 4 +-- lib/manager/npm/post-update/yarn.ts | 2 +- lib/manager/types.ts | 5 +++- .../__snapshots__/manager-files.spec.ts.snap | 2 +- 14 files changed, 64 insertions(+), 64 deletions(-) diff --git a/docs/usage/self-hosted-configuration.md b/docs/usage/self-hosted-configuration.md index 58a81776bb32bb..86c10c63deae4d 100644 --- a/docs/usage/self-hosted-configuration.md +++ b/docs/usage/self-hosted-configuration.md @@ -28,7 +28,7 @@ module.exports = { In the `renovate.json` file, define the commands and files to be included in the final commit. -The command to install dependencies (`npm ci --ignore-scripts`) is necessary because, by default, the installation of dependencies is skipped (see the `artifactUpdateApproach` admin option). +The command to install dependencies (`npm ci --ignore-scripts`) is necessary because, by default, the installation of dependencies is skipped (see the `skipInstalls` admin option). ```json { @@ -69,12 +69,6 @@ e.g. } ``` -## artifactUpdateApproach - -By default, Renovate will use the most efficient approach to updating package files and lock files, which in most cases skips the need to perform a full module install by the bot. -If this is set to 'deep', then a full install of modules will be done. -This is currently applicable to `npm` and `yarn` only, and automatically set to `deep` when a full install is detected as necessary. - ## autodiscover When you enable `autodiscover`, by default, Renovate will run on _every_ repository that the bot account can access. @@ -372,6 +366,12 @@ It could then be used in a repository config or preset like so: Secret names must start with a upper or lower case character and can contain only characters, digits, or underscores. +## skipInstalls + +By default, Renovate will use the most efficient approach to updating package files and lock files, which in most cases skips the need to perform a full module install by the bot. +If this is set to false, then a full install of modules will be done. +This is currently applicable to `npm` and `lerna`/`npm` only, and only used in cases where bugs in `npm` result in incorrect lock files being updated. + ## token ## username diff --git a/lib/config/__snapshots__/migration.spec.ts.snap b/lib/config/__snapshots__/migration.spec.ts.snap index c9c32342a28022..7713dfac4bc167 100644 --- a/lib/config/__snapshots__/migration.spec.ts.snap +++ b/lib/config/__snapshots__/migration.spec.ts.snap @@ -80,7 +80,6 @@ Object { "additionalBranchPrefix": "{{parentDir}}-", "allowCustomCrateRegistries": true, "allowScripts": true, - "artifactUpdateApproach": "shallow", "autodiscover": true, "automerge": false, "automergeType": "branch", @@ -175,7 +174,6 @@ Object { "versioning": "maven", }, Object { - "artifactUpdateApproach": "deep", "matchDepTypes": Array [ "peerDependencies", ], diff --git a/lib/config/definitions.ts b/lib/config/definitions.ts index 54f8f9eb9a71df..76dcf0d28c0de3 100644 --- a/lib/config/definitions.ts +++ b/lib/config/definitions.ts @@ -556,12 +556,11 @@ const options: RenovateOptions[] = [ type: 'boolean', }, { - name: 'artifactUpdateApproach', + name: 'skipInstalls', description: - 'Whether to employ a deep or shallow approach to artifact updating.', - type: 'string', - allowedValues: ['auto', 'deep', 'shallow'], - default: 'auto', + 'Skip installing modules/dependencies if lock file updating is possible alone.', + type: 'boolean', + default: null, admin: true, }, { diff --git a/lib/config/migration.spec.ts b/lib/config/migration.spec.ts index ba6fff96902167..eb384c3abe331e 100644 --- a/lib/config/migration.spec.ts +++ b/lib/config/migration.spec.ts @@ -53,7 +53,6 @@ describe(getName(__filename), () => { binarySource: 'auto', automergeMinor: true, automergePatch: true, - skipInstalls: true, masterIssue: 'true', masterIssueTitle: 'foo', gomodTidy: true, @@ -97,7 +96,6 @@ describe(getName(__filename), () => { ], peerDependencies: { versionStrategy: 'widen', - skipInstalls: false, }, packageRules: [ { diff --git a/lib/config/migration.ts b/lib/config/migration.ts index aa109e44fe7c6d..559a1d8c1a6a81 100644 --- a/lib/config/migration.ts +++ b/lib/config/migration.ts @@ -172,13 +172,6 @@ export function migrateConfig( migratedConfig[key] = val.replace(/{{depNameShort}}/g, '{{depName}}'); } else if (key === 'gitFs') { delete migratedConfig.gitFs; - } else if (key === 'skipInstalls') { - delete migratedConfig.skipInstalls; - if (val) { - migratedConfig.artifactUpdateApproach = 'shallow'; - } else { - migratedConfig.artifactUpdateApproach = 'deep'; - } } else if (key === 'rebaseStalePrs') { delete migratedConfig.rebaseStalePrs; if (!migratedConfig.rebaseWhen) { diff --git a/lib/manager/npm/extract/__snapshots__/index.spec.ts.snap b/lib/manager/npm/extract/__snapshots__/index.spec.ts.snap index ec3e8ca7139576..4898775c004ac5 100644 --- a/lib/manager/npm/extract/__snapshots__/index.spec.ts.snap +++ b/lib/manager/npm/extract/__snapshots__/index.spec.ts.snap @@ -14,7 +14,6 @@ Object { "lernaClient": undefined, "lernaPackages": undefined, "managerData": Object { - "hasFileRefs": false, "lernaJsonFile": undefined, }, "npmLock": undefined, @@ -23,6 +22,7 @@ Object { "packageJsonName": undefined, "packageJsonType": "app", "pnpmShrinkwrap": undefined, + "skipInstalls": true, "yarnLock": undefined, "yarnWorkspacesPackages": undefined, "yarnrc": undefined, @@ -137,7 +137,6 @@ Object { "lernaClient": undefined, "lernaPackages": undefined, "managerData": Object { - "hasFileRefs": true, "lernaJsonFile": undefined, }, "npmLock": undefined, @@ -146,6 +145,7 @@ Object { "packageJsonName": undefined, "packageJsonType": "library", "pnpmShrinkwrap": undefined, + "skipInstalls": false, "yarnLock": undefined, "yarnWorkspacesPackages": undefined, "yarnrc": undefined, @@ -300,7 +300,6 @@ Object { "lernaClient": undefined, "lernaPackages": undefined, "managerData": Object { - "hasFileRefs": false, "lernaJsonFile": undefined, }, "npmLock": undefined, @@ -309,6 +308,7 @@ Object { "packageJsonName": undefined, "packageJsonType": "app", "pnpmShrinkwrap": undefined, + "skipInstalls": true, "yarnLock": undefined, "yarnWorkspacesPackages": undefined, "yarnrc": undefined, @@ -349,7 +349,6 @@ Object { "lernaClient": undefined, "lernaPackages": undefined, "managerData": Object { - "hasFileRefs": false, "lernaJsonFile": undefined, }, "npmLock": undefined, @@ -358,6 +357,7 @@ Object { "packageJsonName": undefined, "packageJsonType": "app", "pnpmShrinkwrap": undefined, + "skipInstalls": true, "yarnLock": undefined, "yarnWorkspacesPackages": undefined, "yarnrc": undefined, @@ -409,7 +409,6 @@ Object { "lernaClient": undefined, "lernaPackages": undefined, "managerData": Object { - "hasFileRefs": false, "lernaJsonFile": undefined, }, "npmLock": undefined, @@ -418,6 +417,7 @@ Object { "packageJsonName": undefined, "packageJsonType": "library", "pnpmShrinkwrap": undefined, + "skipInstalls": true, "yarnLock": undefined, "yarnWorkspacesPackages": undefined, "yarnrc": undefined, @@ -463,7 +463,6 @@ Object { "lernaClient": undefined, "lernaPackages": undefined, "managerData": Object { - "hasFileRefs": false, "lernaJsonFile": undefined, }, "npmLock": undefined, @@ -472,6 +471,7 @@ Object { "packageJsonName": undefined, "packageJsonType": "library", "pnpmShrinkwrap": undefined, + "skipInstalls": true, "yarnLock": undefined, "yarnWorkspacesPackages": undefined, "yarnrc": undefined, @@ -599,7 +599,6 @@ Object { "lernaClient": "npm", "lernaPackages": undefined, "managerData": Object { - "hasFileRefs": false, "lernaJsonFile": "lerna.json", }, "npmLock": undefined, @@ -608,6 +607,7 @@ Object { "packageJsonName": "renovate", "packageJsonType": "app", "pnpmShrinkwrap": undefined, + "skipInstalls": true, "yarnLock": undefined, "yarnWorkspacesPackages": undefined, "yarnrc": undefined, @@ -735,7 +735,6 @@ Object { "lernaClient": "yarn", "lernaPackages": undefined, "managerData": Object { - "hasFileRefs": false, "lernaJsonFile": "lerna.json", }, "npmLock": undefined, @@ -744,6 +743,7 @@ Object { "packageJsonName": "renovate", "packageJsonType": "app", "pnpmShrinkwrap": undefined, + "skipInstalls": true, "yarnLock": undefined, "yarnWorkspacesPackages": undefined, "yarnrc": undefined, @@ -871,7 +871,6 @@ Object { "lernaClient": undefined, "lernaPackages": undefined, "managerData": Object { - "hasFileRefs": false, "lernaJsonFile": undefined, }, "npmLock": undefined, @@ -880,6 +879,7 @@ Object { "packageJsonName": "renovate", "packageJsonType": "app", "pnpmShrinkwrap": undefined, + "skipInstalls": true, "yarnLock": "yarn.lock", "yarnWorkspacesPackages": undefined, "yarnrc": undefined, @@ -893,7 +893,6 @@ Object { "lernaClient": "npm", "lernaPackages": undefined, "managerData": Object { - "hasFileRefs": false, "lernaJsonFile": "lerna.json", }, "npmLock": undefined, @@ -902,6 +901,7 @@ Object { "packageJsonName": "@a/b", "packageJsonType": "app", "pnpmShrinkwrap": undefined, + "skipInstalls": true, "yarnLock": undefined, "yarnWorkspacesPackages": Array [ "packages/*", @@ -1031,7 +1031,6 @@ Object { "lernaClient": "npm", "lernaPackages": undefined, "managerData": Object { - "hasFileRefs": false, "lernaJsonFile": "lerna.json", }, "npmLock": undefined, @@ -1040,6 +1039,7 @@ Object { "packageJsonName": "renovate", "packageJsonType": "app", "pnpmShrinkwrap": undefined, + "skipInstalls": true, "yarnLock": undefined, "yarnWorkspacesPackages": undefined, "yarnrc": undefined, @@ -1053,7 +1053,6 @@ Object { "lernaClient": "npm", "lernaPackages": undefined, "managerData": Object { - "hasFileRefs": false, "lernaJsonFile": "lerna.json", }, "npmLock": undefined, @@ -1062,6 +1061,7 @@ Object { "packageJsonName": "@a/b", "packageJsonType": "app", "pnpmShrinkwrap": undefined, + "skipInstalls": true, "yarnLock": undefined, "yarnWorkspacesPackages": Array [ "packages/*", @@ -1077,7 +1077,6 @@ Object { "lernaClient": undefined, "lernaPackages": undefined, "managerData": Object { - "hasFileRefs": false, "lernaJsonFile": undefined, }, "npmLock": undefined, @@ -1086,6 +1085,7 @@ Object { "packageJsonName": "@a/b", "packageJsonType": "app", "pnpmShrinkwrap": undefined, + "skipInstalls": true, "yarnLock": undefined, "yarnWorkspacesPackages": Array [ "packages/*", @@ -1215,7 +1215,6 @@ Object { "lernaClient": undefined, "lernaPackages": undefined, "managerData": Object { - "hasFileRefs": false, "lernaJsonFile": undefined, }, "npmLock": undefined, @@ -1224,6 +1223,7 @@ Object { "packageJsonName": "renovate", "packageJsonType": "app", "pnpmShrinkwrap": undefined, + "skipInstalls": true, "yarnLock": undefined, "yarnWorkspacesPackages": undefined, "yarnrc": undefined, diff --git a/lib/manager/npm/extract/index.ts b/lib/manager/npm/extract/index.ts index 7d745f386604d9..c10c120f9a87cd 100644 --- a/lib/manager/npm/extract/index.ts +++ b/lib/manager/npm/extract/index.ts @@ -237,10 +237,6 @@ export async function extractPackageFile( } if (dep.currentValue.startsWith('file:')) { dep.skipReason = SkipReason.File; - // https://github.com/npm/cli/issues/1432 - // Explanation: - // - npm install --package-lock-only is buggy for transitive deps in file: references - // - So we set artifactUpdateApproach to false if file: refs are found *and* the user hasn't explicitly set the value already hasFileRefs = true; return dep; } @@ -347,6 +343,19 @@ export async function extractPackageFile( return null; } } + let skipInstalls = config.skipInstalls; + if (skipInstalls === null) { + if (hasFileRefs) { + // https://github.com/npm/cli/issues/1432 + // Explanation: + // - npm install --package-lock-only is buggy for transitive deps in file: references + // - So we set skipInstalls to false if file: refs are found *and* the user hasn't explicitly set the value already + logger.debug('Automatically setting skipInstalls to false'); + skipInstalls = false; + } else { + skipInstalls = true; + } + } return { deps, @@ -357,11 +366,11 @@ export async function extractPackageFile( yarnrc, ...lockFiles, managerData: { - hasFileRefs, lernaJsonFile, }, lernaClient, lernaPackages, + skipInstalls, yarnWorkspacesPackages, constraints, }; diff --git a/lib/manager/npm/post-update/lerna.spec.ts b/lib/manager/npm/post-update/lerna.spec.ts index 8e771c6eab2bcc..267bef2c31d306 100644 --- a/lib/manager/npm/post-update/lerna.spec.ts +++ b/lib/manager/npm/post-update/lerna.spec.ts @@ -47,26 +47,26 @@ describe(getName(__filename), () => { }); it('generates package-lock.json files', async () => { const execSnapshots = mockExecAll(exec); - const artifactUpdateApproach = 'shallow'; + const skipInstalls = true; const res = await lernaHelper.generateLockFiles( lernaPkgFile('npm'), 'some-dir', {}, {}, - artifactUpdateApproach + skipInstalls ); expect(res.error).toBe(false); expect(execSnapshots).toMatchSnapshot(); }); it('performs full npm install', async () => { const execSnapshots = mockExecAll(exec); - const artifactUpdateApproach = 'deep'; + const skipInstalls = false; const res = await lernaHelper.generateLockFiles( lernaPkgFile('npm'), 'some-dir', {}, {}, - artifactUpdateApproach + skipInstalls ); expect(res.error).toBe(false); expect(execSnapshots).toMatchSnapshot(); diff --git a/lib/manager/npm/post-update/lerna.ts b/lib/manager/npm/post-update/lerna.ts index e66c779dedba42..4cf0bc54750c0c 100644 --- a/lib/manager/npm/post-update/lerna.ts +++ b/lib/manager/npm/post-update/lerna.ts @@ -32,7 +32,7 @@ export async function generateLockFiles( cwd: string, config: PostUpdateConfig, env: NodeJS.ProcessEnv, - artifactUpdateApproach?: string + skipInstalls?: boolean ): Promise { const lernaClient = lernaPackageFile.lernaClient; if (!lernaClient) { @@ -51,7 +51,7 @@ export async function generateLockFiles( installYarn += `@${quote(yarnCompatibility)}`; } preCommands.push(installYarn); - if (artifactUpdateApproach !== 'deep') { + if (skipInstalls !== false) { preCommands.push(getOptimizeCommand()); } cmdOptions = '--ignore-scripts --ignore-engines --ignore-platform'; @@ -63,7 +63,7 @@ export async function generateLockFiles( } preCommands.push(installNpm, 'hash -d npm'); cmdOptions = '--ignore-scripts --no-audit'; - if (artifactUpdateApproach !== 'deep') { + if (skipInstalls !== false) { cmdOptions += ' --package-lock-only'; } } else { diff --git a/lib/manager/npm/post-update/npm.spec.ts b/lib/manager/npm/post-update/npm.spec.ts index 08a02a81165659..b896545b9bde2e 100644 --- a/lib/manager/npm/post-update/npm.spec.ts +++ b/lib/manager/npm/post-update/npm.spec.ts @@ -26,7 +26,7 @@ describe('generateLockFile', () => { it('generates lock files', async () => { const execSnapshots = mockExecAll(exec); fs.readFile = jest.fn(() => 'package-lock-contents') as never; - const artifactUpdateApproach = 'deep'; + const skipInstalls = true; const postUpdateOptions = ['npmDedupe']; const updates = [ { depName: 'some-dep', newVersion: '1.0.1', isLockfileUpdate: false }, @@ -35,7 +35,7 @@ describe('generateLockFile', () => { 'some-dir', {}, 'package-lock.json', - { artifactUpdateApproach, postUpdateOptions }, + { skipInstalls, postUpdateOptions }, updates ); expect(fs.readFile).toHaveBeenCalledTimes(1); @@ -46,7 +46,7 @@ describe('generateLockFile', () => { it('performs lock file updates', async () => { const execSnapshots = mockExecAll(exec); fs.readFile = jest.fn(() => 'package-lock-contents') as never; - const artifactUpdateApproach = 'shallow'; + const skipInstalls = true; const updates = [ { depName: 'some-dep', newVersion: '1.0.1', isLockfileUpdate: true }, ]; @@ -54,7 +54,7 @@ describe('generateLockFile', () => { 'some-dir', {}, 'package-lock.json', - { artifactUpdateApproach }, + { skipInstalls }, updates ); expect(fs.readFile).toHaveBeenCalledTimes(1); @@ -67,12 +67,12 @@ describe('generateLockFile', () => { fs.pathExists.mockResolvedValueOnce(true); fs.move = jest.fn(); fs.readFile = jest.fn(() => 'package-lock-contents') as never; - const artifactUpdateApproach = 'shallow'; + const skipInstalls = true; const res = await npmHelper.generateLockFile( 'some-dir', {}, 'npm-shrinkwrap.json', - { artifactUpdateApproach } + { skipInstalls } ); expect(fs.pathExists).toHaveBeenCalledWith( upath.join('some-dir', 'package-lock.json') @@ -96,12 +96,12 @@ describe('generateLockFile', () => { fs.pathExists.mockResolvedValueOnce(false); fs.move = jest.fn(); fs.readFile = jest.fn((_, _1) => 'package-lock-contents') as never; - const artifactUpdateApproach = 'shallow'; + const skipInstalls = true; const res = await npmHelper.generateLockFile( 'some-dir', {}, 'npm-shrinkwrap.json', - { artifactUpdateApproach } + { skipInstalls } ); expect(fs.pathExists).toHaveBeenCalledWith( upath.join('some-dir', 'package-lock.json') @@ -119,13 +119,13 @@ describe('generateLockFile', () => { it('performs full install', async () => { const execSnapshots = mockExecAll(exec); fs.readFile = jest.fn(() => 'package-lock-contents') as never; - const artifactUpdateApproach = 'deep'; + const skipInstalls = false; const binarySource = BinarySource.Global; const res = await npmHelper.generateLockFile( 'some-dir', {}, 'package-lock.json', - { artifactUpdateApproach, binarySource } + { skipInstalls, binarySource } ); expect(fs.readFile).toHaveBeenCalledTimes(1); expect(res.error).toBeUndefined(); diff --git a/lib/manager/npm/post-update/npm.ts b/lib/manager/npm/post-update/npm.ts index 31e78e3c9595a7..8b28332155c787 100644 --- a/lib/manager/npm/post-update/npm.ts +++ b/lib/manager/npm/post-update/npm.ts @@ -26,7 +26,7 @@ export async function generateLockFile( upgrades: Upgrade[] = [] ): Promise { logger.debug(`Spawning npm install to create ${cwd}/${filename}`); - const { artifactUpdateApproach } = config; + const { skipInstalls, postUpdateOptions } = config; let lockFile = null; try { @@ -49,7 +49,7 @@ export async function generateLockFile( const preCommands = [installNpm, 'hash -d npm']; const commands = []; let cmdOptions = ''; - if (artifactUpdateApproach === 'deep') { + if (postUpdateOptions?.includes('npmDedupe') || skipInstalls === false) { logger.debug('Performing node_modules install'); cmdOptions += '--ignore-scripts --no-audit'; } else { diff --git a/lib/manager/npm/post-update/yarn.ts b/lib/manager/npm/post-update/yarn.ts index d7756d9fc7b64f..f555bcbce78f02 100644 --- a/lib/manager/npm/post-update/yarn.ts +++ b/lib/manager/npm/post-update/yarn.ts @@ -82,7 +82,7 @@ export async function generateLockFile( CI: 'true', }; - if (isYarn1 && config.artifactUpdateApproach !== 'deep') { + if (isYarn1 && config.skipInstalls !== false) { const { offlineMirror, yarnPath } = await checkYarnrc(cwd); if (!offlineMirror) { logger.debug('Updating yarn.lock only - skipping node_modules'); diff --git a/lib/manager/types.ts b/lib/manager/types.ts index d19073f1ff208b..c73d71f4775962 100644 --- a/lib/manager/types.ts +++ b/lib/manager/types.ts @@ -25,6 +25,7 @@ export interface ExtractConfig extends ManagerConfig { aliases?: Record; npmrc?: string; yarnrc?: string; + skipInstalls?: boolean; versioning?: string; updateInternalDeps?: boolean; } @@ -98,6 +99,7 @@ export interface PackageFile> packageJsonType?: 'app' | 'library'; packageFileVersion?: string; parent?: string; + skipInstalls?: boolean; yarnrc?: string; yarnWorkspacesPackages?: string[] | string; matchStrings?: string[]; @@ -274,7 +276,8 @@ export interface PostUpdateConfig extends ManagerConfig, Record { cacheDir?: string; updatedPackageFiles?: File[]; postUpdateOptions?: string[]; - artifactUpdateApproach?: 'auto' | 'deep' | 'shallow'; + skipInstalls?: boolean; + platform?: string; upgrades?: Upgrade[]; npmLock?: string; diff --git a/lib/workers/repository/extract/__snapshots__/manager-files.spec.ts.snap b/lib/workers/repository/extract/__snapshots__/manager-files.spec.ts.snap index 579ce1c3515dc0..52bad56bc98bae 100644 --- a/lib/workers/repository/extract/__snapshots__/manager-files.spec.ts.snap +++ b/lib/workers/repository/extract/__snapshots__/manager-files.spec.ts.snap @@ -17,7 +17,6 @@ Array [ "lernaClient": undefined, "lernaPackages": undefined, "managerData": Object { - "hasFileRefs": false, "lernaJsonFile": undefined, }, "npmLock": undefined, @@ -27,6 +26,7 @@ Array [ "packageJsonName": undefined, "packageJsonType": "app", "pnpmShrinkwrap": undefined, + "skipInstalls": undefined, "yarnLock": undefined, "yarnWorkspacesPackages": undefined, "yarnrc": undefined,