Skip to content

Commit

Permalink
support reading the fetch algorithm from configuration
Browse files Browse the repository at this point in the history
  • Loading branch information
Byron committed May 11, 2023
1 parent 4f879bf commit 33b7770
Show file tree
Hide file tree
Showing 10 changed files with 156 additions and 26 deletions.
9 changes: 6 additions & 3 deletions gix/src/config/tree/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ pub(crate) mod root {
pub const DIFF: sections::Diff = sections::Diff;
/// The `extensions` section.
pub const EXTENSIONS: sections::Extensions = sections::Extensions;
/// The `fetch` section.
pub const FETCH: sections::Fetch = sections::Fetch;
/// The `gitoxide` section.
pub const GITOXIDE: sections::Gitoxide = sections::Gitoxide;
/// The `http` section.
Expand Down Expand Up @@ -69,6 +71,7 @@ pub(crate) mod root {
&Self::CREDENTIAL,
&Self::DIFF,
&Self::EXTENSIONS,
&Self::FETCH,
&Self::GITOXIDE,
&Self::HTTP,
&Self::INDEX,
Expand All @@ -87,9 +90,9 @@ pub(crate) mod root {

mod sections;
pub use sections::{
branch, checkout, core, credential, diff, extensions, gitoxide, http, index, protocol, remote, ssh, Author, Branch,
Checkout, Clone, Committer, Core, Credential, Diff, Extensions, Gitoxide, Http, Index, Init, Pack, Protocol,
Remote, Safe, Ssh, Url, User,
branch, checkout, core, credential, diff, extensions, fetch, gitoxide, http, index, protocol, remote, ssh, Author,
Branch, Checkout, Clone, Committer, Core, Credential, Diff, Extensions, Fetch, Gitoxide, Http, Index, Init, Pack,
Protocol, Remote, Safe, Ssh, Url, User,
};

/// Generic value implementations for static instantiation.
Expand Down
67 changes: 67 additions & 0 deletions gix/src/config/tree/sections/fetch.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
use crate::{
config,
config::tree::{keys, Fetch, Key, Section},
};

impl Fetch {
/// The `fetch.negotiationAlgorithm` key.
pub const NEGOTIATION_ALGORITHM: NegotiationAlgorithm = NegotiationAlgorithm::new_with_validate(
"negotiationAlgorithm",
&config::Tree::FETCH,
validate::NegotiationAlgorithm,
);
}

impl Section for Fetch {
fn name(&self) -> &str {
"fetch"
}

fn keys(&self) -> &[&dyn Key] {
&[&Self::NEGOTIATION_ALGORITHM]
}
}

/// The `fetch.negotiationAlgorithm` key.
pub type NegotiationAlgorithm = keys::Any<validate::NegotiationAlgorithm>;

mod algorithm {
use gix_object::bstr::ByteSlice;
use std::borrow::Cow;

use crate::remote::fetch::negotiate;
use crate::{
bstr::BStr,
config::{key::GenericErrorWithValue, tree::sections::fetch::NegotiationAlgorithm},
};

impl NegotiationAlgorithm {
/// Derive the negotiation algorithm identified by `name`, case-sensitively.
pub fn try_into_negotiation_algorithm(
&'static self,
name: Cow<'_, BStr>,
) -> Result<negotiate::Algorithm, GenericErrorWithValue> {
Ok(match name.as_ref().as_bytes() {
b"noop" => negotiate::Algorithm::Noop,
b"consecutive" | b"default" => negotiate::Algorithm::Consecutive,
b"skipping" => negotiate::Algorithm::Skipping,
_ => return Err(GenericErrorWithValue::from_value(self, name.into_owned())),
})
}
}
}

mod validate {
use crate::{
bstr::BStr,
config::tree::{keys, Fetch},
};

pub struct NegotiationAlgorithm;
impl keys::Validate for NegotiationAlgorithm {
fn validate(&self, value: &BStr) -> Result<(), Box<dyn std::error::Error + Send + Sync + 'static>> {
Fetch::NEGOTIATION_ALGORITHM.try_into_negotiation_algorithm(value.into())?;
Ok(())
}
}
}
5 changes: 5 additions & 0 deletions gix/src/config/tree/sections/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ pub mod diff;
pub struct Extensions;
pub mod extensions;

/// The `fetch` top-level section.
#[derive(Copy, Clone, Default)]
pub struct Fetch;
pub mod fetch;

/// The `gitoxide` top-level section.
#[derive(Copy, Clone, Default)]
pub struct Gitoxide;
Expand Down
2 changes: 2 additions & 0 deletions gix/src/remote/connection/fetch/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ pub enum Error {
RejectShallowRemoteConfig(#[from] config::boolean::Error),
#[error("Receiving objects from shallow remotes is prohibited due to the value of `clone.rejectShallow`")]
RejectShallowRemote,
#[error(transparent)]
NegotiationAlgorithmConfig(#[from] config::key::GenericErrorWithValue),
}

impl gix_protocol::transport::IsSpuriousError for Error {
Expand Down
3 changes: 1 addition & 2 deletions gix/src/remote/connection/fetch/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,7 @@ impl From<ProgressId> for gix_features::progress::Id {
}
}

///
pub mod negotiate;
pub(crate) mod negotiate;

///
pub mod prepare {
Expand Down
15 changes: 6 additions & 9 deletions gix/src/remote/connection/fetch/negotiate.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
use crate::remote::fetch;

/// The way the negotiation is performed
#[derive(Copy, Clone)]
pub(crate) enum Algorithm {
/// Our very own implementation that probably should be replaced by one of the known algorithms soon.
Naive,
}
use crate::remote::fetch::negotiate::Algorithm;

/// The error returned during negotiation.
#[derive(Debug, thiserror::Error)]
Expand All @@ -23,8 +17,8 @@ pub(crate) fn one_round(
algo: Algorithm,
round: usize,
repo: &crate::Repository,
ref_map: &crate::remote::fetch::RefMap,
fetch_tags: crate::remote::fetch::Tags,
ref_map: &fetch::RefMap,
fetch_tags: fetch::Tags,
arguments: &mut gix_protocol::fetch::Arguments,
_previous_response: Option<&gix_protocol::fetch::Response>,
shallow: Option<&fetch::Shallow>,
Expand All @@ -39,6 +33,9 @@ pub(crate) fn one_round(
}

match algo {
Algorithm::Noop | Algorithm::Skipping | Algorithm::Consecutive => {
todo!()
}
Algorithm::Naive => {
assert_eq!(round, 1, "Naive always finishes after the first round, it claims.");
let mut has_missing_tracking_branch = false;
Expand Down
14 changes: 13 additions & 1 deletion gix/src/remote/connection/fetch/receive_pack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ use gix_protocol::{
transport::{client::Transport, packetline::read::ProgressAction},
};

use crate::config::cache::util::ApplyLeniency;
use crate::config::tree::{Fetch, Key};
use crate::remote::fetch::negotiate::Algorithm;
use crate::{
config::tree::Clone,
remote,
Expand Down Expand Up @@ -109,12 +112,21 @@ where
});
}

let algorithm = repo
.config
.resolved
.string_by_key(Fetch::NEGOTIATION_ALGORITHM.logical_name().as_str())
.map(|n| Fetch::NEGOTIATION_ALGORITHM.try_into_negotiation_algorithm(n))
.transpose()
.with_leniency(repo.config.lenient_config)?
.unwrap_or(Algorithm::Naive); // TODO: use the default instead once consecutive is implemented

let reader = 'negotiation: loop {
progress.step();
progress.set_name(format!("negotiate (round {round})"));

let is_done = match negotiate::one_round(
negotiate::Algorithm::Naive,
algorithm,
round,
repo,
&self.ref_map,
Expand Down
31 changes: 26 additions & 5 deletions gix/src/remote/fetch.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,29 @@
///
pub mod negotiate {
/// The way the negotiation is performed.
#[derive(Default, Debug, Copy, Clone, Eq, PartialEq)]
pub enum Algorithm {
/// Do not send any information at all, likely at cost of larger-than-necessary packs.
Noop,
/// Walk over consecutive commits and check each one. This can be costly be assures packs are exactly the size they need to be.
#[default]
Consecutive,
/// Like `Consecutive`, but skips commits to converge faster, at the cost of receiving packs that are larger than they have to be.
Skipping,
/// Our very own implementation that probably should be replaced by one of the known algorithms soon.
Naive,
}

#[cfg(any(feature = "blocking-network-client", feature = "async-network-client"))]
pub use super::super::connection::fetch::negotiate::Error;

#[cfg(any(feature = "blocking-network-client", feature = "async-network-client"))]
pub(crate) use super::super::connection::fetch::negotiate::one_round;
}

#[cfg(any(feature = "blocking-network-client", feature = "async-network-client"))]
pub use super::connection::fetch::{prepare, refs, Error, Outcome, Prepare, ProgressId, RefLogMessage, Status};

/// If `Yes`, don't really make changes but do as much as possible to get an idea of what would be done.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
#[cfg(any(feature = "blocking-network-client", feature = "async-network-client"))]
Expand Down Expand Up @@ -197,8 +223,3 @@ pub struct Mapping {
/// The index into the fetch ref-specs used to produce the mapping, allowing it to be recovered.
pub spec_index: SpecIndex,
}

#[cfg(any(feature = "blocking-network-client", feature = "async-network-client"))]
pub use super::connection::fetch::{
negotiate, prepare, refs, Error, Outcome, Prepare, ProgressId, RefLogMessage, Status,
};
30 changes: 30 additions & 0 deletions gix/tests/config/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,36 @@ mod ssh {
}
}

mod fetch {
use crate::config::tree::bcow;
use gix::config::tree::{Fetch, Key};
use gix::remote::fetch::negotiate::Algorithm;

#[test]
fn algorithm() -> crate::Result {
for (actual, expected) in [
("noop", Algorithm::Noop),
("consecutive", Algorithm::Consecutive),
("skipping", Algorithm::Skipping),
("default", Algorithm::Consecutive), // actually, default can be Skipping of `feature.experimental` is true, but we don't deal with that yet until we implement `skipping`
] {
assert_eq!(
Fetch::NEGOTIATION_ALGORITHM.try_into_negotiation_algorithm(bcow(actual))?,
expected
);
assert!(Fetch::NEGOTIATION_ALGORITHM.validate(actual.into()).is_ok());
}
assert_eq!(
Fetch::NEGOTIATION_ALGORITHM
.try_into_negotiation_algorithm(bcow("foo"))
.unwrap_err()
.to_string(),
"The key \"fetch.negotiationAlgorithm=foo\" was invalid"
);
Ok(())
}
}

mod diff {
use gix::{
config::tree::{Diff, Key},
Expand Down
6 changes: 0 additions & 6 deletions src/plumbing/progress.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,12 +294,6 @@ static GIT_CONFIG: &[Record] = &[
config: "fetch.output",
usage: NotPlanned {reason: "'gix' might support it, but there is no intention on copying the 'git' CLI"},
},
Record {
config: "fetch.negotiationAlgorithm",
usage: Planned {
note: Some("Implements our own 'naive' algorithm, only"),
},
},
Record {
config: "remotes.<group>",
usage: Planned {
Expand Down

0 comments on commit 33b7770

Please sign in to comment.