From ff11fcdb9000e4d26f9f4865bb6091d046f8df26 Mon Sep 17 00:00:00 2001 From: Scott Chacon Date: Tue, 7 May 2024 10:28:29 +0200 Subject: [PATCH 01/13] Add change-id property to commit Each commit has a change-id added to it's header field so we can more easily track patches by unique ids --- app/src/lib/components/CommitCard.svelte | 17 ++-- app/src/lib/vbranches/types.ts | 4 + .../src/gb_repository/repository.rs | 2 + crates/gitbutler-core/src/git/commit.rs | 16 ++++ crates/gitbutler-core/src/git/repository.rs | 69 +++++++++++--- .../src/project_repository/repository.rs | 6 +- .../src/virtual_branches/base.rs | 3 +- .../src/virtual_branches/integration.rs | 5 ++ .../src/virtual_branches/virtual.rs | 89 +++++++------------ crates/gitbutler-testsupport/src/suite.rs | 2 + .../gitbutler-testsupport/src/test_project.rs | 3 + .../handler/push_project_to_gitbutler.rs | 2 + 12 files changed, 141 insertions(+), 77 deletions(-) diff --git a/app/src/lib/components/CommitCard.svelte b/app/src/lib/components/CommitCard.svelte index b0684b2f7..492eba2ea 100644 --- a/app/src/lib/components/CommitCard.svelte +++ b/app/src/lib/components/CommitCard.svelte @@ -144,11 +144,18 @@
{#if $advancedCommitOperations} - {#if !showFiles} -
- {commit.id.substring(0, 6)} -
- {/if} +
+ + {#if commit.isSigned} + 🔒 + {/if} + {#if commit.changeId} + {commit.changeId.split('-')[0]} + {:else} + {commit.id.substring(0, 6)} + {/if} + +
{/if}
{#if isUndoable} diff --git a/app/src/lib/vbranches/types.ts b/app/src/lib/vbranches/types.ts index c72fd40d1..64b039b0f 100644 --- a/app/src/lib/vbranches/types.ts +++ b/app/src/lib/vbranches/types.ts @@ -162,6 +162,8 @@ export class Commit { files!: LocalFile[]; parentIds!: string[]; branchId!: string; + changeId!: string; + isSigned!: boolean; get isLocal() { return !this.isRemote && !this.isIntegrated; @@ -196,6 +198,8 @@ export class RemoteCommit { description!: string; @Transform((obj) => new Date(obj.value * 1000)) createdAt!: Date; + changeId!: string; + isSigned!: boolean; get isLocal() { return false; diff --git a/crates/gitbutler-core/src/gb_repository/repository.rs b/crates/gitbutler-core/src/gb_repository/repository.rs index 8abfad223..6306c3f47 100644 --- a/crates/gitbutler-core/src/gb_repository/repository.rs +++ b/crates/gitbutler-core/src/gb_repository/repository.rs @@ -889,6 +889,7 @@ fn write_gb_commit( "gitbutler check", // commit message &gb_repository.git_repository.find_tree(tree_id).unwrap(), // tree &[&last_commit], // parents + None, )?; Ok(new_commit) } @@ -900,6 +901,7 @@ fn write_gb_commit( "gitbutler check", // commit message &gb_repository.git_repository.find_tree(tree_id).unwrap(), // tree &[], // parents + None, )?; Ok(new_commit) } diff --git a/crates/gitbutler-core/src/git/commit.rs b/crates/gitbutler-core/src/git/commit.rs index 67a6407da..665d468ce 100644 --- a/crates/gitbutler-core/src/git/commit.rs +++ b/crates/gitbutler-core/src/git/commit.rs @@ -71,6 +71,22 @@ impl<'repo> Commit<'repo> { self.commit.committer().into() } + pub fn change_id(&self) -> Option { + let cid = self.commit.header_field_bytes("change-id").ok()?; + if cid.is_empty() { + None + } else { + // convert the Buf to a string + let ch_id = std::str::from_utf8(&cid).ok()?.to_owned(); + Some(ch_id) + } + } + + pub fn is_signed(&self) -> bool { + let cid = self.commit.header_field_bytes("gpgsig").ok(); + cid.is_some() + } + pub fn raw_header(&self) -> Option<&str> { self.commit.raw_header() } diff --git a/crates/gitbutler-core/src/git/repository.rs b/crates/gitbutler-core/src/git/repository.rs index 7c9430941..492aa351f 100644 --- a/crates/gitbutler-core/src/git/repository.rs +++ b/crates/gitbutler-core/src/git/repository.rs @@ -242,6 +242,7 @@ impl Repository { self.0.blob(data).map(Into::into).map_err(Into::into) } + #[allow(clippy::too_many_arguments)] pub fn commit( &self, update_ref: Option<&Refname>, @@ -250,22 +251,35 @@ impl Repository { message: &str, tree: &Tree<'_>, parents: &[&Commit<'_>], + change_id: Option<&str>, ) -> Result { let parents: Vec<&git2::Commit> = parents .iter() .map(|c| c.to_owned().into()) .collect::>(); - self.0 - .commit( - update_ref.map(ToString::to_string).as_deref(), - author.into(), - committer.into(), - message, - tree.into(), - &parents, - ) - .map(Into::into) - .map_err(Into::into) + + let commit_buffer = self.0.commit_create_buffer( + author.into(), + committer.into(), + message, + tree.into(), + &parents, + )?; + + let commit_buffer = Self::inject_change_id(&commit_buffer, change_id)?; + + let oid = self + .0 + .odb()? + .write(git2::ObjectType::Commit, commit_buffer.as_bytes())?; + + // check git config for gpg.signingkey + + // update reference + if let Some(refname) = update_ref { + self.0.reference(&refname.to_string(), oid, true, message)?; + } + Ok(oid.into()) } pub fn commit_signed( @@ -275,6 +289,7 @@ impl Repository { tree: &Tree<'_>, parents: &[&Commit<'_>], key: &keys::PrivateKey, + change_id: Option<&str>, ) -> Result { let parents: Vec<&git2::Commit> = parents .iter() @@ -289,14 +304,42 @@ impl Repository { tree.into(), &parents, )?; - let commit_buffer = str::from_utf8(&commit_buffer).unwrap(); + let commit_buffer = Self::inject_change_id(&commit_buffer, change_id)?; let signature = key.sign(commit_buffer.as_bytes())?; self.0 - .commit_signed(commit_buffer, &signature, None) + .commit_signed(&commit_buffer, &signature, None) .map(Into::into) .map_err(Into::into) } + // in commit_buffer, inject a line right before the first `\n\n` that we see: + // `change-id: ` + fn inject_change_id(commit_buffer: &[u8], change_id: Option<&str>) -> Result { + // if no change id, generate one + let change_id = change_id + .map(|id| id.to_string()) + .unwrap_or_else(|| format!("{}", uuid::Uuid::new_v4())); + + let commit_ends_in_newline = commit_buffer.ends_with(b"\n"); + let commit_buffer = str::from_utf8(commit_buffer).unwrap(); + let lines = commit_buffer.lines(); + let mut new_buffer = String::new(); + let mut found = false; + for line in lines { + if line.is_empty() && !found { + new_buffer.push_str(&format!("change-id {}\n", change_id)); + found = true; + } + new_buffer.push_str(line); + new_buffer.push('\n'); + } + if !commit_ends_in_newline { + // strip last \n + new_buffer.pop(); + } + Ok(new_buffer) + } + pub fn config(&self) -> Result { self.0.config().map(Into::into).map_err(Into::into) } diff --git a/crates/gitbutler-core/src/project_repository/repository.rs b/crates/gitbutler-core/src/project_repository/repository.rs index be8639d7d..fe8695b57 100644 --- a/crates/gitbutler-core/src/project_repository/repository.rs +++ b/crates/gitbutler-core/src/project_repository/repository.rs @@ -345,15 +345,17 @@ impl Repository { tree: &git::Tree, parents: &[&git::Commit], signing_key: Option<&keys::PrivateKey>, + change_id: Option<&str>, ) -> Result { let (author, committer) = self.git_signatures(user)?; + if let Some(key) = signing_key { self.git_repository - .commit_signed(&author, message, tree, parents, key) + .commit_signed(&author, message, tree, parents, key, change_id) .context("failed to commit signed") } else { self.git_repository - .commit(None, &author, &committer, message, tree, parents) + .commit(None, &author, &committer, message, tree, parents, change_id) .context("failed to commit") } } diff --git a/crates/gitbutler-core/src/virtual_branches/base.rs b/crates/gitbutler-core/src/virtual_branches/base.rs index 3513dd1f9..7664e977a 100644 --- a/crates/gitbutler-core/src/virtual_branches/base.rs +++ b/crates/gitbutler-core/src/virtual_branches/base.rs @@ -508,12 +508,13 @@ pub fn update_base_branch( "Merged {}/{} into {}", target.branch.remote(), target.branch.branch(), - branch.name + branch.name, ) .as_str(), &branch_head_merge_tree, &[&branch_head_commit, &new_target_commit], signing_key, + None, ) .context("failed to commit merge")?; diff --git a/crates/gitbutler-core/src/virtual_branches/integration.rs b/crates/gitbutler-core/src/virtual_branches/integration.rs index bffb1634e..a412bcdbb 100644 --- a/crates/gitbutler-core/src/virtual_branches/integration.rs +++ b/crates/gitbutler-core/src/virtual_branches/integration.rs @@ -89,6 +89,7 @@ pub fn get_workspace_head( WORKSPACE_HEAD, &workspace_tree, branch_head_refs.as_slice(), + None, )?; Ok(workspace_head_id) } @@ -240,6 +241,7 @@ pub fn update_gitbutler_integration_with_commit( &message, &integration_commit.tree()?, &[&target_commit], + None, )?; // write final_tree as the current index @@ -270,6 +272,7 @@ pub fn update_gitbutler_integration_with_commit( &message, &wip_tree, &[&branch_head], + None, )?; branch_head = repo.find_commit(branch_head_oid)?; } @@ -333,6 +336,7 @@ fn verify_head_is_clean( ) .context("failed to reset to integration commit")?; + dbg!("Head creating virtual branch"); let mut new_branch = super::create_virtual_branch( project_repository, &BranchCreateRequest { @@ -363,6 +367,7 @@ fn verify_head_is_clean( &commit.message().to_str_lossy(), &commit.tree().unwrap(), &[&new_branch_head], + None, ) .context(format!( "failed to rebase commit {} onto new branch", diff --git a/crates/gitbutler-core/src/virtual_branches/virtual.rs b/crates/gitbutler-core/src/virtual_branches/virtual.rs index e21c39e9b..8751f9568 100644 --- a/crates/gitbutler-core/src/virtual_branches/virtual.rs +++ b/crates/gitbutler-core/src/virtual_branches/virtual.rs @@ -99,6 +99,8 @@ pub struct VirtualBranchCommit { pub is_integrated: bool, pub parent_ids: Vec, pub branch_id: BranchId, + pub change_id: Option, + pub is_signed: bool, } // this struct is a mapping to the view `File` type in Typescript @@ -358,6 +360,7 @@ pub fn apply_branch( &merged_branch_tree, &[&head_commit, &target_commit], signing_key, + None, )?; // ok, update the virtual branch @@ -427,6 +430,7 @@ pub fn apply_branch( &merge_tree, &[&head_commit, &target_commit], signing_key, + None, ) .context("failed to commit merge")?; @@ -1047,6 +1051,8 @@ fn commit_to_vbranch_commit( is_integrated, parent_ids, branch_id: branch.id, + change_id: commit.change_id(), + is_signed: commit.is_signed(), }; Ok(commit) @@ -1359,6 +1365,7 @@ pub fn merge_virtual_branch_upstream( &merge_tree, &[&head_commit, &upstream_commit], signing_key, + None, )?; // checkout the merge tree @@ -2409,11 +2416,14 @@ pub fn commit( &tree, &[&parent_commit, &merge_parent], signing_key, + None, )?; conflicts::clear(project_repository).context("failed to clear conflicts")?; commit_oid } - None => project_repository.commit(user, message, &tree, &[&parent_commit], signing_key)?, + None => { + project_repository.commit(user, message, &tree, &[&parent_commit], signing_key, None)? + } }; if run_hooks { @@ -2871,6 +2881,7 @@ pub fn move_commit_file( let new_from_tree = &repo .find_tree(new_from_tree_oid) .map_err(|_error| errors::VirtualBranchError::GitObjectNotFound(new_from_tree_oid))?; + let change_id = from_commit.change_id(); let new_from_commit_oid = repo .commit( None, @@ -2879,6 +2890,7 @@ pub fn move_commit_file( &from_commit.message().to_str_lossy(), new_from_tree, &[&from_parent], + change_id.as_deref(), ) .map_err(|_error| errors::VirtualBranchError::CommitFailed)?; @@ -2949,6 +2961,7 @@ pub fn move_commit_file( let parents = amend_commit .parents() .context("failed to find head commit parents")?; + let change_id = amend_commit.change_id(); let commit_oid = project_repository .git_repository .commit( @@ -2958,6 +2971,7 @@ pub fn move_commit_file( &amend_commit.message().to_str_lossy(), &new_tree, &parents.iter().collect::>(), + change_id.as_deref(), ) .context("failed to create commit")?; @@ -3146,6 +3160,7 @@ pub fn amend( &amend_commit.message().to_str_lossy(), &new_tree, &parents.iter().collect::>(), + None, ) .context("failed to create commit")?; @@ -3325,7 +3340,8 @@ pub fn insert_blank_commit( } let commit_tree = commit.tree().unwrap(); - let blank_commit_oid = project_repository.commit(user, "", &commit_tree, &[&commit], None)?; + let blank_commit_oid = + project_repository.commit(user, "", &commit_tree, &[&commit], None, None)?; if commit.id() == branch.head && offset < 0 { // inserting before the first commit @@ -3493,6 +3509,8 @@ fn cherry_rebase_group( .find_tree(merge_tree_oid) .context("failed to find merge tree")?; + let change_id = to_rebase.change_id(); + let commit_oid = project_repository .git_repository .commit( @@ -3502,6 +3520,7 @@ fn cherry_rebase_group( &to_rebase.message().to_str_lossy(), &merge_tree, &[&head], + change_id.as_deref(), ) .context("failed to create commit")?; @@ -3516,60 +3535,6 @@ fn cherry_rebase_group( Ok(Some(new_head_id)) } -// runs a simple libgit2 based in-memory rebase on a commit range onto a target commit -// possibly not used in favor of cherry_rebase -pub fn simple_rebase( - project_repository: &project_repository::Repository, - target_commit_oid: git::Oid, - start_commit_oid: git::Oid, - end_commit_oid: git::Oid, -) -> Result, anyhow::Error> { - let repo = &project_repository.git_repository; - let (_, committer) = project_repository.git_signatures(None)?; - - let mut rebase_options = git2::RebaseOptions::new(); - rebase_options.quiet(true); - rebase_options.inmemory(true); - let mut rebase = repo - .rebase( - Some(end_commit_oid), - Some(start_commit_oid), - Some(target_commit_oid), - Some(&mut rebase_options), - ) - .context("failed to rebase")?; - - let mut rebase_success = true; - // check to see if these commits have already been pushed - let mut last_rebase_head = target_commit_oid; - while rebase.next().is_some() { - let index = rebase - .inmemory_index() - .context("failed to get inmemory index")?; - if index.has_conflicts() { - rebase_success = false; - break; - } - - if let Ok(commit_id) = rebase.commit(None, &committer.clone().into(), None) { - last_rebase_head = commit_id.into(); - } else { - rebase_success = false; - break; - } - } - - if rebase_success { - // rebase worked out, rewrite the branch head - rebase.finish(None).context("failed to finish rebase")?; - Ok(Some(last_rebase_head)) - } else { - // rebase failed, do a merge commit - rebase.abort().context("failed to abort rebase")?; - Ok(None) - } -} - pub fn cherry_pick( project_repository: &project_repository::Repository, branch_id: &BranchId, @@ -3653,6 +3618,7 @@ pub fn cherry_pick( "wip cherry picking commit", &wip_tree, &[&branch_head_commit], + None, ) .context("failed to commit wip work")?; project_repository @@ -3716,6 +3682,7 @@ pub fn cherry_pick( .find_commit(branch.head) .context("failed to find branch head commit")?; + let change_id = target_commit.change_id(); let commit_oid = project_repository .git_repository .commit( @@ -3725,6 +3692,7 @@ pub fn cherry_pick( &target_commit.message().to_str_lossy(), &merge_tree, &[&branch_head_commit], + change_id.as_deref(), ) .context("failed to create commit")?; @@ -3840,6 +3808,9 @@ pub fn squash( .parents() .context("failed to find head commit parents")?; + // use the squash commit's change id + let change_id = commit_to_squash.change_id(); + let new_commit_oid = project_repository .git_repository .commit( @@ -3853,6 +3824,7 @@ pub fn squash( ), &commit_to_squash.tree().context("failed to find tree")?, &parents.iter().collect::>(), + change_id.as_deref(), ) .context("failed to commit")?; @@ -3963,6 +3935,8 @@ pub fn update_commit_message( .parents() .context("failed to find head commit parents")?; + let change_id = target_commit.change_id(); + let new_commit_oid = project_repository .git_repository .commit( @@ -3972,6 +3946,7 @@ pub fn update_commit_message( message, &target_commit.tree().context("failed to find tree")?, &parents.iter().collect::>(), + change_id.as_deref(), ) .context("failed to commit")?; @@ -4155,6 +4130,7 @@ pub fn move_commit( .find_tree(new_destination_tree_oid) .context("failed to find tree")?; + let change_id = source_branch_head.change_id(); let new_destination_head_oid = project_repository .commit( user, @@ -4165,6 +4141,7 @@ pub fn move_commit( .find_commit(destination_branch.head) .context("failed to get dst branch head commit")?], signing_key, + change_id.as_deref(), ) .context("failed to commit")?; diff --git a/crates/gitbutler-testsupport/src/suite.rs b/crates/gitbutler-testsupport/src/suite.rs index 4667c6786..d0190440d 100644 --- a/crates/gitbutler-testsupport/src/suite.rs +++ b/crates/gitbutler-testsupport/src/suite.rs @@ -194,6 +194,7 @@ pub fn test_repository() -> (gitbutler_core::git::Repository, TempDir) { "Initial commit", &repository.find_tree(oid).expect("failed to find tree"), &[], + None, ) .expect("failed to commit"); (repository, tmp) @@ -222,6 +223,7 @@ pub fn commit_all(repository: &gitbutler_core::git::Repository) -> gitbutler_cor .expect("failed to get head"), ) .expect("failed to find commit")], + None, ) .expect("failed to commit"); commit_oid diff --git a/crates/gitbutler-testsupport/src/test_project.rs b/crates/gitbutler-testsupport/src/test_project.rs index d65a5e421..90442f4e1 100644 --- a/crates/gitbutler-testsupport/src/test_project.rs +++ b/crates/gitbutler-testsupport/src/test_project.rs @@ -43,6 +43,7 @@ impl Default for TestProject { .find_tree(oid) .expect("failed to find tree"), &[], + None, ) .expect("failed to commit"); @@ -237,6 +238,7 @@ impl TestProject { &format!("Merge pull request from {}", branch_name), &merge_tree, &[&master_branch_commit, &branch_commit], + None, ) .unwrap(); } @@ -312,6 +314,7 @@ impl TestProject { .expect("failed to get head"), ) .expect("failed to find commit")], + None, ) .expect("failed to commit") } diff --git a/crates/gitbutler-watcher/tests/handler/push_project_to_gitbutler.rs b/crates/gitbutler-watcher/tests/handler/push_project_to_gitbutler.rs index 1f4386b8f..582ae58c0 100644 --- a/crates/gitbutler-watcher/tests/handler/push_project_to_gitbutler.rs +++ b/crates/gitbutler-watcher/tests/handler/push_project_to_gitbutler.rs @@ -200,6 +200,7 @@ fn create_initial_commit(repo: &git::Repository) -> git::Oid { "initial commit", &repo.find_tree(oid).unwrap(), &[], + None, ) .unwrap() } @@ -224,6 +225,7 @@ fn create_test_commits(repo: &git::Repository, commits: usize) -> git::Oid { &[&repo .find_commit(repo.refname_to_id("HEAD").unwrap()) .unwrap()], + None, ) .unwrap(), ); From 51f9769024e4446ceb7b1509beda574f8b235ffc Mon Sep 17 00:00:00 2001 From: Scott Chacon Date: Tue, 7 May 2024 15:15:25 +0200 Subject: [PATCH 02/13] sign commits with real git in the strangest possible way --- Cargo.lock | 1 + crates/gitbutler-core/src/git/repository.rs | 23 ++++ .../src/project_repository/repository.rs | 15 +-- .../src/virtual_branches/base.rs | 3 - .../src/virtual_branches/controller.rs | 110 ++-------------- .../src/virtual_branches/virtual.rs | 22 +--- .../tests/suite/virtual_branches/init.rs | 3 +- .../tests/suite/virtual_branches/mod.rs | 5 +- .../tests/virtual_branches/mod.rs | 34 +---- crates/gitbutler-git/Cargo.toml | 1 + crates/gitbutler-git/src/lib.rs | 2 +- crates/gitbutler-git/src/repository.rs | 120 +++++++++++++++++- crates/gitbutler-tauri/src/main.rs | 1 - crates/gitbutler-watcher/tests/handler/mod.rs | 1 - 14 files changed, 168 insertions(+), 173 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 442ee0c7e..92d1ee6c0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2190,6 +2190,7 @@ dependencies = [ "sysinfo", "thiserror", "tokio", + "uuid", "winapi 0.3.9", "windows-named-pipe", ] diff --git a/crates/gitbutler-core/src/git/repository.rs b/crates/gitbutler-core/src/git/repository.rs index 492aa351f..26e3f2a2e 100644 --- a/crates/gitbutler-core/src/git/repository.rs +++ b/crates/gitbutler-core/src/git/repository.rs @@ -274,6 +274,29 @@ impl Repository { .write(git2::ObjectType::Commit, commit_buffer.as_bytes())?; // check git config for gpg.signingkey + let signing_key = self.0.config()?.get_string("commit.gpgSign"); + dbg!(&signing_key); + if let Ok(_should_sign) = signing_key { + dbg!("SIGN IT"); + /* + SOMETHING SOMETHING DARK SIDE... + let path = self.path().to_path_buf(); + let res = std::thread::spawn(move || { + tokio::runtime::Runtime::new() + .unwrap() + .block_on(gitbutler_git::sign_commit( + path, + gitbutler_git::tokio::TokioExecutor, + oid.to_string(), + handle_git_prompt_sign, + extra, + )) + }) + .join() + .unwrap() + .map_err(|e| Err(anyhow::anyhow!("fuck you"))); + */ + } // update reference if let Some(refname) = update_ref { diff --git a/crates/gitbutler-core/src/project_repository/repository.rs b/crates/gitbutler-core/src/project_repository/repository.rs index fe8695b57..1e014878d 100644 --- a/crates/gitbutler-core/src/project_repository/repository.rs +++ b/crates/gitbutler-core/src/project_repository/repository.rs @@ -13,7 +13,6 @@ use crate::{ askpass::AskpassBroker, error, git::{self, credentials::HelpError, Url}, - keys, projects::{self, AuthKey}, ssh, users, virtual_branches::{Branch, BranchId}, @@ -344,20 +343,12 @@ impl Repository { message: &str, tree: &git::Tree, parents: &[&git::Commit], - signing_key: Option<&keys::PrivateKey>, change_id: Option<&str>, ) -> Result { let (author, committer) = self.git_signatures(user)?; - - if let Some(key) = signing_key { - self.git_repository - .commit_signed(&author, message, tree, parents, key, change_id) - .context("failed to commit signed") - } else { - self.git_repository - .commit(None, &author, &committer, message, tree, parents, change_id) - .context("failed to commit") - } + self.git_repository + .commit(None, &author, &committer, message, tree, parents, change_id) + .context("failed to commit") } pub fn push_to_gitbutler_server( diff --git a/crates/gitbutler-core/src/virtual_branches/base.rs b/crates/gitbutler-core/src/virtual_branches/base.rs index 7664e977a..806e95831 100644 --- a/crates/gitbutler-core/src/virtual_branches/base.rs +++ b/crates/gitbutler-core/src/virtual_branches/base.rs @@ -12,7 +12,6 @@ use super::{ }; use crate::{ git::{self, diff}, - keys, project_repository::{self, LogUntil}, projects::FetchResult, users, @@ -342,7 +341,6 @@ fn _print_tree(repo: &git2::Repository, tree: &git2::Tree) -> Result<()> { pub fn update_base_branch( project_repository: &project_repository::Repository, user: Option<&users::User>, - signing_key: Option<&keys::PrivateKey>, ) -> Result<(), errors::UpdateBaseBranchError> { if project_repository.is_resolving() { return Err(errors::UpdateBaseBranchError::Conflict( @@ -513,7 +511,6 @@ pub fn update_base_branch( .as_str(), &branch_head_merge_tree, &[&branch_head_commit, &new_target_commit], - signing_key, None, ) .context("failed to commit merge")?; diff --git a/crates/gitbutler-core/src/virtual_branches/controller.rs b/crates/gitbutler-core/src/virtual_branches/controller.rs index 62c592c46..37eee628a 100644 --- a/crates/gitbutler-core/src/virtual_branches/controller.rs +++ b/crates/gitbutler-core/src/virtual_branches/controller.rs @@ -17,7 +17,7 @@ use super::{ }; use crate::{ askpass::AskpassBroker, - git, keys, project_repository, + git, project_repository, projects::{self, ProjectId}, users, }; @@ -26,7 +26,6 @@ use crate::{ pub struct Controller { projects: projects::Controller, users: users::Controller, - keys: keys::Controller, helper: git::credentials::Helper, by_project_id: Arc>>, @@ -36,7 +35,6 @@ impl Controller { pub fn new( projects: projects::Controller, users: users::Controller, - keys: keys::Controller, helper: git::credentials::Helper, ) -> Self { Self { @@ -44,7 +42,6 @@ impl Controller { projects, users, - keys, helper, } } @@ -54,9 +51,7 @@ impl Controller { .lock() .await .entry(*project_id) - .or_insert_with(|| { - ControllerInner::new(&self.projects, &self.users, &self.keys, &self.helper) - }) + .or_insert_with(|| ControllerInner::new(&self.projects, &self.users, &self.helper)) .clone() } @@ -430,7 +425,6 @@ struct ControllerInner { projects: projects::Controller, users: users::Controller, - keys: keys::Controller, helper: git::credentials::Helper, } @@ -438,14 +432,12 @@ impl ControllerInner { pub fn new( projects: &projects::Controller, users: &users::Controller, - keys: &keys::Controller, helper: &git::credentials::Helper, ) -> Self { Self { semaphore: Arc::new(Semaphore::new(1)), projects: projects.clone(), users: users.clone(), - keys: keys.clone(), helper: helper.clone(), } } @@ -461,23 +453,11 @@ impl ControllerInner { let _permit = self.semaphore.acquire().await; self.with_verify_branch(project_id, |project_repository, user| { - let signing_key = project_repository - .config() - .sign_commits() - .context("failed to get sign commits option")? - .then(|| { - self.keys - .get_or_create() - .context("failed to get private key") - }) - .transpose()?; - let result = super::commit( project_repository, branch_id, message, ownership, - signing_key.as_ref(), user, run_hooks, ) @@ -548,22 +528,8 @@ impl ControllerInner { let _permit = self.semaphore.acquire().await; self.with_verify_branch(project_id, |project_repository, user| { - let signing_key = project_repository - .config() - .sign_commits() - .context("failed to get sign commits option")? - .then(|| { - self.keys - .get_or_create() - .context("failed to get private key") - }) - .transpose()?; - let result = super::create_virtual_branch_from_branch( - project_repository, - branch, - signing_key.as_ref(), - user, - )?; + let result = + super::create_virtual_branch_from_branch(project_repository, branch, user)?; let _ = project_repository .project() .create_snapshot(SnapshotDetails::new(OperationType::CreateBranch)); @@ -624,24 +590,8 @@ impl ControllerInner { let _permit = self.semaphore.acquire().await; self.with_verify_branch(project_id, |project_repository, user| { - let signing_key = project_repository - .config() - .sign_commits() - .context("failed to get sign commits option")? - .then(|| { - self.keys - .get_or_create() - .context("failed to get private key") - }) - .transpose()?; - - let result = super::merge_virtual_branch_upstream( - project_repository, - branch_id, - signing_key.as_ref(), - user, - ) - .map_err(Into::into); + let result = super::merge_virtual_branch_upstream(project_repository, branch_id, user) + .map_err(Into::into); let _ = project_repository .project() .create_snapshot(SnapshotDetails::new(OperationType::MergeUpstream)); @@ -653,19 +603,7 @@ impl ControllerInner { let _permit = self.semaphore.acquire().await; self.with_verify_branch(project_id, |project_repository, user| { - let signing_key = project_repository - .config() - .sign_commits() - .context("failed to get sign commits option")? - .then(|| { - self.keys - .get_or_create() - .context("failed to get private key") - }) - .transpose()?; - - let result = super::update_base_branch(project_repository, user, signing_key.as_ref()) - .map_err(Into::into); + let result = super::update_base_branch(project_repository, user).map_err(Into::into); let _ = project_repository .project() .create_snapshot(SnapshotDetails::new(OperationType::UpdateWorkspaceBase)); @@ -726,20 +664,8 @@ impl ControllerInner { let _permit = self.semaphore.acquire().await; self.with_verify_branch(project_id, |project_repository, user| { - let signing_key = project_repository - .config() - .sign_commits() - .context("failed to get sign commits option")? - .then(|| { - self.keys - .get_or_create() - .context("failed to get private key") - }) - .transpose()?; - let result = - super::apply_branch(project_repository, branch_id, signing_key.as_ref(), user) - .map_err(Into::into); + super::apply_branch(project_repository, branch_id, user).map_err(Into::into); let _ = project_repository .project() .create_snapshot(SnapshotDetails::new(OperationType::ApplyBranch)); @@ -1083,24 +1009,8 @@ impl ControllerInner { let _permit = self.semaphore.acquire().await; self.with_verify_branch(project_id, |project_repository, user| { - let signing_key = project_repository - .config() - .sign_commits() - .context("failed to get sign commits option")? - .then(|| { - self.keys - .get_or_create() - .context("failed to get private key") - }) - .transpose()?; - let result = super::move_commit( - project_repository, - target_branch_id, - commit_oid, - user, - signing_key.as_ref(), - ) - .map_err(Into::into); + let result = super::move_commit(project_repository, target_branch_id, commit_oid, user) + .map_err(Into::into); let _ = project_repository .project() .create_snapshot(SnapshotDetails::new(OperationType::MoveCommit)); diff --git a/crates/gitbutler-core/src/virtual_branches/virtual.rs b/crates/gitbutler-core/src/virtual_branches/virtual.rs index 8751f9568..f3d150001 100644 --- a/crates/gitbutler-core/src/virtual_branches/virtual.rs +++ b/crates/gitbutler-core/src/virtual_branches/virtual.rs @@ -34,7 +34,6 @@ use crate::{ diff::{self}, Commit, Refname, RemoteRefname, }, - keys, project_repository::{self, conflicts, LogUntil}, reader, users, }; @@ -101,6 +100,7 @@ pub struct VirtualBranchCommit { pub branch_id: BranchId, pub change_id: Option, pub is_signed: bool, + pub stack_points: Option>>, } // this struct is a mapping to the view `File` type in Typescript @@ -215,7 +215,6 @@ pub fn normalize_branch_name(name: &str) -> String { pub fn apply_branch( project_repository: &project_repository::Repository, branch_id: &BranchId, - signing_key: Option<&keys::PrivateKey>, user: Option<&users::User>, ) -> Result<(), errors::ApplyBranchError> { if project_repository.is_resolving() { @@ -359,7 +358,6 @@ pub fn apply_branch( .as_str(), &merged_branch_tree, &[&head_commit, &target_commit], - signing_key, None, )?; @@ -429,7 +427,6 @@ pub fn apply_branch( .as_str(), &merge_tree, &[&head_commit, &target_commit], - signing_key, None, ) .context("failed to commit merge")?; @@ -1053,6 +1050,7 @@ fn commit_to_vbranch_commit( branch_id: branch.id, change_id: commit.change_id(), is_signed: commit.is_signed(), + stack_points: Some(stack_points), }; Ok(commit) @@ -1171,7 +1169,6 @@ pub fn create_virtual_branch( pub fn merge_virtual_branch_upstream( project_repository: &project_repository::Repository, branch_id: &BranchId, - signing_key: Option<&keys::PrivateKey>, user: Option<&users::User>, ) -> Result<(), errors::MergeVirtualBranchUpstreamError> { if conflicts::is_conflicting::<&Path>(project_repository, None)? { @@ -1364,7 +1361,6 @@ pub fn merge_virtual_branch_upstream( .as_str(), &merge_tree, &[&head_commit, &upstream_commit], - signing_key, None, )?; @@ -2312,7 +2308,6 @@ pub fn commit( branch_id: &BranchId, message: &str, ownership: Option<&branch::BranchOwnershipClaims>, - signing_key: Option<&keys::PrivateKey>, user: Option<&users::User>, run_hooks: bool, ) -> Result { @@ -2415,15 +2410,12 @@ pub fn commit( message, &tree, &[&parent_commit, &merge_parent], - signing_key, None, )?; conflicts::clear(project_repository).context("failed to clear conflicts")?; commit_oid } - None => { - project_repository.commit(user, message, &tree, &[&parent_commit], signing_key, None)? - } + None => project_repository.commit(user, message, &tree, &[&parent_commit], None)?, }; if run_hooks { @@ -3340,8 +3332,7 @@ pub fn insert_blank_commit( } let commit_tree = commit.tree().unwrap(); - let blank_commit_oid = - project_repository.commit(user, "", &commit_tree, &[&commit], None, None)?; + let blank_commit_oid = project_repository.commit(user, "", &commit_tree, &[&commit], None)?; if commit.id() == branch.head && offset < 0 { // inserting before the first commit @@ -3983,7 +3974,6 @@ pub fn move_commit( target_branch_id: &BranchId, commit_oid: git::Oid, user: Option<&users::User>, - signing_key: Option<&keys::PrivateKey>, ) -> Result<(), errors::MoveCommitError> { if project_repository.is_resolving() { return Err(errors::MoveCommitError::Conflicted( @@ -4140,7 +4130,6 @@ pub fn move_commit( .git_repository .find_commit(destination_branch.head) .context("failed to get dst branch head commit")?], - signing_key, change_id.as_deref(), ) .context("failed to commit")?; @@ -4158,7 +4147,6 @@ pub fn move_commit( pub fn create_virtual_branch_from_branch( project_repository: &project_repository::Repository, upstream: &git::Refname, - signing_key: Option<&keys::PrivateKey>, user: Option<&users::User>, ) -> Result { if !matches!(upstream, git::Refname::Local(_) | git::Refname::Remote(_)) { @@ -4297,7 +4285,7 @@ pub fn create_virtual_branch_from_branch( project_repository.add_branch_reference(&branch)?; - match apply_branch(project_repository, &branch.id, signing_key, user) { + match apply_branch(project_repository, &branch.id, user) { Ok(()) => Ok(branch.id), Err(errors::ApplyBranchError::BranchConflicts(_)) => { // if branch conflicts with the workspace, it's ok. keep it unapplied diff --git a/crates/gitbutler-core/tests/suite/virtual_branches/init.rs b/crates/gitbutler-core/tests/suite/virtual_branches/init.rs index ba39156fc..1be497e78 100644 --- a/crates/gitbutler-core/tests/suite/virtual_branches/init.rs +++ b/crates/gitbutler-core/tests/suite/virtual_branches/init.rs @@ -3,14 +3,13 @@ use super::*; #[tokio::test] async fn twice() { let data_dir = paths::data_dir(); - let keys = keys::Controller::from_path(data_dir.path()); let projects = projects::Controller::from_path(data_dir.path()); let users = users::Controller::from_path(data_dir.path()); let helper = git::credentials::Helper::from_path(data_dir.path()); let test_project = TestProject::default(); - let controller = Controller::new(projects.clone(), users, keys, helper); + let controller = Controller::new(projects.clone(), users, helper); { let project = projects diff --git a/crates/gitbutler-core/tests/suite/virtual_branches/mod.rs b/crates/gitbutler-core/tests/suite/virtual_branches/mod.rs index cd211e436..90fa8ea14 100644 --- a/crates/gitbutler-core/tests/suite/virtual_branches/mod.rs +++ b/crates/gitbutler-core/tests/suite/virtual_branches/mod.rs @@ -1,7 +1,7 @@ use std::{fs, path, str::FromStr}; use gitbutler_core::{ - git, keys, + git, projects::{self, ProjectId}, users, virtual_branches::{branch, errors, Controller}, @@ -29,7 +29,6 @@ impl Drop for Test { impl Default for Test { fn default() -> Self { let data_dir = paths::data_dir(); - let keys = keys::Controller::from_path(data_dir.path()); let projects = projects::Controller::from_path(data_dir.path()); let users = users::Controller::from_path(data_dir.path()); let helper = git::credentials::Helper::from_path(data_dir.path()); @@ -42,7 +41,7 @@ impl Default for Test { Self { repository: test_project, project_id: project.id, - controller: Controller::new(projects.clone(), users, keys, helper), + controller: Controller::new(projects.clone(), users, helper), projects, data_dir: Some(data_dir), } diff --git a/crates/gitbutler-core/tests/virtual_branches/mod.rs b/crates/gitbutler-core/tests/virtual_branches/mod.rs index f73356d6f..190e837ef 100644 --- a/crates/gitbutler-core/tests/virtual_branches/mod.rs +++ b/crates/gitbutler-core/tests/virtual_branches/mod.rs @@ -64,7 +64,6 @@ fn commit_on_branch_then_change_file_then_get_status() -> Result<()> { "test commit", None, None, - None, false, )?; @@ -123,7 +122,6 @@ fn signed_commit() -> Result<()> { &branch1_id, "test commit", None, - Some(suite.keys.get_or_create()?).as_ref(), None, false, )?; @@ -212,7 +210,6 @@ fn track_binary_files() -> Result<()> { "test commit", None, None, - None, false, )?; @@ -244,7 +241,6 @@ fn track_binary_files() -> Result<()> { "test commit", None, None, - None, false, )?; @@ -811,12 +807,7 @@ fn merge_vbranch_upstream_clean_rebase() -> Result<()> { assert_eq!(branch1.commits.len(), 1); // assert_eq!(branch1.upstream.as_ref().unwrap().commits.len(), 1); - merge_virtual_branch_upstream( - project_repository, - &branch1.id, - Some(suite.keys.get_or_create()?).as_ref(), - None, - )?; + merge_virtual_branch_upstream(project_repository, &branch1.id, None)?; let (branches, _) = virtual_branches::list_virtual_branches(project_repository)?; let branch1 = &branches[0]; @@ -931,7 +922,7 @@ fn merge_vbranch_upstream_conflict() -> Result<()> { assert_eq!(branch1.commits.len(), 1); // assert_eq!(branch1.upstream.as_ref().unwrap().commits.len(), 1); - merge_virtual_branch_upstream(project_repository, &branch1.id, None, None)?; + merge_virtual_branch_upstream(project_repository, &branch1.id, None)?; let (branches, _) = virtual_branches::list_virtual_branches(project_repository)?; let branch1 = &branches[0]; @@ -963,7 +954,6 @@ fn merge_vbranch_upstream_conflict() -> Result<()> { "fix merge conflict", None, None, - None, false, )?; @@ -1095,7 +1085,7 @@ fn unapply_branch() -> Result<()> { assert_eq!(branch.files.len(), 1); assert!(!branch.active); - apply_branch(project_repository, &branch1_id, None, None)?; + apply_branch(project_repository, &branch1_id, None)?; let contents = std::fs::read(Path::new(&project.path).join(file_path))?; assert_eq!( "line1\nline2\nline3\nline4\nbranch1\n", @@ -1168,11 +1158,11 @@ fn apply_unapply_added_deleted_files() -> Result<()> { // check that file3 is gone assert!(!Path::new(&project.path).join(file_path3).exists()); - apply_branch(project_repository, &branch2_id, None, None)?; + apply_branch(project_repository, &branch2_id, None)?; // check that file2 is gone assert!(!Path::new(&project.path).join(file_path2).exists()); - apply_branch(project_repository, &branch3_id, None, None)?; + apply_branch(project_repository, &branch3_id, None)?; // check that file3 is back let contents = std::fs::read(Path::new(&project.path).join(file_path3))?; assert_eq!("file3\n", String::from_utf8(contents)?); @@ -1461,7 +1451,6 @@ fn upstream_integrated_vbranch() -> Result<()> { "integrated commit", None, None, - None, false, )?; commit( @@ -1470,7 +1459,6 @@ fn upstream_integrated_vbranch() -> Result<()> { "non-integrated commit", None, None, - None, false, )?; @@ -1531,7 +1519,6 @@ fn commit_same_hunk_twice() -> Result<()> { "first commit to test.txt", None, None, - None, false, )?; @@ -1567,7 +1554,6 @@ fn commit_same_hunk_twice() -> Result<()> { "second commit to test.txt", None, None, - None, false, )?; @@ -1626,7 +1612,6 @@ fn commit_same_file_twice() -> Result<()> { "first commit to test.txt", None, None, - None, false, )?; @@ -1662,7 +1647,6 @@ fn commit_same_file_twice() -> Result<()> { "second commit to test.txt", None, None, - None, false, )?; @@ -1721,7 +1705,6 @@ fn commit_partial_by_hunk() -> Result<()> { "first commit to test.txt", Some(&"test.txt:1-6".parse::().unwrap()), None, - None, false, )?; @@ -1740,7 +1723,6 @@ fn commit_partial_by_hunk() -> Result<()> { "second commit to test.txt", Some(&"test.txt:16-22".parse::().unwrap()), None, - None, false, )?; @@ -1799,7 +1781,6 @@ fn commit_partial_by_file() -> Result<()> { "branch1 commit", None, None, - None, false, )?; @@ -1867,7 +1848,6 @@ fn commit_add_and_delete_files() -> Result<()> { "branch1 commit", None, None, - None, false, )?; @@ -1933,7 +1913,6 @@ fn commit_executable_and_symlinks() -> Result<()> { "branch1 commit", None, None, - None, false, )?; @@ -2110,7 +2089,6 @@ fn pre_commit_hook_rejection() -> Result<()> { &branch1_id, "test commit", None, - Some(suite.keys.get_or_create()?).as_ref(), None, true, ); @@ -2175,7 +2153,6 @@ fn post_commit_hook() -> Result<()> { &branch1_id, "test commit", None, - Some(suite.keys.get_or_create()?).as_ref(), None, true, )?; @@ -2224,7 +2201,6 @@ fn commit_msg_hook_rejection() -> Result<()> { &branch1_id, "test commit", None, - Some(suite.keys.get_or_create()?).as_ref(), None, true, ); diff --git a/crates/gitbutler-git/Cargo.toml b/crates/gitbutler-git/Cargo.toml index 2c8e384c0..d124aaf6b 100644 --- a/crates/gitbutler-git/Cargo.toml +++ b/crates/gitbutler-git/Cargo.toml @@ -29,6 +29,7 @@ test-askpass-path = [] thiserror.workspace = true serde = { workspace = true, optional = true } tokio = { workspace = true, optional = true, features = ["process", "time", "io-util", "net", "fs"] } +uuid.workspace = true rand = "0.8.5" futures = "0.3.30" sysinfo = "0.30.11" diff --git a/crates/gitbutler-git/src/lib.rs b/crates/gitbutler-git/src/lib.rs index ef34cc903..847fc302b 100644 --- a/crates/gitbutler-git/src/lib.rs +++ b/crates/gitbutler-git/src/lib.rs @@ -24,5 +24,5 @@ pub use self::executor::tokio; pub use self::{ error::Error, refspec::{Error as RefSpecError, RefSpec}, - repository::{fetch, push}, + repository::{fetch, push, sign_commit}, }; diff --git a/crates/gitbutler-git/src/repository.rs b/crates/gitbutler-git/src/repository.rs index 288331e52..eacd1b3d6 100644 --- a/crates/gitbutler-git/src/repository.rs +++ b/crates/gitbutler-git/src/repository.rs @@ -59,7 +59,7 @@ pub type Error = RepositoryError< #[cold] async fn execute_with_auth_harness( repo_path: P, - executor: E, + executor: &E, args: &[&str], envs: Option>, mut on_prompt: F, @@ -155,7 +155,7 @@ where .or_else(|| std::env::var("GIT_SSH").ok()) { Some(v) => v, - None => get_core_sshcommand(&executor, &repo_path) + None => get_core_sshcommand(executor, &repo_path) .await .unwrap_or_else(|| "ssh".into()), }; @@ -300,7 +300,7 @@ where args.push(&refspec); let (status, stdout, stderr) = - execute_with_auth_harness(repo_path, executor, &args, None, on_prompt, extra).await?; + execute_with_auth_harness(repo_path, &executor, &args, None, on_prompt, extra).await?; if status == 0 { Ok(()) @@ -362,7 +362,7 @@ where } let (status, stdout, stderr) = - execute_with_auth_harness(repo_path, executor, &args, None, on_prompt, extra).await?; + execute_with_auth_harness(repo_path, &executor, &args, None, on_prompt, extra).await?; if status == 0 { Ok(()) @@ -392,6 +392,118 @@ where } } +/// Signs the given commit-ish in the repository at the given path. +/// Returns the newly signed commit SHA. +/// +/// Any prompts for the user are passed to the asynchronous callback `on_prompt`, +/// which should return the user's response or `None` if the operation should be +/// aborted, in which case an `Err` value is returned from this function. +pub async fn sign_commit( + repo_path: P, + executor: E, + base_commitish: String, + on_prompt: F, + extra: Extra, +) -> Result>> +where + P: AsRef, + E: GitExecutor, + F: FnMut(String, Extra) -> Fut, + Fut: std::future::Future>, + Extra: Send + Clone, +{ + let repo_path = repo_path.as_ref(); + + // First, create a worktree to perform the commit. + let worktree_path = repo_path + .join(".git") + .join("gitbutler") + .join(".wt") + .join(uuid::Uuid::new_v4().to_string()); + let args = [ + "worktree", + "add", + "--detach", + "--no-checkout", + worktree_path.to_str().unwrap(), + base_commitish.as_str(), + ]; + let (status, stdout, stderr) = executor + .execute(&args, repo_path, None) + .await + .map_err(Error::::Exec)?; + if status != 0 { + return Err(Error::::Failed { + status, + args: args.into_iter().map(Into::into).collect(), + stdout, + stderr, + })?; + } + + // Now, perform the commit. + let args = [ + "commit", + "--amend", + "-S", + "-o", + "--no-edit", + "--no-verify", + "--no-post-rewrite", + "--allow-empty", + "--allow-empty-message", + ]; + let (status, stdout, stderr) = + execute_with_auth_harness(&worktree_path, &executor, &args, None, on_prompt, extra).await?; + if status != 0 { + return Err(Error::::Failed { + status, + args: args.into_iter().map(Into::into).collect(), + stdout, + stderr, + })?; + } + + // Get the commit hash that was generated + let args = ["rev-parse", "--verify", "HEAD"]; + let (status, stdout, stderr) = executor + .execute(&args, &worktree_path, None) + .await + .map_err(Error::::Exec)?; + if status != 0 { + return Err(Error::::Failed { + status, + args: args.into_iter().map(Into::into).collect(), + stdout, + stderr, + })?; + } + + let commit_hash = stdout.trim().to_string(); + + // Finally, remove the worktree + let args = [ + "worktree", + "remove", + "--force", + worktree_path.to_str().unwrap(), + ]; + let (status, stdout, stderr) = executor + .execute(&args, repo_path, None) + .await + .map_err(Error::::Exec)?; + if status != 0 { + return Err(Error::::Failed { + status, + args: args.into_iter().map(Into::into).collect(), + stdout, + stderr, + })?; + } + + Ok(commit_hash) +} + async fn get_core_sshcommand>( executor: &E, cwd: P, diff --git a/crates/gitbutler-tauri/src/main.rs b/crates/gitbutler-tauri/src/main.rs index 18258c2f3..ca981ca13 100644 --- a/crates/gitbutler-tauri/src/main.rs +++ b/crates/gitbutler-tauri/src/main.rs @@ -140,7 +140,6 @@ fn main() { app_handle.manage(gitbutler_core::virtual_branches::controller::Controller::new( projects_controller.clone(), users_controller.clone(), - keys_controller.clone(), git_credentials_controller.clone(), )); diff --git a/crates/gitbutler-watcher/tests/handler/mod.rs b/crates/gitbutler-watcher/tests/handler/mod.rs index 0dd6b9c12..727ee1c51 100644 --- a/crates/gitbutler-watcher/tests/handler/mod.rs +++ b/crates/gitbutler-watcher/tests/handler/mod.rs @@ -37,7 +37,6 @@ mod support { let vbranch_controller = virtual_branches::Controller::new( inner.projects.clone(), inner.users.clone(), - inner.keys.clone(), git_credentials_helper, ); let assets_proxy = assets::Proxy::new(tmp.path().to_owned()); From 4eaf7d2b1d44b1373d1713aa7b38b311c82de45e Mon Sep 17 00:00:00 2001 From: Scott Chacon Date: Mon, 13 May 2024 14:19:59 +0200 Subject: [PATCH 03/13] prettier --- app/src/lib/components/CommitCard.svelte | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/lib/components/CommitCard.svelte b/app/src/lib/components/CommitCard.svelte index 492eba2ea..d51dc0e34 100644 --- a/app/src/lib/components/CommitCard.svelte +++ b/app/src/lib/components/CommitCard.svelte @@ -147,7 +147,7 @@
{#if commit.isSigned} - 🔒 + 🔒 {/if} {#if commit.changeId} {commit.changeId.split('-')[0]} From b17ab37a01a8c77010332549d1defa56d3675c7a Mon Sep 17 00:00:00 2001 From: Scott Chacon Date: Mon, 13 May 2024 16:19:45 +0200 Subject: [PATCH 04/13] actually run gpg if specified --- crates/gitbutler-core/src/git/repository.rs | 81 +++++++++++++-------- 1 file changed, 50 insertions(+), 31 deletions(-) diff --git a/crates/gitbutler-core/src/git/repository.rs b/crates/gitbutler-core/src/git/repository.rs index 26e3f2a2e..c2c3b4f55 100644 --- a/crates/gitbutler-core/src/git/repository.rs +++ b/crates/gitbutler-core/src/git/repository.rs @@ -1,7 +1,7 @@ -use std::{io::Write, path::Path, str}; - use git2::{BlameOptions, Submodule}; use git2_hooks::HookResult; +use std::process::Stdio; +use std::{io::Write, path::Path, str}; use super::{ Blob, Branch, Commit, Config, Index, Oid, Reference, Refname, Remote, Result, Signature, Tree, @@ -268,35 +268,7 @@ impl Repository { let commit_buffer = Self::inject_change_id(&commit_buffer, change_id)?; - let oid = self - .0 - .odb()? - .write(git2::ObjectType::Commit, commit_buffer.as_bytes())?; - - // check git config for gpg.signingkey - let signing_key = self.0.config()?.get_string("commit.gpgSign"); - dbg!(&signing_key); - if let Ok(_should_sign) = signing_key { - dbg!("SIGN IT"); - /* - SOMETHING SOMETHING DARK SIDE... - let path = self.path().to_path_buf(); - let res = std::thread::spawn(move || { - tokio::runtime::Runtime::new() - .unwrap() - .block_on(gitbutler_git::sign_commit( - path, - gitbutler_git::tokio::TokioExecutor, - oid.to_string(), - handle_git_prompt_sign, - extra, - )) - }) - .join() - .unwrap() - .map_err(|e| Err(anyhow::anyhow!("fuck you"))); - */ - } + let oid = self.commit_buffer(commit_buffer)?; // update reference if let Some(refname) = update_ref { @@ -305,6 +277,53 @@ impl Repository { Ok(oid.into()) } + pub fn commit_buffer(&self, buffer: String) -> Result { + // check git config for gpg.signingkey + let should_sign = self.0.config()?.get_string("commit.gpgSign"); + if let Ok(_should_sign) = should_sign { + let signing_key = self.0.config()?.get_string("user.signingkey"); + if let Ok(signing_key) = signing_key { + dbg!(&signing_key); + let mut cmd = std::process::Command::new("gpg"); + cmd.args(["--status-fd=2", "-bsau", &signing_key]) + //.arg(&signed_storage) + .arg("-") + .stdout(Stdio::piped()) + .stdin(Stdio::piped()); + + let mut child = cmd.spawn()?; + child + .stdin + .take() + .expect("configured") + .write_all(buffer.to_string().as_ref())?; + + let output = child.wait_with_output()?; + if output.status.success() { + // read stdout + let signature = String::from_utf8_lossy(&output.stdout); + dbg!(&signature); + let oid = self + .0 + .commit_signed(&buffer, &signature, None) + .map(Into::into) + .map_err(Into::into); + return oid; + } else { + let stderr = String::from_utf8_lossy(&output.stderr); + dbg!(stderr); + } + } + } + + let oid = self + .0 + .odb()? + .write(git2::ObjectType::Commit, buffer.as_bytes())?; + + Ok(oid) + } + pub fn commit_signed( &self, author: &Signature<'_>, From f106bfa24670beae37be710f38b3a11703dc4134 Mon Sep 17 00:00:00 2001 From: Scott Chacon Date: Tue, 14 May 2024 10:08:22 +0200 Subject: [PATCH 05/13] basic ssh signing too --- crates/gitbutler-core/Cargo.toml | 12 +-- crates/gitbutler-core/src/git/repository.rs | 101 +++++++++++++++----- 2 files changed, 81 insertions(+), 32 deletions(-) diff --git a/crates/gitbutler-core/Cargo.toml b/crates/gitbutler-core/Cargo.toml index 5bdf9720f..49a1eb921 100644 --- a/crates/gitbutler-core/Cargo.toml +++ b/crates/gitbutler-core/Cargo.toml @@ -8,9 +8,8 @@ publish = false [dev-dependencies] once_cell = "1.19" pretty_assertions = "1.4" -tempfile = "3.10" gitbutler-testsupport.workspace = true -gitbutler-git = { workspace = true, features = ["test-askpass-path" ]} +gitbutler-git = { workspace = true, features = ["test-askpass-path"] } [dependencies] toml = "0.8.12" @@ -33,22 +32,23 @@ hex = "0.4.3" r2d2 = "0.8.10" r2d2_sqlite = "0.22.0" rand = "0.8.5" -refinery = { version = "0.8", features = [ "rusqlite" ] } +refinery = { version = "0.8", features = ["rusqlite"] } regex = "1.10" reqwest = { version = "0.12.4", features = ["json"] } resolve-path = "0.1.0" rusqlite.workspace = true serde.workspace = true -serde_json = { version = "1.0", features = [ "std", "arbitrary_precision" ] } +serde_json = { version = "1.0", features = ["std", "arbitrary_precision"] } sha2 = "0.10.8" similar = { version = "2.5.0", features = ["unicode"] } slug = "0.1.5" -ssh-key = { version = "0.6.6", features = [ "alloc", "ed25519" ] } +ssh-key = { version = "0.6.6", features = ["alloc", "ed25519"] } ssh2 = { version = "0.9.4", features = ["vendored-openssl"] } strum = { version = "0.26", features = ["derive"] } log = "^0.4" +tempfile = "3.10" thiserror.workspace = true -tokio = { workspace = true, features = [ "rt-multi-thread", "rt", "macros" ] } +tokio = { workspace = true, features = ["rt-multi-thread", "rt", "macros"] } tracing = "0.1.40" url = { version = "2.5", features = ["serde"] } urlencoding = "2.1.3" diff --git a/crates/gitbutler-core/src/git/repository.rs b/crates/gitbutler-core/src/git/repository.rs index c2c3b4f55..ce546cae0 100644 --- a/crates/gitbutler-core/src/git/repository.rs +++ b/crates/gitbutler-core/src/git/repository.rs @@ -284,34 +284,83 @@ impl Repository { let signing_key = self.0.config()?.get_string("user.signingkey"); if let Ok(signing_key) = signing_key { dbg!(&signing_key); - let mut cmd = std::process::Command::new("gpg"); - cmd.args(["--status-fd=2", "-bsau", &signing_key]) - //.arg(&signed_storage) - .arg("-") - .stdout(Stdio::piped()) - .stdin(Stdio::piped()); - let mut child = cmd.spawn()?; - child - .stdin - .take() - .expect("configured") - .write_all(buffer.to_string().as_ref())?; - - let output = child.wait_with_output()?; - if output.status.success() { - // read stdout - let signature = String::from_utf8_lossy(&output.stdout); - dbg!(&signature); - let oid = self - .0 - .commit_signed(&buffer, &signature, None) - .map(Into::into) - .map_err(Into::into); - return oid; + let sign_format = self.0.config()?.get_string("gpg.format"); + let is_ssh = if let Ok(sign_format) = sign_format { + sign_format == "ssh" } else { - let stderr = String::from_utf8_lossy(&output.stderr); - dbg!(stderr); + false + }; + + // todo: support gpg.program + + if is_ssh { + // is ssh + + // write commit data to a temp file so we can sign it + let mut signature_storage = tempfile::NamedTempFile::new()?; + signature_storage.write_all(buffer.as_ref())?; + let signed_storage = signature_storage.into_temp_path(); + + let mut cmd = std::process::Command::new("ssh-keygen"); + cmd.args(["-Y", "sign", "-n", "git", "-f"]) + .arg(&signing_key) + .arg(&signed_storage) + .stdout(Stdio::piped()); + + // todo: support literal ssh key + // strvec_push(&signer.args, "-U"); + + let child = cmd.spawn()?; + let output = child.wait_with_output()?; + if output.status.success() { + // read signed_storage path plus .sig + let signature_path = signed_storage.with_extension("sig"); + let sig_data = std::fs::read(signature_path)?; + let signature = String::from_utf8_lossy(&sig_data); + dbg!(&signature); + let oid = self + .0 + .commit_signed(&buffer, &signature, None) + .map(Into::into) + .map_err(Into::into); + dbg!(&oid); + return oid; + } else { + let stderr = String::from_utf8_lossy(&output.stderr); + dbg!(stderr); + } + } else { + // is gpg + let mut cmd = std::process::Command::new("gpg"); + cmd.args(["--status-fd=2", "-bsau", &signing_key]) + //.arg(&signed_storage) + .arg("-") + .stdout(Stdio::piped()) + .stdin(Stdio::piped()); + + let mut child = cmd.spawn()?; + child + .stdin + .take() + .expect("configured") + .write_all(buffer.to_string().as_ref())?; + + let output = child.wait_with_output()?; + if output.status.success() { + // read stdout + let signature = String::from_utf8_lossy(&output.stdout); + dbg!(&signature); + let oid = self + .0 + .commit_signed(&buffer, &signature, None) + .map(Into::into) + .map_err(Into::into); + return oid; + } else { + let stderr = String::from_utf8_lossy(&output.stderr); + dbg!(stderr); + } } } } From b5c9f934d53afc86f7a4eeea5eedac6ae90d6c6e Mon Sep 17 00:00:00 2001 From: Scott Chacon Date: Tue, 14 May 2024 11:57:23 +0200 Subject: [PATCH 06/13] supporting gpg.ssh.program --- crates/gitbutler-core/src/git/repository.rs | 109 ++++++++++---------- 1 file changed, 56 insertions(+), 53 deletions(-) diff --git a/crates/gitbutler-core/src/git/repository.rs b/crates/gitbutler-core/src/git/repository.rs index ce546cae0..3cdb232d0 100644 --- a/crates/gitbutler-core/src/git/repository.rs +++ b/crates/gitbutler-core/src/git/repository.rs @@ -1,13 +1,13 @@ -use git2::{BlameOptions, Submodule}; -use git2_hooks::HookResult; -use std::process::Stdio; -use std::{io::Write, path::Path, str}; - use super::{ Blob, Branch, Commit, Config, Index, Oid, Reference, Refname, Remote, Result, Signature, Tree, TreeBuilder, Url, }; -use crate::{keys, path::Normalize}; +use crate::path::Normalize; +use git2::{BlameOptions, Submodule}; +use git2_hooks::HookResult; +use std::os::unix::fs::PermissionsExt; +use std::process::Stdio; +use std::{io::Write, path::Path, str}; // wrapper around git2::Repository to get control over how it's used. pub struct Repository(git2::Repository); @@ -283,8 +283,6 @@ impl Repository { if let Ok(_should_sign) = should_sign { let signing_key = self.0.config()?.get_string("user.signingkey"); if let Ok(signing_key) = signing_key { - dbg!(&signing_key); - let sign_format = self.0.config()?.get_string("gpg.format"); let is_ssh = if let Ok(sign_format) = sign_format { sign_format == "ssh" @@ -292,33 +290,56 @@ impl Repository { false }; - // todo: support gpg.program - if is_ssh { - // is ssh - // write commit data to a temp file so we can sign it let mut signature_storage = tempfile::NamedTempFile::new()?; signature_storage.write_all(buffer.as_ref())?; - let signed_storage = signature_storage.into_temp_path(); + let buffer_file_to_sign_path = signature_storage.into_temp_path(); - let mut cmd = std::process::Command::new("ssh-keygen"); - cmd.args(["-Y", "sign", "-n", "git", "-f"]) - .arg(&signing_key) - .arg(&signed_storage) - .stdout(Stdio::piped()); + let gpg_program = self.0.config()?.get_string("gpg.ssh.program"); + let mut cmd = + std::process::Command::new(gpg_program.unwrap_or("ssh-keygen".to_string())); + cmd.args(["-Y", "sign", "-n", "git", "-f"]); - // todo: support literal ssh key - // strvec_push(&signer.args, "-U"); + let output; + // support literal ssh key + if let (true, signing_key) = Self::is_literal_ssh_key(&signing_key) { + dbg!(&signing_key); + + // write the key to a temp file + let mut key_storage = tempfile::NamedTempFile::new()?; + key_storage.write_all(signing_key.as_bytes())?; + + // make sure the tempfile permissions are acceptable for a private ssh key + let mut permissions = key_storage.as_file().metadata()?.permissions(); + permissions.set_mode(0o600); + key_storage.as_file().set_permissions(permissions)?; + + let key_file_path = key_storage.into_temp_path(); + let key_storage = std::fs::read_to_string(&key_file_path)?; + dbg!(&key_storage); + + cmd.arg(&key_file_path); + cmd.arg("-U"); + cmd.arg(&buffer_file_to_sign_path); + cmd.stdout(Stdio::piped()); + + let child = cmd.spawn()?; + output = child.wait_with_output()?; + } else { + cmd.arg(signing_key); + cmd.arg(&buffer_file_to_sign_path); + cmd.stdout(Stdio::piped()); + + let child = cmd.spawn()?; + output = child.wait_with_output()?; + } - let child = cmd.spawn()?; - let output = child.wait_with_output()?; if output.status.success() { // read signed_storage path plus .sig - let signature_path = signed_storage.with_extension("sig"); + let signature_path = buffer_file_to_sign_path.with_extension("sig"); let sig_data = std::fs::read(signature_path)?; let signature = String::from_utf8_lossy(&sig_data); - dbg!(&signature); let oid = self .0 .commit_signed(&buffer, &signature, None) @@ -332,7 +353,9 @@ impl Repository { } } else { // is gpg - let mut cmd = std::process::Command::new("gpg"); + let gpg_program = self.0.config()?.get_string("gpg.program"); + let mut cmd = + std::process::Command::new(gpg_program.unwrap_or("gpg".to_string())); cmd.args(["--status-fd=2", "-bsau", &signing_key]) //.arg(&signed_storage) .arg("-") @@ -373,34 +396,14 @@ impl Repository { Ok(oid) } - pub fn commit_signed( - &self, - author: &Signature<'_>, - message: &str, - tree: &Tree<'_>, - parents: &[&Commit<'_>], - key: &keys::PrivateKey, - change_id: Option<&str>, - ) -> Result { - let parents: Vec<&git2::Commit> = parents - .iter() - .map(|c| c.to_owned().into()) - .collect::>(); - let commit_buffer = self.0.commit_create_buffer( - author.into(), - // author and committer must be the same - // for signed commits - author.into(), - message, - tree.into(), - &parents, - )?; - let commit_buffer = Self::inject_change_id(&commit_buffer, change_id)?; - let signature = key.sign(commit_buffer.as_bytes())?; - self.0 - .commit_signed(&commit_buffer, &signature, None) - .map(Into::into) - .map_err(Into::into) + fn is_literal_ssh_key(string: &str) -> (bool, &str) { + if let Some(key) = string.strip_prefix("key::") { + return (true, key); + } + if string.starts_with("ssh-") { + return (true, string); + } + (false, string) } // in commit_buffer, inject a line right before the first `\n\n` that we see: From 202c551bc7f99f0eb78fb3a29f3e23117f262d56 Mon Sep 17 00:00:00 2001 From: Scott Chacon Date: Tue, 14 May 2024 13:15:11 +0200 Subject: [PATCH 07/13] remove signing setting --- app/src/routes/settings/git/+page.svelte | 25 ---------- crates/gitbutler-core/src/git/repository.rs | 1 + .../tests/virtual_branches/mod.rs | 48 ------------------- 3 files changed, 1 insertion(+), 73 deletions(-) diff --git a/app/src/routes/settings/git/+page.svelte b/app/src/routes/settings/git/+page.svelte index 270a81229..881ec8088 100644 --- a/app/src/routes/settings/git/+page.svelte +++ b/app/src/routes/settings/git/+page.svelte @@ -16,7 +16,6 @@ const gitConfig = getContext(GitConfigService); const authService = getContext(AuthService); - let signCommits = false; let annotateCommits = true; let sshKey = ''; @@ -25,15 +24,9 @@ gitConfig.set('gitbutler.gitbutlerCommitter', annotateCommits ? '1' : '0'); } - function toggleSigningSetting() { - signCommits = !signCommits; - gitConfig.set('gitbutler.signCommits', signCommits ? 'true' : 'false'); - } - onMount(async () => { sshKey = await authService.getPublicKey(); annotateCommits = (await gitConfig.get('gitbutler.gitbutlerCommitter')) == '1'; - signCommits = (await gitConfig.get('gitbutler.signCommits')) == 'true'; }); @@ -82,22 +75,4 @@
- - - Sign commits with the above SSH key - - If you want GitButler to sign your commits with the SSH key we generated, then you can add - that key to GitHub as a signing key to have those commits verified. - - Learn more - - - - - - diff --git a/crates/gitbutler-core/src/git/repository.rs b/crates/gitbutler-core/src/git/repository.rs index 3cdb232d0..030ecc95b 100644 --- a/crates/gitbutler-core/src/git/repository.rs +++ b/crates/gitbutler-core/src/git/repository.rs @@ -281,6 +281,7 @@ impl Repository { // check git config for gpg.signingkey let should_sign = self.0.config()?.get_string("commit.gpgSign"); if let Ok(_should_sign) = should_sign { + // TODO: support gpg.ssh.defaultKeyCommand to get the signing key if this value doesn't exist let signing_key = self.0.config()?.get_string("user.signingkey"); if let Ok(signing_key) = signing_key { let sign_format = self.0.config()?.get_string("gpg.format"); diff --git a/crates/gitbutler-core/tests/virtual_branches/mod.rs b/crates/gitbutler-core/tests/virtual_branches/mod.rs index 9a234e284..5335fc1ee 100644 --- a/crates/gitbutler-core/tests/virtual_branches/mod.rs +++ b/crates/gitbutler-core/tests/virtual_branches/mod.rs @@ -87,54 +87,6 @@ fn commit_on_branch_then_change_file_then_get_status() -> Result<()> { Ok(()) } -#[test] -fn signed_commit() -> Result<()> { - let suite = Suite::default(); - let Case { - project, - project_repository, - .. - } = &suite.new_case_with_files(HashMap::from([ - (PathBuf::from("test.txt"), "line1\nline2\nline3\nline4\n"), - (PathBuf::from("test2.txt"), "line5\nline6\nline7\nline8\n"), - ])); - - set_test_target(project_repository)?; - - let branch1_id = create_virtual_branch(project_repository, &BranchCreateRequest::default()) - .expect("failed to create virtual branch") - .id; - - std::fs::write( - Path::new(&project.path).join("test.txt"), - "line0\nline1\nline2\nline3\nline4\n", - )?; - - let mut config = project_repository - .git_repository - .config() - .with_context(|| "failed to get config")?; - config.set_str("gitbutler.signCommits", "true")?; - - // commit - commit( - project_repository, - &branch1_id, - "test commit", - None, - None, - false, - )?; - - let (branches, _) = virtual_branches::list_virtual_branches(project_repository).unwrap(); - let commit_id = &branches[0].commits[0].id; - let commit_obj = project_repository.git_repository.find_commit(*commit_id)?; - // check the raw_header contains the string "SSH SIGNATURE" - assert!(commit_obj.raw_header().unwrap().contains("SSH SIGNATURE")); - - Ok(()) -} - #[test] fn track_binary_files() -> Result<()> { let suite = Suite::default(); From 4cde9ce71d7517c6eac5b4664c37109e22c04166 Mon Sep 17 00:00:00 2001 From: Scott Chacon Date: Tue, 14 May 2024 13:43:49 +0200 Subject: [PATCH 08/13] windows doesn't have permissions like this --- crates/gitbutler-core/src/git/repository.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/crates/gitbutler-core/src/git/repository.rs b/crates/gitbutler-core/src/git/repository.rs index 030ecc95b..ce94fafa4 100644 --- a/crates/gitbutler-core/src/git/repository.rs +++ b/crates/gitbutler-core/src/git/repository.rs @@ -5,6 +5,7 @@ use super::{ use crate::path::Normalize; use git2::{BlameOptions, Submodule}; use git2_hooks::HookResult; +#[cfg(unix)] use std::os::unix::fs::PermissionsExt; use std::process::Stdio; use std::{io::Write, path::Path, str}; @@ -311,10 +312,16 @@ impl Repository { let mut key_storage = tempfile::NamedTempFile::new()?; key_storage.write_all(signing_key.as_bytes())?; - // make sure the tempfile permissions are acceptable for a private ssh key - let mut permissions = key_storage.as_file().metadata()?.permissions(); - permissions.set_mode(0o600); - key_storage.as_file().set_permissions(permissions)?; + // if on unix + #[cfg(unix)] + { + // make sure the tempfile permissions are acceptable for a private ssh key + let mut permissions = key_storage.as_file().metadata()?.permissions(); + permissions.set_mode(0o600); + key_storage.as_file().set_permissions(permissions)?; + } + + // todo: what about windows? let key_file_path = key_storage.into_temp_path(); let key_storage = std::fs::read_to_string(&key_file_path)?; From a23253e57de861b1d9d50831fbe61310f2ae6a93 Mon Sep 17 00:00:00 2001 From: Scott Chacon Date: Wed, 15 May 2024 11:14:05 +0200 Subject: [PATCH 09/13] who committed this? --- crates/gitbutler-core/src/virtual_branches/controller.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/gitbutler-core/src/virtual_branches/controller.rs b/crates/gitbutler-core/src/virtual_branches/controller.rs index dc27d44e7..deaed4761 100644 --- a/crates/gitbutler-core/src/virtual_branches/controller.rs +++ b/crates/gitbutler-core/src/virtual_branches/controller.rs @@ -640,8 +640,7 @@ impl ControllerInner { self.with_verify_branch(project_id, |project_repository, user| { let result = - super::apply_branch(project_repository, branch_id, user) - .map_err(Into::into); + super::apply_branch(project_repository, branch_id, user).map_err(Into::into); result }) } From d267dbc77d7971f7ae37bf5e23ee29a3d6f8e732 Mon Sep 17 00:00:00 2001 From: Scott Chacon Date: Wed, 15 May 2024 13:40:27 +0200 Subject: [PATCH 10/13] rust is not my favorite language --- crates/gitbutler-core/src/virtual_branches/controller.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/gitbutler-core/src/virtual_branches/controller.rs b/crates/gitbutler-core/src/virtual_branches/controller.rs index deaed4761..e1f415ced 100644 --- a/crates/gitbutler-core/src/virtual_branches/controller.rs +++ b/crates/gitbutler-core/src/virtual_branches/controller.rs @@ -639,9 +639,7 @@ impl ControllerInner { let _permit = self.semaphore.acquire().await; self.with_verify_branch(project_id, |project_repository, user| { - let result = - super::apply_branch(project_repository, branch_id, user).map_err(Into::into); - result + super::apply_branch(project_repository, branch_id, user).map_err(Into::into) }) } From 2ce4e5bfd01756019e984a93818ed20d41b1de13 Mon Sep 17 00:00:00 2001 From: Scott Chacon Date: Wed, 15 May 2024 14:48:26 +0200 Subject: [PATCH 11/13] remove a bunch of debugging --- crates/gitbutler-core/src/git/repository.rs | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/crates/gitbutler-core/src/git/repository.rs b/crates/gitbutler-core/src/git/repository.rs index ce94fafa4..a19deea77 100644 --- a/crates/gitbutler-core/src/git/repository.rs +++ b/crates/gitbutler-core/src/git/repository.rs @@ -306,8 +306,6 @@ impl Repository { let output; // support literal ssh key if let (true, signing_key) = Self::is_literal_ssh_key(&signing_key) { - dbg!(&signing_key); - // write the key to a temp file let mut key_storage = tempfile::NamedTempFile::new()?; key_storage.write_all(signing_key.as_bytes())?; @@ -321,11 +319,8 @@ impl Repository { key_storage.as_file().set_permissions(permissions)?; } - // todo: what about windows? - let key_file_path = key_storage.into_temp_path(); let key_storage = std::fs::read_to_string(&key_file_path)?; - dbg!(&key_storage); cmd.arg(&key_file_path); cmd.arg("-U"); @@ -353,11 +348,7 @@ impl Repository { .commit_signed(&buffer, &signature, None) .map(Into::into) .map_err(Into::into); - dbg!(&oid); return oid; - } else { - let stderr = String::from_utf8_lossy(&output.stderr); - dbg!(stderr); } } else { // is gpg @@ -381,16 +372,12 @@ impl Repository { if output.status.success() { // read stdout let signature = String::from_utf8_lossy(&output.stdout); - dbg!(&signature); let oid = self .0 .commit_signed(&buffer, &signature, None) .map(Into::into) .map_err(Into::into); return oid; - } else { - let stderr = String::from_utf8_lossy(&output.stderr); - dbg!(stderr); } } } From b0c5c07a57018ed9ebcff15c62cb3653c2e61fa0 Mon Sep 17 00:00:00 2001 From: Scott Chacon Date: Wed, 15 May 2024 15:21:19 +0200 Subject: [PATCH 12/13] some change-id based tests --- crates/gitbutler-core/src/git/repository.rs | 6 +- .../virtual_branches/move_commit_file.rs | 9 ++ .../virtual_branches/update_commit_message.rs | 9 ++ .../tests/virtual_branches/mod.rs | 83 +++++++++++++++++++ 4 files changed, 105 insertions(+), 2 deletions(-) diff --git a/crates/gitbutler-core/src/git/repository.rs b/crates/gitbutler-core/src/git/repository.rs index a19deea77..2d8b8e5c5 100644 --- a/crates/gitbutler-core/src/git/repository.rs +++ b/crates/gitbutler-core/src/git/repository.rs @@ -278,10 +278,13 @@ impl Repository { Ok(oid.into()) } + /// takes raw commit data and commits it to the repository + /// - if the git config commit.gpgSign is set, it will sign the commit + /// returns an oid of the new commit object pub fn commit_buffer(&self, buffer: String) -> Result { // check git config for gpg.signingkey let should_sign = self.0.config()?.get_string("commit.gpgSign"); - if let Ok(_should_sign) = should_sign { + if should_sign.unwrap_or("false".to_string()) != "false" { // TODO: support gpg.ssh.defaultKeyCommand to get the signing key if this value doesn't exist let signing_key = self.0.config()?.get_string("user.signingkey"); if let Ok(signing_key) = signing_key { @@ -320,7 +323,6 @@ impl Repository { } let key_file_path = key_storage.into_temp_path(); - let key_storage = std::fs::read_to_string(&key_file_path)?; cmd.arg(&key_file_path); cmd.arg("-U"); diff --git a/crates/gitbutler-core/tests/suite/virtual_branches/move_commit_file.rs b/crates/gitbutler-core/tests/suite/virtual_branches/move_commit_file.rs index bdb57a685..c1db30331 100644 --- a/crates/gitbutler-core/tests/suite/virtual_branches/move_commit_file.rs +++ b/crates/gitbutler-core/tests/suite/virtual_branches/move_commit_file.rs @@ -25,6 +25,7 @@ async fn move_file_down() { .create_commit(project_id, &branch_id, "commit one", None, false) .await .unwrap(); + let commit1 = repository.find_commit(commit1_id).unwrap(); // create commit fs::write(repository.path().join("file2.txt"), "content2").unwrap(); @@ -33,6 +34,7 @@ async fn move_file_down() { .create_commit(project_id, &branch_id, "commit two", None, false) .await .unwrap(); + let commit2 = repository.find_commit(commit2_id).unwrap(); // amend another hunk let to_amend: branch::BranchOwnershipClaims = "file2.txt:1-2".parse().unwrap(); @@ -50,6 +52,13 @@ async fn move_file_down() { .find(|b| b.id == branch_id) .unwrap(); + // shas changed but change_id is the same + assert_eq!(&commit1.change_id(), &branch.commits[1].change_id); + assert_ne!(&commit1.id(), &branch.commits[1].id); + assert_eq!(&commit2.change_id(), &branch.commits[0].change_id); + assert_ne!(&commit2.id(), &branch.commits[0].id); + + assert_eq!(branch.commits[0].files.len(), 1); assert_eq!(branch.commits.len(), 2); assert_eq!(branch.commits[0].files.len(), 1); assert_eq!(branch.commits[1].files.len(), 2); // this now has both file changes diff --git a/crates/gitbutler-core/tests/suite/virtual_branches/update_commit_message.rs b/crates/gitbutler-core/tests/suite/virtual_branches/update_commit_message.rs index 8b9169ff5..aa0794a0e 100644 --- a/crates/gitbutler-core/tests/suite/virtual_branches/update_commit_message.rs +++ b/crates/gitbutler-core/tests/suite/virtual_branches/update_commit_message.rs @@ -42,6 +42,8 @@ async fn head() { .await .unwrap() }; + let commit_three = repository.find_commit(commit_three_oid).unwrap(); + let before_change_id = &commit_three.change_id(); controller .update_commit_message( @@ -68,6 +70,13 @@ async fn head() { .map(|c| c.description.clone()) .collect::>(); + // get the last commit + let commit = repository.find_commit(branch.head).unwrap(); + + // make sure the SHA changed, but the change ID did not + assert_ne!(&commit_three.id(), &commit.id()); + assert_eq!(before_change_id, &commit.change_id()); + assert_eq!( descriptions, vec!["commit three updated", "commit two", "commit one"] diff --git a/crates/gitbutler-core/tests/virtual_branches/mod.rs b/crates/gitbutler-core/tests/virtual_branches/mod.rs index 5335fc1ee..327b1059e 100644 --- a/crates/gitbutler-core/tests/virtual_branches/mod.rs +++ b/crates/gitbutler-core/tests/virtual_branches/mod.rs @@ -666,6 +666,89 @@ fn add_new_hunk_to_the_end() -> Result<()> { Ok(()) } +#[test] +fn commit_id_can_be_generated_or_specified() -> Result<()> { + let suite = Suite::default(); + let Case { + project_repository, + project, + .. + } = &suite.new_case(); + + let file_path = Path::new("test.txt"); + std::fs::write( + Path::new(&project.path).join(file_path), + "line1\nline2\nline3\nline4\n", + )?; + commit_all(&project_repository.git_repository); + + // lets make sure a change id is generated + let target_oid = project_repository + .git_repository + .head() + .unwrap() + .target() + .unwrap(); + let target = project_repository + .git_repository + .find_commit(target_oid) + .unwrap(); + let change_id = target.change_id(); + + // make sure we created a change-id + assert!(change_id.is_some()); + + // ok, make another change and specify a change-id + let file_path = Path::new("test.txt"); + std::fs::write( + Path::new(&project.path).join(file_path), + "line1\nline2\nline3\nline4\nline5\n", + )?; + + let repository = &project_repository.git_repository; + let mut index = repository.index().expect("failed to get index"); + index + .add_all(["."], git2::IndexAddOption::DEFAULT, None) + .expect("failed to add all"); + index.write().expect("failed to write index"); + let oid = index.write_tree().expect("failed to write tree"); + let signature = gitbutler_core::git::Signature::now("test", "test@email.com").unwrap(); + let head = repository.head().expect("failed to get head"); + repository + .commit( + Some(&head.name().unwrap()), + &signature, + &signature, + "some commit", + &repository.find_tree(oid).expect("failed to find tree"), + &[&repository + .find_commit( + repository + .refname_to_id("HEAD") + .expect("failed to get head"), + ) + .expect("failed to find commit")], + Some("my-change-id"), + ) + .expect("failed to commit"); + + let target_oid = project_repository + .git_repository + .head() + .unwrap() + .target() + .unwrap(); + let target = project_repository + .git_repository + .find_commit(target_oid) + .unwrap(); + let change_id = target.change_id(); + + // the change id should be what we specified, rather than randomly generated + assert_eq!(change_id, Some("my-change-id".to_string())); + Ok(()) +} + #[test] fn merge_vbranch_upstream_clean_rebase() -> Result<()> { let suite = Suite::default(); From 5becf4e6a63b43ec0dd5887168a7cde214821281 Mon Sep 17 00:00:00 2001 From: Scott Chacon Date: Wed, 15 May 2024 15:24:08 +0200 Subject: [PATCH 13/13] small fixes --- crates/gitbutler-core/Cargo.toml | 6 +++--- crates/gitbutler-core/src/virtual_branches/integration.rs | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/crates/gitbutler-core/Cargo.toml b/crates/gitbutler-core/Cargo.toml index 51bf2a878..c37822fb7 100644 --- a/crates/gitbutler-core/Cargo.toml +++ b/crates/gitbutler-core/Cargo.toml @@ -35,15 +35,15 @@ reqwest = { version = "0.12.4", features = ["json"] } resolve-path = "0.1.0" rusqlite.workspace = true serde.workspace = true -serde_json = { version = "1.0", features = ["std", "arbitrary_precision"] } +serde_json = { version = "1.0", features = [ "std", "arbitrary_precision" ] } sha2 = "0.10.8" -ssh-key = { version = "0.6.6", features = ["alloc", "ed25519"] } +ssh-key = { version = "0.6.6", features = [ "alloc", "ed25519" ] } ssh2 = { version = "0.9.4", features = ["vendored-openssl"] } strum = { version = "0.26", features = ["derive"] } log = "^0.4" tempfile = "3.10" thiserror.workspace = true -tokio = { workspace = true, features = ["rt-multi-thread", "rt", "macros"] } +tokio = { workspace = true, features = [ "rt-multi-thread", "rt", "macros" ] } tracing = "0.1.40" url = { version = "2.5", features = ["serde"] } urlencoding = "2.1.3" diff --git a/crates/gitbutler-core/src/virtual_branches/integration.rs b/crates/gitbutler-core/src/virtual_branches/integration.rs index 6cdb85b58..aaae9f410 100644 --- a/crates/gitbutler-core/src/virtual_branches/integration.rs +++ b/crates/gitbutler-core/src/virtual_branches/integration.rs @@ -324,7 +324,6 @@ fn verify_head_is_clean( ) .context("failed to reset to integration commit")?; - dbg!("Head creating virtual branch"); let mut new_branch = super::create_virtual_branch( project_repository, &BranchCreateRequest {