Skip to content

bunfig: keep CLI flags ahead of bunfig.toml when it is loaded after argv (bun run) #38599

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.

Sign up for GitHub

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

Open
robobun wants to merge 1 commit into
base: main
Choose a base branch
from
+340 −43
Open

bunfig: keep CLI flags ahead of bunfig.toml when it is loaded after argv (bun run) #38599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
88 changes: 55 additions & 33 deletions src/bunfig/bunfig.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ impl<'a> Parser<'a> {
}

fn load_preload(&mut self, expr: &Expr) -> crate::Result<()> {
match &expr.data {
let mut preloads: Vec<Box<[u8]>> = match &expr.data {
ExprData::EArray(array) => {
let items = array.items.slice();
let mut preloads: Vec<Box<[u8]>> = Vec::with_capacity(items.len());
Expand All @@ -256,18 +256,22 @@ impl<'a> Parser<'a> {
}
}
}
self.ctx.preloads = preloads;
preloads
}
ExprData::EString(s) => {
if s.len() > 0 {
self.ctx.preloads = vec![estring_to_owned(s, self.bump)];
if s.len() == 0 {
return Ok(());
}
vec![estring_to_owned(s, self.bump)]
}
ExprData::ENull(_) => {}
_ => {
self.add_error(expr.loc, b"Expected preload to be an array")?;
}
}
ExprData::ENull(_) => return Ok(()),
_ => return self.add_error(expr.loc, b"Expected preload to be an array"),
};
// `ctx.preloads` already holds the `--preload`s when bunfig.toml is
// loaded after argument parsing (`bun run`); as in the other load
// order, the config's preloads run first and argv's follow.
preloads.append(&mut self.ctx.preloads);
self.ctx.preloads = preloads;
Ok(())
}

Expand Down Expand Up @@ -359,8 +363,15 @@ impl<'a> Parser<'a> {
self.load_log_level(&expr)?;
}

// Keys with a command-line counterpart are still validated but only
// applied when that flag was not given: see `CliOverrides`.
let cli = self.ctx.cli_overrides;

if let Some(expr) = json.get(b"define") {
self.ctx.args.define = Some(self.parse_define_map(&expr)?);
let define = self.parse_define_map(&expr)?;
if !cli.define {
self.ctx.args.define = Some(define);
}
}

