Skip to content

Commit

Permalink
fix: which should use cwd if given a relative filepath (#9761)
Browse files Browse the repository at this point in the history
* Revert "fix!: do not lookup cwd in which (#9691)"

This reverts commit 4869ebf.

* fix which implementation to be more accurate

* t

* which tests windows

* [autofix.ci] apply automated fixes

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
  • Loading branch information
paperclover and autofix-ci[bot] authored Mar 31, 2024
1 parent c177e05 commit f027525
Show file tree
Hide file tree
Showing 22 changed files with 136 additions and 187 deletions.
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 @@ -713,6 +713,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
2 changes: 1 addition & 1 deletion src/allocators.zig
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const bun = @import("root").bun;

pub fn isSliceInBufferT(comptime T: type, slice: []const T, buffer: []const T) bool {
return (@intFromPtr(buffer.ptr) <= @intFromPtr(slice.ptr) and
(@intFromPtr(slice.ptr) + slice.len) <= (@intFromPtr(buffer.ptr) + buffer.len));
(@intFromPtr(slice.ptr) + slice.len * @sizeOf(T)) <= (@intFromPtr(buffer.ptr) + buffer.len * @sizeOf(T)));
}

/// Checks if a slice's pointer is contained within another slice.
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 {
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 @@ -452,6 +452,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 @@ -462,6 +463,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 @@ -532,13 +534,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 @@ -659,6 +663,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 @@ -683,6 +688,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 @@ -114,7 +114,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

0 comments on commit f027525

Please sign in to comment.