diff --git a/Cargo.lock b/Cargo.lock index 02d3e820d..caf3cff1f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1657,6 +1657,7 @@ dependencies = [ "git-branchless-opts", "git-branchless-revset", "git-branchless-test", + "indexmap 2.2.1", "insta", "itertools 0.12.1", "lazy_static", @@ -1664,6 +1665,7 @@ dependencies = [ "regex", "serde", "serde_json", + "tempfile", "thiserror", "tracing", ] diff --git a/git-branchless-lib/src/git/reference.rs b/git-branchless-lib/src/git/reference.rs index 50cb81e10..9f6688019 100644 --- a/git-branchless-lib/src/git/reference.rs +++ b/git-branchless-lib/src/git/reference.rs @@ -3,7 +3,7 @@ use std::ffi::OsStr; use std::string::FromUtf8Error; use thiserror::Error; -use tracing::instrument; +use tracing::{instrument, warn}; use crate::git::config::ConfigRead; use crate::git::oid::make_non_zero_oid; @@ -306,6 +306,37 @@ impl<'repo> Branch<'repo> { Ok(target_oid) } + /// If this branch tracks a remote ("upstream") branch, return the name of + /// that branch without the leading remote name. For example, if the + /// upstream branch is `origin/main`, this will return `main`. (Usually, + /// this is the same as the name of the local branch, but not always.) + pub fn get_upstream_branch_name_without_push_remote_name( + &self, + ) -> eyre::Result> { + let push_remote_name = match self.get_push_remote_name()? { + Some(stack_remote_name) => stack_remote_name, + None => return Ok(None), + }; + let upstream_branch = match self.get_upstream_branch()? { + Some(upstream_branch) => upstream_branch, + None => return Ok(None), + }; + let upstream_branch_name = upstream_branch.get_name()?; + let upstream_branch_name_without_remote = + match upstream_branch_name.strip_prefix(&format!("{push_remote_name}/")) { + Some(upstream_branch_name_without_remote) => upstream_branch_name_without_remote, + None => { + warn!( + ?push_remote_name, + ?upstream_branch, + "Upstream branch name did not start with push remote name" + ); + upstream_branch_name + } + }; + Ok(Some(upstream_branch_name_without_remote.to_owned())) + } + /// Get the associated remote to push to for this branch. If there is no /// associated remote, returns `None`. Note that this never reads the value /// of `push.remoteDefault`. diff --git a/git-branchless-lib/src/git/repo.rs b/git-branchless-lib/src/git/repo.rs index 7186317cf..6ac3c282a 100644 --- a/git-branchless-lib/src/git/repo.rs +++ b/git-branchless-lib/src/git/repo.rs @@ -1035,6 +1035,31 @@ impl Repo { Ok((snapshot, statuses)) } + /// Create a new branch or update an existing one. The provided name should + /// be a branch name and not a reference name, i.e. it should not start with + /// `refs/heads/`. + #[instrument] + pub fn create_branch(&self, branch_name: &str, commit: &Commit, force: bool) -> Result { + if branch_name.starts_with("refs/heads/") { + warn!( + ?branch_name, + "Branch name starts with refs/heads/; this is probably not what you intended." + ); + } + + let branch = self + .inner + .branch(branch_name, &commit.inner, force) + .map_err(|err| Error::CreateBranch { + source: err, + name: branch_name.to_owned(), + })?; + Ok(Branch { + repo: self, + inner: branch, + }) + } + /// Create a new reference or update an existing one. #[instrument] pub fn create_reference( diff --git a/git-branchless-lib/src/git/test.rs b/git-branchless-lib/src/git/test.rs index cb42c75b6..c1014a1b3 100644 --- a/git-branchless-lib/src/git/test.rs +++ b/git-branchless-lib/src/git/test.rs @@ -46,7 +46,7 @@ pub fn make_test_command_slug(command: String) -> String { /// A version of `NonZeroOid` that can be serialized and deserialized. This /// exists in case we want to move this type (back) into a separate module which /// has a `serde` dependency in the interest of improving build times. -#[derive(Debug)] +#[derive(Clone, Debug, Eq, PartialEq)] pub struct SerializedNonZeroOid(pub NonZeroOid); impl Serialize for SerializedNonZeroOid { diff --git a/git-branchless-lib/src/testing.rs b/git-branchless-lib/src/testing.rs index 3783b5638..5ef2a2333 100644 --- a/git-branchless-lib/src/testing.rs +++ b/git-branchless-lib/src/testing.rs @@ -540,6 +540,7 @@ then you can only run tests in the main `git-branchless` and \ /// Commit a file with given contents and message. The `time` argument is /// used to set the commit timestamp, which is factored into the commit /// hash. The filename is always appended to the message prefix. + #[track_caller] #[instrument] pub fn commit_file_with_contents_and_message( &self, @@ -570,6 +571,7 @@ then you can only run tests in the main `git-branchless` and \ /// Commit a file with given contents and a default message. The `time` /// argument is used to set the commit timestamp, which is factored into the /// commit hash. + #[track_caller] #[instrument] pub fn commit_file_with_contents( &self, @@ -582,6 +584,8 @@ then you can only run tests in the main `git-branchless` and \ /// Commit a file with default contents. The `time` argument is used to set /// the commit timestamp, which is factored into the commit hash. + #[track_caller] + #[instrument] pub fn commit_file(&self, name: &str, time: isize) -> eyre::Result { self.commit_file_with_contents(name, time, &format!("{name} contents\n")) } diff --git a/git-branchless-submit/Cargo.toml b/git-branchless-submit/Cargo.toml index 464523d94..304290820 100644 --- a/git-branchless-submit/Cargo.toml +++ b/git-branchless-submit/Cargo.toml @@ -16,6 +16,7 @@ git-branchless-invoke = { workspace = true } git-branchless-opts = { workspace = true } git-branchless-revset = { workspace = true } git-branchless-test = { workspace = true } +indexmap = { workspace = true } itertools = { workspace = true } lazy_static = { workspace = true } lib = { workspace = true } @@ -23,6 +24,7 @@ rayon = { workspace = true } regex = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } +tempfile = { workspace = true } thiserror = { workspace = true } tracing = { workspace = true } diff --git a/git-branchless-submit/src/branch_forge.rs b/git-branchless-submit/src/branch_forge.rs index 1c79994fa..5484de52f 100644 --- a/git-branchless-submit/src/branch_forge.rs +++ b/git-branchless-submit/src/branch_forge.rs @@ -13,7 +13,7 @@ use lib::git::{ }; use lib::try_exit_code; use lib::util::{ExitCode, EyreExitOr}; -use tracing::warn; +use tracing::{instrument, warn}; use crate::{CommitStatus, CreateStatus, Forge, SubmitOptions, SubmitStatus}; @@ -28,6 +28,7 @@ pub struct BranchForge<'a> { } impl Forge for BranchForge<'_> { + #[instrument] fn query_status( &mut self, commit_set: CommitSet, @@ -185,6 +186,7 @@ impl Forge for BranchForge<'_> { Ok(Ok(commit_statuses)) } + #[instrument] fn create( &mut self, commits: HashMap, @@ -204,6 +206,10 @@ impl Forge for BranchForge<'_> { .sorted() .collect_vec(); + // FIXME: in principle, it's possible for a branch to have been assigned + // a remote without having been pushed and having created a + // corresponding remote branch. In those cases, we should use the + // branch's associated remote. let push_remote: String = match self.repo.get_default_push_remote()? { Some(push_remote) => push_remote, None => { @@ -258,6 +264,7 @@ These remotes are available: {}", } } + #[instrument] fn update( &mut self, commits: HashMap, @@ -275,7 +282,7 @@ These remotes are available: {}", commit_status => { warn!( ?commit_status, -"Commit was requested to be updated, but it did not have the requisite information." + "Commit was requested to be updated, but it did not have the requisite information (remote name, local branch name)." ); None } diff --git a/git-branchless-submit/src/github.rs b/git-branchless-submit/src/github.rs index e52b2edcb..2e8abbbb7 100644 --- a/git-branchless-submit/src/github.rs +++ b/git-branchless-submit/src/github.rs @@ -1,18 +1,108 @@ //! GitHub backend for submitting patch stacks. use std::collections::HashMap; +use std::env; +use std::fmt::{Debug, Write}; +use std::hash::Hash; +use cursive_core::theme::Effect; +use cursive_core::utils::markup::StyledString; +use indexmap::IndexMap; +use itertools::Itertools; +use lib::core::config::get_main_branch_name; use lib::core::dag::CommitSet; use lib::core::dag::Dag; use lib::core::effects::Effects; +use lib::core::effects::OperationType; use lib::core::eventlog::EventLogDb; +use lib::core::repo_ext::RepoExt; +use lib::core::repo_ext::RepoReferencesSnapshot; +use lib::git::CategorizedReferenceName; +use lib::git::GitErrorCode; use lib::git::GitRunInfo; +use lib::git::RepoError; +use lib::git::{BranchType, ConfigRead}; use lib::git::{NonZeroOid, Repo}; - +use lib::try_exit_code; +use lib::util::ExitCode; use lib::util::EyreExitOr; +use tracing::debug; +use tracing::instrument; +use tracing::warn; + +use crate::branch_forge::BranchForge; +use crate::SubmitStatus; use crate::{CommitStatus, CreateStatus, Forge, SubmitOptions}; +/// Testing environment variable. When this is set, the executable will use the +/// mock Github implementation. This should be set to the path of an existing +/// repository that represents the remote/Github. +pub const MOCK_REMOTE_REPO_PATH_ENV_KEY: &str = "BRANCHLESS_SUBMIT_GITHUB_MOCK_REMOTE_REPO_PATH"; + +fn commit_summary_slug(summary: &str) -> String { + let summary_slug: String = summary + .chars() + .map(|c| if c.is_alphanumeric() { c } else { '-' }) + .flat_map(|c| c.to_lowercase()) + .dedup_by(|lhs, rhs| { + // Deduplicate adjacent hyphens. + *lhs == '-' && *rhs == '-' + }) + .collect(); + let summary_slug = summary_slug.trim_matches('-'); + if summary_slug.is_empty() { + "to-review".to_string() + } else { + summary_slug.to_owned() + } +} + +fn singleton( + map: &HashMap, + key: K, + f: impl Fn(V) -> V, +) -> HashMap { + let mut result = HashMap::new(); + match map.get(&key) { + Some(value) => { + result.insert(key, f(value.clone())); + } + None => { + warn!(?key, "No match for key in map"); + } + } + result +} + +/// Get the name of the remote repository to push to in the course of creating +/// pull requests. +/// +/// NOTE: The `gh` command-line utility might infer the push remote if only one +/// remote is available. This function only returns a remote if it is explicitly +/// set by the user using `gh repo set-default`.` +pub fn github_push_remote(repo: &Repo) -> eyre::Result> { + let config = repo.get_readonly_config()?; + for remote_name in repo.get_all_remote_names()? { + // This is set by `gh repo set-default`. Note that `gh` can + // sometimes infer which repo to push to without invoking + // `gh repo set-default` explicitly. We could probably check + // the remote URL to see if it's associated with + // `github.com`, but we would still need `gh` to be + // installed on the system in that case. The presence of + // this value means that `gh` was actively used. + // + // Possible values seem to be `base` and `other`. See: + // https://github.com/search?q=repo%3Acli%2Fcli%20gh-resolved&type=code + let gh_resolved: Option = + config.get(&format!("remote.{remote_name}.gh-resolved"))?; + if gh_resolved.as_deref() == Some("base") { + return Ok(Some(remote_name)); + } + } + Ok(None) +} + /// The [GitHub](https://en.wikipedia.org/wiki/GitHub) code hosting platform. /// This forge integrates specifically with the `gh` command-line utility. #[allow(missing_docs)] @@ -23,29 +113,1132 @@ pub struct GithubForge<'a> { pub repo: &'a Repo, pub event_log_db: &'a EventLogDb<'a>, pub dag: &'a Dag, + pub client: Box, } impl Forge for GithubForge<'_> { + #[instrument] fn query_status( &mut self, - _commit_set: CommitSet, + commit_set: CommitSet, ) -> EyreExitOr> { - unimplemented!("stub") + let effects = self.effects; + let pull_request_infos = + try_exit_code!(self.client.query_repo_pull_request_infos(effects)?); + let references_snapshot = self.repo.get_references_snapshot()?; + + let mut result = HashMap::new(); + for branch in self.repo.get_all_local_branches()? { + let local_branch_oid = match branch.get_oid()? { + Some(branch_oid) => branch_oid, + None => continue, + }; + if !self.dag.set_contains(&commit_set, local_branch_oid)? { + continue; + } + + let local_branch_name = branch.get_name()?; + let remote_name = branch.get_push_remote_name()?; + let remote_branch_name = branch.get_upstream_branch_name_without_push_remote_name()?; + + let submit_status = match remote_branch_name + .as_ref() + .and_then(|remote_branch_name| pull_request_infos.get(remote_branch_name)) + { + None => SubmitStatus::Unsubmitted, + Some(pull_request_info) => { + let updated_pull_request_info = try_exit_code!(self + .make_updated_pull_request_info( + effects, + &references_snapshot, + &pull_request_infos, + local_branch_oid + )?); + debug!( + ?pull_request_info, + ?updated_pull_request_info, + "Comparing pull request info" + ); + if updated_pull_request_info + .fields_to_update(pull_request_info) + .is_empty() + { + SubmitStatus::UpToDate + } else { + SubmitStatus::NeedsUpdate + } + } + }; + result.insert( + local_branch_oid, + CommitStatus { + submit_status, + remote_name, + local_commit_name: Some(local_branch_name.to_owned()), + remote_commit_name: remote_branch_name, + }, + ); + } + + for commit_oid in self.dag.commit_set_to_vec(&commit_set)? { + result.entry(commit_oid).or_insert(CommitStatus { + submit_status: SubmitStatus::Unsubmitted, + remote_name: None, + local_commit_name: None, + remote_commit_name: None, + }); + } + + Ok(Ok(result)) } + #[instrument] fn create( &mut self, - _commits: HashMap, - _options: &SubmitOptions, + commits: HashMap, + options: &SubmitOptions, ) -> EyreExitOr> { - unimplemented!("stub") + let effects = self.effects; + let commit_oids = self.dag.sort(&commits.keys().copied().collect())?; + + let references_snapshot = self.repo.get_references_snapshot()?; + let mut branch_forge = BranchForge { + effects, + git_run_info: self.git_run_info, + dag: self.dag, + repo: self.repo, + event_log_db: self.event_log_db, + references_snapshot: &references_snapshot, + }; + let push_remote_name = match github_push_remote(self.repo)? { + Some(remote_name) => remote_name, + None => match self.repo.get_default_push_remote()? { + Some(remote_name) => remote_name, + None => { + writeln!( + effects.get_output_stream(), + "No default push repository configured. To configure, run: {}", + effects.get_glyphs().render(StyledString::styled( + "gh repo set-default ", + Effect::Bold, + ))? + )?; + return Ok(Err(ExitCode(1))); + } + }, + }; + let github_username = try_exit_code!(self.client.query_github_username(effects)?); + + // Generate branches for all the commits to create. + let commits_to_create = commit_oids + .into_iter() + .map(|commit_oid| (commit_oid, commits.get(&commit_oid).unwrap())) + .filter_map( + |(commit_oid, commit_status)| match commit_status.submit_status { + SubmitStatus::Local + | SubmitStatus::Unknown + | SubmitStatus::NeedsUpdate + | SubmitStatus::UpToDate => None, + SubmitStatus::Unsubmitted => Some((commit_oid, commit_status)), + }, + ) + .collect_vec(); + let mut created_branches = HashMap::new(); + for (commit_oid, commit_status) in commits_to_create.iter().copied() { + let commit = self.repo.find_commit_or_fail(commit_oid)?; + + let local_branch_name = match &commit_status.local_commit_name { + Some(local_branch_name) => local_branch_name.clone(), + None => { + let summary = commit.get_summary()?; + let summary = String::from_utf8_lossy(&summary); + let summary_slug = commit_summary_slug(&summary); + let new_branch_name_base = format!("{github_username}/{summary_slug}"); + let mut new_branch_name = new_branch_name_base.clone(); + for i in 2.. { + if i > 6 { + writeln!( + effects.get_output_stream(), + "Could not generate fresh branch name for commit: {}", + effects + .get_glyphs() + .render(commit.friendly_describe(effects.get_glyphs())?)?, + )?; + return Ok(Err(ExitCode(1))); + } + match self.repo.find_branch(&new_branch_name, BranchType::Local)? { + Some(_) => { + new_branch_name = format!("{new_branch_name_base}-{i}"); + } + None => break, + } + } + match self.repo.create_branch(&new_branch_name, &commit, false) { + Ok(_branch) => {} + Err(RepoError::CreateBranch { source, name: _ }) + if source.code() == GitErrorCode::Exists => {} + Err(err) => return Err(err.into()), + }; + new_branch_name + } + }; + + let created_branch = try_exit_code!(branch_forge.create( + singleton(&commits, commit_oid, |commit_status| CommitStatus { + local_commit_name: Some(local_branch_name.clone()), + ..commit_status.clone() + }), + options + )?); + created_branches.extend(created_branch.into_iter()); + } + + let commit_statuses: HashMap = commits_to_create + .iter() + .copied() + .map(|(commit_oid, commit_status)| { + let commit_status = match created_branches.get(&commit_oid) { + Some(CreateStatus { + final_commit_oid: _, + local_commit_name, + }) => CommitStatus { + // To be updated below: + submit_status: SubmitStatus::NeedsUpdate, + remote_name: Some(push_remote_name.clone()), + local_commit_name: Some(local_commit_name.clone()), + // Expecting this to be the same as the local branch name (for now): + remote_commit_name: Some(local_commit_name.clone()), + }, + None => commit_status.clone(), + }; + (commit_oid, commit_status) + }) + .collect(); + + // Create the pull requests only after creating all the branches because + // we rely on the presence of a branch on each commit in the stack to + // know that it should be included/linked in the pull request body. + // FIXME: is this actually necessary? + for (commit_oid, _) in commits_to_create { + let local_branch_name = match commit_statuses.get(&commit_oid) { + Some(CommitStatus { + local_commit_name: Some(local_commit_name), + .. + }) => local_commit_name, + Some(CommitStatus { + local_commit_name: None, + .. + }) + | None => { + writeln!( + effects.get_output_stream(), + "Could not find local branch name for commit: {}", + effects.get_glyphs().render( + self.repo + .find_commit_or_fail(commit_oid)? + .friendly_describe(effects.get_glyphs())? + )? + )?; + return Ok(Err(ExitCode(1))); + } + }; + + let commit = self.repo.find_commit_or_fail(commit_oid)?; + let title = String::from_utf8_lossy(&commit.get_summary()?).into_owned(); + let body = String::from_utf8_lossy(&commit.get_message_pretty()).into_owned(); + try_exit_code!(self.client.create_pull_request( + effects, + client::CreatePullRequestArgs { + head_ref_oid: commit_oid, + head_ref_name: local_branch_name.clone(), + title, + body, + }, + options + )?); + } + + try_exit_code!(self.update(commit_statuses, options)?); + + Ok(Ok(created_branches)) } + #[instrument] fn update( &mut self, - _commits: HashMap, - _options: &SubmitOptions, + commit_statuses: HashMap, + options: &SubmitOptions, ) -> EyreExitOr<()> { - unimplemented!("stub") + let effects = self.effects; + let SubmitOptions { + create: _, + draft: _, + execution_strategy: _, + num_jobs: _, + message: _, + } = options; + + let pull_request_infos = + try_exit_code!(self.client.query_repo_pull_request_infos(effects)?); + let references_snapshot = self.repo.get_references_snapshot()?; + let mut branch_forge = BranchForge { + effects, + git_run_info: self.git_run_info, + dag: self.dag, + repo: self.repo, + event_log_db: self.event_log_db, + references_snapshot: &references_snapshot, + }; + + let commit_set: CommitSet = commit_statuses.keys().copied().collect(); + let commit_oids = self.dag.sort(&commit_set)?; + { + let (effects, progress) = effects.start_operation(OperationType::UpdateCommits); + progress.notify_progress(0, commit_oids.len()); + for commit_oid in commit_oids { + let commit_status = match commit_statuses.get(&commit_oid) { + Some(commit_status) => commit_status, + None => { + warn!( + ?commit_oid, + ?commit_statuses, + "Commit not found in commit statuses" + ); + continue; + } + }; + let remote_branch_name = match &commit_status.remote_commit_name { + Some(remote_branch_name) => remote_branch_name, + None => { + warn!( + ?commit_oid, + ?commit_statuses, + "Commit does not have remote branch name" + ); + continue; + } + }; + let pull_request_info = match pull_request_infos.get(remote_branch_name) { + Some(pull_request_info) => pull_request_info, + None => { + warn!( + ?commit_oid, + ?commit_statuses, + "Commit does not have pull request" + ); + continue; + } + }; + + let updated_pull_request_info = try_exit_code!(self + .make_updated_pull_request_info( + &effects, + &references_snapshot, + &pull_request_infos, + commit_oid + )?); + let updated_fields = { + let fields = updated_pull_request_info.fields_to_update(pull_request_info); + if fields.is_empty() { + "none (this should not happen)".to_owned() + } else { + fields.join(", ") + } + }; + let client::UpdatePullRequestArgs { + head_ref_oid: _, // Updated by `branch_forge.update`. + base_ref_name, + title, + body, + } = updated_pull_request_info; + writeln!( + effects.get_output_stream(), + "Updating pull request ({updated_fields}) for commit {}", + effects.get_glyphs().render( + self.repo + .find_commit_or_fail(commit_oid)? + .friendly_describe(effects.get_glyphs())? + )? + )?; + + // Make sure to update the branch and metadata at the same time, + // rather than all the branches at first. Otherwise, when + // reordering commits, GitHub may close one of the pull requests + // as it seems to have all the commits of its parent (or + // something like that). + + // Push branch: + try_exit_code!( + branch_forge.update(singleton(&commit_statuses, commit_oid, |x| x), options)? + ); + + // Update metdata: + try_exit_code!(self.client.update_pull_request( + &effects, + pull_request_info.number, + client::UpdatePullRequestArgs { + head_ref_oid: commit_oid, + base_ref_name, + title, + body, + }, + options + )?); + progress.notify_progress_inc(1); + } + } + + Ok(Ok(())) + } +} + +impl GithubForge<'_> { + /// Construct a real or mock GitHub client according to the environment. + pub fn client(git_run_info: GitRunInfo) -> Box { + match env::var(MOCK_REMOTE_REPO_PATH_ENV_KEY) { + Ok(path) => Box::new(client::MockGithubClient { + remote_repo_path: path.into(), + }), + Err(_) => { + let GitRunInfo { + path_to_git: _, + working_directory, + env, + } = git_run_info; + let gh_run_info = GitRunInfo { + path_to_git: "gh".into(), + working_directory: working_directory.clone(), + env: env.clone(), + }; + Box::new(client::RealGithubClient { gh_run_info }) + } + } + } + + #[instrument] + fn make_updated_pull_request_info( + &self, + effects: &Effects, + references_snapshot: &RepoReferencesSnapshot, + pull_request_infos: &HashMap, + commit_oid: NonZeroOid, + ) -> EyreExitOr { + let mut stack_index = None; + let mut stack_pull_request_infos: IndexMap = + Default::default(); + + // Ensure we iterate over the stack in topological order so that the + // stack indexes are correct. + let stack_commit_oids = self + .dag + .sort(&self.dag.query_stack_commits(CommitSet::from(commit_oid))?)?; + let get_pull_request_info = + |commit_oid: NonZeroOid| -> eyre::Result> { + let commit = self.repo.find_commit_or_fail(commit_oid)?; // for debug output + + debug!(?commit, "Checking commit for pull request info"); + let stack_branch_names = + match references_snapshot.branch_oid_to_names.get(&commit_oid) { + Some(stack_branch_names) => stack_branch_names, + None => { + debug!(?commit, "Commit has no associated branches"); + return Ok(None); + } + }; + + // The commit should have at most one associated branch with a pull + // request. + for stack_branch_name in stack_branch_names.iter().sorted() { + let stack_local_branch = match self.repo.find_branch( + &CategorizedReferenceName::new(stack_branch_name).render_suffix(), + BranchType::Local, + )? { + Some(stack_local_branch) => stack_local_branch, + None => { + debug!( + ?commit, + ?stack_branch_name, + "Skipping branch with no local branch" + ); + continue; + } + }; + + let stack_remote_branch_name = match stack_local_branch + .get_upstream_branch_name_without_push_remote_name()? + { + Some(stack_remote_branch_name) => stack_remote_branch_name, + None => { + debug!( + ?commit, + ?stack_local_branch, + "Skipping local branch with no remote branch" + ); + continue; + } + }; + + let pull_request_info = match pull_request_infos.get(&stack_remote_branch_name) + { + Some(pull_request_info) => pull_request_info, + None => { + debug!( + ?commit, + ?stack_local_branch, + ?stack_remote_branch_name, + "Skipping remote branch with no pull request info" + ); + continue; + } + }; + + debug!( + ?commit, + ?pull_request_info, + "Found pull request info for commit" + ); + return Ok(Some(pull_request_info)); + } + + debug!( + ?commit, + "Commit has no branches with associated pull request info" + ); + Ok(None) + }; + for stack_commit_oid in stack_commit_oids { + let pull_request_info = match get_pull_request_info(stack_commit_oid)? { + Some(info) => info, + None => continue, + }; + stack_pull_request_infos.insert(stack_commit_oid, pull_request_info); + if stack_commit_oid == commit_oid { + stack_index = Some(stack_pull_request_infos.len()); + } + } + + let stack_size = stack_pull_request_infos.len(); + if stack_size == 0 { + warn!( + ?commit_oid, + ?stack_pull_request_infos, + "No pull requests in stack for commit" + ); + } + let stack_index = match stack_index { + Some(stack_index) => stack_index.to_string(), + None => { + warn!( + ?commit_oid, + ?stack_pull_request_infos, + "Could not determine index in stack for commit" + ); + "?".to_string() + } + }; + + let stack_list = { + let mut result = String::new(); + for stack_pull_request_info in stack_pull_request_infos.values() { + // Github will render a lone pull request URL as a title and + // open/closed status. + writeln!(result, "* {}", stack_pull_request_info.url)?; + } + result + }; + + let commit = self.repo.find_commit_or_fail(commit_oid)?; + let commit_summary = commit.get_summary()?; + let commit_summary = String::from_utf8_lossy(&commit_summary).into_owned(); + let title = format!("[{stack_index}/{stack_size}] {commit_summary}"); + let commit_message = commit.get_message_pretty(); + let commit_message = String::from_utf8_lossy(&commit_message); + let body = format!( + "\ +**Stack:** + +{stack_list} + +--- + +{commit_message} +" + ); + + let stack_ancestor_oids = { + let main_branch_oid = CommitSet::from(references_snapshot.main_branch_oid); + let stack_ancestor_oids = self + .dag + .query_only(CommitSet::from(commit_oid), main_branch_oid)? + .difference(&CommitSet::from(commit_oid)); + self.dag.commit_set_to_vec(&stack_ancestor_oids)? + }; + let nearest_ancestor_with_pull_request_info = { + let mut result = None; + for stack_ancestor_oid in stack_ancestor_oids.into_iter().rev() { + if let Some(info) = get_pull_request_info(stack_ancestor_oid)? { + result = Some(info); + break; + } + } + result + }; + let base_ref_name = match nearest_ancestor_with_pull_request_info { + Some(info) => info.head_ref_name.clone(), + None => get_main_branch_name(self.repo)?, + }; + + Ok(Ok(client::UpdatePullRequestArgs { + head_ref_oid: commit_oid, + base_ref_name, + title, + body, + })) + } +} + +mod client { + use std::collections::{BTreeMap, HashMap}; + use std::fmt::{Debug, Write}; + use std::fs::{self, File}; + use std::path::{Path, PathBuf}; + use std::process::{Command, Stdio}; + use std::sync::Arc; + + use eyre::Context; + use itertools::Itertools; + use lib::core::dag::Dag; + use lib::core::effects::{Effects, OperationType}; + use lib::core::eventlog::{EventLogDb, EventReplayer}; + use lib::core::formatting::Glyphs; + use lib::core::repo_ext::RepoExt; + use lib::git::{GitRunInfo, NonZeroOid, Repo, SerializedNonZeroOid}; + use lib::try_exit_code; + use lib::util::{ExitCode, EyreExitOr}; + use serde::{Deserialize, Serialize}; + use tempfile::NamedTempFile; + use tracing::{debug, instrument}; + + use crate::SubmitOptions; + + #[derive(Clone, Debug, Deserialize, Serialize)] + pub struct PullRequestInfo { + #[serde(rename = "number")] + pub number: usize, + #[serde(rename = "url")] + pub url: String, + #[serde(rename = "headRefName")] + pub head_ref_name: String, + #[serde(rename = "headRefOid")] + pub head_ref_oid: SerializedNonZeroOid, + #[serde(rename = "baseRefName")] + pub base_ref_name: String, + #[serde(rename = "closed")] + pub closed: bool, + #[serde(rename = "isDraft")] + pub is_draft: bool, + #[serde(rename = "title")] + pub title: String, + #[serde(rename = "body")] + pub body: String, + } + + #[derive(Debug)] + pub struct CreatePullRequestArgs { + pub head_ref_oid: NonZeroOid, + pub head_ref_name: String, + pub title: String, + pub body: String, + } + + #[derive(Debug, Eq, PartialEq)] + pub struct UpdatePullRequestArgs { + pub head_ref_oid: NonZeroOid, + pub base_ref_name: String, + pub title: String, + pub body: String, + } + + impl UpdatePullRequestArgs { + pub fn fields_to_update(&self, pull_request_info: &PullRequestInfo) -> Vec<&'static str> { + let PullRequestInfo { + number: _, + url: _, + head_ref_name: _, + head_ref_oid: SerializedNonZeroOid(old_head_ref_oid), + base_ref_name: old_base_ref_name, + closed: _, + is_draft: _, + title: old_title, + body: old_body, + } = pull_request_info; + let Self { + head_ref_oid: new_head_ref_oid, + base_ref_name: new_base_ref_name, + title: new_title, + body: new_body, + } = self; + + let mut updated_fields = Vec::new(); + if old_head_ref_oid != new_head_ref_oid { + updated_fields.push("commit"); + } + if old_base_ref_name != new_base_ref_name { + updated_fields.push("base branch"); + } + if old_title != new_title { + updated_fields.push("title"); + } + if old_body != new_body { + updated_fields.push("body"); + } + updated_fields + } + } + + pub trait GithubClient: Debug { + /// Get the username of the currently-logged-in user. + fn query_github_username(&self, effects: &Effects) -> EyreExitOr; + + /// Get the details of all pull requests for the currently-logged-in user in + /// the current repository. The resulting map is keyed by remote branch + /// name. + fn query_repo_pull_request_infos( + &self, + effects: &Effects, + ) -> EyreExitOr>; + + fn create_pull_request( + &self, + effects: &Effects, + args: CreatePullRequestArgs, + submit_options: &super::SubmitOptions, + ) -> EyreExitOr; + + fn update_pull_request( + &self, + effects: &Effects, + number: usize, + args: UpdatePullRequestArgs, + submit_options: &super::SubmitOptions, + ) -> EyreExitOr<()>; + } + + #[derive(Debug)] + pub struct RealGithubClient { + pub gh_run_info: GitRunInfo, + } + + impl RealGithubClient { + #[instrument] + fn run_gh(&self, effects: &Effects, args: &[&str]) -> EyreExitOr> { + let exe = "gh"; + let exe_invocation = format!("{exe} {}", args.join(" ")); + debug!(?exe_invocation, "Invoking gh"); + let (effects, progress) = + effects.start_operation(OperationType::RunTests(Arc::new(exe_invocation.clone()))); + let _progress = progress; + + let child = Command::new("gh") + .args(args) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .context("Invoking `gh` command-line executable")?; + let output = child + .wait_with_output() + .context("Waiting for `gh` invocation")?; + if !output.status.success() { + writeln!( + effects.get_output_stream(), + "Call to `{exe_invocation}` failed", + )?; + writeln!(effects.get_output_stream(), "Stdout:")?; + writeln!( + effects.get_output_stream(), + "{}", + String::from_utf8_lossy(&output.stdout) + )?; + writeln!(effects.get_output_stream(), "Stderr:")?; + writeln!( + effects.get_output_stream(), + "{}", + String::from_utf8_lossy(&output.stderr) + )?; + return Ok(Err(ExitCode::try_from(output.status)?)); + } + Ok(Ok(output.stdout)) + } + + #[instrument] + fn write_body_file(&self, body: &str) -> eyre::Result { + use std::io::Write; + let mut body_file = NamedTempFile::new()?; + body_file.write_all(body.as_bytes())?; + body_file.flush()?; + Ok(body_file) + } + } + + impl GithubClient for RealGithubClient { + /// Get the username of the currently-logged-in user. + #[instrument] + fn query_github_username(&self, effects: &Effects) -> EyreExitOr { + let username = + try_exit_code!(self.run_gh(effects, &["api", "user", "--jq", ".login"])?); + let username = String::from_utf8(username)?; + let username = username.trim().to_owned(); + Ok(Ok(username)) + } + + /// Get the details of all pull requests for the currently-logged-in user in + /// the current repository. The resulting map is keyed by remote branch + /// name. + #[instrument] + fn query_repo_pull_request_infos( + &self, + effects: &Effects, + ) -> EyreExitOr> { + let output = try_exit_code!(self.run_gh( + effects, + &[ + "pr", + "list", + "--author", + "@me", + "--json", + "number,url,headRefName,headRefOid,baseRefName,closed,isDraft,title,body", + ] + )?); + let pull_request_infos: Vec = + serde_json::from_slice(&output).wrap_err("Deserializing output from gh pr list")?; + let pull_request_infos = pull_request_infos + .into_iter() + .map(|item| (item.head_ref_name.clone(), item)) + .collect(); + Ok(Ok(pull_request_infos)) + } + + #[instrument] + fn create_pull_request( + &self, + effects: &Effects, + args: CreatePullRequestArgs, + submit_options: &SubmitOptions, + ) -> EyreExitOr { + let CreatePullRequestArgs { + head_ref_oid: _, + head_ref_name, + title, + body, + } = args; + let body_file = self.write_body_file(&body)?; + let mut args = vec![ + "pr", + "create", + "--head", + &head_ref_name, + "--title", + &title, + "--body-file", + body_file.path().to_str().unwrap(), + ]; + + let SubmitOptions { + create: _, + draft, + execution_strategy: _, + num_jobs: _, + message: _, + } = submit_options; + if *draft { + args.push("--draft"); + } + + let stdout = try_exit_code!(self.run_gh(effects, &args)?); + let pull_request_url = match std::str::from_utf8(&stdout) { + Ok(url) => url, + Err(err) => { + writeln!( + effects.get_output_stream(), + "Could not parse output from `gh pr create` as UTF-8: {err}", + )?; + return Ok(Err(ExitCode(1))); + } + }; + let pull_request_url = pull_request_url.trim(); + Ok(Ok(pull_request_url.to_owned())) + } + + fn update_pull_request( + &self, + effects: &Effects, + number: usize, + args: UpdatePullRequestArgs, + _submit_options: &super::SubmitOptions, + ) -> EyreExitOr<()> { + let UpdatePullRequestArgs { + head_ref_oid: _, // branch should have been pushed by caller + base_ref_name, + title, + body, + } = args; + let body_file = self.write_body_file(&body)?; + try_exit_code!(self.run_gh( + effects, + &[ + "pr", + "edit", + &number.to_string(), + "--base", + &base_ref_name, + "--title", + &title, + "--body-file", + &body_file.path().to_str().unwrap(), + ], + )?); + Ok(Ok(())) + } + } + + /// The mock state on disk, representing the remote Github repository and + /// server. + #[derive(Debug, Default, Deserialize, Serialize)] + pub struct MockState { + /// The next index to assign a newly-created pull request. + pub pull_request_index: usize, + + /// Information about all pull requests open for the repository. Sorted + /// for determinism when dumping state for testing. + pub pull_requests: BTreeMap, + } + + impl MockState { + fn load(path: &Path) -> eyre::Result { + let file = match File::open(path) { + Ok(file) => file, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => { + return Ok(Default::default()); + } + Err(err) => return Err(err).wrap_err("Opening mock GitHub client state file"), + }; + let state = serde_json::from_reader(file)?; + Ok(state) + } + + fn restore_invariants(&mut self, remote_repo: &Repo) -> eyre::Result<()> { + let effects = Effects::new_suppress_for_test(Glyphs::text()); + let conn = remote_repo.get_db_conn()?; + let event_log_db = EventLogDb::new(&conn)?; + let event_replayer = + EventReplayer::from_event_log_db(&effects, remote_repo, &event_log_db)?; + let event_cursor = event_replayer.make_default_cursor(); + let references_snapshot = remote_repo.get_references_snapshot()?; + let dag = Dag::open_and_sync( + &effects, + remote_repo, + &event_replayer, + event_cursor, + &references_snapshot, + )?; + + let branches: HashMap = remote_repo + .get_all_local_branches()? + .into_iter() + .map(|branch| -> eyre::Result<_> { + let branch_name = branch.get_name()?.to_owned(); + let branch_oid = branch.get_oid()?.unwrap(); + Ok((branch_name, branch_oid)) + }) + .try_collect()?; + for (_, pull_request_info) in self.pull_requests.iter_mut() { + let base_ref_name = &pull_request_info.base_ref_name; + let base_branch_oid = match branches.get(base_ref_name) { + Some(oid) => *oid, + None => { + eyre::bail!("Could not find base branch {base_ref_name:?} for pull request: {pull_request_info:?}"); + } + }; + let SerializedNonZeroOid(head_ref_oid) = pull_request_info.head_ref_oid; + if dag.query_is_ancestor(head_ref_oid, base_branch_oid)? { + pull_request_info.closed = true; + } + } + Ok(()) + } + + fn save(&self, path: &Path) -> eyre::Result<()> { + let state = serde_json::to_string_pretty(self)?; + fs::write(path, state)?; + Ok(()) + } + } + + /// A mock client representing the remote Github repository and server. + #[derive(Debug)] + pub struct MockGithubClient { + /// The path to the remote repository on disk. + pub remote_repo_path: PathBuf, + } + + impl GithubClient for MockGithubClient { + fn query_github_username(&self, _effects: &Effects) -> EyreExitOr { + Ok(Ok(Self::username().to_owned())) + } + + fn query_repo_pull_request_infos( + &self, + _effects: &Effects, + ) -> EyreExitOr> { + let pull_requests_infos = self.with_state_mut(|state| { + let pull_request_infos = state + .pull_requests + .values() + .cloned() + .map(|pull_request_info| { + (pull_request_info.head_ref_name.clone(), pull_request_info) + }) + .collect(); + Ok(pull_request_infos) + })?; + Ok(Ok(pull_requests_infos)) + } + + fn create_pull_request( + &self, + _effects: &Effects, + args: CreatePullRequestArgs, + submit_options: &super::SubmitOptions, + ) -> EyreExitOr { + let url = self.with_state_mut(|state| { + state.pull_request_index += 1; + let CreatePullRequestArgs { + head_ref_oid, + head_ref_name, + title, + body, + } = args; + let SubmitOptions { + create, + draft, + execution_strategy: _, + num_jobs: _, + message: _, + } = submit_options; + assert!(create); + let url = format!( + "https://example.com/{}/{}/pulls/{}", + Self::username(), + Self::repo_name(), + state.pull_request_index + ); + let pull_request_info = PullRequestInfo { + number: state.pull_request_index, + url: url.clone(), + head_ref_name: head_ref_name.clone(), + head_ref_oid: SerializedNonZeroOid(head_ref_oid), + base_ref_name: Self::main_branch().to_owned(), + closed: false, + is_draft: *draft, + title, + body, + }; + state.pull_requests.insert(head_ref_name, pull_request_info); + Ok(url) + })?; + Ok(Ok(url)) + } + + fn update_pull_request( + &self, + _effects: &Effects, + number: usize, + args: UpdatePullRequestArgs, + _submit_options: &super::SubmitOptions, + ) -> EyreExitOr<()> { + self.with_state_mut(|state| -> eyre::Result<()> { + let UpdatePullRequestArgs { + head_ref_oid, + base_ref_name, + title, + body, + } = args; + let pull_request_info = match state + .pull_requests + .values_mut() + .find(|pull_request_info| pull_request_info.number == number) + { + Some(pull_request_info) => pull_request_info, + None => { + eyre::bail!("Could not find pull request with number {number}"); + } + }; + pull_request_info.head_ref_oid = SerializedNonZeroOid(head_ref_oid); + pull_request_info.base_ref_name = base_ref_name; + pull_request_info.title = title; + pull_request_info.body = body; + Ok(()) + })?; + Ok(Ok(())) + } + } + + impl MockGithubClient { + fn username() -> &'static str { + "mock-github-username" + } + + fn repo_name() -> &'static str { + "mock-github-repo" + } + + fn main_branch() -> &'static str { + "master" + } + + /// Get the path on disk where the mock state is stored. + pub fn state_path(&self) -> PathBuf { + self.remote_repo_path.join("mock-github-client-state.json") + } + + /// Load the mock state from disk, run the given function, and then save + /// the state back to disk. Github-specific pull request invariants are + /// restored before and after running the function. + pub fn with_state_mut( + &self, + f: impl FnOnce(&mut MockState) -> eyre::Result, + ) -> eyre::Result { + let repo = Repo::from_dir(&self.remote_repo_path)?; + let state_path = self.state_path(); + let mut state = MockState::load(&state_path)?; + state.restore_invariants(&repo)?; + let result = f(&mut state)?; + state.restore_invariants(&repo)?; + state.save(&state_path)?; + Ok(result) + } + } +} + +/// Testing utilities. +pub mod testing { + pub use super::client::MockGithubClient; +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_commit_summary_slug() { + assert_eq!(commit_summary_slug("hello: foo bar"), "hello-foo-bar"); + assert_eq!( + commit_summary_slug("category(topic): `foo` bar!"), + "category-topic-foo-bar" + ); + assert_eq!(commit_summary_slug("foo_~_bar"), "foo-bar"); + assert_eq!(commit_summary_slug("!!!"), "to-review") } } diff --git a/git-branchless-submit/src/lib.rs b/git-branchless-submit/src/lib.rs index ac777702f..780d3cd70 100644 --- a/git-branchless-submit/src/lib.rs +++ b/git-branchless-submit/src/lib.rs @@ -40,6 +40,8 @@ use git_branchless_revset::resolve_commits; use phabricator::PhabricatorForge; use tracing::{debug, info, instrument, warn}; +use crate::github::github_push_remote; + lazy_static! { /// The style for branches which were successfully submitted. pub static ref STYLE_PUSHED: Style = @@ -104,7 +106,6 @@ pub struct CommitStatus { /// individual forge implementation can return this from /// `Forge::query_status` and it will be passed back to /// `Forge::create`/`Forge::update`. - #[allow(dead_code)] remote_commit_name: Option, } @@ -299,7 +300,7 @@ fn submit( &references_snapshot, &unioned_revset, forge_kind, - ); + )?; let statuses = try_exit_code!(forge.query_status(commit_set)?); debug!(?statuses, "Commit statuses"); @@ -522,13 +523,20 @@ fn select_forge<'a>( event_log_db: &'a EventLogDb, references_snapshot: &'a RepoReferencesSnapshot, revset: &'a Revset, - forge: Option, -) -> Box { - let forge_kind = match forge { + forge_kind: Option, +) -> eyre::Result> { + // Check if explicitly set: + let forge_kind = match forge_kind { Some(forge_kind) => { info!(?forge_kind, "Forge kind was explicitly set"); - forge_kind + Some(forge_kind) } + None => None, + }; + + // Check Phabricator: + let forge_kind = match forge_kind { + Some(forge_kind) => Some(forge_kind), None => { let use_phabricator = if let Some(working_copy_path) = repo.get_working_copy_path() { let arcconfig_path = &working_copy_path.join(".arcconfig"); @@ -542,16 +550,21 @@ fn select_forge<'a>( } else { false }; - if use_phabricator { - ForgeKind::Phabricator - } else { - ForgeKind::Branch - } + use_phabricator.then_some(ForgeKind::Phabricator) } }; + // Check Github: + let forge_kind = match forge_kind { + Some(forge_kind) => Some(forge_kind), + None => github_push_remote(repo)?.map(|_| ForgeKind::Github), + }; + + // Default: + let forge_kind = forge_kind.unwrap_or(ForgeKind::Branch); + info!(?forge_kind, "Selected forge kind"); - match forge_kind { + let forge: Box = match forge_kind { ForgeKind::Branch => Box::new(BranchForge { effects, git_run_info, @@ -567,6 +580,7 @@ fn select_forge<'a>( repo, dag, event_log_db, + client: GithubForge::client(git_run_info.clone()), }), ForgeKind::Phabricator => Box::new(PhabricatorForge { @@ -577,5 +591,6 @@ fn select_forge<'a>( event_log_db, revset, }), - } + }; + Ok(forge) } diff --git a/git-branchless-submit/tests/test_github_forge.rs b/git-branchless-submit/tests/test_github_forge.rs new file mode 100644 index 000000000..6778beab5 --- /dev/null +++ b/git-branchless-submit/tests/test_github_forge.rs @@ -0,0 +1,598 @@ +use std::collections::HashMap; +use std::fs; + +use git_branchless_submit::github::testing::MockGithubClient; +use lib::git::{GitVersion, SerializedNonZeroOid}; +use lib::testing::{ + make_git_with_remote_repo, remove_rebase_lines, Git, GitRunOptions, GitWrapperWithRemoteRepo, +}; + +/// Minimum version due to changes in the output of `git push`. +const MIN_VERSION: GitVersion = GitVersion(2, 36, 0); + +fn mock_env(git: &Git) -> HashMap { + git.get_base_env(0) + .into_iter() + .map(|(k, v)| { + ( + k.to_str().unwrap().to_string(), + v.to_str().unwrap().to_string(), + ) + }) + .chain([( + git_branchless_submit::github::MOCK_REMOTE_REPO_PATH_ENV_KEY.to_string(), + git.repo_path.clone().to_str().unwrap().to_owned(), + )]) + .collect() +} + +fn dump_state(local_repo: &Git, remote_repo: &Git) -> eyre::Result { + let local_repo_smartlog: String = local_repo.smartlog()?; + let remote_repo_smartlog = remote_repo.smartlog()?; + let client = MockGithubClient { + remote_repo_path: remote_repo.repo_path.clone(), + }; + let pull_request_info_path = client.state_path(); + let pull_request_info = + fs::read_to_string(pull_request_info_path).unwrap_or_else(|err| format!("Error: {err}")); + let state = format!( + "\ +Local state: +{local_repo_smartlog} + +Remote state: +{remote_repo_smartlog} + +Pull request info: +{pull_request_info} +" + ); + Ok(state) +} + +fn rebase_and_merge(remote_repo: &Git, branch_name: &str) -> eyre::Result<()> { + remote_repo.run(&["cherry-pick", branch_name])?; + remote_repo.run(&["branch", "-f", branch_name, "HEAD"])?; + let head_info = remote_repo.get_repo()?.get_head_info()?; + let head_oid = head_info.oid.unwrap(); + let client = MockGithubClient { + remote_repo_path: remote_repo.repo_path.clone(), + }; + client.with_state_mut(|state| { + state + .pull_requests + .get_mut(branch_name) + .unwrap() + .head_ref_oid = SerializedNonZeroOid(head_oid); + Ok(()) + })?; + Ok(()) +} + +#[test] +fn test_github_forge_reorder_commits() -> eyre::Result<()> { + let GitWrapperWithRemoteRepo { + temp_dir: _temp_dir, + original_repo: remote_repo, + cloned_repo: local_repo, + } = make_git_with_remote_repo()?; + if remote_repo.get_version()? < MIN_VERSION { + return Ok(()); + } + + remote_repo.init_repo()?; + remote_repo.clone_repo_into(&local_repo, &[])?; + + local_repo.detach_head()?; + local_repo.commit_file("test1", 1)?; + local_repo.commit_file("test2", 2)?; + { + let (stdout, _stderr) = local_repo.branchless_with_options( + "submit", + &["--create", "--forge", "github"], + &GitRunOptions { + env: mock_env(&remote_repo), + ..Default::default() + }, + )?; + insta::assert_snapshot!(stdout, @r###" + branchless: running command: push --set-upstream origin mock-github-username/create-test1-txt + branch 'mock-github-username/create-test1-txt' set up to track 'origin/mock-github-username/create-test1-txt'. + branchless: running command: push --set-upstream origin mock-github-username/create-test2-txt + branch 'mock-github-username/create-test2-txt' set up to track 'origin/mock-github-username/create-test2-txt'. + Updating pull request (title, body) for commit 62fc20d create test1.txt + branchless: running command: push --force-with-lease origin mock-github-username/create-test1-txt + Updating pull request (base branch, title, body) for commit 96d1c37 create test2.txt + branchless: running command: push --force-with-lease origin mock-github-username/create-test2-txt + Submitted 2 commits: mock-github-username/create-test1-txt, mock-github-username/create-test2-txt + "###); + } + { + let state = dump_state(&local_repo, &remote_repo)?; + insta::assert_snapshot!(state, @r###" + Local state: + O f777ecc (master) create initial.txt + | + o 62fc20d (mock-github-username/create-test1-txt) create test1.txt + | + @ 96d1c37 (mock-github-username/create-test2-txt) create test2.txt + + + Remote state: + @ f777ecc (> master) create initial.txt + | + o 62fc20d (mock-github-username/create-test1-txt) create test1.txt + | + o 96d1c37 (mock-github-username/create-test2-txt) create test2.txt + + + Pull request info: + { + "pull_request_index": 2, + "pull_requests": { + "mock-github-username/create-test1-txt": { + "number": 1, + "url": "https://example.com/mock-github-username/mock-github-repo/pulls/1", + "headRefName": "mock-github-username/create-test1-txt", + "headRefOid": "62fc20d2a290daea0d52bdc2ed2ad4be6491010e", + "baseRefName": "master", + "closed": false, + "isDraft": false, + "title": "[1/2] create test1.txt", + "body": "**Stack:**\n\n* https://example.com/mock-github-username/mock-github-repo/pulls/1\n* https://example.com/mock-github-username/mock-github-repo/pulls/2\n\n\n---\n\ncreate test1.txt\n\n" + }, + "mock-github-username/create-test2-txt": { + "number": 2, + "url": "https://example.com/mock-github-username/mock-github-repo/pulls/2", + "headRefName": "mock-github-username/create-test2-txt", + "headRefOid": "96d1c37a3d4363611c49f7e52186e189a04c531f", + "baseRefName": "mock-github-username/create-test1-txt", + "closed": false, + "isDraft": false, + "title": "[2/2] create test2.txt", + "body": "**Stack:**\n\n* https://example.com/mock-github-username/mock-github-repo/pulls/1\n* https://example.com/mock-github-username/mock-github-repo/pulls/2\n\n\n---\n\ncreate test2.txt\n\n" + } + } + } + "###); + } + + local_repo.branchless( + "move", + &["--source", "HEAD", "--dest", "master", "--insert"], + )?; + { + let (stdout, _stderr) = local_repo.branchless_with_options( + "submit", + &["--forge", "github"], + &GitRunOptions { + env: mock_env(&remote_repo), + ..Default::default() + }, + )?; + insta::assert_snapshot!(stdout, @r###" + Updating pull request (commit, base branch, title, body) for commit fe65c1f create test2.txt + branchless: running command: push --force-with-lease origin mock-github-username/create-test2-txt + Updating pull request (commit, base branch, title, body) for commit 0770943 create test1.txt + branchless: running command: push --force-with-lease origin mock-github-username/create-test1-txt + Updated 2 commits: mock-github-username/create-test1-txt, mock-github-username/create-test2-txt + "###); + } + { + let state = dump_state(&local_repo, &remote_repo)?; + insta::assert_snapshot!(state, @r###" + Local state: + O f777ecc (master) create initial.txt + | + @ fe65c1f (> mock-github-username/create-test2-txt) create test2.txt + | + o 0770943 (mock-github-username/create-test1-txt) create test1.txt + + + Remote state: + @ f777ecc (> master) create initial.txt + | + o fe65c1f (mock-github-username/create-test2-txt) create test2.txt + | + o 0770943 (mock-github-username/create-test1-txt) create test1.txt + + + Pull request info: + { + "pull_request_index": 2, + "pull_requests": { + "mock-github-username/create-test1-txt": { + "number": 1, + "url": "https://example.com/mock-github-username/mock-github-repo/pulls/1", + "headRefName": "mock-github-username/create-test1-txt", + "headRefOid": "07709435a8f6d1566e0091896d130c78acd429dd", + "baseRefName": "mock-github-username/create-test2-txt", + "closed": false, + "isDraft": false, + "title": "[2/2] create test1.txt", + "body": "**Stack:**\n\n* https://example.com/mock-github-username/mock-github-repo/pulls/2\n* https://example.com/mock-github-username/mock-github-repo/pulls/1\n\n\n---\n\ncreate test1.txt\n\n" + }, + "mock-github-username/create-test2-txt": { + "number": 2, + "url": "https://example.com/mock-github-username/mock-github-repo/pulls/2", + "headRefName": "mock-github-username/create-test2-txt", + "headRefOid": "fe65c1fe15584744e649b2c79d4cf9b0d878f92e", + "baseRefName": "master", + "closed": false, + "isDraft": false, + "title": "[1/2] create test2.txt", + "body": "**Stack:**\n\n* https://example.com/mock-github-username/mock-github-repo/pulls/2\n* https://example.com/mock-github-username/mock-github-repo/pulls/1\n\n\n---\n\ncreate test2.txt\n\n" + } + } + } + "###); + } + + Ok(()) +} + +#[test] +fn test_github_forge_mock_client_closes_pull_requests() -> eyre::Result<()> { + let GitWrapperWithRemoteRepo { + temp_dir: _temp_dir, + original_repo: remote_repo, + cloned_repo: local_repo, + } = make_git_with_remote_repo()?; + if remote_repo.get_version()? < MIN_VERSION { + return Ok(()); + } + + remote_repo.init_repo()?; + remote_repo.clone_repo_into(&local_repo, &[])?; + + local_repo.detach_head()?; + local_repo.commit_file("test1", 1)?; + local_repo.commit_file("test2", 2)?; + + { + let (stdout, _stderr) = local_repo.branchless_with_options( + "submit", + &["--forge", "github", "--create"], + &GitRunOptions { + env: mock_env(&remote_repo), + ..Default::default() + }, + )?; + insta::assert_snapshot!(stdout, @r###" + branchless: running command: push --set-upstream origin mock-github-username/create-test1-txt + branch 'mock-github-username/create-test1-txt' set up to track 'origin/mock-github-username/create-test1-txt'. + branchless: running command: push --set-upstream origin mock-github-username/create-test2-txt + branch 'mock-github-username/create-test2-txt' set up to track 'origin/mock-github-username/create-test2-txt'. + Updating pull request (title, body) for commit 62fc20d create test1.txt + branchless: running command: push --force-with-lease origin mock-github-username/create-test1-txt + Updating pull request (base branch, title, body) for commit 96d1c37 create test2.txt + branchless: running command: push --force-with-lease origin mock-github-username/create-test2-txt + Submitted 2 commits: mock-github-username/create-test1-txt, mock-github-username/create-test2-txt + "###); + } + { + let state = dump_state(&local_repo, &remote_repo)?; + insta::assert_snapshot!(state, @r###" + Local state: + O f777ecc (master) create initial.txt + | + o 62fc20d (mock-github-username/create-test1-txt) create test1.txt + | + @ 96d1c37 (mock-github-username/create-test2-txt) create test2.txt + + + Remote state: + @ f777ecc (> master) create initial.txt + | + o 62fc20d (mock-github-username/create-test1-txt) create test1.txt + | + o 96d1c37 (mock-github-username/create-test2-txt) create test2.txt + + + Pull request info: + { + "pull_request_index": 2, + "pull_requests": { + "mock-github-username/create-test1-txt": { + "number": 1, + "url": "https://example.com/mock-github-username/mock-github-repo/pulls/1", + "headRefName": "mock-github-username/create-test1-txt", + "headRefOid": "62fc20d2a290daea0d52bdc2ed2ad4be6491010e", + "baseRefName": "master", + "closed": false, + "isDraft": false, + "title": "[1/2] create test1.txt", + "body": "**Stack:**\n\n* https://example.com/mock-github-username/mock-github-repo/pulls/1\n* https://example.com/mock-github-username/mock-github-repo/pulls/2\n\n\n---\n\ncreate test1.txt\n\n" + }, + "mock-github-username/create-test2-txt": { + "number": 2, + "url": "https://example.com/mock-github-username/mock-github-repo/pulls/2", + "headRefName": "mock-github-username/create-test2-txt", + "headRefOid": "96d1c37a3d4363611c49f7e52186e189a04c531f", + "baseRefName": "mock-github-username/create-test1-txt", + "closed": false, + "isDraft": false, + "title": "[2/2] create test2.txt", + "body": "**Stack:**\n\n* https://example.com/mock-github-username/mock-github-repo/pulls/1\n* https://example.com/mock-github-username/mock-github-repo/pulls/2\n\n\n---\n\ncreate test2.txt\n\n" + } + } + } + "###); + } + + rebase_and_merge(&remote_repo, "mock-github-username/create-test1-txt")?; + { + let (stdout, _stderr) = local_repo.branchless("sync", &["--pull"])?; + let stdout = remove_rebase_lines(stdout); + insta::assert_snapshot!(stdout, @r###" + branchless: running command: fetch --all + Fast-forwarding branch master to 047b7ad create test1.txt + Attempting rebase in-memory... + [1/2] Skipped commit (was already applied upstream): 62fc20d create test1.txt + [2/2] Committed as: fa46633 create test2.txt + branchless: running command: checkout mock-github-username/create-test2-txt + Your branch and 'origin/mock-github-username/create-test2-txt' have diverged, + and have 2 and 2 different commits each, respectively. + In-memory rebase succeeded. + Synced 62fc20d create test1.txt + "###); + } + { + let stdout = local_repo.smartlog()?; + insta::assert_snapshot!(stdout, @r###" + : + O 047b7ad (master) create test1.txt + | + @ fa46633 (> mock-github-username/create-test2-txt) create test2.txt + "###); + } + + { + let (stdout, _stderr) = local_repo.branchless_with_options( + "submit", + &["--forge", "github"], + &GitRunOptions { + env: mock_env(&remote_repo), + ..Default::default() + }, + )?; + insta::assert_snapshot!(stdout, @r###" + Updating pull request (commit, base branch, title, body) for commit fa46633 create test2.txt + branchless: running command: push --force-with-lease origin mock-github-username/create-test2-txt + Updated 1 commit: mock-github-username/create-test2-txt + "###); + } + { + let state = dump_state(&local_repo, &remote_repo)?; + insta::assert_snapshot!(state, @r###" + Local state: + : + O 047b7ad (master) create test1.txt + | + @ fa46633 (> mock-github-username/create-test2-txt) create test2.txt + + + Remote state: + : + @ 047b7ad (> master, mock-github-username/create-test1-txt) create test1.txt + | + o fa46633 (mock-github-username/create-test2-txt) create test2.txt + + + Pull request info: + { + "pull_request_index": 2, + "pull_requests": { + "mock-github-username/create-test1-txt": { + "number": 1, + "url": "https://example.com/mock-github-username/mock-github-repo/pulls/1", + "headRefName": "mock-github-username/create-test1-txt", + "headRefOid": "047b7ad7790bd443d78ea38854cecb9d9cc7fb7a", + "baseRefName": "master", + "closed": true, + "isDraft": false, + "title": "[1/2] create test1.txt", + "body": "**Stack:**\n\n* https://example.com/mock-github-username/mock-github-repo/pulls/1\n* https://example.com/mock-github-username/mock-github-repo/pulls/2\n\n\n---\n\ncreate test1.txt\n\n" + }, + "mock-github-username/create-test2-txt": { + "number": 2, + "url": "https://example.com/mock-github-username/mock-github-repo/pulls/2", + "headRefName": "mock-github-username/create-test2-txt", + "headRefOid": "fa46633239bfa767036e41a77b67258286e4ddb9", + "baseRefName": "master", + "closed": false, + "isDraft": false, + "title": "[1/1] create test2.txt", + "body": "**Stack:**\n\n* https://example.com/mock-github-username/mock-github-repo/pulls/2\n\n\n---\n\ncreate test2.txt\n\n" + } + } + } + "###); + } + + Ok(()) +} + +#[test] +fn test_github_forge_no_include_unsubmitted_commits_in_stack() -> eyre::Result<()> { + let GitWrapperWithRemoteRepo { + temp_dir: _temp_dir, + original_repo: remote_repo, + cloned_repo: local_repo, + } = make_git_with_remote_repo()?; + if remote_repo.get_version()? < MIN_VERSION { + return Ok(()); + } + + remote_repo.init_repo()?; + remote_repo.clone_repo_into(&local_repo, &[])?; + + local_repo.detach_head()?; + local_repo.commit_file("test1", 1)?; + local_repo.commit_file("test2", 2)?; + local_repo.commit_file("test3", 3)?; + { + let stdout = local_repo.smartlog()?; + insta::assert_snapshot!(stdout, @r###" + O f777ecc (master) create initial.txt + | + o 62fc20d create test1.txt + | + o 96d1c37 create test2.txt + | + @ 70deb1e create test3.txt + "###); + } + + { + let (stdout, _stderr) = local_repo.branchless_with_options( + "submit", + &[&"--forge", "github", "--create", "HEAD^^"], + &GitRunOptions { + env: mock_env(&remote_repo), + ..Default::default() + }, + )?; + insta::assert_snapshot!(stdout, @r###" + branchless: running command: push --set-upstream origin mock-github-username/create-test1-txt + branch 'mock-github-username/create-test1-txt' set up to track 'origin/mock-github-username/create-test1-txt'. + Updating pull request (title, body) for commit 62fc20d create test1.txt + branchless: running command: push --force-with-lease origin mock-github-username/create-test1-txt + Submitted 1 commit: mock-github-username/create-test1-txt + "###); + } + { + let state = dump_state(&local_repo, &remote_repo)?; + insta::assert_snapshot!(state, @r###" + Local state: + O f777ecc (master) create initial.txt + | + o 62fc20d (mock-github-username/create-test1-txt) create test1.txt + | + o 96d1c37 create test2.txt + | + @ 70deb1e create test3.txt + + + Remote state: + @ f777ecc (> master) create initial.txt + | + o 62fc20d (mock-github-username/create-test1-txt) create test1.txt + + + Pull request info: + { + "pull_request_index": 1, + "pull_requests": { + "mock-github-username/create-test1-txt": { + "number": 1, + "url": "https://example.com/mock-github-username/mock-github-repo/pulls/1", + "headRefName": "mock-github-username/create-test1-txt", + "headRefOid": "62fc20d2a290daea0d52bdc2ed2ad4be6491010e", + "baseRefName": "master", + "closed": false, + "isDraft": false, + "title": "[1/1] create test1.txt", + "body": "**Stack:**\n\n* https://example.com/mock-github-username/mock-github-repo/pulls/1\n\n\n---\n\ncreate test1.txt\n\n" + } + } + } + "###); + } + + Ok(()) +} + +#[test] +fn test_github_forge_multiple_commits_in_pull_request() -> eyre::Result<()> { + let GitWrapperWithRemoteRepo { + temp_dir: _temp_dir, + original_repo: remote_repo, + cloned_repo: local_repo, + } = make_git_with_remote_repo()?; + if remote_repo.get_version()? < MIN_VERSION { + return Ok(()); + } + + remote_repo.init_repo()?; + remote_repo.clone_repo_into(&local_repo, &[])?; + + local_repo.detach_head()?; + local_repo.commit_file("test1", 1)?; + local_repo.commit_file("test2", 2)?; + local_repo.commit_file("test3", 3)?; + { + let stdout = local_repo.smartlog()?; + insta::assert_snapshot!(stdout, @r###" + O f777ecc (master) create initial.txt + | + o 62fc20d create test1.txt + | + o 96d1c37 create test2.txt + | + @ 70deb1e create test3.txt + "###); + } + + { + let (stdout, _stderr) = local_repo.branchless_with_options( + "submit", + &[&"--forge", "github", "--create", "HEAD"], + &GitRunOptions { + env: mock_env(&remote_repo), + ..Default::default() + }, + )?; + insta::assert_snapshot!(stdout, @r###" + branchless: running command: push --set-upstream origin mock-github-username/create-test3-txt + branch 'mock-github-username/create-test3-txt' set up to track 'origin/mock-github-username/create-test3-txt'. + Updating pull request (title, body) for commit 70deb1e create test3.txt + branchless: running command: push --force-with-lease origin mock-github-username/create-test3-txt + Submitted 1 commit: mock-github-username/create-test3-txt + "###); + } + { + let state = dump_state(&local_repo, &remote_repo)?; + insta::assert_snapshot!(state, @r###" + Local state: + O f777ecc (master) create initial.txt + | + o 62fc20d create test1.txt + | + o 96d1c37 create test2.txt + | + @ 70deb1e (mock-github-username/create-test3-txt) create test3.txt + + + Remote state: + @ f777ecc (> master) create initial.txt + | + o 62fc20d create test1.txt + | + o 96d1c37 create test2.txt + | + o 70deb1e (mock-github-username/create-test3-txt) create test3.txt + + + Pull request info: + { + "pull_request_index": 1, + "pull_requests": { + "mock-github-username/create-test3-txt": { + "number": 1, + "url": "https://example.com/mock-github-username/mock-github-repo/pulls/1", + "headRefName": "mock-github-username/create-test3-txt", + "headRefOid": "70deb1e28791d8e7dd5a1f0c871a51b91282562f", + "baseRefName": "master", + "closed": false, + "isDraft": false, + "title": "[1/1] create test3.txt", + "body": "**Stack:**\n\n* https://example.com/mock-github-username/mock-github-repo/pulls/1\n\n\n---\n\ncreate test3.txt\n\n" + } + } + } + "###); + } + + Ok(()) +}