mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-05 08:10:39 +00:00
Simplify local RunIntent producer support
Share canonicalize_location and resolve_existing_workflow_location between the local package resolver and the version collector, drop the redundant package-root pre-check and the PackageFileReadError enum in favor of anyhow context, and read HEAD's SHA from git2 instead of a separate rev-parse subprocess. Make GitRunTargetObservation a plain struct, replace the repo-info tuple with a named struct, tighten the closure view trait, avoid deep-cloning the root workflow during collected validation, remove the unused into_closure accessor, and dedupe test helpers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
52ff7b2ef2
commit
0b46e1d735
9 changed files with 162 additions and 237 deletions
1
Cargo.lock
generated
1
Cargo.lock
generated
|
|
@ -2826,6 +2826,7 @@ dependencies = [
|
|||
"fabro-template",
|
||||
"fabro-test",
|
||||
"fabro-types",
|
||||
"fabro-util",
|
||||
"fabro-workflow",
|
||||
"fabro-workflow-version",
|
||||
"git2",
|
||||
|
|
|
|||
|
|
@ -46,8 +46,8 @@ pub fn validate_collected_workflow(
|
|||
let lowered = run_intent::lower_collected_workflow_closure(closure)?;
|
||||
let workflow = lowered
|
||||
.workflow_bundle
|
||||
.workflow(&lowered.entrypoint)
|
||||
.cloned()
|
||||
.into_workflows()
|
||||
.remove(&lowered.entrypoint)
|
||||
.ok_or_else(|| anyhow!("lowered root workflow is missing from its bundle"))?;
|
||||
let mut builder = WorkflowSettingsBuilder::new()
|
||||
.server_manifest_defaults(
|
||||
|
|
@ -160,6 +160,10 @@ dockerfile = { path = "Dockerfile" }
|
|||
root.join("root/workflow.toml")
|
||||
}
|
||||
|
||||
fn owner_input() -> HashMap<String, toml::Value> {
|
||||
HashMap::from([("owner".to_string(), toml::Value::String("Ada".to_string()))])
|
||||
}
|
||||
|
||||
fn run_overrides(goal: &str) -> RunLayer {
|
||||
RunLayer {
|
||||
goal: Some(RunGoalLayer::Inline(InterpString::parse(goal))),
|
||||
|
|
@ -172,7 +176,7 @@ dockerfile = { path = "Dockerfile" }
|
|||
let temp = tempfile::tempdir().unwrap();
|
||||
let workflow = write_complete_fixture(temp.path());
|
||||
let run = run_overrides("inline goal");
|
||||
let inputs = HashMap::from([("owner".to_string(), toml::Value::String("Ada".to_string()))]);
|
||||
let inputs = owner_input();
|
||||
let package = fabro_manifest::resolve_local_workflow_package(
|
||||
&workflow,
|
||||
temp.path(),
|
||||
|
|
@ -226,7 +230,7 @@ dockerfile = { path = "Dockerfile" }
|
|||
let present = validate_collected_workflow(
|
||||
package.closure(),
|
||||
Some(&run_overrides("resolved inline goal")),
|
||||
&HashMap::from([("owner".to_string(), toml::Value::String("Ada".to_string()))]),
|
||||
&owner_input(),
|
||||
)
|
||||
.unwrap();
|
||||
assert!(
|
||||
|
|
|
|||
|
|
@ -184,11 +184,16 @@ pub(crate) enum WorkflowClosureLoweringError {
|
|||
},
|
||||
}
|
||||
|
||||
/// Read access to a workflow-version closure, whether loaded from the store
|
||||
/// or collected from a local checkout, so both lower through one path.
|
||||
trait WorkflowClosureView {
|
||||
fn root_id(&self) -> WorkflowVersionId;
|
||||
fn root(&self) -> &WorkflowVersion;
|
||||
fn validated_root(&self) -> &ValidatedWorkflowVersion;
|
||||
fn get(&self, id: &WorkflowVersionId) -> Option<&WorkflowVersion>;
|
||||
|
||||
fn root(&self) -> &WorkflowVersion {
|
||||
self.validated_root().version()
|
||||
}
|
||||
}
|
||||
|
||||
impl WorkflowClosureView for LoadedWorkflowVersionClosure {
|
||||
|
|
@ -196,10 +201,6 @@ impl WorkflowClosureView for LoadedWorkflowVersionClosure {
|
|||
self.root_id()
|
||||
}
|
||||
|
||||
fn root(&self) -> &WorkflowVersion {
|
||||
self.root()
|
||||
}
|
||||
|
||||
fn validated_root(&self) -> &ValidatedWorkflowVersion {
|
||||
self.validated_root()
|
||||
}
|
||||
|
|
@ -211,6 +212,7 @@ impl WorkflowClosureView for LoadedWorkflowVersionClosure {
|
|||
|
||||
struct CollectedWorkflowClosureView<'a> {
|
||||
root_id: WorkflowVersionId,
|
||||
root: &'a ValidatedWorkflowVersion,
|
||||
versions: HashMap<WorkflowVersionId, &'a ValidatedWorkflowVersion>,
|
||||
}
|
||||
|
||||
|
|
@ -218,10 +220,14 @@ impl<'a> CollectedWorkflowClosureView<'a> {
|
|||
fn new(closure: &'a CollectedWorkflowClosure) -> Result<Self, WorkflowClosureLoweringError> {
|
||||
let root_id = closure.root_id();
|
||||
let versions = closure.versions().collect::<HashMap<_, _>>();
|
||||
if !versions.contains_key(&root_id) {
|
||||
return Err(WorkflowClosureLoweringError::MissingVersion { id: root_id });
|
||||
}
|
||||
Ok(Self { root_id, versions })
|
||||
let root = *versions
|
||||
.get(&root_id)
|
||||
.ok_or(WorkflowClosureLoweringError::MissingVersion { id: root_id })?;
|
||||
Ok(Self {
|
||||
root_id,
|
||||
root,
|
||||
versions,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -230,14 +236,8 @@ impl WorkflowClosureView for CollectedWorkflowClosureView<'_> {
|
|||
self.root_id
|
||||
}
|
||||
|
||||
fn root(&self) -> &WorkflowVersion {
|
||||
self.validated_root().version()
|
||||
}
|
||||
|
||||
fn validated_root(&self) -> &ValidatedWorkflowVersion {
|
||||
self.versions
|
||||
.get(&self.root_id)
|
||||
.expect("collected closure view construction verifies its root")
|
||||
self.root
|
||||
}
|
||||
|
||||
fn get(&self, id: &WorkflowVersionId) -> Option<&WorkflowVersion> {
|
||||
|
|
|
|||
|
|
@ -28,6 +28,7 @@ toml.workspace = true
|
|||
|
||||
[dev-dependencies]
|
||||
fabro-test.workspace = true
|
||||
fabro-util = { path = "../../foundation/fabro-util" }
|
||||
insta.workspace = true
|
||||
serde_json.workspace = true
|
||||
tempfile = "3"
|
||||
|
|
|
|||
|
|
@ -29,7 +29,7 @@ use fabro_types::{
|
|||
DirtyStatus, GitContext, GitHubRepositorySlug, GitRunTarget, ManifestPath, WorkflowSettings,
|
||||
};
|
||||
use fabro_workflow::git::{
|
||||
GitSyncStatus, branch_needs_push, head_sha, push_branch_noninteractive, sync_status,
|
||||
GitSyncStatus, branch_needs_push, push_branch_noninteractive, sync_status,
|
||||
};
|
||||
|
||||
pub use crate::local_workflow_package::{
|
||||
|
|
@ -217,7 +217,7 @@ pub fn build_run_manifest(input: ManifestBuildInput) -> Result<BuiltManifest> {
|
|||
|
||||
let configured_repo_origin_url = configured_repo_origin_url(&workflow_settings);
|
||||
let git = observe_git_run_target(&working_directory, configured_repo_origin_url.as_deref())
|
||||
.map(GitRunTargetObservation::into_legacy_git_context);
|
||||
.map(|observation| observation.legacy_git_context);
|
||||
let args = input.args.filter(|args| !manifest_args_is_empty(args));
|
||||
|
||||
Ok(BuiltManifest {
|
||||
|
|
@ -316,35 +316,8 @@ fn resolved_goal_to_manifest(resolved: ResolvedRunGoal) -> types::ManifestGoal {
|
|||
/// SHA and normalized-origin behavior independently.
|
||||
#[derive(Clone, Debug)]
|
||||
pub struct GitRunTargetObservation {
|
||||
run_target: Option<GitRunTarget>,
|
||||
legacy_git_context: GitContext,
|
||||
}
|
||||
|
||||
impl GitRunTargetObservation {
|
||||
#[must_use]
|
||||
pub fn run_target(&self) -> Option<&GitRunTarget> {
|
||||
self.run_target.as_ref()
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn into_run_target(self) -> Option<GitRunTarget> {
|
||||
self.run_target
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn dirty(&self) -> DirtyStatus {
|
||||
self.legacy_git_context.dirty
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn legacy_git_context(&self) -> &GitContext {
|
||||
&self.legacy_git_context
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn into_legacy_git_context(self) -> GitContext {
|
||||
self.legacy_git_context
|
||||
}
|
||||
pub run_target: Option<GitRunTarget>,
|
||||
pub legacy_git_context: GitContext,
|
||||
}
|
||||
|
||||
/// Observe Git facts without choosing an environment or a non-Git target.
|
||||
|
|
@ -357,8 +330,12 @@ pub fn observe_git_run_target(
|
|||
repo_path: &Path,
|
||||
configured_repo_origin_url: Option<&str>,
|
||||
) -> Option<GitRunTargetObservation> {
|
||||
let (origin_url, push_origin_url, branch) = detect_manifest_repo_info(repo_path)?;
|
||||
let sha = head_sha(repo_path).ok();
|
||||
let ManifestRepoInfo {
|
||||
origin_url,
|
||||
push_origin_url,
|
||||
branch,
|
||||
sha,
|
||||
} = detect_manifest_repo_info(repo_path)?;
|
||||
let dirty = match sync_status(repo_path, "origin", Some(&branch)) {
|
||||
GitSyncStatus::Dirty => DirtyStatus::Dirty,
|
||||
GitSyncStatus::Synced | GitSyncStatus::Unsynced => DirtyStatus::Clean,
|
||||
|
|
@ -382,7 +359,7 @@ pub fn observe_git_run_target(
|
|||
let run_target = github_run_target(
|
||||
&repo_origin_url,
|
||||
&branch,
|
||||
remotely_available.then(|| sha.clone()).flatten(),
|
||||
sha.clone().filter(|_| remotely_available),
|
||||
);
|
||||
Some(GitRunTargetObservation {
|
||||
run_target,
|
||||
|
|
@ -425,7 +402,17 @@ fn configured_repo_origin_url(settings: &WorkflowSettings) -> Option<String> {
|
|||
(!normalized.is_empty()).then_some(normalized)
|
||||
}
|
||||
|
||||
fn detect_manifest_repo_info(repo_path: &Path) -> Option<(Option<String>, Option<String>, String)> {
|
||||
struct ManifestRepoInfo {
|
||||
/// The `origin` URL as libgit2 reports it (after `insteadOf` rewrites).
|
||||
origin_url: Option<String>,
|
||||
/// The raw configured `remote.origin.url`, used to compare repository
|
||||
/// identity before pushing.
|
||||
push_origin_url: Option<String>,
|
||||
branch: String,
|
||||
sha: Option<String>,
|
||||
}
|
||||
|
||||
fn detect_manifest_repo_info(repo_path: &Path) -> Option<ManifestRepoInfo> {
|
||||
let repo = git2::Repository::discover(repo_path).ok()?;
|
||||
if repo.is_bare() {
|
||||
return None;
|
||||
|
|
@ -435,6 +422,7 @@ fn detect_manifest_repo_info(repo_path: &Path) -> Option<(Option<String>, Option
|
|||
return None;
|
||||
}
|
||||
let branch = head.shorthand().map(ToOwned::to_owned)?;
|
||||
let sha = head.target().map(|oid| oid.to_string());
|
||||
let origin_url = repo
|
||||
.find_remote("origin")
|
||||
.ok()
|
||||
|
|
@ -448,7 +436,12 @@ fn detect_manifest_repo_info(repo_path: &Path) -> Option<(Option<String>, Option
|
|||
.ok()
|
||||
.and_then(|config| config.get_string("remote.origin.url").ok())
|
||||
.or_else(|| origin_url.clone());
|
||||
Some((origin_url, push_origin_url, branch))
|
||||
Some(ManifestRepoInfo {
|
||||
origin_url,
|
||||
push_origin_url,
|
||||
branch,
|
||||
sha,
|
||||
})
|
||||
}
|
||||
|
||||
/// Best-effort push of the local branch so clone-based execution can see
|
||||
|
|
@ -485,12 +478,19 @@ fn push_manifest_branch_best_effort(
|
|||
/// Resolve a workflow reference and reject it when neither its config nor
|
||||
/// its graph exists on disk.
|
||||
/// A missing workflow surfaces as `fabro_config::Error::WorkflowNotFound`.
|
||||
fn resolve_existing_workflow_location(workflow: &Path, cwd: &Path) -> Result<WorkflowLocation> {
|
||||
#[expect(
|
||||
clippy::result_large_err,
|
||||
reason = "callers match on the concrete config error to classify missing workflows"
|
||||
)]
|
||||
fn resolve_existing_workflow_location(
|
||||
workflow: &Path,
|
||||
cwd: &Path,
|
||||
) -> Result<WorkflowLocation, fabro_config::Error> {
|
||||
let location = WorkflowLocation::resolve(workflow, cwd)?;
|
||||
if location.toml.is_none() && !location.graph.is_file() {
|
||||
return Err(
|
||||
fabro_config::Error::WorkflowNotFound(location.graph.display().to_string()).into(),
|
||||
);
|
||||
return Err(fabro_config::Error::WorkflowNotFound(
|
||||
location.graph.display().to_string(),
|
||||
));
|
||||
}
|
||||
Ok(location)
|
||||
}
|
||||
|
|
@ -547,6 +547,8 @@ pub(crate) mod test_fixtures {
|
|||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use fabro_workflow::git::head_sha;
|
||||
|
||||
use super::*;
|
||||
|
||||
fn test_environment_defaults() -> MergeMap<EnvironmentLayer> {
|
||||
|
|
@ -1724,14 +1726,14 @@ exit 1
|
|||
mark_origin_branch_synced(&workspace, "feature");
|
||||
|
||||
let observation = observe_git_run_target(&workspace, None).unwrap();
|
||||
let target = observation.run_target().unwrap();
|
||||
let legacy = observation.legacy_git_context();
|
||||
let target = observation.run_target.as_ref().unwrap();
|
||||
let legacy = &observation.legacy_git_context;
|
||||
|
||||
assert_eq!(target.repo, "acme/widgets");
|
||||
assert_eq!(target.branch, "feature");
|
||||
assert_eq!(target.tag, None);
|
||||
assert_eq!(target.sha, legacy.sha);
|
||||
assert_eq!(observation.dirty(), DirtyStatus::Clean);
|
||||
assert_eq!(legacy.dirty, DirtyStatus::Clean);
|
||||
assert_eq!(legacy.origin_url, "https://github.com/acme/widgets");
|
||||
}
|
||||
|
||||
|
|
@ -1751,10 +1753,10 @@ exit 1
|
|||
|
||||
let observation =
|
||||
observe_git_run_target(&workspace, Some("https://github.com/acme/widgets")).unwrap();
|
||||
let target = observation.run_target().unwrap();
|
||||
let target = observation.run_target.as_ref().unwrap();
|
||||
|
||||
assert_eq!(target.sha, observation.legacy_git_context().sha);
|
||||
assert_eq!(bare_remote_branch_sha(&bare_origin, "feature"), target.sha,);
|
||||
assert_eq!(target.sha, observation.legacy_git_context.sha);
|
||||
assert_eq!(bare_remote_branch_sha(&bare_origin, "feature"), target.sha);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
@ -1777,9 +1779,9 @@ exit 1
|
|||
let failed =
|
||||
observe_git_run_target(&failed_workspace, Some("https://github.com/acme/widgets"))
|
||||
.unwrap();
|
||||
assert_eq!(failed.run_target().unwrap().sha, None);
|
||||
assert!(failed.legacy_git_context().sha.is_some());
|
||||
assert_eq!(failed.dirty(), DirtyStatus::Clean);
|
||||
assert_eq!(failed.run_target.as_ref().unwrap().sha, None);
|
||||
assert!(failed.legacy_git_context.sha.is_some());
|
||||
assert_eq!(failed.legacy_git_context.dirty, DirtyStatus::Clean);
|
||||
|
||||
let mismatched_workspace = temp.path().join("mismatched");
|
||||
std::fs::create_dir_all(&mismatched_workspace).unwrap();
|
||||
|
|
@ -1795,7 +1797,7 @@ exit 1
|
|||
Some("https://github.com/acme/configured"),
|
||||
)
|
||||
.unwrap();
|
||||
let target = mismatched.run_target().unwrap();
|
||||
let target = mismatched.run_target.as_ref().unwrap();
|
||||
assert_eq!(target.repo, "acme/configured");
|
||||
assert_eq!(target.sha, None);
|
||||
assert_eq!(bare_remote_branch_sha(&bare_origin, "feature"), None);
|
||||
|
|
@ -1811,10 +1813,10 @@ exit 1
|
|||
std::fs::write(workspace.join("dirty.txt"), "dirty").unwrap();
|
||||
|
||||
let unsupported = observe_git_run_target(&workspace, None).unwrap();
|
||||
assert!(unsupported.run_target().is_none());
|
||||
assert_eq!(unsupported.dirty(), DirtyStatus::Dirty);
|
||||
assert!(unsupported.run_target.is_none());
|
||||
assert_eq!(unsupported.legacy_git_context.dirty, DirtyStatus::Dirty);
|
||||
assert_eq!(
|
||||
unsupported.legacy_git_context().origin_url,
|
||||
unsupported.legacy_git_context.origin_url,
|
||||
fabro_github::normalize_repo_origin_url(&bare_origin.to_string_lossy()),
|
||||
);
|
||||
|
||||
|
|
|
|||
|
|
@ -8,7 +8,9 @@ use std::path::{Path, PathBuf};
|
|||
use fabro_config::project::{WorkflowLocation, discover_project_config};
|
||||
use thiserror::Error;
|
||||
|
||||
use crate::workflow_version_collector::collect_workflow_versions_at_location;
|
||||
use crate::workflow_version_collector::{
|
||||
canonicalize_location, collect_workflow_versions_at_location,
|
||||
};
|
||||
use crate::{CollectedWorkflowClosure, WorkflowVersionCollectError};
|
||||
|
||||
/// One local workflow resolved to a canonical package root and collected
|
||||
|
|
@ -41,11 +43,6 @@ pub enum LocalWorkflowPackageError {
|
|||
#[source]
|
||||
source: std::io::Error,
|
||||
},
|
||||
#[error("local workflow package path `{path}` escapes source root `{source_root}`")]
|
||||
EscapesSourceRoot {
|
||||
path: PathBuf,
|
||||
source_root: PathBuf,
|
||||
},
|
||||
#[error("failed to collect local workflow package `{workflow}`")]
|
||||
Collect {
|
||||
workflow: PathBuf,
|
||||
|
|
@ -69,11 +66,6 @@ impl ResolvedLocalWorkflowPackage {
|
|||
pub fn closure(&self) -> &CollectedWorkflowClosure {
|
||||
&self.closure
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn into_closure(self) -> CollectedWorkflowClosure {
|
||||
self.closure
|
||||
}
|
||||
}
|
||||
|
||||
/// Resolve producer-readable workflow bytes under one stable local source
|
||||
|
|
@ -95,8 +87,12 @@ pub fn resolve_local_workflow_package(
|
|||
resolve_explicit_workflow(workflow, cwd, user_workflows_root)?
|
||||
};
|
||||
let source_root = canonicalize(&source_root)?;
|
||||
let workflow_location = canonicalize_location(location)?;
|
||||
ensure_location_is_within_root(&workflow_location, &source_root)?;
|
||||
let workflow_location = canonicalize_location(location, |path, source| {
|
||||
LocalWorkflowPackageError::Canonicalize {
|
||||
path: path.to_path_buf(),
|
||||
source,
|
||||
}
|
||||
})?;
|
||||
let closure = collect_workflow_versions_at_location(&workflow_location, &source_root, workflow)
|
||||
.map_err(|source| LocalWorkflowPackageError::Collect {
|
||||
workflow: workflow.to_path_buf(),
|
||||
|
|
@ -227,19 +223,12 @@ fn resolve_location(
|
|||
selected: &Path,
|
||||
cwd: &Path,
|
||||
) -> Result<WorkflowLocation, LocalWorkflowPackageError> {
|
||||
let location = WorkflowLocation::resolve(selected, cwd).map_err(|source| {
|
||||
crate::resolve_existing_workflow_location(selected, cwd).map_err(|source| {
|
||||
LocalWorkflowPackageError::Resolve {
|
||||
workflow: workflow.to_path_buf(),
|
||||
source,
|
||||
}
|
||||
})?;
|
||||
if location.toml.is_none() && !location.graph.is_file() {
|
||||
return Err(LocalWorkflowPackageError::Resolve {
|
||||
workflow: workflow.to_path_buf(),
|
||||
source: fabro_config::Error::WorkflowNotFound(location.graph.display().to_string()),
|
||||
});
|
||||
}
|
||||
Ok(location)
|
||||
})
|
||||
}
|
||||
|
||||
fn worktree_root(
|
||||
|
|
@ -275,38 +264,6 @@ fn canonicalize(path: &Path) -> Result<PathBuf, LocalWorkflowPackageError> {
|
|||
})
|
||||
}
|
||||
|
||||
fn canonicalize_location(
|
||||
location: WorkflowLocation,
|
||||
) -> Result<WorkflowLocation, LocalWorkflowPackageError> {
|
||||
let graph = canonicalize(&location.graph)?;
|
||||
let toml = location.toml.as_deref().map(canonicalize).transpose()?;
|
||||
let dir = graph
|
||||
.parent()
|
||||
.expect("a canonical workflow graph has a parent")
|
||||
.to_path_buf();
|
||||
Ok(WorkflowLocation {
|
||||
dir,
|
||||
graph,
|
||||
toml,
|
||||
slug: location.slug,
|
||||
})
|
||||
}
|
||||
|
||||
fn ensure_location_is_within_root(
|
||||
location: &WorkflowLocation,
|
||||
source_root: &Path,
|
||||
) -> Result<(), LocalWorkflowPackageError> {
|
||||
for path in std::iter::once(&location.graph).chain(location.toml.iter()) {
|
||||
if !path.starts_with(source_root) {
|
||||
return Err(LocalWorkflowPackageError::EscapesSourceRoot {
|
||||
path: path.clone(),
|
||||
source_root: source_root.to_path_buf(),
|
||||
});
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
#![expect(
|
||||
|
|
@ -317,6 +274,9 @@ mod tests {
|
|||
use std::fs;
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
use fabro_types::WorkflowVersion;
|
||||
use fabro_util::error::collect_chain;
|
||||
|
||||
use crate::{LocalWorkflowPackageError, resolve_local_workflow_package};
|
||||
|
||||
fn write(root: &Path, path: &str, content: &str) {
|
||||
|
|
@ -349,14 +309,12 @@ mod tests {
|
|||
.collect()
|
||||
}
|
||||
|
||||
fn error_chain(error: &dyn std::error::Error) -> String {
|
||||
let mut messages = vec![error.to_string()];
|
||||
let mut source = error.source();
|
||||
while let Some(error) = source {
|
||||
messages.push(error.to_string());
|
||||
source = error.source();
|
||||
}
|
||||
messages.join(": ")
|
||||
fn error_chain(error: &(dyn std::error::Error + 'static)) -> String {
|
||||
collect_chain(error).join(": ")
|
||||
}
|
||||
|
||||
fn root_version(package: &crate::ResolvedLocalWorkflowPackage) -> &WorkflowVersion {
|
||||
package.closure().versions().last().unwrap().1.version()
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
@ -381,15 +339,7 @@ mod tests {
|
|||
.unwrap(),
|
||||
);
|
||||
assert_eq!(
|
||||
package
|
||||
.closure()
|
||||
.versions()
|
||||
.last()
|
||||
.unwrap()
|
||||
.1
|
||||
.version()
|
||||
.entrypoint()
|
||||
.as_str(),
|
||||
root_version(&package).entrypoint().as_str(),
|
||||
".fabro/workflows/hello/workflow.fabro",
|
||||
);
|
||||
}
|
||||
|
|
@ -406,15 +356,7 @@ mod tests {
|
|||
resolve_local_workflow_package(Path::new("hello"), &cwd, Some(&user)).unwrap();
|
||||
assert_eq!(package.source_root(), user.canonicalize().unwrap());
|
||||
assert_eq!(
|
||||
package
|
||||
.closure()
|
||||
.versions()
|
||||
.last()
|
||||
.unwrap()
|
||||
.1
|
||||
.version()
|
||||
.entrypoint()
|
||||
.as_str(),
|
||||
root_version(&package).entrypoint().as_str(),
|
||||
"hello/workflow.fabro",
|
||||
);
|
||||
|
||||
|
|
@ -474,13 +416,7 @@ mod tests {
|
|||
|
||||
assert_eq!(package.source_root(), package_root.canonicalize().unwrap());
|
||||
assert!(
|
||||
package
|
||||
.closure()
|
||||
.versions()
|
||||
.last()
|
||||
.unwrap()
|
||||
.1
|
||||
.version()
|
||||
root_version(&package)
|
||||
.files()
|
||||
.keys()
|
||||
.any(|path| path.as_str() == "shared.md"),
|
||||
|
|
@ -503,8 +439,12 @@ mod tests {
|
|||
package.source_root(),
|
||||
temp.path().join("loose").canonicalize().unwrap(),
|
||||
);
|
||||
let root = package.closure().versions().last().unwrap().1.version();
|
||||
assert!(root.files().keys().any(|path| path.as_str() == "prompt.md"));
|
||||
assert!(
|
||||
root_version(&package)
|
||||
.files()
|
||||
.keys()
|
||||
.any(|path| path.as_str() == "prompt.md")
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
|
|
@ -581,10 +521,8 @@ mod tests {
|
|||
|
||||
let error = resolve_local_workflow_package(Path::new("hello"), &project, None).unwrap_err();
|
||||
|
||||
assert!(matches!(
|
||||
error,
|
||||
LocalWorkflowPackageError::EscapesSourceRoot { .. }
|
||||
));
|
||||
assert!(matches!(error, LocalWorkflowPackageError::Collect { .. }));
|
||||
assert!(error_chain(&error).contains("escapes source root"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ use std::collections::{BTreeSet, HashMap, HashSet};
|
|||
use std::path::{Component, Path, PathBuf};
|
||||
use std::sync::Arc;
|
||||
|
||||
use anyhow::{Context as _, Result, anyhow};
|
||||
use anyhow::{Context as _, Result, anyhow, bail};
|
||||
use fabro_api::types;
|
||||
use fabro_config::project::WorkflowLocation;
|
||||
use fabro_config::{
|
||||
|
|
@ -16,7 +16,6 @@ use fabro_template::{
|
|||
};
|
||||
use fabro_types::ManifestPath;
|
||||
use fabro_types::graph::ReferenceKind;
|
||||
use thiserror::Error;
|
||||
|
||||
use crate::{manifest_path_from_absolute, normalize_absolute_path};
|
||||
|
||||
|
|
@ -481,50 +480,28 @@ impl<'a> WorkflowBundler<'a> {
|
|||
return std::fs::read_to_string(path)
|
||||
.with_context(|| format!("Failed to read {}", path.display()));
|
||||
}
|
||||
let canonical =
|
||||
path.canonicalize()
|
||||
.map_err(|source| PackageFileReadError::Canonicalize {
|
||||
path: path.to_path_buf(),
|
||||
source,
|
||||
})?;
|
||||
let canonical = path.canonicalize().with_context(|| {
|
||||
format!(
|
||||
"failed to canonicalize workflow package file `{}`",
|
||||
path.display()
|
||||
)
|
||||
})?;
|
||||
if !canonical.starts_with(self.package_root) {
|
||||
return Err(PackageFileReadError::EscapesSourceRoot {
|
||||
path: path.to_path_buf(),
|
||||
source_root: self.package_root.to_path_buf(),
|
||||
}
|
||||
.into());
|
||||
bail!(
|
||||
"workflow package file `{}` escapes source root `{}`",
|
||||
path.display(),
|
||||
self.package_root.display()
|
||||
);
|
||||
}
|
||||
std::fs::read_to_string(&canonical).map_err(|source| {
|
||||
PackageFileReadError::Read {
|
||||
path: canonical,
|
||||
source,
|
||||
}
|
||||
.into()
|
||||
std::fs::read_to_string(&canonical).with_context(|| {
|
||||
format!(
|
||||
"failed to read workflow package file `{}`",
|
||||
canonical.display()
|
||||
)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Error)]
|
||||
enum PackageFileReadError {
|
||||
#[error("failed to canonicalize workflow package file `{path}`")]
|
||||
Canonicalize {
|
||||
path: PathBuf,
|
||||
#[source]
|
||||
source: std::io::Error,
|
||||
},
|
||||
#[error("workflow package file `{path}` escapes source root `{source_root}`")]
|
||||
EscapesSourceRoot {
|
||||
path: PathBuf,
|
||||
source_root: PathBuf,
|
||||
},
|
||||
#[error("failed to read workflow package file `{path}`")]
|
||||
Read {
|
||||
path: PathBuf,
|
||||
#[source]
|
||||
source: std::io::Error,
|
||||
},
|
||||
}
|
||||
|
||||
struct WorkflowScanInput {
|
||||
absolute_dot_path: PathBuf,
|
||||
dot_path: ManifestPath,
|
||||
|
|
|
|||
|
|
@ -84,7 +84,19 @@ pub fn collect_workflow_versions(
|
|||
) -> Result<CollectedWorkflowClosure, WorkflowVersionCollectError> {
|
||||
let repository_workflow = repository_workflow_path(workflow);
|
||||
let location = crate::resolve_existing_workflow_location(&repository_workflow, checkout_root)
|
||||
.map_err(|source| location_error(workflow, source))?;
|
||||
.map_err(|source| {
|
||||
match source {
|
||||
fabro_config::Error::WorkflowNotFound(_) => {
|
||||
WorkflowVersionCollectError::WorkflowNotFound {
|
||||
path: workflow.to_path_buf(),
|
||||
}
|
||||
}
|
||||
source => WorkflowVersionCollectError::Collect {
|
||||
path: workflow.to_path_buf(),
|
||||
source: source.into(),
|
||||
},
|
||||
}
|
||||
})?;
|
||||
|
||||
let package_root =
|
||||
checkout_root
|
||||
|
|
@ -96,24 +108,25 @@ pub fn collect_workflow_versions(
|
|||
checkout_root.display()
|
||||
)),
|
||||
})?;
|
||||
let location = canonicalize_location(location, workflow)?;
|
||||
let location = canonicalize_location(location, |path, source| {
|
||||
WorkflowVersionCollectError::Collect {
|
||||
path: workflow.to_path_buf(),
|
||||
source: anyhow::Error::new(source).context(format!(
|
||||
"failed to canonicalize workflow path {}",
|
||||
path.display()
|
||||
)),
|
||||
}
|
||||
})?;
|
||||
collect_workflow_versions_at_location(&location, &package_root, workflow)
|
||||
}
|
||||
|
||||
fn canonicalize_location(
|
||||
/// Canonicalize a resolved workflow location so its paths compare against a
|
||||
/// canonical package root. `map_err` receives the path that failed.
|
||||
pub(super) fn canonicalize_location<E>(
|
||||
location: WorkflowLocation,
|
||||
workflow: &Path,
|
||||
) -> Result<WorkflowLocation, WorkflowVersionCollectError> {
|
||||
let canonicalize = |path: &Path| {
|
||||
path.canonicalize()
|
||||
.map_err(|source| WorkflowVersionCollectError::Collect {
|
||||
path: workflow.to_path_buf(),
|
||||
source: anyhow::Error::new(source).context(format!(
|
||||
"failed to canonicalize workflow path {}",
|
||||
path.display()
|
||||
)),
|
||||
})
|
||||
};
|
||||
map_err: impl Fn(&Path, std::io::Error) -> E,
|
||||
) -> Result<WorkflowLocation, E> {
|
||||
let canonicalize = |path: &Path| path.canonicalize().map_err(|source| map_err(path, source));
|
||||
let graph = canonicalize(&location.graph)?;
|
||||
let toml = location.toml.as_deref().map(canonicalize).transpose()?;
|
||||
let dir = graph
|
||||
|
|
@ -153,22 +166,6 @@ fn repository_workflow_path(workflow: &Path) -> PathBuf {
|
|||
}
|
||||
}
|
||||
|
||||
fn location_error(workflow: &Path, source: anyhow::Error) -> WorkflowVersionCollectError {
|
||||
if matches!(
|
||||
source.downcast_ref::<fabro_config::Error>(),
|
||||
Some(fabro_config::Error::WorkflowNotFound(_))
|
||||
) {
|
||||
WorkflowVersionCollectError::WorkflowNotFound {
|
||||
path: workflow.to_path_buf(),
|
||||
}
|
||||
} else {
|
||||
WorkflowVersionCollectError::Collect {
|
||||
path: workflow.to_path_buf(),
|
||||
source,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
struct VersionAssembler {
|
||||
root_key: String,
|
||||
/// Sources still waiting to be assembled; each is removed once visited.
|
||||
|
|
|
|||
|
|
@ -61,6 +61,11 @@ impl WorkflowBundle {
|
|||
pub fn workflows(&self) -> &HashMap<ManifestPath, BundledWorkflow> {
|
||||
&self.workflows
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn into_workflows(self) -> HashMap<ManifestPath, BundledWorkflow> {
|
||||
self.workflows
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, Serialize, Deserialize)]
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue