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(windows): fix directory cache regression "expected to end with a trailing slash" #9144

Merged
merged 10 commits into from
Feb 28, 2024
2 changes: 1 addition & 1 deletion packages/bun-internal-test/src/runner.node.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ function defaultConcurrency() {
// Concurrency causes more flaky tests, only enable it by default on windows
// See https://github.com/oven-sh/bun/issues/8071
if (windows) {
return Math.floor((cpus().length - 2) / 2);
return Math.floor((cpus().length - 2) / 3);
}
return 1;
}
Expand Down
16 changes: 7 additions & 9 deletions src/bun.js/javascript.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1710,31 +1710,29 @@ pub const VirtualMachine = struct {
if (std.fs.path.isAbsolute(normalized_specifier)) {
if (std.fs.path.dirname(normalized_specifier)) |dir| {
// Normalized with trailing slash
break :name bun.strings.normalizeSlashesOnly(
&specifier_cache_resolver_buf,
if (dir.len == 1) dir else normalized_specifier[0 .. dir.len + 1],
'/',
);
break :name bun.strings.normalizeSlashesOnly(&specifier_cache_resolver_buf, dir, std.fs.path.sep);
}
}

var parts = [_]string{
source_to_use,
normalized_specifier,
bun.pathLiteral("../"),
bun.pathLiteral(".."),
};

break :name bun.path.joinAbsStringBufZTrailingSlash(
break :name bun.path.joinAbsStringBufZ(
jsc_vm.bundler.fs.top_level_dir,
&specifier_cache_resolver_buf,
&parts,
.auto,
);
};

// Only re-query if we previously had something cached.
if (jsc_vm.bundler.resolver.bustDirCache(buster_name)) {
continue;
}

return error.ModuleNotFound;
},
};
Expand Down Expand Up @@ -3446,7 +3444,7 @@ pub fn NewHotReloader(comptime Ctx: type, comptime EventLoopType: type, comptime
// on windows we receive file events for all items affected by a directory change
// so we only need to clear the directory cache. all other effects will be handled
// by the file events
_ = resolver.bustDirCache(file_path);
_ = resolver.bustDirCache(strings.pathWithoutTrailingSlashOne(file_path));
continue;
}
var affected_buf: [128][]const u8 = undefined;
Expand Down Expand Up @@ -3496,7 +3494,7 @@ pub fn NewHotReloader(comptime Ctx: type, comptime EventLoopType: type, comptime
}
}

_ = resolver.bustDirCache(file_path);
_ = resolver.bustDirCache(strings.pathWithoutTrailingSlashOne(file_path));

if (entries_option) |dir_ent| {
var last_file_hash: GenericWatcher.HashType = std.math.maxInt(GenericWatcher.HashType);
Expand Down
10 changes: 3 additions & 7 deletions src/bundler.zig
Original file line number Diff line number Diff line change
Expand Up @@ -403,20 +403,16 @@ pub const Bundler = struct {
if (std.fs.path.isAbsolute(entry_point)) {
if (std.fs.path.dirname(entry_point)) |dir| {
// Normalized with trailing slash
break :name bun.strings.normalizeSlashesOnly(
&cache_bust_buf,
if (dir.len == 1) dir else entry_point[0 .. dir.len + 1],
'/',
);
break :name bun.strings.normalizeSlashesOnly(&cache_bust_buf, dir, std.fs.path.sep);
}
}

var parts = [_]string{
entry_point,
bun.pathLiteral("../"),
bun.pathLiteral(".."),
};

break :name bun.path.joinAbsStringBufZTrailingSlash(
break :name bun.path.joinAbsStringBufZ(
bundler.fs.top_level_dir,
&cache_bust_buf,
&parts,
Expand Down
10 changes: 9 additions & 1 deletion src/codegen/bundle-modules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,15 @@ if (!debug) {

namespace Bun {
namespace InternalModuleRegistryConstants {
${moduleList.map((id, n) => declareASCIILiteral(`${idToEnumName(id)}Code`, outputs.get(id.slice(0, -3)))).join("\n")}
${moduleList
.map((id, n) => {
const out = outputs.get(id.slice(0, -3).replaceAll("/", path.sep));
if (!out) {
throw new Error(`Missing output for ${id}`);
}
return declareASCIILiteral(`${idToEnumName(id)}Code`, out);
})
.join("\n")}
}
}`,
);
Expand Down
9 changes: 0 additions & 9 deletions src/codegen/generate-classes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -567,9 +567,6 @@ JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES ${name}::construct(JSC::JSGlobalObj
obj.estimatedSize
? `
auto size = ${symbolName(typeName, "estimatedSize")}(ptr);
#if ASSERT_ENABLED
ASSERT(size > 0);
#endif
vm.heap.reportExtraMemoryAllocated(instance, size);`
: ""
}
Expand Down Expand Up @@ -1231,9 +1228,6 @@ void ${name}::visitChildrenImpl(JSCell* cell, Visitor& visitor)
estimatedSize
? `if (auto* ptr = thisObject->wrapped()) {
auto size = ${symbolName(typeName, "estimatedSize")}(ptr);
#if ASSERT_ENABLED
ASSERT(size > 0);
#endif
visitor.reportExtraMemoryVisited(size);
}`
: ""
Expand Down Expand Up @@ -1404,9 +1398,6 @@ extern "C" EncodedJSValue ${typeName}__create(Zig::GlobalObject* globalObject, v
obj.estimatedSize
? `
auto size = ${symbolName(typeName, "estimatedSize")}(ptr);
#if ASSERT_ENABLED
ASSERT(size > 0);
#endif
vm.heap.reportExtraMemoryAllocated(instance, size);`
: ""
}
Expand Down
35 changes: 19 additions & 16 deletions src/fs.zig
Original file line number Diff line number Diff line change
Expand Up @@ -109,16 +109,11 @@ pub const FileSystem = struct {
ENOTDIR,
};

pub fn init(
top_level_dir: ?string,
) !*FileSystem {
pub fn init(top_level_dir: ?string) !*FileSystem {
return initWithForce(top_level_dir, false);
}

pub fn initWithForce(
top_level_dir_: ?string,
comptime force: bool,
) !*FileSystem {
pub fn initWithForce(top_level_dir_: ?string, comptime force: bool) !*FileSystem {
const allocator = bun.fs_allocator;
var top_level_dir = top_level_dir_ orelse (if (Environment.isBrowser) "/project/" else try bun.getcwdAlloc(allocator));

Expand All @@ -134,9 +129,7 @@ pub const FileSystem = struct {
if (!instance_loaded or force) {
instance = FileSystem{
.top_level_dir = top_level_dir,
.fs = Implementation.init(
top_level_dir,
),
.fs = Implementation.init(top_level_dir),
// must always use default_allocator since the other allocators may not be threadsafe when an element resizes
.dirname_store = DirnameStore.init(bun.default_allocator),
.filename_store = FilenameStore.init(bun.default_allocator),
Expand Down Expand Up @@ -1001,8 +994,18 @@ pub const FileSystem = struct {
// https://twitter.com/jarredsumner/status/1655787337027309568
// https://twitter.com/jarredsumner/status/1655714084569120770
// https://twitter.com/jarredsumner/status/1655464485245845506
pub fn readDirectoryWithIterator(fs: *RealFS, _dir: string, _handle: ?std.fs.Dir, generation: bun.Generation, store_fd: bool, comptime Iterator: type, iterator: Iterator) !*EntriesOption {
var dir = _dir;
pub fn readDirectoryWithIterator(
fs: *RealFS,
dir_maybe_trail_slash: string,
maybe_handle: ?std.fs.Dir,
generation: bun.Generation,
store_fd: bool,
comptime Iterator: type,
iterator: Iterator,
) !*EntriesOption {
var dir = bun.strings.pathWithoutTrailingSlashOne(dir_maybe_trail_slash);

bun.resolver.Resolver.assertValidCacheKey(dir);
var cache_result: ?allocators.Result = null;
if (comptime FeatureFlags.enable_entry_cache) {
fs.entries_mutex.lock();
Expand All @@ -1028,20 +1031,20 @@ pub const FileSystem = struct {
}
}

var handle = _handle orelse try fs.openDir(dir);
var handle = maybe_handle orelse try fs.openDir(dir);

defer {
if (_handle == null and (!store_fd or fs.needToCloseFiles())) {
if (maybe_handle == null and (!store_fd or fs.needToCloseFiles())) {
handle.close();
}
}

// if we get this far, it's a real directory, so we can just store the dir name.
if (_handle == null) {
if (maybe_handle == null) {
dir = try if (in_place) |existing|
existing.dir
else
DirnameStore.instance.append(string, _dir);
DirnameStore.instance.append(string, dir_maybe_trail_slash);
}

// Cache miss: read the directory entries
Expand Down
10 changes: 4 additions & 6 deletions src/resolver/resolve_path.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1085,23 +1085,21 @@ pub fn normalizeStringBuf(
str: []const u8,
buf: []u8,
comptime allow_above_root: bool,
comptime _platform: Platform,
comptime platform: Platform,
comptime preserve_trailing_slash: bool,
) []u8 {
return normalizeStringBufT(u8, str, buf, allow_above_root, _platform, preserve_trailing_slash);
return normalizeStringBufT(u8, str, buf, allow_above_root, platform, preserve_trailing_slash);
}

pub fn normalizeStringBufT(
comptime T: type,
str: []const T,
buf: []T,
comptime allow_above_root: bool,
comptime _platform: Platform,
comptime platform: Platform,
comptime preserve_trailing_slash: bool,
) []T {
const platform = comptime _platform.resolve();

switch (comptime platform) {
switch (comptime platform.resolve()) {
.auto => @compileError("unreachable"),

.windows => {
Expand Down
Loading
Loading