if let Some(expr) = json.get(b"origin") {
Expand Down Expand Up @@ -723,9 +734,9 @@ impl<'a> Parser<'a> {
}

if let Some(auto_install_expr) = install_obj.get(b"auto") {
if let ExprData::EString(_) = &auto_install_expr.data {
let auto_install = if let ExprData::EString(_) = &auto_install_expr.data {
let key = auto_install_expr.as_string(self.bump).unwrap_or(b"");
self.ctx.debug.global_cache = match GlobalCache::MAP.get(key) {
match GlobalCache::MAP.get(key) {
Some(v) => *v,
None => {
self.add_error(
Expand All @@ -734,19 +745,22 @@ impl<'a> Parser<'a> {
)?;
return Ok(());
}
};
}
} else if let ExprData::EBoolean(b) = auto_install_expr.data {
self.ctx.debug.global_cache = if b.value {
if b.value {
GlobalCache::allow_install
} else {
GlobalCache::disable
};
}
} else {
self.add_error(
auto_install_expr.loc,
b"Invalid auto install setting, must be one of true, false, or \"force\" \"fallback\" \"disable\"",
)?;
return Ok(());
};
if !cli.auto_install {
self.ctx.debug.global_cache = auto_install;
}
}

Expand Down Expand Up @@ -827,13 +841,15 @@ impl<'a> Parser<'a> {
if let Some(console_expr) = json.get(b"console") {
if let Some(depth) = console_expr.get(b"depth") {
if let Some(n) = depth.as_number() {
let depth_value = n as u16;
// Treat depth=0 as maxInt(u16) for infinite depth
self.ctx.runtime_options.console_depth = Some(if depth_value == 0 {
u16::MAX
} else {
depth_value
});
if !cli.console_depth {
let depth_value = n as u16;
// Treat depth=0 as maxInt(u16) for infinite depth
self.ctx.runtime_options.console_depth = Some(if depth_value == 0 {
u16::MAX
} else {
depth_value
});
}
} else {
self.add_error(depth.loc, b"Expected number")?;
}
Expand Down Expand Up @@ -987,16 +1003,18 @@ impl<'a> Parser<'a> {
}
{
if let Some(jsx) = self.ctx.args.jsx.as_mut() {
if !jsx_factory.is_empty() {
if !jsx_factory.is_empty() && !cli.jsx_factory {
jsx.factory = jsx_factory;
}
if !jsx_fragment.is_empty() {
if !jsx_fragment.is_empty() && !cli.jsx_fragment {
jsx.fragment = jsx_fragment;
}
if !jsx_import_source.is_empty() {
if !jsx_import_source.is_empty() && !cli.jsx_import_source {
jsx.import_source = jsx_import_source;
}
jsx.runtime = jsx_runtime;
if !cli.jsx_runtime {
jsx.runtime = jsx_runtime;
}
jsx.development = jsx_dev;
} else {
self.ctx.args.jsx = Some(api::Jsx {
Expand All @@ -1020,12 +1038,14 @@ impl<'a> Parser<'a> {

if let Some(expr) = json.get(b"macros") {
if let ExprData::EBoolean(b) = expr.data {
if !b.value {
if !b.value && !cli.macros {
self.ctx.debug.macros = MacroOptions::Disable;
}
} else {
self.ctx.debug.macros =
MacroOptions::Map(parse_macros_json(&expr, self.log, self.source, self.bump));
let remaps = parse_macros_json(&expr, self.log, self.source, self.bump);
if !cli.macros {
self.ctx.debug.macros = MacroOptions::Map(remaps);
}
}
bun_analytics::features::macros.fetch_add(1, Ordering::Relaxed);
}
Expand Down Expand Up @@ -1084,10 +1104,12 @@ impl<'a> Parser<'a> {
loader_names.push(key.into());
loader_values.push(loader.to_api());
}
self.ctx.args.loaders = Some(api::LoaderMap {
extensions: loader_names,
loaders: loader_values,
});
if !cli.loaders {
self.ctx.args.loaders = Some(api::LoaderMap {
extensions: loader_names,
loaders: loader_values,
});
}
}

Ok(())
Expand Down
31 changes: 31 additions & 0 deletions src/options_types/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,36 @@ pub struct ContextData {

pub preloads: Vec<Box<[u8]>>,
pub has_loaded_global_config: bool,
pub cli_overrides: CliOverrides,
}

/// Settings that were given on the command line.
///
/// bunfig.toml is usually parsed before argv is applied, so argv simply
/// overwrites it. `bun run <target>`, the `node` shim, `bun repl` and
/// standalone executables only load it afterwards (the `loaded_bunfig`
/// checks in run_command.rs and repl_command.rs); the bunfig parser skips the
/// keys recorded here so the command line wins in that order too.
#[derive(Clone, Copy, Default)]
pub struct CliOverrides {
/// `--define`
pub define: bool,
/// `--loader`
pub loaders: bool,
/// `--jsx-runtime`
pub jsx_runtime: bool,
/// `--jsx-factory`
pub jsx_factory: bool,
/// `--jsx-fragment`
pub jsx_fragment: bool,
/// `--jsx-import-source`
pub jsx_import_source: bool,
/// `--console-depth`
pub console_depth: bool,
/// `--install`, `-i` or `--no-install`
pub auto_install: bool,
/// `--no-macros`
pub macros: bool,
}

impl Default for ContextData {
Expand Down Expand Up @@ -84,6 +114,7 @@ impl Default for ContextData {
no_exit_on_error: false,
preloads: Vec::new(),
has_loaded_global_config: false,
cli_overrides: CliOverrides::default(),
}
}
}
Expand Down
24 changes: 19 additions & 5 deletions src/runtime/cli/Arguments.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -926,6 +926,7 @@ pub(crate) fn parse(cmd: CommandTag, ctx: Context<'_>) -> crate::Result<api::Tra
let defines_tuple = DefineColonList::resolve(args.options(b"--define"))?;

if !defines_tuple.keys.is_empty() {
ctx.cli_overrides.define = true;
opts.define = Some(api::StringMap {
keys: defines_tuple
.keys
Expand Down Expand Up @@ -955,6 +956,7 @@ pub(crate) fn parse(cmd: CommandTag, ctx: Context<'_>) -> crate::Result<api::Tra
};

if !loader_tuple.keys.is_empty() {
ctx.cli_overrides.loaders = true;
opts.loaders = Some(api::LoaderMap {
extensions: loader_tuple
.keys
Expand Down Expand Up @@ -1179,26 +1181,32 @@ pub(crate) fn parse(cmd: CommandTag, ctx: Context<'_>) -> crate::Result<api::Tra
bun_options_types::offline_mode::OfflineMode::Online
});

if args.flag(b"--no-install") {
ctx.debug.global_cache = options::GlobalCache::disable;
let auto_install = if args.flag(b"--no-install") {
Some(options::GlobalCache::disable)
} else if args.flag(b"-i") && cmd != CommandTag::RunAsNodeCommand {
// Under node emulation `-i` is node's --interactive alias, not
// --install=fallback (auto-install is meaningless there).
ctx.debug.global_cache = options::GlobalCache::fallback;
Some(options::GlobalCache::fallback)
} else if let Some(enum_value) = args.option(b"--install") {
// -i=auto --install=force, --install=disable
if let Some(result) = options::GlobalCache::MAP.get(enum_value) {
ctx.debug.global_cache = *result;
Some(*result)
// -i, --install
} else if enum_value.is_empty() {
ctx.debug.global_cache = options::GlobalCache::force;
Some(options::GlobalCache::force)
} else {
Output::err_generic(
"Invalid value for --install: \"{}\". Must be either \"auto\", \"fallback\", \"force\", or \"disable\"\n",
format_args!("{}", BStr::new(enum_value)),
);
Global::exit(1);
}
} else {
None
};
if let Some(auto_install) = auto_install {
ctx.debug.global_cache = auto_install;
ctx.cli_overrides.auto_install = true;
}

if let Some(script) = args.option(b"--print") {
Expand Down Expand Up @@ -1239,6 +1247,7 @@ pub(crate) fn parse(cmd: CommandTag, ctx: Context<'_>) -> crate::Result<api::Tra
};
// Treat depth=0 as maxInt(u16) for infinite depth
ctx.runtime_options.console_depth = Some(if depth == 0 { u16::MAX } else { depth });
ctx.cli_overrides.console_depth = true;
}

if let Some(order) = args.option(b"--dns-result-order") {
Expand Down Expand Up @@ -1602,6 +1611,10 @@ pub(crate) fn parse(cmd: CommandTag, ctx: Context<'_>) -> crate::Result<api::Tra
|| jsx_import_source.is_some()
|| jsx_runtime.is_some()
{
ctx.cli_overrides.jsx_factory = jsx_factory.is_some();
ctx.cli_overrides.jsx_fragment = jsx_fragment.is_some();
ctx.cli_overrides.jsx_import_source = jsx_import_source.is_some();
ctx.cli_overrides.jsx_runtime = jsx_runtime.is_some();
let default_factory: &[u8] = b"";
let default_fragment: &[u8] = b"";
let default_import_source: &[u8] = b"";
Expand Down Expand Up @@ -1686,6 +1699,7 @@ pub(crate) fn parse(cmd: CommandTag, ctx: Context<'_>) -> crate::Result<api::Tra

if args.flag(b"--no-macros") {
ctx.debug.macros = MacroOptions::Disable;
ctx.cli_overrides.macros = true;
}

opts.output_dir = output_dir.map(Box::<[u8]>::from);
Expand Down
Loading