Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Copy go-turbo as well as turbo in install script #3202

Merged
merged 3 commits into from
Jan 6, 2023
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
48 changes: 27 additions & 21 deletions packages/turbo/install.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const {
const TURBO_VERSION = require("./package.json").version;

const toPath = path.join(__dirname, "bin", "turbo");
const goToPath = path.join(__dirname, "bin", "go-turbo");
let isToPathJS = true;

function validateBinaryVersion(...command) {
Expand Down Expand Up @@ -195,30 +196,35 @@ function maybeOptimizePackage(binPath) {
// This optimization also doesn't apply when npm's "--ignore-scripts" flag is
// used since in that case this install script will not be run.
if (os.platform() !== "win32" && !isYarn()) {
const tempPath = path.join(__dirname, "bin-turbo");
try {
// 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(binPath, tempPath);
const optimizeBin = (from, to, temp) => {
const tempPath = path.join(__dirname, temp);
try {
// 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(from, 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);
// 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, to);

// If we get here, then we know that the target location is now a binary
// executable instead of a JavaScript file.
isToPathJS = false;
// If we get here, then we know that the target location is now a binary
// executable instead of a JavaScript file.
isToPathJS = false;

// If this install script is being re-run, then "renameSync" will fail
// since the underlying inode is the same (it just returns without doing
// anything, and without throwing an error). In that case we should remove
// the file manually.
fs.unlinkSync(tempPath);
} catch {
// Ignore errors here since this optimization is optional
}
// If this install script is being re-run, then "renameSync" will fail
// since the underlying inode is the same (it just returns without doing
// anything, and without throwing an error). In that case we should remove
// the file manually.
fs.unlinkSync(tempPath);
} catch {
// Ignore errors here since this optimization is optional
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realize it's probably quite the edge case, but do we need to worry about only one of the binaries successfully copying?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, actually, I'm going to reverse the order, since if we fail to copy turbo, I think we're still ok.

}
};
optimizeBin(binPath, toPath, "bin-turbo");
const goBinPath = path.join(path.dirname(binPath), "go-turbo");
optimizeBin(goBinPath, goToPath, "bin-go-turbo");
}
}

Expand Down