Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix pack receive #1731

Merged
merged 6 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion gitoxide-core/src/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,20 @@ mod impls {
}

#[cfg(any(feature = "async-client", feature = "blocking-client"))]
pub use gix::protocol::transport::connect;
#[gix::protocol::maybe_async::maybe_async]
pub async fn connect<Url, E>(
url: Url,
options: gix::protocol::transport::client::connect::Options,
) -> Result<
gix::protocol::SendFlushOnDrop<Box<dyn gix::protocol::transport::client::Transport + Send>>,
gix::protocol::transport::client::connect::Error,
>
where
Url: TryInto<gix::url::Url, Error = E>,
gix::url::parse::Error: From<E>,
{
Ok(gix::protocol::SendFlushOnDrop::new(
gix::protocol::transport::connect(url, options).await?,
false,
))
}
21 changes: 17 additions & 4 deletions gitoxide-core/src/pack/receive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::pack::receive::protocol::fetch::negotiate;
use crate::OutputFormat;
use gix::config::tree::Key;
use gix::protocol::maybe_async;
use gix::remote::fetch::Error;
use gix::DynNestedProgress;
pub use gix::{
hash::ObjectId,
Expand Down Expand Up @@ -64,7 +65,7 @@ where

let agent = gix::protocol::agent(gix::env::agent());
let mut handshake = gix::protocol::fetch::handshake(
&mut transport,
&mut transport.inner,
gix::protocol::credentials::builtin,
vec![("agent".into(), Some(agent.clone()))],
&mut progress,
Expand All @@ -85,13 +86,22 @@ where
&fetch_refspecs,
gix::protocol::fetch::Context {
handshake: &mut handshake,
transport: &mut transport,
transport: &mut transport.inner,
user_agent: user_agent.clone(),
trace_packetlines,
},
gix::protocol::fetch::refmap::init::Options::default(),
)
.await?;

if refmap.mappings.is_empty() && !refmap.remote_refs.is_empty() {
return Err(Error::NoMapping {
refspecs: refmap.refspecs.clone(),
num_remote_refs: refmap.remote_refs.len(),
}
.into());
}

let mut negotiate = Negotiate { refmap: &refmap };
gix::protocol::fetch(
&mut negotiate,
Expand All @@ -114,7 +124,7 @@ where
&ctx.should_interrupt,
gix::protocol::fetch::Context {
handshake: &mut handshake,
transport: &mut transport,
transport: &mut transport.inner,
user_agent,
trace_packetlines,
},
Expand All @@ -140,10 +150,13 @@ impl gix::protocol::fetch::Negotiate for Negotiate<'_> {
})
}

fn add_wants(&mut self, arguments: &mut Arguments, _remote_ref_target_known: &[bool]) {
fn add_wants(&mut self, arguments: &mut Arguments, _remote_ref_target_known: &[bool]) -> bool {
let mut has_want = false;
for id in self.refmap.mappings.iter().filter_map(|m| m.remote.as_id()) {
arguments.want(id);
has_want = true;
}
has_want
}

fn one_round(
Expand Down
4 changes: 2 additions & 2 deletions gix-protocol/src/fetch/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ pub enum Error {
LockShallowFile(#[from] gix_lock::acquire::Error),
#[error("Receiving objects from shallow remotes is prohibited due to the value of `clone.rejectShallow`")]
RejectShallowRemote,
#[error("Failed to consume the pack sent by the remove")]
ConsumePack(Box<dyn std::error::Error + Send + Sync + 'static>),
#[error("Failed to consume the pack sent by the remote")]
ConsumePack(#[source] Box<dyn std::error::Error + Send + Sync + 'static>),
#[error("Failed to read remaining bytes in stream")]
ReadRemainingBytes(#[source] std::io::Error),
}
Expand Down
15 changes: 11 additions & 4 deletions gix-protocol/src/fetch/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ use std::sync::atomic::{AtomicBool, Ordering};
/// * …update local refs
/// * …end the interaction after the fetch
///
/// Note that the interaction will never be ended, even on error or failure, leaving it up to the caller to do that, maybe
/// **Note that the interaction will never be ended**, even on error or failure, leaving it up to the caller to do that, maybe
/// with the help of [`SendFlushOnDrop`](crate::SendFlushOnDrop) which can wrap `transport`.
/// Generally, the `transport` is left in a state that allows for more commands to be run.
///
/// Return `Ok(None)` if there was nothing to do because all remote refs are at the same state as they are locally, or `Ok(Some(outcome))`
/// to inform about all the changes that were made.
/// Return `Ok(None)` if there was nothing to do because all remote refs are at the same state as they are locally,
/// or there was nothing wanted, or `Ok(Some(outcome))` to inform about all the changes that were made.
#[maybe_async::maybe_async]
pub async fn fetch<P, T, E>(
negotiate: &mut impl Negotiate,
Expand Down Expand Up @@ -91,14 +91,21 @@ where
negotiate::Action::MustNegotiate {
remote_ref_target_known,
} => {
negotiate.add_wants(&mut arguments, remote_ref_target_known);
if !negotiate.add_wants(&mut arguments, remote_ref_target_known) {
return Ok(None);
}
let mut rounds = Vec::new();
let is_stateless = arguments.is_stateless(!transport.connection_persists_across_multiple_requests());
let mut state = negotiate::one_round::State::new(is_stateless);
let mut reader = 'negotiation: loop {
let _round = gix_trace::detail!("negotiate round", round = rounds.len() + 1);
progress.step();
progress.set_name(format!("negotiate (round {})", rounds.len() + 1));
if should_interrupt.load(Ordering::Relaxed) {
return Err(Error::Negotiate(negotiate::Error::NegotiationFailed {
rounds: rounds.len(),
}));
}

let is_done = match negotiate.one_round(&mut state, &mut arguments, previous_response.as_ref()) {
Ok((round, is_done)) => {
Expand Down
9 changes: 8 additions & 1 deletion gix-protocol/src/fetch/negotiate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,17 +284,21 @@ pub fn make_refmapping_ignore_predicate(fetch_tags: Tags, ref_map: &RefMap) -> i
/// * `ref_map` is the state of refs as known on the remote.
/// * `shallow` defines if the history should be shallow.
/// * `mapping_is_ignored` is typically initialized with [`make_refmapping_ignore_predicate`].
///
/// Returns `true` if at least one [want](crate::fetch::Arguments::want()) was added, or `false` otherwise.
/// Note that not adding a single want can make the remote hang, so it's avoided on the client side by ending the fetch operation.
pub fn add_wants(
objects: &impl gix_object::FindHeader,
arguments: &mut crate::fetch::Arguments,
ref_map: &RefMap,
remote_ref_target_known: &[bool],
shallow: &Shallow,
mapping_is_ignored: impl Fn(&refmap::Mapping) -> bool,
) {
) -> bool {
// When using shallow, we can't exclude `wants` as the remote won't send anything then. Thus, we have to resend everything
// we have as want instead to get exactly the same graph, but possibly deepened.
let is_shallow = !matches!(shallow, Shallow::NoChange);
let mut has_want = false;
let wants = ref_map
.mappings
.iter()
Expand All @@ -306,13 +310,15 @@ pub fn add_wants(
if !arguments.can_use_ref_in_want() || matches!(want.remote, refmap::Source::ObjectId(_)) {
if let Some(id) = id_on_remote {
arguments.want(id);
has_want = true;
}
} else {
arguments.want_ref(
want.remote
.as_name()
.expect("name available if this isn't an object id"),
);
has_want = true;
}
let id_is_annotated_tag_we_have = id_on_remote
.and_then(|id| objects.try_header(id).ok().flatten().map(|h| (id, h)))
Expand All @@ -324,6 +330,7 @@ pub fn add_wants(
arguments.have(tag_on_remote);
}
}
has_want
}

/// Remove all commits that are more recent than the cut-off, which is the commit time of the oldest common commit we have with the server.
Expand Down
4 changes: 3 additions & 1 deletion gix-protocol/src/fetch/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ mod with_fetch {
/// Typically invokes [`negotiate::mark_complete_and_common_ref()`].
fn mark_complete_and_common_ref(&mut self) -> Result<negotiate::Action, negotiate::Error>;
/// Typically invokes [`negotiate::add_wants()`].
fn add_wants(&mut self, arguments: &mut fetch::Arguments, remote_ref_target_known: &[bool]);
/// Returns `true` if wants were added, or `false` if the negotiation should be aborted.
#[must_use]
fn add_wants(&mut self, arguments: &mut fetch::Arguments, remote_ref_target_known: &[bool]) -> bool;
/// Typically invokes [`negotiate::one_round()`].
fn one_round(
&mut self,
Expand Down
4 changes: 2 additions & 2 deletions gix/src/remote/connection/fetch/receive_pack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,15 +274,15 @@ impl gix_protocol::fetch::Negotiate for Negotiate<'_, '_, '_> {
)
}

fn add_wants(&mut self, arguments: &mut Arguments, remote_ref_target_known: &[bool]) {
fn add_wants(&mut self, arguments: &mut Arguments, remote_ref_target_known: &[bool]) -> bool {
negotiate::add_wants(
self.objects,
arguments,
self.ref_map,
remote_ref_target_known,
self.shallow,
negotiate::make_refmapping_ignore_predicate(self.tags, self.ref_map),
);
)
}

fn one_round(
Expand Down
4 changes: 2 additions & 2 deletions tests/journey/gix.sh
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ title "gix (with repository)"

# for some reason, on CI the daemon always shuts down before we can connect,
# or isn't actually ready despite having accepted the first connection already.
(not_on_ci with "git:// protocol"
(with "git:// protocol"
launch-git-daemon
(with "version 1"
it "generates the correct output" && {
Expand Down Expand Up @@ -278,7 +278,7 @@ title "gix commit-graph"
)
)
fi
(not_on_ci with "git:// protocol"
(with "git:// protocol"
launch-git-daemon
(with "version 1"
(with "NO output directory"
Expand Down
Loading