Skip to content

Commit

Permalink
fix #1642: permission issues with install script
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Oct 4, 2021
1 parent 78e0468 commit 95f10ac
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 10 deletions.
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,17 @@
# Changelog

## Unreleased

* Fix permission issues with the install script ([#1642](https://github.com/evanw/esbuild/issues/1642))

The `esbuild` package contains a small JavaScript stub file that implements the CLI (command-line interface). Its only purpose is to spawn the binary esbuild executable as a child process and forward the command-line arguments to it.

The install script contains an optimization that replaces this small JavaScript stub with the actual binary executable at install time to avoid the overhead of unnecessarily creating a new `node` process. This optimization can't be done at package publish time because there is only one `esbuild` package but there are many supported platforms, so the binary executable for the current platform must live outside of the `esbuild` package.

However, the optimization was implemented with an [unlink](https://www.man7.org/linux/man-pages/man2/unlink.2.html) operation followed by a [link](https://www.man7.org/linux/man-pages/man2/link.2.html) operation. This means that if the first step fails, the package is left in a broken state since the JavaScript stub file is deleted but not yet replaced.

With this release, the optimization is now implemented with a [link](https://www.man7.org/linux/man-pages/man2/link.2.html) operation followed by a [rename](https://www.man7.org/linux/man-pages/man2/rename.2.html) operation. This should always leave the package in a working state even if either step fails.

## 0.13.3

* Support TypeScript type-only import/export specifiers ([#1637](https://github.com/evanw/esbuild/pull/1637))
Expand Down
30 changes: 20 additions & 10 deletions lib/npm/node-install.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import child_process = require('child_process');

declare const ESBUILD_VERSION: string;
const toPath = path.join(__dirname, 'bin', 'esbuild');
let isToPathJS = true;

function validateBinaryVersion(...command: string[]): void {
command.push('--version');
Expand Down Expand Up @@ -40,9 +41,6 @@ if (process.env.ESBUILD_BINARY_PATH) {
const libMain = path.join(__dirname, 'lib', 'main.js');
const code = fs.readFileSync(libMain, 'utf8');
fs.writeFileSync(libMain, `var ESBUILD_BINARY_PATH = ${pathString};\n${code}`);

// Windows needs "node" before this command since it's a JavaScript file
validateBinaryVersion('node', toPath);
}

// This package contains a "bin/esbuild" JavaScript file that finds and runs
Expand All @@ -60,18 +58,30 @@ if (process.env.ESBUILD_BINARY_PATH) {
// this install script will not be run.
else if (os.platform() !== 'win32' && !isYarn2OrAbove()) {
const bin = binPathForCurrentPlatform();
const tempPath = path.join(__dirname, 'bin-esbuild');
try {
fs.unlinkSync(toPath);
fs.linkSync(bin, toPath);
// First link the binary with a temporary file. If this fails and throws an
// error, then we'll just end up doing nothing. This uses a hard link to
// avoid taking up additional space on the file system.
fs.linkSync(bin, tempPath);

// Then use rename to atomically replace the target file with the temporary
// file. If this fails and throws an error, then we'll just end up leaving
// the temporary file there, which is harmless.
fs.renameSync(tempPath, toPath);

// If we get here, then we know that the target location is now a binary
// executable instead of a JavaScript file.
isToPathJS = false;
} catch (e) {
// Ignore errors here since this optimization is optional
}

// This is no longer a JavaScript file so don't run it using "node"
validateBinaryVersion(toPath);
}

else {
// Windows needs "node" before this command since it's a JavaScript file
if (isToPathJS) {
// We need "node" before this command since it's a JavaScript file
validateBinaryVersion('node', toPath);
} else {
// This is no longer a JavaScript file so don't run it using "node"
validateBinaryVersion(toPath);
}

0 comments on commit 95f10ac

Please sign in to comment.