Remove lazy loading of registry
This removes the need for interior mutability and makes the Project
Send + Sync.
Change-Id: I4fe3004f33e0d3502e4ea99a974603605183acdf
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/qg/+/126423
Reviewed-by: Alexei Frolov <frolv@google.com>
Commit-Queue: Erik Gilling <konkers@google.com>
diff --git a/qg-cli/src/graph.rs b/qg-cli/src/graph.rs
index d311063..13cd3cf 100644
--- a/qg-cli/src/graph.rs
+++ b/qg-cli/src/graph.rs
@@ -23,7 +23,7 @@
pub async fn run(_args: &Command) -> Result<()> {
let project = Project::locate()?;
- let graph = project.registry()?.dot_dependency_graph();
+ let graph = project.registry().dot_dependency_graph();
io::stdout().write_all(graph.as_bytes()).await?;
Ok(())
diff --git a/qg-cli/src/target.rs b/qg-cli/src/target.rs
index 07f5ccf..3b3b6a1 100644
--- a/qg-cli/src/target.rs
+++ b/qg-cli/src/target.rs
@@ -24,7 +24,7 @@
#[allow(clippy::unused_async)]
pub async fn run(_args: &Command) -> Result<()> {
let project = Project::locate()?;
- let registry = project.registry()?;
+ let registry = project.registry();
println!(
"Project {}, {} total targets",
diff --git a/qg/src/project/mod.rs b/qg/src/project/mod.rs
index c4ff0d4..35efbbf 100644
--- a/qg/src/project/mod.rs
+++ b/qg/src/project/mod.rs
@@ -13,12 +13,12 @@
// the License.
use std::{
- cell::{Ref, RefCell},
ffi::OsStr,
path::{Path, PathBuf},
+ sync::Arc,
};
-use crate::{registry::Registry, target::Provider};
+use crate::registry::Registry;
use crate::{Error, Result};
use manifest::Manifest;
@@ -34,9 +34,7 @@
root: PathBuf,
cache_dir: PathBuf,
name: String,
- // Use a RefCell to provide interior mutability on the registry so that it can
- // be lazily loaded without taking a mutable reference on the Project.
- registry: RefCell<Option<Registry>>,
+ registry: Arc<Registry>,
}
impl Project {
@@ -93,18 +91,21 @@
/// - [`Error::ProjectNotFound`]: No project manifest found in `project_root`.
pub fn load<P: AsRef<Path>>(project_root: P) -> Result<Self> {
let project_root = project_root.as_ref();
- if !project_root.join(Self::MANIFEST_FILE).exists() {
+ let manifest_path = project_root.join(Self::MANIFEST_FILE);
+ if !manifest_path.exists() {
return Err(Error::ProjectNotFound);
}
- let manifest = self::File::new(project_root.join(Self::MANIFEST_FILE))
- .deserialize_toml::<Manifest>()?;
+ let manifest_file = self::File::new(manifest_path);
+ let manifest = manifest_file.deserialize_toml::<Manifest>()?;
+
+ let registry = Registry::parse_manifests(&manifest_file)?;
Ok(Self {
root: project_root.to_owned(),
cache_dir: project_root.join(Self::CACHE_DIRECTORY),
name: manifest.project.name,
- registry: RefCell::new(None),
+ registry: Arc::new(registry),
})
}
@@ -131,17 +132,10 @@
return Err(Error::InvalidName(name.into()));
}
- let project = Self {
- root: root.to_owned(),
- cache_dir: root.join(Self::CACHE_DIRECTORY),
- name: name.into(),
- registry: RefCell::new(None),
- };
-
let manifest = Manifest::new(name);
- project.root_manifest().serialize_toml(&manifest)?;
+ Self::relative_file(root, Path::new(Self::MANIFEST_FILE))?.serialize_toml(&manifest)?;
- Ok(project)
+ Self::load(root)
}
/// Returns the root directory of the project.
@@ -174,90 +168,10 @@
Self::relative_file(&self.cache_dir, Path::new(path.as_ref()))
}
- /// Returns the target registry for this project. Will locate and parse
- /// manifests if they have not already been read.
- ///
- /// # Errors
- /// Returns an error if manifest files could not be read or are improperly
- /// structured.
- pub fn registry(&self) -> Result<Ref<Registry>> {
- {
- // Scope the mutable borrow to just the existence check and
- // manifest parsing.
- let mut registry = self.registry.borrow_mut();
- if registry.is_none() {
- *registry = Some(self.parse_manifests()?);
- }
- }
-
- Ok(Ref::map(self.registry.borrow(), |registry| {
- registry
- .as_ref()
- .expect("registry should be valid due to being created above")
- }))
- }
-
- /// Reads `qg` manifest files starting from the project root into a registry
- /// of available packages.
- ///
- /// # Errors
- /// Returns an error if manifest files could not be read or are improperly
- /// structured.
- fn parse_manifests(&self) -> Result<Registry> {
- let root_manifest_file = self.root_manifest();
- let root_manifest = root_manifest_file.deserialize_toml::<Manifest>()?;
- let mut registry = Registry::new();
-
- let project_provider = Provider::new(
- &root_manifest.project.name,
- root_manifest_file.path(),
- false,
- );
- registry.add_provider(project_provider)?;
-
- for (name, target) in root_manifest.targets {
- registry.add_target(crate::Target::from_manifest(
- &name,
- &root_manifest.project.name,
- target.namespace.is_global(),
- target,
- )?)?;
- }
-
- for (provider, desc) in &root_manifest.providers {
- if let Some(manifest) = &desc.manifest {
- let provider_file = root_manifest_file.relative_file(manifest);
-
- let provider_data = provider_file.deserialize_toml::<manifest::ProviderFile>()?;
- let global_provider = if let Some(desc) = &provider_data.provider {
- desc.namespace.is_global()
- } else {
- false
- };
-
- registry.add_provider(Provider::new(
- provider,
- provider_file.path(),
- global_provider,
- ))?;
-
- for (name, target) in provider_data.targets {
- registry.add_target(crate::Target::from_manifest(
- &name,
- provider,
- global_provider || target.namespace.is_global(),
- target,
- )?)?;
- }
- }
- }
-
- Ok(registry)
- }
-
- fn root_manifest(&self) -> self::File {
- self.file(Self::MANIFEST_FILE)
- .expect("manifest path is not absolute")
+ /// Returns the target registry for this project.
+ #[must_use]
+ pub fn registry(&self) -> Arc<Registry> {
+ self.registry.clone()
}
fn relative_file(root: &Path, path: &Path) -> Result<self::File> {
@@ -299,7 +213,7 @@
cache_dir: root.join("qg-cache"),
root,
name: "qg2".into(),
- registry: RefCell::new(None),
+ registry: Arc::new(Registry::new()),
};
assert_eq!(
@@ -323,7 +237,7 @@
cache_dir: root.join("qg-cache"),
root,
name: "qg2".into(),
- registry: RefCell::new(None),
+ registry: Arc::new(Registry::new()),
};
assert!(matches!(
@@ -364,7 +278,7 @@
let project = Project::load("./src/test_projects/dependency_test").unwrap();
assert_eq!(project.name, "dep-test");
- let registry = project.registry().unwrap();
+ let registry = project.registry();
let a_id = registry.get_node_id("dep-test:a").unwrap();
let b_id = registry.get_node_id("dep-test:b").unwrap();
let c_id = registry.get_node_id("dep-test:c").unwrap();
diff --git a/qg/src/registry.rs b/qg/src/registry.rs
index 6876f0f..b52f10e 100644
--- a/qg/src/registry.rs
+++ b/qg/src/registry.rs
@@ -24,8 +24,14 @@
Directed, Direction, Graph,
};
-use crate::target::{Provider, Target};
-use crate::{Error, Result};
+use crate::{
+ project::{
+ manifest::{self, Manifest},
+ File,
+ },
+ target::{Provider, Target},
+ Error, Result,
+};
/// A node in the dependency graph.
#[derive(Debug)]
@@ -97,6 +103,63 @@
}
}
+ /// Reads `qg` manifest files starting from the project root into a registry
+ /// of available packages.
+ ///
+ /// # Errors
+ /// Returns an error if manifest files could not be read or are improperly
+ /// structured.
+ pub fn parse_manifests(root_manifest_file: &File) -> Result<Self> {
+ let root_manifest = root_manifest_file.deserialize_toml::<Manifest>()?;
+ let mut registry = Registry::new();
+
+ let project_provider = Provider::new(
+ &root_manifest.project.name,
+ root_manifest_file.path(),
+ false,
+ );
+ registry.add_provider(project_provider)?;
+
+ for (name, target) in root_manifest.targets {
+ registry.add_target(crate::Target::from_manifest(
+ &name,
+ &root_manifest.project.name,
+ target.namespace.is_global(),
+ target,
+ )?)?;
+ }
+
+ for (provider, desc) in &root_manifest.providers {
+ if let Some(manifest) = &desc.manifest {
+ let provider_file = root_manifest_file.relative_file(manifest);
+
+ let provider_data = provider_file.deserialize_toml::<manifest::ProviderFile>()?;
+ let global_provider = if let Some(desc) = &provider_data.provider {
+ desc.namespace.is_global()
+ } else {
+ false
+ };
+
+ registry.add_provider(Provider::new(
+ provider,
+ provider_file.path(),
+ global_provider,
+ ))?;
+
+ for (name, target) in provider_data.targets {
+ registry.add_target(crate::Target::from_manifest(
+ &name,
+ provider,
+ global_provider || target.namespace.is_global(),
+ target,
+ )?)?;
+ }
+ }
+ }
+
+ Ok(registry)
+ }
+
/// Returns the number of registered targets.
#[must_use]
pub fn target_count(&self) -> usize {
