mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-14 23:22:51 +00:00
Preserve shared target inference with explicit CLI targets
This commit is contained in:
parent
a9f28828a6
commit
94c7159ef8
4 changed files with 126 additions and 96 deletions
|
|
@ -181,6 +181,10 @@ Temporary files are removed after collection or failure. Interrupting acquisitio
|
|||
stops owned Git processes before cleanup; collection already in progress must
|
||||
finish before its files can be removed.
|
||||
|
||||
For local workflows without target flags, the existing `run.scm` repository
|
||||
configuration still participates in target inference. Explicit `--target-path`
|
||||
and `--target-git` selections take precedence over that inferred repository.
|
||||
|
||||
Target selection depends on the environment:
|
||||
|
||||
| Selection | Local environment | Clone-based environment (Docker, Daytona, or plugin) |
|
||||
|
|
|
|||
|
|
@ -96,10 +96,21 @@ pub(crate) async fn create_run(
|
|||
Some(package) => package,
|
||||
None => resolve_workflow().await?,
|
||||
};
|
||||
// Preserve configured repository inference for the existing local workflow
|
||||
// path. Explicit targets select their own repository independently.
|
||||
let configured_repo_origin_url = match &package {
|
||||
ResolvedWorkflow::Local(package)
|
||||
if args.target_path.is_none() && args.target_git.is_none() =>
|
||||
{
|
||||
fabro_manifest::configured_repo_origin_url_for_location(package.workflow_location())?
|
||||
}
|
||||
_ => None,
|
||||
};
|
||||
let (target, dirty_worktree) = resolution::target(
|
||||
&target_selection,
|
||||
&environment.settings.provider,
|
||||
&canonical_cwd,
|
||||
configured_repo_origin_url.as_deref(),
|
||||
interruption,
|
||||
)
|
||||
.await?;
|
||||
|
|
|
|||
|
|
@ -1,15 +1,16 @@
|
|||
use std::path::Path;
|
||||
|
||||
use anyhow::{Context as _, anyhow, bail};
|
||||
use anyhow::{Context as _, bail};
|
||||
use fabro_manifest::{CollectedWorkflowClosure, ResolvedLocalWorkflowPackage};
|
||||
use fabro_types::{DirtyStatus, RunTarget, SandboxProviderKind};
|
||||
use fabro_types::{RunTarget, SandboxProviderKind};
|
||||
use tokio::task;
|
||||
|
||||
use super::remote_workflow::{Interruption, NativeGit};
|
||||
use super::selection::{TargetSelection, WorkflowSelection};
|
||||
|
||||
/// Owns the canonical collector result without copying its contents. Local
|
||||
/// location metadata remains available solely for existing settings warnings.
|
||||
/// location metadata remains available for settings warnings and target
|
||||
/// inference.
|
||||
pub(super) enum ResolvedWorkflow {
|
||||
Local(ResolvedLocalWorkflowPackage),
|
||||
Git(CollectedWorkflowClosure),
|
||||
|
|
@ -71,6 +72,7 @@ pub(super) async fn target(
|
|||
selection: &TargetSelection,
|
||||
provider: &SandboxProviderKind,
|
||||
cwd: &Path,
|
||||
configured_repo_origin_url: Option<&str>,
|
||||
interruption: &Interruption,
|
||||
) -> anyhow::Result<(RunTarget, bool)> {
|
||||
let path = match selection {
|
||||
|
|
@ -99,92 +101,17 @@ pub(super) async fn target(
|
|||
// The existing observer can push/query Git synchronously. Preserve its
|
||||
// behavior without blocking a Tokio worker or promising a new timeout.
|
||||
let provider = provider.clone();
|
||||
task::spawn_blocking(move || run_target_for_environment(&provider, &path))
|
||||
.await
|
||||
.context("target observation task failed")?
|
||||
}
|
||||
|
||||
/// Derives the run target from the selected directory for the environment's
|
||||
/// provider. Returns the target plus whether a clone-based observation found a
|
||||
/// dirty Git worktree, so the caller can warn about it.
|
||||
fn run_target_for_environment(
|
||||
provider: &SandboxProviderKind,
|
||||
canonical_cwd: &Path,
|
||||
) -> anyhow::Result<(RunTarget, bool)> {
|
||||
if !provider.clones_workspace() {
|
||||
let path = canonical_cwd.to_str().ok_or_else(|| {
|
||||
anyhow!(
|
||||
"target directory is not valid UTF-8: {}",
|
||||
canonical_cwd.display()
|
||||
)
|
||||
})?;
|
||||
return Ok((
|
||||
RunTarget::Folder {
|
||||
path: path.to_string(),
|
||||
},
|
||||
false,
|
||||
));
|
||||
}
|
||||
let Some(observation) = fabro_manifest::observe_git_run_target(canonical_cwd, None) else {
|
||||
return Ok((none_target_for_unversioned_directory(canonical_cwd)?, false));
|
||||
};
|
||||
let dirty = observation.legacy_git_context.dirty == DirtyStatus::Dirty;
|
||||
let target = observation.run_target.ok_or_else(|| {
|
||||
anyhow!("the target Git checkout cannot be represented as a canonical GitHub run target")
|
||||
})?;
|
||||
if target.sha.is_none() {
|
||||
bail!(
|
||||
"the exact local Git commit could not be made available from the canonical GitHub origin; push the commit and try again"
|
||||
);
|
||||
}
|
||||
Ok((RunTarget::Git(target), dirty))
|
||||
}
|
||||
|
||||
fn none_target_for_unversioned_directory(canonical_cwd: &Path) -> anyhow::Result<RunTarget> {
|
||||
let repository = match git2::Repository::discover(canonical_cwd) {
|
||||
Ok(repository) => repository,
|
||||
Err(source) if source.code() == git2::ErrorCode::NotFound => return Ok(RunTarget::None {}),
|
||||
Err(source) => {
|
||||
return Err(anyhow::Error::new(source)).with_context(|| {
|
||||
format!(
|
||||
"failed to inspect target directory {} for Git metadata",
|
||||
canonical_cwd.display()
|
||||
)
|
||||
});
|
||||
}
|
||||
};
|
||||
|
||||
if repository.is_bare() {
|
||||
bail!(
|
||||
"the target directory resolves to a bare Git repository; clone-based runs require a non-bare checkout with an attached branch"
|
||||
);
|
||||
}
|
||||
match repository.head() {
|
||||
Err(source)
|
||||
if matches!(
|
||||
source.code(),
|
||||
git2::ErrorCode::UnbornBranch | git2::ErrorCode::NotFound
|
||||
) =>
|
||||
{
|
||||
bail!(
|
||||
"the target Git checkout has no commits; create a commit before using a clone-based environment"
|
||||
);
|
||||
}
|
||||
Err(source) => {
|
||||
return Err(anyhow::Error::new(source))
|
||||
.context("failed to inspect the target Git checkout HEAD");
|
||||
}
|
||||
Ok(head) if !head.is_branch() => {
|
||||
bail!(
|
||||
"the target Git checkout has a detached HEAD; check out a branch before using a clone-based environment"
|
||||
);
|
||||
}
|
||||
Ok(_) => {}
|
||||
}
|
||||
|
||||
bail!(
|
||||
"the target Git checkout does not have a usable attached branch for a clone-based run target"
|
||||
)
|
||||
let configured_repo_origin_url = configured_repo_origin_url.map(str::to_owned);
|
||||
let derived = task::spawn_blocking(move || {
|
||||
fabro_manifest::derive_run_target_for_provider(
|
||||
&provider,
|
||||
&path,
|
||||
configured_repo_origin_url.as_deref(),
|
||||
)
|
||||
})
|
||||
.await
|
||||
.context("target observation task failed")??;
|
||||
Ok((derived.target, derived.dirty_worktree))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
|
@ -205,10 +132,16 @@ mod tests {
|
|||
let selected = TargetSelection::Path("target".into());
|
||||
let interruption = Interruption::new(false);
|
||||
assert_eq!(
|
||||
target(&selected, &SandboxProviderKind::LOCAL, &root, &interruption)
|
||||
.await
|
||||
.unwrap()
|
||||
.0,
|
||||
target(
|
||||
&selected,
|
||||
&SandboxProviderKind::LOCAL,
|
||||
&root,
|
||||
None,
|
||||
&interruption
|
||||
)
|
||||
.await
|
||||
.unwrap()
|
||||
.0,
|
||||
RunTarget::Folder {
|
||||
path: root.join("target").to_str().unwrap().into(),
|
||||
}
|
||||
|
|
@ -219,7 +152,7 @@ mod tests {
|
|||
SandboxProviderKind::try_new("host").unwrap(),
|
||||
] {
|
||||
assert_eq!(
|
||||
target(&selected, &provider, &root, &interruption)
|
||||
target(&selected, &provider, &root, None, &interruption)
|
||||
.await
|
||||
.unwrap()
|
||||
.0,
|
||||
|
|
@ -231,6 +164,7 @@ mod tests {
|
|||
&TargetSelection::Path(".".into()),
|
||||
&SandboxProviderKind::LOCAL,
|
||||
&root,
|
||||
None,
|
||||
&interruption
|
||||
)
|
||||
.await
|
||||
|
|
@ -248,6 +182,7 @@ mod tests {
|
|||
},
|
||||
&SandboxProviderKind::LOCAL,
|
||||
&root,
|
||||
None,
|
||||
&interruption
|
||||
)
|
||||
.await
|
||||
|
|
@ -261,6 +196,7 @@ mod tests {
|
|||
&TargetSelection::Path(path.into()),
|
||||
&SandboxProviderKind::LOCAL,
|
||||
&root,
|
||||
None,
|
||||
&interruption
|
||||
)
|
||||
.await
|
||||
|
|
|
|||
|
|
@ -675,6 +675,85 @@ fn create_preserves_named_user_other_checkout_and_loose_file_selection() {
|
|||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn create_preserves_configured_repository_inference_but_explicit_target_path_wins() {
|
||||
let context = test_context!();
|
||||
let server = MockServer::start();
|
||||
let environment = mock_environment(&server, "default", "docker");
|
||||
let versions = mock_workflow_version_registrations(&server);
|
||||
let requests = Arc::new(Mutex::new(Vec::new()));
|
||||
let create = mock_intent_create(&server, &unique_run_id(), Arc::clone(&requests));
|
||||
let root = tempfile::tempdir().unwrap();
|
||||
let checkout = root.path().join("checkout");
|
||||
let workflow = write_workflow(&checkout, "workflow", "ConfiguredRepository");
|
||||
std::fs::write(&workflow, "_version = 1\n[workflow]\ngraph = \"workflow.fabro\"\n[run.scm]\nowner = \"acme\"\nrepository = \"configured\"\n").unwrap();
|
||||
let sha = init_remote_fixture(&checkout, "topic");
|
||||
let origin = root.path().join("origin.git");
|
||||
let bare = git2::Repository::init_bare(&origin).unwrap();
|
||||
run_git(&checkout, &[
|
||||
"remote",
|
||||
"add",
|
||||
"origin",
|
||||
"https://github.com/acme/actual.git",
|
||||
]);
|
||||
run_git(&checkout, &[
|
||||
"remote",
|
||||
"set-url",
|
||||
"--push",
|
||||
"origin",
|
||||
&format!("file://{}", origin.display()),
|
||||
]);
|
||||
let server_url = format!("{}/api/v1", server.base_url());
|
||||
let inferred = context
|
||||
.create_cmd()
|
||||
.current_dir(&checkout)
|
||||
.args(["--server", &server_url, workflow.to_str().unwrap()])
|
||||
.output()
|
||||
.unwrap();
|
||||
assert!(!inferred.status.success());
|
||||
assert!(
|
||||
output_stderr(&inferred)
|
||||
.contains("run.scm repository that is not the local checkout's origin")
|
||||
);
|
||||
assert!(
|
||||
bare.find_reference("refs/heads/topic").is_err(),
|
||||
"rejected inference must not publish the branch"
|
||||
);
|
||||
versions.assert_calls(0);
|
||||
create.assert_calls(0);
|
||||
|
||||
let explicit = context
|
||||
.create_cmd()
|
||||
.current_dir(&checkout)
|
||||
.args([
|
||||
"--server",
|
||||
&server_url,
|
||||
workflow.to_str().unwrap(),
|
||||
"--target-path",
|
||||
".",
|
||||
])
|
||||
.output()
|
||||
.unwrap();
|
||||
assert!(explicit.status.success(), "{}", output_stderr(&explicit));
|
||||
assert_eq!(
|
||||
requests.lock().unwrap()[0]["target"],
|
||||
json!({
|
||||
"kind": "git", "repo": "acme/actual", "branch": "topic", "sha": sha
|
||||
})
|
||||
);
|
||||
assert_eq!(
|
||||
bare.find_reference("refs/heads/topic")
|
||||
.unwrap()
|
||||
.target()
|
||||
.unwrap()
|
||||
.to_string(),
|
||||
sha
|
||||
);
|
||||
environment.assert_calls(2);
|
||||
versions.assert_calls(1);
|
||||
create.assert_calls(1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn create_clone_targets_require_exact_git_observations() {
|
||||
let context = test_context!();
|
||||
|
|
@ -896,9 +975,9 @@ fn create_rejects_unusable_git_checkouts_instead_of_sending_an_empty_target() {
|
|||
for (working_directory, expected_error) in [
|
||||
(
|
||||
detached.path(),
|
||||
"the target Git checkout has a detached HEAD",
|
||||
"the caller Git checkout has a detached HEAD",
|
||||
),
|
||||
(unborn.path(), "the target Git checkout has no commits"),
|
||||
(unborn.path(), "the caller Git checkout has no commits"),
|
||||
] {
|
||||
let output = context
|
||||
.create_cmd()
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue