change!: remote lock_mode from all methods dealing with reference e…

…dits. (#450)

It is now read from `core.filesRefLockTimeout` accordingly.

git-repository/src/config/cache/access.rs

@@ -0,0 +1,60 @@
+use crate::config::Cache;
+use crate::{remote, repository::identity};
+use git_lock::acquire::Fail;
+use std::convert::TryInto;
+use std::time::Duration;
+
+/// Access
+impl Cache {
+    pub(crate) fn personas(&self) -> &identity::Personas {
+        self.personas
+            .get_or_init(|| identity::Personas::from_config_and_env(&self.resolved, self.git_prefix))
+    }
+
+    pub(crate) fn url_rewrite(&self) -> &remote::url::Rewrite {
+        self.url_rewrite
+            .get_or_init(|| remote::url::Rewrite::from_config(&self.resolved, self.filter_config_section))
+    }
+
+    #[cfg(any(feature = "blocking-network-client", feature = "async-network-client"))]
+    pub(crate) fn url_scheme(
+        &self,
+    ) -> Result<&remote::url::SchemePermission, remote::url::scheme_permission::init::Error> {
+        self.url_scheme.get_or_try_init(|| {
+            remote::url::SchemePermission::from_config(&self.resolved, self.git_prefix, self.filter_config_section)
+        })
+    }
+
+    /// Returns (file-timeout, pack-refs timeout)
+    pub(crate) fn lock_timeout(
+        &self,
+    ) -> Result<(git_lock::acquire::Fail, git_lock::acquire::Fail), git_config::value::Error> {
+        enum Kind {
+            RefFiles,
+            RefPackFile,
+        }
+        let mut out: [git_lock::acquire::Fail; 2] = Default::default();
+
+        for (idx, kind) in [Kind::RefFiles, Kind::RefPackFile].iter().enumerate() {
+            let (key, default_ms) = match kind {
+                Kind::RefFiles => ("filesRefLockTimeout", 100),
+                Kind::RefPackFile => ("packedRefsTimeout", 1000),
+            };
+            let mk_default = || Fail::AfterDurationWithBackoff(Duration::from_millis(default_ms));
+            let mut fnp = self.filter_config_section;
+
+            let lock_mode = match self.resolved.integer_filter("core", None, key, &mut fnp) {
+                Some(Ok(val)) if val < 0 => Fail::AfterDurationWithBackoff(Duration::from_secs(u64::MAX)),
+                Some(Ok(val)) if val == 0 => Fail::Immediately,
+                Some(Ok(val)) => Fail::AfterDurationWithBackoff(Duration::from_millis(
+                    val.try_into().expect("i64 can be repsented by u64"),
+                )),
+                Some(Err(_)) if self.lenient_config => mk_default(),
+                Some(Err(err)) => return Err(err),
+                None => mk_default(),
+            };
+            out[idx] = lock_mode;
+        }
+        Ok((out[0], out[1]))
+    }
+}

git-repository/src/config/cache/init.rs

@@ -147,6 +147,7 @@ impl Cache {
             excludes_file,
             xdg_config_home_env,
             home_env,
+            lenient_config,
             personas: Default::default(),
             url_rewrite: Default::default(),
             #[cfg(any(feature = "blocking-network-client", feature = "async-network-client"))]

git-repository/src/config/cache/mod.rs

@@ -1,5 +1,4 @@
 use super::{Cache, Error};
-use crate::{remote, repository::identity};
 
 mod incubate;
 pub(crate) use incubate::StageOne;
@@ -12,27 +11,7 @@ impl std::fmt::Debug for Cache {
     }
 }
 
-/// Access
-impl Cache {
-    pub(crate) fn personas(&self) -> &identity::Personas {
-        self.personas
-            .get_or_init(|| identity::Personas::from_config_and_env(&self.resolved, self.git_prefix))
-    }
-
-    pub(crate) fn url_rewrite(&self) -> &remote::url::Rewrite {
-        self.url_rewrite
-            .get_or_init(|| remote::url::Rewrite::from_config(&self.resolved, self.filter_config_section))
-    }
-
-    #[cfg(any(feature = "blocking-network-client", feature = "async-network-client"))]
-    pub(crate) fn url_scheme(
-        &self,
-    ) -> Result<&remote::url::SchemePermission, remote::url::scheme_permission::init::Error> {
-        self.url_scheme.get_or_try_init(|| {
-            remote::url::SchemePermission::from_config(&self.resolved, self.git_prefix, self.filter_config_section)
-        })
-    }
-}
+mod access;
 
 mod util;
 pub(crate) use util::interpolate_context;

git-repository/src/config/mod.rs

@@ -14,14 +14,15 @@ pub struct Snapshot<'repo> {
     pub(crate) repo: &'repo Repository,
 }
 
