From 9e54a317afba57b9896aebf170075567e5d7e76d Mon Sep 17 00:00:00 2001 From: Evan Mesterhazy Date: Tue, 13 Feb 2024 22:47:09 -0500 Subject: [PATCH] Replace uses of content_hash! with #[derive(ContentHash)] This is a pure refactor with no behavior changes. #3054 --- lib/src/backend.rs | 98 ++++++++++++---------------- lib/src/content_hash.rs | 62 ++---------------- lib/src/object_id.rs | 6 +- lib/src/op_store.rs | 140 ++++++++++++++++++---------------------- lib/src/repo_path.rs | 15 ++--- 5 files changed, 120 insertions(+), 201 deletions(-) diff --git a/lib/src/backend.rs b/lib/src/backend.rs index 5a55c90e4f..644d269c5e 100644 --- a/lib/src/backend.rs +++ b/lib/src/backend.rs @@ -39,18 +39,14 @@ id_type!(pub FileId); id_type!(pub SymlinkId); id_type!(pub ConflictId); -content_hash! { - #[derive(Debug, PartialEq, Eq, Clone, PartialOrd, Ord)] - pub struct MillisSinceEpoch(pub i64); -} - -content_hash! { - #[derive(Debug, PartialEq, Eq, Clone, PartialOrd, Ord)] - pub struct Timestamp { - pub timestamp: MillisSinceEpoch, - // time zone offset in minutes - pub tz_offset: i32, - } +#[derive(ContentHash, Debug, PartialEq, Eq, Clone, PartialOrd, Ord)] +pub struct MillisSinceEpoch(pub i64); + +#[derive(ContentHash, Debug, PartialEq, Eq, Clone, PartialOrd, Ord)] +pub struct Timestamp { + pub timestamp: MillisSinceEpoch, + // time zone offset in minutes + pub tz_offset: i32, } impl Timestamp { @@ -68,21 +64,17 @@ impl Timestamp { } } -content_hash! { - #[derive(Debug, PartialEq, Eq, Clone)] - pub struct Signature { - pub name: String, - pub email: String, - pub timestamp: Timestamp, - } +#[derive(ContentHash, Debug, PartialEq, Eq, Clone)] +pub struct Signature { + pub name: String, + pub email: String, + pub timestamp: Timestamp, } -content_hash! { - #[derive(Debug, PartialEq, Eq, Clone)] - pub struct SecureSig { - pub data: Vec, - pub sig: Vec, - } +#[derive(ContentHash, Debug, PartialEq, Eq, Clone)] +pub struct SecureSig { + pub data: Vec, + pub sig: Vec, } pub type SigningFn<'a> = dyn FnMut(&[u8]) -> SignResult> + 'a; @@ -140,37 +132,31 @@ impl MergedTreeId { } } -content_hash! { - #[derive(Debug, PartialEq, Eq, Clone)] - pub struct Commit { - pub parents: Vec, - pub predecessors: Vec, - pub root_tree: MergedTreeId, - pub change_id: ChangeId, - pub description: String, - pub author: Signature, - pub committer: Signature, - pub secure_sig: Option, - } +#[derive(ContentHash, Debug, PartialEq, Eq, Clone)] +pub struct Commit { + pub parents: Vec, + pub predecessors: Vec, + pub root_tree: MergedTreeId, + pub change_id: ChangeId, + pub description: String, + pub author: Signature, + pub committer: Signature, + pub secure_sig: Option, } -content_hash! { - #[derive(Debug, PartialEq, Eq, Clone)] - pub struct ConflictTerm { - pub value: TreeValue, - } +#[derive(ContentHash, Debug, PartialEq, Eq, Clone)] +pub struct ConflictTerm { + pub value: TreeValue, } -content_hash! { - #[derive(Default, Debug, PartialEq, Eq, Clone)] - pub struct Conflict { - // A conflict is represented by a list of positive and negative states that need to be applied. - // In a simple 3-way merge of B and C with merge base A, the conflict will be { add: [B, C], - // remove: [A] }. Also note that a conflict of the form { add: [A], remove: [] } is the - // same as non-conflict A. - pub removes: Vec, - pub adds: Vec, - } +#[derive(ContentHash, Default, Debug, PartialEq, Eq, Clone)] +pub struct Conflict { + // A conflict is represented by a list of positive and negative states that need to be applied. + // In a simple 3-way merge of B and C with merge base A, the conflict will be { add: [B, C], + // remove: [A] }. Also note that a conflict of the form { add: [A], remove: [] } is the + // same as non-conflict A. + pub removes: Vec, + pub adds: Vec, } /// Error that may occur during backend initialization. @@ -309,11 +295,9 @@ impl<'a> Iterator for TreeEntriesNonRecursiveIterator<'a> { } } -content_hash! { - #[derive(Default, PartialEq, Eq, Debug, Clone)] - pub struct Tree { - entries: BTreeMap, - } +#[derive(ContentHash, Default, PartialEq, Eq, Debug, Clone)] +pub struct Tree { + entries: BTreeMap, } impl Tree { diff --git a/lib/src/content_hash.rs b/lib/src/content_hash.rs index 274c23376c..7439c609e1 100644 --- a/lib/src/content_hash.rs +++ b/lib/src/content_hash.rs @@ -133,33 +133,6 @@ where } } -macro_rules! content_hash { - ($(#[$meta:meta])* $vis:vis struct $name:ident { - $($(#[$field_meta:meta])* $field_vis:vis $field:ident : $ty:ty),* $(,)? - }) => { - $(#[$meta])* - $vis struct $name { - $($(#[$field_meta])* $field_vis $field : $ty),* - } - - impl crate::content_hash::ContentHash for $name { - fn hash(&self, state: &mut impl digest::Update) { - $(<$ty as crate::content_hash::ContentHash>::hash(&self.$field, state);)* - } - } - }; - ($(#[$meta:meta])* $vis:vis struct $name:ident($field_vis:vis $ty:ty);) => { - $(#[$meta])* - $vis struct $name($field_vis $ty); - - impl crate::content_hash::ContentHash for $name { - fn hash(&self, state: &mut impl digest::Update) { - <$ty as crate::content_hash::ContentHash>::hash(&self.0, state); - } - } - }; -} - #[cfg(test)] mod tests { use std::collections::{BTreeMap, HashMap}; @@ -203,8 +176,9 @@ mod tests { #[test] fn test_struct_sanity() { - content_hash! { - struct Foo { x: i32 } + #[derive(ContentHash)] + struct Foo { + x: i32, } assert_ne!(hash(&Foo { x: 42 }), hash(&Foo { x: 12 })); } @@ -227,8 +201,10 @@ mod tests { fn test_consistent_hashing() { let expected_hash = "14e42ea3d680bc815d0cea8ac20d3e872120014fb7bba8d82c3ffa7a8e6d63c41ef9631c60b73b150e3dd72efe50e8b0248321fe2b7eea09d879f3757b879372"; - content_hash! { - struct Foo { x: Vec>, y: i64 } + #[derive(ContentHash)] + struct Foo { + x: Vec>, + y: i64, } assert_eq!( hex::encode(hash(&Foo { @@ -265,30 +241,6 @@ mod tests { assert_eq!(hash(&Option::::None), hash(&MyOption::::None)); assert_eq!(hash(&Some(1)), hash(&MyOption::Some(1))); } - // This will be removed once all uses of content_hash! are replaced by the - // derive version. - #[test] - fn derive_is_equivalent_to_macro() { - content_hash! { - struct FooMacro { x: Vec>, y: i64} - } - - #[derive(ContentHash)] - struct FooDerive { - x: Vec>, - y: i64, - } - - let foo_macro = FooMacro { - x: vec![None, Some(42)], - y: 17, - }; - let foo_derive = FooDerive { - x: vec![None, Some(42)], - y: 17, - }; - assert_eq!(hash(&foo_macro), hash(&foo_derive)); - } fn hash(x: &(impl ContentHash + ?Sized)) -> digest::Output { blake2b_hash(x) diff --git a/lib/src/object_id.rs b/lib/src/object_id.rs index cd117f3e65..89f6c9bb6d 100644 --- a/lib/src/object_id.rs +++ b/lib/src/object_id.rs @@ -23,10 +23,8 @@ pub trait ObjectId { macro_rules! id_type { ($vis:vis $name:ident) => { - content_hash! { - #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Hash)] - $vis struct $name(Vec); - } + #[derive(ContentHash, PartialEq, Eq, PartialOrd, Ord, Clone, Hash)] + $vis struct $name(Vec); $crate::object_id::impl_id_type!($name); }; } diff --git a/lib/src/op_store.rs b/lib/src/op_store.rs index 25c2aad92e..cf2f0d3851 100644 --- a/lib/src/op_store.rs +++ b/lib/src/op_store.rs @@ -29,10 +29,8 @@ use crate::content_hash::{ContentHash, DigestUpdate}; use crate::merge::Merge; use crate::object_id::{id_type, HexPrefix, ObjectId, PrefixResolution}; -content_hash! { - #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Hash)] - pub struct WorkspaceId(String); -} +#[derive(ContentHash, PartialEq, Eq, PartialOrd, Ord, Clone, Hash)] +pub struct WorkspaceId(String); impl Debug for WorkspaceId { fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), Error> { @@ -59,11 +57,9 @@ impl WorkspaceId { id_type!(pub ViewId); id_type!(pub OperationId); -content_hash! { - #[derive(PartialEq, Eq, Hash, Clone, Debug)] - pub struct RefTarget { - merge: Merge>, - } +#[derive(ContentHash, PartialEq, Eq, Hash, Clone, Debug)] +pub struct RefTarget { + merge: Merge>, } impl Default for RefTarget { @@ -147,13 +143,11 @@ impl RefTarget { } } -content_hash! { - /// Remote branch or tag. - #[derive(Clone, Debug, Eq, Hash, PartialEq)] - pub struct RemoteRef { - pub target: RefTarget, - pub state: RemoteRefState, - } +/// Remote branch or tag. +#[derive(ContentHash, Clone, Debug, Eq, Hash, PartialEq)] +pub struct RemoteRef { + pub target: RefTarget, + pub state: RemoteRefState, } impl RemoteRef { @@ -266,39 +260,35 @@ pub struct BranchTarget<'a> { pub remote_refs: Vec<(&'a str, &'a RemoteRef)>, } -content_hash! { - /// Represents the way the repo looks at a given time, just like how a Tree - /// object represents how the file system looks at a given time. - #[derive(PartialEq, Eq, Clone, Debug, Default)] - pub struct View { - /// All head commits - pub head_ids: HashSet, - pub local_branches: BTreeMap, - pub tags: BTreeMap, - pub remote_views: BTreeMap, - pub git_refs: BTreeMap, - /// The commit the Git HEAD points to. - // TODO: Support multiple Git worktrees? - // TODO: Do we want to store the current branch name too? - pub git_head: RefTarget, - // The commit that *should be* checked out in the workspace. Note that the working copy - // (.jj/working_copy/) has the source of truth about which commit *is* checked out (to be - // precise: the commit to which we most recently completed an update to). - pub wc_commit_ids: HashMap, - } +/// Represents the way the repo looks at a given time, just like how a Tree +/// object represents how the file system looks at a given time. +#[derive(ContentHash, PartialEq, Eq, Clone, Debug, Default)] +pub struct View { + /// All head commits + pub head_ids: HashSet, + pub local_branches: BTreeMap, + pub tags: BTreeMap, + pub remote_views: BTreeMap, + pub git_refs: BTreeMap, + /// The commit the Git HEAD points to. + // TODO: Support multiple Git worktrees? + // TODO: Do we want to store the current branch name too? + pub git_head: RefTarget, + // The commit that *should be* checked out in the workspace. Note that the working copy + // (.jj/working_copy/) has the source of truth about which commit *is* checked out (to be + // precise: the commit to which we most recently completed an update to). + pub wc_commit_ids: HashMap, } -content_hash! { - /// Represents the state of the remote repo. - #[derive(Clone, Debug, Default, Eq, PartialEq)] - pub struct RemoteView { - // TODO: Do we need to support tombstones for remote branches? For example, if the branch - // has been deleted locally and you pull from a remote, maybe it should make a difference - // whether the branch is known to have existed on the remote. We may not want to resurrect - // the branch if the branch's state on the remote was just not known. - pub branches: BTreeMap, - // TODO: pub tags: BTreeMap, - } +/// Represents the state of the remote repo. +#[derive(ContentHash, Clone, Debug, Default, Eq, PartialEq)] +pub struct RemoteView { + // TODO: Do we need to support tombstones for remote branches? For example, if the branch + // has been deleted locally and you pull from a remote, maybe it should make a difference + // whether the branch is known to have existed on the remote. We may not want to resurrect + // the branch if the branch's state on the remote was just not known. + pub branches: BTreeMap, + // TODO: pub tags: BTreeMap, } /// Iterates pair of local and remote branches by branch name. @@ -353,25 +343,23 @@ pub(crate) fn flatten_remote_branches( .kmerge_by(|(full_name1, _), (full_name2, _)| full_name1 < full_name2) } -content_hash! { - /// Represents an operation (transaction) on the repo view, just like how a - /// Commit object represents an operation on the tree. - /// - /// Operations and views are not meant to be exchanged between repos or users; - /// they represent local state and history. - /// - /// The operation history will almost always be linear. It will only have - /// forks when parallel operations occurred. The parent is determined when - /// the transaction starts. When the transaction commits, a lock will be - /// taken and it will be checked that the current head of the operation - /// graph is unchanged. If the current head has changed, there has been - /// concurrent operation. - #[derive(PartialEq, Eq, Clone, Debug)] - pub struct Operation { - pub view_id: ViewId, - pub parents: Vec, - pub metadata: OperationMetadata, - } +/// Represents an operation (transaction) on the repo view, just like how a +/// Commit object represents an operation on the tree. +/// +/// Operations and views are not meant to be exchanged between repos or users; +/// they represent local state and history. +/// +/// The operation history will almost always be linear. It will only have +/// forks when parallel operations occurred. The parent is determined when +/// the transaction starts. When the transaction commits, a lock will be +/// taken and it will be checked that the current head of the operation +/// graph is unchanged. If the current head has changed, there has been +/// concurrent operation. +#[derive(ContentHash, PartialEq, Eq, Clone, Debug)] +pub struct Operation { + pub view_id: ViewId, + pub parents: Vec, + pub metadata: OperationMetadata, } impl Operation { @@ -396,17 +384,15 @@ impl Operation { } } -content_hash! { - #[derive(PartialEq, Eq, Clone, Debug)] - pub struct OperationMetadata { - pub start_time: Timestamp, - pub end_time: Timestamp, - // Whatever is useful to the user, such as exact command line call - pub description: String, - pub hostname: String, - pub username: String, - pub tags: HashMap, - } +#[derive(ContentHash, PartialEq, Eq, Clone, Debug)] +pub struct OperationMetadata { + pub start_time: Timestamp, + pub end_time: Timestamp, + // Whatever is useful to the user, such as exact command line call + pub description: String, + pub hostname: String, + pub username: String, + pub tags: HashMap, } #[derive(Debug, Error)] diff --git a/lib/src/repo_path.rs b/lib/src/repo_path.rs index 707ca3c1fb..d1006d7376 100644 --- a/lib/src/repo_path.rs +++ b/lib/src/repo_path.rs @@ -24,16 +24,15 @@ use std::path::{Component, Path, PathBuf}; use ref_cast::{ref_cast_custom, RefCastCustom}; use thiserror::Error; +use crate::content_hash::ContentHash; use crate::file_util; -content_hash! { - /// Owned `RepoPath` component. - #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)] - pub struct RepoPathComponentBuf { - // Don't add more fields. Eq, Hash, and Ord must be compatible with the - // borrowed RepoPathComponent type. - value: String, - } +/// Owned `RepoPath` component. +#[derive(ContentHash, Clone, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)] +pub struct RepoPathComponentBuf { + // Don't add more fields. Eq, Hash, and Ord must be compatible with the + // borrowed RepoPathComponent type. + value: String, } /// Borrowed `RepoPath` component.