Skip to content

Commit 0a4a949

Browse files
rhysdrhysd
committed
remove temporary directories after installation
because temporary directories can cause issues when this action is used multiple times. A temporary directory created by previous action run can cause a conflict with a subsequent action run.
1 parent cfb29ff commit 0a4a949

File tree

4 files changed

+138
-82
lines changed

4 files changed

+138
-82
lines changed

‎src/neovim.ts‎

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import fetch from 'node-fetch';
66
import * as core from '@actions/core';
77
import * as io from '@actions/io';
88
import * as github from '@actions/github';
9-
import { makeTmpdir, type Os, type Arch, ensureError, getSystemHttpsProxyAgent } from './system.js';
9+
import { TmpDir, type Os, type Arch, ensureError, getSystemHttpsProxyAgent } from './system.js';
1010
import { exec, unzip } from './shell.js';
1111
import type { Installed, ExeName } from './install.js';
1212

@@ -160,8 +160,8 @@ export async function downloadNeovim(version: string, os: Os, arch: Arch): Promi
160160
const url = `https://github.com/neovim/neovim/releases/download/${version}/${file}`;
161161
console.log(`Downloading Neovim ${version} on ${os} from ${url} to ${destDir}`);
162162

163-
const dlDir = await makeTmpdir();
164-
const asset = path.join(dlDir, file);
163+
const tmpDir = await TmpDir.create();
164+
const asset = path.join(tmpDir.path, file);
165165

166166
try {
167167
core.debug(`Downloading asset ${asset}`);
@@ -191,14 +191,16 @@ export async function downloadNeovim(version: string, os: Os, arch: Arch): Promi
191191
core.warning(
192192
`Fall back to x86_64 build because downloading Neovim for arm64 windows from ${url} failed: ${err}`,
193193
);
194-
return downloadNeovim(version, os, 'x86_64');
194+
return await downloadNeovim(version, os, 'x86_64');
195195
}
196196

197197
let msg = `Could not download Neovim release from ${url}: ${err.message}. Please visit https://github.com/neovim/neovim/releases/tag/${version} to check the asset for ${os} was really uploaded`;
198198
if (version === 'nightly') {
199199
msg += ". Note that some assets are sometimes missing on nightly build due to Neovim's CI failure";
200200
}
201201
throw new Error(msg);
202+
} finally {
203+
await tmpDir.cleanup();
202204
}
203205
}
204206