-/// A platform to access configuration values and modify them in memory, while making them available when this platform is dropped.
+/// A platform to access configuration values and modify them in memory, while making them available when this platform is dropped
+/// as form of auto-commit.
 /// Note that the values will only affect this instance of the parent repository, and not other clones that may exist.
 ///
 /// Note that these values won't update even if the underlying file(s) change.
 ///
 /// Use [`forget()`][Self::forget()] to not apply any of the changes.
 // TODO: make it possible to load snapshots with reloading via .config() and write mutated snapshots back to disk which should be the way
-//       to affect all instances of a repo.
+//       to affect all instances of a repo, probably via `config_mut()` and `config_mut_at()`.
 pub struct SnapshotMut<'repo> {
     pub(crate) repo: &'repo mut Repository,
     pub(crate) config: git_config::File<'static>,
@@ -86,6 +87,9 @@ pub(crate) struct Cache {
     pub ignore_case: bool,
     /// The path to the user-level excludes file to ignore certain files in the worktree.
     pub excludes_file: Option<std::path::PathBuf>,
+    /// If true, we should default what's possible if something is misconfigured, on case by case basis, to be more resilient.
+    /// Also available in options! Keep in sync!
+    pub lenient_config: bool,
     /// Define how we can use values obtained with `xdg_config(…)` and its `XDG_CONFIG_HOME` variable.
     xdg_config_home_env: git_sec::Permission,
     /// Define how we can use values obtained with `xdg_config(…)`. and its `HOME` variable.

git-repository/src/reference/edits.rs

@@ -55,35 +55,23 @@ pub mod delete {
 
     use crate::Reference;
 
-    mod error {
-        /// The error returned by [`Reference::delete()`][super::Reference::delete()].
-        #[derive(Debug, thiserror::Error)]
-        #[allow(missing_docs)]
-        pub enum Error {
-            #[error(transparent)]
-            ReferenceEdit(#[from] crate::reference::edit::Error),
-        }
-    }
-    pub use error::Error;
-    use git_lock::acquire::Fail;
-
     impl<'repo> Reference<'repo> {
         /// Delete this reference or fail if it was changed since last observed.
         /// Note that this instance remains available in memory but probably shouldn't be used anymore.
-        pub fn delete(&self) -> Result<(), Error> {
-            self.repo.edit_reference(
-                RefEdit {
-                    change: Change::Delete {
-                        expected: PreviousValue::MustExistAndMatch(self.inner.target.clone()),
-                        log: RefLog::AndReference,
+        pub fn delete(&self) -> Result<(), crate::reference::edit::Error> {
+            self.repo
+                .edit_reference(
+                    RefEdit {
+                        change: Change::Delete {
+                            expected: PreviousValue::MustExistAndMatch(self.inner.target.clone()),
+                            log: RefLog::AndReference,
+                        },
+                        name: self.inner.name.clone(),
+                        deref: false,
                     },
-                    name: self.inner.name.clone(),
-                    deref: false,
-                },
-                Fail::Immediately,
-                self.repo.committer_or_default(),
-            )?;
-            Ok(())
+                    self.repo.committer_or_default(),
+                )
+                .map(|_| ())
         }
     }
 }

git-repository/src/reference/errors.rs

@@ -11,6 +11,8 @@ pub mod edit {
         FileTransactionCommit(#[from] git_ref::file::transaction::commit::Error),
         #[error(transparent)]
         NameValidation(#[from] git_validate::reference::name::Error),
+        #[error("Could not interpret core.filesRefLockTimeout or core.packedRefsTimeout, it must be the number in milliseconds to wait for locks or negative to wait forever")]
+        LockTimeoutConfiguration(#[from] git_config::value::Error),
     }
 }
 

git-repository/src/remote/connection/fetch/update_refs/mod.rs

@@ -90,9 +90,7 @@ pub(crate) fn update(
     }
 
     let edits = match dry_run {
-        fetch::DryRun::No => {
-            repo.edit_references(edits, git_lock::acquire::Fail::Immediately, repo.committer_or_default())?
-        }
+        fetch::DryRun::No => repo.edit_references(edits, repo.committer_or_default())?,
         fetch::DryRun::Yes => edits,
     };
 

git-repository/src/repository/object.rs

@@ -195,7 +195,6 @@ impl crate::Repository {
                 name: reference,
                 deref: true,
             },
-            git_lock::acquire::Fail::Immediately,
             commit.committer.to_ref(),
         )?;
         Ok(commit_id)

git-repository/src/repository/reference.rs

@@ -2,16 +2,13 @@ use std::convert::TryInto;
 
 use git_actor as actor;
 use git_hash::ObjectId;
-use git_lock as lock;
 use git_ref::{
     transaction::{Change, LogChange, PreviousValue, RefEdit, RefLog},
     FullName, PartialNameRef, Target,
 };
 
 use crate::{bstr::BString, ext::ReferenceExt, reference, Reference};
 
-const DEFAULT_LOCK_MODE: git_lock::acquire::Fail = git_lock::acquire::Fail::Immediately;
-
 /// Obtain and alter references comfortably
 impl crate::Repository {
     /// Create a lightweight tag with given `name` (and without `refs/tags/` prefix) pointing to the given `target`, and return it as reference.
@@ -35,7 +32,6 @@ impl crate::Repository {
                 name: format!("refs/tags/{}", name.as_ref()).try_into()?,
                 deref: false,
             },
-            DEFAULT_LOCK_MODE,
             self.committer_or_default(),
         )?;
         assert_eq!(edits.len(), 1, "reference splits should ever happen");
@@ -109,7 +105,6 @@ impl crate::Repository {
                 name,
                 deref: false,
             },
-            DEFAULT_LOCK_MODE,
             self.committer_or_default(),
         )?;
         assert_eq!(
@@ -126,33 +121,30 @@ impl crate::Repository {
         .attach(self))
     }
 
-    /// Edit a single reference as described in `edit`, handle locks via `lock_mode` and write reference logs as `log_committer`.
+    /// Edit a single reference as described in `edit`, and write reference logs as `log_committer`.
     ///
     /// One or more `RefEdit`s  are returned - symbolic reference splits can cause more edits to be performed. All edits have the previous
     /// reference values set to the ones encountered at rest after acquiring the respective reference's lock.
     pub fn edit_reference(
         &self,
         edit: RefEdit,
-        lock_mode: lock::acquire::Fail,
         log_committer: actor::SignatureRef<'_>,
     ) -> Result<Vec<RefEdit>, reference::edit::Error> {
-        self.edit_references(Some(edit), lock_mode, log_committer)
+        self.edit_references(Some(edit), log_committer)
     }
 
-    /// Edit one or more references as described by their `edits`, with `lock_mode` deciding on how to handle competing
-    /// transactions. `log_committer` is the name appearing in reference logs.
+    /// Edit one or more references as described by their `edits`. `log_committer` is the name appearing in reference logs.
     ///
     /// Returns all reference edits, which might be more than where provided due the splitting of symbolic references, and
     /// whose previous (_old_) values are the ones seen on in storage after the reference was locked.
     pub fn edit_references(
         &self,
         edits: impl IntoIterator<Item = RefEdit>,
-        lock_mode: lock::acquire::Fail,
         log_committer: actor::SignatureRef<'_>,
     ) -> Result<Vec<RefEdit>, reference::edit::Error> {
         self.refs
             .transaction()
-            .prepare(edits, lock_mode)?
+            .prepare(edits, self.config.lock_timeout()?.0)?
             .commit(log_committer)
             .map_err(Into::into)
     }

git-repository/tests/repository/mod.rs

@@ -10,7 +10,7 @@ mod worktree;
 
 #[test]
 fn size_in_memory() {
-    let expected = [760, 800];
+    let expected = [768, 808];
     let actual_size = std::mem::size_of::<Repository>();
     assert!(
         expected.contains(&actual_size),

src/plumbing/progress.rs

@@ -118,7 +118,7 @@ static GIT_CONFIG: &[Record] = &[
     },
     Record {
         config: "core.filesRefLockTimeout",
-        usage: Planned {note: Some("to be cached and used for all ref operations")},
+        usage: InModule {name: "config::cache::access", deviation: None},
     },
     Record {
         config: "core.packedRefsTimeout",