Skip to content

shallow points in V1 #1604

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

Merged
merged 3 commits into from
Sep 23, 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
9 changes: 9 additions & 0 deletions gix-protocol/src/fetch/response/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,15 @@ impl Response {
&self.shallows
}

/// Append the given `updates` which may have been obtained from a
/// (handshake::Outcome)[crate::handshake::Outcome::v1_shallow_updates].
///
/// In V2, these are received as part of the pack, but V1 sends them early, so we
/// offer to re-integrate them here.
pub fn append_v1_shallow_updates(&mut self, updates: Option<Vec<ShallowUpdate>>) {
self.shallows.extend(updates.into_iter().flatten());
}

/// Return all wanted-refs [parsed previously][Response::from_line_reader()].
pub fn wanted_refs(&self) -> &[WantedRef] {
&self.wanted_refs
Expand Down
1 change: 1 addition & 0 deletions gix-protocol/src/fetch_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ where
let crate::handshake::Outcome {
server_protocol_version: protocol_version,
refs,
v1_shallow_updates: _ignored_shallow_updates_as_it_is_deprecated,
capabilities,
} = crate::fetch::handshake(
&mut transport,
Expand Down
5 changes: 5 additions & 0 deletions gix-protocol/src/handshake/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,14 @@ where
(actual_protocol, parsed_refs, capabilities)
}; // this scope is needed, see https://github.com/rust-lang/rust/issues/76149

let (refs, v1_shallow_updates) = refs
.map(|(refs, shallow)| (Some(refs), Some(shallow)))
.unwrap_or_default();

Ok(Outcome {
server_protocol_version,
refs,
v1_shallow_updates,
capabilities,
})
}
6 changes: 5 additions & 1 deletion gix-protocol/src/handshake/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,11 @@ pub enum Ref {
pub struct Outcome {
/// The protocol version the server responded with. It might have downgraded the desired version.
pub server_protocol_version: gix_transport::Protocol,
/// The references reported as part of the Protocol::V1 handshake, or `None` otherwise as V2 requires a separate request.
/// The references reported as part of the `Protocol::V1` handshake, or `None` otherwise as V2 requires a separate request.
pub refs: Option<Vec<Ref>>,
/// Shallow updates as part of the `Protocol::V1`, to shallow a particular object.
/// Note that unshallowing isn't supported here.
pub v1_shallow_updates: Option<Vec<ShallowUpdate>>,
/// The server capabilities.
pub capabilities: Capabilities,
}
Expand Down Expand Up @@ -93,6 +96,7 @@ mod error {
}
}
}
use crate::fetch::response::ShallowUpdate;
pub use error::Error;

pub(crate) mod function;
Expand Down
13 changes: 10 additions & 3 deletions gix-protocol/src/handshake/refs/async_io.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::fetch::response::ShallowUpdate;
use crate::handshake::{refs, refs::parse::Error, Ref};

/// Parse refs from the given input line by line. Protocol V2 is required for this to succeed.
Expand Down Expand Up @@ -26,8 +27,9 @@ pub async fn from_v2_refs(in_refs: &mut dyn gix_transport::client::ReadlineBufRe
pub async fn from_v1_refs_received_as_part_of_handshake_and_capabilities<'a>(
in_refs: &mut dyn gix_transport::client::ReadlineBufRead,
capabilities: impl Iterator<Item = gix_transport::client::capabilities::Capability<'a>>,
) -> Result<Vec<Ref>, refs::parse::Error> {
) -> Result<(Vec<Ref>, Vec<ShallowUpdate>), refs::parse::Error> {
let mut out_refs = refs::shared::from_capabilities(capabilities)?;
let mut out_shallow = Vec::new();
let number_of_possible_symbolic_refs_for_lookup = out_refs.len();

while let Some(line) = in_refs
Expand All @@ -37,7 +39,12 @@ pub async fn from_v1_refs_received_as_part_of_handshake_and_capabilities<'a>(
.transpose()?
.and_then(|l| l.as_bstr())
{
refs::shared::parse_v1(number_of_possible_symbolic_refs_for_lookup, &mut out_refs, line)?;
refs::shared::parse_v1(
number_of_possible_symbolic_refs_for_lookup,
&mut out_refs,
&mut out_shallow,
line,
)?;
}
Ok(out_refs.into_iter().map(Into::into).collect())
Ok((out_refs.into_iter().map(Into::into).collect(), out_shallow))
}
13 changes: 10 additions & 3 deletions gix-protocol/src/handshake/refs/blocking_io.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::fetch::response::ShallowUpdate;
use crate::handshake::{refs, refs::parse::Error, Ref};

