refactor: skipInstalls -> artifactUpdateApproach (#9119)

Renames skipInstalls to artifactUpdateApproach and changes from boolean to enum. BREAKING CHANGE: skipInstalls has been changed to artifactsUpdateApproach
renovatebot · Apr 22, 2021 · 59d728b · 59d728b
1 parent 6e40bae
commit 59d728b
Show file tree

Hide file tree

Showing 14 changed files with 66 additions and 67 deletions.
diff --git 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 `skipInstalls` admin option).
+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).
 
 ```json
 {
@@ -69,6 +69,12 @@ 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.
@@ -377,12 +383,6 @@ 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
@@ -80,6 +80,7 @@ Object {
   "additionalBranchPrefix": "{{parentDir}}-",
   "allowCustomCrateRegistries": true,
   "allowScripts": true,
+  "artifactUpdateApproach": "shallow",
   "autodiscover": true,
   "automerge": false,
   "automergeType": "branch",
@@ -173,6 +174,7 @@ Object {
       "versioning": "maven",
     },
     Object {
+      "artifactUpdateApproach": "deep",
       "matchDepTypes": Array [
         "peerDependencies",
       ],

diff --git a/lib/config/definitions.ts b/lib/config/definitions.ts
@@ -564,11 +564,12 @@ const options: RenovateOptions[] = [
     type: 'boolean',
   },
   {
-    name: 'skipInstalls',
+    name: 'artifactUpdateApproach',
     description:
-      'Skip installing modules/dependencies if lock file updating is possible alone.',
-    type: 'boolean',
-    default: null,
+      'Whether to employ a deep or shallow approach to artifact updating.',
+    type: 'string',
+    allowedValues: ['auto', 'deep', 'shallow'],
+    default: 'auto',
     admin: true,
   },
   {

diff --git a/lib/config/migration.spec.ts b/lib/config/migration.spec.ts
@@ -52,6 +52,7 @@ describe(getName(__filename), () => {
         binarySource: 'auto',
         automergeMinor: true,
         automergePatch: true,
+        skipInstalls: true,
         masterIssue: 'true',
         masterIssueTitle: 'foo',
         gomodTidy: true,
@@ -95,6 +96,7 @@ describe(getName(__filename), () => {
         ],
         peerDependencies: {
           versionStrategy: 'widen',
+          skipInstalls: false,
         },
         packageRules: [
           {

diff --git a/lib/config/migration.ts b/lib/config/migration.ts
@@ -172,6 +172,13 @@ 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
@@ -15,6 +15,7 @@ Object {
   "lernaClient": undefined,
   "lernaPackages": undefined,
   "managerData": Object {
+    "hasFileRefs": false,
     "lernaJsonFile": undefined,
   },
   "npmLock": undefined,
@@ -23,7 +24,6 @@ Object {
   "packageJsonName": undefined,
   "packageJsonType": "app",
   "pnpmShrinkwrap": undefined,
-  "skipInstalls": true,
   "yarnLock": undefined,
   "yarnWorkspacesPackages": undefined,
   "yarnrc": undefined,
@@ -139,6 +139,7 @@ Object {
   "lernaClient": undefined,
   "lernaPackages": undefined,
   "managerData": Object {
+    "hasFileRefs": true,
     "lernaJsonFile": undefined,
   },
   "npmLock": undefined,
@@ -147,7 +148,6 @@ Object {
   "packageJsonName": undefined,
   "packageJsonType": "library",
   "pnpmShrinkwrap": undefined,
-  "skipInstalls": false,
   "yarnLock": undefined,
   "yarnWorkspacesPackages": undefined,
   "yarnrc": undefined,
@@ -303,6 +303,7 @@ Object {
   "lernaClient": undefined,
   "lernaPackages": undefined,
   "managerData": Object {
+    "hasFileRefs": false,
     "lernaJsonFile": undefined,
   },
   "npmLock": undefined,
@@ -311,7 +312,6 @@ Object {
   "packageJsonName": undefined,
   "packageJsonType": "app",
   "pnpmShrinkwrap": undefined,
-  "skipInstalls": true,
   "yarnLock": undefined,
   "yarnWorkspacesPackages": undefined,
   "yarnrc": undefined,
@@ -353,6 +353,7 @@ Object {
   "lernaClient": undefined,
   "lernaPackages": undefined,
   "managerData": Object {
+    "hasFileRefs": false,
     "lernaJsonFile": undefined,
   },
   "npmLock": undefined,
@@ -361,7 +362,6 @@ Object {
   "packageJsonName": undefined,
   "packageJsonType": "app",
   "pnpmShrinkwrap": undefined,
-  "skipInstalls": false,
   "yarnLock": undefined,
   "yarnWorkspacesPackages": undefined,
   "yarnrc": undefined,
@@ -414,6 +414,7 @@ Object {
   "lernaClient": undefined,
   "lernaPackages": undefined,
   "managerData": Object {
+    "hasFileRefs": false,
     "lernaJsonFile": undefined,
   },
   "npmLock": undefined,
@@ -422,7 +423,6 @@ Object {
   "packageJsonName": undefined,
   "packageJsonType": "library",
   "pnpmShrinkwrap": undefined,
-  "skipInstalls": true,
   "yarnLock": undefined,
   "yarnWorkspacesPackages": undefined,
   "yarnrc": undefined,
@@ -469,6 +469,7 @@ Object {
   "lernaClient": undefined,
   "lernaPackages": undefined,
   "managerData": Object {
+    "hasFileRefs": false,
     "lernaJsonFile": undefined,
   },
   "npmLock": undefined,
@@ -477,7 +478,6 @@ Object {
   "packageJsonName": undefined,
   "packageJsonType": "library",
   "pnpmShrinkwrap": undefined,
-  "skipInstalls": true,
   "yarnLock": undefined,
   "yarnWorkspacesPackages": undefined,
   "yarnrc": undefined,
@@ -606,6 +606,7 @@ Object {
   "lernaClient": "npm",
   "lernaPackages": undefined,
   "managerData": Object {
+    "hasFileRefs": false,
     "lernaJsonFile": "lerna.json",
   },
   "npmLock": undefined,
@@ -614,7 +615,6 @@ Object {
   "packageJsonName": "renovate",
   "packageJsonType": "app",
   "pnpmShrinkwrap": undefined,
-  "skipInstalls": true,
   "yarnLock": undefined,
   "yarnWorkspacesPackages": undefined,
   "yarnrc": undefined,
@@ -743,6 +743,7 @@ Object {
   "lernaClient": "yarn",
   "lernaPackages": undefined,
   "managerData": Object {
+    "hasFileRefs": false,
     "lernaJsonFile": "lerna.json",
   },
   "npmLock": undefined,
@@ -751,7 +752,6 @@ Object {
   "packageJsonName": "renovate",
   "packageJsonType": "app",
   "pnpmShrinkwrap": undefined,
-  "skipInstalls": true,
   "yarnLock": undefined,
   "yarnWorkspacesPackages": undefined,
   "yarnrc": undefined,
@@ -880,6 +880,7 @@ Object {
   "lernaClient": undefined,
   "lernaPackages": undefined,
   "managerData": Object {
+    "hasFileRefs": false,
     "lernaJsonFile": undefined,
   },
   "npmLock": undefined,
@@ -888,7 +889,6 @@ Object {
   "packageJsonName": "renovate",
   "packageJsonType": "app",
   "pnpmShrinkwrap": undefined,
-  "skipInstalls": true,
   "yarnLock": "yarn.lock",
   "yarnWorkspacesPackages": undefined,
   "yarnrc": undefined,
@@ -903,6 +903,7 @@ Object {
   "lernaClient": "npm",
   "lernaPackages": undefined,
   "managerData": Object {
+    "hasFileRefs": false,
     "lernaJsonFile": "lerna.json",
   },
   "npmLock": undefined,
@@ -911,7 +912,6 @@ Object {
   "packageJsonName": "@a/b",
   "packageJsonType": "app",
   "pnpmShrinkwrap": undefined,
-  "skipInstalls": true,
   "yarnLock": undefined,
   "yarnWorkspacesPackages": Array [
     "packages/*",
@@ -1042,6 +1042,7 @@ Object {
   "lernaClient": "npm",
   "lernaPackages": undefined,
   "managerData": Object {
+    "hasFileRefs": false,
     "lernaJsonFile": "lerna.json",
   },
   "npmLock": undefined,
@@ -1050,7 +1051,6 @@ Object {
   "packageJsonName": "renovate",
   "packageJsonType": "app",
   "pnpmShrinkwrap": undefined,
-  "skipInstalls": true,
   "yarnLock": undefined,
   "yarnWorkspacesPackages": undefined,
   "yarnrc": undefined,
@@ -1065,6 +1065,7 @@ Object {
   "lernaClient": "npm",
   "lernaPackages": undefined,
   "managerData": Object {
+    "hasFileRefs": false,
     "lernaJsonFile": "lerna.json",
   },
   "npmLock": undefined,
@@ -1073,7 +1074,6 @@ Object {
   "packageJsonName": "@a/b",
   "packageJsonType": "app",
   "pnpmShrinkwrap": undefined,
-  "skipInstalls": true,
   "yarnLock": undefined,
   "yarnWorkspacesPackages": Array [
     "packages/*",
@@ -1090,6 +1090,7 @@ Object {
   "lernaClient": undefined,
   "lernaPackages": undefined,
   "managerData": Object {
+    "hasFileRefs": false,
     "lernaJsonFile": undefined,
   },
   "npmLock": undefined,
@@ -1098,7 +1099,6 @@ Object {
   "packageJsonName": "@a/b",
   "packageJsonType": "app",
   "pnpmShrinkwrap": undefined,
-  "skipInstalls": true,
   "yarnLock": undefined,
   "yarnWorkspacesPackages": Array [
     "packages/*",
@@ -1229,6 +1229,7 @@ Object {
   "lernaClient": undefined,
   "lernaPackages": undefined,
   "managerData": Object {
+    "hasFileRefs": false,
     "lernaJsonFile": undefined,
   },
   "npmLock": undefined,
@@ -1237,7 +1238,6 @@ 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
@@ -126,7 +126,7 @@ export async function extractPackageFile(
   let lernaJsonFile: string;
   let lernaPackages: string[];
   let lernaClient: 'yarn' | 'npm';
-  let hasFancyRefs = false;
+  let hasFileRefs = false;
   let lernaJson: {
     packages: string[];
     npmClient: string;
@@ -224,7 +224,6 @@ export async function extractPackageFile(
 
     if (dep.currentValue.startsWith('npm:')) {
       dep.npmPackageAlias = true;
-      hasFancyRefs = true;
       const valSplit = dep.currentValue.replace('npm:', '').split('@');
       if (valSplit.length === 2) {
         dep.lookupName = valSplit[0];
@@ -238,7 +237,11 @@ export async function extractPackageFile(
     }
     if (dep.currentValue.startsWith('file:')) {
       dep.skipReason = SkipReason.File;
-      hasFancyRefs = true;
+      // 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;
     }
     if (isValid(dep.currentValue)) {
@@ -344,19 +347,6 @@ export async function extractPackageFile(
       return null;
     }
   }
-  let skipInstalls = config.skipInstalls;
-  if (skipInstalls === null) {
-    if (hasFancyRefs) {
-      // https://github.com/npm/cli/issues/1432
-      // Explanation:
-      //  - npm install --package-lock-only is buggy for transitive deps in file: and npm: references
-      //  - So we set skipInstalls to false if file: or npm: 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,
@@ -368,11 +358,11 @@ export async function extractPackageFile(
     yarnrc,
     ...lockFiles,
     managerData: {
+      hasFileRefs,
       lernaJsonFile,
     },
     lernaClient,
     lernaPackages,
-    skipInstalls,
     yarnWorkspacesPackages,
     constraints,
   };