From 6c82f31fd4009e9806a928de0bc2b21f043ca1d6 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 23 Sep 2024 17:42:14 +0200 Subject: [PATCH 1/3] reproduce #1598 --- gix-protocol/src/handshake/refs/shared.rs | 11 +++-- gix-protocol/src/handshake/refs/tests.rs | 52 +++++++++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/gix-protocol/src/handshake/refs/shared.rs b/gix-protocol/src/handshake/refs/shared.rs index cf0f4729243..47fc69ecd2e 100644 --- a/gix-protocol/src/handshake/refs/shared.rs +++ b/gix-protocol/src/handshake/refs/shared.rs @@ -1,6 +1,5 @@ -use bstr::{BStr, BString, ByteSlice}; - use crate::handshake::{refs::parse::Error, Ref}; +use bstr::{BStr, BString, ByteSlice}; impl From for Ref { fn from(v: InternalRef) -> Self { @@ -160,7 +159,13 @@ 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" => { + todo!("shallow"); + } + Err(err) => return Err(err.into()), + }; match out_refs .iter() .take(num_initial_out_refs) diff --git a/gix-protocol/src/handshake/refs/tests.rs b/gix-protocol/src/handshake/refs/tests.rs index 7b69e6eca4a..8a58043e9e4 100644 --- a/gix-protocol/src/handshake/refs/tests.rs +++ b/gix-protocol/src/handshake/refs/tests.rs @@ -118,6 +118,58 @@ dce0ea858eef7ff61ad345cc5cdac62203fb3c10 refs/tags/gix-commitgraph-v0.0.0 ); } +#[maybe_async::test(feature = "blocking-client", async(feature = "async-client", async_std::test))] +async fn extract_references_from_v1_refs_with_shallow() { + 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 = 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!( + 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") + }, + ] + ); +} + #[test] fn extract_symbolic_references_from_capabilities() -> Result<(), client::Error> { let caps = client::Capabilities::from_bytes( From 24da857dbd236ae1cc75a4efd0816e717b885c7f Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 23 Sep 2024 19:26:12 +0200 Subject: [PATCH 2/3] fix!: make V1 shallow announcements possible (#1604) --- gix-protocol/src/fetch/response/mod.rs | 9 +++++++++ gix-protocol/src/fetch_fn.rs | 1 + gix-protocol/src/handshake/function.rs | 5 +++++ gix-protocol/src/handshake/mod.rs | 6 +++++- gix-protocol/src/handshake/refs/async_io.rs | 13 ++++++++++--- gix-protocol/src/handshake/refs/blocking_io.rs | 13 ++++++++++--- gix-protocol/src/handshake/refs/shared.rs | 6 +++++- gix-protocol/src/handshake/refs/tests.rs | 14 ++++++++++++-- 8 files changed, 57 insertions(+), 10 deletions(-) diff --git a/gix-protocol/src/fetch/response/mod.rs b/gix-protocol/src/fetch/response/mod.rs index 5f2f7f007fc..5073cc13e29 100644 --- a/gix-protocol/src/fetch/response/mod.rs +++ b/gix-protocol/src/fetch/response/mod.rs @@ -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>) { + 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 diff --git a/gix-protocol/src/fetch_fn.rs b/gix-protocol/src/fetch_fn.rs index 73c9a753a5e..2cf60fdc4fd 100644 --- a/gix-protocol/src/fetch_fn.rs +++ b/gix-protocol/src/fetch_fn.rs @@ -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, diff --git a/gix-protocol/src/handshake/function.rs b/gix-protocol/src/handshake/function.rs index daf405e10d4..6e5a6cc041f 100644 --- a/gix-protocol/src/handshake/function.rs +++ b/gix-protocol/src/handshake/function.rs @@ -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, }) } diff --git a/gix-protocol/src/handshake/mod.rs b/gix-protocol/src/handshake/mod.rs index 28243e96da7..b0baf4b498f 100644 --- a/gix-protocol/src/handshake/mod.rs +++ b/gix-protocol/src/handshake/mod.rs @@ -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>, + /// 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>, /// The server capabilities. pub capabilities: Capabilities, } @@ -93,6 +96,7 @@ mod error { } } } +use crate::fetch::response::ShallowUpdate; pub use error::Error; pub(crate) mod function; diff --git a/gix-protocol/src/handshake/refs/async_io.rs b/gix-protocol/src/handshake/refs/async_io.rs index 19ea543c7ba..5b9dfcee7f3 100644 --- a/gix-protocol/src/handshake/refs/async_io.rs +++ b/gix-protocol/src/handshake/refs/async_io.rs @@ -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. @@ -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>, -) -> Result, refs::parse::Error> { +) -> Result<(Vec, Vec), 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 @@ -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)) } diff --git a/gix-protocol/src/handshake/refs/blocking_io.rs b/gix-protocol/src/handshake/refs/blocking_io.rs index 7ad695b7728..ed7e49a312b 100644 --- a/gix-protocol/src/handshake/refs/blocking_io.rs +++ b/gix-protocol/src/handshake/refs/blocking_io.rs @@ -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. @@ -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>, -) -> Result, Error> { +) -> Result<(Vec, Vec), 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)) } diff --git a/gix-protocol/src/handshake/refs/shared.rs b/gix-protocol/src/handshake/refs/shared.rs index 47fc69ecd2e..aa0d94f292d 100644 --- a/gix-protocol/src/handshake/refs/shared.rs +++ b/gix-protocol/src/handshake/refs/shared.rs @@ -1,3 +1,4 @@ +use crate::fetch::response::ShallowUpdate; use crate::handshake::{refs::parse::Error, Ref}; use bstr::{BStr, BString, ByteSlice}; @@ -122,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, + out_shallow: &mut Vec, line: &BStr, ) -> Result<(), Error> { let trimmed = line.trim_end(); @@ -162,7 +164,9 @@ pub(in crate::handshake::refs) fn parse_v1( let object = match gix_hash::ObjectId::from_hex(hex_hash.as_bytes()) { Ok(id) => id, Err(_) if hex_hash.as_bstr() == "shallow" => { - todo!("shallow"); + let id = gix_hash::ObjectId::from_hex(path)?; + out_shallow.push(ShallowUpdate::Shallow(id)); + return Ok(()); } Err(err) => return Err(err.into()), }; diff --git a/gix-protocol/src/handshake/refs/tests.rs b/gix-protocol/src/handshake/refs/tests.rs index 8a58043e9e4..31f04df52d6 100644 --- a/gix-protocol/src/handshake/refs/tests.rs +++ b/gix-protocol/src/handshake/refs/tests.rs @@ -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") @@ -88,6 +88,7 @@ dce0ea858eef7ff61ad345cc5cdac62203fb3c10 refs/tags/gix-commitgraph-v0.0.0 ) .await .expect("no failure from valid input"); + assert!(shallow.is_empty()); assert_eq!( out, vec![ @@ -120,6 +121,7 @@ dce0ea858eef7ff61ad345cc5cdac62203fb3c10 refs/tags/gix-commitgraph-v0.0.0 #[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 @@ -131,7 +133,7 @@ shallow 21c9b7500cb144b3169a6537961ec2b9e865be81 shallow dce0ea858eef7ff61ad345cc5cdac62203fb3c10" .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") @@ -140,6 +142,14 @@ shallow dce0ea858eef7ff61ad345cc5cdac62203fb3c10" ) .await .expect("no failure from valid input"); + + assert_eq!( + shallow, + vec![ + ShallowUpdate::Shallow(oid("21c9b7500cb144b3169a6537961ec2b9e865be81")), + ShallowUpdate::Shallow(oid("dce0ea858eef7ff61ad345cc5cdac62203fb3c10")) + ] + ); assert_eq!( out, vec![ From 0d3b480e5e7d27c308fb5f76f36831dfc7af3e8f Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 23 Sep 2024 20:09:25 +0200 Subject: [PATCH 3/3] adapt to changes in `gix-protocol` --- gix/src/remote/connection/fetch/receive_pack.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/gix/src/remote/connection/fetch/receive_pack.rs b/gix/src/remote/connection/fetch/receive_pack.rs index f6bfa24fd74..d0d2eefcbc1 100644 --- a/gix/src/remote/connection/fetch/receive_pack.rs +++ b/gix/src/remote/connection/fetch/receive_pack.rs @@ -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; @@ -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