/// Parse refs from the given input line by line. Protocol V2 is required for this to succeed.
Expand All @@ -20,12 +21,18 @@ pub fn from_v2_refs(in_refs: &mut dyn gix_transport::client::ReadlineBufRead) ->
pub fn from_v1_refs_received_as_part_of_handshake_and_capabilities<'a>(
in_refs: &mut dyn gix_transport::client::ReadlineBufRead,
capabilities: impl Iterator<Item = gix_transport::client::capabilities::Capability<'a>>,
) -> Result<Vec<Ref>, Error> {
) -> Result<(Vec<Ref>, Vec<ShallowUpdate>), Error> {
let mut out_refs = refs::shared::from_capabilities(capabilities)?;
let mut out_shallow = Vec::new();
let number_of_possible_symbolic_refs_for_lookup = out_refs.len();

while let Some(line) = in_refs.readline().transpose()?.transpose()?.and_then(|l| l.as_bstr()) {
refs::shared::parse_v1(number_of_possible_symbolic_refs_for_lookup, &mut out_refs, line)?;
refs::shared::parse_v1(
number_of_possible_symbolic_refs_for_lookup,
&mut out_refs,
&mut out_shallow,
line,
)?;
}
Ok(out_refs.into_iter().map(Into::into).collect())
Ok((out_refs.into_iter().map(Into::into).collect(), out_shallow))
}
15 changes: 12 additions & 3 deletions gix-protocol/src/handshake/refs/shared.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use bstr::{BStr, BString, ByteSlice};

use crate::fetch::response::ShallowUpdate;
use crate::handshake::{refs::parse::Error, Ref};
use bstr::{BStr, BString, ByteSlice};

