Allow version to be specified when searching workspace

Versions can be given for each template in the workspace that define
which versions can be built from that file. Then, when running commands
like 'spk build' we can further disambiguate possible errors by
specifying an exact version to build.

Signed-off-by: Ryan Bottriell <[email protected]>

crates/spk-cli/common/src/flags.rs

@@ -738,6 +738,10 @@ impl std::str::FromStr for PackageSpecifier {
 #[derive(Args, Default, Clone)]
 pub struct Packages {
     /// The package names or yaml spec files to operate on
+    ///
+    /// Package requests may also come with a version when multiple
+    /// versions might be found in the local workspace or configured
+    /// repositories.
     #[clap(name = "PKG|SPEC_FILE")]
     pub packages: Vec<PackageSpecifier>,
 
@@ -828,7 +832,7 @@ where
             res.must_be_found();
             unreachable!()
         }
-        FindPackageTemplateResult::NoTemplateFiles | FindPackageTemplateResult::NotFound(_) => {
+        FindPackageTemplateResult::NoTemplateFiles | FindPackageTemplateResult::NotFound(..) => {
             // If couldn't find a template file, maybe there's an
             // existing package/version that's been published
             match package_name.map(AsRef::as_ref) {

crates/spk-schema/src/spec.rs

@@ -137,6 +137,7 @@ macro_rules! spec {
 
 /// A generic, structured data object that can be turned into a recipe
 /// when provided with the necessary option values
+#[derive(Debug)]
 pub struct SpecTemplate {
     name: Option<PkgNameBuf>,
     versions: HashSet<Version>,

crates/spk-workspace/Cargo.toml

@@ -32,3 +32,4 @@ tracing = { workspace = true }
 rstest = { workspace = true }
 serde_json = { workspace = true }
 tempfile = { workspace = true }
+tracing-subscriber = { workspace = true, features = ["env-filter"] }

crates/spk-workspace/src/builder.rs

@@ -8,6 +8,7 @@ use crate::error;
 
 #[derive(Default)]
 pub struct WorkspaceBuilder {
+    root: Option<std::path::PathBuf>,
     spec_files: HashMap<std::path::PathBuf, crate::file::TemplateConfig>,
 }
 
@@ -22,8 +23,9 @@ impl WorkspaceBuilder {
         self,
         dir: impl AsRef<std::path::Path>,
     ) -> Result<Self, error::FromPathError> {
-        let file = crate::file::WorkspaceFile::discover(dir)?;
-        self.load_from_file(file)
+        let (file, root) = crate::file::WorkspaceFile::discover(dir)?;
+        self.with_root(root)
+            .load_from_file(file)
             .map_err(error::FromPathError::from)
     }
 
@@ -32,9 +34,18 @@ impl WorkspaceBuilder {
         self,
         file: crate::file::WorkspaceFile,
     ) -> Result<Self, error::FromFileError> {
-        file.recipes.iter().try_fold(self, |builder, pattern| {
-            builder.with_glob_pattern(pattern.path.as_str())
-        })
+        file.recipes
+            .iter()
+            .try_fold(self, |builder, item| builder.with_recipes_item(item))
+    }
+
+    /// Specify the root directory for the workspace.
+    ///
+    /// This is the path that will be used to resolve all
+    /// relative paths and relative glob patterns.
+    pub fn with_root(mut self, root: impl Into<std::path::PathBuf>) -> Self {
+        self.root = Some(root.into());
+        self
     }
 
     /// Add all recipe files matching a glob pattern to the workspace.
@@ -45,12 +56,17 @@ impl WorkspaceBuilder {
         mut self,
         item: &crate::file::RecipesItem,
     ) -> Result<Self, error::FromFileError> {
-        let mut glob_results = glob::glob(item.path.as_str())?;
+        let with_root = self.root.as_deref().map(|p| p.join(item.path.as_str()));
+        let pattern = with_root
+            .as_deref()
+            .and_then(|p| p.to_str())
+            .unwrap_or(item.path.as_str());
+        let mut glob_results = glob::glob(pattern)?;
         while let Some(path) = glob_results.next().transpose()? {
             self.spec_files
                 .entry(path)
                 .or_default()
-                .update(&item.config);
+                .update(item.config.clone());
         }
 
         Ok(self)
@@ -62,15 +78,13 @@ impl WorkspaceBuilder {
     /// current working directory. All configuration for this path will be
     /// left as the defaults unless already set.
     pub fn with_glob_pattern<S: AsRef<str>>(
-        mut self,
+        self,
         pattern: S,
     ) -> Result<Self, error::FromFileError> {
-        let mut glob_results = glob::glob(pattern.as_ref())?;
-        while let Some(path) = glob_results.next().transpose()? {
-            self.spec_files.entry(path).or_default();
-        }
-
-        Ok(self)
+        self.with_recipes_item(&crate::file::RecipesItem {
+            path: glob::Pattern::new(pattern.as_ref())?,
+            config: Default::default(),
+        })
     }
 
     pub fn build(self) -> Result<super::Workspace, error::BuildError> {

crates/spk-workspace/src/file.rs

@@ -3,7 +3,7 @@
 // https://github.com/spkenv/spk
 
 use std::collections::BTreeSet;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::str::FromStr;
 
 use bracoxide::tokenizer::TokenizationError;
@@ -46,8 +46,9 @@ impl WorkspaceFile {
     }
 
     /// Load the workspace for a given dir, looking at parent directories
-    /// as necessary to find the workspace root
-    pub fn discover<P: AsRef<Path>>(cwd: P) -> Result<Self, LoadWorkspaceFileError> {
+    /// as necessary to find the workspace root. Returns the workspace root directory
+    /// that was found, if any.
+    pub fn discover<P: AsRef<Path>>(cwd: P) -> Result<(Self, PathBuf), LoadWorkspaceFileError> {
         let cwd = if cwd.as_ref().is_absolute() {
             cwd.as_ref().to_owned()
         } else {
@@ -60,7 +61,7 @@ impl WorkspaceFile {
         let mut candidate: std::path::PathBuf = cwd.clone();
         loop {
             if candidate.join(WorkspaceFile::FILE_NAME).is_file() {
-                return Self::load(candidate);
+                return Self::load(&candidate).map(|l| (l, candidate));
             }
             if !candidate.pop() {
                 break;
@@ -136,9 +137,14 @@ pub struct TemplateConfig {
 }
 
 impl TemplateConfig {
-    pub fn update(&mut self, other: &Self) {
-        if !other.versions.is_empty() {
-            self.versions = other.versions.clone();
+    /// Update this config with newly specified data.
+    ///
+    /// Default values in the provided `other` value do not
+    /// overwrite existing data in this instance.
+    pub fn update(&mut self, other: Self) {
+        let Self { versions } = other;
+        if !versions.is_empty() {
+            self.versions = versions;
         }
     }
 }

crates/spk-workspace/src/workspace.rs

@@ -2,13 +2,19 @@
 // SPDX-License-Identifier: Apache-2.0
 // https://github.com/spkenv/spk
 
-use std::collections::{BTreeSet, HashMap};
+use std::collections::HashMap;
+use std::str::FromStr;
 
 use spk_schema::name::{PkgName, PkgNameBuf};
+use spk_schema::version::Version;
 use spk_schema::{SpecTemplate, Template, TemplateExt};
 
 use crate::error;
 
+#[cfg(test)]
+#[path = "workspace_test.rs"]
+mod workspace_test;
+
 /// A collection of recipes and build targets.
 ///
 /// Workspaces are used to define and build many recipes
@@ -26,6 +32,7 @@ pub struct Workspace {
     pub(crate) templates: HashMap<PkgNameBuf, Vec<ConfiguredTemplate>>,
 }
 
+#[derive(Debug)]
 pub struct ConfiguredTemplate {
     pub template: SpecTemplate,
     pub config: crate::file::TemplateConfig,
@@ -42,6 +49,10 @@ impl Workspace {
             .flat_map(|(name, templates)| templates.iter().map(|t| (name.as_ref(), t)))
     }
 
+    /// Returns the default package template file for the current workspace.
+    ///
+    /// The default template in a workspace is a lone template file, and
+    /// this function will return an error if there is more than one template.
     pub fn default_package_template(&self) -> FindPackageTemplateResult<'_> {
         let mut iter = self.iter();
         // This must catch and convert all the errors into the appropriate
@@ -53,52 +64,73 @@ impl Workspace {
         };
 
         if iter.next().is_some() {
-            let files = self
-                .templates
-                .values()
-                .flat_map(|templates| templates.iter().map(|t| t.template.file_path().to_owned()))
-                .collect();
-            return FindPackageTemplateResult::MultipleTemplateFiles(files);
+            let all = self.templates.values().flatten().collect();
+            return FindPackageTemplateResult::MultipleTemplateFiles(all);
         };
 
         FindPackageTemplateResult::Found(template)
     }
 
     /// Find a package template file for the requested package, if any.
     ///
-    /// This function will use the current directory and the provided
-    /// package name or filename to try and discover the matching
-    /// yaml template file.
-    pub fn find_package_template<S>(&self, package: &S) -> FindPackageTemplateResult
+    /// A package name, name with version, or filename can be provided.
+    pub fn find_package_template<S>(&self, package: S) -> FindPackageTemplateResult
     where
         S: AsRef<str>,
     {
         let package = package.as_ref();
+        let found = if let Ok(name) = spk_schema::name::PkgName::new(package) {
+            tracing::debug!("Find package template by name: {name}");
+            self.find_package_templates(name)
+        } else if let Ok(ident) = spk_schema::VersionIdent::from_str(package) {
+            tracing::debug!("Find package template for version: {ident}");
+            self.find_package_template_for_version(ident.name(), ident.version())
+        } else {
+            tracing::debug!("Find package template by path: {package}");
+            self.find_package_template_by_file(std::path::Path::new(package))
+        };
 
-        if let Ok(name) = spk_schema::name::PkgName::new(package) {
-            match self.templates.get(name) {
-                Some(templates) if templates.len() == 1 => {
-                    return FindPackageTemplateResult::Found(&templates[0]);
-                }
-                Some(templates) => {
-                    return FindPackageTemplateResult::MultipleTemplateFiles(
-                        templates
-                            .iter()
-                            .map(|t| t.template.file_path().to_owned())
-                            .collect(),
-                    );
-                }
-                None => {}
-            }
+        if found.is_empty() {
+            return FindPackageTemplateResult::NotFound(package.to_owned());
+        }
+        if found.len() > 1 {
+            return FindPackageTemplateResult::MultipleTemplateFiles(found);
         }
+        FindPackageTemplateResult::Found(&found[0])
+    }
 
-        for entry in self.templates.values().flatten() {
-            if entry.template.file_path() == std::path::Path::new(package) {
-                return FindPackageTemplateResult::Found(entry);
-            }
+    /// Like [`Self::find_package_template`], but further filters by package version.
+    pub fn find_package_template_for_version(
+        &self,
+        package: &PkgName,
+        version: &Version,
+    ) -> Vec<&ConfiguredTemplate> {
+        self.find_package_templates(package)
+            .into_iter()
+            .filter(|t| t.config.versions.is_empty() || t.config.versions.contains(version))
+            .collect::<Vec<_>>()
+    }
+
+    /// Find a package templates for the requested package, if any.
+    ///
+    /// Either a package name or filename can be provided.
+    pub fn find_package_templates(&self, name: &PkgName) -> Vec<&ConfiguredTemplate> {
+        if let Some(templates) = self.templates.get(name) {
+            templates.iter().collect()
+        } else {
+            Default::default()
         }
+    }
 
-        FindPackageTemplateResult::NotFound(package.to_owned())
+    pub fn find_package_template_by_file(
+        &self,
+        file: &std::path::Path,
+    ) -> Vec<&ConfiguredTemplate> {
+        self.templates
+            .values()
+            .flat_map(|templates| templates.iter())
+            .filter(|t| t.template.file_path() == file)
+            .collect()
     }
 
     /// Load an additional template into this workspace from an arbitrary path on disk.
@@ -114,8 +146,8 @@ impl Workspace {
 
     /// Load an additional template into this workspace from an arbitrary path on disk.
     ///
-    /// No checks are done to ensure that this template has not already been loaded
-    /// or that it actually appears in/logically belongs in this workspace.
+    /// No checks are done to ensure that this template actually appears in or
+    /// logically belongs in this workspace.
     pub fn load_template_file_with_config<P: AsRef<std::path::Path>>(
         &mut self,
         path: P,
@@ -137,23 +169,32 @@ impl Workspace {
                 file: path.as_ref().to_owned(),
             });
         };
+        let loaded_path = template.file_path();
         let by_name = self.templates.entry(name.clone()).or_default();
-        by_name.push(ConfiguredTemplate { template, config });
-        Ok(by_name.last_mut().expect("just pushed something"))
+        let existing = by_name
+            .iter()
+            .position(|t| t.template.file_path() == loaded_path);
+        if let Some(existing) = existing {
+            by_name[existing].config.update(config);
+            Ok(&mut by_name[existing])
+        } else {
+            by_name.push(ConfiguredTemplate { template, config });
+            Ok(by_name.last_mut().expect("just pushed something"))
+        }
     }
 }
 
 /// The result of the [`Workspace::find_package_template`] function.
 // We are okay with the large variant here because it's specifically
 // used as the positive result of the function, with the others simply
 // denoting unique error cases.
-#[allow(clippy::large_enum_variant)]
+#[derive(Debug)]
 pub enum FindPackageTemplateResult<'a> {
     /// A non-ambiguous package template file was found
     Found(&'a ConfiguredTemplate),
     /// No package was specifically requested, and there are multiple
     /// files in the current repository.
-    MultipleTemplateFiles(BTreeSet<std::path::PathBuf>),
+    MultipleTemplateFiles(Vec<&'a ConfiguredTemplate>),
     /// No package was specifically requested, and there no template
     /// files in the current repository.
     NoTemplateFiles,
@@ -169,12 +210,29 @@ impl<'a> FindPackageTemplateResult<'a> {
     pub fn must_be_found(self) -> &'a ConfiguredTemplate {
         match self {
             Self::Found(template) => return template,
-            Self::MultipleTemplateFiles(files) => {
+            Self::MultipleTemplateFiles(templates) => {
+                let mut here = std::env::current_dir().unwrap_or_default();
+                here = here.canonicalize().unwrap_or(here);
                 tracing::error!("Multiple package specs in current workspace:");
-                for file in files {
-                    tracing::error!("- {}", file.into_os_string().to_string_lossy());
+                for configured in templates {
+                    // attempt to strip the current working directory from each path
+                    // because in most cases it was loaded from the active workspace
+                    // and the additional path prefix is just noise
+                    let path = configured.template.file_path();
+                    let path = path.strip_prefix(&here).unwrap_or(path).to_string_lossy();
+                    let mut versions = configured
+                        .config
+                        .versions
+                        .iter()
+                        .map(ToString::to_string)
+                        .collect::<Vec<_>>()
+                        .join(", ");
+                    if versions.is_empty() {
+                        versions.push_str("?");
+                    }
+                    tracing::error!(" - {path} versions=[{versions}]",);
                 }
-                tracing::error!(" > please specify a package name or filepath");
+                tracing::error!(" > ensure that you specify a package name, file path or version");
             }
             Self::NoTemplateFiles => {
                 tracing::error!("No package specs found in current workspace");