fix!: ref namespaces are now thread-local (#263)

Previously these were shared in the shared Repo instance, which makes threaded applications impossible to remain deterministic across multiple connections. Now they are local to the thread, which allowed some methods to remove their Result<> as they cannot fail anymore, the reason for this being a breaking change.

Sebastian Thiel committed 4 years ago

Commit fc8e85c1 parent 38c814611 files · +58 −66Browse files

Unified · Split

11 files

git-repository/src/easy/ext/reference.rs +14 −20
git-repository/src/easy/impls.rs +16 −0
git-repository/src/easy/mod.rs +5 −2
git-repository/src/easy/reference/errors.rs +0 −18
git-repository/src/easy/reference/iter.rs +8 −11
git-repository/src/easy/reference/mod.rs +3 −3
git-repository/src/easy/state.rs +3 −6
git-repository/src/lib.rs +4 −1
git-repository/src/path/is.rs +1 −1
git-repository/src/repository.rs +1 −1
git-repository/tests/easy/ext/reference.rs +3 −3

git-repository/src/easy/ext/reference.rs+14−20

@@ -1,7 +1,4 @@
1		−use std::{
2		− convert::TryInto,
3		− ops::{Deref, DerefMut},
4		−};
	1	+use std::convert::TryInto;
5	2
6	3	use git_actor as actor;
7	4	use git_hash::ObjectId;
@@ -60,14 +57,14 @@ pub trait ReferenceAccessExt: easy::Access + Sized { · 52 unmodified lines
60	57
61	58	/// Returns the currently set namespace for references, or `None` if it is not set.
62	59	///
63		− /// Namespaces allow to partition references.
64		− fn namespace(&self) -> Result<Option<git_ref::Namespace>, easy::borrow::repo::Error> {
65		− self.repo().map(\|repo\| repo.deref().refs.namespace.clone())
	60	+ /// Namespaces allow to partition references, and is configured per `Easy`.
	61	+ fn namespace(&self) -> Option<&git_ref::Namespace> {
	62	+ self.state().refs.namespace.as_ref()
66	63	}
67	64
68		− /// Remove the currently set reference namespace and return it.
69		− fn clear_namespace(&mut self) -> Result<Option<git_ref::Namespace>, easy::borrow::repo::Error> {
70		− self.repo_mut().map(\|mut repo\| repo.deref_mut().refs.namespace.take())
	65	+ /// Remove the currently set reference namespace and return it, affecting only this `Easy`.
	66	+ fn clear_namespace(&mut self) -> Option<git_ref::Namespace> {
	67	+ self.state_mut().refs.namespace.take()
71	68	}
72	69
73	70	/// Set the reference namespace to the given value, like `"foo"` or `"foo/bar"`.
@@ -76,13 +73,13 @@ pub trait ReferenceAccessExt: easy::Access + Sized { · 2 unmodified lines
76	73	fn set_namespace<'a, Name, E>(
77	74	&mut self,
78	75	namespace: Name,
79		− ) -> Result<Option<git_ref::Namespace>, easy::reference::namespace::set::Error>
	76	+ ) -> Result<Option<git_ref::Namespace>, git_validate::refname::Error>
80	77	where
81	78	Name: TryInto<PartialNameRef<'a>, Error = E>,
82	79	git_validate::refname::Error: From<E>,
83	80	{
84	81	let namespace = git_ref::namespace::expand(namespace)?;
85		− Ok(self.repo_mut()?.deref_mut().refs.namespace.replace(namespace))
	82	+ Ok(self.state_mut().refs.namespace.replace(namespace))
86	83	}
87	84
88	85	// TODO: more tests or usage
@@ -166,8 +163,8 @@ pub trait ReferenceAccessExt: easy::Access + Sized { · 77 unmodified lines
166	163	&committer_storage
167	164	}
168	165	};
169		− let repo = self.repo()?;
170		− repo.refs
	166	+ self.state()
	167	+ .refs
171	168	.transaction()
172	169	.prepare(edits, lock_mode)?
173	170	.commit(committer)
@@ -213,10 +210,8 @@ pub trait ReferenceAccessExt: easy::Access + Sized { · 39 unmodified lines
213	210	/// references.
214	211	fn references(&self) -> Result<easy::reference::iter::Platform<'_, Self>, easy::reference::iter::Error> {
215	212	let state = self.state();
216		− let repo = self.repo()?;
217		− let packed_refs = state.assure_packed_refs_uptodate(&repo.refs)?;
	213	+ let packed_refs = state.assure_packed_refs_uptodate()?;
218	214	Ok(easy::reference::iter::Platform {
219		− repo,
220	215	packed_refs,
221	216	access: self,
222	217	})
@@ -232,10 +227,9 @@ pub trait ReferenceAccessExt: easy::Access + Sized { · 9 unmodified lines
232	227	git_ref::file::find::Error: From<E>,
233	228	{
234	229	let state = self.state();
235		− let repo = self.repo()?;
236		− match repo
	230	+ match state
237	231	.refs
238		− .try_find(name, state.assure_packed_refs_uptodate(&repo.refs)?.buffer.as_ref())
	232	+ .try_find(name, state.assure_packed_refs_uptodate()?.buffer.as_ref())
239	233	{
240	234	Ok(r) => match r {
241	235	Some(r) => Ok(Some(Reference::from_ref(r, self))),

git-repository/src/easy/impls.rs+16−0

@@ -103,6 +103,10 @@ impl<'repo> easy::Access for EasyShared<'repo> { · 102 unmodified lines
103	103	fn state(&self) -> &easy::State {
104	104	&self.state
105	105	}
	106	+
	107	+ fn state_mut(&mut self) -> &mut easy::State {
	108	+ &mut self.state
	109	+ }
106	110	}
107	111
108	112	impl easy::Access for Easy {
@@ -121,6 +125,10 @@ impl easy::Access for Easy { · 12 unmodified lines
121	125	fn state(&self) -> &easy::State {
122	126	&self.state
123	127	}
	128	+
	129	+ fn state_mut(&mut self) -> &mut easy::State {
	130	+ &mut self.state
	131	+ }
124	132	}
125	133
126	134	impl easy::Access for EasyArc {
@@ -136,6 +144,10 @@ impl easy::Access for EasyArc { · 9 unmodified lines
136	144	fn state(&self) -> &easy::State {
137	145	&self.state
138	146	}
	147	+
	148	+ fn state_mut(&mut self) -> &mut easy::State {
	149	+ &mut self.state
	150	+ }
139	151	}
140	152
141	153	impl easy::Access for EasyArcExclusive {
@@ -151,4 +163,8 @@ impl easy::Access for EasyArcExclusive { · 9 unmodified lines
151	163	fn state(&self) -> &easy::State {
152	164	&self.state
153	165	}
	166	+
	167	+ fn state_mut(&mut self) -> &mut easy::State {
	168	+ &mut self.state
	169	+ }
154	170	}

git-repository/src/easy/mod.rs+5−2

@@ -114,7 +114,7 @@ pub struct State { · 113 unmodified lines
114	114	/// As the packed-buffer may hold onto a memory map, so ideally this State is freed after use instead of keeping it around
115	115	/// for too long. At least `packed_refs` is lazily initialized.
116	116	packed_refs: RefCell<reference::packed::ModifieablePackedRefsBuffer>,
117		− refs: git_ref::file::Store,
	117	+ refs: crate::RefStore,
118	118	pack_cache: RefCell<PackCache>,
119	119	object_cache: RefCell<Option<object::cache::MemoryCappedHashmap>>,
120	120	buf: RefCell<Vec<u8>>,
@@ -148,6 +148,9 @@ pub trait Access { · 27 unmodified lines
148	148	/// relevant for long-running applications that make changes to the repository.
149	149	fn repo_mut(&self) -> borrow::repo::Result<Self::RepoRefMut>;
150	150
151		− /// Return a shared borrow of the repository state, with support for interior mutability.
	151	+ /// Return a shared borrow of the repository state.
152	152	fn state(&self) -> &State;
	153	+
	154	+ /// Return a shared borrow of the repository state.
	155	+ fn state_mut(&mut self) -> &mut State;
153	156	}

git-repository/src/easy/reference/errors.rs+0−18

@@ -1,21 +1,3 @@
1		−///
2		−pub mod namespace {
3		− ///
4		− pub mod set {
5		− use crate::easy;
6		−
7		− /// The error returned by [ReferenceAccessExt::set_namespace(…)][easy::ext::ReferenceAccessExt::set_namespace()].
8		− #[derive(Debug, thiserror::Error)]
9		− #[allow(missing_docs)]
10		− pub enum Error {
11		− #[error(transparent)]
12		− BorrowRepoMut(#[from] easy::borrow::repo::Error),
13		− #[error(transparent)]
14		− NameValidation(#[from] git_validate::refname::Error),
15		− }
16		− }
17		−}
18		−
19	1	///
20	2	pub mod edit {
21	3	use crate::easy;

git-repository/src/easy/reference/iter.rs+8−11

@@ -1,9 +1,5 @@
1	1	//!
2		−use std::{
3		− cell::Ref,
4		− ops::{Deref, DerefMut},
5		− path::Path,
6		−};
	2	+use std::{cell::Ref, ops::DerefMut, path::Path};
7	3
8	4	use git_odb::Find;
9	5	use git_ref::file::ReferenceExt;
@@ -16,7 +12,6 @@ pub struct Platform<'r, A> · 6 unmodified lines
16	12	where
17	13	A: easy::Access + Sized,
18	14	{
19		− pub(crate) repo: A::RepoRef,
20	15	pub(crate) packed_refs: Ref<'r, easy::reference::packed::ModifieablePackedRefsBuffer>,
21	16	pub(crate) access: &'r A,
22	17	}
@@ -38,9 +33,8 @@ where · 15 unmodified lines
38	33	/// Even broken or otherwise unparsible or inaccessible references are returned and have to be handled by the caller on a
39	34	/// case by case basis.
40	35	pub fn all(&self) -> Result<Iter<'_, A>, init::Error> {
41		− let repo = self.repo.deref();
42	36	Ok(Iter {
43		− inner: repo.refs.iter(self.packed_refs.buffer.as_ref())?,
	37	+ inner: self.access.state().refs.iter(self.packed_refs.buffer.as_ref())?,
44	38	packed_refs: self.packed_refs.buffer.as_ref(),
45	39	peel: false,
46	40	access: self.access,
@@ -51,9 +45,12 @@ where · 4 unmodified lines
51	45	///
52	46	/// These are of the form `refs/heads` or `refs/remotes/origin`, and must not contain relative paths components like `.` or `..`.
53	47	pub fn prefixed(&self, prefix: impl AsRef<Path>) -> Result<Iter<'_, A>, init::Error> {
54		− let repo = self.repo.deref();
55	48	Ok(Iter {
56		− inner: repo.refs.iter_prefixed(self.packed_refs.buffer.as_ref(), prefix)?,
	49	+ inner: self
	50	+ .access
	51	+ .state()
	52	+ .refs
	53	+ .iter_prefixed(self.packed_refs.buffer.as_ref(), prefix)?,
57	54	packed_refs: self.packed_refs.buffer.as_ref(),
58	55	peel: false,
59	56	access: self.access,
@@ -90,7 +87,7 @@ where · 30 unmodified lines
90	87	let repo = self.access.repo()?;
91	88	let state = self.access.state();
92	89	let mut pack_cache = state.try_borrow_mut_pack_cache()?;
93		− r.peel_to_id_in_place(&repo.refs, self.packed_refs, \|oid, buf\| {
	90	+ r.peel_to_id_in_place(&state.refs, self.packed_refs, \|oid, buf\| {
94	91	repo.odb
95	92	.try_find(oid, buf, pack_cache.deref_mut())
96	93	.map(\|po\| po.map(\|o\| (o.kind, o.data)))

git-repository/src/easy/reference/mod.rs+3−3

@@ -14,7 +14,7 @@ pub mod iter; · 13 unmodified lines
14	14	mod errors;
15	15	use std::{borrow::Borrow, cell::RefMut, marker::PhantomData};
16	16
17		−pub use errors::{edit, find, namespace, peel};
	17	+pub use errors::{edit, find, peel};
18	18
19	19	use crate::ext::ObjectIdExt;
20	20
@@ -78,8 +78,8 @@ where · 57 unmodified lines
78	78	let state = self.access.state();
79	79	let mut pack_cache = state.try_borrow_mut_pack_cache()?;
80	80	let oid = self.inner.peel_to_id_in_place(
81		− &repo.refs,
82		− state.assure_packed_refs_uptodate(&repo.refs)?.buffer.as_ref(),
	81	+ &state.refs,
	82	+ state.assure_packed_refs_uptodate()?.buffer.as_ref(),
83	83	\|oid, buf\| {
84	84	repo.odb
85	85	.try_find(oid, buf, pack_cache.deref_mut())

git-repository/src/easy/state.rs+3−6

@@ -1,8 +1,6 @@
1	1	//!
2	2	use std::cell::{Ref, RefCell, RefMut};
3	3
4		−use git_ref::file;
5		−
6	4	use crate::{easy, easy::borrow, Repository};
7	5
8	6	impl Clone for easy::State {
@@ -12,8 +10,8 @@ impl Clone for easy::State { · 3 unmodified lines
12	10	}
13	11
14	12	// TODO: Expand this as needed as more higher-level stores exist (i.e. odb)
15		−impl From<git_ref::file::Store> for easy::State {
16		− fn from(refs: git_ref::file::Store) -> Self {
	13	+impl From<crate::RefStore> for easy::State {
	14	+ fn from(refs: crate::RefStore) -> Self {
17	15	easy::State {
18	16	packed_refs: RefCell::new(Default::default()),
19	17	#[cfg(not(feature = "max-performance"))]
@@ -36,10 +34,9 @@ impl From<&Repository> for easy::State { · 16 unmodified lines
36	34	impl easy::State {
37	35	pub(crate) fn assure_packed_refs_uptodate(
38	36	&self,
39		− file: &file::Store,
40	37	) -> Result<Ref<'_, easy::reference::packed::ModifieablePackedRefsBuffer>, easy::reference::packed::Error> {
41	38	let mut packed_refs = self.packed_refs.try_borrow_mut()?;
42		− packed_refs.assure_packed_refs_uptodate(file)?;
	39	+ packed_refs.assure_packed_refs_uptodate(&self.refs)?;
43	40	drop(packed_refs);
44	41	Ok(self.packed_refs.try_borrow()?)
45	42	}

git-repository/src/lib.rs+4−1

@@ -154,6 +154,9 @@ pub mod path; · 153 unmodified lines
154	154	mod repository;
155	155	pub use repository::{discover, init, open};
156	156
	157	+/// The standard type for a store to handle git references.
	158	+pub type RefStore = git_ref::file::Store;
	159	+
157	160	/// A repository path which either points to a work tree or the `.git` repository itself.
158	161	#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
159	162	pub enum Path {
@@ -173,7 +176,7 @@ pub enum Path { · 13 unmodified lines
173	176	/// Namely, this is an object database, a reference database to point to objects.
174	177	pub struct Repository {
175	178	/// A store for references to point at objects
176		− pub refs: git_ref::file::Store,
	179	+ pub refs: RefStore,
177	180	/// A store for objects that contain data
178	181	#[cfg(feature = "unstable")]
179	182	pub odb: git_odb::linked::Store,

git-repository/src/path/is.rs+1−1

@@ -35,7 +35,7 @@ pub fn git(git_dir: impl AsRef<Path>) -> Result<crate::Kind, Error> { · 34 unmodified lines
35	35	let dot_git = git_dir.as_ref();
36	36
37	37	{
38		− let refs = git_ref::file::Store::at(&dot_git, Default::default());
	38	+ let refs = crate::RefStore::at(&dot_git, Default::default());
39	39	let head = refs.find_loose("HEAD")?;
40	40	if head.name.as_bstr() != "HEAD" {
41	41	return Err(Error::MisplacedHead {

git-repository/src/repository.rs+1−1

@@ -101,7 +101,7 @@ pub mod open { · 100 unmodified lines
101	101
102	102	Ok(crate::Repository {
103	103	odb: git_odb::linked::Store::at(git_dir.join("objects"))?,
104		− refs: git_ref::file::Store::at(
	104	+ refs: crate::RefStore::at(
105	105	git_dir,
106	106	if worktree_dir.is_none() {
107	107	git_ref::file::WriteReflog::Disable

git-repository/tests/easy/ext/reference.rs+3−3

@@ -14,7 +14,7 @@ mod set_namespace { · 13 unmodified lines
14	14	15,
15	15	"there are plenty of references in the default namespace"
16	16	);
17		− assert!(repo.namespace()?.is_none(), "no namespace is set initially");
	17	+ assert!(repo.namespace().is_none(), "no namespace is set initially");
18	18	assert!(repo.set_namespace("foo")?.is_none(), "there is no previous namespace");
19	19
20	20	assert_eq!(
@@ -67,9 +67,9 @@ mod set_namespace { · 46 unmodified lines
67	67	"namespaces are transparent"
68	68	);
69	69
70		− let previous_ns = repo.clear_namespace()?.expect("namespace set");
	70	+ let previous_ns = repo.clear_namespace().expect("namespace set");
71	71	assert_eq!(previous_ns.as_bstr(), "refs/namespaces/foo/");
72		− assert!(repo.clear_namespace()?.is_none(), "it doesn't invent namespaces");
	72	+ assert!(repo.clear_namespace().is_none(), "it doesn't invent namespaces");
73	73
74	74	assert_eq!(
75	75	repo.references()?.all()?.count(),