Create starlark loader crate Bug: 528225104 Change-Id: Ib96b3251e9d048deb65c4a29320b29f66a6a6964 Reviewed-on: https://gn-review.googlesource.com/c/gn/+/23801 Reviewed-by: Cole Faust <colefaust@google.com> Commit-Queue: Matt Stark <msta@google.com> Reviewed-by: Philipp Wollermann <philwo@google.com> Reviewed-by: Matt Stark <msta@google.com>
diff --git a/src/gn/starlark/Cargo.lock b/src/gn/starlark/Cargo.lock index 52ad4e0..de40a89 100644 --- a/src/gn/starlark/Cargo.lock +++ b/src/gn/starlark/Cargo.lock
@@ -850,6 +850,19 @@ checksum = "32a66949e030da00e8c7d4434b251670a91556f4144941d37452769c25d58a53" [[package]] +name = "loader" +version = "0.1.0" +dependencies = [ + "allocative", + "anyhow", + "starlark", + "starlark_derive", + "testutils", + "thiserror", + "types", +] + +[[package]] name = "lock_api" version = "0.4.14" source = "registry+https://github.com/rust-lang/crates.io-index"
diff --git a/src/gn/starlark/Cargo.toml b/src/gn/starlark/Cargo.toml index 45a0c66..4645b08 100644 --- a/src/gn/starlark/Cargo.toml +++ b/src/gn/starlark/Cargo.toml
@@ -15,6 +15,7 @@ ".", "crates/attr", "crates/ffi", + "crates/loader", "crates/testutils", "crates/types", ]
diff --git a/src/gn/starlark/crates/loader/Cargo.toml b/src/gn/starlark/crates/loader/Cargo.toml new file mode 100644 index 0000000..51998e2 --- /dev/null +++ b/src/gn/starlark/crates/loader/Cargo.toml
@@ -0,0 +1,19 @@ +[package] +name = "loader" +version = "0.1.0" +edition = "2021" + +[lints] +workspace = true + +[dependencies] +types = { path = "../types" } +starlark = { workspace = true } +starlark_derive = { workspace = true } +allocative = { workspace = true } +thiserror = { workspace = true } +anyhow = { workspace = true } + +[dev-dependencies] +testutils = { path = "../testutils" } +
diff --git a/src/gn/starlark/crates/loader/src/errors.rs b/src/gn/starlark/crates/loader/src/errors.rs new file mode 100644 index 0000000..768c0a3 --- /dev/null +++ b/src/gn/starlark/crates/loader/src/errors.rs
@@ -0,0 +1,18 @@ +// Copyright 2026 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +/// Errors returned by Starlark module loading and evaluation. +#[derive(thiserror::Error, Debug)] +pub enum Error { + #[error("Failed to read file: {0}")] + ReadFailed(String), + #[error("cycle detected: {0:?}")] + CycleDetected(Vec<String>), +} + +impl From<Error> for starlark::Error { + fn from(err: Error) -> Self { + Self::new_other(err) + } +}
diff --git a/src/gn/starlark/crates/loader/src/lib.rs b/src/gn/starlark/crates/loader/src/lib.rs new file mode 100644 index 0000000..dcc6106 --- /dev/null +++ b/src/gn/starlark/crates/loader/src/lib.rs
@@ -0,0 +1,9 @@ +// Copyright 2026 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +pub mod errors; +pub mod loader; + +pub use errors::Error; +pub use loader::FileLoader;
diff --git a/src/gn/starlark/crates/loader/src/loader.rs b/src/gn/starlark/crates/loader/src/loader.rs new file mode 100644 index 0000000..b148751 --- /dev/null +++ b/src/gn/starlark/crates/loader/src/loader.rs
@@ -0,0 +1,311 @@ +// Copyright 2026 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +use std::{ + collections::{hash_map::Entry, HashMap}, + fs, + pin::Pin, + sync::{Arc, Condvar, Mutex, RwLock}, +}; + +use starlark::{ + collections::SmallMap, + environment::{FrozenModule, Globals, Module}, + eval::{Evaluator, FileLoader as StarlarkFileLoader}, + syntax::{AstModule, Dialect}, + values::FrozenHeapName, +}; +use types::{EvalContext, EvaluatorContextExt as _, LabelRef, PackageRef, PathResolver}; + +use crate::Error; + +/// The Starlark compilation dialect used for `.bzl` configuration files. +pub const BZL_FILE_DIALECT: Dialect = Dialect { + enable_lambda: false, + enable_load_reexport: false, + ..Dialect::Standard +}; + +enum FileStatus { + Loading { + // A CondVar to wait on for the filestatus to resolve to Loaded. + wait: Arc<Condvar>, + // What needs to finish evaluating before this can be evaluated. + // This is used purely for cycle detection. + needs: Option<String>, + }, + Loaded(starlark::Result<Pin<Box<FrozenModule>>>), +} + +impl Default for FileStatus { + fn default() -> Self { + Self::Loading { + wait: Arc::new(Condvar::new()), + needs: None, + } + } +} + +/// A thread-safe loader for reading, compiling, and caching Starlark modules +/// (`.bzl` files). +#[derive(Default)] +pub struct FileLoader { + files: RwLock<HashMap<String, Arc<Mutex<FileStatus>>>>, +} + +impl FileLoader { + fn wait_for_load( + &self, + file_status: &Arc<Mutex<FileStatus>>, + ) -> starlark::Result<FrozenModule> { + let mut status = file_status.lock().unwrap(); + while let FileStatus::Loading { wait, .. } = &*status { + let wait = wait.clone(); + // Note: this temporarily releases the status mutex, so we don't + // prevent other threads from accessing it. + status = wait.wait(status).unwrap(); + } + self.get_loaded(&status) + } + + fn set_complete( + &self, + file_status: &Arc<Mutex<FileStatus>>, + result: starlark::Result<FrozenModule>, + ) -> starlark::Result<FrozenModule> { + let mut status = file_status.lock().unwrap(); + if let FileStatus::Loading { wait, .. } = &*status { + wait.notify_all(); + } + *status = FileStatus::Loaded(result.map(Box::pin)); + self.get_loaded(&status) + } + + /// Gets an already loaded file. + fn get_loaded(&self, status: &FileStatus) -> starlark::Result<FrozenModule> { + match status { + FileStatus::Loading { .. } => unreachable!(), + // Since `FrozenModule` is a cheap-to-clone handle around an Arc'd frozen heap, + // we clone and return it by value instead of using unsafe pointer casts. + FileStatus::Loaded(Ok(m)) => Ok(m.as_ref().get_ref().clone()), + // starlark::Error doesn't implement Clone, so we do a poor man's clone. + FileStatus::Loaded(Err(e)) => Err(starlark::Error::new_other(anyhow::anyhow!("{e}"))), + } + } + + /// Loads, parses, compiles, and evaluates a Starlark module, resolving + /// dependencies recursively and caching the result. + pub fn load<'b, C: EvalContext, F: Fn(&PackageRef) -> Box<C>>( + &self, + label: LabelRef<'b>, + path_resolver: &PathResolver, + globals: &Globals, + make_eval_context: &F, + ) -> starlark::Result<FrozenModule> { + // Starlark-rs requires module identifiers to be strings. + let label_str = label.to_string(); + let file_status = { + let mut loader = self.files.write().unwrap(); + match loader.entry(label_str.clone()) { + Entry::Occupied(entry) => { + let file_status = entry.get().clone(); + drop(loader); + return self.wait_for_load(&file_status); + }, + // We're about to start evaluating it. + Entry::Vacant(entry) => entry.insert(Default::default()).clone(), + } + }; + + let result = self.load_and_evaluate( + label, + &label_str, + &file_status, + path_resolver, + globals, + make_eval_context, + ); + + self.set_complete(&file_status, result) + } + + fn load_and_evaluate<'b, C: EvalContext, F: Fn(&PackageRef) -> Box<C>>( + &self, + label: LabelRef<'b>, + label_str: &str, + file_status: &Arc<Mutex<FileStatus>>, + path_resolver: &PathResolver, + globals: &Globals, + make_eval_context: &F, + ) -> starlark::Result<FrozenModule> { + // Read and parse the file to get its dependencies. + let absolute_path = path_resolver.absolute_path(label.package(), label.name()); + let content = fs::read_to_string(&absolute_path) + .map_err(|_| Error::ReadFailed(label_str.to_owned()))?; + let ast = AstModule::parse(label_str, content, &BZL_FILE_DIALECT)?; + + let mut deps: Vec<(String, FrozenModule)> = Default::default(); + if !ast.loads().is_empty() { + for load in ast.loads() { + let dep_label = types::Label::parse(load.module_id, label.package())?; + let dep_label_str = dep_label.to_string(); + if let Some(cycle) = self.find_cycle_path(file_status, &dep_label_str) { + return Err(Error::CycleDetected(cycle).into()); + } + + deps.push(( + load.module_id.to_owned(), + self.load( + dep_label.as_ref(), + path_resolver, + globals, + make_eval_context, + )?, + )); + } + } + + let deps_map: SmallMap<&str, &FrozenModule> = + deps.iter().map(|(k, v)| (k.as_str(), v)).collect(); + + let loader = PreloadedLoader { modules: &deps_map }; + Module::with_temp_heap(|module| { + let mut extra = make_eval_context(label.package()); + { + let mut eval = Evaluator::new(&module); + eval.set_context(&mut *extra); + eval.set_loader(&loader); + eval.eval_module(ast, globals)?; + } + Ok(module.freeze_named(FrozenHeapName::User(Box::new(label_str.to_owned())))?) + }) + } + + fn find_cycle_path( + &self, + current: &Arc<Mutex<FileStatus>>, + target: &str, + ) -> Option<Vec<String>> { + // Set the dependency before we start doing cycle detection. + // This prevents multiple threads simultaneously calling find_cycle_path + // not seeing a cycle on each other, then adding a dependency on each + // other after the fact. + { + let mut status = current.lock().unwrap(); + let FileStatus::Loading { ref mut needs, .. } = &mut *status else { + // find_cycle_path should only be called from the thing you're + // currently trying to load. + unreachable!(); + }; + *needs = Some(target.to_owned()); + } + let loader = self.files.read().unwrap(); + let mut cur = target.to_owned(); + let mut cycle = vec![cur.clone()]; + while let Some(status_mutex) = loader.get(&cur) { + let status = status_mutex.lock().unwrap(); + if let FileStatus::Loading { + needs: Some(need), .. + } = &*status + { + cycle.push(need.clone()); + cur = need.clone(); + } else { + break; + } + if Arc::ptr_eq(current, status_mutex) { + return Some(cycle); + } + } + None + } +} + +/// Helper loader to load preloaded dependencies during evaluator execution. +struct PreloadedLoader<'a> { + /// A map of pre-loaded Starlark modules indexed by their module path. + modules: &'a SmallMap<&'a str, &'a FrozenModule>, +} + +impl StarlarkFileLoader for PreloadedLoader<'_> { + fn load(&self, path: &str) -> starlark::Result<FrozenModule> { + match self.modules.get(path) { + Some(m) => Ok((*m).clone()), + None => unreachable!(), + } + } +} + +#[cfg(test)] +mod tests { + use testutils::FakeEvalContext; + use types::Label; + + use super::*; + + fn make_eval_context(pkg: &PackageRef) -> Box<FakeEvalContext> { + Box::new(FakeEvalContext::new(pkg.as_str())) + } + + fn load(loader: &FileLoader, label_str: &str) -> starlark::Result<FrozenModule> { + loader.load( + Label::parse(label_str, PackageRef::root()) + .unwrap() + .as_ref(), + &PathResolver::new_for_testing(), + &Globals::standard(), + &make_eval_context, + ) + } + + #[test] + fn test_simple_load() { + let loader = FileLoader::default(); + let module = load(&loader, "//load:absolute.bzl").unwrap(); + assert_eq!( + module.get("absolute").unwrap().unpack_str(), + Some("absolute") + ); + } + + #[test] + fn test_transitive_load() { + let loader = FileLoader::default(); + let module = load(&loader, "//load:root.bzl").unwrap(); + assert_eq!( + module.get("absolute_value").unwrap().unpack_str(), + Some("absolute") + ); + assert_eq!( + module.get("relative_value").unwrap().unpack_str(), + Some("relative") + ); + assert_eq!( + module + .get("relative_as_absolute_value") + .unwrap() + .unpack_str(), + Some("relative") + ); + // Any symbol imported via load should be inaccessible to transitive users. + assert!(module.get("absolute").is_err()); + } + + #[test] + fn test_cycle_detection() { + let loader = FileLoader::default(); + let err_msg = load(&loader, "//cycle:a.bzl").unwrap_err().to_string(); + assert!(err_msg.contains("cycle detected")); + } + + #[test] + fn test_load_error_caching() { + let loader = FileLoader::default(); + let first_err = load(&loader, "//load:does_not_exist.bzl").unwrap_err(); + let second_err = load(&loader, "//load:does_not_exist.bzl").unwrap_err(); + + assert!(first_err.to_string().contains("Failed to read")); + assert!(second_err.to_string().contains("Failed to read")); + } +}
diff --git a/src/gn/starlark/src/testdata/cycle/a.bzl b/src/gn/starlark/src/testdata/cycle/a.bzl new file mode 100644 index 0000000..69ec274 --- /dev/null +++ b/src/gn/starlark/src/testdata/cycle/a.bzl
@@ -0,0 +1,3 @@ +load("//cycle:b.bzl", "b") + +a = 1
diff --git a/src/gn/starlark/src/testdata/cycle/b.bzl b/src/gn/starlark/src/testdata/cycle/b.bzl new file mode 100644 index 0000000..0e1dc81 --- /dev/null +++ b/src/gn/starlark/src/testdata/cycle/b.bzl
@@ -0,0 +1,3 @@ +load("//cycle:a.bzl", "a") + +b = 1
diff --git a/src/gn/starlark/src/testdata/load/absolute.bzl b/src/gn/starlark/src/testdata/load/absolute.bzl new file mode 100644 index 0000000..d8ebc02 --- /dev/null +++ b/src/gn/starlark/src/testdata/load/absolute.bzl
@@ -0,0 +1 @@ +absolute = "absolute"
diff --git a/src/gn/starlark/src/testdata/load/relative.bzl b/src/gn/starlark/src/testdata/load/relative.bzl new file mode 100644 index 0000000..119b8c7 --- /dev/null +++ b/src/gn/starlark/src/testdata/load/relative.bzl
@@ -0,0 +1 @@ +relative = "relative"
diff --git a/src/gn/starlark/src/testdata/load/root.bzl b/src/gn/starlark/src/testdata/load/root.bzl new file mode 100644 index 0000000..c11f8f1 --- /dev/null +++ b/src/gn/starlark/src/testdata/load/root.bzl
@@ -0,0 +1,7 @@ +load(":relative.bzl", "relative") +load("//load:absolute.bzl", "absolute") +load("//load:relative.bzl", relative_as_absolute = "relative") + +absolute_value = absolute +relative_value = relative +relative_as_absolute_value = relative_as_absolute