From d4b19e2425d007de6891e099ca52d67ed69e02f0 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Fri, 31 Jul 2026 11:30:21 +0200 Subject: [PATCH] interpreter: load data files with an explicit `?format=` override Loading static data with `load()` required the file to be named `.json` or `.toml`, because the extension was the only thing selecting a parser. That rules out the files people actually want to read: `Cargo.lock` is TOML, a `package.json.tmpl` is JSON, and neither can be renamed. Add a `?format=` query on the load path (`load(":deps.lock?format=toml", ...)`) accepting `bzl`, `json`, or `toml`. It cannot be a `load()` kwarg: `format = "toml"` is already valid syntax meaning "alias the local name `format` to the exported symbol `toml`", so taking that spelling would need a grammar change and would silently change the meaning of existing code. The `?` was already reserved by `ImportPath`. The format is stored on `ImportPath` and participates in its hash and equality, so it is part of the DICE key rather than a side channel. That makes the loaded file a tracked dependency of the parse and gets invalidation for free, and the extension check is skipped exactly when a format was given. Every site that dispatched on the extension now asks `ImportPath::load_format()` instead. A format that merely restates the extension collapses to `None`, so `:a.bzl?format=bzl` and `:a.bzl` stay one module. Otherwise the same file would be evaluated under two keys and hand out two copies of its providers and transitive sets, breaking the pointer-identity checks that `resolve_load` already warns about. An unparseable file stays a load-time error, which fails the whole package. That is inherent: a parse that is not at load time is not a tracked dependency. Since the extension no longer implies the parser, the error now names the format and where it came from. The blast radius is documented, with a pointer to reading the file in an action when failure needs to be scoped to its consumers. Incidentally fixes prelude `.toml` files being evaluated as Starlark: the prelude branch special-cased only `.json`, and now shares the format dispatch. Covered by unit tests for format inference, the relaxed extension check, the redundant-format collapse and display round-tripping, plus e2e tests for the three formats, invalidation on edit, and each error path. Signed-off-by: Daan De Meyer --- app/buck2_core/src/bzl.rs | 201 +++++++++++++++++- app/buck2_interpreter/src/load_module.rs | 9 +- app/buck2_interpreter/src/parse_import.rs | 41 ++++ .../interpreter/dice_calculation_delegate.rs | 25 ++- .../src/interpreter/interpreter_for_dir.rs | 40 ++-- docs/users/loading_data.md | 32 ++- tests/core/interpreter/test_load_format.py | 88 ++++++++ .../test_load_format_data/.buckconfig | 5 + .../test_load_format_data/.buckroot | 0 .../test_load_format_data/TARGETS.fixture | 10 + .../test_load_format_data/deps.lock | 9 + .../test_load_format_data/metadata.txt | 4 + .../test_load_format_data/rules.bzl.in | 9 + 13 files changed, 443 insertions(+), 30 deletions(-) create mode 100644 tests/core/interpreter/test_load_format.py create mode 100644 tests/core/interpreter/test_load_format_data/.buckconfig create mode 100644 tests/core/interpreter/test_load_format_data/.buckroot create mode 100644 tests/core/interpreter/test_load_format_data/TARGETS.fixture create mode 100644 tests/core/interpreter/test_load_format_data/deps.lock create mode 100644 tests/core/interpreter/test_load_format_data/metadata.txt create mode 100644 tests/core/interpreter/test_load_format_data/rules.bzl.in diff --git a/app/buck2_core/src/bzl.rs b/app/buck2_core/src/bzl.rs index b0848292f8f06..42d111523249a 100644 --- a/app/buck2_core/src/bzl.rs +++ b/app/buck2_core/src/bzl.rs @@ -13,6 +13,7 @@ use std::fmt::Formatter; use allocative::Allocative; use buck2_fs::paths::file_name::FileName; +use dupe::Dupe; use pagable::Pagable; use strong_hash::StrongHash; @@ -27,8 +28,64 @@ use crate::cells::paths::CellRelativePath; enum ImportPathError { #[error("Invalid import path `{0}`")] Invalid(CellPath), - #[error("Import path must have suffix `.bzl`, `.json`, or `.toml`: `{0}`")] + #[error( + "Import path must have suffix `.bzl`, `.json`, or `.toml`, or specify `?format=`: `{0}`" + )] Suffix(CellPath), + #[error("Unknown load format `{0}`, expected one of `bzl`, `json`, or `toml`")] + UnknownFormat(String), +} + +/// How the contents of a loaded file are parsed. +#[derive( + Clone, + Copy, + Dupe, + Hash, + StrongHash, + Eq, + PartialEq, + Debug, + Allocative, + Pagable +)] +pub enum LoadFormat { + Bzl, + Json, + Toml, +} + +impl LoadFormat { + pub const fn as_str(self) -> &'static str { + match self { + LoadFormat::Bzl => "bzl", + LoadFormat::Json => "json", + LoadFormat::Toml => "toml", + } + } + + pub fn parse(format: &str) -> buck2_error::Result { + match format { + "bzl" => Ok(LoadFormat::Bzl), + "json" => Ok(LoadFormat::Json), + "toml" => Ok(LoadFormat::Toml), + _ => Err(ImportPathError::UnknownFormat(format.to_owned()).into()), + } + } + + fn from_extension(extension: Option<&str>) -> Self { + match extension { + Some("json") => LoadFormat::Json, + Some("toml") => LoadFormat::Toml, + _ => LoadFormat::Bzl, + } + } +} + +impl Display for LoadFormat { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + f.write_str(self.as_str()) + } } /// Path of a `.bzl` file. @@ -40,6 +97,10 @@ pub struct ImportPath { /// The cell of the top-level build module that this is being loaded /// (perhaps transitively) into. build_file_cell: BuildFileCell, + /// Explicit `?format=` from the `load()` string, or `None` to infer from the extension. + /// This participates in equality and hashing: the same file loaded under two formats is + /// two distinct modules, and both must remain separately cached and invalidated. + format: Option, } impl ImportPath { @@ -54,6 +115,17 @@ impl ImportPath { pub fn new_with_build_file_cells( path: CellPath, build_file_cell: BuildFileCell, + ) -> buck2_error::Result { + Self::new_with_format(path, build_file_cell, None) + } + + /// Like [`ImportPath::new_with_build_file_cells`], but with an explicit `?format=` from the + /// `load()` string. An explicit format lifts the extension requirement, so that e.g. + /// `Cargo.lock` can be loaded as TOML. + pub fn new_with_format( + path: CellPath, + build_file_cell: BuildFileCell, + format: Option, ) -> buck2_error::Result { if path.parent().is_none() { return Err(ImportPathError::Invalid(path).into()); @@ -63,13 +135,29 @@ impl ImportPath { return Err(ImportPathError::Invalid(path).into()); } - if !matches!(path.path().extension(), Some("bzl" | "json" | "toml")) { + let extension_is_known = matches!(path.path().extension(), Some("bzl" | "json" | "toml")); + if format.is_none() && !extension_is_known { return Err(ImportPathError::Suffix(path).into()); } + // A format that merely restates the extension must collapse to `None`, so that + // `:a.bzl?format=bzl` and `:a.bzl` are one module rather than two. Evaluating the same + // `.bzl` twice would hand out two copies of its providers and transitive sets, and + // anything doing pointer equality on those would then silently go wrong. + let format = match format { + Some(format) + if extension_is_known + && format == LoadFormat::from_extension(path.path().extension()) => + { + None + } + format => format, + }; + Ok(Self { path, build_file_cell, + format, }) } @@ -89,6 +177,7 @@ impl ImportPath { Ok(Self { path, build_file_cell, + format: None, }) } @@ -128,6 +217,19 @@ impl ImportPath { &self.path } + /// How this file should be parsed: the explicit `?format=`, else inferred from the extension. + pub fn load_format(&self) -> LoadFormat { + self.format + .unwrap_or_else(|| LoadFormat::from_extension(self.path.path().extension())) + } + + /// Whether [`ImportPath::load_format`] came from an explicit `?format=` rather than the + /// extension. Only useful for diagnostics: the extension is otherwise the reader's only + /// clue as to how the file was parsed. + pub fn has_explicit_format(&self) -> bool { + self.format.is_some() + } + /// Parent directory of the import path. pub fn path_parent(&self) -> CellPathRef<'_> { self.path @@ -139,9 +241,100 @@ impl ImportPath { impl Display for ImportPath { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { if self.build_file_cell.name() == self.path.cell() { - write!(f, "{}", self.path) + write!(f, "{}", self.path)?; } else { - write!(f, "{}@{}", self.path, self.build_file_cell.name()) + write!(f, "{}@{}", self.path, self.build_file_cell.name())?; + } + if let Some(format) = self.format { + write!(f, "?format={format}")?; } + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn import(filename: &str, format: Option) -> buck2_error::Result { + let path = CellPath::new( + CellName::testing_new("root"), + CellRelativePath::unchecked_new("pkg").join(FileName::unchecked_new(filename)), + ); + ImportPath::new_with_format( + path, + BuildFileCell::new(CellName::testing_new("root")), + format, + ) + } + + #[test] + fn format_is_inferred_from_extension() { + assert_eq!(LoadFormat::Bzl, import("a.bzl", None).unwrap().load_format()); + assert_eq!( + LoadFormat::Json, + import("a.json", None).unwrap().load_format() + ); + assert_eq!( + LoadFormat::Toml, + import("a.toml", None).unwrap().load_format() + ); + } + + #[test] + fn unknown_extension_requires_an_explicit_format() { + for filename in ["Cargo.lock", "Makefile", "rules.bzl.in"] { + assert!(import(filename, None).is_err(), "{filename}"); + + let explicit = import(filename, Some(LoadFormat::Toml)).unwrap(); + assert_eq!(LoadFormat::Toml, explicit.load_format(), "{filename}"); + assert!(explicit.has_explicit_format(), "{filename}"); + } + } + + #[test] + fn an_extensionless_file_keeps_a_redundant_looking_format() { + // `from_extension` defaults to `Bzl`, but an extensionless file has no extension for the + // format to be redundant with, so it must stay explicit or the path would not construct. + let explicit = import("Makefile", Some(LoadFormat::Bzl)).unwrap(); + assert_eq!(LoadFormat::Bzl, explicit.load_format()); + assert!(explicit.has_explicit_format()); + } + + #[test] + fn explicit_format_overrides_the_extension() { + let as_toml = import("a.json", Some(LoadFormat::Toml)).unwrap(); + assert_eq!(LoadFormat::Toml, as_toml.load_format()); + // The same file under two formats must stay two distinct modules. + assert_ne!(import("a.json", None).unwrap(), as_toml); + } + + #[test] + fn redundant_format_collapses_so_the_module_is_not_duplicated() { + for (filename, format) in [ + ("a.bzl", LoadFormat::Bzl), + ("a.json", LoadFormat::Json), + ("a.toml", LoadFormat::Toml), + ] { + let explicit = import(filename, Some(format)).unwrap(); + assert_eq!(import(filename, None).unwrap(), explicit); + assert!(!explicit.has_explicit_format()); + } + } + + #[test] + fn display_round_trips_an_explicit_format() { + assert_eq!("root//pkg/a.bzl", import("a.bzl", None).unwrap().to_string()); + assert_eq!( + "root//pkg/Cargo.lock?format=toml", + import("Cargo.lock", Some(LoadFormat::Toml)) + .unwrap() + .to_string() + ); + } + + #[test] + fn a_path_containing_a_question_mark_is_still_rejected() { + assert!(import("a.bzl?format=toml", Some(LoadFormat::Toml)).is_err()); } } diff --git a/app/buck2_interpreter/src/load_module.rs b/app/buck2_interpreter/src/load_module.rs index 0878e6c56e446..2b22eaf097d60 100644 --- a/app/buck2_interpreter/src/load_module.rs +++ b/app/buck2_interpreter/src/load_module.rs @@ -10,6 +10,7 @@ use async_trait::async_trait; use buck2_core::bzl::ImportPath; +use buck2_core::bzl::LoadFormat; use buck2_core::package::PackageLabel; use buck2_util::late_binding::LateBinding; use dice::DiceComputations; @@ -65,10 +66,10 @@ pub trait InterpreterCalculation { &mut self, path: &ImportPath, ) -> buck2_error::Result { - let module_path = match path.path().path().extension() { - Some("json") => StarlarkModulePath::JsonFile(path), - Some("toml") => StarlarkModulePath::TomlFile(path), - _ => StarlarkModulePath::LoadFile(path), + let module_path = match path.load_format() { + LoadFormat::Json => StarlarkModulePath::JsonFile(path), + LoadFormat::Toml => StarlarkModulePath::TomlFile(path), + LoadFormat::Bzl => StarlarkModulePath::LoadFile(path), }; self.get_loaded_module(module_path).await } diff --git a/app/buck2_interpreter/src/parse_import.rs b/app/buck2_interpreter/src/parse_import.rs index 6d570dd32a558..b6dc174b0ae96 100644 --- a/app/buck2_interpreter/src/parse_import.rs +++ b/app/buck2_interpreter/src/parse_import.rs @@ -11,6 +11,7 @@ //! Parses imports for load_file() calls in build files. use buck2_core::bzl::ImportPath; +use buck2_core::bzl::LoadFormat; use buck2_core::cells::CellAliasResolver; use buck2_core::cells::build_file_cell::BuildFileCell; use buck2_core::cells::cell_path::CellPath; @@ -37,6 +38,24 @@ enum ImportParseError { "Unable to parse import spec. Expected format `(@)//package/name:filename.bzl` or `:filename.bzl`, but got a path. Got `{0}`" )] NotAFileName(String), + #[error("Unable to parse import spec. Expected a `?format=` query, got `?{1}` in `{0}`")] + UnknownQuery(String, String), +} + +/// Split a trailing `?format=` query off a `load()` string. +/// +/// The format overrides the extension-based choice of parser, so that files whose names carry +/// no usable extension (`Cargo.lock`) or a misleading one (`rules.bzl.in`) can still be loaded. +pub fn split_load_format(import: &str) -> buck2_error::Result<(&str, Option)> { + match import.split_once('?') { + None => Ok((import, None)), + Some((path, query)) => match query.strip_prefix("format=") { + Some(format) => Ok((path, Some(LoadFormat::parse(format)?))), + None => { + Err(ImportParseError::UnknownQuery(import.to_owned(), query.to_owned()).into()) + } + }, + } } pub enum RelativeImports<'a> { @@ -421,4 +440,26 @@ mod tests { }; Ok(()) } + + #[test] + fn test_split_load_format() -> buck2_error::Result<()> { + assert_eq!((":a.bzl", None), split_load_format(":a.bzl")?); + assert_eq!( + (":Cargo.lock", Some(LoadFormat::Toml)), + split_load_format(":Cargo.lock?format=toml")? + ); + assert_eq!( + ("cell//pkg:package.json.tmpl", Some(LoadFormat::Json)), + split_load_format("cell//pkg:package.json.tmpl?format=json")? + ); + assert_eq!( + (":rules.bzl.in", Some(LoadFormat::Bzl)), + split_load_format(":rules.bzl.in?format=bzl")? + ); + + assert!(split_load_format(":a.bzl?format=yaml").is_err()); + assert!(split_load_format(":a.bzl?other=1").is_err()); + assert!(split_load_format(":a.bzl?").is_err()); + Ok(()) + } } diff --git a/app/buck2_interpreter_for_build/src/interpreter/dice_calculation_delegate.rs b/app/buck2_interpreter_for_build/src/interpreter/dice_calculation_delegate.rs index 6945b026de32e..66a7547944aec 100644 --- a/app/buck2_interpreter_for_build/src/interpreter/dice_calculation_delegate.rs +++ b/app/buck2_interpreter_for_build/src/interpreter/dice_calculation_delegate.rs @@ -22,6 +22,7 @@ use buck2_common::package_boundary::HasPackageBoundaryExceptions; use buck2_common::package_listing::dice::DicePackageListingResolver; use buck2_common::package_listing::listing::PackageListing; use buck2_core::build_file_path::BuildFilePath; +use buck2_core::bzl::LoadFormat; use buck2_core::cells::build_file_cell::BuildFileCell; use buck2_core::cells::cell_path::CellPath; use buck2_core::package::PackageLabel; @@ -70,6 +71,24 @@ use crate::interpreter::interpreter_for_dir::ParseData; use crate::interpreter::interpreter_for_dir::ParseResult; use crate::super_package::package_value::SuperPackageValuesImpl; +/// Context for a load-time parse failure. A `load()` failure fails the whole package, so the +/// message has to say which parser ran and, when the extension does not imply it, where that +/// choice came from. +fn parse_error_context(starlark_file: StarlarkModulePath<'_>, format: LoadFormat) -> String { + let explicit = match starlark_file { + StarlarkModulePath::LoadFile(path) + | StarlarkModulePath::JsonFile(path) + | StarlarkModulePath::TomlFile(path) => path.has_explicit_format(), + StarlarkModulePath::BxlFile(_) => false, + }; + let path = starlark_file.path(); + if explicit { + format!("Parsing `{path}` as {format}, as requested by `?format={format}`") + } else { + format!("Parsing `{path}` as {format}") + } +} + fn toml_value_to_json(value: toml::Value) -> serde_json::Value { match value { toml::Value::String(s) => serde_json::Value::String(s), @@ -284,7 +303,7 @@ impl<'c, 'd: 'c> DiceCalculationDelegate<'c, 'd> { .with_package_context_information(path.path().to_string())?; let value: serde_json::Value = serde_json::from_str(&contents) - .with_buck_error_context(|| format!("Parsing {path}"))?; + .with_buck_error_context(|| parse_error_context(starlark_file, LoadFormat::Json))?; // patternlint-disable-next-line buck2-no-starlark-module: We expect these to be small + simple let frozen = Module::with_temp_heap(|module| { @@ -311,8 +330,8 @@ impl<'c, 'd: 'c> DiceCalculationDelegate<'c, 'd> { .await .with_package_context_information(path.path().to_string())?; - let value: toml::Value = - toml::from_str(&contents).with_buck_error_context(|| format!("Parsing {path}"))?; + let value: toml::Value = toml::from_str(&contents) + .with_buck_error_context(|| parse_error_context(starlark_file, LoadFormat::Toml))?; let json_value = toml_value_to_json(value); // patternlint-disable-next-line buck2-no-starlark-module: We expect these to be small + simple diff --git a/app/buck2_interpreter_for_build/src/interpreter/interpreter_for_dir.rs b/app/buck2_interpreter_for_build/src/interpreter/interpreter_for_dir.rs index ac8e409d17cbf..43265ec413d44 100644 --- a/app/buck2_interpreter_for_build/src/interpreter/interpreter_for_dir.rs +++ b/app/buck2_interpreter_for_build/src/interpreter/interpreter_for_dir.rs @@ -23,6 +23,7 @@ use buck2_common::package_listing::listing::PackageListing; use buck2_core::build_file_path::BuildFilePath; use buck2_core::bxl::BxlFilePath; use buck2_core::bzl::ImportPath; +use buck2_core::bzl::LoadFormat; use buck2_core::cells::build_file_cell::BuildFileCell; use buck2_core::cells::cell_path::CellPath; use buck2_core::cells::cell_path_with_allowed_relative_dir::CellPathWithAllowedRelativeDir; @@ -42,6 +43,7 @@ use buck2_interpreter::import_paths::ImplicitImportPaths; use buck2_interpreter::package_imports::ImplicitImport; use buck2_interpreter::parse_import::RelativeImports; use buck2_interpreter::parse_import::parse_import; +use buck2_interpreter::parse_import::split_load_format; use buck2_interpreter::paths::module::OwnedStarlarkModulePath; use buck2_interpreter::paths::module::StarlarkModulePath; use buck2_interpreter::paths::package::PackageFilePath; @@ -192,6 +194,7 @@ impl LoadResolver for InterpreterLoadResolver { let relative_import_option = RelativeImports::Allow { current_dir_with_allowed_relative: &self.config.current_dir_with_allowed_relative_dirs, }; + let (path, format) = split_load_format(path)?; let path = parse_import( self.config.cell_info.cell_alias_resolver(), relative_import_option, @@ -200,7 +203,8 @@ impl LoadResolver for InterpreterLoadResolver { // check for bxl files first before checking for prelude. // All bxl imports are parsed the same regardless of prelude or not. - if path.path().extension() == Some("bxl") { + // An explicit `?format=` means the file is being loaded as data, not evaluated as bxl. + if format.is_none() && path.path().extension() == Some("bxl") { match self.loader_file_type { StarlarkFileType::Bzl | StarlarkFileType::Buck @@ -247,23 +251,27 @@ impl LoadResolver for InterpreterLoadResolver { // checks in t-sets, which would fail if we had > 1 copy of the prelude. if let Some(prelude_import) = self.config.global_state.configuror.prelude_import() { if prelude_import.is_prelude_path(&path) { - if path.path().extension() == Some("json") { - return Ok(OwnedStarlarkModulePath::JsonFile( - ImportPath::new_same_cell(path)?, - )); - } else { - return Ok(OwnedStarlarkModulePath::LoadFile( - ImportPath::new_same_cell(path)?, - )); - } + let build_file_cell = BuildFileCell::new(path.cell()); + return Ok(module_path_for_format(ImportPath::new_with_format( + path, + build_file_cell, + format, + )?)); } } - let import_path = ImportPath::new_with_build_file_cells(path, self.build_file_cell)?; - Ok(match import_path.path().path().extension() { - Some("json") => OwnedStarlarkModulePath::JsonFile(import_path), - Some("toml") => OwnedStarlarkModulePath::TomlFile(import_path), - _ => OwnedStarlarkModulePath::LoadFile(import_path), - }) + Ok(module_path_for_format(ImportPath::new_with_format( + path, + self.build_file_cell, + format, + )?)) + } +} + +fn module_path_for_format(import_path: ImportPath) -> OwnedStarlarkModulePath { + match import_path.load_format() { + LoadFormat::Json => OwnedStarlarkModulePath::JsonFile(import_path), + LoadFormat::Toml => OwnedStarlarkModulePath::TomlFile(import_path), + LoadFormat::Bzl => OwnedStarlarkModulePath::LoadFile(import_path), } } diff --git a/docs/users/loading_data.md b/docs/users/loading_data.md index 83ebfd5d72bfc..92f27c6a9b5e8 100644 --- a/docs/users/loading_data.md +++ b/docs/users/loading_data.md @@ -20,12 +20,38 @@ build process, but is necessary for the build itself - like URLs and checksums being passed into [`http_archive`](https://buck2.build/docs/prelude/rules/core/http_archive/). -The file name must end in `.toml` or `.json`, and `load()` can only import the -name `value` from these files. For TOML, `value` is always going to be a -dictionary with `str` keys (i.e. a +By default the file name must end in `.toml` or `.json`, and `load()` can only +import the name `value` from these files. For TOML, `value` is always going to be +a dictionary with `str` keys (i.e. a [TOML table](https://toml.io/en/v1.0.0#table)), and for JSON it maps to whatever value is in the file. +## Files with a different extension + +Plenty of data files do not have a usable extension - `Cargo.lock` is TOML, and a +`package.json.tmpl` is not JSON. Append `?format=` to the load path to say how the +file should be parsed, and the extension is no longer consulted: + +```python +load("//foo:Cargo.lock?format=toml", lock = "value") +load("//foo:package.json.tmpl?format=json", tmpl = "value") +load("//foo:rules.bzl.in?format=bzl", "my_rule") +``` + +The accepted formats are `bzl`, `json`, and `toml`. `?format=bzl` evaluates the +file as Starlark, exactly as a `.bzl` file would be, so it exports whatever +symbols it defines rather than a single `value`. + +A file loaded this way is a dependency of the build file that loads it, the same +as any `.bzl`: editing it invalidates the build file and everything downstream. + +That also means an unparseable file is a **load-time** error, and a load-time +error fails every target in the package - not just the ones that use the data. +A malformed `Cargo.lock` will make `buck2 build //foo:some-unrelated-target` fail +too. This is the cost of the data being a tracked, cached dependency; if you need +a corrupt file to fail only the targets that read it, read it in an action +instead. + If a name other than `value` is needed, `load()` allows aliasing: ```python diff --git a/tests/core/interpreter/test_load_format.py b/tests/core/interpreter/test_load_format.py new file mode 100644 index 0000000000000..039240f30811e --- /dev/null +++ b/tests/core/interpreter/test_load_format.py @@ -0,0 +1,88 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates. +# +# This source code is dual-licensed under either the MIT license found in the +# LICENSE-MIT file in the root directory of this source tree or the Apache +# License, Version 2.0 found in the LICENSE-APACHE file in the root directory +# of this source tree. You may select, at your option, one of the +# above-listed licenses. + +# pyre-strict + +import ast +from typing import Any + +from buck2.tests.e2e_util.api.buck import Buck +from buck2.tests.e2e_util.asserts import expect_failure +from buck2.tests.e2e_util.buck_workspace import buck_test + + +def extract_test_output(stderr: str) -> dict[str, Any]: + """Extract the DATA_LOAD_TEST_OUTPUT dict from buck2 stderr.""" + marker = "DATA_LOAD_TEST_OUTPUT: " + for line in stderr.splitlines(): + idx = line.find(marker) + if idx != -1: + return ast.literal_eval(line[idx + len(marker) :]) # type: ignore[no-any-return] + raise AssertionError(f"Could not find '{marker}' in stderr:\n{stderr}") + + +@buck_test() +async def test_load_format_selects_the_parser(buck: Buck) -> None: + result = await buck.targets("root//:") + data = extract_test_output(result.stderr) + + assert data["lock"]["version"] == 3 + assert [p["name"] for p in data["lock"]["package"]] == ["serde", "toml"] + assert data["metadata"] == {"name": "example", "version": 1} + assert data["greeting"] == "hello" + + +@buck_test() +async def test_load_format_files_are_tracked_dependencies(buck: Buck) -> None: + """The point of routing `format=` through ImportPath is that the loaded file is a + dependency of the parse, so editing it invalidates the build file.""" + await buck.targets("root//:") + + (buck.cwd / "deps.lock").write_text('version = 4\n\n[[package]]\nname = "anyhow"\n') + (buck.cwd / "metadata.txt").write_text('{"name": "renamed", "version": 2}\n') + (buck.cwd / "rules.bzl.in").write_text('greeting = "goodbye"\n') + + result = await buck.targets("root//:") + data = extract_test_output(result.stderr) + + assert data["lock"]["version"] == 4 + assert [p["name"] for p in data["lock"]["package"]] == ["anyhow"] + assert data["metadata"] == {"name": "renamed", "version": 2} + assert data["greeting"] == "goodbye" + + +@buck_test() +async def test_load_format_parse_error_names_the_format(buck: Buck) -> None: + """An unparseable file fails the whole package at load time. Because the extension no + longer implies the parser, the error has to say which one ran and why.""" + (buck.cwd / "deps.lock").write_text("this is not valid toml {{{\n") + + await expect_failure( + buck.targets("root//:"), + stderr_regex=r"Parsing `root//deps\.lock` as toml, as requested by `\?format=toml`", + ) + + +@buck_test() +async def test_load_format_rejects_an_unknown_format(buck: Buck) -> None: + (buck.cwd / "TARGETS.fixture").write_text('load(":metadata.txt?format=yaml", "value")\n') + + await expect_failure( + buck.targets("root//:"), + stderr_regex=r"Unknown load format `yaml`", + ) + + +@buck_test() +async def test_load_without_format_still_requires_a_known_extension(buck: Buck) -> None: + (buck.cwd / "TARGETS.fixture").write_text('load(":metadata.txt", "value")\n') + + await expect_failure( + buck.targets("root//:"), + stderr_regex=r"must have suffix `\.bzl`, `\.json`, or `\.toml`, or specify `\?format=`", + ) diff --git a/tests/core/interpreter/test_load_format_data/.buckconfig b/tests/core/interpreter/test_load_format_data/.buckconfig new file mode 100644 index 0000000000000..df06a02c03ca2 --- /dev/null +++ b/tests/core/interpreter/test_load_format_data/.buckconfig @@ -0,0 +1,5 @@ +[cells] + root = . + +[buildfile] + name = TARGETS.fixture diff --git a/tests/core/interpreter/test_load_format_data/.buckroot b/tests/core/interpreter/test_load_format_data/.buckroot new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/tests/core/interpreter/test_load_format_data/TARGETS.fixture b/tests/core/interpreter/test_load_format_data/TARGETS.fixture new file mode 100644 index 0000000000000..5e54955f3d6d0 --- /dev/null +++ b/tests/core/interpreter/test_load_format_data/TARGETS.fixture @@ -0,0 +1,10 @@ +load(":deps.lock?format=toml", lock = "value") +load(":metadata.txt?format=json", metadata = "value") +load(":rules.bzl.in?format=bzl", "greeting") + +# buildifier: disable=print +print("DATA_LOAD_TEST_OUTPUT: " + repr({ + "lock": lock, + "metadata": metadata, + "greeting": greeting, +})) diff --git a/tests/core/interpreter/test_load_format_data/deps.lock b/tests/core/interpreter/test_load_format_data/deps.lock new file mode 100644 index 0000000000000..946a5b528653c --- /dev/null +++ b/tests/core/interpreter/test_load_format_data/deps.lock @@ -0,0 +1,9 @@ +version = 3 + +[[package]] +name = "serde" +version = "1.0.0" + +[[package]] +name = "toml" +version = "0.8.0" diff --git a/tests/core/interpreter/test_load_format_data/metadata.txt b/tests/core/interpreter/test_load_format_data/metadata.txt new file mode 100644 index 0000000000000..0dcfdf2b74958 --- /dev/null +++ b/tests/core/interpreter/test_load_format_data/metadata.txt @@ -0,0 +1,4 @@ +{ + "name": "example", + "version": 1 +} diff --git a/tests/core/interpreter/test_load_format_data/rules.bzl.in b/tests/core/interpreter/test_load_format_data/rules.bzl.in new file mode 100644 index 0000000000000..c3a20e21c979d --- /dev/null +++ b/tests/core/interpreter/test_load_format_data/rules.bzl.in @@ -0,0 +1,9 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates. +# +# This source code is dual-licensed under either the MIT license found in the +# LICENSE-MIT file in the root directory of this source tree or the Apache +# License, Version 2.0 found in the LICENSE-APACHE file in the root directory +# of this source tree. You may select, at your option, one of the +# above-listed licenses. + +greeting = "hello"