From 9f6c01c5ab9df97d5771694f0df58e9850b9b186 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 24 Sep 2024 10:58:46 +0200 Subject: [PATCH 1/5] make tests work under more conditions Currently tests rely to be run globally so `branch-actions` can set the required feature flags. Now more crates that need it will set it so that their tests can be run individually. --- crates/gitbutler-repo/Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/gitbutler-repo/Cargo.toml b/crates/gitbutler-repo/Cargo.toml index 90eb8ac53..f1ad5f7d8 100644 --- a/crates/gitbutler-repo/Cargo.toml +++ b/crates/gitbutler-repo/Cargo.toml @@ -45,4 +45,5 @@ path = "tests/mod.rs" [dev-dependencies] gitbutler-testsupport.workspace = true gitbutler-user.workspace = true +gitbutler-git = { workspace = true, features = ["test-askpass-path"] } serde_json = { version = "1.0", features = ["std", "arbitrary_precision"] } From ada4b17c47edce86fde47ef6667021d29fe4d7df Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 24 Sep 2024 11:04:00 +0200 Subject: [PATCH 2/5] Make sure test doesn't fail if the default branch isn't `master`. Ideally repository creation is standardized to avoid these issues, but for now this is just the minimal fix to make the test suite work for me. --- crates/gitbutler-branch-actions/src/upstream_integration.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/gitbutler-branch-actions/src/upstream_integration.rs b/crates/gitbutler-branch-actions/src/upstream_integration.rs index c8a8526d7..619c57798 100644 --- a/crates/gitbutler-branch-actions/src/upstream_integration.rs +++ b/crates/gitbutler-branch-actions/src/upstream_integration.rs @@ -611,7 +611,9 @@ mod test { #[test] fn test_conflicted_head_branch() { let tempdir = tempdir().unwrap(); - let repository = git2::Repository::init(tempdir.path()).unwrap(); + let repository = + git2::Repository::init_opts(tempdir.path(), &gitbutler_testsupport::init_opts()) + .unwrap(); let initial_commit = commit_file(&repository, None, &[("foo.txt", "bar")]); // Create refs/heads/master repository.branch("master", &initial_commit, false).unwrap(); From d16656816d8c46bd71646f40f332f02eb4a94c26 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 24 Sep 2024 11:16:33 +0200 Subject: [PATCH 3/5] Use `gix` to create signed commits with multiple headers. --- crates/gitbutler-commit/src/commit_headers.rs | 18 ++++ crates/gitbutler-repo/src/commands.rs | 5 +- crates/gitbutler-repo/src/repository_ext.rs | 96 +++++++++++-------- 3 files changed, 76 insertions(+), 43 deletions(-) diff --git a/crates/gitbutler-commit/src/commit_headers.rs b/crates/gitbutler-commit/src/commit_headers.rs index cb9a57ea3..87383dc6d 100644 --- a/crates/gitbutler-commit/src/commit_headers.rs +++ b/crates/gitbutler-commit/src/commit_headers.rs @@ -53,6 +53,24 @@ impl From for CommitHeadersV2 { } } +// TODO: remove CommitBuffer entirely +impl From for Vec<(BString, BString)> { + fn from(hdr: CommitHeadersV2) -> Self { + let mut out = vec![ + ( + BString::from(HEADERS_VERSION_HEADER), + BString::from(V2_HEADERS_VERSION), + ), + (V2_CHANGE_ID_HEADER.into(), hdr.change_id.clone().into()), + ]; + + if let Some(conflicted) = hdr.conflicted { + out.push((V2_CONFLICTED_HEADER.into(), conflicted.to_string().into())); + } + out + } +} + pub trait HasCommitHeaders { fn gitbutler_headers(&self) -> Option; } diff --git a/crates/gitbutler-repo/src/commands.rs b/crates/gitbutler-repo/src/commands.rs index 538322a75..a1d613448 100644 --- a/crates/gitbutler-repo/src/commands.rs +++ b/crates/gitbutler-repo/src/commands.rs @@ -1,5 +1,4 @@ use anyhow::Result; -use bstr::BString; use gitbutler_command_context::CommandContext; use gitbutler_project::Project; use std::path::Path; @@ -30,9 +29,7 @@ impl RepoCommands for Project { fn check_signing_settings(&self) -> Result { let ctx = CommandContext::open(self)?; - let signed = ctx - .repository() - .sign_buffer(&BString::new("test".into()).into()); + let signed = ctx.repository().sign_buffer(b"test"); match signed { Ok(_) => Ok(true), Err(e) => Err(e), diff --git a/crates/gitbutler-repo/src/repository_ext.rs b/crates/gitbutler-repo/src/repository_ext.rs index 4bf7f4b86..f8bbeef69 100644 --- a/crates/gitbutler-repo/src/repository_ext.rs +++ b/crates/gitbutler-repo/src/repository_ext.rs @@ -8,10 +8,11 @@ use anyhow::{anyhow, bail, Context, Result}; use bstr::{BString, ByteSlice}; use git2::{BlameOptions, Tree}; use gitbutler_branch::{gix_to_git2_signature, SignaturePurpose}; -use gitbutler_commit::{commit_buffer::CommitBuffer, commit_headers::CommitHeadersV2}; +use gitbutler_commit::commit_headers::CommitHeadersV2; use gitbutler_config::git::{GbConfig, GitConfig}; use gitbutler_error::error::Code; use gitbutler_reference::{Refname, RemoteRefname}; +use gix::objs::WriteTo; use gix::status::plumbing::index_as_worktree::{Change, EntryStatus}; use tracing::instrument; @@ -45,8 +46,9 @@ pub trait RepositoryExt { fn in_memory_repo(&self) -> Result; /// Fetches the workspace commit from the gitbutler/workspace branch fn workspace_commit(&self) -> Result>; - /// Takes a CommitBuffer and returns it after being signed by by your git signing configuration - fn sign_buffer(&self, buffer: &CommitBuffer) -> Result; + /// `buffer` is the commit object to sign, but in theory could be anything to compute the signature for. + /// Returns the computed signature. + fn sign_buffer(&self, buffer: &[u8]) -> Result; fn checkout_index_builder<'a>(&'a self, index: &'a mut git2::Index) -> CheckoutIndexBuilder; fn checkout_index_path_builder>(&self, path: P) -> Result<()>; @@ -248,45 +250,43 @@ impl RepositoryExt for git2::Repository { parents: &[&git2::Commit<'_>], commit_headers: Option, ) -> Result { - fn commit_buffer( - repository: &git2::Repository, - commit_buffer: &CommitBuffer, - ) -> Result { - let oid = repository - .odb()? - .write(git2::ObjectType::Commit, &commit_buffer.as_bstring())?; + let repo = gix::open(self.path())?; + let mut commit = gix::objs::Commit { + message: message.into(), + tree: git2_to_gix_object_id(tree.id()), + author: git2_signature_to_gix_signature(author), + committer: git2_signature_to_gix_signature(committer), + encoding: None, + parents: parents + .iter() + .map(|commit| git2_to_gix_object_id(commit.id())) + .collect(), + extra_headers: commit_headers.unwrap_or_default().into(), + }; - Ok(oid) - } - - let mut buffer: CommitBuffer = self - .commit_create_buffer(author, committer, message, tree, parents)? - .into(); - - buffer.set_gitbutler_headers(commit_headers); - - let oid = if self.gb_config()?.sign_commits.unwrap_or(false) { - let signature = self.sign_buffer(&buffer); + if self.gb_config()?.sign_commits.unwrap_or(false) { + let mut buf = Vec::new(); + commit.write_to(&mut buf)?; + let signature = self.sign_buffer(&buf); match signature { - Ok(signature) => self - .commit_signed( - buffer.as_bstring().to_string().as_str(), - signature.to_string().as_str(), - None, - ) - .map_err(Into::into), + Ok(signature) => { + commit.extra_headers.push(("gpgsig".into(), signature)); + } Err(e) => { // If signing fails, set the "gitbutler.signCommits" config to false before erroring out self.set_gb_config(GbConfig { sign_commits: Some(false), ..GbConfig::default() })?; - Err(anyhow!("Failed to sign commit: {}", e).context(Code::CommitSigningFailed)) + return Err( + anyhow!("Failed to sign commit: {}", e).context(Code::CommitSigningFailed) + ); } } - } else { - commit_buffer(self, &buffer) - }?; + } + // TODO: extra-headers should be supported in `gix` directly. + let oid = gix_to_git2_oid(repo.write_object(&commit)?); + // update reference if let Some(refname) = update_ref { self.reference(&refname.to_string(), oid, true, message)?; @@ -311,7 +311,7 @@ impl RepositoryExt for git2::Repository { self.blame_file(path, Some(&mut opts)) } - fn sign_buffer(&self, buffer: &CommitBuffer) -> Result { + fn sign_buffer(&self, buffer: &[u8]) -> Result { // check git config for gpg.signingkey // TODO: support gpg.ssh.defaultKeyCommand to get the signing key if this value doesn't exist let signing_key = self.config()?.get_string("user.signingkey"); @@ -326,7 +326,7 @@ impl RepositoryExt for git2::Repository { if 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_bstring())?; + signature_storage.write_all(buffer)?; let buffer_file_to_sign_path = signature_storage.into_temp_path(); let gpg_program = self.config()?.get_string("gpg.ssh.program"); @@ -421,11 +421,7 @@ impl RepositoryExt for git2::Repository { .context(format!("Could not execute GPG program using {:?}", cmd)) } }; - child - .stdin - .take() - .expect("configured") - .write_all(&buffer.as_bstring())?; + child.stdin.take().expect("configured").write_all(buffer)?; let output = child.wait_with_output()?; if output.status.success() { @@ -637,3 +633,25 @@ impl GixRepositoryExt for gix::Repository { Ok(self) } } + +// TODO: expose in separate crate to deduplicate +fn git2_to_gix_object_id(id: git2::Oid) -> gix::ObjectId { + gix::ObjectId::try_from(id.as_bytes()).expect("git2 oid is always valid") +} + +// TODO: expose in separate crate to deduplicate +fn git2_signature_to_gix_signature(input: &git2::Signature<'_>) -> gix::actor::Signature { + gix::actor::Signature { + name: input.name_bytes().into(), + email: input.email_bytes().into(), + time: gix::date::Time { + seconds: input.when().seconds(), + offset: input.when().offset_minutes() * 60, + sign: input.when().offset_minutes().into(), + }, + } +} + +fn gix_to_git2_oid(id: impl Into) -> git2::Oid { + git2::Oid::from_bytes(id.into().as_bytes()).expect("always valid") +} From fe7d5d92e701e5fdcf166e856ead9c3865c3470c Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 24 Sep 2024 15:38:13 +0200 Subject: [PATCH 4/5] add crate to keep gitoxide conversions to prevent duplication --- Cargo.lock | 78 +++++++++++-------- Cargo.toml | 2 + crates/gitbutler-branch-actions/Cargo.toml | 1 + crates/gitbutler-branch-actions/src/branch.rs | 9 +-- .../gitbutler-branch-actions/src/virtual.rs | 13 +--- crates/gitbutler-branch/Cargo.toml | 1 + crates/gitbutler-branch/src/lib.rs | 23 +----- crates/gitbutler-oxidize/Cargo.toml | 11 +++ crates/gitbutler-oxidize/src/lib.rs | 48 ++++++++++++ crates/gitbutler-repo/Cargo.toml | 1 + crates/gitbutler-repo/src/repository_ext.rs | 33 ++------ 11 files changed, 118 insertions(+), 102 deletions(-) create mode 100644 crates/gitbutler-oxidize/Cargo.toml create mode 100644 crates/gitbutler-oxidize/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 31c133c0a..d6c9da242 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -160,9 +160,9 @@ dependencies = [ [[package]] name = "anyhow" -version = "1.0.86" +version = "1.0.89" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b3d1d046238990b9cf5bcde22a3fb3584ee5cf65fb2765f454ed428c7a0063da" +checksum = "86fdf8605db99b54d3cd748a44c6d04df638eb5dafb219b135d0149bd0db01f6" [[package]] name = "arc-swap" @@ -328,9 +328,9 @@ dependencies = [ [[package]] name = "async-signal" -version = "0.2.10" +version = "0.2.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "637e00349800c0bdf8bfc21ebbc0b6524abea702b0da4168ac00d070d0c0b9f3" +checksum = "dfb3634b73397aa844481f814fad23bbf07fdb0eabec10f2eb95e58944b1ec32" dependencies = [ "async-io 2.3.4", "async-lock 3.4.0", @@ -341,7 +341,7 @@ dependencies = [ "rustix 0.38.34", "signal-hook-registry", "slab", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -404,7 +404,7 @@ dependencies = [ "glib-sys", "gobject-sys", "libc", - "system-deps 6.2.2", + "system-deps 6.2.0", ] [[package]] @@ -689,9 +689,9 @@ dependencies = [ [[package]] name = "bytemuck" -version = "1.17.0" +version = "1.18.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6fd4c6dcc3b0aea2f5c0b4b82c2b15fe39ddbc76041a310848f4706edf76bb31" +checksum = "94bbb0ad554ad961ddc5da507a12a29b14e4ae5bda06b19f575a3e6079d2e2ae" [[package]] name = "byteorder" @@ -750,7 +750,7 @@ checksum = "3c55d429bef56ac9172d25fecb85dc8068307d17acd74b377866b7a1ef25d3c8" dependencies = [ "glib-sys", "libc", - "system-deps 6.2.2", + "system-deps 6.2.0", ] [[package]] @@ -1971,7 +1971,7 @@ dependencies = [ "glib-sys", "gobject-sys", "libc", - "system-deps 6.2.2", + "system-deps 6.2.0", ] [[package]] @@ -1988,7 +1988,7 @@ dependencies = [ "libc", "pango-sys", "pkg-config", - "system-deps 6.2.2", + "system-deps 6.2.0", ] [[package]] @@ -2002,7 +2002,7 @@ dependencies = [ "gobject-sys", "libc", "pkg-config", - "system-deps 6.2.2", + "system-deps 6.2.0", ] [[package]] @@ -2014,7 +2014,7 @@ dependencies = [ "gdk-sys", "glib-sys", "libc", - "system-deps 6.2.2", + "system-deps 6.2.0", "x11", ] @@ -2105,7 +2105,7 @@ dependencies = [ "glib-sys", "gobject-sys", "libc", - "system-deps 6.2.2", + "system-deps 6.2.0", "winapi", ] @@ -2147,6 +2147,7 @@ dependencies = [ "gitbutler-error", "gitbutler-fs", "gitbutler-id", + "gitbutler-oxidize", "gitbutler-reference", "gitbutler-serde", "gix", @@ -2180,6 +2181,7 @@ dependencies = [ "gitbutler-id", "gitbutler-operating-modes", "gitbutler-oplog", + "gitbutler-oxidize", "gitbutler-project", "gitbutler-reference", "gitbutler-repo", @@ -2423,6 +2425,15 @@ dependencies = [ "tracing", ] +[[package]] +name = "gitbutler-oxidize" +version = "0.0.0" +dependencies = [ + "anyhow", + "git2", + "gix", +] + [[package]] name = "gitbutler-project" version = "0.0.0" @@ -2473,6 +2484,7 @@ dependencies = [ "gitbutler-error", "gitbutler-git", "gitbutler-id", + "gitbutler-oxidize", "gitbutler-project", "gitbutler-reference", "gitbutler-testsupport", @@ -3877,7 +3889,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ef4b192f8e65e9cf76cbf4ea71fa8e3be4a0e18ffe3d68b8da6836974cc5bad4" dependencies = [ "libc", - "system-deps 6.2.2", + "system-deps 6.2.0", ] [[package]] @@ -3907,7 +3919,7 @@ checksum = "0d57ce44246becd17153bd035ab4d32cfee096a657fc01f2231c9278378d1e0a" dependencies = [ "glib-sys", "libc", - "system-deps 6.2.2", + "system-deps 6.2.0", ] [[package]] @@ -3948,7 +3960,7 @@ dependencies = [ "gobject-sys", "libc", "pango-sys", - "system-deps 6.2.2", + "system-deps 6.2.0", ] [[package]] @@ -5489,7 +5501,7 @@ dependencies = [ "glib-sys", "gobject-sys", "libc", - "system-deps 6.2.2", + "system-deps 6.2.0", ] [[package]] @@ -7251,15 +7263,15 @@ dependencies = [ [[package]] name = "system-deps" -version = "6.2.2" +version = "6.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a3e535eb8dded36d55ec13eddacd30dec501792ff23a0b1682c38601b8cf2349" +checksum = "2a2d580ff6a20c55dfb86be5f9c238f67835d0e81cbdea8bf5680e0897320331" dependencies = [ "cfg-expr 0.15.8", - "heck 0.5.0", + "heck 0.4.1", "pkg-config", "toml 0.8.19", - "version-compare 0.2.0", + "version-compare 0.1.1", ] [[package]] @@ -7311,13 +7323,13 @@ dependencies = [ [[package]] name = "tao-macros" -version = "0.1.3" +version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f4e16beb8b2ac17db28eab8bca40e62dbfbb34c0fcdc6d9826b11b7b5d047dfd" +checksum = "ec114582505d158b669b136e6851f85840c109819d77c42bb7c0709f727d18c2" dependencies = [ "proc-macro2", "quote", - "syn 2.0.76", + "syn 1.0.109", ] [[package]] @@ -8192,9 +8204,9 @@ checksum = "1c18c859eead79d8b95d09e4678566e8d70105c4e7b251f707a03df32442661b" [[package]] name = "version-compare" -version = "0.2.0" +version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "852e951cb7832cb45cb1169900d19760cfa39b82bc0ea9c0e5a14ae88411c98b" +checksum = "579a42fc0b8e0c63b76519a339be31bed574929511fa53c1a3acae26eb258f29" [[package]] name = "version_check" @@ -8402,7 +8414,7 @@ dependencies = [ "pango-sys", "pkg-config", "soup2-sys", - "system-deps 6.2.2", + "system-deps 6.2.0", ] [[package]] @@ -8745,9 +8757,9 @@ checksum = "f838de2fe15fe6bac988e74b798f26499a8b21a9d97edec321e79b28d1d7f597" [[package]] name = "windows-version" -version = "0.1.1" +version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6998aa457c9ba8ff2fb9f13e9d2a930dabcea28f1d0ab94d687d8b3654844515" +checksum = "75aa004c988e080ad34aff5739c39d0312f4684699d6d71fc8a198d057b8b9b4" dependencies = [ "windows-targets 0.52.6", ] @@ -9021,12 +9033,12 @@ dependencies = [ [[package]] name = "xdg-home" -version = "1.3.0" +version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ec1cdab258fb55c0da61328dc52c8764709b249011b2cad0454c72f0bf10a1f6" +checksum = "21e5a325c3cb8398ad6cf859c1135b25dd29e186679cf2da7581d9679f63b38e" dependencies = [ "libc", - "windows-sys 0.59.0", + "winapi", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 6d962fbd5..f69f6f3a0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,6 +31,7 @@ members = [ "crates/gitbutler-operating-modes", "crates/gitbutler-edit-mode", "crates/gitbutler-cherry-pick", + "crates/gitbutler-oxidize", ] resolver = "2" @@ -83,6 +84,7 @@ gitbutler-diff = { path = "crates/gitbutler-diff" } gitbutler-operating-modes = { path = "crates/gitbutler-operating-modes" } gitbutler-edit-mode = { path = "crates/gitbutler-edit-mode" } gitbutler-cherry-pick = { path = "crates/gitbutler-cherry-pick" } +gitbutler-oxidize = { path = "crates/gitbutler-oxidize" } [profile.release] codegen-units = 1 # Compile crates one after another so the compiler can optimize better diff --git a/crates/gitbutler-branch-actions/Cargo.toml b/crates/gitbutler-branch-actions/Cargo.toml index 064e44e76..fd1fdc1a5 100644 --- a/crates/gitbutler-branch-actions/Cargo.toml +++ b/crates/gitbutler-branch-actions/Cargo.toml @@ -26,6 +26,7 @@ gitbutler-fs.workspace = true gitbutler-diff.workspace = true gitbutler-operating-modes.workspace = true gitbutler-cherry-pick.workspace = true +gitbutler-oxidize.workspace = true serde = { workspace = true, features = ["std"] } bstr.workspace = true diffy = "0.4.0" diff --git a/crates/gitbutler-branch-actions/src/branch.rs b/crates/gitbutler-branch-actions/src/branch.rs index fe4cacccc..e583a8fd2 100644 --- a/crates/gitbutler-branch-actions/src/branch.rs +++ b/crates/gitbutler-branch-actions/src/branch.rs @@ -7,6 +7,7 @@ use gitbutler_branch::{ }; use gitbutler_command_context::CommandContext; use gitbutler_diff::DiffByPathMap; +use gitbutler_oxidize::{git2_to_gix_object_id, gix_to_git2_oid}; use gitbutler_project::access::WorktreeReadPermission; use gitbutler_reference::normalize_branch_name; use gitbutler_repo::{GixRepositoryExt, RepositoryExt as _}; @@ -290,14 +291,6 @@ fn branch_group_to_branch( })) } -fn gix_to_git2_oid(id: impl Into) -> git2::Oid { - git2::Oid::from_bytes(id.into().as_bytes()).expect("always valid") -} - -fn git2_to_gix_object_id(id: git2::Oid) -> gix::ObjectId { - gix::ObjectId::try_from(id.as_bytes()).expect("git2 oid is always valid") -} - /// A sum type of branch that can be a plain git branch or a virtual branch #[allow(clippy::large_enum_variant)] enum GroupBranch<'a> { diff --git a/crates/gitbutler-branch-actions/src/virtual.rs b/crates/gitbutler-branch-actions/src/virtual.rs index a693893fd..bdc654f92 100644 --- a/crates/gitbutler-branch-actions/src/virtual.rs +++ b/crates/gitbutler-branch-actions/src/virtual.rs @@ -21,6 +21,7 @@ use gitbutler_commit::{commit_ext::CommitExt, commit_headers::HasCommitHeaders}; use gitbutler_diff::{trees, GitHunk, Hunk}; use gitbutler_error::error::{Code, Marker}; use gitbutler_operating_modes::assure_open_workspace_mode; +use gitbutler_oxidize::git2_signature_to_gix_signature; use gitbutler_project::access::WorktreeWritePermission; use gitbutler_reference::{normalize_branch_name, Refname, RemoteRefname}; use gitbutler_repo::{ @@ -454,18 +455,6 @@ impl TryFrom<&git2::Commit<'_>> for CommitData { } } -fn git2_signature_to_gix_signature(input: git2::Signature<'_>) -> gix::actor::Signature { - gix::actor::Signature { - name: input.name_bytes().into(), - email: input.email_bytes().into(), - time: gix::date::Time { - seconds: input.when().seconds(), - offset: input.when().offset_minutes() * 60, - sign: input.when().offset_minutes().into(), - }, - } -} - fn branches_with_large_files_abridged(mut branches: Vec) -> Vec { for branch in &mut branches { for file in &mut branch.files { diff --git a/crates/gitbutler-branch/Cargo.toml b/crates/gitbutler-branch/Cargo.toml index 81e462787..38afbac7d 100644 --- a/crates/gitbutler-branch/Cargo.toml +++ b/crates/gitbutler-branch/Cargo.toml @@ -15,6 +15,7 @@ gitbutler-id.workspace = true gitbutler-error.workspace = true gitbutler-fs.workspace = true gitbutler-diff.workspace = true +gitbutler-oxidize.workspace = true itertools = "0.13" toml.workspace = true serde = { workspace = true, features = ["std"] } diff --git a/crates/gitbutler-branch/src/lib.rs b/crates/gitbutler-branch/src/lib.rs index db60dd3ef..0077c3a0d 100644 --- a/crates/gitbutler-branch/src/lib.rs +++ b/crates/gitbutler-branch/src/lib.rs @@ -1,8 +1,6 @@ mod branch; -use anyhow::Context; pub use branch::{Branch, BranchCreateRequest, BranchId, BranchIdentity, BranchUpdateRequest}; -use bstr::ByteSlice; mod branch_ext; pub use branch_ext::BranchExt; mod reference_ext; @@ -20,6 +18,7 @@ mod reference; pub use reference::ChangeReference; mod state; +use gitbutler_oxidize::gix_to_git2_signature; use lazy_static::lazy_static; pub use state::{VirtualBranches as VirtualBranchesState, VirtualBranchesHandle}; lazy_static! { @@ -49,26 +48,6 @@ pub fn signature(purpose: SignaturePurpose) -> anyhow::Result, -) -> anyhow::Result> { - let offset_in_minutes = actor.time.offset / 60; - let time = git2::Time::new(actor.time.seconds, offset_in_minutes); - Ok(git2::Signature::new( - actor - .name - .to_str() - .with_context(|| format!("Could not process actor name: {}", actor.name))?, - actor - .email - .to_str() - .with_context(|| format!("Could not process actor email: {}", actor.email))?, - &time, - )?) -} - /// Return the time of a commit as `now` unless the `overriding_variable_name` contains a parseable date, /// which is used instead. fn commit_time(overriding_variable_name: &str) -> gix::date::Time { diff --git a/crates/gitbutler-oxidize/Cargo.toml b/crates/gitbutler-oxidize/Cargo.toml new file mode 100644 index 000000000..acd6808f8 --- /dev/null +++ b/crates/gitbutler-oxidize/Cargo.toml @@ -0,0 +1,11 @@ +[package] +name = "gitbutler-oxidize" +version = "0.0.0" +edition = "2021" +authors = ["GitButler "] +publish = false + +[dependencies] +anyhow.workspace = true +gix.workspace = true +git2.workspace = true \ No newline at end of file diff --git a/crates/gitbutler-oxidize/src/lib.rs b/crates/gitbutler-oxidize/src/lib.rs new file mode 100644 index 000000000..224da1ed1 --- /dev/null +++ b/crates/gitbutler-oxidize/src/lib.rs @@ -0,0 +1,48 @@ +//! A crate with various utilities to make the migration to `gitoxide` less cumbersome and repetitive. + +use anyhow::Context; +use gix::bstr::ByteSlice; +use std::borrow::Borrow; + +pub fn git2_to_gix_object_id(id: git2::Oid) -> gix::ObjectId { + gix::ObjectId::try_from(id.as_bytes()).expect("git2 oid is always valid") +} + +pub fn gix_to_git2_oid(id: impl Into) -> git2::Oid { + git2::Oid::from_bytes(id.into().as_bytes()).expect("always valid") +} + +pub fn git2_signature_to_gix_signature<'a>( + input: impl Borrow>, +) -> gix::actor::Signature { + let input = input.borrow(); + gix::actor::Signature { + name: input.name_bytes().into(), + email: input.email_bytes().into(), + time: gix::date::Time { + seconds: input.when().seconds(), + offset: input.when().offset_minutes() * 60, + sign: input.when().offset_minutes().into(), + }, + } +} + +/// Convert `actor` to a `git2` representation or fail if that's not possible. +/// Note that the current time as provided by `gix` is also used as it. +pub fn gix_to_git2_signature( + actor: gix::actor::SignatureRef<'_>, +) -> anyhow::Result> { + let offset_in_minutes = actor.time.offset / 60; + let time = git2::Time::new(actor.time.seconds, offset_in_minutes); + Ok(git2::Signature::new( + actor + .name + .to_str() + .with_context(|| format!("Could not process actor name: {}", actor.name))?, + actor + .email + .to_str() + .with_context(|| format!("Could not process actor email: {}", actor.email))?, + &time, + )?) +} diff --git a/crates/gitbutler-repo/Cargo.toml b/crates/gitbutler-repo/Cargo.toml index f1ad5f7d8..eb81f695d 100644 --- a/crates/gitbutler-repo/Cargo.toml +++ b/crates/gitbutler-repo/Cargo.toml @@ -35,6 +35,7 @@ gitbutler-time.workspace = true gitbutler-commit.workspace = true gitbutler-url.workspace = true gitbutler-cherry-pick.workspace = true +gitbutler-oxidize.workspace = true uuid.workspace = true itertools = "0.13" diff --git a/crates/gitbutler-repo/src/repository_ext.rs b/crates/gitbutler-repo/src/repository_ext.rs index f8bbeef69..1d39038e9 100644 --- a/crates/gitbutler-repo/src/repository_ext.rs +++ b/crates/gitbutler-repo/src/repository_ext.rs @@ -4,20 +4,22 @@ use std::os::unix::fs::PermissionsExt; use std::os::windows::process::CommandExt; use std::{io::Write, path::Path, process::Stdio, str}; +use crate::{Config, LogUntil}; use anyhow::{anyhow, bail, Context, Result}; use bstr::{BString, ByteSlice}; use git2::{BlameOptions, Tree}; -use gitbutler_branch::{gix_to_git2_signature, SignaturePurpose}; +use gitbutler_branch::SignaturePurpose; use gitbutler_commit::commit_headers::CommitHeadersV2; use gitbutler_config::git::{GbConfig, GitConfig}; use gitbutler_error::error::Code; +use gitbutler_oxidize::{ + git2_signature_to_gix_signature, git2_to_gix_object_id, gix_to_git2_oid, gix_to_git2_signature, +}; use gitbutler_reference::{Refname, RemoteRefname}; use gix::objs::WriteTo; use gix::status::plumbing::index_as_worktree::{Change, EntryStatus}; use tracing::instrument; -use crate::{Config, LogUntil}; - /// Extension trait for `git2::Repository`. /// /// For now, it collects useful methods from `gitbutler-core::git::Repository` @@ -536,7 +538,7 @@ impl RepositoryExt for git2::Repository { let author = repo .author() .transpose()? - .map(gitbutler_branch::gix_to_git2_signature) + .map(gix_to_git2_signature) .transpose()? .context("No author is configured in Git") .context(Code::AuthorMissing)?; @@ -619,7 +621,6 @@ impl CheckoutIndexBuilder<'_> { } } -// TODO(ST): put this into `gix`, the logic seems good, add unit-test for number generation. pub trait GixRepositoryExt: Sized { /// Configure the repository for diff operations between trees. /// This means it needs an object cache relative to the amount of files in the repository. @@ -633,25 +634,3 @@ impl GixRepositoryExt for gix::Repository { Ok(self) } } - -// TODO: expose in separate crate to deduplicate -fn git2_to_gix_object_id(id: git2::Oid) -> gix::ObjectId { - gix::ObjectId::try_from(id.as_bytes()).expect("git2 oid is always valid") -} - -// TODO: expose in separate crate to deduplicate -fn git2_signature_to_gix_signature(input: &git2::Signature<'_>) -> gix::actor::Signature { - gix::actor::Signature { - name: input.name_bytes().into(), - email: input.email_bytes().into(), - time: gix::date::Time { - seconds: input.when().seconds(), - offset: input.when().offset_minutes() * 60, - sign: input.when().offset_minutes().into(), - }, - } -} - -fn gix_to_git2_oid(id: impl Into) -> git2::Oid { - git2::Oid::from_bytes(id.into().as_bytes()).expect("always valid") -} From 0e633234562c7e3390262a9fce7b1f77004e4e25 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 24 Sep 2024 15:54:12 +0200 Subject: [PATCH 5/5] remove commit-buffer as it's not needed anymore for dealing with headers. --- crates/gitbutler-commit/src/commit_buffer.rs | 91 ------------------- crates/gitbutler-commit/src/commit_headers.rs | 17 +--- crates/gitbutler-commit/src/lib.rs | 1 - 3 files changed, 1 insertion(+), 108 deletions(-) diff --git a/crates/gitbutler-commit/src/commit_buffer.rs b/crates/gitbutler-commit/src/commit_buffer.rs index 45ea1f286..8b1378917 100644 --- a/crates/gitbutler-commit/src/commit_buffer.rs +++ b/crates/gitbutler-commit/src/commit_buffer.rs @@ -1,92 +1 @@ -use core::str; -use bstr::{BStr, BString, ByteSlice, ByteVec}; - -use crate::commit_headers::CommitHeadersV2; - -pub struct CommitBuffer { - heading: Vec<(BString, BString)>, - message: BString, -} - -impl CommitBuffer { - pub fn new(buffer: &[u8]) -> Self { - let buffer = BStr::new(buffer); - if let Some((heading, message)) = buffer.split_once_str("\n\n") { - let heading = heading - .lines() - .filter_map(|line| line.split_once_str(" ")) - .map(|(key, value)| (key.into(), value.into())) - .collect(); - - Self { - heading, - message: message.into(), - } - } else { - Self { - heading: vec![], - message: buffer.into(), - } - } - } - - pub fn set_header(&mut self, key: &str, value: &str) { - let mut set_heading = false; - self.heading.iter_mut().for_each(|(k, v)| { - if k == key { - *v = value.into(); - set_heading = true; - } - }); - - if !set_heading { - self.heading.push((key.into(), value.into())); - } - } - - /// Defers to the CommitHeadersV2 struct about which headers should be injected. - /// If `commit_headers: None` is provided, a default set of headers, including a generated change-id will be used - pub fn set_gitbutler_headers(&mut self, commit_headers: Option) { - if let Some(commit_headers) = commit_headers { - commit_headers.inject_into(self) - } else { - CommitHeadersV2::inject_default(self) - } - } - - pub fn as_bstring(&self) -> BString { - let mut output = BString::new(vec![]); - - for (key, value) in &self.heading { - output.push_str(key); - output.push_str(" "); - output.push_str(value); - output.push_str("\n"); - } - - output.push_str("\n"); - - output.push_str(&self.message); - - output - } -} - -impl From for CommitBuffer { - fn from(git2_buffer: git2::Buf) -> Self { - Self::new(&git2_buffer) - } -} - -impl From for CommitBuffer { - fn from(s: BString) -> Self { - Self::new(s.as_bytes()) - } -} - -impl From for BString { - fn from(buffer: CommitBuffer) -> BString { - buffer.as_bstring() - } -} diff --git a/crates/gitbutler-commit/src/commit_headers.rs b/crates/gitbutler-commit/src/commit_headers.rs index 87383dc6d..d2a2342a4 100644 --- a/crates/gitbutler-commit/src/commit_headers.rs +++ b/crates/gitbutler-commit/src/commit_headers.rs @@ -1,8 +1,6 @@ use bstr::{BStr, BString}; use uuid::Uuid; -use crate::commit_buffer::CommitBuffer; - /// Header used to determine which version of the headers is in use. This should never be changed const HEADERS_VERSION_HEADER: &str = "gitbutler-headers-version"; @@ -53,7 +51,6 @@ impl From for CommitHeadersV2 { } } -// TODO: remove CommitBuffer entirely impl From for Vec<(BString, BString)> { fn from(hdr: CommitHeadersV2) -> Self { let mut out = vec![ @@ -116,6 +113,7 @@ impl HasCommitHeaders for git2::Commit<'_> { } } +/// Lifecycle impl CommitHeadersV2 { /// Used to create a CommitHeadersV2. This does not allow a change_id to be /// provided in order to ensure a consistent format. @@ -124,17 +122,4 @@ impl CommitHeadersV2 { ..Default::default() } } - - pub fn inject_default(commit_buffer: &mut CommitBuffer) { - CommitHeadersV2::default().inject_into(commit_buffer) - } - - pub fn inject_into(&self, commit_buffer: &mut CommitBuffer) { - commit_buffer.set_header(HEADERS_VERSION_HEADER, V2_HEADERS_VERSION); - commit_buffer.set_header(V2_CHANGE_ID_HEADER, &self.change_id); - - if let Some(conflicted) = self.conflicted { - commit_buffer.set_header(V2_CONFLICTED_HEADER, &conflicted.to_string()) - } - } } diff --git a/crates/gitbutler-commit/src/lib.rs b/crates/gitbutler-commit/src/lib.rs index 339a7deb2..0f217f50c 100644 --- a/crates/gitbutler-commit/src/lib.rs +++ b/crates/gitbutler-commit/src/lib.rs @@ -1,3 +1,2 @@ -pub mod commit_buffer; pub mod commit_ext; pub mod commit_headers;