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

fix: which should use cwd if given a relative filepath #9761

Merged
merged 5 commits into from
Mar 31, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/bun-types/bun.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,9 @@ declare module "bun" {
*
* @param {string} command The name of the executable or script
* @param {string} options.PATH Overrides the PATH environment variable
* @param {string} options.cwd When given a relative path, use this path to join it.
*/
function which(command: string, options?: { PATH?: string }): string | null;
function which(command: string, options?: { PATH?: string; cwd?: string }): string | null;

/**
* Get the column count of a string as it would be displayed in a terminal.
Expand Down
1 change: 1 addition & 0 deletions src/StandaloneModuleGraph.zig
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,7 @@ pub const StandaloneModuleGraph = struct {
if (bun.which(
&whichbuf,
bun.getenvZ("PATH") orelse return error.FileNotFound,
"",
bun.argv()[0],
)) |path| {
return bun.toFD((try std.fs.cwd().openFileZ(path, .{})).handle);
Expand Down
8 changes: 8 additions & 0 deletions src/bun.js/api/BunObject.zig
Original file line number Diff line number Diff line change
Expand Up @@ -563,18 +563,26 @@ pub fn which(
path_str = ZigString.Slice.fromUTF8NeverFree(
globalThis.bunVM().bundler.env.get("PATH") orelse "",
);
cwd_str = ZigString.Slice.fromUTF8NeverFree(
globalThis.bunVM().bundler.fs.top_level_dir,
);

if (arguments.nextEat()) |arg| {
if (!arg.isEmptyOrUndefinedOrNull() and arg.isObject()) {
if (arg.get(globalThis, "PATH")) |str_| {
path_str = str_.toSlice(globalThis, globalThis.bunVM().allocator);
}

if (arg.get(globalThis, "cwd")) |str_| {
cwd_str = str_.toSlice(globalThis, globalThis.bunVM().allocator);
}
}
}

if (Which.which(
&path_buf,
path_str.slice(),
cwd_str.slice(),
bin_str.slice(),
)) |bin_path| {
return ZigString.init(bin_path).withEncoding().toValueGC(globalThis);
Expand Down
4 changes: 2 additions & 2 deletions src/bun.js/api/bun/subprocess.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1642,7 +1642,7 @@ pub const Subprocess = struct {

if (argv0 == null) {
var path_buf: [bun.MAX_PATH_BYTES]u8 = undefined;
const resolved = Which.which(&path_buf, PATH, arg0.slice()) orelse {
const resolved = Which.which(&path_buf, PATH, cwd, arg0.slice()) orelse {
Copy link
Member

Choose a reason for hiding this comment

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

should this cwd be the one on line 1709?

Copy link
Member Author

Choose a reason for hiding this comment

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

this was from reverting #9691. i'll look if this is correct code.

globalThis.throwInvalidArguments("Executable not found in $PATH: \"{s}\"", .{arg0.slice()});
return .zero;
};
Expand All @@ -1652,7 +1652,7 @@ pub const Subprocess = struct {
};
} else {
var path_buf: [bun.MAX_PATH_BYTES]u8 = undefined;
const resolved = Which.which(&path_buf, PATH, bun.sliceTo(argv0.?, 0)) orelse {
const resolved = Which.which(&path_buf, PATH, cwd, bun.sliceTo(argv0.?, 0)) orelse {
globalThis.throwInvalidArguments("Executable not found in $PATH: \"{s}\"", .{arg0.slice()});
return .zero;
};
Expand Down
21 changes: 0 additions & 21 deletions src/bun.js/bindings/ZigGlobalObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2070,26 +2070,6 @@ extern "C" int32_t ReadableStreamTag__tagged(Zig::GlobalObject* globalObject, JS
return 0;
}

extern "C" JSC__JSValue ReadableStream__consume(Zig::GlobalObject* globalObject, JSC__JSValue stream, JSC__JSValue nativeType, JSC__JSValue nativePtr);
extern "C" JSC__JSValue ReadableStream__consume(Zig::GlobalObject* globalObject, JSC__JSValue stream, JSC__JSValue nativeType, JSC__JSValue nativePtr)
{
ASSERT(globalObject);

auto& vm = globalObject->vm();
auto scope = DECLARE_CATCH_SCOPE(vm);

auto& builtinNames = WebCore::builtinNames(vm);

auto function = globalObject->getDirect(vm, builtinNames.consumeReadableStreamPrivateName()).getObject();
JSC::MarkedArgumentBuffer arguments = JSC::MarkedArgumentBuffer();
arguments.append(JSValue::decode(nativePtr));
arguments.append(JSValue::decode(nativeType));
arguments.append(JSValue::decode(stream));

auto callData = JSC::getCallData(function);
return JSC::JSValue::encode(call(globalObject, function, callData, JSC::jsUndefined(), arguments));
}

extern "C" JSC__JSValue ZigGlobalObject__createNativeReadableStream(Zig::GlobalObject* globalObject, JSC__JSValue nativePtr)
{
auto& vm = globalObject->vm();
Expand Down Expand Up @@ -3390,7 +3370,6 @@ void GlobalObject::addBuiltinGlobals(JSC::VM& vm)
putDirectBuiltinFunction(vm, this, builtinNames.createFIFOPrivateName(), streamInternalsCreateFIFOCodeGenerator(vm), PropertyAttribute::Builtin | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly);
putDirectBuiltinFunction(vm, this, builtinNames.createEmptyReadableStreamPrivateName(), readableStreamCreateEmptyReadableStreamCodeGenerator(vm), PropertyAttribute::Builtin | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly);
putDirectBuiltinFunction(vm, this, builtinNames.createUsedReadableStreamPrivateName(), readableStreamCreateUsedReadableStreamCodeGenerator(vm), PropertyAttribute::Builtin | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly);
putDirectBuiltinFunction(vm, this, builtinNames.consumeReadableStreamPrivateName(), readableStreamConsumeReadableStreamCodeGenerator(vm), PropertyAttribute::Builtin | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly);
putDirectBuiltinFunction(vm, this, builtinNames.createNativeReadableStreamPrivateName(), readableStreamCreateNativeReadableStreamCodeGenerator(vm), PropertyAttribute::Builtin | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly);
putDirectBuiltinFunction(vm, this, builtinNames.requireESMPrivateName(), importMetaObjectRequireESMCodeGenerator(vm), PropertyAttribute::Builtin | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly);
putDirectBuiltinFunction(vm, this, builtinNames.loadCJS2ESMPrivateName(), importMetaObjectLoadCJS2ESMCodeGenerator(vm), PropertyAttribute::Builtin | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly);
Expand Down
6 changes: 6 additions & 0 deletions src/cli/bunx_command.zig
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,7 @@ pub const BunxCommand = struct {
destination_ = bun.which(
&path_buf,
PATH_FOR_BIN_DIRS,
if (ignore_cwd.len > 0) "" else this_bundler.fs.top_level_dir,
initial_bin_name,
);
}
Expand All @@ -459,6 +460,7 @@ pub const BunxCommand = struct {
if (destination_ orelse bun.which(
&path_buf,
bunx_cache_dir,
if (ignore_cwd.len > 0) "" else this_bundler.fs.top_level_dir,
absolute_in_cache_dir,
)) |destination| {
const out = bun.asByteSlice(destination);
Expand Down Expand Up @@ -529,13 +531,15 @@ pub const BunxCommand = struct {
destination_ = bun.which(
&path_buf,
bunx_cache_dir,
if (ignore_cwd.len > 0) "" else this_bundler.fs.top_level_dir,
package_name_for_bin,
);
}

if (destination_ orelse bun.which(
&path_buf,
bunx_cache_dir,
if (ignore_cwd.len > 0) "" else this_bundler.fs.top_level_dir,
absolute_in_cache_dir,
)) |destination| {
const out = bun.asByteSlice(destination);
Expand Down Expand Up @@ -651,6 +655,7 @@ pub const BunxCommand = struct {
if (bun.which(
&path_buf,
bunx_cache_dir,
if (ignore_cwd.len > 0) "" else this_bundler.fs.top_level_dir,
absolute_in_cache_dir,
)) |destination| {
const out = bun.asByteSlice(destination);
Expand All @@ -675,6 +680,7 @@ pub const BunxCommand = struct {
if (bun.which(
&path_buf,
bunx_cache_dir,
if (ignore_cwd.len > 0) "" else this_bundler.fs.top_level_dir,
absolute_in_cache_dir,
)) |destination| {
const out = bun.asByteSlice(destination);
Expand Down
4 changes: 2 additions & 2 deletions src/cli/create_command.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1564,7 +1564,7 @@ pub const CreateCommand = struct {
Output.flush();

if (create_options.open) {
if (which(&bun_path_buf, PATH, "bun")) |bin| {
if (which(&bun_path_buf, PATH, destination, "bun")) |bin| {
var argv = [_]string{bun.asByteSlice(bin)};
var child = std.ChildProcess.init(&argv, ctx.allocator);
child.cwd = destination;
Expand Down Expand Up @@ -2289,7 +2289,7 @@ const GitHandler = struct {
// Time (mean ± σ): 306.7 ms ± 6.1 ms [User: 31.7 ms, System: 269.8 ms]
// Range (min … max): 299.5 ms … 318.8 ms 10 runs

if (which(&bun_path_buf, PATH, "git")) |git| {
if (which(&bun_path_buf, PATH, destination, "git")) |git| {
const git_commands = .{
&[_]string{ git, "init", "--quiet" },
&[_]string{ git, "add", destination, "--ignore-errors" },
Expand Down
2 changes: 1 addition & 1 deletion src/cli/install_completions_command.zig
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ pub const InstallCompletionsCommand = struct {
var buf: [bun.MAX_PATH_BYTES]u8 = undefined;

// don't install it if it's already there
if (bun.which(&buf, bun.getenvZ("PATH") orelse cwd, bunx_name) != null)
if (bun.which(&buf, bun.getenvZ("PATH") orelse cwd, cwd, bunx_name) != null)
return;

// first try installing the symlink into the same directory as the bun executable
Expand Down
12 changes: 6 additions & 6 deletions src/cli/run_command.zig
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,13 @@ pub const RunCommand = struct {
"zsh",
};

fn findShellImpl(PATH: string) ?stringZ {
fn findShellImpl(PATH: string, cwd: string) ?stringZ {
if (comptime Environment.isWindows) {
return "C:\\Windows\\System32\\cmd.exe";
}

inline for (shells_to_search) |shell| {
if (which(&path_buf, PATH, shell)) |shell_| {
if (which(&path_buf, PATH, cwd, shell)) |shell_| {
return shell_;
}
}
Expand Down Expand Up @@ -101,7 +101,7 @@ pub const RunCommand = struct {

/// Find the "best" shell to use
/// Cached to only run once
pub fn findShell(PATH: string) ?stringZ {
pub fn findShell(PATH: string, cwd: string) ?stringZ {
const bufs = struct {
pub var shell_buf_once: [bun.MAX_PATH_BYTES]u8 = undefined;
pub var found_shell: [:0]const u8 = "";
Expand All @@ -110,7 +110,7 @@ pub const RunCommand = struct {
return bufs.found_shell;
}

if (findShellImpl(PATH)) |found| {
if (findShellImpl(PATH, cwd)) |found| {
if (found.len < bufs.shell_buf_once.len) {
@memcpy(bufs.shell_buf_once[0..found.len], found);
bufs.shell_buf_once[found.len] = 0;
Expand Down Expand Up @@ -275,7 +275,7 @@ pub const RunCommand = struct {
silent: bool,
use_system_shell: bool,
) !bool {
const shell_bin = findShell(env.get("PATH") orelse "") orelse return error.MissingShell;
const shell_bin = findShell(env.get("PATH") orelse "", cwd) orelse return error.MissingShell;

const script = original_script;
var copy_script = try std.ArrayList(u8).initCapacity(allocator, script.len);
Expand Down Expand Up @@ -1587,7 +1587,7 @@ pub const RunCommand = struct {
}

if (path_for_which.len > 0) {
if (which(&path_buf, path_for_which, script_name_to_search)) |destination| {
if (which(&path_buf, path_for_which, this_bundler.fs.top_level_dir, script_name_to_search)) |destination| {
const out = bun.asByteSlice(destination);
return try runBinaryWithoutBunxPath(
ctx,
Expand Down
2 changes: 1 addition & 1 deletion src/cli/upgrade_command.zig
Original file line number Diff line number Diff line change
Expand Up @@ -612,7 +612,7 @@ pub const UpgradeCommand = struct {
}

if (comptime Environment.isPosix) {
const unzip_exe = which(&unzip_path_buf, env_loader.map.get("PATH") orelse "", "unzip") orelse {
const unzip_exe = which(&unzip_path_buf, env_loader.map.get("PATH") orelse "", filesystem.top_level_dir, "unzip") orelse {
save_dir.deleteFileZ(tmpname) catch {};
Output.prettyErrorln("<r><red>error:<r> Failed to locate \"unzip\" in PATH. bun upgrade needs \"unzip\" to work.", .{});
Global.exit(1);
Expand Down
13 changes: 7 additions & 6 deletions src/env_loader.zig
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,14 @@ pub const Loader = struct {
return strings.eqlComptime(env, "test");
}

pub fn getNodePath(this: *Loader, buf: *bun.PathBuffer) ?[:0]const u8 {
pub fn getNodePath(this: *Loader, fs: *Fs.FileSystem, buf: *bun.PathBuffer) ?[:0]const u8 {
if (this.get("NODE") orelse this.get("npm_node_execpath")) |node| {
@memcpy(buf[0..node.len], node);
buf[node.len] = 0;
return buf[0..node.len :0];
}

if (which(buf, this.get("PATH") orelse return null, "node")) |node| {
if (which(buf, this.get("PATH") orelse return null, fs.top_level_dir, "node")) |node| {
return node;
}

Expand Down Expand Up @@ -180,22 +180,23 @@ pub const Loader = struct {

var did_load_ccache_path: bool = false;

pub fn loadCCachePath(this: *Loader) void {
pub fn loadCCachePath(this: *Loader, fs: *Fs.FileSystem) void {
if (did_load_ccache_path) {
return;
}
did_load_ccache_path = true;
loadCCachePathImpl(this) catch {};
loadCCachePathImpl(this, fs) catch {};
}

fn loadCCachePathImpl(this: *Loader) !void {
fn loadCCachePathImpl(this: *Loader, fs: *Fs.FileSystem) !void {

// if they have ccache installed, put it in env variable `CMAKE_CXX_COMPILER_LAUNCHER` so
// cmake can use it to hopefully speed things up
var buf: [bun.MAX_PATH_BYTES]u8 = undefined;
const ccache_path = bun.which(
&buf,
this.get("PATH") orelse return,
fs.top_level_dir,
"ccache",
) orelse "";

Expand Down Expand Up @@ -228,7 +229,7 @@ pub const Loader = struct {
if (node_path_to_use_set_once.len > 0) {
node_path_to_use = node_path_to_use_set_once;
} else {
const node = this.getNodePath(&buf) orelse return false;
const node = this.getNodePath(fs, &buf) orelse return false;
node_path_to_use = try fs.dirname_store.append([]const u8, bun.asByteSlice(node));
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/install/install.zig
Original file line number Diff line number Diff line change
Expand Up @@ -2121,11 +2121,11 @@ pub const PackageManager = struct {
};
}

this.env.loadCCachePath();
this.env.loadCCachePath(this_bundler.fs);

{
var node_path: [bun.MAX_PATH_BYTES]u8 = undefined;
if (this.env.getNodePath(&node_path)) |node_pathZ| {
if (this.env.getNodePath(this_bundler.fs, &node_path)) |node_pathZ| {
_ = try this.env.loadNodeJSConfig(this_bundler.fs, bun.default_allocator.dupe(u8, node_pathZ) catch bun.outOfMemory());
} else brk: {
const current_path = this.env.get("PATH") orelse "";
Expand Down
2 changes: 1 addition & 1 deletion src/install/lifecycle_script_runner.zig
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ pub const LifecycleScriptSubprocess = struct {
this.current_script_index = next_script_index;
this.has_called_process_exit = false;

const shell_bin = bun.CLI.RunCommand.findShell(env.get("PATH") orelse "") orelse return error.MissingShell;
const shell_bin = bun.CLI.RunCommand.findShell(env.get("PATH") orelse "", cwd) orelse return error.MissingShell;

var copy_script = try std.ArrayList(u8).initCapacity(manager.allocator, original_script.script.len + 1);
defer copy_script.deinit();
Expand Down
1 change: 0 additions & 1 deletion src/js/builtins.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,6 @@ declare function $closedPromise(): TODO;
declare function $closedPromiseCapability(): TODO;
declare function $code(): TODO;
declare function $connect(): TODO;
declare function $consumeReadableStream(): TODO;
declare function $controlledReadableStream(): TODO;
declare function $controller(): TODO;
declare function $cork(): TODO;
Expand Down
1 change: 0 additions & 1 deletion src/js/builtins/BunBuiltinNames.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ using namespace JSC;
macro(closeRequested) \
macro(code) \
macro(connect) \
macro(consumeReadableStream) \
macro(controlledReadableStream) \
macro(controller) \
macro(cork) \
Expand Down
Loading
Loading