Skip to content

Commit

Permalink
revert #91 to try to fix #407
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Sep 23, 2020
1 parent 4ba207a commit 0c22da3
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 16 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
# Changelog

## Unreleased

* Undo an earlier change to try to improve yarn compatibility ([#91](https://github.com/evanw/esbuild/pull/91) and [#407](https://github.com/evanw/esbuild/issues/407))

The [yarn package manager](https://github.com/yarnpkg/yarn) behaves differently from npm and is not compatible in many ways. While npm is the only officially supported package manager for esbuild, people have contributed fixes for other package managers including yarn. One such fix is PR [#91](https://github.com/evanw/esbuild/pull/91) which makes sure the install script only runs once for a given installation directory.

I suspect this fix is actually incorrect, and is the cause of issue [#407](https://github.com/evanw/esbuild/issues/407). The problem seems to be that if you change the version of a package using `yarn add esbuild@version`, yarn doesn't clear out the installation directory before reinstalling the package so the package ends up with a mix of files from both package versions. This is not how npm behaves and seems like a pretty severe bug in yarn. I am reverting PR [#91](https://github.com/evanw/esbuild/pull/91) in an attempt to fix this issue.

## 0.7.3

* Fix compile error due to missing `unix.SYS_IOCTL` in the latest `golang.org/x/sys` ([#396](https://github.com/evanw/esbuild/pull/396))
Expand Down
17 changes: 1 addition & 16 deletions lib/install.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,8 @@ declare const ESBUILD_VERSION: string;

const version = ESBUILD_VERSION;
const binPath = path.join(__dirname, 'bin', 'esbuild');
const stampPath = path.join(__dirname, 'stamp.txt');

async function installBinaryFromPackage(name: string, fromPath: string, toPath: string): Promise<void> {
// It turns out that some package managers (e.g. yarn) sometimes re-run the
// postinstall script for this package after we have already been installed.
// That means this script must be idempotent. Let's skip the install if it's
// already happened.
if (fs.existsSync(stampPath)) {
return;
}

// Try to install from the cache if possible
const cachePath = getCachePath(name);
try {
Expand All @@ -33,9 +24,6 @@ async function installBinaryFromPackage(name: string, fromPath: string, toPath:
// Mark the cache entry as used for LRU
const now = new Date;
fs.utimesSync(cachePath, now, now);

// Mark the operation as successful so this script is idempotent
fs.writeFileSync(stampPath, '');
return;
} catch {
}
Expand Down Expand Up @@ -83,9 +71,6 @@ async function installBinaryFromPackage(name: string, fromPath: string, toPath:
process.exit(1);
}

// Mark the operation as successful so this script is idempotent
fs.writeFileSync(stampPath, '');

// Also try to cache the file to speed up future installs
try {
fs.mkdirSync(path.dirname(cachePath), { recursive: true });
Expand Down Expand Up @@ -172,7 +157,7 @@ function extractFileFromTarGzip(buffer: Buffer, file: string): Buffer {

function installUsingNPM(name: string, file: string): Buffer {
const installDir = path.join(__dirname, '.install');
fs.mkdirSync(installDir);
fs.mkdirSync(installDir, { recursive: true });
fs.writeFileSync(path.join(installDir, 'package.json'), '{}');

// Erase "npm_config_global" so that "npm install --global esbuild" works.
Expand Down

0 comments on commit 0c22da3

Please sign in to comment.