From e591fa33381802685d32d8a74b3a5ac806010e2a Mon Sep 17 00:00:00 2001 From: Waleed Khan Date: Sun, 7 Jan 2024 16:05:30 -0800 Subject: [PATCH] feat(submit:github): implement Github forge - It's not very good. It's probably buggy since there's a huge amount of state to keep in sync between commits, branches, and pull requests. - It's not very useful. When using Github's "Rebase and merge" button, it always rewrites the commit to update the committer signature. Then, for some reason, descendant commits can't be rebased automatically, and you have to manually rebase them. --- Cargo.lock | 2 + git-branchless-lib/src/git/reference.rs | 33 +- git-branchless-lib/src/git/repo.rs | 25 + git-branchless-lib/src/git/test.rs | 2 +- git-branchless-lib/src/testing.rs | 4 + git-branchless-submit/Cargo.toml | 2 + git-branchless-submit/src/branch_forge.rs | 11 +- git-branchless-submit/src/github.rs | 1211 ++++++++++++++++- git-branchless-submit/src/lib.rs | 41 +- .../tests/test_github_forge.rs | 598 ++++++++ 10 files changed, 1903 insertions(+), 26 deletions(-) create mode 100644 git-branchless-submit/tests/test_github_forge.rs diff --git a/Cargo.lock b/Cargo.lock index 02d3e820d..720fbee8c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1657,6 +1657,7 @@ dependencies = [ "git-branchless-opts", "git-branchless-revset", "git-branchless-test", + "indexmap 2.2.3", "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 d1cea37a6..622e9bcc8 100644 --- a/git-branchless-lib/src/testing.rs +++ b/git-branchless-lib/src/testing.rs @@ -533,6 +533,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, @@ -563,6 +564,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, @@ -575,6 +577,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(()) +}