Unify the way tmp dir is used (#6052)

Instead of having several globals, set via environment variables,
explicitly set the "correct" env var if passed `--tmpDir` and then
consistently use it in the rest of the program.

See @apmorton's comments in #1707

app.ts

@@ -150,12 +150,17 @@ if (process.platform === 'linux' && child_process.execSync('uname -a').toString(
     process.env.wsl = 'true';
 }
 
-// AP: Allow setting of tmpDir (used in lib/base-compiler.js & lib/exec.js) through opts.
-// WSL requires a directory on a Windows volume. Set that to Windows %TEMP% if no tmpDir supplied.
+// Allow setting of the temporary directory (that which `os.tmpdir()` returns).
+// WSL requires a directory on a Windows volume. Set that to Windows %TEMP% if no -tmpDir supplied.
 // If a tempDir is supplied then assume that it will work for WSL processes as well.
 if (opts.tmpDir) {
-    process.env.tmpDir = opts.tmpDir;
-    process.env.winTmp = opts.tmpDir;
+    if (process.env.wsl) {
+        process.env.TEMP = opts.tmpDir; // for Windows
+    } else {
+        process.env.TMP = opts.tmpDir; // for Linux
+    }
+    if (os.tmpdir() !== opts.tmpDir)
+        throw new Error(`Unable to set the temporary dir to ${opts.tmpDir} - stuck at  ${os.tmpdir()}`);
 } else if (process.env.wsl) {
     // Dec 2017 preview builds of WSL include /bin/wslpath; do the parsing work for now.
     // Parsing example %TEMP% is C:\Users\apardoe\AppData\Local\Temp
@@ -163,11 +168,12 @@ if (opts.tmpDir) {
         const windowsTemp = child_process.execSync('cmd.exe /c echo %TEMP%').toString().replaceAll('\\', '/');
         const driveLetter = windowsTemp.substring(0, 1).toLowerCase();
         const directoryPath = windowsTemp.substring(2).trim();
-        process.env.winTmp = path.join('/mnt', driveLetter, directoryPath);
+        process.env.TEMP = path.join('/mnt', driveLetter, directoryPath);
     } catch (e) {
         logger.warn('Unable to invoke cmd.exe to get windows %TEMP% path.');
     }
 }
+logger.info(`Using temporary dir: ${os.tmpdir()}`);
 
 const distPath = utils.resolvePathFromAppRoot('.');
 logger.debug(`Distpath=${distPath}`);

docs/WindowsSubsystemForLinux.md

@@ -67,20 +67,15 @@ CE only required a few changes in order to run properly under WSL. Those changes
 - `app.ts`:
   - `process.env.wsl` is set if CE if the string "Microsoft" in found in the output of `uname -a`. This works for all
     WSL distros as they all run on the base Microsoft Linux kernel.
-  - If the `-tmpDir` option is specified on the command line, both `process.env.tmpDir` and `process.env.winTmp` are set
-    to the specified value Note that if this is specified as a non-Windows volume, Windows executables will fail to run
-    properly. Otherwise, `process.env.winTmp` is set to the value of the Windows `%TEMP%` directory if CE can get the
-    temp path from invoking `cmd.exe` from WSL.
-- `lib/exec.ts`: Execute the compiler in the temporary directory. If the compiler's binary is located on a mounted
-  volume (`startsWith("/mnt"`)) and CE is running under WSL, run the compiler in the `winTmp` directory. Otherwise, use
-  the Linux temp directory.
+  - If the `-tmpDir` option is specified on the command line, os.tmpdir()'s return value is set to the specified value.
+    Note that if this is specified as a non-Windows volume, Windows executables will fail to run properly. Otherwise,
+    os.tmpdir() is set to the value of the Windows `%TEMP%` directory if CE can get the temp path from invoking
+    `cmd.exe` from WSL.
+- `lib/exec.ts`: Execute the compiler in the temporary directory.
 - `lib/compilers/wsl-vc.ts`: See also `wine-vc.ts`, the Wine version of this compiler-specific file. These files provide
   custom behaviors for a compiler. This file does two interesting things:
   - The `CompileCl` function translates from Linux-style directories to Windows-style directories (`/mnt/c/tmp` to
     `c:/tmp`) so that `CL.exe` can find its input files.
-  - The `newTempDir` function creates a temporary directory in `winTmp`. CEs creates directories under the temp
-    directory that start with `compiler-explorer-compiler` where the compiler and compiler output lives. This is similar
-    to the function in `lib/base-compiler.ts`.
 - `etc/config/c++.defaults.properties`: Add a configuration (`&cl19`) for MSVC compilers. This edits in here are
   currently wrong in two ways, but it doesn't affect the main CE instance as it uses `amazon` properties files, and it
   doesn't affect anyone running a local copy of CE because CE will just fail silently when it can't find a compiler.

lib/base-compiler.ts

@@ -115,6 +115,7 @@ import {HeaptrackWrapper} from './runtime-tools/heaptrack-wrapper.js';
 import {propsFor} from './properties.js';
 import stream from 'node:stream';
 import {SentryCapture} from './sentry.js';
+import os from 'os';
 
 const compilationTimeHistogram = new PromClient.Histogram({
     name: 'ce_base_compiler_compilation_duration_seconds',
@@ -381,13 +382,10 @@ export class BaseCompiler implements ICompiler {
         return env;
     }
 
-    newTempDir(): Promise<string> {
-        return new Promise((resolve, reject) => {
-            temp.mkdir({prefix: 'compiler-explorer-compiler', dir: process.env.tmpDir}, (err, dirPath) => {
-                if (err) reject(`Unable to open temp file: ${err}`);
-                else resolve(dirPath);
-            });
-        });
+    async newTempDir(): Promise<string> {
+        // `temp` caches the os tmp dir on import (which we may change), so here we ensure we use the current os.tmpdir
+        // each time.
+        return await temp.mkdir({prefix: 'compiler-explorer-compiler', dir: os.tmpdir()});
     }
 
     optOutputRequested(options: string[]) {

lib/compilers/ewarm.ts

@@ -22,8 +22,6 @@
 // ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 // POSSIBILITY OF SUCH DAMAGE.
 
-import temp from 'temp';
-
 import type {PreliminaryCompilerInfo} from '../../types/compiler.interfaces.js';
 import type {ParseFiltersAndOutputOptions} from '../../types/features/filters.interfaces.js';
 import {BaseCompiler} from '../base-compiler.js';
@@ -39,15 +37,6 @@ export class EWARMCompiler extends BaseCompiler {
         this.asm = new AsmEWAVRParser(this.compilerProps);
     }
 
-    override newTempDir() {
-        return new Promise<string>((resolve, reject) => {
-            temp.mkdir({prefix: 'compiler-explorer-compiler', dir: process.env.TMP}, (err, dirPath) => {
-                if (err) reject(`Unable to open temp file: ${err}`);
-                else resolve(dirPath);
-            });
-        });
-    }
-
     override optionsForFilter(filters: ParseFiltersAndOutputOptions, outputFilename: string) {
         if (filters.binary) {
             return [];

lib/compilers/ewavr.ts

@@ -22,8 +22,6 @@
 // ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 // POSSIBILITY OF SUCH DAMAGE.
 
-import temp from 'temp';
-
 import type {PreliminaryCompilerInfo} from '../../types/compiler.interfaces.js';
 import type {ParseFiltersAndOutputOptions} from '../../types/features/filters.interfaces.js';
 import {BaseCompiler} from '../base-compiler.js';
@@ -41,15 +39,6 @@ export class EWAVRCompiler extends BaseCompiler {
         this.asm = new AsmEWAVRParser(this.compilerProps);
     }
 
-    override newTempDir() {
-        return new Promise<string>((resolve, reject) => {
-            temp.mkdir({prefix: 'compiler-explorer-compiler', dir: process.env.TMP}, (err, dirPath) => {
-                if (err) reject(`Unable to open temp file: ${err}`);
-                else resolve(dirPath);
-            });
-        });
-    }
-
     override optionsForFilter(filters: ParseFiltersAndOutputOptions, outputFilename: string) {
         if (filters.binary) {
             return [];

lib/compilers/win32.ts

@@ -24,7 +24,6 @@
 
 import path from 'path';
 
-import temp from 'temp';
 import _ from 'underscore';
 
 import type {ExecutionOptions} from '../../types/compilation/compilation.interfaces.js';
@@ -55,15 +54,6 @@ export class Win32Compiler extends BaseCompiler {
         return ['/std:<value>'];
     }
 
-    override newTempDir() {
-        return new Promise<string>((resolve, reject) => {
-            temp.mkdir({prefix: 'compiler-explorer-compiler', dir: process.env.TMP}, (err, dirPath) => {
-                if (err) reject(`Unable to open temp file: ${err}`);
-                else resolve(dirPath);
-            });
-        });
-    }
-
     override getExecutableFilename(dirPath: string, outputFilebase: string, key?) {
         return this.getOutputFilename(dirPath, outputFilebase, key) + '.exe';
     }

lib/compilers/wsl-vc.ts

@@ -27,14 +27,13 @@
 
 import path from 'path';
 
-import temp from 'temp';
-
 import type {ExecutionOptions} from '../../types/compilation/compilation.interfaces.js';
 import type {PreliminaryCompilerInfo} from '../../types/compiler.interfaces.js';
 import {unwrap} from '../assert.js';
 import {VcAsmParser} from '../parsers/asm-parser-vc.js';
 
 import {Win32VcCompiler} from './win32-vc.js';
+import os from 'os';
 
 export class WslVcCompiler extends Win32VcCompiler {
     static override get key() {
@@ -46,25 +45,15 @@ export class WslVcCompiler extends Win32VcCompiler {
         this.asm = new VcAsmParser();
     }
 
-    override filename(fn: string) {
+    override filename(fn: string, tmpDirForTest?: string) {
         // AP: Need to translate compiler paths from what the Node.js process sees
         // on a Unix mounted volume (/mnt/c/tmp) to what CL sees on Windows (c:/tmp)
-        // We know process.env.tmpDir is of format /mnt/X/dir where X is drive letter.
-        const driveLetter = unwrap(process.env.winTmp).substring(5, 6);
-        const directoryPath = unwrap(process.env.winTmp).substring(7);
+        // We know os.tmpdir() is of format /mnt/X/dir where X is drive letter.
+        const tmpdir = tmpDirForTest || os.tmpdir();
+        const driveLetter = unwrap(tmpdir).substring(5, 6);
+        const directoryPath = unwrap(tmpdir).substring(7);
         const windowsStyle = driveLetter.concat(':/', directoryPath);
-        return fn.replace(unwrap(process.env.winTmp), windowsStyle);
-    }
-
-    // AP: Create CE temp directory in winTmp directory instead of the tmpDir directory.
-    // NPM temp package: https://www.npmjs.com/package/temp, see Affixes
-    override newTempDir() {
-        return new Promise<string>((resolve, reject) => {
-            temp.mkdir({prefix: 'compiler-explorer-compiler', dir: unwrap(process.env.winTmp)}, (err, dirPath) => {
-                if (err) reject(`Unable to open temp file: ${err}`);
-                else resolve(dirPath);
-            });
-        });
+        return fn.replace(unwrap(tmpdir), windowsStyle);
     }
 
     override exec(compiler: string, args: string[], options_: ExecutionOptions) {

lib/exec.ts

@@ -37,6 +37,7 @@ import {logger} from './logger.js';
 import {propsFor} from './properties.js';
 import {Graceful} from './node-graceful.js';
 import {unwrapString} from './assert.js';
+import os from 'os';
 
 type NsJailOptions = {
     args: string[];
@@ -78,14 +79,11 @@ export function executeDirect(
 
     let okToCache = true;
     let timedOut = false;
-    const cwd =
-        options.customCwd ||
-        (command.startsWith('/mnt') && process.env.wsl && process.env.winTmp ? process.env.winTmp : process.env.tmpDir);
+    // In WSL; run Windows-volume executables in a temp directory.
+    const cwd = options.customCwd || (command.startsWith('/mnt') && process.env.wsl ? os.tmpdir() : undefined);
     logger.debug('Execution', {type: 'executing', command: command, args: args, env: env, cwd: cwd});
     const startTime = process.hrtime.bigint();
 
-    // AP: Run Windows-volume executables in winTmp. Otherwise, run in tmpDir (which may be undefined).
-    // https://nodejs.org/api/child_process.html#child_process_child_process_spawn_command_args_options
     const child = child_process.spawn(command, args, {
         cwd: cwd,
         env: env,
@@ -108,7 +106,7 @@ export function executeDirect(
         stdout: '',
         truncated: false,
     };
-    let timeout;
+    let timeout: NodeJS.Timeout | undefined;
     if (timeoutMs)
         timeout = setTimeout(() => {
             logger.warn(`Timeout for ${command} ${args} after ${timeoutMs}ms`);

test/cache-tests.ts

@@ -27,7 +27,6 @@ import {Readable} from 'stream';
 import {GetObjectCommand, NoSuchKey, PutObjectCommand, S3} from '@aws-sdk/client-s3';
 import {sdkStreamMixin} from '@smithy/util-stream';
 import {AwsClientStub, mockClient} from 'aws-sdk-client-mock';
-import temp from 'temp';
 
 import {BaseCache} from '../lib/cache/base.js';
 import {createCacheFromConfig} from '../lib/cache/from-config.js';
@@ -37,12 +36,7 @@ import {NullCache} from '../lib/cache/null.js';
 import {OnDiskCache} from '../lib/cache/on-disk.js';
 import {S3Cache} from '../lib/cache/s3.js';
 
-import {fs, path, shouldExist} from './utils.js';
-
-function newTempDir() {
-    temp.track(true);
-    return temp.mkdirSync({prefix: 'compiler-explorer-cache-tests', dir: process.env.tmpDir});
-}
+import {fs, newTempDir, path, shouldExist} from './utils.js';
 
 function basicTests(factory: () => BaseCache) {
     it('should start empty', () => {

test/packager-tests.ts

@@ -22,20 +22,9 @@
 // ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 // POSSIBILITY OF SUCH DAMAGE.
 
-import temp from 'temp';
-
 import {Packager} from '../lib/packager.js';
 
-import {fs, path} from './utils.js';
-
-function newTempDir(): Promise<string> {
-    return new Promise((resolve, reject) => {
-        temp.mkdir({prefix: 'compiler-explorer-compiler', dir: process.env.tmpDir}, (err, dirPath) => {
-            if (err) reject(`Unable to open temp file: ${err}`);
-            else resolve(dirPath);
-        });
-    });
-}
+import {fs, newTempDir, path} from './utils.js';
 
 function writeTestFile(filepath) {
     return fs.writeFile(filepath, '#!/bin/sh\n\necho Hello, world!\n\n');
@@ -45,7 +34,7 @@ describe('Packager', function () {
     it('should be able to package 1 file', async () => {
         const pack = new Packager();
 
-        const dirPath = await newTempDir();
+        const dirPath = newTempDir();
         await writeTestFile(path.join(dirPath, 'hello.txt'));
 
         const targzPath = path.join(dirPath, 'package.tgz');
@@ -57,13 +46,13 @@ describe('Packager', function () {
     it('should be able to unpack', async () => {
         const pack = new Packager();
 
-        const dirPath = await newTempDir();
+        const dirPath = newTempDir();
         await writeTestFile(path.join(dirPath, 'hello.txt'));
 
         const targzPath = path.join(dirPath, 'package.tgz');
         await pack.package(dirPath, targzPath);
 
-        const unpackPath = await newTempDir();
+        const unpackPath = newTempDir();
         const pack2 = new Packager();
         await pack2.unpack(targzPath, unpackPath);
 

test/utils.ts

@@ -22,11 +22,13 @@
 // ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 // POSSIBILITY OF SUCH DAMAGE.
 
+import os from 'os';
 import path from 'path';
 import {fileURLToPath} from 'url';
 
 import chai from 'chai';
 import fs from 'fs-extra';
+import temp from 'temp';
 
 import {CompilationEnvironment} from '../lib/compilation-env.js';
 import {CompilationQueue} from '../lib/compilation-queue.js';
@@ -77,5 +79,11 @@ export function resolvePathFromTestRoot(...args: string[]): string {
     return path.resolve(TEST_ROOT, ...args);
 }
 
+// Tracked temporary directories.
+export function newTempDir() {
+    temp.track(true);
+    return temp.mkdirSync({prefix: 'compiler-explorer-tests', dir: os.tmpdir()});
+}
+
 // eslint-disable-next-line -- do not rewrite exports
 export {chai, path, fs};

test/win-path-tests.ts

@@ -57,10 +57,8 @@ describe('Paths', () => {
     });
 
     it('Linux -> Windows path', function () {
-        process.env.winTmp = '/mnt/c/tmp';
-
         const compiler = new WslVcCompiler(makeFakeCompilerInfo(info), env);
-        compiler.filename('/mnt/c/tmp/123456/output.s').should.equal('c:/tmp/123456/output.s');
+        compiler.filename('/mnt/c/tmp/123456/output.s', '/mnt/c/tmp').should.equal('c:/tmp/123456/output.s');
     });
 });