@@ -286,16 +288,25 @@ export async function buildNightlyNeovim(os: Os): Promise<Installed> {
286288
// Add -nightly suffix since building stable Neovim from source may be supported in the future
287289
const installDir = path.join(homedir(), 'nvim-nightly');
288290
core.debug(`Building and installing Neovim to ${installDir}`);
289-
const dir = path.join(await makeTmpdir(), 'build-nightly-neovim');
291+
const tmpDir = await TmpDir.create();
292+
try {
293+
const dir = path.join(tmpDir.path, 'build-nightly-neovim');
290294

291-
await exec('git', ['clone', '--depth=1', 'https://github.com/neovim/neovim.git', dir]);
295+
await exec('git', ['clone', '--depth=1', 'https://github.com/neovim/neovim.git', dir]);
292296

293-
const opts = { cwd: dir };
294-
const makeArgs = ['-j', `CMAKE_EXTRA_FLAGS=-DCMAKE_INSTALL_PREFIX=${installDir}`, 'CMAKE_BUILD_TYPE=RelWithDebug'];
295-
await exec('make', makeArgs, opts);
296-
core.debug(`Built Neovim in ${opts.cwd}. Installing it via 'make install'`);
297-
await exec('make', ['install'], opts);
298-
core.debug(`Installed Neovim to ${installDir}`);
297+
const opts = { cwd: dir };
298+
const makeArgs = [
299+
'-j',
300+
`CMAKE_EXTRA_FLAGS=-DCMAKE_INSTALL_PREFIX=${installDir}`,
301+
'CMAKE_BUILD_TYPE=RelWithDebug',
302+
];
303+
await exec('make', makeArgs, opts);
304+
core.debug(`Built Neovim in ${opts.cwd}. Installing it via 'make install'`);
305+
await exec('make', ['install'], opts);
306+
core.debug(`Installed Neovim to ${installDir}`);
307+
} finally {
308+
await tmpDir.cleanup();
309+
}
299310

300311
return {
301312
executable: exeName(os),

‎src/system.ts‎

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,40 @@
11
import { tmpdir } from 'node:os';
2+
import * as path from 'node:path';
23
import process from 'node:process';
34
import * as core from '@actions/core';
4-
import { mkdirP } from '@actions/io';
5+
import { mkdirP, rmRF } from '@actions/io';
56
import { HttpsProxyAgent } from 'https-proxy-agent';
67
import { getProxyForUrl } from 'proxy-from-env';
78

89
export type Os = 'macos' | 'linux' | 'windows';
910
export type Arch = 'arm64' | 'x86_64' | 'arm32';
1011

11-
export async function makeTmpdir(): Promise<string> {
12-
const dir = tmpdir();
13-
await mkdirP(dir);
14-
core.debug(`Created temporary directory ${dir}`);
15-
return dir;
12+
export function ensureError(err: unknown): Error {
13+
if (err instanceof Error) {
14+
return err;
15+
}
16+
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions
17+
return new Error(`Unknown fatal error: ${err}`);
18+
}
19+
20+
export class TmpDir {
21+
private constructor(public path: string) {}
22+
23+
async cleanup(): Promise<void> {
24+
try {
25+
await rmRF(this.path);
26+
} catch (err) {
27+
core.debug(`Could not remove the temporary directory ${this.path}: ${ensureError(err)}`);
28+
}
29+
}
30+
31+
static async create(): Promise<TmpDir> {
32+
const timestamp = new Date().getTime();
33+
const dir = path.join(tmpdir(), `action-setup-vim-${timestamp}`);
34+
await mkdirP(dir);
35+
core.debug(`Created temporary directory ${dir}`);
36+
return new TmpDir(dir);
37+
}
1638
}
1739

1840
export function getOs(): Os {
@@ -41,14 +63,6 @@ export function getArch(): Arch {
4163
}
4264
}
4365

44-
export function ensureError(err: unknown): Error {
45-
if (err instanceof Error) {
46-
return err;
47-
}
48-
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions
49-
return new Error(`Unknown fatal error: ${err}`);
50-
}
51-
5266
export function getSystemHttpsProxyAgent(url: string): HttpsProxyAgent<string> | undefined {
5367
const u = getProxyForUrl(url);
5468
return u ? new HttpsProxyAgent(u) : undefined;

‎src/vim.ts‎

Lines changed: 64 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import * as core from '@actions/core';
99
import * as io from '@actions/io';
1010
import { split as shlexSplit } from 'shlex';
1111
import { exec, unzip, Env } from './shell.js';
12-
import { makeTmpdir, type Os, type Arch, ensureError, getSystemHttpsProxyAgent } from './system.js';
12+
import { TmpDir, type Os, type Arch, ensureError, getSystemHttpsProxyAgent } from './system.js';
1313
import type { Installed, ExeName } from './install.js';
1414

1515
function exeName(os: Os): ExeName {
@@ -60,60 +60,67 @@ export async function buildVim(version: string, os: Os, configureArgs: string |
6060
assert.notEqual(version, 'stable');
6161
const installDir = path.join(homedir(), `vim-${version}`);
6262
core.debug(`Building and installing Vim to ${installDir} (version=${version ?? 'HEAD'})`);
63-
const dir = path.join(await makeTmpdir(), 'vim');
63+
const tmpDir = await TmpDir.create();
64+
const dir = path.join(tmpDir.path, 'vim');
6465

65-
{
66-
const args = ['clone', '--depth=1', '--single-branch'];
67-
if (version === 'nightly') {
68-
args.push('--no-tags');
69-
} else {
70-
args.push('--branch', version);
71-
}
72-
args.push('https://github.com/vim/vim', dir);
73-
74-
await exec('git', args);
75-
}
66+
try {
67+
{
68+
const args = ['clone', '--depth=1', '--single-branch'];
69+
if (version === 'nightly') {
70+
args.push('--no-tags');
71+
} else {
72+
args.push('--branch', version);
73+
}
74+
args.push('https://github.com/vim/vim', dir);
7675

77-
const env: Env = {};
78-
if (os === 'macos' && versionIsOlderThan(version, 8, 2, 1119)) {
79-
const dir = await getXcode11DevDir();
80-
if (dir !== null) {
81-
// Vim before v8.2.1119 cannot be built with Xcode 12 or later. It requires Xcode 11.
82-
// ref: https://github.com/vim/vim/commit/5289783e0b07cfc3f92ee933261ca4c4acdca007
83-
// By setting $DEVELOPER_DIR environment variable, Xcode11 is used to build Vim.
84-
// ref: https://www.jessesquires.com/blog/2020/01/06/selecting-an-xcode-version-on-github-ci/
85-
// Note that xcode-select command is not available since it changes Xcode version in system global.
86-
env['DEVELOPER_DIR'] = dir;
87-
core.debug(`Building Vim older than 8.2.1119 on macOS with Xcode11 at ${dir} instead of the latest Xcode`);
88-
} else {
89-
core.warning(
90-
`Building Vim older than 8.2.1119 on macOS needs Xcode11 but proper Xcode is not found at ${dir}. Using the latest Xcode as fallback. If you're using macos-latest or macos-12 runner and see some build error, try macos-11 runner`,
91-
);
76+
await exec('git', args);
9277
}
93-
}
9478

95-
const opts = { cwd: dir, env };
96-
{
97-
const args = [`--prefix=${installDir}`];
98-
if (configureArgs === null) {
99-
args.push('--with-features=huge', '--enable-fail-if-missing');
100-
} else {
101-
args.push(...shlexSplit(configureArgs));
102-
}
103-
try {
104-
await exec('./configure', args, opts);
105-
} catch (err) {
106-
if (os === 'macos' && versionIsOlderThan(version, 8, 2, 5135)) {
79+
const env: Env = {};
80+
if (os === 'macos' && versionIsOlderThan(version, 8, 2, 1119)) {
81+
const dir = await getXcode11DevDir();
82+
if (dir !== null) {
83+
// Vim before v8.2.1119 cannot be built with Xcode 12 or later. It requires Xcode 11.
84+
// ref: https://github.com/vim/vim/commit/5289783e0b07cfc3f92ee933261ca4c4acdca007
85+
// By setting $DEVELOPER_DIR environment variable, Xcode11 is used to build Vim.
86+
// ref: https://www.jessesquires.com/blog/2020/01/06/selecting-an-xcode-version-on-github-ci/
87+
// Note that xcode-select command is not available since it changes Xcode version in system global.
88+
env['DEVELOPER_DIR'] = dir;
89+
core.debug(
90+
`Building Vim older than 8.2.1119 on macOS with Xcode11 at ${dir} instead of the latest Xcode`,
91+
);
92+
} else {
10793
core.warning(
108-
'This version of Vim has a bug where ./configure cannot find a terminal library correctly. See the following issue for more details: https://github.com/rhysd/action-setup-vim/issues/38',
94+
`Building Vim older than 8.2.1119 on macOS needs Xcode11 but proper Xcode is not found at ${dir}. Using the latest Xcode as fallback. If you're using macos-latest or macos-12 runner and see some build error, try macos-11 runner`,
10995
);
11096
}
111-
throw err;
11297
}
98+
99+
const opts = { cwd: dir, env };
100+
{
101+
const args = [`--prefix=${installDir}`];
102+
if (configureArgs === null) {
103+
args.push('--with-features=huge', '--enable-fail-if-missing');
104+
} else {
105+
args.push(...shlexSplit(configureArgs));
106+
}
107+
try {
108+
await exec('./configure', args, opts);
109+
} catch (err) {
110+
if (os === 'macos' && versionIsOlderThan(version, 8, 2, 5135)) {
111+
core.warning(
112+
'This version of Vim has a bug where ./configure cannot find a terminal library correctly. See the following issue for more details: https://github.com/rhysd/action-setup-vim/issues/38',
113+
);
114+
}
115+
throw err;
116+
}
117+
}
118+
await exec('make', ['-j'], opts);
119+
await exec('make', ['install'], opts);
120+
core.debug(`Built and installed Vim to ${installDir} (version=${version})`);
121+
} finally {
122+
await tmpDir.cleanup();
113123
}
114-
await exec('make', ['-j'], opts);
115-
await exec('make', ['install'], opts);
116-
core.debug(`Built and installed Vim to ${installDir} (version=${version})`);
117124

118125
return {
119126
executable: exeName(os),
@@ -181,10 +188,11 @@ export async function detectLatestWindowsReleaseTag(): Promise<string> {
181188
}
182189

183190
async function installVimAssetOnWindows(file: string, url: string, dirSuffix: string): Promise<string> {
184-
const tmpdir = await makeTmpdir();
185-
const dlDir = path.join(tmpdir, 'vim-installer');
191+
const tmpDir = await TmpDir.create();
192+
const dlDir = path.join(tmpDir.path, 'vim-installer');
186193
await io.mkdirP(dlDir);
187194
const assetFile = path.join(dlDir, file);
195+
const destDir = path.join(homedir(), `vim-${dirSuffix}`);
188196

189197
try {
190198
core.debug(`Downloading asset at ${url} to ${dlDir}`);
@@ -197,19 +205,20 @@ async function installVimAssetOnWindows(file: string, url: string, dirSuffix: st
197205
core.debug(`Downloaded installer from ${url} to ${assetFile}`);
198206

199207
await unzip(assetFile, dlDir);
208+
209+
const vimDir = path.join(dlDir, 'vim'); // Unarchived to 'vim' directory
210+
core.debug(`Unzipped installer from ${url} to ${vimDir}`);
211+
212+
await io.mv(vimDir, destDir);
213+
core.debug(`Vim was installed to ${destDir}`);
200214
} catch (e) {
201215
const err = ensureError(e);
202216
core.debug(err.stack ?? err.message);
203217
throw new Error(`Could not download and unarchive asset ${url} at ${dlDir}: ${err.message}`);
218+
} finally {
219+
await tmpDir.cleanup();
204220
}
205221

206-
const vimDir = path.join(dlDir, 'vim'); // Unarchived to 'vim' directory
207-
core.debug(`Unzipped installer from ${url} to ${vimDir}`);
208-
209-
const destDir = path.join(homedir(), `vim-${dirSuffix}`);
210-
await io.mv(vimDir, destDir);
211-
core.debug(`Vim was installed to ${destDir}`);
212-
213222
return destDir;
214223
}
215224

‎test/system.ts‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { strict as A } from 'node:assert';
22
import process from 'node:process';
3-
import { getSystemHttpsProxyAgent, ensureError, getOs, getArch } from '../src/system.js';
3+
import fs from 'node:fs/promises';
4+
import * as path from 'node:path';
5+
import { getSystemHttpsProxyAgent, ensureError, getOs, getArch, TmpDir } from '../src/system.js';
46

57
describe('getSystemHttpsProxyAgent()', function () {
68
let savedEnv: Record<string, string | undefined>;
@@ -63,3 +65,23 @@ describe('getArch()', function () {
6365
A.ok(a === 'x86_64' || a === 'arm64' || a === 'arm32', `${a}`);
6466
});
6567
});
68+
69+
describe('TmpDir', function () {
70+
it('creates a temporary directory on create()', async function () {
71+
const tmp = await TmpDir.create();
72+
try {
73+
const stat = await fs.stat(tmp.path);
74+
A.ok(stat.isDirectory());
75+
const name = path.basename(tmp.path);
76+
A.match(name, /^action-setup-vim-\d+$/);
77+
} finally {
78+
await tmp.cleanup();
79+
}
80+
});
81+
82+
it('deletes a temporary directory on cleanup()', async function () {
83+
const tmp = await TmpDir.create();
84+
await tmp.cleanup();
85+
await A.rejects(() => fs.stat(tmp.path));
86+
});
87+
});

0 commit comments

Comments
 (0)