signing: pass SigningFn by reference

write_commit() doesn't need ownership of the signing function.
This commit is contained in:
Yuya Nishihara 2023-11-28 15:34:02 +09:00
parent ec7f9e79f6
commit d747879aee
8 changed files with 20 additions and 17 deletions

View file

@ -166,7 +166,7 @@ impl Backend for JitBackend {
fn write_commit( fn write_commit(
&self, &self,
contents: Commit, contents: Commit,
sign_with: Option<SigningFn>, sign_with: Option<&mut SigningFn>,
) -> BackendResult<(CommitId, Commit)> { ) -> BackendResult<(CommitId, Commit)> {
self.inner.write_commit(contents, sign_with) self.inner.write_commit(contents, sign_with)
} }

View file

@ -148,7 +148,7 @@ content_hash! {
} }
} }
pub type SigningFn = Box<dyn FnMut(&[u8]) -> SignResult<Vec<u8>>>; pub type SigningFn<'a> = dyn FnMut(&[u8]) -> SignResult<Vec<u8>> + 'a;
/// Identifies a single legacy tree, which may have path-level conflicts, or a /// Identifies a single legacy tree, which may have path-level conflicts, or a
/// merge of multiple trees, where the individual trees do not have conflicts. /// merge of multiple trees, where the individual trees do not have conflicts.
@ -531,6 +531,6 @@ pub trait Backend: Send + Sync + Debug {
fn write_commit( fn write_commit(
&self, &self,
contents: Commit, contents: Commit,
sign_with: Option<SigningFn>, sign_with: Option<&mut SigningFn>,
) -> BackendResult<(CommitId, Commit)>; ) -> BackendResult<(CommitId, Commit)>;
} }

View file

@ -183,14 +183,13 @@ impl CommitBuilder<'_> {
} }
} }
let sign_settings = self.sign_settings; let sign_settings = &self.sign_settings;
let store = self.mut_repo.store(); let store = self.mut_repo.store();
let signing_fn = (store.signer().can_sign() && sign_settings.should_sign(&self.commit)) let mut signing_fn = (store.signer().can_sign() && sign_settings.should_sign(&self.commit))
.then(|| { .then(|| -> Box<SigningFn> {
let store = store.clone(); let store = store.clone();
Box::new(move |data: &_| store.signer().sign(data, sign_settings.key.as_deref())) Box::new(move |data: &_| store.signer().sign(data, sign_settings.key.as_deref()))
as SigningFn
}); });
// Commit backend doesn't use secure_sig for writing and enforces it with an // Commit backend doesn't use secure_sig for writing and enforces it with an
@ -198,7 +197,9 @@ impl CommitBuilder<'_> {
// if we're rewriting a signed commit // if we're rewriting a signed commit
self.commit.secure_sig = None; self.commit.secure_sig = None;
let commit = self.mut_repo.write_commit(self.commit, signing_fn)?; let commit = self
.mut_repo
.write_commit(self.commit, signing_fn.as_deref_mut())?;
if let Some(rewrite_source_id) = rewrite_source_id { if let Some(rewrite_source_id) = rewrite_source_id {
self.mut_repo self.mut_repo
.record_rewritten_commit(rewrite_source_id, commit.id().clone()) .record_rewritten_commit(rewrite_source_id, commit.id().clone())

View file

@ -904,7 +904,7 @@ impl Backend for GitBackend {
fn write_commit( fn write_commit(
&self, &self,
mut contents: Commit, mut contents: Commit,
mut sign_with: Option<SigningFn>, mut sign_with: Option<&mut SigningFn>,
) -> BackendResult<(CommitId, Commit)> { ) -> BackendResult<(CommitId, Commit)> {
assert!(contents.secure_sig.is_none(), "commit.secure_sig was set"); assert!(contents.secure_sig.is_none(), "commit.secure_sig was set");
@ -1668,12 +1668,14 @@ mod tests {
secure_sig: None, secure_sig: None,
}; };
let signer = Box::new(|data: &_| { let mut signer = |data: &_| {
let hash: String = blake2b_hash(data).encode_hex(); let hash: String = blake2b_hash(data).encode_hex();
Ok(format!("test sig\n\n\nhash={hash}").into_bytes()) Ok(format!("test sig\n\n\nhash={hash}").into_bytes())
}); };
let (id, commit) = backend.write_commit(commit, Some(signer)).unwrap(); let (id, commit) = backend
.write_commit(commit, Some(&mut signer as &mut SigningFn))
.unwrap();
let git_repo = backend.git_repo(); let git_repo = backend.git_repo();
let obj = git_repo.find_object(id.as_bytes()).unwrap(); let obj = git_repo.find_object(id.as_bytes()).unwrap();

View file

@ -267,7 +267,7 @@ impl Backend for LocalBackend {
fn write_commit( fn write_commit(
&self, &self,
mut commit: Commit, mut commit: Commit,
sign_with: Option<SigningFn>, sign_with: Option<&mut SigningFn>,
) -> BackendResult<(CommitId, Commit)> { ) -> BackendResult<(CommitId, Commit)> {
assert!(commit.secure_sig.is_none(), "commit.secure_sig was set"); assert!(commit.secure_sig.is_none(), "commit.secure_sig was set");
@ -279,7 +279,7 @@ impl Backend for LocalBackend {
let temp_file = NamedTempFile::new_in(&self.path).map_err(to_other_err)?; let temp_file = NamedTempFile::new_in(&self.path).map_err(to_other_err)?;
let mut proto = commit_to_proto(&commit); let mut proto = commit_to_proto(&commit);
if let Some(mut sign) = sign_with { if let Some(sign) = sign_with {
let data = proto.encode_to_vec(); let data = proto.encode_to_vec();
let sig = sign(&data).map_err(to_other_err)?; let sig = sign(&data).map_err(to_other_err)?;
proto.secure_sig = Some(sig.clone()); proto.secure_sig = Some(sig.clone());

View file

@ -801,7 +801,7 @@ impl MutableRepo {
pub fn write_commit( pub fn write_commit(
&mut self, &mut self,
commit: backend::Commit, commit: backend::Commit,
sign_with: Option<SigningFn>, sign_with: Option<&mut SigningFn>,
) -> BackendResult<Commit> { ) -> BackendResult<Commit> {
let commit = self.store().write_commit(commit, sign_with)?; let commit = self.store().write_commit(commit, sign_with)?;
self.add_head(&commit); self.add_head(&commit);

View file

@ -138,7 +138,7 @@ impl Store {
pub fn write_commit( pub fn write_commit(
self: &Arc<Self>, self: &Arc<Self>,
commit: backend::Commit, commit: backend::Commit,
sign_with: Option<SigningFn>, sign_with: Option<&mut SigningFn>,
) -> BackendResult<Commit> { ) -> BackendResult<Commit> {
assert!(!commit.parents.is_empty()); assert!(!commit.parents.is_empty());

View file

@ -276,7 +276,7 @@ impl Backend for TestBackend {
fn write_commit( fn write_commit(
&self, &self,
mut contents: Commit, mut contents: Commit,
mut sign_with: Option<SigningFn>, mut sign_with: Option<&mut SigningFn>,
) -> BackendResult<(CommitId, Commit)> { ) -> BackendResult<(CommitId, Commit)> {
assert!(contents.secure_sig.is_none(), "commit.secure_sig was set"); assert!(contents.secure_sig.is_none(), "commit.secure_sig was set");