Skip to content

Commit

Permalink
feat!: respect the core.commitGraph option.
Browse files Browse the repository at this point in the history
Previously, we would always use the commitgraph when available, but now we only do so
if the `core.commitGraph` option is set.
  • Loading branch information
Byron committed Jun 11, 2023
1 parent 7aaaebf commit 574e0f4
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 9 deletions.
13 changes: 13 additions & 0 deletions gix/src/config/cache/access.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::{borrow::Cow, path::PathBuf, time::Duration};
use gix_attributes::Source;
use gix_lock::acquire::Fail;

use crate::config::cache::util::ApplyLeniencyDefaultValue;
use crate::{
bstr::BStr,
config,
Expand Down Expand Up @@ -69,6 +70,18 @@ impl Cache {
.get_or_try_init(|| remote::url::SchemePermission::from_config(&self.resolved, self.filter_config_section))
}

pub(crate) fn may_use_commit_graph(&self) -> Result<bool, config::boolean::Error> {
const DEFAULT: bool = true;
self.resolved
.boolean_by_key("core.commitGraph")
.map(|res| {
Core::COMMIT_GRAPH
.enrich_error(res)
.with_lenient_default_value(self.lenient_config, DEFAULT)
})
.unwrap_or(Ok(DEFAULT))
}

pub(crate) fn diff_renames(
&self,
) -> Result<Option<crate::object::tree::diff::Rewrites>, crate::object::tree::diff::rewrites::Error> {
Expand Down
14 changes: 14 additions & 0 deletions gix/src/config/cache/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ pub trait ApplyLeniencyDefault {
fn with_lenient_default(self, is_lenient: bool) -> Self;
}

pub trait ApplyLeniencyDefaultValue<T> {
fn with_lenient_default_value(self, is_lenient: bool, default: T) -> Self;
}

impl<T, E> ApplyLeniency for Result<Option<T>, E> {
fn with_leniency(self, is_lenient: bool) -> Self {
match self {
Expand All @@ -146,3 +150,13 @@ where
}
}
}

impl<T, E> ApplyLeniencyDefaultValue<T> for Result<T, E> {
fn with_lenient_default_value(self, is_lenient: bool, default: T) -> Self {
match self {
Ok(v) => Ok(v),
Err(_) if is_lenient => Ok(default),
Err(err) => Err(err),
}
}
}
3 changes: 3 additions & 0 deletions gix/src/config/tree/sections/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ impl Core {
/// The `core.useReplaceRefs` key.
pub const USE_REPLACE_REFS: keys::Boolean = keys::Boolean::new_boolean("useReplaceRefs", &config::Tree::CORE)
.with_environment_override("GIT_NO_REPLACE_OBJECTS");
/// The `core.commitGraph` key.
pub const COMMIT_GRAPH: keys::Boolean = keys::Boolean::new_boolean("commitGraph", &config::Tree::CORE);
}

impl Section for Core {
Expand Down Expand Up @@ -96,6 +98,7 @@ impl Section for Core {
&Self::ATTRIBUTES_FILE,
&Self::SSH_COMMAND,
&Self::USE_REPLACE_REFS,
&Self::COMMIT_GRAPH,
]
}
}
Expand Down
9 changes: 8 additions & 1 deletion gix/src/repository/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ impl crate::Repository {
/// Note that the commitgraph will be used if it is present and readable, but it won't be an error if it is corrupted. In that case,
/// it will just not be used.
///
/// Note that a commitgraph is only allowed to be used if `core.commitGraph` is true (the default), and that configuration errors are
/// ignored as well.
///
/// ### Performance
///
/// Note that the [Graph][gix_revision::Graph] can be sensitive to various object database settings that may affect the performance
Expand All @@ -18,7 +21,11 @@ impl crate::Repository {
.try_find(id, buf)
.map(|r| r.and_then(|d| d.try_into_commit_iter()))
},
gix_commitgraph::at(self.objects.store_ref().path().join("info")).ok(),
self.config
.may_use_commit_graph()
.unwrap_or(true)
.then(|| gix_commitgraph::at(self.objects.store_ref().path().join("info")).ok())
.flatten(),
)
}

Expand Down
23 changes: 15 additions & 8 deletions gix/src/revision/walk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ pub enum Error {
AncestorIter(#[from] gix_traverse::commit::ancestors::Error),
#[error(transparent)]
ShallowCommits(#[from] crate::shallow::open::Error),
#[error(transparent)]
ConfigBoolean(#[from] crate::config::boolean::Error),
}

/// Information about a commit that we obtained naturally as part of the iteration.
Expand Down Expand Up @@ -92,7 +94,7 @@ pub struct Platform<'repo> {
pub(crate) tips: Vec<ObjectId>,
pub(crate) sorting: gix_traverse::commit::Sorting,
pub(crate) parents: gix_traverse::commit::Parents,
pub(crate) use_commit_graph: bool,
pub(crate) use_commit_graph: Option<bool>,
pub(crate) commit_graph: Option<gix_commitgraph::Graph>,
}

Expand All @@ -103,7 +105,7 @@ impl<'repo> Platform<'repo> {
tips: tips.into_iter().map(Into::into).collect(),
sorting: Default::default(),
parents: Default::default(),
use_commit_graph: true,
use_commit_graph: None,
commit_graph: None,
}
}
Expand All @@ -123,12 +125,12 @@ impl<'repo> Platform<'repo> {
self
}

/// Allow using the commitgraph, if present, if `toggle` is `true`, or disallow it with `false`.
/// Allow using the commitgraph, if present, if `toggle` is `true`, or disallow it with `false`. Set it to `None` to leave
/// control over this to the configuration of `core.commitGraph` (the default).
///
/// Note that the commitgraph will be used by default, and that errors just lead to falling back to the object database,
/// it's treated as a cache.
pub fn use_commit_graph(mut self, toggle: bool) -> Self {
self.use_commit_graph = toggle;
/// Errors when loading the graph lead to falling back to the object database, it's treated as optional cache.
pub fn use_commit_graph(mut self, toggle: impl Into<Option<bool>>) -> Self {
self.use_commit_graph = toggle.into();
self
}

Expand Down Expand Up @@ -201,7 +203,12 @@ impl<'repo> Platform<'repo> {
)
.sorting(sorting)?
.parents(parents)
.commit_graph(commit_graph.or(use_commit_graph.then(|| self.repo.commit_graph().ok()).flatten())),
.commit_graph(
commit_graph.or(use_commit_graph
.map_or_else(|| self.repo.config.may_use_commit_graph(), Ok)?
.then(|| self.repo.commit_graph().ok())
.flatten()),
),
),
})
}
Expand Down

0 comments on commit 574e0f4

Please sign in to comment.