impl From<InternalRef> for Ref {
fn from(v: InternalRef) -> Self {
Expand Down Expand Up @@ -123,6 +123,7 @@ pub(crate) fn from_capabilities<'a>(
pub(in crate::handshake::refs) fn parse_v1(
num_initial_out_refs: usize,
out_refs: &mut Vec<InternalRef>,
out_shallow: &mut Vec<ShallowUpdate>,
line: &BStr,
) -> Result<(), Error> {
let trimmed = line.trim_end();
Expand Down Expand Up @@ -160,7 +161,15 @@ pub(in crate::handshake::refs) fn parse_v1(
});
}
None => {
let object = gix_hash::ObjectId::from_hex(hex_hash.as_bytes())?;
let object = match gix_hash::ObjectId::from_hex(hex_hash.as_bytes()) {
Ok(id) => id,
Err(_) if hex_hash.as_bstr() == "shallow" => {
let id = gix_hash::ObjectId::from_hex(path)?;
out_shallow.push(ShallowUpdate::Shallow(id));
return Ok(());
}
Err(err) => return Err(err.into()),
};
match out_refs
.iter()
.take(num_initial_out_refs)
Expand Down
64 changes: 63 additions & 1 deletion gix-protocol/src/handshake/refs/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ dce0ea858eef7ff61ad345cc5cdac62203fb3c10 refs/tags/gix-commitgraph-v0.0.0
21c9b7500cb144b3169a6537961ec2b9e865be81 refs/tags/gix-commitgraph-v0.0.0^{}"
.as_bytes(),
);
let out = refs::from_v1_refs_received_as_part_of_handshake_and_capabilities(
let (out, shallow) = refs::from_v1_refs_received_as_part_of_handshake_and_capabilities(
input,
Capabilities::from_bytes(b"\0symref=HEAD:refs/heads/main symref=MISSING_NAMESPACE_TARGET:(null)")
.expect("valid capabilities")
Expand All @@ -88,6 +88,68 @@ dce0ea858eef7ff61ad345cc5cdac62203fb3c10 refs/tags/gix-commitgraph-v0.0.0
)
.await
.expect("no failure from valid input");
assert!(shallow.is_empty());
assert_eq!(
out,
vec![
Ref::Symbolic {
full_ref_name: "HEAD".into(),
target: "refs/heads/main".into(),
tag: None,
object: oid("73a6868963993a3328e7d8fe94e5a6ac5078a944")
},
Ref::Direct {
full_ref_name: "MISSING_NAMESPACE_TARGET".into(),
object: oid("21c9b7500cb144b3169a6537961ec2b9e865be81")
},
Ref::Direct {
full_ref_name: "refs/heads/main".into(),
object: oid("73a6868963993a3328e7d8fe94e5a6ac5078a944")
},
Ref::Direct {
full_ref_name: "refs/pull/13/head".into(),
object: oid("8e472f9ccc7d745927426cbb2d9d077de545aa4e")
},
Ref::Peeled {
full_ref_name: "refs/tags/gix-commitgraph-v0.0.0".into(),
tag: oid("dce0ea858eef7ff61ad345cc5cdac62203fb3c10"),
object: oid("21c9b7500cb144b3169a6537961ec2b9e865be81")
},
]
);
}

#[maybe_async::test(feature = "blocking-client", async(feature = "async-client", async_std::test))]
async fn extract_references_from_v1_refs_with_shallow() {
use crate::fetch::response::ShallowUpdate;
let input = &mut Fixture(
"73a6868963993a3328e7d8fe94e5a6ac5078a944 HEAD
21c9b7500cb144b3169a6537961ec2b9e865be81 MISSING_NAMESPACE_TARGET
73a6868963993a3328e7d8fe94e5a6ac5078a944 refs/heads/main
8e472f9ccc7d745927426cbb2d9d077de545aa4e refs/pull/13/head
dce0ea858eef7ff61ad345cc5cdac62203fb3c10 refs/tags/gix-commitgraph-v0.0.0
21c9b7500cb144b3169a6537961ec2b9e865be81 refs/tags/gix-commitgraph-v0.0.0^{}
shallow 21c9b7500cb144b3169a6537961ec2b9e865be81
shallow dce0ea858eef7ff61ad345cc5cdac62203fb3c10"
.as_bytes(),
);
let (out, shallow) = refs::from_v1_refs_received_as_part_of_handshake_and_capabilities(
input,
Capabilities::from_bytes(b"\0symref=HEAD:refs/heads/main symref=MISSING_NAMESPACE_TARGET:(null)")
.expect("valid capabilities")
.0
.iter(),
)
.await
.expect("no failure from valid input");

assert_eq!(
shallow,
vec![
ShallowUpdate::Shallow(oid("21c9b7500cb144b3169a6537961ec2b9e865be81")),
ShallowUpdate::Shallow(oid("dce0ea858eef7ff61ad345cc5cdac62203fb3c10"))
]
);
assert_eq!(
out,
vec![
Expand Down
5 changes: 4 additions & 1 deletion gix/src/remote/connection/fetch/receive_pack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ where
});
}

let v1_shallow_updates = self.ref_map.handshake.v1_shallow_updates.take();
let handshake = &self.ref_map.handshake;
let protocol_version = handshake.server_protocol_version;

Expand Down Expand Up @@ -253,7 +254,9 @@ where
drop(graph_repo);
drop(negotiate_span);

let previous_response = previous_response.expect("knowledge of a pack means a response was received");
let mut previous_response =
previous_response.expect("knowledge of a pack means a response was received");
previous_response.append_v1_shallow_updates(v1_shallow_updates);
if !previous_response.shallow_updates().is_empty() && shallow_lock.is_none() {
let reject_shallow_remote = repo
.config
Expand Down
Loading