From ed2c11131acfce4a05eb07f7f47aa76cb47a109d Mon Sep 17 00:00:00 2001 From: Waleed Khan Date: Fri, 10 Mar 2023 12:11:04 -0800 Subject: [PATCH 1/8] cleanup(build): sort some dependencies --- git-branchless-lib/Cargo.toml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/git-branchless-lib/Cargo.toml b/git-branchless-lib/Cargo.toml index 6212f0c58..2056e909a 100644 --- a/git-branchless-lib/Cargo.toml +++ b/git-branchless-lib/Cargo.toml @@ -44,6 +44,8 @@ test = true [dependencies] anyhow = "1.0.69" assert_cmd = "2.0.7" +async-trait = "0.1.64" +bstr = "1.3.0" chashmap = "2.2.2" chrono = "0.4.19" color-eyre = "0.6.2" @@ -52,28 +54,26 @@ console = "0.15.5" cursive = { version = "0.20.0", default-features = false } eden_dag = { package = "esl01-dag", version = "0.3.0" } eyre = "0.6.8" -git2 = { version = "0.16.1", default-features = false } +futures = "0.3.26" git-record = { version = "0.3", path = "../git-record" } +git2 = { version = "0.16.1", default-features = false } indicatif = { version = "0.17.3", features = ["improved_unicode"] } itertools = "0.10.3" lazy_static = "1.4.0" once_cell = "1.17.1" +portable-pty = "0.7.0" rayon = "1.6.1" regex = "1.7.1" rusqlite = { version = "0.28.0", features = ["bundled"] } +serde = { version = "1.0.152", features = ["derive"] } tempfile = "3.4.0" textwrap = "0.16.0" +thiserror = "1.0.39" tracing = "0.1.37" tracing-chrome = "0.6.0" tracing-error = "0.2.0" tracing-subscriber = { version = "=0.3.11", features = ["env-filter"] } -thiserror = "1.0.39" -bstr = "1.3.0" -serde = { version = "1.0.152", features = ["derive"] } -portable-pty = "0.7.0" vt100 = "0.15.2" -async-trait = "0.1.64" -futures = "0.3.26" [dev-dependencies] criterion = { version = "0.4.0", features = ["html_reports"] } From 84d586e22b768e47ad61460d4d3d6473ce2898e6 Mon Sep 17 00:00:00 2001 From: Waleed Khan Date: Mon, 13 Feb 2023 19:43:53 -0500 Subject: [PATCH 2/8] cleanup(submit): enable standard lint diagnostics for `git-branchless-submit` --- git-branchless-submit/src/lib.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/git-branchless-submit/src/lib.rs b/git-branchless-submit/src/lib.rs index 1aa69bf12..b90189eca 100644 --- a/git-branchless-submit/src/lib.rs +++ b/git-branchless-submit/src/lib.rs @@ -1,3 +1,14 @@ +//! Push the user's commits to a remote. + +#![warn(missing_docs)] +#![warn( + clippy::all, + clippy::as_conversions, + clippy::clone_on_ref_ptr, + clippy::dbg_macro +)] +#![allow(clippy::too_many_arguments, clippy::blocks_in_if_conditions)] + use std::fmt::Write; use std::time::SystemTime; @@ -23,6 +34,7 @@ lazy_static! { Style::merge(&[BaseColor::Yellow.light().into(), Effect::Bold.into()]); } +/// `submit` command. pub fn command_main(ctx: CommandContext, args: SubmitArgs) -> eyre::Result { let CommandContext { effects, From fb22b0adc38cce7fa583cc128c7c3626975bc9df Mon Sep 17 00:00:00 2001 From: Waleed Khan Date: Fri, 10 Mar 2023 15:19:40 -0500 Subject: [PATCH 3/8] feat(submit): create `BranchPushForge` The idea being to create other "forges", representing Git hosting providers, that can be pushed to in ways that support code review workflows. --- Cargo.lock | 1 + git-branchless-lib/src/core/repo_ext.rs | 36 +- git-branchless-submit/Cargo.toml | 1 + .../src/branch_push_forge.rs | 293 +++++++++++++++ git-branchless-submit/src/lib.rs | 334 ++++++++---------- git-branchless-submit/tests/test_submit.rs | 8 +- 6 files changed, 481 insertions(+), 192 deletions(-) create mode 100644 git-branchless-submit/src/branch_push_forge.rs diff --git a/Cargo.lock b/Cargo.lock index 28a263d7d..3764bb811 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1557,6 +1557,7 @@ dependencies = [ "insta", "itertools", "lazy_static", + "tracing", ] [[package]] diff --git a/git-branchless-lib/src/core/repo_ext.rs b/git-branchless-lib/src/core/repo_ext.rs index 11e78a988..6d102d17f 100644 --- a/git-branchless-lib/src/core/repo_ext.rs +++ b/git-branchless-lib/src/core/repo_ext.rs @@ -6,7 +6,9 @@ use color_eyre::Help; use eyre::Context; use tracing::instrument; -use crate::git::{Branch, BranchType, NonZeroOid, ReferenceName, Repo}; +use crate::git::{ + Branch, BranchType, CategorizedReferenceName, ConfigRead, NonZeroOid, ReferenceName, Repo, +}; use super::config::get_main_branch_name; @@ -39,6 +41,9 @@ pub trait RepoExt { /// Get the positions of references in the repository. fn get_references_snapshot(&self) -> eyre::Result; + + /// Get the default remote to push to for new branches in this repository. + fn get_default_push_remote(&self) -> eyre::Result>; } impl RepoExt for Repo { @@ -119,4 +124,33 @@ https://github.com/arxanas/git-branchless/discussions/595 for more details.", branch_oid_to_names, }) } + + fn get_default_push_remote(&self) -> eyre::Result> { + let main_branch_name = self.get_main_branch()?.get_reference_name()?; + match CategorizedReferenceName::new(&main_branch_name) { + name @ CategorizedReferenceName::LocalBranch { .. } => { + if let Some(main_branch) = + self.find_branch(&name.remove_prefix()?, BranchType::Local)? + { + if let Some(remote_name) = main_branch.get_push_remote_name()? { + return Ok(Some(remote_name)); + } + } + } + + name @ CategorizedReferenceName::RemoteBranch { .. } => { + let name = name.remove_prefix()?; + if let Some((remote_name, _reference_name)) = name.split_once('/') { + return Ok(Some(remote_name.to_owned())); + } + } + + CategorizedReferenceName::OtherRef { .. } => { + // Do nothing. + } + } + + let push_default_remote_opt = self.get_readonly_config()?.get("remote.pushDefault")?; + Ok(push_default_remote_opt) + } } diff --git a/git-branchless-submit/Cargo.toml b/git-branchless-submit/Cargo.toml index 91103ed6d..c9d2fbf68 100644 --- a/git-branchless-submit/Cargo.toml +++ b/git-branchless-submit/Cargo.toml @@ -17,6 +17,7 @@ git-branchless-revset = { version = "0.7.0", path = "../git-branchless-revset" } itertools = "0.10.5" lazy_static = "1.4.0" lib = { package = "git-branchless-lib", version = "0.7.0", path = "../git-branchless-lib" } +tracing = "0.1.37" [dev-dependencies] insta = "1.28.0" diff --git a/git-branchless-submit/src/branch_push_forge.rs b/git-branchless-submit/src/branch_push_forge.rs new file mode 100644 index 000000000..90ea0fcfa --- /dev/null +++ b/git-branchless-submit/src/branch_push_forge.rs @@ -0,0 +1,293 @@ +use std::collections::{BTreeMap, BTreeSet, HashMap}; +use std::fmt::Write; +use std::time::SystemTime; + +use itertools::Itertools; +use lib::core::config::get_main_branch_name; +use lib::core::dag::{CommitSet, Dag}; +use lib::core::effects::{Effects, OperationType}; +use lib::core::eventlog::EventLogDb; +use lib::core::repo_ext::{RepoExt, RepoReferencesSnapshot}; +use lib::git::{ + Branch, BranchType, CategorizedReferenceName, GitRunInfo, NonZeroOid, ReferenceName, Repo, +}; +use lib::util::ExitCode; +use tracing::warn; + +use crate::{CommitStatus, SubmitOptions, SubmitStatus}; + +pub struct BranchPushForge<'a> { + pub effects: &'a Effects, + pub git_run_info: &'a GitRunInfo, + pub repo: &'a Repo, + pub dag: &'a Dag, + pub event_log_db: &'a EventLogDb<'a>, + pub references_snapshot: &'a RepoReferencesSnapshot, +} + +impl BranchPushForge<'_> { + pub fn query_status( + &self, + commit_set: CommitSet, + ) -> eyre::Result, ExitCode>> { + struct BranchInfo<'a> { + branch: Branch<'a>, + branch_name: String, + remote_name: Option, + } + let main_branch_name = get_main_branch_name(self.repo)?; + let branch_infos: HashMap = { + let branch_reference_names = self + .dag + .commit_set_to_vec(&commit_set)? + .into_iter() + .flat_map(|commit_oid| { + match self + .references_snapshot + .branch_oid_to_names + .get(&commit_oid) + { + Some(names) => names.iter().cloned().collect(), + None => Vec::new(), + } + }); + + let mut branch_infos = HashMap::new(); + for branch_reference_name in branch_reference_names { + let branch_name = match CategorizedReferenceName::new(&branch_reference_name) { + name @ CategorizedReferenceName::LocalBranch { .. } => name.remove_prefix()?, + CategorizedReferenceName::RemoteBranch { .. } + | CategorizedReferenceName::OtherRef { .. } => continue, + }; + if branch_name == main_branch_name { + continue; + } + let branch = self + .repo + .find_branch(&branch_name, BranchType::Local)? + .ok_or_else(|| eyre::eyre!("Could not look up branch {branch_name:?}"))?; + + let remote_name = branch.get_push_remote_name()?; + let branch_info = BranchInfo { + branch, + branch_name, + remote_name, + }; + branch_infos.insert(branch_reference_name, branch_info); + } + branch_infos + }; + + // Fetch latest branches so that we know which commits are out-of-date + // and need to be pushed. + let event_tx_id = self + .event_log_db + .make_transaction_id(SystemTime::now(), "fetch remotes")?; + let remote_to_branches: BTreeMap<&String, Vec<&Branch>> = branch_infos + .values() + .flat_map(|branch_info| { + let BranchInfo { + branch, + branch_name: _, + remote_name, + } = branch_info; + remote_name + .as_ref() + .map(|remote_name| (remote_name, branch)) + }) + .into_group_map() + .into_iter() + .collect(); + // Make sure not to call `git fetch` with no remotes, as that will fetch + // something by default. + for (remote_name, branches) in remote_to_branches.iter() { + let remote_args = { + let mut result = vec!["fetch".to_owned()]; + result.push((*remote_name).clone()); + let branch_reference_names: BTreeSet = branches + .iter() + .map(|branch| branch.get_reference_name()) + .try_collect()?; + for branch_reference_name in branch_reference_names { + result.push(branch_reference_name.as_str().to_owned()); + } + result + }; + let exit_code = self + .git_run_info + .run(self.effects, Some(event_tx_id), &remote_args)?; + if !exit_code.is_success() { + writeln!( + self.effects.get_output_stream(), + "Failed to fetch from remote: {}", + remote_name + )?; + return Ok(Err(exit_code)); + } + } + + // Determine status of each commit/branch. + let mut commit_statuses = HashMap::new(); + for (commit_oid, branches) in &self.references_snapshot.branch_oid_to_names { + let branch_infos = branches + .iter() + .sorted() + .flat_map(|branch_reference_name| branch_infos.get(branch_reference_name)) + .collect_vec(); + + let commit_status = match branch_infos.as_slice() { + [] => CommitStatus { + submit_status: SubmitStatus::Unsubmitted, + remote_name: None, + local_branch_name: None, + remote_branch_name: None, + }, + + [BranchInfo { + branch, + branch_name, + remote_name, + }] => match branch.get_upstream_branch()? { + None => CommitStatus { + submit_status: SubmitStatus::Unsubmitted, + remote_name: None, + local_branch_name: Some(branch_name.clone()), + remote_branch_name: None, + }, + + Some(upstream_branch) => CommitStatus { + submit_status: if branch.get_oid()? == upstream_branch.get_oid()? { + SubmitStatus::UpToDate + } else { + SubmitStatus::NeedsUpdate + }, + remote_name: remote_name.clone(), + local_branch_name: Some(branch_name.clone()), + remote_branch_name: Some(upstream_branch.get_name()?.to_owned()), + }, + }, + + _branch_infos => CommitStatus { + submit_status: SubmitStatus::Unresolved, + remote_name: None, + local_branch_name: None, + remote_branch_name: None, + }, + }; + commit_statuses.insert(*commit_oid, commit_status); + } + + Ok(Ok(commit_statuses)) + } + + pub fn create( + &self, + commits: HashMap, + _options: &SubmitOptions, + ) -> eyre::Result { + let unsubmitted_branch_names = commits + .values() + .filter_map(|commit_status| { + let CommitStatus { + submit_status: _, + remote_name: _, + local_branch_name, + remote_branch_name: _, + } = commit_status; + local_branch_name.clone() + }) + .sorted() + .collect_vec(); + + let push_remote: String = match self.repo.get_default_push_remote()? { + Some(push_remote) => push_remote, + None => { + writeln!( + self.effects.get_output_stream(), + "\ +No upstream repository was associated with {} and no value was +specified for `remote.pushDefault`, so cannot push these branches: {} +Configure a value with: git config remote.pushDefault +These remotes are available: {}", + CategorizedReferenceName::new( + &self.repo.get_main_branch()?.get_reference_name()?, + ) + .friendly_describe(), + unsubmitted_branch_names.join(", "), + self.repo.get_all_remote_names()?.join(", "), + )?; + return Ok(ExitCode(1)); + } + }; + + if unsubmitted_branch_names.is_empty() { + Ok(ExitCode(0)) + } else { + // This will fail if somebody else created the branch on the remote and we don't + // know about it. + let mut args = vec!["push", "--set-upstream", &push_remote]; + args.extend(unsubmitted_branch_names.iter().map(|s| s.as_str())); + let event_tx_id = self + .event_log_db + .make_transaction_id(SystemTime::now(), "submit unsubmitted commits")?; + let (effects, progress) = self.effects.start_operation(OperationType::PushBranches); + let _effects = effects; + progress.notify_progress(0, unsubmitted_branch_names.len()); + self.git_run_info + .run(self.effects, Some(event_tx_id), &args) + } + } + + pub fn update( + &self, + commits: HashMap, + _options: &SubmitOptions, + ) -> eyre::Result { + let branches_by_remote: BTreeMap> = commits + .into_values() + .flat_map(|commit_status| match commit_status { + CommitStatus { + submit_status: _, + remote_name: Some(remote_name), + local_branch_name: Some(local_branch_name), + remote_branch_name: _, + } => Some((remote_name, local_branch_name)), + commit_status => { + warn!( + ?commit_status, +"Commit was requested to be updated, but it did not have the requisite information." + ); + None + } + }) + .into_group_map() + .into_iter() + .map(|(k, v)| (k, v.into_iter().collect::>())) + .collect(); + + let now = SystemTime::now(); + let event_tx_id = self.event_log_db.make_transaction_id(now, "submit")?; + let (effects, progress) = self.effects.start_operation(OperationType::PushBranches); + let total_num_branches = branches_by_remote + .values() + .map(|branch_names| branch_names.len()) + .sum(); + progress.notify_progress(0, total_num_branches); + for (remote_name, branch_names) in branches_by_remote { + let mut args = vec!["push", "--force-with-lease", &remote_name]; + args.extend(branch_names.iter().map(|s| s.as_str())); + let exit_code = self.git_run_info.run(&effects, Some(event_tx_id), &args)?; + if !exit_code.is_success() { + writeln!( + effects.get_output_stream(), + "Failed to push branches: {}", + branch_names.into_iter().join(", ") + )?; + return Ok(exit_code); + } + progress.notify_progress_inc(branch_names.len()); + } + + Ok(ExitCode(0)) + } +} diff --git a/git-branchless-submit/src/lib.rs b/git-branchless-submit/src/lib.rs index b90189eca..576d69607 100644 --- a/git-branchless-submit/src/lib.rs +++ b/git-branchless-submit/src/lib.rs @@ -9,19 +9,22 @@ )] #![allow(clippy::too_many_arguments, clippy::blocks_in_if_conditions)] +mod branch_push_forge; + +use std::collections::{BTreeSet, HashMap}; use std::fmt::Write; -use std::time::SystemTime; +use branch_push_forge::BranchPushForge; use cursive_core::theme::{BaseColor, Effect, Style}; use git_branchless_invoke::CommandContext; -use itertools::{Either, Itertools}; +use itertools::Itertools; use lazy_static::lazy_static; use lib::core::dag::Dag; -use lib::core::effects::{Effects, OperationType}; +use lib::core::effects::Effects; use lib::core::eventlog::{EventLogDb, EventReplayer}; use lib::core::formatting::{Pluralize, StyledStringBuilder}; use lib::core::repo_ext::RepoExt; -use lib::git::{Branch, BranchType, CategorizedReferenceName, ConfigRead, GitRunInfo, Repo}; +use lib::git::{GitRunInfo, NonZeroOid, Repo}; use lib::util::ExitCode; use git_branchless_opts::{ResolveRevsetOptions, Revset, SubmitArgs}; @@ -34,6 +37,43 @@ lazy_static! { Style::merge(&[BaseColor::Yellow.light().into(), Effect::Bold.into()]); } +/// The status of a commit, indicating whether it needs to be updated remotely. +#[derive(Clone, Debug)] +pub enum SubmitStatus { + /// The commit exists locally but has not been pushed remotely. + Unsubmitted, + + /// It could not be determined whether the remote commit exists. + Unresolved, + + /// The same commit exists both locally and remotely. + UpToDate, + + /// The commit exists locally but is associated with a different remote + /// commit, so it needs to be updated. + NeedsUpdate, +} + +/// Information about each commit. +#[derive(Clone, Debug)] +pub struct CommitStatus { + submit_status: SubmitStatus, + remote_name: Option, + local_branch_name: Option, + #[allow(dead_code)] + remote_branch_name: Option, +} + +/// Options for submitting commits to the forge. +pub struct SubmitOptions { + /// Create associated branches, code reviews, etc. for each of the provided commits. + /// + /// This should be an idempotent behavior, i.e. setting `create` to `true` + /// and submitting a commit which already has an associated remote item + /// should not have any additional effect. + pub create: bool, +} + /// `submit` command. pub fn command_main(ctx: CommandContext, args: SubmitArgs) -> eyre::Result { let CommandContext { @@ -64,8 +104,6 @@ fn submit( let repo = Repo::from_current_dir()?; let conn = repo.get_db_conn()?; let event_log_db = EventLogDb::new(&conn)?; - let now = SystemTime::now(); - let event_tx_id = event_log_db.make_transaction_id(now, "submit")?; let event_replayer = EventReplayer::from_event_log_db(effects, &repo, &event_log_db)?; let event_cursor = event_replayer.make_default_cursor(); let references_snapshot = repo.get_references_snapshot()?; @@ -86,163 +124,114 @@ fn submit( } }; - let branches: Vec = dag - .commit_set_to_vec(&commit_set)? - .into_iter() - .flat_map(|commit_oid| references_snapshot.branch_oid_to_names.get(&commit_oid)) - .flatten() - .filter_map( - |reference_name| match CategorizedReferenceName::new(reference_name) { - name @ CategorizedReferenceName::LocalBranch { .. } => name.remove_prefix().ok(), - CategorizedReferenceName::RemoteBranch { .. } - | CategorizedReferenceName::OtherRef { .. } => None, - }, - ) - .map(|branch_name| -> eyre::Result { - let branch = repo.find_branch(&branch_name, BranchType::Local)?; - let branch = - branch.ok_or_else(|| eyre::eyre!("Could not look up branch {branch_name:?}"))?; - Ok(branch) - }) - .collect::>()?; - let branches_and_remotes: Vec<(Branch, Option)> = branches - .into_iter() - .map(|branch| -> eyre::Result<_> { - let remote_name = branch.get_push_remote_name()?; - Ok((branch, remote_name)) - }) - .collect::>()?; - let (branches_without_remotes, branches_with_remotes): (Vec<_>, Vec<_>) = branches_and_remotes - .into_iter() - .partition_map(|(branch, remote_name)| match remote_name { - None => Either::Left(branch), - Some(remote_name) => Either::Right((branch, remote_name)), - }); - let remotes_to_branches = branches_with_remotes - .into_iter() - .map(|(v, k)| (k, v)) - .into_group_map(); + let submit_options = SubmitOptions { create }; + let forge = BranchPushForge { + effects, + git_run_info, + repo: &repo, + dag: &dag, + event_log_db: &event_log_db, + references_snapshot: &references_snapshot, + }; + let statuses = match forge.query_status(commit_set)? { + Ok(statuses) => statuses, + Err(exit_code) => return Ok(exit_code), + }; - let (created_branches, uncreated_branches) = { - let mut branch_names: Vec<&str> = branches_without_remotes - .iter() - .map(|branch| branch.get_name()) - .collect::>()?; - branch_names.sort_unstable(); - if branches_without_remotes.is_empty() { - Default::default() - } else if create { - let push_remote: String = match get_default_remote(&repo)? { - Some(push_remote) => push_remote, - None => { - writeln!( - effects.get_output_stream(), - "\ -No upstream repository was associated with {} and no value was -specified for `remote.pushDefault`, so cannot push these branches: {} -Configure a value with: git config remote.pushDefault -These remotes are available: {}", - CategorizedReferenceName::new( - &repo.get_main_branch()?.get_reference_name()?, - ) - .friendly_describe(), - branch_names.join(", "), - repo.get_all_remote_names()?.join(", "), - )?; - return Ok(ExitCode(1)); - } - }; + let (unsubmitted_commits, commits_to_update, commits_to_skip): ( + HashMap, + HashMap, + HashMap, + ) = statuses.into_iter().fold(Default::default(), |acc, elem| { + let (mut unsubmitted, mut to_update, mut to_skip) = acc; + let (commit_oid, commit_status) = elem; + match commit_status { + CommitStatus { + submit_status: SubmitStatus::Unsubmitted, + remote_name: _, + local_branch_name: Some(_), + remote_branch_name: _, + } => { + unsubmitted.insert(commit_oid, commit_status); + } + + // Not currently implemented: generating a branch for + // unsubmitted commits which don't yet have branches. + CommitStatus { + submit_status: SubmitStatus::Unsubmitted, + remote_name: _, + local_branch_name: None, + remote_branch_name: _, + } => {} - // This will fail if somebody else created the branch on the remote and we don't - // know about it. - let mut args = vec!["push", "--set-upstream", &push_remote]; - args.extend(branch_names.iter()); - { - let (effects, progress) = effects.start_operation(OperationType::PushBranches); - progress.notify_progress(0, branch_names.len()); - let exit_code = git_run_info.run(&effects, Some(event_tx_id), &args)?; - if !exit_code.is_success() { - return Ok(exit_code); - } + CommitStatus { + submit_status: SubmitStatus::NeedsUpdate, + remote_name: _, + local_branch_name: _, + remote_branch_name: _, + } => { + to_update.insert(commit_oid, commit_status); } - (branch_names, Default::default()) - } else { - (Default::default(), branch_names) - } - }; - // TODO: explain why fetching here. - for (remote_name, branches) in remotes_to_branches.iter() { - let remote_args = { - let mut result = vec!["fetch".to_owned()]; - result.push((*remote_name).clone()); - for branch in branches { - let branch_ref = branch.get_reference_name()?; - result.push(branch_ref.as_str().to_owned()); + CommitStatus { + submit_status: SubmitStatus::UpToDate, + remote_name: _, + local_branch_name: Some(_), + remote_branch_name: _, + } => { + to_skip.insert(commit_oid, commit_status); } - result - }; - let exit_code = git_run_info.run(effects, Some(event_tx_id), &remote_args)?; - if !exit_code.is_success() { - writeln!( - effects.get_output_stream(), - "Failed to fetch from remote: {}", - remote_name - )?; - return Ok(exit_code); - } - } - let (pushed_branches, skipped_branches) = { - let (effects, progress) = effects.start_operation(OperationType::PushBranches); + // Don't know what to do in these cases 🙃. + CommitStatus { + submit_status: SubmitStatus::Unresolved, + remote_name: _, + local_branch_name: _, + remote_branch_name: _, + } + | CommitStatus { + submit_status: SubmitStatus::UpToDate, + remote_name: _, + local_branch_name: None, + remote_branch_name: _, + } => {} + } + (unsubmitted, to_update, to_skip) + }); - let mut pushed_branches: Vec<&str> = Vec::new(); - let mut skipped_branches: Vec<&str> = Vec::new(); - let total_num_branches = remotes_to_branches + let (created_branches, uncreated_branches): (BTreeSet, BTreeSet) = { + let unsubmitted_branches = unsubmitted_commits .values() - .map(|branches| branches.len()) - .sum(); - progress.notify_progress(0, total_num_branches); - for (remote_name, branches) in remotes_to_branches + .flat_map(|commit_status| commit_status.local_branch_name.clone()) + .collect(); + if unsubmitted_commits.is_empty() { + Default::default() + } else if create { + let exit_code = forge.create(unsubmitted_commits, &submit_options)?; + if !exit_code.is_success() { + return Ok(exit_code); + } + (unsubmitted_branches, Default::default()) + } else { + (Default::default(), unsubmitted_branches) + } + }; + + let (updated_branch_names, skipped_branch_names): (BTreeSet, BTreeSet) = { + let updated_branch_names = commits_to_update + .iter() + .flat_map(|(_commit_oid, commit_status)| commit_status.local_branch_name.clone()) + .collect(); + let skipped_branch_names = commits_to_skip .iter() - .sorted_by(|(k1, _v1), (k2, _v2)| k1.cmp(k2)) - { - let (branches_to_push_names, branches_to_skip_names) = { - let mut branches_to_push_names = Vec::new(); - let mut branches_to_skip_names = Vec::new(); - for branch in branches { - let branch_name = branch.get_name()?; - if let Some(upstream_branch) = branch.get_upstream_branch()? { - if upstream_branch.get_oid()? == branch.get_oid()? { - branches_to_skip_names.push(branch_name); - continue; - } - } - branches_to_push_names.push(branch_name); - } - branches_to_push_names.sort_unstable(); - branches_to_skip_names.sort_unstable(); - (branches_to_push_names, branches_to_skip_names) - }; - pushed_branches.extend(branches_to_push_names.iter()); - skipped_branches.extend(branches_to_skip_names.iter()); + .flat_map(|(_commit_oid, commit_status)| commit_status.local_branch_name.clone()) + .collect(); - if !pushed_branches.is_empty() { - let mut args = vec!["push", "--force-with-lease", remote_name]; - args.extend(branches_to_push_names.iter()); - let exit_code = git_run_info.run(&effects, Some(event_tx_id), &args)?; - if !exit_code.is_success() { - writeln!( - effects.get_output_stream(), - "Failed to push branches: {}", - branches_to_push_names.into_iter().join(", ") - )?; - return Ok(exit_code); - } - } - progress.notify_progress_inc(branches.len()); + let exit_code = forge.update(commits_to_update, &submit_options)?; + if !exit_code.is_success() { + return Ok(exit_code); } - (pushed_branches, skipped_branches) + (updated_branch_names, skipped_branch_names) }; if !created_branches.is_empty() { @@ -267,16 +256,16 @@ These remotes are available: {}", .join(", ") )?; } - if !pushed_branches.is_empty() { + if !updated_branch_names.is_empty() { writeln!( effects.get_output_stream(), "Pushed {}: {}", Pluralize { determiner: None, - amount: pushed_branches.len(), + amount: updated_branch_names.len(), unit: ("branch", "branches") }, - pushed_branches + updated_branch_names .into_iter() .map(|branch_name| effects .get_glyphs() @@ -289,16 +278,16 @@ These remotes are available: {}", .join(", ") )?; } - if !skipped_branches.is_empty() { + if !skipped_branch_names.is_empty() { writeln!( effects.get_output_stream(), "Skipped {} (already up-to-date): {}", Pluralize { determiner: None, - amount: skipped_branches.len(), + amount: skipped_branch_names.len(), unit: ("branch", "branches") }, - skipped_branches + skipped_branch_names .into_iter() .map(|branch_name| effects .get_glyphs() @@ -342,32 +331,3 @@ create and push them, retry this operation with the --create option." Ok(ExitCode(0)) } - -fn get_default_remote(repo: &Repo) -> eyre::Result> { - let main_branch_name = repo.get_main_branch()?.get_reference_name()?; - match CategorizedReferenceName::new(&main_branch_name) { - name @ CategorizedReferenceName::LocalBranch { .. } => { - if let Some(main_branch) = - repo.find_branch(&name.remove_prefix()?, BranchType::Local)? - { - if let Some(remote_name) = main_branch.get_push_remote_name()? { - return Ok(Some(remote_name)); - } - } - } - - name @ CategorizedReferenceName::RemoteBranch { .. } => { - let name = name.remove_prefix()?; - if let Some((remote_name, _reference_name)) = name.split_once('/') { - return Ok(Some(remote_name.to_owned())); - } - } - - CategorizedReferenceName::OtherRef { .. } => { - // Do nothing. - } - } - - let push_default_remote_opt = repo.get_readonly_config()?.get("remote.pushDefault")?; - Ok(push_default_remote_opt) -} diff --git a/git-branchless-submit/tests/test_submit.rs b/git-branchless-submit/tests/test_submit.rs index 00bcc1b30..21fc2467b 100644 --- a/git-branchless-submit/tests/test_submit.rs +++ b/git-branchless-submit/tests/test_submit.rs @@ -99,15 +99,15 @@ fn test_submit() -> eyre::Result<()> { let stderr = redact_remotes(stderr); insta::assert_snapshot!(stderr, @r###" From: file:// - * branch qux -> FETCH_HEAD * branch bar -> FETCH_HEAD + * branch qux -> FETCH_HEAD branchless: processing 1 update: branch qux To: file:// + 20230db...bae8307 qux -> qux (forced update) branchless: processing 1 update: remote branch origin/qux "###); insta::assert_snapshot!(stdout, @r###" - branchless: running command: fetch origin refs/heads/qux refs/heads/bar + branchless: running command: fetch origin refs/heads/bar refs/heads/qux branchless: running command: push --force-with-lease origin qux Pushed 1 branch: qux Skipped 1 branch (already up-to-date): bar @@ -120,11 +120,11 @@ fn test_submit() -> eyre::Result<()> { let stderr = redact_remotes(stderr); insta::assert_snapshot!(stderr, @r###" From: file:// - * branch qux -> FETCH_HEAD * branch bar -> FETCH_HEAD + * branch qux -> FETCH_HEAD "###); insta::assert_snapshot!(stdout, @r###" - branchless: running command: fetch origin refs/heads/qux refs/heads/bar + branchless: running command: fetch origin refs/heads/bar refs/heads/qux Skipped 2 branches (already up-to-date): bar, qux "###); } From 2c4dea7efc6334ea09aaf644aa41130c137008e0 Mon Sep 17 00:00:00 2001 From: Waleed Khan Date: Fri, 10 Mar 2023 00:20:49 -0500 Subject: [PATCH 4/8] refactor(repo): make `Commit::get_message*` functions infallible --- .../bin/testing/regression_test_record.rs | 2 +- git-branchless-lib/src/core/node_descriptors.rs | 2 +- git-branchless-lib/src/core/rewrite/execute.rs | 4 ++-- git-branchless-lib/src/git/object.rs | 12 ++++++------ git-branchless-revset/src/builtins.rs | 2 +- git-branchless-revset/src/eval.rs | 2 +- git-branchless-reword/src/lib.rs | 2 +- git-branchless-test/src/lib.rs | 2 +- git-branchless/src/commands/amend.rs | 2 +- 9 files changed, 15 insertions(+), 15 deletions(-) diff --git a/git-branchless-lib/bin/testing/regression_test_record.rs b/git-branchless-lib/bin/testing/regression_test_record.rs index 7caa0dbe7..c4aa34bd8 100644 --- a/git-branchless-lib/bin/testing/regression_test_record.rs +++ b/git-branchless-lib/bin/testing/regression_test_record.rs @@ -100,7 +100,7 @@ fn main() -> eyre::Result<()> { let actual_commit = { let author = current_commit.get_author(); let committer = current_commit.get_committer(); - let message = current_commit.get_message_raw()?; + let message = current_commit.get_message_raw(); let message = message.to_str_lossy(); let parents = current_commit.get_parents(); let actual_oid = repo.create_commit( diff --git a/git-branchless-lib/src/core/node_descriptors.rs b/git-branchless-lib/src/core/node_descriptors.rs index 28ca4117b..83c6c2818 100644 --- a/git-branchless-lib/src/core/node_descriptors.rs +++ b/git-branchless-lib/src/core/node_descriptors.rs @@ -432,7 +432,7 @@ impl<'a> NodeDescriptor for DifferentialRevisionDescriptor<'a> { NodeObject::GarbageCollected { oid: _ } => return Ok(None), }; - let diff_number = match extract_diff_number(&commit.get_message_raw()?.to_str_lossy()) { + let diff_number = match extract_diff_number(&commit.get_message_raw().to_str_lossy()) { Some(diff_number) => diff_number, None => return Ok(None), }; diff --git a/git-branchless-lib/src/core/rewrite/execute.rs b/git-branchless-lib/src/core/rewrite/execute.rs index 107ef3e4e..9f17c757d 100644 --- a/git-branchless-lib/src/core/rewrite/execute.rs +++ b/git-branchless-lib/src/core/rewrite/execute.rs @@ -609,7 +609,7 @@ mod in_memory { Err(other) => eyre::bail!(other), }; - let commit_message = commit_to_apply.get_message_raw()?; + let commit_message = commit_to_apply.get_message_raw(); let commit_message = commit_message.to_str().with_context(|| { eyre::eyre!( "Could not decode commit message for commit: {:?}", @@ -706,7 +706,7 @@ mod in_memory { let replacement_commit = repo.find_commit_or_fail(*replacement_commit_oid)?; let replacement_tree = replacement_commit.get_tree()?; - let replacement_message = replacement_commit.get_message_raw()?; + let replacement_message = replacement_commit.get_message_raw(); let replacement_commit_message = replacement_message.to_str().with_context(|| { eyre::eyre!( diff --git a/git-branchless-lib/src/git/object.rs b/git-branchless-lib/src/git/object.rs index cead861f7..e7b851074 100644 --- a/git-branchless-lib/src/git/object.rs +++ b/git-branchless-lib/src/git/object.rs @@ -105,14 +105,14 @@ impl<'repo> Commit<'repo> { /// Get the commit message with some whitespace trimmed. #[instrument] - pub fn get_message_pretty(&self) -> Result { - Ok(BString::from(self.inner.message_bytes())) + pub fn get_message_pretty(&self) -> BString { + BString::from(self.inner.message_bytes()) } /// Get the commit message, without any whitespace trimmed. #[instrument] - pub fn get_message_raw(&self) -> Result { - Ok(BString::from(self.inner.message_raw_bytes())) + pub fn get_message_raw(&self) -> BString { + BString::from(self.inner.message_raw_bytes()) } /// Get the author of this commit. @@ -151,7 +151,7 @@ impl<'repo> Commit<'repo> { /// like `Signed-off-by: foo` which appear at the end of the commit message. #[instrument] pub fn get_trailers(&self) -> Result> { - let message = self.get_message_raw()?; + let message = self.get_message_raw(); let message = message.to_str().map_err(|_| Error::DecodeUtf8 { item: "raw message", })?; @@ -249,7 +249,7 @@ impl<'repo> Commit<'repo> { BaseColor::Green.light(), ), StyledString::plain(textwrap::indent( - &self.get_message_pretty()?.to_str_lossy(), + &self.get_message_pretty().to_str_lossy(), " ", )), ]); diff --git a/git-branchless-revset/src/builtins.rs b/git-branchless-revset/src/builtins.rs index 651b3af4d..7dba5cf04 100644 --- a/git-branchless-revset/src/builtins.rs +++ b/git-branchless-revset/src/builtins.rs @@ -312,7 +312,7 @@ fn fn_message(ctx: &mut Context, name: &str, args: &[Expr]) -> EvalResult { name, args, Box::new(move |_repo, commit| { - let message = commit.get_message_raw().map_err(PatternError::Repo)?; + let message = commit.get_message_raw(); let message = match message.to_str() { Ok(message) => message, Err(err) => { diff --git a/git-branchless-revset/src/eval.rs b/git-branchless-revset/src/eval.rs index adc2fd253..a81689286 100644 --- a/git-branchless-revset/src/eval.rs +++ b/git-branchless-revset/src/eval.rs @@ -372,7 +372,7 @@ mod tests { .into_iter() .map(|oid| repo.find_commit_or_fail(oid)) .try_collect()?; - commits.sort_by_key(|commit| (commit.get_message_pretty().unwrap(), commit.get_time())); + commits.sort_by_key(|commit| (commit.get_message_pretty(), commit.get_time())); Ok(commits) } diff --git a/git-branchless-reword/src/lib.rs b/git-branchless-reword/src/lib.rs index 520cb1c48..5709c7fc6 100644 --- a/git-branchless-reword/src/lib.rs +++ b/git-branchless-reword/src/lib.rs @@ -427,7 +427,7 @@ fn prepare_messages( let oid = commit.get_short_oid()?; let original_message = commit - .get_message_raw()? + .get_message_raw() .to_str() .with_context(|| { eyre::eyre!( diff --git a/git-branchless-test/src/lib.rs b/git-branchless-test/src/lib.rs index 80a0f16a7..cd2281c48 100644 --- a/git-branchless-test/src/lib.rs +++ b/git-branchless-test/src/lib.rs @@ -1901,7 +1901,7 @@ fn apply_fixes( for (original_commit_oid, fixed_tree_oid) in fixed_tree_oids { let original_commit = repo.find_commit_or_fail(original_commit_oid)?; let original_tree_oid = original_commit.get_tree_oid(); - let commit_message = original_commit.get_message_raw()?; + let commit_message = original_commit.get_message_raw(); let commit_message = commit_message.to_str().with_context(|| { eyre::eyre!( "Could not decode commit message for commit: {:?}", diff --git a/git-branchless/src/commands/amend.rs b/git-branchless/src/commands/amend.rs index ee62a0085..1e6d2bd81 100644 --- a/git-branchless/src/commands/amend.rs +++ b/git-branchless/src/commands/amend.rs @@ -254,7 +254,7 @@ pub fn amend( .into_iter() .map(|parent_oid| repo.find_commit_or_fail(parent_oid)) .try_collect()?; - let descendant_message = descendant_commit.get_message_raw()?; + let descendant_message = descendant_commit.get_message_raw(); let descendant_message = descendant_message.to_str().with_context(|| { eyre::eyre!( "Could not decode commit message for descendant commit: {:?}", From 4971d5e679d1c04d59df43d464960622472f343a Mon Sep 17 00:00:00 2001 From: Waleed Khan Date: Wed, 8 Mar 2023 23:02:35 -0500 Subject: [PATCH 5/8] feat(test): expose `head_commit_oid` The intention is to use this after Phabricator's `arc diff` amends the current commit's message, so that we can adopt the resulting message. --- git-branchless-lib/src/git/test.rs | 3 +- git-branchless-revset/src/builtins.rs | 20 ++-- git-branchless-test/src/lib.rs | 141 ++++++++++++++++---------- 3 files changed, 101 insertions(+), 63 deletions(-) diff --git a/git-branchless-lib/src/git/test.rs b/git-branchless-lib/src/git/test.rs index 544b2190e..3202c7253 100644 --- a/git-branchless-lib/src/git/test.rs +++ b/git-branchless-lib/src/git/test.rs @@ -70,7 +70,8 @@ impl<'de> Deserialize<'de> for SerializedNonZeroOid { pub struct SerializedTestResult { pub command: String, pub exit_code: i32, - pub fixed_tree_oid: Option, + pub head_commit_oid: Option, + pub snapshot_tree_oid: Option, #[serde(default)] pub interactive: bool, } diff --git a/git-branchless-revset/src/builtins.rs b/git-branchless-revset/src/builtins.rs index 7dba5cf04..2913676d4 100644 --- a/git-branchless-revset/src/builtins.rs +++ b/git-branchless-revset/src/builtins.rs @@ -572,7 +572,8 @@ fn fn_tests_passed(ctx: &mut Context, name: &str, args: &[Expr]) -> EvalResult { let SerializedTestResult { command, exit_code, - fixed_tree_oid: _, + head_commit_oid: _, + snapshot_tree_oid: _, interactive: _, } = test_result; exit_code == TEST_SUCCESS_EXIT_CODE && pattern.matches_text(&command) @@ -597,7 +598,8 @@ fn fn_tests_failed(ctx: &mut Context, name: &str, args: &[Expr]) -> EvalResult { let SerializedTestResult { command, exit_code, - fixed_tree_oid: _, + head_commit_oid: _, + snapshot_tree_oid: _, interactive: _, } = test_result; exit_code != TEST_SUCCESS_EXIT_CODE @@ -625,16 +627,18 @@ fn fn_tests_fixable(ctx: &mut Context, name: &str, args: &[Expr]) -> EvalResult let SerializedTestResult { command, exit_code, - fixed_tree_oid, + head_commit_oid: _, + snapshot_tree_oid, interactive: _, } = test_result; exit_code == TEST_SUCCESS_EXIT_CODE && pattern.matches_text(&command) - && match fixed_tree_oid { - None => false, - Some(SerializedNonZeroOid(fixed_tree_oid)) => { - commit.get_tree_oid() != MaybeZeroOid::NonZero(fixed_tree_oid) - } + && match (snapshot_tree_oid, commit.get_tree_oid()) { + ( + Some(SerializedNonZeroOid(snapshot_tree_oid)), + MaybeZeroOid::NonZero(original_tree_oid), + ) => snapshot_tree_oid != original_tree_oid, + (None, _) | (_, MaybeZeroOid::Zero) => false, } }); Ok(result) diff --git a/git-branchless-test/src/lib.rs b/git-branchless-test/src/lib.rs index cd2281c48..8b2693dfe 100644 --- a/git-branchless-test/src/lib.rs +++ b/git-branchless-test/src/lib.rs @@ -870,10 +870,8 @@ enum TestStatus { /// actually re-run the test). cached: bool, - /// The resulting contents of the working copy after the command was - /// executed (if taking a working copy snapshot succeeded and there were - /// no merge conflicts, etc.). - fixed_tree_oid: Option, + /// Information about the working copy state after running the test command. + fix_info: FixInfo, /// Whether the test was run interactively (the user executed the /// command via `--interactive`). @@ -881,6 +879,20 @@ enum TestStatus { }, } +/// Information about the working copy state after running the test command. +#[derive(Clone, Debug)] +struct FixInfo { + /// The resulting commit which was checked out as `HEAD`. This is usually + /// the same commit as was being tested, but the test command could amend or + /// switch to a different commit. + head_commit_oid: Option, + + /// The resulting contents of the working copy after the command was + /// executed (if taking a working copy snapshot succeeded and there were no + /// merge conflicts, etc.). + snapshot_tree_oid: Option, +} + impl TestStatus { #[instrument] fn get_icon(&self) -> &'static str { @@ -998,18 +1010,27 @@ fn make_test_status_description( TestStatus::Passed { cached, interactive, - fixed_tree_oid, + fix_info: + FixInfo { + head_commit_oid: _, + snapshot_tree_oid, + }, } => { let mut descriptors = Vec::new(); if *cached { descriptors.push("cached".to_string()); } - if fixed_tree_oid.is_some() { - descriptors.push(if apply_fixes { - "fixed".to_string() - } else { - "fixable".to_string() - }); + match (snapshot_tree_oid, commit.get_tree_oid()) { + (Some(snapshot_tree_oid), MaybeZeroOid::NonZero(original_tree_oid)) => { + if *snapshot_tree_oid != original_tree_oid { + descriptors.push(if apply_fixes { + "fixed".to_string() + } else { + "fixable".to_string() + }); + } + } + (None, _) | (_, MaybeZeroOid::Zero) => {} } if *interactive { descriptors.push("interactive".to_string()); @@ -1613,8 +1634,8 @@ fn event_loop( TestStatus::Passed { cached: _, + fix_info: _, interactive: _, - fixed_tree_oid: _, } => (None, search::Status::Success), }; if search_strategy.is_some() { @@ -1700,7 +1721,7 @@ fn print_summary( } TestStatus::Passed { cached, - fixed_tree_oid: _, + fix_info: _, interactive: _, } => { num_passed += 1; @@ -1870,13 +1891,21 @@ fn apply_fixes( .filter_map(|(commit_oid, test_output)| match test_output.test_status { TestStatus::Passed { cached: _, - fixed_tree_oid: Some(fixed_tree_oid), + fix_info: + FixInfo { + head_commit_oid: _, + snapshot_tree_oid: Some(snapshot_tree_oid), + }, interactive: _, - } => Some((*commit_oid, fixed_tree_oid)), + } => Some((*commit_oid, snapshot_tree_oid)), TestStatus::Passed { cached: _, - fixed_tree_oid: None, + fix_info: + FixInfo { + head_commit_oid: _, + snapshot_tree_oid: None, + }, interactive: _, } | TestStatus::CheckoutFailed @@ -2339,18 +2368,24 @@ fn make_test_files( Ok(SerializedTestResult { command: _, exit_code: 0, - fixed_tree_oid, + head_commit_oid, + snapshot_tree_oid, interactive, }) => TestStatus::Passed { cached: true, - fixed_tree_oid: fixed_tree_oid.map(|SerializedNonZeroOid(oid)| oid), + fix_info: FixInfo { + head_commit_oid: head_commit_oid.map(|SerializedNonZeroOid(oid)| oid), + snapshot_tree_oid: snapshot_tree_oid.map(|SerializedNonZeroOid(oid)| oid), + }, + interactive, }, Ok(SerializedTestResult { command: _, exit_code, - fixed_tree_oid: _, + head_commit_oid: _, + snapshot_tree_oid: _, interactive: _, }) if exit_code == TEST_INDETERMINATE_EXIT_CODE => { TestStatus::Indeterminate { exit_code } @@ -2359,14 +2394,16 @@ fn make_test_files( Ok(SerializedTestResult { command: _, exit_code, - fixed_tree_oid: _, + head_commit_oid: _, + snapshot_tree_oid: _, interactive: _, }) if exit_code == TEST_ABORT_EXIT_CODE => TestStatus::Abort { exit_code }, Ok(SerializedTestResult { command: _, exit_code, - fixed_tree_oid: _, + head_commit_oid: _, + snapshot_tree_oid: _, interactive, }) => TestStatus::Failed { cached: true, @@ -2646,9 +2683,9 @@ To abort testing entirely, run: {exit127}", }; let test_status = match exit_code { TEST_SUCCESS_EXIT_CODE => { - let fixed_tree_oid = { + let fix_info = { let repo = Repo::from_dir(working_directory)?; - let snapshot = { + let (head_commit_oid, snapshot) = { let index = repo.get_index()?; let head_info = repo.get_head_info()?; let (snapshot, _status) = repo.get_status( @@ -2658,24 +2695,12 @@ To abort testing entirely, run: {exit127}", &head_info, Some(event_tx_id), )?; - if head_info.oid != Some(commit.get_oid()) { - warn!( - ?commit, - ?head_info, - "Repository was not checked out to expected commit" - ); - } - snapshot + (head_info.oid, snapshot) }; - match snapshot.get_working_copy_changes_type()? { + let snapshot_tree_oid = match snapshot.get_working_copy_changes_type()? { WorkingCopyChangesType::None | WorkingCopyChangesType::Unstaged => { let fixed_tree_oid: MaybeZeroOid = snapshot.commit_unstaged.get_tree_oid(); - if commit.get_tree_oid() != fixed_tree_oid { - let fixed_tree_oid: Option = fixed_tree_oid.into(); - fixed_tree_oid - } else { - None - } + fixed_tree_oid.into() } changes_type @ (WorkingCopyChangesType::Staged | WorkingCopyChangesType::Conflicts) => { @@ -2686,11 +2711,15 @@ To abort testing entirely, run: {exit127}", ); None } + }; + FixInfo { + head_commit_oid, + snapshot_tree_oid, } }; TestStatus::Passed { cached: false, - fixed_tree_oid, + fix_info, interactive: options.is_interactive, } } @@ -2705,24 +2734,28 @@ To abort testing entirely, run: {exit127}", }, }; + let fix_info = match &test_status { + TestStatus::Passed { + cached: _, + fix_info, + interactive: _, + } => Some(fix_info), + TestStatus::CheckoutFailed + | TestStatus::SpawnTestFailed(_) + | TestStatus::TerminatedBySignal + | TestStatus::AlreadyInProgress + | TestStatus::ReadCacheFailed(_) + | TestStatus::Failed { .. } + | TestStatus::Abort { .. } + | TestStatus::Indeterminate { .. } => None, + }; let serialized_test_result = SerializedTestResult { command: options.command.clone(), exit_code, - fixed_tree_oid: match &test_status { - TestStatus::Passed { - cached: _, - fixed_tree_oid, - interactive: _, - } => (*fixed_tree_oid).map(SerializedNonZeroOid), - TestStatus::CheckoutFailed - | TestStatus::SpawnTestFailed(_) - | TestStatus::TerminatedBySignal - | TestStatus::AlreadyInProgress - | TestStatus::ReadCacheFailed(_) - | TestStatus::Failed { .. } - | TestStatus::Abort { .. } - | TestStatus::Indeterminate { .. } => None, - }, + head_commit_oid: fix_info + .and_then(|fix_info| fix_info.head_commit_oid.map(SerializedNonZeroOid)), + snapshot_tree_oid: fix_info + .and_then(|fix_info| fix_info.snapshot_tree_oid.map(SerializedNonZeroOid)), interactive: options.is_interactive, }; serde_json::to_writer_pretty(result_file, &serialized_test_result) From 5a8f19870d6dc8a178946b1fb97b3b163ded6909 Mon Sep 17 00:00:00 2001 From: Waleed Khan Date: Fri, 10 Mar 2023 15:30:24 -0500 Subject: [PATCH 6/8] feat(submit): create Phabricator forge --- Cargo.lock | 257 ++++-- git-branchless-lib/src/core/effects.rs | 8 +- git-branchless-opts/src/lib.rs | 8 + git-branchless-submit/Cargo.toml | 10 +- .../examples/dump_phabricator_dependencies.rs | 120 +++ .../src/branch_push_forge.rs | 22 +- git-branchless-submit/src/lib.rs | 186 ++++- git-branchless-submit/src/phabricator.rs | 738 ++++++++++++++++++ git-branchless-test/src/lib.rs | 346 ++++---- 9 files changed, 1427 insertions(+), 268 deletions(-) create mode 100644 git-branchless-submit/examples/dump_phabricator_dependencies.rs create mode 100644 git-branchless-submit/src/phabricator.rs diff --git a/Cargo.lock b/Cargo.lock index 3764bb811..a7ce2e18c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -34,7 +34,7 @@ version = "0.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bf6ccdb167abbf410dcb915cabd428929d7f6a04980b54a11f26a39f1c7f7107" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "getrandom 0.2.8", "once_cell", "version_check", @@ -150,7 +150,7 @@ checksum = "233d376d6d185f2a3093e58f283f60f880315b6c60075b01f36b3b85154564ca" dependencies = [ "addr2line", "cc", - "cfg-if", + "cfg-if 1.0.0", "libc", "miniz_oxide", "object", @@ -249,6 +249,12 @@ dependencies = [ "jobserver", ] +[[package]] +name = "cfg-if" +version = "0.1.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4785bdd1c96b2a846b2bd7cc02e86b6b3dbf14e7e53446c4f54c92a361040822" + [[package]] name = "cfg-if" version = "1.0.0" @@ -398,6 +404,15 @@ dependencies = [ "roff 0.2.1", ] +[[package]] +name = "cloudabi" +version = "0.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ddfc5b9aa5d4507acaf872de71051dfd0e309860e88966e1051e462a077aac4f" +dependencies = [ + "bitflags", +] + [[package]] name = "codespan-reporting" version = "0.11.1" @@ -483,7 +498,7 @@ dependencies = [ "ciborium", "clap 3.2.23", "criterion-plot", - "itertools", + "itertools 0.10.5", "lazy_static", "num-traits", "oorandom", @@ -504,7 +519,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6b50826342786a51a89e2da3a28f1c32b06e387201bc2d19791f622c673706b1" dependencies = [ "cast", - "itertools", + "itertools 0.10.5", ] [[package]] @@ -513,7 +528,7 @@ version = "0.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2801af0d36612ae591caa9568261fddce32ce6e08a7275ea334a06a4ad021a2c" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "crossbeam-channel", "crossbeam-deque", "crossbeam-epoch", @@ -527,7 +542,7 @@ version = "0.5.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2dd04ddaf88237dc3b8d8f9a3c1004b506b54b3313403944054d23c0870c521" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "crossbeam-utils", ] @@ -537,7 +552,7 @@ version = "0.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "715e8152b692bba2d374b53d4875445368fdf21a94751410af607a5ac677d1fc" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "crossbeam-epoch", "crossbeam-utils", ] @@ -549,7 +564,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "01a9af1f4c2ef74bb8aa1f7e19706bc72d03598c8a570bb5de72243c7a9d9d5a" dependencies = [ "autocfg", - "cfg-if", + "cfg-if 1.0.0", "crossbeam-utils", "memoffset 0.7.1", "scopeguard", @@ -561,7 +576,7 @@ version = "0.3.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d1cfb3ea8a53f37c40dea2c7bedcbd88bdfae54f5e2175d6ecaff1c988353add" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "crossbeam-utils", ] @@ -571,7 +586,7 @@ version = "0.8.14" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4fb766fa798726286dbbb842f174001dab8abc7b627a1dd86e0b7222a95d929f" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", ] [[package]] @@ -628,7 +643,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5438eb16bdd8af51b31e74764fef5d0a9260227a5ec82ba75c9d11ce46595839" dependencies = [ "ahash 0.8.2", - "cfg-if", + "cfg-if 1.0.0", "crossbeam-channel", "crossterm 0.25.0", "cursive_core", @@ -812,7 +827,7 @@ version = "2.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b98cf8ebf19c3d1b223e151f99a4f9f0690dca41414773390fc824184ac833e1" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "dirs-sys-next", ] @@ -939,6 +954,30 @@ dependencies = [ "tracing", ] +[[package]] +name = "esl01-dag" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f284f338575e1b6c4f4ba306f2e36a9a31b53b071fc59c2f644769a4d948563d" +dependencies = [ + "anyhow", + "bitflags", + "byteorder", + "esl01-drawdag 0.1.0", + "esl01-indexedlog 0.1.2", + "esl01-minibytes 0.2.0", + "esl01-vlqencoding 0.1.0", + "fs2", + "indexmap", + "itertools 0.8.2", + "once_cell", + "parking_lot 0.10.2", + "serde", + "tempfile", + "thiserror", + "tracing", +] + [[package]] name = "esl01-dag" version = "0.3.0" @@ -950,13 +989,13 @@ dependencies = [ "bitflags", "byteorder", "esl01-dag-types", - "esl01-drawdag", - "esl01-indexedlog", + "esl01-drawdag 0.3.0", + "esl01-indexedlog 0.3.0", "esl01-mincode", - "esl01-minibytes", + "esl01-minibytes 0.3.0", "esl01-nonblocking", "esl01-renderdag", - "esl01-vlqencoding", + "esl01-vlqencoding 0.3.0", "fail", "fs2", "futures", @@ -974,16 +1013,42 @@ version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cd764f3e0441aa076242c5591e8c70e9250ece9a0298b9a118a636c97121608f" dependencies = [ - "esl01-minibytes", + "esl01-minibytes 0.3.0", "serde", ] +[[package]] +name = "esl01-drawdag" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f503f73744061222785c9fdedc5654830da64a09dee5f0bdcaf3ea67d7400422" + [[package]] name = "esl01-drawdag" version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "58614a84fe70f0dcf8e4126950606bbe9339bf931002791e34bae0fab0a8c9a7" +[[package]] +name = "esl01-indexedlog" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "28043d7042ee6d9d9472f69acc8b9f1b2c2c5e5896489eb1298e147e18ed9147" +dependencies = [ + "byteorder", + "esl01-minibytes 0.2.0", + "esl01-vlqencoding 0.1.0", + "fs2", + "hex", + "libc", + "memmap", + "once_cell", + "rand 0.7.3", + "tempfile", + "tracing", + "twox-hash", +] + [[package]] name = "esl01-indexedlog" version = "0.3.0" @@ -992,8 +1057,8 @@ checksum = "ca189b77364e351675e0ac8274a8df0be0c88d3e66103d81f9bd265371f49389" dependencies = [ "byteorder", "esl01-atomicfile", - "esl01-minibytes", - "esl01-vlqencoding", + "esl01-minibytes 0.3.0", + "esl01-vlqencoding 0.3.0", "fs2", "hex", "libc", @@ -1012,7 +1077,17 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "238b140f1ced88489a0b3770b3a7613237d73919d6e4c9b8d295c44142264482" dependencies = [ "byteorder", - "esl01-vlqencoding", + "esl01-vlqencoding 0.3.0", + "serde", +] + +[[package]] +name = "esl01-minibytes" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9c26b8b6a743b61c33507a42ef590a314d3e5d6ffc10524e1270c0e809460fb1" +dependencies = [ + "memmap", "serde", ] @@ -1040,9 +1115,15 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3a1840969ab8be31e186bb6d2f672d586dcd203dd4019a80dc1277a14686eca9" dependencies = [ "bitflags", - "itertools", + "itertools 0.10.5", ] +[[package]] +name = "esl01-vlqencoding" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6b9a5c382a325e954524d0b68f83ff6f51b2b68996d84aef0502b0350a7e16d3" + [[package]] name = "esl01-vlqencoding" version = "0.3.0" @@ -1253,7 +1334,7 @@ version = "0.1.16" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8fc3cb4d91f53b50155bdcfd23f6a4c39ae1969c2ae85982b135750cccaf5fce" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "libc", "wasi 0.9.0+wasi-snapshot-preview1", ] @@ -1264,7 +1345,7 @@ version = "0.2.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c05aeb6a22b8f62540c194aac980f2115af067bfe15a0734d7277a768d396b31" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "libc", "wasi 0.11.0+wasi-snapshot-preview1", ] @@ -1286,7 +1367,7 @@ dependencies = [ "color-eyre", "console", "cursive_core", - "esl01-dag", + "esl01-dag 0.3.0", "eyre", "fslock", "git-branchless-hook", @@ -1306,7 +1387,7 @@ dependencies = [ "git-branchless-undo", "git-record", "insta", - "itertools", + "itertools 0.10.5", "lazy_static", "man", "num_cpus", @@ -1333,7 +1414,7 @@ dependencies = [ "git-branchless-invoke", "git-branchless-lib", "git-branchless-opts", - "itertools", + "itertools 0.10.5", "lazy_static", "regex", "tracing", @@ -1349,7 +1430,7 @@ dependencies = [ "git-branchless-invoke", "git-branchless-lib", "git-branchless-opts", - "itertools", + "itertools 0.10.5", "path-slash", "tracing", ] @@ -1387,14 +1468,14 @@ dependencies = [ "console", "criterion", "cursive", - "esl01-dag", + "esl01-dag 0.3.0", "eyre", "futures", "git-record", "git2", "indicatif", "insta", - "itertools", + "itertools 0.10.5", "lazy_static", "once_cell", "portable-pty", @@ -1416,7 +1497,7 @@ dependencies = [ name = "git-branchless-move" version = "0.7.0" dependencies = [ - "esl01-dag", + "esl01-dag 0.3.0", "eyre", "git-branchless-lib", "git-branchless-opts", @@ -1431,13 +1512,13 @@ name = "git-branchless-navigation" version = "0.7.0" dependencies = [ "cursive", - "esl01-dag", + "esl01-dag 0.3.0", "eyre", "git-branchless-lib", "git-branchless-opts", "git-branchless-revset", "git-branchless-smartlog", - "itertools", + "itertools 0.10.5", "skim", "tracing", ] @@ -1449,21 +1530,21 @@ dependencies = [ "clap 4.0.32", "clap_mangen", "git-branchless-lib", - "itertools", + "itertools 0.10.5", ] [[package]] name = "git-branchless-query" version = "0.7.0" dependencies = [ - "esl01-dag", + "esl01-dag 0.3.0", "eyre", "git-branchless-invoke", "git-branchless-lib", "git-branchless-opts", "git-branchless-revset", "insta", - "itertools", + "itertools 0.10.5", "tracing", ] @@ -1473,14 +1554,14 @@ version = "0.7.0" dependencies = [ "cursive", "cursive_buffered_backend", - "esl01-dag", + "esl01-dag 0.3.0", "eyre", "git-branchless-invoke", "git-branchless-lib", "git-branchless-opts", "git-record", "insta", - "itertools", + "itertools 0.10.5", "rayon", "tracing", ] @@ -1493,14 +1574,14 @@ dependencies = [ "chrono", "chrono-english", "chronoutil", - "esl01-dag", + "esl01-dag 0.3.0", "eyre", "futures", "git-branchless-lib", "git-branchless-opts", "glob", "insta", - "itertools", + "itertools 0.10.5", "lalrpop", "lalrpop-util", "lazy_static", @@ -1517,7 +1598,7 @@ version = "0.7.0" dependencies = [ "bstr", "chrono", - "esl01-dag", + "esl01-dag 0.3.0", "eyre", "git-branchless-lib", "git-branchless-opts", @@ -1534,7 +1615,7 @@ name = "git-branchless-smartlog" version = "0.7.0" dependencies = [ "cursive_core", - "esl01-dag", + "esl01-dag 0.3.0", "eyre", "git-branchless-invoke", "git-branchless-lib", @@ -1548,15 +1629,23 @@ dependencies = [ name = "git-branchless-submit" version = "0.7.0" dependencies = [ + "clap 4.0.32", "cursive_core", + "esl01-dag 0.2.1", "eyre", "git-branchless-invoke", "git-branchless-lib", "git-branchless-opts", "git-branchless-revset", + "git-branchless-test", "insta", - "itertools", + "itertools 0.10.5", "lazy_static", + "rayon", + "regex", + "serde", + "serde_json", + "thiserror", "tracing", ] @@ -1569,7 +1658,7 @@ dependencies = [ "clap 4.0.32", "crossbeam", "cursive", - "esl01-dag", + "esl01-dag 0.3.0", "eyre", "fslock", "git-branchless-invoke", @@ -1578,7 +1667,7 @@ dependencies = [ "git-branchless-revset", "indexmap", "insta", - "itertools", + "itertools 0.10.5", "lazy_static", "maplit", "num_cpus", @@ -1808,7 +1897,7 @@ version = "0.1.12" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7a5bbe824c507c5da5956355e86a746d82e0e1464f65d862cc5e71da70e94b2c" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", ] [[package]] @@ -1842,6 +1931,15 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "itertools" +version = "0.8.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f56a2d0bc861f9165be4eb3442afd3c236d8a98afd426f65d92324ae1091a484" +dependencies = [ + "either", +] + [[package]] name = "itertools" version = "0.10.5" @@ -1892,7 +1990,7 @@ dependencies = [ "bit-set", "diff", "ena", - "itertools", + "itertools 0.10.5", "lalrpop-util", "petgraph", "pico-args", @@ -1987,6 +2085,15 @@ version = "0.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f051f77a7c8e6957c0696eac88f26b0117e54f52d3fc682ab19397a8812846a4" +[[package]] +name = "lock_api" +version = "0.3.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c4da24a77a3d8a6d4862d95f72e6fdb9c09a643ecdb402d754004a557f2bec75" +dependencies = [ + "scopeguard", +] + [[package]] name = "lock_api" version = "0.4.9" @@ -2003,7 +2110,7 @@ version = "0.4.17" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "abb12e687cfb44aa40f41fc3978ef76448f9b6038cad6aef4259d3c095a2382e" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", ] [[package]] @@ -2104,7 +2211,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fa52e972a9a719cecb6864fb88568781eb706bac2cd1d4f04a648542dbf78069" dependencies = [ "bitflags", - "cfg-if", + "cfg-if 1.0.0", "libc", ] @@ -2116,7 +2223,7 @@ checksum = "f346ff70e7dbfd675fe90590b92d59ef2de15a8779ae305ebcbfd3f0caf59be4" dependencies = [ "autocfg", "bitflags", - "cfg-if", + "cfg-if 1.0.0", "libc", "memoffset 0.6.5", "pin-utils", @@ -2272,13 +2379,23 @@ dependencies = [ "parking_lot_core 0.2.14", ] +[[package]] +name = "parking_lot" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d3a704eb390aafdc107b0e392f56a82b668e3a71366993b5340f5833fd62505e" +dependencies = [ + "lock_api 0.3.4", + "parking_lot_core 0.7.3", +] + [[package]] name = "parking_lot" version = "0.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3742b2c103b9f06bc9fff0a37ff4912935851bee6d36f3c02bcc755bcfec228f" dependencies = [ - "lock_api", + "lock_api 0.4.9", "parking_lot_core 0.9.5", ] @@ -2294,15 +2411,29 @@ dependencies = [ "winapi", ] +[[package]] +name = "parking_lot_core" +version = "0.7.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b93f386bb233083c799e6e642a9d73db98c24a5deeb95ffc85bf281255dffc98" +dependencies = [ + "cfg-if 0.1.10", + "cloudabi", + "libc", + "redox_syscall 0.1.57", + "smallvec 1.10.0", + "winapi", +] + [[package]] name = "parking_lot_core" version = "0.9.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7ff9f3fef3968a3ec5945535ed654cb38ff72d7495a25619e2247fb15a2ed9ba" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "libc", - "redox_syscall", + "redox_syscall 0.2.16", "smallvec 1.10.0", "windows-sys", ] @@ -2433,7 +2564,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "59230a63c37f3e18569bdb90e4a89cbf5bf8b06fea0b84e65ea10cc4df47addd" dependencies = [ "difflib", - "itertools", + "itertools 0.10.5", "predicates-core", ] @@ -2673,6 +2804,12 @@ dependencies = [ "rand_core 0.3.1", ] +[[package]] +name = "redox_syscall" +version = "0.1.57" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "41cc0f7e4d5d4544e8861606a285bb08d3e70712ccc7d2b84d7c0ccfaf4b05ce" + [[package]] name = "redox_syscall" version = "0.2.16" @@ -2689,7 +2826,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b033d837a7cf162d7993aded9304e30a83213c648b6e389db233191f891e5c2b" dependencies = [ "getrandom 0.2.8", - "redox_syscall", + "redox_syscall 0.2.16", "thiserror", ] @@ -2810,7 +2947,7 @@ version = "0.1.0" dependencies = [ "indexmap", "insta", - "itertools", + "itertools 0.10.5", "maplit", "proptest", "thiserror", @@ -3113,9 +3250,9 @@ version = "3.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "af18f7ae1acd354b992402e9ec5864359d693cd8a79dcbef59f76891701c1e95" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "fastrand", - "redox_syscall", + "redox_syscall 0.2.16", "rustix", "windows-sys", ] @@ -3284,7 +3421,7 @@ version = "0.1.37" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8ce8c33a8d48bd45d624a6e523445fd21ec13d3653cd51f681abf67418f54eb8" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "pin-project-lite", "tracing-attributes", "tracing-core", @@ -3395,7 +3532,7 @@ version = "1.6.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "97fee6b57c6a41524a810daee9286c02d7752c4253064d0b05472833a438f675" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "rand 0.8.5", "static_assertions", ] @@ -3567,7 +3704,7 @@ version = "0.2.83" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "eaf9f5aceeec8be17c128b2e93e031fb8a4d469bb9c4ae2d7dc1888b26887268" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "wasm-bindgen-macro", ] diff --git a/git-branchless-lib/src/core/effects.rs b/git-branchless-lib/src/core/effects.rs index 4b2d0a43a..74839e335 100644 --- a/git-branchless-lib/src/core/effects.rs +++ b/git-branchless-lib/src/core/effects.rs @@ -35,18 +35,19 @@ pub enum OperationType { InitializeRebase, MakeGraph, ProcessEvents, - PushBranches, + PushCommits, QueryWorkingCopy, ReadingFromCache, RebaseCommits, RepairBranches, RepairCommits, RunGitCommand(Arc), - RunTests(Arc), RunTestOnCommit(Arc), + RunTests(Arc), SortCommits, SyncCommits, UpdateCommitGraph, + UpdateCommits, WalkCommits, } @@ -70,7 +71,7 @@ impl ToString for OperationType { OperationType::GetUpstreamPatchIds => "Enumerating patch IDs", OperationType::InitializeRebase => "Initializing rebase", OperationType::MakeGraph => "Examining local history", - OperationType::PushBranches => "Pushing branches", + OperationType::PushCommits => "Pushing branches", OperationType::ProcessEvents => "Processing events", OperationType::QueryWorkingCopy => "Querying the working copy", OperationType::ReadingFromCache => "Reading from cache", @@ -84,6 +85,7 @@ impl ToString for OperationType { OperationType::RunTestOnCommit(commit) => return format!("Waiting to test {commit}"), OperationType::SortCommits => "Sorting commits", OperationType::SyncCommits => "Syncing commit stacks", + OperationType::UpdateCommits => "Updating commits", OperationType::UpdateCommitGraph => "Updating commit graph", OperationType::WalkCommits => "Walking commits", }; diff --git a/git-branchless-opts/src/lib.rs b/git-branchless-opts/src/lib.rs index a5cecc7f6..c68342ffc 100644 --- a/git-branchless-opts/src/lib.rs +++ b/git-branchless-opts/src/lib.rs @@ -365,6 +365,14 @@ pub struct SubmitArgs { #[clap(action, short = 'c', long = "create")] pub create: bool, + /// If the remote supports it, create code reviews in "draft" mode. + #[clap(action, short = 'd', long = "draft")] + pub draft: bool, + + /// What kind of execution strategy to use for tools which need access to the working copy. + #[clap(short = 's', long = "strategy")] + pub strategy: Option, + /// The commits to push. All branches attached to those commits will be /// pushed. #[clap(value_parser, default_value = "stack()")] diff --git a/git-branchless-submit/Cargo.toml b/git-branchless-submit/Cargo.toml index c9d2fbf68..895c812e7 100644 --- a/git-branchless-submit/Cargo.toml +++ b/git-branchless-submit/Cargo.toml @@ -10,16 +10,22 @@ version = "0.7.0" [dependencies] cursive_core = "0.3.6" +eden_dag = { package = "esl01-dag", version = "0.2.1" } eyre = "0.6.8" git-branchless-invoke = { version = "0.7.0", path = "../git-branchless-invoke" } git-branchless-opts = { version = "0.7.0", path = "../git-branchless-opts" } git-branchless-revset = { version = "0.7.0", path = "../git-branchless-revset" } +git-branchless-test = { version = "0.7.0", path = "../git-branchless-test" } itertools = "0.10.5" lazy_static = "1.4.0" lib = { package = "git-branchless-lib", version = "0.7.0", path = "../git-branchless-lib" } +rayon = "1.5.3" +regex = "1.7.1" +serde = { version = "1.0.152", features = ["derive"] } +serde_json = "1.0.93" +thiserror = "1.0.38" tracing = "0.1.37" [dev-dependencies] +clap = { version = "4.0.23", features = ["derive"] } insta = "1.28.0" - - diff --git a/git-branchless-submit/examples/dump_phabricator_dependencies.rs b/git-branchless-submit/examples/dump_phabricator_dependencies.rs new file mode 100644 index 000000000..8f823723a --- /dev/null +++ b/git-branchless-submit/examples/dump_phabricator_dependencies.rs @@ -0,0 +1,120 @@ +use std::{collections::HashSet, path::PathBuf}; + +use clap::Parser; +use git_branchless_opts::{ResolveRevsetOptions, Revset}; +use git_branchless_revset::resolve_commits; +use git_branchless_submit::phabricator; +use lib::core::dag::{sorted_commit_set, Dag}; +use lib::core::effects::Effects; +use lib::core::eventlog::{EventLogDb, EventReplayer}; +use lib::core::formatting::Glyphs; +use lib::core::repo_ext::RepoExt; +use lib::git::{GitRunInfo, NonZeroOid, Repo}; + +#[derive(Debug, Parser)] +struct Opts { + #[clap(short = 'C', long = "working-directory")] + working_directory: Option, + + #[clap(default_value = ".")] + revset: Revset, +} + +fn main() -> eyre::Result<()> { + let Opts { + working_directory, + revset, + } = Opts::try_parse()?; + match working_directory { + Some(working_directory) => { + std::env::set_current_dir(working_directory)?; + } + None => { + eprintln!("Warning: --working-directory was not passed, so running in current directory. But git-branchless is not hosted on Phabricator, so this seems unlikely to be useful, since there will be no assocated Phabricator information. Try running in your repository."); + } + } + + let effects = Effects::new(Glyphs::detect()); + let git_run_info = GitRunInfo { + path_to_git: "git".into(), + working_directory: std::env::current_dir()?, + env: Default::default(), + }; + let repo = Repo::from_current_dir()?; + let conn = repo.get_db_conn()?; + let event_log_db = EventLogDb::new(&conn)?; + let event_replayer = EventReplayer::from_event_log_db(&effects, &repo, &event_log_db)?; + let event_cursor = event_replayer.make_default_cursor(); + let references_snapshot = repo.get_references_snapshot()?; + let mut dag = Dag::open_and_sync( + &effects, + &repo, + &event_replayer, + event_cursor, + &references_snapshot, + )?; + + let resolved_commits = resolve_commits( + &effects, + &repo, + &mut dag, + &[revset.clone()], + &ResolveRevsetOptions { + show_hidden_commits: false, + }, + )?; + let commits = match resolved_commits.as_slice() { + [commits] => commits, + other => eyre::bail!("Unexpected number of returned commit sets for {revset}: {other:?}"), + }; + let commit_oids: HashSet = dag.commit_set_to_vec(commits)?.into_iter().collect(); + + let commits = sorted_commit_set(&repo, &dag, commits)?; + let phabricator = phabricator::PhabricatorForge { + effects: &effects, + git_run_info: &git_run_info, + repo: &repo, + dag: &mut dag, + event_log_db: &event_log_db, + revset: &revset, + }; + let dependency_oids = phabricator.query_remote_dependencies(commit_oids)?; + for commit in commits { + println!( + "Dependencies for {} {}:", + revision_id_str(&phabricator, commit.get_oid())?, + effects + .get_glyphs() + .render(commit.friendly_describe(effects.get_glyphs())?)? + ); + + let dependency_oids = &dependency_oids[&commit.get_oid()]; + if dependency_oids.is_empty() { + println!("- "); + } else { + for dependency_oid in dependency_oids { + let dependency_commit = repo.find_commit_or_fail(*dependency_oid)?; + println!( + "- {} {}", + revision_id_str(&phabricator, *dependency_oid)?, + effects + .get_glyphs() + .render(dependency_commit.friendly_describe(effects.get_glyphs())?)? + ); + } + } + } + Ok(()) +} + +fn revision_id_str( + phabricator: &phabricator::PhabricatorForge, + commit_oid: NonZeroOid, +) -> eyre::Result { + let id = phabricator.get_revision_id(commit_oid)?; + let revision_id = match id.as_ref() { + Some(phabricator::Id(id)) => id, + None => "???", + }; + Ok(format!("D{revision_id}")) +} diff --git a/git-branchless-submit/src/branch_push_forge.rs b/git-branchless-submit/src/branch_push_forge.rs index 90ea0fcfa..3f2430cb0 100644 --- a/git-branchless-submit/src/branch_push_forge.rs +++ b/git-branchless-submit/src/branch_push_forge.rs @@ -14,7 +14,7 @@ use lib::git::{ use lib::util::ExitCode; use tracing::warn; -use crate::{CommitStatus, SubmitOptions, SubmitStatus}; +use crate::{CommitStatus, Forge, SubmitOptions, SubmitStatus}; pub struct BranchPushForge<'a> { pub effects: &'a Effects, @@ -25,9 +25,9 @@ pub struct BranchPushForge<'a> { pub references_snapshot: &'a RepoReferencesSnapshot, } -impl BranchPushForge<'_> { - pub fn query_status( - &self, +impl Forge for BranchPushForge<'_> { + fn query_status( + &mut self, commit_set: CommitSet, ) -> eyre::Result, ExitCode>> { struct BranchInfo<'a> { @@ -168,7 +168,7 @@ impl BranchPushForge<'_> { }, _branch_infos => CommitStatus { - submit_status: SubmitStatus::Unresolved, + submit_status: SubmitStatus::Unknown, remote_name: None, local_branch_name: None, remote_branch_name: None, @@ -180,8 +180,8 @@ impl BranchPushForge<'_> { Ok(Ok(commit_statuses)) } - pub fn create( - &self, + fn create( + &mut self, commits: HashMap, _options: &SubmitOptions, ) -> eyre::Result { @@ -230,7 +230,7 @@ These remotes are available: {}", let event_tx_id = self .event_log_db .make_transaction_id(SystemTime::now(), "submit unsubmitted commits")?; - let (effects, progress) = self.effects.start_operation(OperationType::PushBranches); + let (effects, progress) = self.effects.start_operation(OperationType::PushCommits); let _effects = effects; progress.notify_progress(0, unsubmitted_branch_names.len()); self.git_run_info @@ -238,8 +238,8 @@ These remotes are available: {}", } } - pub fn update( - &self, + fn update( + &mut self, commits: HashMap, _options: &SubmitOptions, ) -> eyre::Result { @@ -267,7 +267,7 @@ These remotes are available: {}", let now = SystemTime::now(); let event_tx_id = self.event_log_db.make_transaction_id(now, "submit")?; - let (effects, progress) = self.effects.start_operation(OperationType::PushBranches); + let (effects, progress) = self.effects.start_operation(OperationType::PushCommits); let total_num_branches = branches_by_remote .values() .map(|branch_names| branch_names.len()) diff --git a/git-branchless-submit/src/lib.rs b/git-branchless-submit/src/lib.rs index 576d69607..31b2c877b 100644 --- a/git-branchless-submit/src/lib.rs +++ b/git-branchless-submit/src/lib.rs @@ -10,25 +10,29 @@ #![allow(clippy::too_many_arguments, clippy::blocks_in_if_conditions)] mod branch_push_forge; +pub mod phabricator; use std::collections::{BTreeSet, HashMap}; use std::fmt::Write; +use std::time::SystemTime; use branch_push_forge::BranchPushForge; use cursive_core::theme::{BaseColor, Effect, Style}; use git_branchless_invoke::CommandContext; +use git_branchless_test::{RawTestOptions, ResolvedTestOptions, Verbosity}; use itertools::Itertools; use lazy_static::lazy_static; -use lib::core::dag::Dag; +use lib::core::dag::{CommitSet, Dag}; use lib::core::effects::Effects; use lib::core::eventlog::{EventLogDb, EventReplayer}; use lib::core::formatting::{Pluralize, StyledStringBuilder}; -use lib::core::repo_ext::RepoExt; +use lib::core::repo_ext::{RepoExt, RepoReferencesSnapshot}; use lib::git::{GitRunInfo, NonZeroOid, Repo}; use lib::util::ExitCode; -use git_branchless_opts::{ResolveRevsetOptions, Revset, SubmitArgs}; +use git_branchless_opts::{ResolveRevsetOptions, Revset, SubmitArgs, TestExecutionStrategy}; use git_branchless_revset::resolve_commits; +use phabricator::PhabricatorForge; lazy_static! { static ref STYLE_PUSHED: Style = @@ -44,7 +48,7 @@ pub enum SubmitStatus { Unsubmitted, /// It could not be determined whether the remote commit exists. - Unresolved, + Unknown, /// The same commit exists both locally and remotely. UpToDate, @@ -65,6 +69,7 @@ pub struct CommitStatus { } /// Options for submitting commits to the forge. +#[derive(Clone, Debug)] pub struct SubmitOptions { /// Create associated branches, code reviews, etc. for each of the provided commits. /// @@ -72,6 +77,46 @@ pub struct SubmitOptions { /// and submitting a commit which already has an associated remote item /// should not have any additional effect. pub create: bool, + + /// When creating new code reviews for the currently-submitting commits, + /// configure those code reviews to indicate that they are not yet ready for + /// review. + /// + /// If a "draft" state is not meaningful for the forge, then has no effect. + /// If a given commit is already submitted, then has no effect for that + /// commit's code review. + pub draft: bool, + + /// For implementations which need to use the working copy to create the + /// code review, the appropriate execution strategy to do so. + pub execution_strategy: TestExecutionStrategy, + + /// The number of jobs to use when submitting commits. + pub num_jobs: usize, +} + +/// "Forge" refers to a Git hosting provider, such as GitHub, GitLab, etc. +/// Commits can be pushed for review to a forge. +pub trait Forge { + /// Get the status of the provided commits. + fn query_status( + &mut self, + commit_set: CommitSet, + ) -> eyre::Result, ExitCode>>; + + /// Submit the provided set of commits for review. + fn create( + &mut self, + commits: HashMap, + options: &SubmitOptions, + ) -> eyre::Result; + + /// Update existing remote commits to match their local versions. + fn update( + &mut self, + commits: HashMap, + options: &SubmitOptions, + ) -> eyre::Result; } /// `submit` command. @@ -82,6 +127,8 @@ pub fn command_main(ctx: CommandContext, args: SubmitArgs) -> eyre::Result eyre::Result, ) -> eyre::Result { let repo = Repo::from_current_dir()?; let conn = repo.get_db_conn()?; @@ -115,24 +166,79 @@ fn submit( &references_snapshot, )?; - let commit_set = - match resolve_commits(effects, &repo, &mut dag, &[revset], resolve_revset_options) { - Ok(mut commit_sets) => commit_sets.pop().unwrap(), - Err(err) => { - err.describe(effects)?; - return Ok(ExitCode(1)); + let commit_set = match resolve_commits( + effects, + &repo, + &mut dag, + &[revset.clone()], + resolve_revset_options, + ) { + Ok(mut commit_sets) => commit_sets.pop().unwrap(), + Err(err) => { + err.describe(effects)?; + return Ok(ExitCode(1)); + } + }; + + let raw_test_options = RawTestOptions { + exec: Some("".to_string()), + command: None, + dry_run: false, + strategy: execution_strategy, + search: None, + bisect: false, + no_cache: true, + interactive: false, + jobs: None, + verbosity: Verbosity::None, + apply_fixes: false, + }; + let ResolvedTestOptions { + command: _, + execution_strategy, + search_strategy: _, + is_dry_run: _, + use_cache: _, + is_interactive: _, + num_jobs, + verbosity: _, + fix_options: _, + } = { + let now = SystemTime::now(); + let event_tx_id = + event_log_db.make_transaction_id(now, "resolve test options for submit")?; + match ResolvedTestOptions::resolve( + now, + effects, + &dag, + &repo, + event_tx_id, + &commit_set, + None, + &raw_test_options, + )? { + Ok(resolved_test_options) => resolved_test_options, + Err(exit_code) => { + return Ok(exit_code); } - }; + } + }; + let submit_options = SubmitOptions { + create, + draft, + execution_strategy, + num_jobs, + }; - let submit_options = SubmitOptions { create }; - let forge = BranchPushForge { + let mut forge = select_forge( effects, git_run_info, - repo: &repo, - dag: &dag, - event_log_db: &event_log_db, - references_snapshot: &references_snapshot, - }; + &repo, + &mut dag, + &event_log_db, + &references_snapshot, + &revset, + ); let statuses = match forge.query_status(commit_set)? { Ok(statuses) => statuses, Err(exit_code) => return Ok(exit_code), @@ -149,21 +255,12 @@ fn submit( CommitStatus { submit_status: SubmitStatus::Unsubmitted, remote_name: _, - local_branch_name: Some(_), + local_branch_name: _, remote_branch_name: _, } => { unsubmitted.insert(commit_oid, commit_status); } - // Not currently implemented: generating a branch for - // unsubmitted commits which don't yet have branches. - CommitStatus { - submit_status: SubmitStatus::Unsubmitted, - remote_name: _, - local_branch_name: None, - remote_branch_name: _, - } => {} - CommitStatus { submit_status: SubmitStatus::NeedsUpdate, remote_name: _, @@ -184,7 +281,7 @@ fn submit( // Don't know what to do in these cases 🙃. CommitStatus { - submit_status: SubmitStatus::Unresolved, + submit_status: SubmitStatus::Unknown, remote_name: _, local_branch_name: _, remote_branch_name: _, @@ -331,3 +428,34 @@ create and push them, retry this operation with the --create option." Ok(ExitCode(0)) } + +fn select_forge<'a>( + effects: &'a Effects, + git_run_info: &'a GitRunInfo, + repo: &'a Repo, + dag: &'a mut Dag, + event_log_db: &'a EventLogDb, + references_snapshot: &'a RepoReferencesSnapshot, + revset: &'a Revset, +) -> Box { + if let Some(working_copy_path) = repo.get_working_copy_path() { + if working_copy_path.join(".arcconfig").is_file() { + return Box::new(PhabricatorForge { + effects, + git_run_info, + repo, + dag, + event_log_db, + revset, + }); + } + } + Box::new(BranchPushForge { + effects, + git_run_info, + repo, + dag, + event_log_db, + references_snapshot, + }) +} diff --git a/git-branchless-submit/src/phabricator.rs b/git-branchless-submit/src/phabricator.rs new file mode 100644 index 000000000..d85f56f00 --- /dev/null +++ b/git-branchless-submit/src/phabricator.rs @@ -0,0 +1,738 @@ +//! Phabricator backend for submitting patch stacks. + +use std::collections::{HashMap, HashSet}; +use std::fmt::Write; +use std::io; +use std::path::PathBuf; +use std::process::{Command, Stdio}; +use std::time::SystemTime; + +use git_branchless_opts::Revset; +use git_branchless_test::{ + run_tests, FixInfo, ResolvedTestOptions, TestResults, TestStatus, TestingAbortedError, + Verbosity, +}; +use itertools::Itertools; +use lazy_static::lazy_static; +use lib::core::check_out::CheckOutCommitOptions; +use lib::core::dag::{CommitSet, Dag}; +use lib::core::effects::{Effects, OperationType}; +use lib::core::eventlog::EventLogDb; +use lib::core::rewrite::{ + execute_rebase_plan, BuildRebasePlanError, BuildRebasePlanOptions, ExecuteRebasePlanOptions, + ExecuteRebasePlanResult, RebasePlanBuilder, RebasePlanPermissions, RepoResource, +}; +use lib::git::{Commit, GitRunInfo, MaybeZeroOid, NonZeroOid, Repo, RepoError}; +use lib::util::ExitCode; +use rayon::ThreadPoolBuilder; +use regex::bytes::Regex; +use serde::{Deserialize, Serialize}; +use thiserror::Error; +use tracing::{instrument, warn}; + +use crate::{CommitStatus, Forge, SubmitOptions, SubmitStatus}; + +/// Wrapper around the Phabricator "ID" type. (This is *not* a PHID, just a +/// regular ID). +#[derive(Clone, Debug, Serialize, Deserialize, Eq, Hash, PartialEq)] +#[serde(transparent)] +pub struct Id(pub String); + +#[derive(Clone, Debug, Serialize, Deserialize, Eq, Hash, PartialEq)] +#[serde(transparent)] +struct Phid(pub String); + +#[derive(Debug, Default, Serialize, Eq, PartialEq)] +struct DifferentialQueryRequest { + ids: Vec, + phids: Vec, +} + +#[derive(Debug, Serialize, Eq, PartialEq)] +struct DifferentialEditRequest { + #[serde(rename = "objectIdentifier")] + id: Id, // could also be a PHID + transactions: Vec, +} + +#[derive(Debug, Default, Serialize, Eq, PartialEq)] +struct DifferentialEditTransaction { + r#type: String, + value: Vec, +} + +#[derive(Debug, Deserialize)] +struct ConduitResponse { + #[serde(rename = "errorMessage")] + #[allow(dead_code)] + error_message: Option, + response: T, +} + +impl Default for ConduitResponse { + fn default() -> Self { + Self { + error_message: Default::default(), + response: Default::default(), + } + } +} + +#[derive(Debug, Deserialize)] +struct DifferentialQueryRevisionResponse { + id: Id, + phid: Phid, + + #[serde(default)] + auxiliary: DifferentialQueryAuxiliaryResponse, +} + +#[derive(Debug, Default, Deserialize)] +struct DifferentialQueryAuxiliaryResponse { + // TODO: add `default` + #[serde(rename = "phabricator:depends-on")] + phabricator_depends_on: Vec, +} + +/// Error type. +#[allow(missing_docs)] +#[derive(Debug, Error)] +pub enum Error { + #[error("no working copy for repository at path: {}", .repo_path.display())] + NoWorkingCopy { repo_path: PathBuf }, + + #[error("could not iterate commits: {0}")] + IterCommits(#[source] eyre::Error), + + #[error("could not look up commits: {0}")] + LookUpCommits(#[source] RepoError), + + #[error("no commit with hash {commit_oid:?}: {source}")] + NoSuchCommit { + source: RepoError, + commit_oid: NonZeroOid, + }, + + #[error("invocation to `arc {args}` failed: {source}", args = args.join(" "))] + InvokeArc { + source: io::Error, + args: Vec, + }, + + #[error("communication with `arc {args}` failed: {source}", args = args.join(" "))] + CommunicateWithArc { + source: serde_json::Error, + args: Vec, + }, + + #[error("could not create phab for {commit_oid} when running `arc {args}` (exit code {exit_code}): {message}", args = args.join(" "))] + CreatePhab { + exit_code: i32, + message: String, + commit_oid: NonZeroOid, + args: Vec, + }, + + #[error("could not query dependencies when running `arc {args}` (exit code {exit_code}): {message}", args = args.join(" "))] + QueryDependencies { + exit_code: i32, + message: String, + args: Vec, + }, + + #[error("could not update dependencies when running `arc {args}` (exit code {exit_code}): {message}", args = args.join(" "))] + UpdateDependencies { + exit_code: i32, + message: String, + args: Vec, + }, + + #[error("could not parse response when running `arc {args}`: {source}; with output: {output}", args = args.join(" "))] + ParseResponse { + source: serde_json::Error, + output: String, + args: Vec, + }, + + #[error("could not make transaction ID: {source}")] + MakeTransactionId { source: eyre::Error }, + + #[error("could not execute `arc diff` on commits: {source}")] + ExecuteArcDiff { source: eyre::Error }, + + #[error("could not verify permissions to rewrite commits: {source}")] + VerifyPermissions { source: eyre::Error }, + + #[error("could not build rebase plan")] + BuildRebasePlan(BuildRebasePlanError), + + #[error("failed to rewrite commits with exit code {}", exit_code.0)] + RewriteCommits { exit_code: ExitCode }, + + #[error(transparent)] + DagError(#[from] eden_dag::Error), +} + +/// Result type. +pub type Result = std::result::Result; + +/// The [Phabricator](https://en.wikipedia.org/wiki/Phabricator) code review system. +/// +/// Note that Phabricator is no longer actively maintained, but many +/// organizations still use it. +#[allow(missing_docs)] +#[derive(Debug)] +pub struct PhabricatorForge<'a> { + pub effects: &'a Effects, + pub git_run_info: &'a GitRunInfo, + pub repo: &'a Repo, + pub dag: &'a mut Dag, + pub event_log_db: &'a EventLogDb<'a>, + pub revset: &'a Revset, +} + +impl Forge for PhabricatorForge<'_> { + #[instrument] + fn query_status( + &mut self, + commit_set: CommitSet, + ) -> eyre::Result, ExitCode>> { + let commit_oids = self.dag.commit_set_to_vec(&commit_set)?; + let commit_oid_to_revision: HashMap> = commit_oids + .into_iter() + .map(|commit_oid| -> eyre::Result<_> { + let revision_id = self.get_revision_id(commit_oid)?; + Ok((commit_oid, revision_id)) + }) + .try_collect()?; + + let statuses = commit_oid_to_revision + .into_iter() + .map(|(commit_oid, id)| { + let status = CommitStatus { + submit_status: match id { + Some(_) => { + // TODO: could also be `UpToDate` + SubmitStatus::NeedsUpdate + } + None => SubmitStatus::Unsubmitted, + }, + remote_name: None, + local_branch_name: None, + remote_branch_name: None, + }; + (commit_oid, status) + }) + .collect(); + Ok(Ok(statuses)) + } + + #[instrument] + fn create( + &mut self, + commits: HashMap, + options: &SubmitOptions, + ) -> eyre::Result { + let SubmitOptions { + create: _, + draft, + execution_strategy, + num_jobs, + } = options; + + let commit_set = self.dag.sort(&commits.keys().copied().collect())?; + let commit_oids = self + .dag + .commit_set_to_vec(&commit_set) + .map_err(Error::IterCommits)?; + let commits: Vec = commit_oids + .iter() + .map(|commit_oid| self.repo.find_commit_or_fail(*commit_oid)) + .collect::>() + .map_err(Error::LookUpCommits)?; + let now = SystemTime::now(); + let event_tx_id = self + .event_log_db + .make_transaction_id(now, "arc diff") + .map_err(|err| Error::MakeTransactionId { source: err })?; + let build_options = BuildRebasePlanOptions { + force_rewrite_public_commits: false, + dump_rebase_constraints: false, + dump_rebase_plan: false, + detect_duplicate_commits_via_patch_id: false, + }; + let execute_options = ExecuteRebasePlanOptions { + now, + event_tx_id, + preserve_timestamps: true, + force_in_memory: true, + force_on_disk: false, + resolve_merge_conflicts: false, + check_out_commit_options: CheckOutCommitOptions { + render_smartlog: false, + ..Default::default() + }, + }; + let permissions = + RebasePlanPermissions::verify_rewrite_set(self.dag, build_options, &commit_set) + .map_err(|err| Error::VerifyPermissions { source: err })? + .map_err(Error::BuildRebasePlan)?; + let command = format!( + "arc diff --create --verbatim {} -- HEAD^", + if *draft { "--draft" } else { "" } + ); + let test_results = run_tests( + self.effects, + self.git_run_info, + self.dag, + self.repo, + self.event_log_db, + event_tx_id, + self.revset, + &commits, + &ResolvedTestOptions { + command, + execution_strategy: *execution_strategy, + search_strategy: None, + is_dry_run: false, + use_cache: false, + is_interactive: false, + num_jobs: *num_jobs, + verbosity: Verbosity::None, + fix_options: Some((execute_options.clone(), permissions.clone())), + }, + ) + .map_err(|err| Error::ExecuteArcDiff { source: err })? + .map_err(|exit_code| Error::RewriteCommits { exit_code })?; + let TestResults { + search_bounds: _, + test_outputs, + testing_aborted_error, + } = test_results; + if let Some(testing_aborted_error) = testing_aborted_error { + let TestingAbortedError { + commit_oid, + exit_code, + } = testing_aborted_error; + writeln!( + self.effects.get_output_stream(), + "Uploading was aborted with exit code {exit_code} due to commit {}", + self.effects.get_glyphs().render( + self.repo + .friendly_describe_commit_from_oid(self.effects.get_glyphs(), commit_oid)? + )?, + )?; + return Ok(ExitCode(1)); + } + + let rebase_plan = { + let mut builder = RebasePlanBuilder::new(self.dag, permissions); + for (commit_oid, test_output) in test_outputs { + let head_commit_oid = match test_output.test_status { + test_status @ (TestStatus::CheckoutFailed + | TestStatus::SpawnTestFailed(_) + | TestStatus::TerminatedBySignal + | TestStatus::AlreadyInProgress + | TestStatus::ReadCacheFailed(_) + | TestStatus::Indeterminate { .. } + | TestStatus::Abort { .. } + | TestStatus::Failed { .. }) => { + let commit = self.repo.find_commit_or_fail(commit_oid)?; + writeln!( + self.effects.get_output_stream(), + "{}", + self.effects.get_glyphs().render(test_status.describe( + self.effects.get_glyphs(), + &commit, + false + )?)?, + )?; + let stdout = std::fs::read_to_string(&test_output.stdout_path)?; + write!(self.effects.get_output_stream(), "Stdout:\n{stdout}")?; + let stderr = std::fs::read_to_string(&test_output.stderr_path)?; + write!(self.effects.get_output_stream(), "Stderr:\n{stderr}")?; + return Ok(ExitCode(1)); + } + TestStatus::Passed { + cached: _, + fix_info: + FixInfo { + head_commit_oid, + snapshot_tree_oid: _, + }, + interactive: _, + } => head_commit_oid, + }; + + let commit = self.repo.find_commit_or_fail(commit_oid)?; + builder.move_subtree(commit.get_oid(), commit.get_parent_oids())?; + builder.replace_commit(commit.get_oid(), head_commit_oid.unwrap_or(commit_oid))?; + } + + let pool = ThreadPoolBuilder::new().build()?; + let repo_pool = RepoResource::new_pool(self.repo)?; + match builder.build(self.effects, &pool, &repo_pool)? { + Ok(Some(rebase_plan)) => rebase_plan, + Ok(None) => return Ok(ExitCode(0)), + Err(err) => { + err.describe(self.effects, self.repo, self.dag)?; + return Ok(ExitCode(1)); + } + } + }; + + let rewritten_oids = match execute_rebase_plan( + self.effects, + self.git_run_info, + self.repo, + self.event_log_db, + &rebase_plan, + &execute_options, + )? { + ExecuteRebasePlanResult::Succeeded { + rewritten_oids: Some(rewritten_oids), + } => rewritten_oids, + ExecuteRebasePlanResult::Succeeded { + rewritten_oids: None, + } => { + warn!("No rewritten commit OIDs were produced by rebase plan execution"); + Default::default() + } + ExecuteRebasePlanResult::DeclinedToMerge { + failed_merge_info: _, + } => { + writeln!( + self.effects.get_error_stream(), + "BUG: Merge failed, but rewording shouldn't cause any merge failures." + )?; + return Ok(ExitCode(1)); + } + ExecuteRebasePlanResult::Failed { exit_code } => { + return Ok(exit_code); + } + }; + + let latest_commit_oids = commit_oids + .iter() + .copied() + .map(|commit_oid| match rewritten_oids.get(&commit_oid) { + Some(MaybeZeroOid::NonZero(commit_oid)) => *commit_oid, + Some(MaybeZeroOid::Zero) => { + warn!(?commit_oid, "Commit was rewritten to the zero OID",); + commit_oid + } + None => commit_oid, + }) + .collect_vec(); + self.update_dependencies(&latest_commit_oids.into_iter().collect())?; + + Ok(ExitCode(0)) + } + + #[instrument] + fn update( + &mut self, + commits: HashMap, + options: &SubmitOptions, + ) -> eyre::Result { + let commit_set = commits.keys().copied().collect(); + self.update_dependencies(&commit_set)?; + Ok(ExitCode(0)) + } +} + +impl PhabricatorForge<'_> { + fn query_revisions( + &self, + request: &DifferentialQueryRequest, + ) -> Result>> { + // The API call seems to hang if we don't specify any IDs; perhaps it's + // fetching everything? + if request == &DifferentialQueryRequest::default() { + return Ok(Default::default()); + } + + let args = vec![ + "call-conduit".to_string(), + "--".to_string(), + "differential.query".to_string(), + ]; + let mut child = Command::new("arc") + .args(&args) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::inherit()) + .spawn() + .map_err(|err| Error::InvokeArc { + source: err, + args: args.clone(), + })?; + serde_json::to_writer_pretty(child.stdin.take().unwrap(), request).map_err(|err| { + Error::CommunicateWithArc { + source: err, + args: args.clone(), + } + })?; + let result = child.wait_with_output().map_err(|err| Error::InvokeArc { + source: err, + args: args.clone(), + })?; + if !result.status.success() { + return Err(Error::QueryDependencies { + exit_code: result.status.code().unwrap_or(-1), + message: String::from_utf8_lossy(&result.stdout).into_owned(), + args, + }); + } + + let output: ConduitResponse> = + serde_json::from_slice(&result.stdout).map_err(|err| Error::ParseResponse { + source: err, + output: String::from_utf8_lossy(&result.stdout).into_owned(), + args: args.clone(), + })?; + Ok(output) + } + + /// Query the dependencies of a set of commits from Phabricator (not locally). + pub fn query_remote_dependencies( + &self, + commit_oids: HashSet, + ) -> Result>> { + // Convert commit hashes to IDs. + let commit_oid_to_id: HashMap> = { + let mut result = HashMap::new(); + for commit_oid in commit_oids.iter().copied() { + let revision_id = self.get_revision_id(commit_oid)?; + result.insert(commit_oid, revision_id); + } + result + }; + + // Get the reverse mapping of IDs to commit hashes. Note that not every commit + // hash will have an ID -- specifically those which haven't been submitted yet. + let id_to_commit_oid: HashMap = commit_oid_to_id + .iter() + .filter_map(|(commit_oid, id)| id.as_ref().map(|v| (v.clone(), *commit_oid))) + .collect(); + + // Query for revision information by ID. + let query_ids: Vec = commit_oid_to_id + .values() + .filter_map(|id| id.as_ref().cloned()) + .collect(); + + let ConduitResponse { + error_message: _, + response: revisions, + } = self.query_revisions(&DifferentialQueryRequest { + ids: query_ids, + phids: Default::default(), + })?; + + // Get the dependency PHIDs for each revision ID. + let dependency_phids: HashMap> = revisions + .into_iter() + .map(|revision| { + let DifferentialQueryRevisionResponse { + id, + phid: _, + auxiliary: + DifferentialQueryAuxiliaryResponse { + phabricator_depends_on, + }, + } = revision; + (id, phabricator_depends_on) + }) + .collect(); + + // Convert the dependency PHIDs back into revision IDs. + let dependency_ids: HashMap> = { + let all_phids: Vec = dependency_phids.values().flatten().cloned().collect(); + let ConduitResponse { + error_message: _, + response: revisions, + } = self.query_revisions(&DifferentialQueryRequest { + ids: Default::default(), + phids: all_phids, + })?; + let phid_to_id: HashMap = revisions + .into_iter() + .map(|revision| { + let DifferentialQueryRevisionResponse { + id, + phid, + auxiliary: _, + } = revision; + (phid, id) + }) + .collect(); + dependency_phids + .into_iter() + .map(|(id, dependency_phids)| { + ( + id, + dependency_phids + .into_iter() + .filter_map(|dependency_phid| phid_to_id.get(&dependency_phid)) + .cloned() + .collect(), + ) + }) + .collect() + }; + + // Use the looked-up IDs to convert the commit dependencies. Note that + // there may be dependencies not expressed in the set of commits, in + // which case... FIXME. + let result: HashMap> = commit_oid_to_id + .into_iter() + .map(|(commit_oid, id)| { + let dependency_ids = match id { + None => Default::default(), + Some(id) => match dependency_ids.get(&id) { + None => Default::default(), + Some(dependency_ids) => dependency_ids + .iter() + .filter_map(|dependency_id| id_to_commit_oid.get(dependency_id)) + .copied() + .collect(), + }, + }; + (commit_oid, dependency_ids) + }) + .collect(); + Ok(result) + } + + fn update_dependencies(&self, commits: &CommitSet) -> eyre::Result<()> { + // Make sure to update dependencies in topological order to prevent + // dependency cycles. + let commits = self.dag.sort(commits)?; + let commit_oids = self.dag.commit_set_to_vec(&commits)?; + // `.sort` seems to sort it such that the child-most commits are first? + // We need to start with the parent commits. + let commit_oids = commit_oids.into_iter().rev().collect_vec(); + + let (effects, progress) = self.effects.start_operation(OperationType::UpdateCommits); + let _effects = effects; + + let draft_commits = self.dag.query_draft_commits()?; + progress.notify_progress(0, commit_oids.len()); + for commit_oid in commit_oids { + let id = match self.get_revision_id(commit_oid)? { + Some(id) => id, + None => { + warn!(?commit_oid, "No Phabricator commit ID for latest commit"); + continue; + } + }; + let commit = self.repo.find_commit_or_fail(commit_oid)?; + let parent_oids = commit.get_parent_oids(); + + let mut parent_revision_ids = Vec::new(); + for parent_oid in parent_oids { + if !self.dag.set_contains(draft_commits, parent_oid)? { + // FIXME: this will exclude commits that used to be part of + // the stack but have since landed. + continue; + } + let parent_revision_id = match self.get_revision_id(parent_oid)? { + Some(id) => id, + None => continue, + }; + parent_revision_ids.push(parent_revision_id); + } + self.set_dependencies(id, parent_revision_ids)?; + progress.notify_progress_inc(1); + } + Ok(()) + } + + fn set_dependencies(&self, id: Id, parent_revision_ids: Vec) -> Result<()> { + let ConduitResponse { + error_message: _, + response, + } = self.query_revisions(&DifferentialQueryRequest { + ids: parent_revision_ids, + phids: Default::default(), + })?; + let parent_revision_phids: Vec = + response.into_iter().map(|response| response.phid).collect(); + let request = DifferentialEditRequest { + id, + transactions: vec![DifferentialEditTransaction { + r#type: "parents.set".to_string(), + value: parent_revision_phids, + }], + }; + + let args = vec![ + "call-conduit".to_string(), + "--".to_string(), + "differential.revision.edit".to_string(), + ]; + let mut child = Command::new("arc") + .args(&args) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::inherit()) + .spawn() + .map_err(|err| Error::InvokeArc { + source: err, + args: args.clone(), + })?; + serde_json::to_writer_pretty(child.stdin.take().unwrap(), &request).map_err(|err| { + Error::CommunicateWithArc { + source: err, + args: args.clone(), + } + })?; + let result = child.wait_with_output().map_err(|err| Error::InvokeArc { + source: err, + args: args.clone(), + })?; + if !result.status.success() { + return Err(Error::UpdateDependencies { + exit_code: result.status.code().unwrap_or(-1), + message: String::from_utf8_lossy(&result.stdout).into_owned(), + args, + }); + } + + Ok(()) + } + + /// Given a commit for D123, returns a string like "123" by parsing the + /// commit message. + pub fn get_revision_id(&self, commit_oid: NonZeroOid) -> Result> { + let commit = + self.repo + .find_commit_or_fail(commit_oid) + .map_err(|err| Error::NoSuchCommit { + source: err, + commit_oid, + })?; + let message = commit.get_message_raw(); + + lazy_static! { + static ref RE: Regex = Regex::new( + r"(?mx) +^ +Differential[\ ]Revision:[\ ] + (.+ /)? + D(?P[0-9]+) +$", + ) + .expect("Failed to compile `extract_diff_number` regex"); + } + let captures = match RE.captures(message.as_slice()) { + Some(captures) => captures, + None => return Ok(None), + }; + let diff_number = &captures["diff"]; + let diff_number = String::from_utf8(diff_number.to_vec()) + .expect("Regex should have confirmed that this string was only ASCII digits"); + Ok(Some(Id(diff_number))) + } +} diff --git a/git-branchless-test/src/lib.rs b/git-branchless-test/src/lib.rs index 8b2693dfe..293f1e3e3 100644 --- a/git-branchless-test/src/lib.rs +++ b/git-branchless-test/src/lib.rs @@ -80,7 +80,7 @@ lazy_static! { /// How verbose of output to produce. #[derive(Clone, Copy, Debug, Ord, PartialOrd, Eq, PartialEq)] -enum Verbosity { +pub enum Verbosity { /// Do not include test output at all. None, @@ -104,7 +104,7 @@ impl From for Verbosity { /// The options for testing before they've assumed default values or been /// validated. #[derive(Debug)] -struct RawTestOptions { +pub struct RawTestOptions { /// The command to execute, if any. pub exec: Option, @@ -197,21 +197,25 @@ To run a specific command alias, run: git test run -c ", Ok(Err(ExitCode(1))) } +/// The values from a `RawTestOptions` but with defaults provided. See +/// [`RawTestOptions`] for details on these options. +#[allow(missing_docs)] #[derive(Debug)] -struct ResolvedTestOptions { - command: String, - execution_strategy: TestExecutionStrategy, - search_strategy: Option, - is_dry_run: bool, - use_cache: bool, - is_interactive: bool, - num_jobs: usize, - verbosity: Verbosity, - fix_options: Option<(ExecuteRebasePlanOptions, RebasePlanPermissions)>, +pub struct ResolvedTestOptions { + pub command: String, + pub execution_strategy: TestExecutionStrategy, + pub search_strategy: Option, + pub is_dry_run: bool, + pub use_cache: bool, + pub is_interactive: bool, + pub num_jobs: usize, + pub verbosity: Verbosity, + pub fix_options: Option<(ExecuteRebasePlanOptions, RebasePlanPermissions)>, } impl ResolvedTestOptions { - fn resolve( + /// Resolve any missing test options with their defaults from the environment. + pub fn resolve( now: SystemTime, effects: &Effects, dag: &Dag, @@ -812,21 +816,30 @@ fn clear_abort_trap( Ok(exit_code) } +/// The result of running a test. #[derive(Debug)] -struct TestOutput { +pub struct TestOutput { /// Only used for `--no-cache` invocations, to ensure that we don't clobber /// an existing cached entry. - _temp_dir: Option, + pub temp_dir: Option, - _result_path: PathBuf, - stdout_path: PathBuf, - stderr_path: PathBuf, - test_status: TestStatus, + /// The path to the file containing the serialized test result information + /// (such as the exit code). + pub result_path: PathBuf, + + /// The path to the file containing the stdout of the test command. + pub stdout_path: PathBuf, + + /// The path to the file containing the stderr of the test command. + pub stderr_path: PathBuf, + + /// The resulting status of the test. + pub test_status: TestStatus, } /// The possible results of attempting to run a test. #[derive(Clone, Debug)] -enum TestStatus { +pub enum TestStatus { /// Attempting to set up the working directory for the repository failed. CheckoutFailed, @@ -845,10 +858,16 @@ enum TestStatus { ReadCacheFailed(String), /// The test command indicated that the commit should be skipped for testing. - Indeterminate { exit_code: i32 }, + Indeterminate { + /// The exit code of the command. + exit_code: i32, + }, /// The test command indicated that the process should be aborted entirely. - Abort { exit_code: i32 }, + Abort { + /// The exit code of the command. + exit_code: i32, + }, /// The test failed and returned the provided (non-zero) exit code. Failed { @@ -881,16 +900,16 @@ enum TestStatus { /// Information about the working copy state after running the test command. #[derive(Clone, Debug)] -struct FixInfo { +pub struct FixInfo { /// The resulting commit which was checked out as `HEAD`. This is usually /// the same commit as was being tested, but the test command could amend or /// switch to a different commit. - head_commit_oid: Option, + pub head_commit_oid: Option, /// The resulting contents of the working copy after the command was /// executed (if taking a working copy snapshot succeeded and there were no /// merge conflicts, etc.). - snapshot_tree_oid: Option, + pub snapshot_tree_oid: Option, } impl TestStatus { @@ -921,132 +940,128 @@ impl TestStatus { TestStatus::Passed { .. } => *STYLE_SUCCESS, } } -} - -#[derive(Debug)] -struct TestingAbortedError { - commit_oid: NonZeroOid, - exit_code: i32, -} - -#[instrument] -fn make_test_status_description( - glyphs: &Glyphs, - commit: &Commit, - test_status: &TestStatus, - apply_fixes: bool, -) -> eyre::Result { - let description = match test_status { - TestStatus::CheckoutFailed => StyledStringBuilder::new() - .append_styled("Failed to check out: ", test_status.get_style()) - .append(commit.friendly_describe(glyphs)?) - .build(), - TestStatus::SpawnTestFailed(err) => StyledStringBuilder::new() - .append_styled( - format!("Failed to spawn test: {err}: "), - test_status.get_style(), - ) - .append(commit.friendly_describe(glyphs)?) - .build(), + /// Produce a friendly description of the test status. + #[instrument] + pub fn describe( + &self, + glyphs: &Glyphs, + commit: &Commit, + apply_fixes: bool, + ) -> eyre::Result { + let description = match self { + TestStatus::CheckoutFailed => StyledStringBuilder::new() + .append_styled("Failed to check out: ", self.get_style()) + .append(commit.friendly_describe(glyphs)?) + .build(), - TestStatus::TerminatedBySignal => StyledStringBuilder::new() - .append_styled( - "Test command terminated by signal: ", - test_status.get_style(), - ) - .append(commit.friendly_describe(glyphs)?) - .build(), + TestStatus::SpawnTestFailed(err) => StyledStringBuilder::new() + .append_styled(format!("Failed to spawn test: {err}: "), self.get_style()) + .append(commit.friendly_describe(glyphs)?) + .build(), - TestStatus::AlreadyInProgress => StyledStringBuilder::new() - .append_styled("Test already in progress? ", test_status.get_style()) - .append(commit.friendly_describe(glyphs)?) - .build(), + TestStatus::TerminatedBySignal => StyledStringBuilder::new() + .append_styled("Test command terminated by signal: ", self.get_style()) + .append(commit.friendly_describe(glyphs)?) + .build(), - TestStatus::ReadCacheFailed(_) => StyledStringBuilder::new() - .append_styled( - "Could not read cached test result: ", - test_status.get_style(), - ) - .append(commit.friendly_describe(glyphs)?) - .build(), + TestStatus::AlreadyInProgress => StyledStringBuilder::new() + .append_styled("Test already in progress? ", self.get_style()) + .append(commit.friendly_describe(glyphs)?) + .build(), - TestStatus::Indeterminate { exit_code } => StyledStringBuilder::new() - .append_styled( - format!("Exit code indicated to skip this commit (exit code {exit_code}): "), - test_status.get_style(), - ) - .append(commit.friendly_describe(glyphs)?) - .build(), + TestStatus::ReadCacheFailed(_) => StyledStringBuilder::new() + .append_styled("Could not read cached test result: ", self.get_style()) + .append(commit.friendly_describe(glyphs)?) + .build(), - TestStatus::Abort { exit_code } => StyledStringBuilder::new() - .append_styled( - format!("Exit code indicated to abort testing (exit code {exit_code}): "), - test_status.get_style(), - ) - .append(commit.friendly_describe(glyphs)?) - .build(), + TestStatus::Indeterminate { exit_code } => StyledStringBuilder::new() + .append_styled( + format!("Exit code indicated to skip this commit (exit code {exit_code}): "), + self.get_style(), + ) + .append(commit.friendly_describe(glyphs)?) + .build(), - TestStatus::Failed { - cached, - interactive, - exit_code, - } => { - let mut descriptors = Vec::new(); - if *cached { - descriptors.push("cached".to_string()); - } - descriptors.push(format!("exit code {exit_code}")); - if *interactive { - descriptors.push("interactive".to_string()); - } - let descriptors = descriptors.join(", "); - StyledStringBuilder::new() - .append_styled(format!("Failed ({descriptors}): "), test_status.get_style()) + TestStatus::Abort { exit_code } => StyledStringBuilder::new() + .append_styled( + format!("Exit code indicated to abort testing (exit code {exit_code}): "), + self.get_style(), + ) .append(commit.friendly_describe(glyphs)?) - .build() - } + .build(), - TestStatus::Passed { - cached, - interactive, - fix_info: - FixInfo { - head_commit_oid: _, - snapshot_tree_oid, - }, - } => { - let mut descriptors = Vec::new(); - if *cached { - descriptors.push("cached".to_string()); + TestStatus::Failed { + cached, + interactive, + exit_code, + } => { + let mut descriptors = Vec::new(); + if *cached { + descriptors.push("cached".to_string()); + } + descriptors.push(format!("exit code {exit_code}")); + if *interactive { + descriptors.push("interactive".to_string()); + } + let descriptors = descriptors.join(", "); + StyledStringBuilder::new() + .append_styled(format!("Failed ({descriptors}): "), self.get_style()) + .append(commit.friendly_describe(glyphs)?) + .build() } - match (snapshot_tree_oid, commit.get_tree_oid()) { - (Some(snapshot_tree_oid), MaybeZeroOid::NonZero(original_tree_oid)) => { - if *snapshot_tree_oid != original_tree_oid { - descriptors.push(if apply_fixes { - "fixed".to_string() - } else { - "fixable".to_string() - }); + + TestStatus::Passed { + cached, + interactive, + fix_info: + FixInfo { + head_commit_oid: _, + snapshot_tree_oid, + }, + } => { + let mut descriptors = Vec::new(); + if *cached { + descriptors.push("cached".to_string()); + } + match (snapshot_tree_oid, commit.get_tree_oid()) { + (Some(snapshot_tree_oid), MaybeZeroOid::NonZero(original_tree_oid)) => { + if *snapshot_tree_oid != original_tree_oid { + descriptors.push(if apply_fixes { + "fixed".to_string() + } else { + "fixable".to_string() + }); + } } + (None, _) | (_, MaybeZeroOid::Zero) => {} } - (None, _) | (_, MaybeZeroOid::Zero) => {} - } - if *interactive { - descriptors.push("interactive".to_string()); + if *interactive { + descriptors.push("interactive".to_string()); + } + let descriptors = if descriptors.is_empty() { + "".to_string() + } else { + format!(" ({})", descriptors.join(", ")) + }; + StyledStringBuilder::new() + .append_styled(format!("Passed{descriptors}: "), self.get_style()) + .append(commit.friendly_describe(glyphs)?) + .build() } - let descriptors = if descriptors.is_empty() { - "".to_string() - } else { - format!(" ({})", descriptors.join(", ")) - }; - StyledStringBuilder::new() - .append_styled(format!("Passed{descriptors}: "), test_status.get_style()) - .append(commit.friendly_describe(glyphs)?) - .build() - } - }; - Ok(description) + }; + Ok(description) + } +} + +/// An error produced when testing is aborted due to a certain commit. +#[derive(Debug)] +pub struct TestingAbortedError { + /// The commit which aborted testing. + pub commit_oid: NonZeroOid, + + /// The exit code of the test command when run on that commit. + pub exit_code: i32, } impl TestOutput { @@ -1061,12 +1076,10 @@ impl TestOutput { let description = StyledStringBuilder::new() .append_styled(self.test_status.get_icon(), self.test_status.get_style()) .append_plain(" ") - .append(make_test_status_description( - effects.get_glyphs(), - commit, - &self.test_status, - apply_fixes, - )?) + .append( + self.test_status + .describe(effects.get_glyphs(), commit, apply_fixes)?, + ) .build(); if verbosity == Verbosity::None { @@ -1220,15 +1233,23 @@ impl<'a> search::SearchGraph for SearchGraph<'a> { } } +/// The results of running all tests. #[derive(Debug)] -struct TestResults { - search_bounds: search::Bounds, - test_outputs: IndexMap, - testing_aborted_error: Option, +pub struct TestResults { + /// If a search strategy was provided, the calculated bounds on the input + /// commit set. + pub search_bounds: search::Bounds, + + /// The test output for each commit. + pub test_outputs: IndexMap, + + /// If testing was aborted, the corresponding error. + pub testing_aborted_error: Option, } +/// Run tests on the provided set of commits. #[instrument] -fn run_tests<'a>( +pub fn run_tests<'a>( effects: &Effects, git_run_info: &GitRunInfo, dag: &Dag, @@ -2200,8 +2221,8 @@ fn run_test( stderr_file: _, } = test_files; TestOutput { - _temp_dir: temp_dir, - _result_path: result_path, + temp_dir, + result_path, stdout_path, stderr_path, test_status: TestStatus::CheckoutFailed, @@ -2243,10 +2264,9 @@ fn run_test( }; let description = StyledStringBuilder::new() - .append(make_test_status_description( + .append(test_output.test_status.describe( effects.get_glyphs(), commit, - &test_output.test_status, fix_options.is_some(), )?) .build(); @@ -2347,8 +2367,8 @@ fn make_test_files( .wrap_err_with(|| format!("Locking file {lock_path:?}"))? { return Ok(TestFilesResult::Cached(TestOutput { - _temp_dir: None, - _result_path: result_path, + temp_dir: None, + result_path, stdout_path, stderr_path, test_status: TestStatus::AlreadyInProgress, @@ -2413,8 +2433,8 @@ fn make_test_files( Err(err) => TestStatus::ReadCacheFailed(err.to_string()), }; return Ok(TestFilesResult::Cached(TestOutput { - _temp_dir: None, - _result_path: result_path, + temp_dir: None, + result_path, stdout_path, stderr_path, test_status, @@ -2661,8 +2681,8 @@ To abort testing entirely, run: {exit127}", Ok(status) => status.code(), Err(err) => { return Ok(TestOutput { - _temp_dir: temp_dir, - _result_path: result_path, + temp_dir, + result_path, stdout_path, stderr_path, test_status: TestStatus::SpawnTestFailed(err.to_string()), @@ -2673,8 +2693,8 @@ To abort testing entirely, run: {exit127}", Some(exit_code) => exit_code, None => { return Ok(TestOutput { - _temp_dir: temp_dir, - _result_path: result_path, + temp_dir, + result_path, stdout_path, stderr_path, test_status: TestStatus::TerminatedBySignal, @@ -2762,8 +2782,8 @@ To abort testing entirely, run: {exit127}", .wrap_err_with(|| format!("Writing test status {test_status:?} to {result_path:?}"))?; Ok(TestOutput { - _temp_dir: temp_dir, - _result_path: result_path, + temp_dir, + result_path, stdout_path, stderr_path, test_status, From eddef8209bd83468dbe636d5891f3e2aa076254d Mon Sep 17 00:00:00 2001 From: Waleed Khan Date: Thu, 9 Mar 2023 22:22:31 -0500 Subject: [PATCH 7/8] feat(submit): automatically create branches after `git submit --create` (Only if the branches don't already exist.) --- .../src/branch_push_forge.rs | 30 ++++++-- git-branchless-submit/src/lib.rs | 49 +++++++++++-- git-branchless-submit/src/phabricator.rs | 71 +++++++++++++++---- 3 files changed, 122 insertions(+), 28 deletions(-) diff --git a/git-branchless-submit/src/branch_push_forge.rs b/git-branchless-submit/src/branch_push_forge.rs index 3f2430cb0..67e9e36f0 100644 --- a/git-branchless-submit/src/branch_push_forge.rs +++ b/git-branchless-submit/src/branch_push_forge.rs @@ -14,7 +14,7 @@ use lib::git::{ use lib::util::ExitCode; use tracing::warn; -use crate::{CommitStatus, Forge, SubmitOptions, SubmitStatus}; +use crate::{CommitStatus, CreateStatus, Forge, SubmitOptions, SubmitStatus}; pub struct BranchPushForge<'a> { pub effects: &'a Effects, @@ -184,7 +184,7 @@ impl Forge for BranchPushForge<'_> { &mut self, commits: HashMap, _options: &SubmitOptions, - ) -> eyre::Result { + ) -> eyre::Result, ExitCode>> { let unsubmitted_branch_names = commits .values() .filter_map(|commit_status| { @@ -216,12 +216,12 @@ These remotes are available: {}", unsubmitted_branch_names.join(", "), self.repo.get_all_remote_names()?.join(", "), )?; - return Ok(ExitCode(1)); + return Ok(Err(ExitCode(1))); } }; if unsubmitted_branch_names.is_empty() { - Ok(ExitCode(0)) + Ok(Ok(Default::default())) } else { // This will fail if somebody else created the branch on the remote and we don't // know about it. @@ -233,8 +233,26 @@ These remotes are available: {}", let (effects, progress) = self.effects.start_operation(OperationType::PushCommits); let _effects = effects; progress.notify_progress(0, unsubmitted_branch_names.len()); - self.git_run_info - .run(self.effects, Some(event_tx_id), &args) + let exit_code = self + .git_run_info + .run(self.effects, Some(event_tx_id), &args)?; + if !exit_code.is_success() { + return Ok(Err(exit_code)); + } + Ok(Ok(commits + .into_iter() + .filter_map(|(commit_oid, commit_status)| { + commit_status.local_branch_name.map(|local_branch_name| { + ( + commit_oid, + CreateStatus { + final_commit_oid: commit_oid, + local_branch_name, + }, + ) + }) + }) + .collect())) } } diff --git a/git-branchless-submit/src/lib.rs b/git-branchless-submit/src/lib.rs index 31b2c877b..9ca11a49e 100644 --- a/git-branchless-submit/src/lib.rs +++ b/git-branchless-submit/src/lib.rs @@ -12,7 +12,7 @@ mod branch_push_forge; pub mod phabricator; -use std::collections::{BTreeSet, HashMap}; +use std::collections::{BTreeSet, HashMap, HashSet}; use std::fmt::Write; use std::time::SystemTime; @@ -27,7 +27,7 @@ use lib::core::effects::Effects; use lib::core::eventlog::{EventLogDb, EventReplayer}; use lib::core::formatting::{Pluralize, StyledStringBuilder}; use lib::core::repo_ext::{RepoExt, RepoReferencesSnapshot}; -use lib::git::{GitRunInfo, NonZeroOid, Repo}; +use lib::git::{GitRunInfo, NonZeroOid, ReferenceName, Repo}; use lib::util::ExitCode; use git_branchless_opts::{ResolveRevsetOptions, Revset, SubmitArgs, TestExecutionStrategy}; @@ -95,6 +95,18 @@ pub struct SubmitOptions { pub num_jobs: usize, } +/// The result of creating a commit. +pub struct CreateStatus { + /// The commit OID after carrying out the creation process. Usually, this + /// will be the same as the original commit OID, unless the forge amends it + /// (e.g. to include a change ID). + pub final_commit_oid: NonZeroOid, + + /// The local branch name to use. The caller will try to create the branch + /// pointing to that commit (assuming that it doesn't already exist). + pub local_branch_name: String, +} + /// "Forge" refers to a Git hosting provider, such as GitHub, GitLab, etc. /// Commits can be pushed for review to a forge. pub trait Forge { @@ -109,7 +121,7 @@ pub trait Forge { &mut self, commits: HashMap, options: &SubmitOptions, - ) -> eyre::Result; + ) -> eyre::Result, ExitCode>>; /// Update existing remote commits to match their local versions. fn update( @@ -304,11 +316,34 @@ fn submit( if unsubmitted_commits.is_empty() { Default::default() } else if create { - let exit_code = forge.create(unsubmitted_commits, &submit_options)?; - if !exit_code.is_success() { - return Ok(exit_code); + let create_statuses = match forge.create(unsubmitted_commits, &submit_options)? { + Ok(create_statuses) => create_statuses, + Err(exit_code) => return Ok(exit_code), + }; + let all_branches: HashSet<_> = references_snapshot + .branch_oid_to_names + .values() + .flatten() + .collect(); + let mut created_branches = BTreeSet::new(); + for (_commit_oid, create_status) in create_statuses { + let CreateStatus { + final_commit_oid, + local_branch_name, + } = create_status; + let branch_reference_name = + ReferenceName::from(format!("refs/heads/{local_branch_name}")); + created_branches.insert(local_branch_name); + if !all_branches.contains(&branch_reference_name) { + repo.create_reference( + &branch_reference_name, + final_commit_oid, + false, + "submit", + )?; + } } - (unsubmitted_branches, Default::default()) + (created_branches, Default::default()) } else { (Default::default(), unsubmitted_branches) } diff --git a/git-branchless-submit/src/phabricator.rs b/git-branchless-submit/src/phabricator.rs index d85f56f00..f90b59304 100644 --- a/git-branchless-submit/src/phabricator.rs +++ b/git-branchless-submit/src/phabricator.rs @@ -30,7 +30,7 @@ use serde::{Deserialize, Serialize}; use thiserror::Error; use tracing::{instrument, warn}; -use crate::{CommitStatus, Forge, SubmitOptions, SubmitStatus}; +use crate::{CommitStatus, CreateStatus, Forge, SubmitOptions, SubmitStatus}; /// Wrapper around the Phabricator "ID" type. (This is *not* a PHID, just a /// regular ID). @@ -232,7 +232,7 @@ impl Forge for PhabricatorForge<'_> { &mut self, commits: HashMap, options: &SubmitOptions, - ) -> eyre::Result { + ) -> eyre::Result, ExitCode>> { let SubmitOptions { create: _, draft, @@ -322,7 +322,7 @@ impl Forge for PhabricatorForge<'_> { .friendly_describe_commit_from_oid(self.effects.get_glyphs(), commit_oid)? )?, )?; - return Ok(ExitCode(1)); + return Ok(Err(ExitCode(1))); } let rebase_plan = { @@ -351,7 +351,7 @@ impl Forge for PhabricatorForge<'_> { write!(self.effects.get_output_stream(), "Stdout:\n{stdout}")?; let stderr = std::fs::read_to_string(&test_output.stderr_path)?; write!(self.effects.get_output_stream(), "Stderr:\n{stderr}")?; - return Ok(ExitCode(1)); + return Ok(Err(ExitCode(1))); } TestStatus::Passed { cached: _, @@ -373,10 +373,10 @@ impl Forge for PhabricatorForge<'_> { let repo_pool = RepoResource::new_pool(self.repo)?; match builder.build(self.effects, &pool, &repo_pool)? { Ok(Some(rebase_plan)) => rebase_plan, - Ok(None) => return Ok(ExitCode(0)), + Ok(None) => return Ok(Ok(Default::default())), Err(err) => { err.describe(self.effects, self.repo, self.dag)?; - return Ok(ExitCode(1)); + return Ok(Err(ExitCode(1))); } } }; @@ -405,28 +405,69 @@ impl Forge for PhabricatorForge<'_> { self.effects.get_error_stream(), "BUG: Merge failed, but rewording shouldn't cause any merge failures." )?; - return Ok(ExitCode(1)); + return Ok(Err(ExitCode(1))); } ExecuteRebasePlanResult::Failed { exit_code } => { - return Ok(exit_code); + return Ok(Err(exit_code)); } }; - let latest_commit_oids = commit_oids - .iter() - .copied() - .map(|commit_oid| match rewritten_oids.get(&commit_oid) { + let mut create_statuses = HashMap::new(); + for commit_oid in commit_oids { + let final_commit_oid = match rewritten_oids.get(&commit_oid) { Some(MaybeZeroOid::NonZero(commit_oid)) => *commit_oid, Some(MaybeZeroOid::Zero) => { warn!(?commit_oid, "Commit was rewritten to the zero OID",); commit_oid } None => commit_oid, + }; + let local_branch_name = { + match self.get_revision_id(final_commit_oid)? { + Some(Id(id)) => format!("D{id}"), + None => { + writeln!( + self.effects.get_output_stream(), + "Failed to upload {}", + self.effects.get_glyphs().render( + self.repo.friendly_describe_commit_from_oid( + self.effects.get_glyphs(), + final_commit_oid + )? + )?, + )?; + return Ok(Err(ExitCode(1))); + } + } + }; + create_statuses.insert( + commit_oid, + CreateStatus { + final_commit_oid, + local_branch_name, + }, + ); + } + + let final_commit_oids: CommitSet = create_statuses + .values() + .map(|create_status| { + let CreateStatus { + final_commit_oid, + local_branch_name: _, + } = create_status; + *final_commit_oid }) - .collect_vec(); - self.update_dependencies(&latest_commit_oids.into_iter().collect())?; + .collect(); + self.dag.sync_from_oids( + self.effects, + self.repo, + CommitSet::empty(), + final_commit_oids.clone(), + )?; + self.update_dependencies(&final_commit_oids)?; - Ok(ExitCode(0)) + Ok(Ok(create_statuses)) } #[instrument] From 40ba0726506bfb29a33342bee31bc9c68207e440 Mon Sep 17 00:00:00 2001 From: Waleed Khan Date: Fri, 10 Mar 2023 00:38:26 -0500 Subject: [PATCH 8/8] feat(test): adjust some wording to say "command" instead of "test" Since we also use the testing infrastructure to submit to Phabricator, it's inaccurate to say "test" in many of these places. (jj also plans to use `jj run` instead of `jj test`.) --- git-branchless-lib/src/core/effects.rs | 4 +- git-branchless-test/src/lib.rs | 25 ++-- git-branchless-test/tests/test_test.rs | 156 ++++++++++++------------- 3 files changed, 94 insertions(+), 91 deletions(-) diff --git a/git-branchless-lib/src/core/effects.rs b/git-branchless-lib/src/core/effects.rs index 74839e335..3afbdd471 100644 --- a/git-branchless-lib/src/core/effects.rs +++ b/git-branchless-lib/src/core/effects.rs @@ -81,8 +81,8 @@ impl ToString for OperationType { OperationType::RunGitCommand(command) => { return format!("Running Git command: {}", &command) } - OperationType::RunTests(command) => return format!("Running tests: {command}"), - OperationType::RunTestOnCommit(commit) => return format!("Waiting to test {commit}"), + OperationType::RunTests(command) => return format!("Running command: {command}"), + OperationType::RunTestOnCommit(commit) => return format!("Waiting to run on {commit}"), OperationType::SortCommits => "Sorting commits", OperationType::SyncCommits => "Syncing commit stacks", OperationType::UpdateCommits => "Updating commits", diff --git a/git-branchless-test/src/lib.rs b/git-branchless-test/src/lib.rs index 293f1e3e3..7af35f3ea 100644 --- a/git-branchless-test/src/lib.rs +++ b/git-branchless-test/src/lib.rs @@ -806,7 +806,7 @@ fn clear_abort_trap( effects.get_glyphs().render( StyledStringBuilder::new() .append_styled( - "Error: Could not abort tests with `git rebase --abort`.", + "Error: Could not abort running commands with `git rebase --abort`.", BaseColor::Red.light() ) .build() @@ -956,22 +956,25 @@ impl TestStatus { .build(), TestStatus::SpawnTestFailed(err) => StyledStringBuilder::new() - .append_styled(format!("Failed to spawn test: {err}: "), self.get_style()) + .append_styled( + format!("Failed to spawn command: {err}: "), + self.get_style(), + ) .append(commit.friendly_describe(glyphs)?) .build(), TestStatus::TerminatedBySignal => StyledStringBuilder::new() - .append_styled("Test command terminated by signal: ", self.get_style()) + .append_styled("Command terminated by signal: ", self.get_style()) .append(commit.friendly_describe(glyphs)?) .build(), TestStatus::AlreadyInProgress => StyledStringBuilder::new() - .append_styled("Test already in progress? ", self.get_style()) + .append_styled("Command already in progress? ", self.get_style()) .append(commit.friendly_describe(glyphs)?) .build(), TestStatus::ReadCacheFailed(_) => StyledStringBuilder::new() - .append_styled("Could not read cached test result: ", self.get_style()) + .append_styled("Could not read cached command result: ", self.get_style()) .append(commit.friendly_describe(glyphs)?) .build(), @@ -985,7 +988,7 @@ impl TestStatus { TestStatus::Abort { exit_code } => StyledStringBuilder::new() .append_styled( - format!("Exit code indicated to abort testing (exit code {exit_code}): "), + format!("Exit code indicated to abort command (exit code {exit_code}): "), self.get_style(), ) .append(commit.friendly_describe(glyphs)?) @@ -1294,7 +1297,7 @@ pub fn run_tests<'a>( if let Some(strategy_value) = execution_strategy.to_possible_value() { writeln!( effects.get_output_stream(), - "Using test execution strategy: {}", + "Using command execution strategy: {}", effects.get_glyphs().render( StyledStringBuilder::new() .append_styled(strategy_value.get_name(), Effect::Bold) @@ -1360,7 +1363,7 @@ pub fn run_tests<'a>( let (_effects, progress) = effects.start_operation(operation_type.clone()); progress.notify_status( OperationIcon::InProgress, - format!("Waiting to test {commit_description}"), + format!("Waiting to run on {commit_description}"), ); results.insert( commit.get_oid(), @@ -1755,7 +1758,7 @@ fn print_summary( writeln!( effects.get_output_stream(), - "Tested {} with {}:", + "Ran command on {}: {}:", Pluralize { determiner: None, amount: test_results.test_outputs.len(), @@ -1875,7 +1878,7 @@ fn print_summary( let commit = repo.find_commit_or_fail(*commit_oid)?; writeln!( effects.get_output_stream(), - "Aborted testing with exit code {} at commit: {}", + "Aborted running commands with exit code {} at commit: {}", exit_code, effects .get_glyphs() @@ -2235,7 +2238,7 @@ fn run_test( progress.notify_status( OperationIcon::InProgress, format!( - "Testing {}", + "Running on {}", effects .get_glyphs() .render(commit.friendly_describe(effects.get_glyphs())?)? diff --git a/git-branchless-test/tests/test_test.rs b/git-branchless-test/tests/test_test.rs index affe1518e..8759260e7 100644 --- a/git-branchless-test/tests/test_test.rs +++ b/git-branchless-test/tests/test_test.rs @@ -36,11 +36,11 @@ fn test_test() -> eyre::Result<()> { branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy branchless: running command: rebase --abort ✓ Passed: fe65c1f create test2.txt ✓ Passed: 0206717 create test3.txt - Tested 2 commits with exit 0: + Ran command on 2 commits: exit 0: 2 passed, 0 failed, 0 skipped "###); } @@ -62,11 +62,11 @@ fn test_test() -> eyre::Result<()> { branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy branchless: running command: rebase --abort X Failed (exit code 1): fe65c1f create test2.txt X Failed (exit code 1): 0206717 create test3.txt - Tested 2 commits with exit 1: + Ran command on 2 commits: exit 1: 0 passed, 2 failed, 0 skipped "###); } @@ -98,7 +98,7 @@ fn test_test_abort() -> eyre::Result<()> { branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy "###); } @@ -159,12 +159,12 @@ fn test_test_cached_results() -> eyre::Result<()> { branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy branchless: running command: rebase --abort ✓ Passed: fe65c1f create test2.txt ✓ Passed: 0206717 create test3.txt ✓ Passed (cached): 1b0d484 Revert "create test3.txt" - Tested 3 commits with exit 0: + Ran command on 3 commits: exit 0: 3 passed, 0 failed, 0 skipped hint: there was 1 cached test result hint: to clear these cached results, run: git test clean "stack() | @" @@ -178,12 +178,12 @@ fn test_test_cached_results() -> eyre::Result<()> { branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy branchless: running command: rebase --abort ✓ Passed (cached): fe65c1f create test2.txt ✓ Passed (cached): 0206717 create test3.txt ✓ Passed (cached): 1b0d484 Revert "create test3.txt" - Tested 3 commits with exit 0: + Ran command on 3 commits: exit 0: 3 passed, 0 failed, 0 skipped hint: there were 3 cached test results hint: to clear these cached results, run: git test clean "stack() | @" @@ -213,7 +213,7 @@ fn test_test_verbosity() -> eyre::Result<()> { branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy branchless: running command: rebase --abort ✓ Passed: fe65c1f create test2.txt Stdout: /.git/branchless/test/48bb2464c55090a387ed70b3d229705a94856efb/bash__test.sh__10/stdout @@ -229,7 +229,7 @@ fn test_test_verbosity() -> eyre::Result<()> { This is line 10 Stderr: /.git/branchless/test/48bb2464c55090a387ed70b3d229705a94856efb/bash__test.sh__10/stderr - Tested 1 commit with bash test.sh 10: + Ran command on 1 commit: bash test.sh 10: 1 passed, 0 failed, 0 skipped "###); } @@ -240,7 +240,7 @@ fn test_test_verbosity() -> eyre::Result<()> { branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy branchless: running command: rebase --abort ✓ Passed (cached): fe65c1f create test2.txt Stdout: /.git/branchless/test/48bb2464c55090a387ed70b3d229705a94856efb/bash__test.sh__10/stdout @@ -256,7 +256,7 @@ fn test_test_verbosity() -> eyre::Result<()> { This is line 10 Stderr: /.git/branchless/test/48bb2464c55090a387ed70b3d229705a94856efb/bash__test.sh__10/stderr - Tested 1 commit with bash test.sh 10: + Ran command on 1 commit: bash test.sh 10: 1 passed, 0 failed, 0 skipped hint: there was 1 cached test result hint: to clear these cached results, run: git test clean "stack() | @" @@ -270,7 +270,7 @@ fn test_test_verbosity() -> eyre::Result<()> { branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy branchless: running command: rebase --abort ✓ Passed: fe65c1f create test2.txt Stdout: /.git/branchless/test/48bb2464c55090a387ed70b3d229705a94856efb/bash__test.sh__15/stdout @@ -287,7 +287,7 @@ fn test_test_verbosity() -> eyre::Result<()> { This is line 15 Stderr: /.git/branchless/test/48bb2464c55090a387ed70b3d229705a94856efb/bash__test.sh__15/stderr - Tested 1 commit with bash test.sh 15: + Ran command on 1 commit: bash test.sh 15: 1 passed, 0 failed, 0 skipped "###); } @@ -298,7 +298,7 @@ fn test_test_verbosity() -> eyre::Result<()> { branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy branchless: running command: rebase --abort ✓ Passed (cached): fe65c1f create test2.txt Stdout: /.git/branchless/test/48bb2464c55090a387ed70b3d229705a94856efb/bash__test.sh__15/stdout @@ -319,7 +319,7 @@ fn test_test_verbosity() -> eyre::Result<()> { This is line 15 Stderr: /.git/branchless/test/48bb2464c55090a387ed70b3d229705a94856efb/bash__test.sh__15/stderr - Tested 1 commit with bash test.sh 15: + Ran command on 1 commit: bash test.sh 15: 1 passed, 0 failed, 0 skipped hint: there was 1 cached test result hint: to clear these cached results, run: git test clean "stack() | @" @@ -345,10 +345,10 @@ fn test_test_show() -> eyre::Result<()> { branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy branchless: running command: rebase --abort ✓ Passed: 96d1c37 create test2.txt - Tested 1 commit with echo hi: + Ran command on 1 commit: echo hi: 1 passed, 0 failed, 0 skipped "###); } @@ -462,10 +462,10 @@ fn test_test_command_alias() -> eyre::Result<()> { branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy branchless: running command: rebase --abort ✓ Passed: f777ecc create initial.txt - Tested 1 commit with echo default: + Ran command on 1 commit: echo default: 1 passed, 0 failed, 0 skipped "###); } @@ -476,10 +476,10 @@ fn test_test_command_alias() -> eyre::Result<()> { branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy branchless: running command: rebase --abort ✓ Passed: f777ecc create initial.txt - Tested 1 commit with echo foo: + Ran command on 1 commit: echo foo: 1 passed, 0 failed, 0 skipped "###); } @@ -550,13 +550,13 @@ fn test_test_worktree_strategy() -> eyre::Result<()> { )?; insta::assert_snapshot!(stderr, @""); insta::assert_snapshot!(stdout, @r###" - Using test execution strategy: worktree + Using command execution strategy: worktree ✓ Passed: 62fc20d create test1.txt Stdout: /.git/branchless/test/8108c01b1930423879f106c1ebf725fcbfedccda/echo__hello/stdout hello Stderr: /.git/branchless/test/8108c01b1930423879f106c1ebf725fcbfedccda/echo__hello/stderr - Tested 1 commit with echo hello: + Ran command on 1 commit: echo hello: 1 passed, 0 failed, 0 skipped "###); } @@ -576,13 +576,13 @@ fn test_test_worktree_strategy() -> eyre::Result<()> { )?; insta::assert_snapshot!(stderr, @""); insta::assert_snapshot!(stdout, @r###" - Using test execution strategy: worktree + Using command execution strategy: worktree ✓ Passed (cached): 62fc20d create test1.txt Stdout: /.git/branchless/test/8108c01b1930423879f106c1ebf725fcbfedccda/echo__hello/stdout hello Stderr: /.git/branchless/test/8108c01b1930423879f106c1ebf725fcbfedccda/echo__hello/stderr - Tested 1 commit with echo hello: + Ran command on 1 commit: echo hello: 1 passed, 0 failed, 0 skipped hint: there was 1 cached test result hint: to clear these cached results, run: git test clean "@" @@ -636,13 +636,13 @@ echo hello let (stdout, stderr) = git.branchless("test", &["run", "-vv", "@"])?; insta::assert_snapshot!(stderr, @""); insta::assert_snapshot!(stdout, @r###" - Using test execution strategy: worktree + Using command execution strategy: worktree ✓ Passed: c82ebfa create test2.txt Stdout: /.git/branchless/test/a3ae41e24abf7537423d8c72d07df7af456de6dd/bash__test.sh/stdout hello Stderr: /.git/branchless/test/a3ae41e24abf7537423d8c72d07df7af456de6dd/bash__test.sh/stderr - Tested 1 commit with bash test.sh: + Ran command on 1 commit: bash test.sh: 1 passed, 0 failed, 0 skipped "###); } @@ -706,11 +706,11 @@ fn test_test_jobs_argument_handling() -> eyre::Result<()> { branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy branchless: running command: rebase --abort ✓ Passed: 62fc20d create test1.txt ✓ Passed: 96d1c37 create test2.txt - Tested 2 commits with exit 0: + Ran command on 2 commits: exit 0: 2 passed, 0 failed, 0 skipped "###); } @@ -734,10 +734,10 @@ fn test_test_jobs_argument_handling() -> eyre::Result<()> { let (stdout, stderr) = git.branchless("test", &["run", "--jobs", "2"])?; insta::assert_snapshot!(stderr, @""); insta::assert_snapshot!(stdout, @r###" - Using test execution strategy: worktree + Using command execution strategy: worktree ✓ Passed (cached): 62fc20d create test1.txt ✓ Passed (cached): 96d1c37 create test2.txt - Tested 2 commits with exit 0: + Ran command on 2 commits: exit 0: 2 passed, 0 failed, 0 skipped hint: there were 2 cached test results hint: to clear these cached results, run: git test clean "stack() | @" @@ -771,7 +771,7 @@ fn test_test_jobs_argument_handling() -> eyre::Result<()> { branchless: running command: rebase --abort ✓ Passed (interactive): 62fc20d create test1.txt ✓ Passed (interactive): 96d1c37 create test2.txt - Tested 2 commits with true: + Ran command on 2 commits: true: 2 passed, 0 failed, 0 skipped "###); } @@ -803,11 +803,11 @@ fn test_test_jobs_argument_handling() -> eyre::Result<()> { branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy branchless: running command: rebase --abort ✓ Passed (cached): 62fc20d create test1.txt ✓ Passed (cached): 96d1c37 create test2.txt - Tested 2 commits with exit 0: + Ran command on 2 commits: exit 0: 2 passed, 0 failed, 0 skipped hint: there were 2 cached test results hint: to clear these cached results, run: git test clean "stack() | @" @@ -855,12 +855,12 @@ done branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy branchless: running command: rebase --abort ✓ Passed (fixed): 62fc20d create test1.txt ✓ Passed (fixed): 96d1c37 create test2.txt ✓ Passed (fixed): 70deb1e create test3.txt - Tested 3 commits with bash test.sh: + Ran command on 3 commits: bash test.sh: 3 passed, 0 failed, 0 skipped Attempting rebase in-memory... [1/3] Committed as: 300cb54 create test1.txt @@ -967,12 +967,12 @@ done branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy branchless: running command: rebase --abort ✓ Passed: 300cb54 create test1.txt ✓ Passed: 2ee3aea create test2.txt ✓ Passed: 6f48e0a create test3.txt - Tested 3 commits with bash test.sh: + Ran command on 3 commits: bash test.sh: 3 passed, 0 failed, 0 skipped No commits to fix. "###); @@ -1026,12 +1026,12 @@ done branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy branchless: running command: rebase --abort ✓ Passed (fixed): 62fc20d create test1.txt X Failed (exit code 1): 96d1c37 create test2.txt X Failed (exit code 1): 70deb1e create test3.txt - Tested 3 commits with bash test.sh: + Ran command on 3 commits: bash test.sh: 1 passed, 2 failed, 0 skipped "###); } @@ -1120,11 +1120,11 @@ done branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy branchless: running command: rebase --abort ✓ Passed (fixed): 62fc20d create test1.txt ✓ Passed (fixed): 75e728f descendant commit - Tested 2 commits with bash test.sh: + Ran command on 2 commits: bash test.sh: 2 passed, 0 failed, 0 skipped Attempting rebase in-memory... [1/2] Committed as: 300cb54 create test1.txt @@ -1247,13 +1247,13 @@ fn test_test_search_binary() -> eyre::Result<()> { branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy Using test search strategy: binary branchless: running command: rebase --abort ✓ Passed: 70deb1e create test3.txt X Failed (exit code 1): 355e173 create test4.txt X Failed (exit code 1): f81d55c create test5.txt - Tested 3 commits with ! git grep -q 'test4': + Ran command on 3 commits: ! git grep -q 'test4': 1 passed, 2 failed, 0 skipped Last passing commit: - 70deb1e create test3.txt @@ -1277,9 +1277,9 @@ fn test_test_run_none() -> eyre::Result<()> { branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy branchless: running command: rebase --abort - Tested 0 commits with true: + Ran command on 0 commits: true: 0 passed, 0 failed, 0 skipped "###); } @@ -1308,7 +1308,7 @@ fn test_test_search_skip_indeterminate() -> eyre::Result<()> { branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy Using test search strategy: binary branchless: running command: rebase --abort ! Exit code indicated to skip this commit (exit code 125): 62fc20d create test1.txt @@ -1318,7 +1318,7 @@ fn test_test_search_skip_indeterminate() -> eyre::Result<()> { ! Exit code indicated to skip this commit (exit code 125): f81d55c create test5.txt ! Exit code indicated to skip this commit (exit code 125): 2831fb5 create test6.txt ! Exit code indicated to skip this commit (exit code 125): c8933b3 create test7.txt - Tested 7 commits with exit 125: + Ran command on 7 commits: exit 125: 0 passed, 0 failed, 7 skipped There were no passing commits in the provided set. There were no failing commits in the provided set. @@ -1333,7 +1333,7 @@ fn test_test_search_skip_indeterminate() -> eyre::Result<()> { branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy Using test search strategy: binary branchless: running command: rebase --abort ! Exit code indicated to skip this commit (exit code 125): 62fc20d create test1.txt @@ -1343,7 +1343,7 @@ fn test_test_search_skip_indeterminate() -> eyre::Result<()> { ! Exit code indicated to skip this commit (exit code 125): f81d55c create test5.txt ! Exit code indicated to skip this commit (exit code 125): 2831fb5 create test6.txt ! Exit code indicated to skip this commit (exit code 125): c8933b3 create test7.txt - Tested 7 commits with exit 125: + Ran command on 7 commits: exit 125: 0 passed, 0 failed, 7 skipped There were no passing commits in the provided set. There were no failing commits in the provided set. @@ -1372,7 +1372,7 @@ fi branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy Using test search strategy: linear branchless: running command: rebase --abort ✓ Passed: 62fc20d create test1.txt @@ -1382,7 +1382,7 @@ fi ! Exit code indicated to skip this commit (exit code 125): f81d55c create test5.txt ! Exit code indicated to skip this commit (exit code 125): 2831fb5 create test6.txt ! Exit code indicated to skip this commit (exit code 125): c8933b3 create test7.txt - Tested 7 commits with bash test.sh: + Ran command on 7 commits: bash test.sh: 3 passed, 0 failed, 4 skipped Last passing commit: - 70deb1e create test3.txt @@ -1399,7 +1399,7 @@ fi branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy Using test search strategy: binary branchless: running command: rebase --abort ✓ Passed (cached): 70deb1e create test3.txt @@ -1407,7 +1407,7 @@ fi ! Exit code indicated to skip this commit (exit code 125): f81d55c create test5.txt ! Exit code indicated to skip this commit (exit code 125): 2831fb5 create test6.txt ! Exit code indicated to skip this commit (exit code 125): c8933b3 create test7.txt - Tested 5 commits with bash test.sh: + Ran command on 5 commits: bash test.sh: 1 passed, 0 failed, 4 skipped Last passing commit: - 70deb1e create test3.txt @@ -1460,7 +1460,7 @@ fn test_test_interactive() -> eyre::Result<()> { branchless: running command: rebase --abort ✓ Passed (interactive): 62fc20d create test1.txt ✓ Passed (interactive): 96d1c37 create test2.txt - Tested 2 commits with true: + Ran command on 2 commits: true: 2 passed, 0 failed, 0 skipped "###); } @@ -1492,7 +1492,7 @@ fn test_test_interactive() -> eyre::Result<()> { branchless: running command: rebase --abort X Failed (exit code 1, interactive): 62fc20d create test1.txt X Failed (exit code 1, interactive): 96d1c37 create test2.txt - Tested 2 commits with false: + Ran command on 2 commits: false: 0 passed, 2 failed, 0 skipped "###); } @@ -1529,7 +1529,7 @@ fn test_test_interactive() -> eyre::Result<()> { branchless: running command: rebase --abort ✓ Passed (interactive): 62fc20d create test1.txt ✓ Passed (interactive): 96d1c37 create test2.txt - Tested 2 commits with bash: + Ran command on 2 commits: bash: 2 passed, 0 failed, 0 skipped "###); } @@ -1551,7 +1551,7 @@ fn test_test_interactive() -> eyre::Result<()> { branchless: running command: rebase --abort ✓ Passed (cached, interactive): 62fc20d create test1.txt ✓ Passed (cached, interactive): 96d1c37 create test2.txt - Tested 2 commits with bash: + Ran command on 2 commits: bash: 2 passed, 0 failed, 0 skipped hint: there were 2 cached test results hint: to clear these cached results, run: git test clean "stack() | @" @@ -1602,17 +1602,17 @@ fi branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy Using test search strategy: linear branchless: running command: rebase --abort ✓ Passed: 62fc20d create test1.txt - X Exit code indicated to abort testing (exit code 127): 96d1c37 create test2.txt - Tested 2 commits with bash test.sh: + X Exit code indicated to abort command (exit code 127): 96d1c37 create test2.txt + Ran command on 2 commits: bash test.sh: 1 passed, 1 failed, 0 skipped Last passing commit: - 62fc20d create test1.txt There were no failing commits in the provided set. - Aborted testing with exit code 127 at commit: 96d1c37 create test2.txt + Aborted running commands with exit code 127 at commit: 96d1c37 create test2.txt "###); } @@ -1630,12 +1630,12 @@ fi branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy Using test search strategy: linear branchless: running command: rebase --abort ✓ Passed (cached): 62fc20d create test1.txt - X Exit code indicated to abort testing (exit code 127): 96d1c37 create test2.txt - Tested 2 commits with bash test.sh: + X Exit code indicated to abort command (exit code 127): 96d1c37 create test2.txt + Ran command on 2 commits: bash test.sh: 1 passed, 1 failed, 0 skipped Last passing commit: - 62fc20d create test1.txt @@ -1643,7 +1643,7 @@ fi hint: there was 1 cached test result hint: to clear these cached results, run: git test clean "stack() | @" hint: disable this hint by running: git config --global branchless.hint.cleanCachedTestResults false - Aborted testing with exit code 127 at commit: 96d1c37 create test2.txt + Aborted running commands with exit code 127 at commit: 96d1c37 create test2.txt "###); } @@ -1670,7 +1670,7 @@ echo "Command is: $BRANCHLESS_TEST_COMMAND" branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy branchless: running command: rebase --abort ✓ Passed: f777ecc create initial.txt Stdout: /.git/branchless/test/d32758e20028dd1cffc2b359bc3766f80a258ee5/bash__test.sh/stdout @@ -1678,7 +1678,7 @@ echo "Command is: $BRANCHLESS_TEST_COMMAND" Command is: bash test.sh Stderr: /.git/branchless/test/d32758e20028dd1cffc2b359bc3766f80a258ee5/bash__test.sh/stderr - Tested 1 commit with bash test.sh: + Ran command on 1 commit: bash test.sh: 1 passed, 0 failed, 0 skipped "###); } @@ -1759,13 +1759,13 @@ esac branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy branchless: running command: rebase --abort ✓ Passed (fixable): 62fc20d create test1.txt X Failed (exit code 1): 96d1c37 create test2.txt ! Exit code indicated to skip this commit (exit code 125): 70deb1e create test3.txt ✓ Passed: 355e173 create test4.txt - Tested 4 commits with bash test.sh: + Ran command on 4 commits: bash test.sh: 2 passed, 1 failed, 1 skipped "###); } @@ -1840,11 +1840,11 @@ fn test_test_no_cache() -> eyre::Result<()> { branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy branchless: running command: rebase --abort ✓ Passed: 62fc20d create test1.txt ✓ Passed: 96d1c37 create test2.txt - Tested 2 commits with bash test.sh: + Ran command on 2 commits: bash test.sh: 2 passed, 0 failed, 0 skipped "###); } @@ -1868,11 +1868,11 @@ fn test_test_no_cache() -> eyre::Result<()> { branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy branchless: running command: rebase --abort X Failed (exit code 1): 62fc20d create test1.txt X Failed (exit code 1): 96d1c37 create test2.txt - Tested 2 commits with bash test.sh: + Ran command on 2 commits: bash test.sh: 0 passed, 2 failed, 0 skipped "###); } @@ -1883,11 +1883,11 @@ fn test_test_no_cache() -> eyre::Result<()> { branchless: running command: diff --quiet Calling Git for on-disk rebase... branchless: running command: rebase --continue - Using test execution strategy: working-copy + Using command execution strategy: working-copy branchless: running command: rebase --abort ✓ Passed (cached): 62fc20d create test1.txt ✓ Passed (cached): 96d1c37 create test2.txt - Tested 2 commits with bash test.sh: + Ran command on 2 commits: bash test.sh: 2 passed, 0 failed, 0 skipped hint: there were 2 cached test results hint: to clear these cached results, run: git test clean "stack() | @"