Skip to content
Open
Show file tree
Hide file tree
Changes from 8 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
18 changes: 8 additions & 10 deletions src/bundler/bundle_v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4122,24 +4122,22 @@ pub mod bv2_impl {
let source = &mut sources[index];

let output_path: Box<[u8]> = {
// TODO: outbase
let pathname =
Fs::PathName::init(bun_paths::resolve_path::relative_platform::<
bun_paths::resolve_path::platform::Loose,
false,
>(
&self.transpiler.options.root_dir,
source.path.text,
));
let pathname = Fs::PathName::init(source.path.text);

template.placeholder.name = pathname.base.to_vec().into_boxed_slice();
template.placeholder.dir = pathname.dir.to_vec().into_boxed_slice();
let mut ext: &[u8] = pathname.ext;
if !ext.is_empty() && ext[0] == b'.' {
ext = &ext[1..];
}
template.placeholder.ext = ext.to_vec().into_boxed_slice();

if template.needs(options::PlaceholderField::Dir) {
template.placeholder.dir = options::source_dir_relative_to_root(
pathname.dir,
&self.transpiler.options.root_dir,
)?;
}

if template.needs(options::PlaceholderField::Hash) {
template.placeholder.hash =
Some(content_hashes_for_additional_files[index]);
Expand Down
45 changes: 4 additions & 41 deletions src/bundler/linker_context/computeChunks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use core::sync::atomic::AtomicUsize;
use bun_alloc::Arena; // bumpalo::Bump re-export
use bun_collections::{ArrayHashMap, AutoBitSet, VecExt};
use bun_core::strings;
use bun_paths::{PathBuffer, resolve_path};
use bun_sourcemap::SourceMapPieces;
use bun_wyhash::{self, Wyhash};

Expand Down Expand Up @@ -634,46 +633,10 @@ pub fn compute_chunks(this: &mut LinkerContext, unique_key: u64) -> crate::Resul
}

if chunk.template.needs(PlaceholderField::Dir) {
// this if check is a specific fix for `bun build hi.ts --external '*'`, without leading `./`
let dir_path: &[u8] = if !pathname.dir.is_empty() {
pathname.dir
} else {
b"."
};
let mut real_path_buf = PathBuffer::uninit();
let dir: &[u8] = 'dir: {
let Ok(dir_file) = bun_sys::File::openat(
bun_sys::Fd::cwd(),
dir_path,
bun_sys::O::PATH | bun_sys::O::DIRECTORY,
0,
) else {
break 'dir &*resolve_path::normalize_buf::<bun_paths::platform::Auto>(
dir_path,
&mut real_path_buf.0,
);
};

match dir_file.get_path(&mut real_path_buf) {
Ok(p) => break 'dir p,
Err(err) => {
// Split-borrow — see `LinkerContext::log_disjoint`.
this.log_disjoint().add_error_fmt(
None,
bun_ast::Loc::EMPTY,
format_args!(
"{}: Failed to get full path for directory '{}'",
bstr::BStr::new(err.name()),
bstr::BStr::new(dir_path)
),
);
return Err(crate::Error::BuildFailed);
}
}
};

let root_dir = &this.resolver().opts.root_dir;
chunk.template.placeholder.dir = resolve_path::relative_alloc(root_dir, dir)?;
chunk.template.placeholder.dir = crate::options::source_dir_relative_to_root(
pathname.dir,
&this.resolver().opts.root_dir,
)?;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
Comment thread
claude[bot] marked this conversation as resolved.
}

Expand Down
34 changes: 34 additions & 0 deletions src/bundler/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2480,6 +2480,40 @@
Target,
}

/// `[dir]` placeholder value: canonicalize `source_dir` via `get_fd_path`
/// (so Windows 8.3 short names / symlinked prefixes match a canonical
/// `root_dir`), fall back to string normalization, then relativize.
pub(crate) fn source_dir_relative_to_root(
source_dir: &[u8],
root_dir: &[u8],
) -> Result<Box<[u8]>, bun_alloc::AllocError> {
let source_dir: &[u8] = if source_dir.is_empty() {
b"."
} else {
source_dir
};

Check warning on line 2494 in src/bundler/options.rs

View check run for this annotation

Claude / Claude Code Review

Extraction dropped two load-bearing why-comments

The extraction dropped two existing why-comments that REVIEW.md's "Don't delete existing why-comments in cleanup passes" rule protects: (1) `// TODO: outbase` at the `process_files_to_copy` site in `bundle_v2.rs` — outbase support was not added, so the TODO remains unaddressed; (2) `// this if check is a specific fix for \`bun build hi.ts --external '*'\`, without leading \`./\`` from `computeChunks.rs` — the empty-dir → `b"."` substitution was preserved at `options.rs:2490-2494` but the comment
Comment thread
robobun marked this conversation as resolved.
let mut buf = bun_paths::path_buffer_pool::get();
let dir: &[u8] = 'dir: {
let Ok(f) = bun_sys::File::openat(
bun_sys::Fd::cwd(),
source_dir,
bun_sys::O::PATH | bun_sys::O::DIRECTORY,
0,
) else {
break 'dir &*bun_paths::resolve_path::normalize_buf::<bun_paths::platform::Auto>(
source_dir, &mut buf.0,
);
};
match f.get_path(&mut buf) {
Ok(p) => p,
Err(_) => &*bun_paths::resolve_path::normalize_buf::<bun_paths::platform::Auto>(
source_dir, &mut buf.0,
),
}
};
bun_paths::resolve_path::relative_alloc(root_dir, dir)
}

// Shared body for PathTemplate::needs / PathTemplateConst::needs (D064).
#[inline]
pub(crate) fn path_template_needs(data: &[u8], field: PlaceholderField) -> bool {
Expand Down
60 changes: 59 additions & 1 deletion test/bundler/bundler_naming.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { describe } from "bun:test";
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe, isWindows, tempDir } from "harness";
import { symlinkSync } from "node:fs";
import { join } from "node:path";
import { ESBUILD, itBundled } from "./expectBundled";

describe("bundler", () => {
Expand Down Expand Up @@ -191,6 +194,61 @@ describe("bundler", () => {
},
],
});
// `[dir]` must resolve relative to the configured root even when root and
// the asset source path spell the same directory differently (Windows 8.3
// short names in the cwd, or a symlinked `Bun.build({ files })` key).
test("naming/AssetNamingDirCanonicalRoot", async () => {
using base = tempDir("asset-naming-dir-canon", {
"real/src/lib/first/.keep": "",
"real/src/lib/second/.keep": "",
});
const real = join(String(base), "real");
const link = join(String(base), "project-link");
// A junction needs no elevation on Windows; on POSIX this is a plain
// directory symlink. Either way `root` below canonicalizes to `real/src`
// while the `files` map keys keep the `project-link` spelling.
Comment thread
robobun marked this conversation as resolved.
symlinkSync(real, link, isWindows ? "junction" : "dir");

const entry = join(link, "src/lib/first/file.js").replaceAll("\\", "/");
const asset = join(link, "src/lib/second/data.file").replaceAll("\\", "/");
const root = join(link, "src");

const script = `
const result = await Bun.build({
entrypoints: [${JSON.stringify(entry)}],
files: {
${JSON.stringify(entry)}: 'import f from "../second/data.file"; console.log(f);',
${JSON.stringify(asset)}: "this is a file",
},
root: ${JSON.stringify(root)},
naming: { entry: "hello.[ext]", asset: "[dir]/test.[ext]" },
loader: { ".file": "file" },
});
if (!result.success) {
for (const m of result.logs) console.error(String(m));
process.exit(1);
}
for (const out of result.outputs) console.log(out.kind + " " + out.path);
`;

await using proc = Bun.spawn({
cmd: [bunExe(), "-e", script],
env: bunEnv,
cwd: String(base),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

const out = stdout.replaceAll("\\", "/");
const assetLine = out.split("\n").find(l => l.startsWith("asset "));
expect({ assetLine, stderr, exitCode }).toEqual({
assetLine: "asset ./lib/second/test.file",
stderr: "",
exitCode: 0,
});
expect(out).not.toContain("_.._");
});
itBundled("naming/AssetNoOverwrite", {
todo: true,
files: {
Expand Down
Loading