From dda0bade2900ce5c06dc15bbccdd5767b85f2cfa Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@arkins.net>
Date: Wed, 12 Sep 2018 06:30:01 +0200
Subject: [PATCH] refactor: use single localDir per repo (#2505)

---
 lib/manager/npm/post-update/index.js         | 40 +++++++++-----------
 lib/manager/npm/post-update/lerna.js         |  4 +-
 lib/manager/npm/post-update/npm.js           |  8 ++--
 lib/manager/npm/post-update/pnpm.js          |  8 ++--
 lib/manager/npm/post-update/yarn.js          |  8 ++--
 lib/workers/repository/process/write.js      | 35 +++++++----------
 test/platform/git/storage.spec.js            |  8 ++--
 test/workers/branch/lock-files/index.spec.js | 12 +++---
 8 files changed, 54 insertions(+), 69 deletions(-)

diff --git a/lib/manager/npm/post-update/index.js b/lib/manager/npm/post-update/index.js
index f27784d2ed..e97ba1aa37 100644
--- a/lib/manager/npm/post-update/index.js
+++ b/lib/manager/npm/post-update/index.js
@@ -90,22 +90,16 @@ function determineLockFileDirs(config, packageFiles) {
 async function writeExistingFiles(config, packageFiles) {
   const lernaJson = await platform.getFile('lerna.json');
   if (lernaJson) {
-    logger.debug(`Writing repo lerna.json (${config.tmpDir.path})`);
-    await fs.outputFile(
-      upath.join(config.tmpDir.path, 'lerna.json'),
-      lernaJson
-    );
+    logger.debug(`Writing repo lerna.json (${config.localDir})`);
+    await fs.outputFile(upath.join(config.localDir, 'lerna.json'), lernaJson);
   }
   if (config.npmrc) {
-    logger.debug(`Writing repo .npmrc (${config.tmpDir.path})`);
-    await fs.outputFile(upath.join(config.tmpDir.path, '.npmrc'), config.npmrc);
+    logger.debug(`Writing repo .npmrc (${config.localDir})`);
+    await fs.outputFile(upath.join(config.localDir, '.npmrc'), config.npmrc);
   }
   if (config.yarnrc) {
-    logger.debug(`Writing repo .yarnrc (${config.tmpDir.path})`);
-    await fs.outputFile(
-      upath.join(config.tmpDir.path, '.yarnrc'),
-      config.yarnrc
-    );
+    logger.debug(`Writing repo .yarnrc (${config.localDir})`);
+    await fs.outputFile(upath.join(config.localDir, '.yarnrc'), config.yarnrc);
   }
   if (!packageFiles.npm) {
     return;
@@ -117,7 +111,7 @@ async function writeExistingFiles(config, packageFiles) {
   );
   for (const packageFile of npmFiles) {
     const basedir = upath.join(
-      config.tmpDir.path,
+      config.localDir,
       path.dirname(packageFile.packageFile)
     );
     logger.trace(`Writing package.json to ${basedir}`);
@@ -145,14 +139,14 @@ async function writeExistingFiles(config, packageFiles) {
           const resolvedLocalPath = upath.join(
             path.resolve(basedir, localPath)
           );
-          if (!resolvedLocalPath.startsWith(upath.join(config.tmpDir.path))) {
+          if (!resolvedLocalPath.startsWith(upath.join(config.localDir))) {
             logger.info(
               `local lib '${toCopy}' will not be copied because it's out of the repo.`
             );
           } else {
             // at the root of local Lib we should find a package.json so that yarn/npm will use it to update *lock file
             const resolvedRepoPath = upath.join(
-              resolvedLocalPath.substring(config.tmpDir.path.length + 1),
+              resolvedLocalPath.substring(config.localDir.length + 1),
               'package.json'
             );
             const fileContent = await platform.getFile(resolvedRepoPath);
@@ -185,7 +179,7 @@ async function writeExistingFiles(config, packageFiles) {
     }
     const { npmLock } = packageFile;
     if (npmLock) {
-      const npmLockPath = upath.join(config.tmpDir.path, npmLock);
+      const npmLockPath = upath.join(config.localDir, npmLock);
       if (
         process.env.RENOVATE_REUSE_PACKAGE_LOCK === 'false' ||
         config.reuseLockFiles === false
@@ -200,7 +194,7 @@ async function writeExistingFiles(config, packageFiles) {
     }
     const { yarnLock } = packageFile;
     if (yarnLock) {
-      const yarnLockPath = upath.join(config.tmpDir.path, yarnLock);
+      const yarnLockPath = upath.join(config.localDir, yarnLock);
       if (config.reuseLockFiles === false) {
         logger.debug(`Ensuring ${yarnLock} is removed`);
         await fs.remove(yarnLockPath);
@@ -263,7 +257,7 @@ async function writeUpdatedPackageFiles(config) {
     delete massagedFile.engines;
     delete massagedFile.scripts;
     await fs.outputFile(
-      upath.join(config.tmpDir.path, packageFile.name),
+      upath.join(config.localDir, packageFile.name),
       JSON.stringify(massagedFile)
     );
   }
@@ -306,7 +300,7 @@ async function getAdditionalFiles(config, packageFiles) {
     const fileName = path.basename(lockFile);
     logger.debug(`Generating ${fileName} for ${lockFileDir}`);
     const res = await npm.generateLockFile(
-      upath.join(config.tmpDir.path, lockFileDir),
+      upath.join(config.localDir, lockFileDir),
       env,
       fileName,
       config.skipInstalls
@@ -359,7 +353,7 @@ async function getAdditionalFiles(config, packageFiles) {
     logger.debug(`Generating yarn.lock for ${lockFileDir}`);
     const lockFileName = upath.join(lockFileDir, 'yarn.lock');
     const res = await yarn.generateLockFile(
-      upath.join(config.tmpDir.path, lockFileDir),
+      upath.join(config.localDir, lockFileDir),
       env
     );
     if (res.error) {
@@ -411,7 +405,7 @@ async function getAdditionalFiles(config, packageFiles) {
     const lockFileDir = path.dirname(lockFile);
     logger.debug(`Generating shrinkwrap.yaml for ${lockFileDir}`);
     const res = await pnpm.generateLockFile(
-      upath.join(config.tmpDir.path, lockFileDir),
+      upath.join(config.localDir, lockFileDir),
       env
     );
     if (res.error) {
@@ -474,7 +468,7 @@ async function getAdditionalFiles(config, packageFiles) {
     }
     const res = await lerna.generateLockFiles(
       lernaPackageFile.lernaClient,
-      upath.join(config.tmpDir.path, lernaDir),
+      upath.join(config.localDir, lernaDir),
       env,
       config.skipInstalls
     );
@@ -533,7 +527,7 @@ async function getAdditionalFiles(config, packageFiles) {
         );
         if (existingContent) {
           logger.trace('Found lock file');
-          const lockFilePath = upath.join(config.tmpDir.path, filename);
+          const lockFilePath = upath.join(config.localDir, filename);
           logger.trace('Checking against ' + lockFilePath);
           try {
             const newContent = await fs.readFile(lockFilePath, 'utf8');
diff --git a/lib/manager/npm/post-update/lerna.js b/lib/manager/npm/post-update/lerna.js
index 27e4e3fe94..03590c1e35 100644
--- a/lib/manager/npm/post-update/lerna.js
+++ b/lib/manager/npm/post-update/lerna.js
@@ -4,7 +4,7 @@ module.exports = {
   generateLockFiles,
 };
 
-async function generateLockFiles(lernaClient, tmpDir, env, skipInstalls) {
+async function generateLockFiles(lernaClient, cwd, env, skipInstalls) {
   if (!lernaClient) {
     logger.warn('No lernaClient specified - returning');
     return { error: false };
@@ -41,7 +41,7 @@ async function generateLockFiles(lernaClient, tmpDir, env, skipInstalls) {
     logger.debug({ cmd });
     // TODO: Switch to native util.promisify once using only node 8
     ({ stdout, stderr } = await exec(cmd, {
-      cwd: tmpDir,
+      cwd,
       shell: true,
       env,
     }));
diff --git a/lib/manager/npm/post-update/npm.js b/lib/manager/npm/post-update/npm.js
index 7c348a7a74..9559d900f9 100644
--- a/lib/manager/npm/post-update/npm.js
+++ b/lib/manager/npm/post-update/npm.js
@@ -7,8 +7,8 @@ module.exports = {
   generateLockFile,
 };
 
-async function generateLockFile(tmpDir, env, filename, skipInstalls) {
-  logger.debug(`Spawning npm install to create ${tmpDir}/${filename}`);
+async function generateLockFile(cwd, env, filename, skipInstalls) {
+  logger.debug(`Spawning npm install to create ${cwd}/${filename}`);
   let lockFile = null;
   let stdout;
   let stderr;
@@ -61,7 +61,7 @@ async function generateLockFile(tmpDir, env, filename, skipInstalls) {
     logger.debug(`Using npm: ${cmd}`);
     // TODO: Switch to native util.promisify once using only node 8
     ({ stdout, stderr } = await exec(cmd, {
-      cwd: tmpDir,
+      cwd,
       shell: true,
       env,
     }));
@@ -69,7 +69,7 @@ async function generateLockFile(tmpDir, env, filename, skipInstalls) {
     logger.debug(`npm stderr:\n${stderr}`);
     const duration = process.hrtime(startTime);
     const seconds = Math.round(duration[0] + duration[1] / 1e9);
-    lockFile = await fs.readFile(upath.join(tmpDir, filename), 'utf8');
+    lockFile = await fs.readFile(upath.join(cwd, filename), 'utf8');
     logger.info(
       { seconds, type: filename, stdout, stderr },
       'Generated lockfile'
diff --git a/lib/manager/npm/post-update/pnpm.js b/lib/manager/npm/post-update/pnpm.js
index 921e8ef942..ef925c54af 100644
--- a/lib/manager/npm/post-update/pnpm.js
+++ b/lib/manager/npm/post-update/pnpm.js
@@ -7,8 +7,8 @@ module.exports = {
   generateLockFile,
 };
 
-async function generateLockFile(tmpDir, env) {
-  logger.debug(`Spawning pnpm install to create ${tmpDir}/shrinkwrap.yaml`);
+async function generateLockFile(cwd, env) {
+  logger.debug(`Spawning pnpm install to create ${cwd}/shrinkwrap.yaml`);
   let lockFile = null;
   let stdout;
   let stderr;
@@ -59,7 +59,7 @@ async function generateLockFile(tmpDir, env) {
     cmd += ' --ignore-pnpmfile';
     // TODO: Switch to native util.promisify once using only node 8
     ({ stdout, stderr } = await exec(cmd, {
-      cwd: tmpDir,
+      cwd,
       shell: true,
       env,
     }));
@@ -67,7 +67,7 @@ async function generateLockFile(tmpDir, env) {
     logger.debug(`pnpm stderr:\n${stderr}`);
     const duration = process.hrtime(startTime);
     const seconds = Math.round(duration[0] + duration[1] / 1e9);
-    lockFile = await fs.readFile(upath.join(tmpDir, 'shrinkwrap.yaml'), 'utf8');
+    lockFile = await fs.readFile(upath.join(cwd, 'shrinkwrap.yaml'), 'utf8');
     logger.info(
       { seconds, type: 'shrinkwrap.yaml', stdout, stderr },
       'Generated lockfile'
diff --git a/lib/manager/npm/post-update/yarn.js b/lib/manager/npm/post-update/yarn.js
index c04a29cbfb..122d88214c 100644
--- a/lib/manager/npm/post-update/yarn.js
+++ b/lib/manager/npm/post-update/yarn.js
@@ -7,8 +7,8 @@ module.exports = {
   generateLockFile,
 };
 
-async function generateLockFile(tmpDir, env) {
-  logger.debug(`Spawning yarn install to create ${tmpDir}/yarn.lock`);
+async function generateLockFile(cwd, env) {
+  logger.debug(`Spawning yarn install to create ${cwd}/yarn.lock`);
   let lockFile = null;
   let stdout;
   let stderr;
@@ -63,7 +63,7 @@ async function generateLockFile(tmpDir, env) {
 
     // TODO: Switch to native util.promisify once using only node 8
     ({ stdout, stderr } = await exec(cmd, {
-      cwd: tmpDir,
+      cwd,
       shell: true,
       env,
     }));
@@ -71,7 +71,7 @@ async function generateLockFile(tmpDir, env) {
     logger.debug(`yarn stderr:\n${stderr}`);
     const duration = process.hrtime(startTime);
     const seconds = Math.round(duration[0] + duration[1] / 1e9);
-    lockFile = await fs.readFile(upath.join(tmpDir, 'yarn.lock'), 'utf8');
+    lockFile = await fs.readFile(upath.join(cwd, 'yarn.lock'), 'utf8');
     logger.info(
       { seconds, type: 'yarn.lock', stdout, stderr },
       'Generated lockfile'
diff --git a/lib/workers/repository/process/write.js b/lib/workers/repository/process/write.js
index e1e397c460..9cbaa4cbcf 100644
--- a/lib/workers/repository/process/write.js
+++ b/lib/workers/repository/process/write.js
@@ -1,5 +1,3 @@
-const tmp = require('tmp-promise');
-
 const branchWorker = require('../../branch');
 const { getPrsRemaining } = require('./limits');
 
@@ -22,27 +20,20 @@ async function writeUpdates(config, packageFiles, allBranches) {
       return true;
     });
   }
-  const tmpDir = await tmp.dir({ unsafeCleanup: true });
   let prsRemaining = await getPrsRemaining(config, branches);
-  try {
-    // eslint-disable-next-line no-param-reassign
-    for (const branch of branches) {
-      const res = await branchWorker.processBranch(
-        {
-          ...branch,
-          tmpDir,
-          prHourlyLimitReached: prsRemaining <= 0,
-        },
-        packageFiles
-      );
-      if (res === 'pr-closed' || res === 'automerged') {
-        // Stop procesing other branches because base branch has been changed
-        return res;
-      }
-      prsRemaining -= res === 'pr-created' ? 1 : 0;
+  for (const branch of branches) {
+    const res = await branchWorker.processBranch(
+      {
+        ...branch,
+        prHourlyLimitReached: prsRemaining <= 0,
+      },
+      packageFiles
+    );
+    if (res === 'pr-closed' || res === 'automerged') {
+      // Stop procesing other branches because base branch has been changed
+      return res;
     }
-    return 'done';
-  } finally {
-    tmpDir.cleanup();
+    prsRemaining -= res === 'pr-created' ? 1 : 0;
   }
+  return 'done';
 }
diff --git a/test/platform/git/storage.spec.js b/test/platform/git/storage.spec.js
index a73aa5fbb0..fcca6638d9 100644
--- a/test/platform/git/storage.spec.js
+++ b/test/platform/git/storage.spec.js
@@ -36,15 +36,15 @@ describe('platform/git/storage', () => {
     await repo.checkout('master');
   });
 
-  let localDir;
+  let tmpDir;
 
   beforeEach(async () => {
     origin = await tmp.dir({ unsafeCleanup: true });
     const repo = Git(origin.path);
     await repo.clone(base.path, '.', ['--bare']);
-    localDir = await tmp.dir({ unsafeCleanup: true });
+    tmpDir = await tmp.dir({ unsafeCleanup: true });
     await git.initRepo({
-      localDir: localDir.path,
+      localDir: tmpDir.path,
       platform: 'github',
       repository: 'owner/repo-name',
       url: origin.path,
@@ -56,7 +56,7 @@ describe('platform/git/storage', () => {
   });
 
   afterEach(() => {
-    localDir.cleanup();
+    tmpDir.cleanup();
     origin.cleanup();
     git.cleanRepo();
   });
diff --git a/test/workers/branch/lock-files/index.spec.js b/test/workers/branch/lock-files/index.spec.js
index ad3f12ab0d..6767c1fdaa 100644
--- a/test/workers/branch/lock-files/index.spec.js
+++ b/test/workers/branch/lock-files/index.spec.js
@@ -128,7 +128,7 @@ describe('manager/npm/post-update', () => {
     beforeEach(() => {
       config = {
         ...defaultConfig,
-        tmpDir: { path: 'some-tmp-dir' },
+        localDir: 'some-tmp-dir',
       };
       fs.outputFile = jest.fn();
       fs.remove = jest.fn();
@@ -167,7 +167,7 @@ describe('manager/npm/post-update', () => {
     });
     it('writes package.json of local lib', async () => {
       const renoPath = upath.join(__dirname, '../../../');
-      config.tmpDir = { path: renoPath };
+      config.localDir = renoPath;
       const packageFiles = {
         npm: [
           {
@@ -191,7 +191,7 @@ describe('manager/npm/post-update', () => {
     });
     it('Try to write package.json of local lib, but file not found', async () => {
       const renoPath = upath.join(__dirname, '../../../');
-      config.tmpDir = { path: renoPath };
+      config.localDir = renoPath;
       const packageFiles = {
         npm: [
           {
@@ -215,7 +215,7 @@ describe('manager/npm/post-update', () => {
     });
     it('detect malicious intent (error config in package.json) local lib is not in the repo', async () => {
       const renoPath = upath.join(__dirname, '../../../');
-      config.tmpDir = { path: renoPath };
+      config.localDir = renoPath;
       const packageFiles = {
         npm: [
           {
@@ -244,7 +244,7 @@ describe('manager/npm/post-update', () => {
     beforeEach(() => {
       config = {
         ...defaultConfig,
-        tmpDir: { path: 'some-tmp-dir' },
+        localDir: 'some-tmp-dir',
       };
       fs.outputFile = jest.fn();
     });
@@ -285,7 +285,7 @@ describe('manager/npm/post-update', () => {
     beforeEach(() => {
       config = {
         ...defaultConfig,
-        tmpDir: { path: 'some-tmp-dir' },
+        localDir: 'some-tmp-dir',
       };
       platform.getFile.mockReturnValue('some lock file contents');
       npm.generateLockFile = jest.fn();
-- 
GitLab