From 72ec01b6e37e56fda72e8f137c92dc2276bc0791 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 7 Mar 2023 20:07:22 +0100 Subject: [PATCH 1/9] fix!: add `ReadLineBufRead::readline_str(&mut String)` to get consistent readline behaviour. Previously the `async` version of the transport implementation would use the default `read_line` implementation which would search for newlines in packetlines. However, these aren't always emitted which could lead to multiple packetlines to be concatenated. Now the we always use our implementation which simply treats one packet line as line, no matter whether or not it ends with optional newline. --- .../src/client/async_io/bufread_ext.rs | 15 +++++++++++++++ .../src/client/blocking_io/bufread_ext.rs | 18 ++++++++++++++++++ .../src/client/blocking_io/http/mod.rs | 5 +++++ 3 files changed, 38 insertions(+) diff --git a/gix-transport/src/client/async_io/bufread_ext.rs b/gix-transport/src/client/async_io/bufread_ext.rs index abe206b8a3e..002af7b8bbf 100644 --- a/gix-transport/src/client/async_io/bufread_ext.rs +++ b/gix-transport/src/client/async_io/bufread_ext.rs @@ -34,6 +34,12 @@ pub trait ReadlineBufRead: AsyncBufRead { async fn readline( &mut self, ) -> Option, gix_packetline::decode::Error>>>; + + /// Read a line similar to `BufRead::read_line()`, but assure it doesn't try to find newlines + /// which might concatenate multiple distinct packet lines. + /// + /// Making this a trait method allows to handle differences between async and blocking. + async fn readline_str(&mut self, line: &mut String) -> io::Result; } /// Provide even more access to the underlying packet reader. @@ -58,6 +64,9 @@ impl<'a, T: ReadlineBufRead + ?Sized + 'a + Unpin> ReadlineBufRead for Box { async fn readline(&mut self) -> Option, gix_packetline::decode::Error>>> { self.deref_mut().readline().await } + async fn readline_str(&mut self, line: &mut String) -> io::Result { + self.deref_mut().readline_str(line).await + } } #[async_trait(?Send)] @@ -84,6 +93,9 @@ impl ReadlineBufRead for gix_packetline::read::WithSideban async fn readline(&mut self) -> Option, gix_packetline::decode::Error>>> { self.read_data_line().await } + async fn readline_str(&mut self, line: &mut String) -> io::Result { + self.read_line(line).await + } } #[async_trait(?Send)] @@ -91,6 +103,9 @@ impl<'a, T: AsyncRead + Unpin> ReadlineBufRead for gix_packetline::read::WithSid async fn readline(&mut self) -> Option, gix_packetline::decode::Error>>> { self.read_data_line().await } + async fn readline_str(&mut self, line: &mut String) -> io::Result { + self.read_line(line).await + } } #[async_trait(?Send)] diff --git a/gix-transport/src/client/blocking_io/bufread_ext.rs b/gix-transport/src/client/blocking_io/bufread_ext.rs index 5842ddd3d25..53105d8919d 100644 --- a/gix-transport/src/client/blocking_io/bufread_ext.rs +++ b/gix-transport/src/client/blocking_io/bufread_ext.rs @@ -1,3 +1,4 @@ +use std::io::BufRead; use std::{ io, ops::{Deref, DerefMut}, @@ -28,6 +29,12 @@ pub trait ReadlineBufRead: io::BufRead { fn readline( &mut self, ) -> Option, gix_packetline::decode::Error>>>; + + /// Read a line similar to `BufRead::read_line()`, but assure it doesn't try to find newlines + /// which might concatenate multiple distinct packet lines. + /// + /// Making this a trait method allows to handle differences between async and blocking. + fn readline_str(&mut self, line: &mut String) -> io::Result; } /// Provide even more access to the underlying packet reader. @@ -50,6 +57,9 @@ impl<'a, T: ReadlineBufRead + ?Sized + 'a> ReadlineBufRead for Box { fn readline(&mut self) -> Option, gix_packetline::decode::Error>>> { ReadlineBufRead::readline(self.deref_mut()) } + fn readline_str(&mut self, line: &mut String) -> io::Result { + self.read_line(line) + } } impl<'a, T: ExtendedBufRead + ?Sized + 'a> ExtendedBufRead for Box { @@ -74,12 +84,20 @@ impl ReadlineBufRead for gix_packetline::read::WithSidebands<'_, T, fn readline(&mut self) -> Option, gix_packetline::decode::Error>>> { self.read_data_line() } + + fn readline_str(&mut self, line: &mut String) -> io::Result { + self.read_line(line) + } } impl<'a, T: io::Read> ReadlineBufRead for gix_packetline::read::WithSidebands<'a, T, HandleProgress> { fn readline(&mut self) -> Option, gix_packetline::decode::Error>>> { self.read_data_line() } + + fn readline_str(&mut self, line: &mut String) -> io::Result { + self.read_line(line) + } } impl<'a, T: io::Read> ExtendedBufRead for gix_packetline::read::WithSidebands<'a, T, HandleProgress> { diff --git a/gix-transport/src/client/blocking_io/http/mod.rs b/gix-transport/src/client/blocking_io/http/mod.rs index dc0449ca0ec..a1f4a86e315 100644 --- a/gix-transport/src/client/blocking_io/http/mod.rs +++ b/gix-transport/src/client/blocking_io/http/mod.rs @@ -486,6 +486,11 @@ impl ReadlineBufRead for HeadersThenBody std::io::Result { + self.handle_headers()?; + self.body.readline_str(line) + } } impl ExtendedBufRead for HeadersThenBody { From 0eecfab26972fe784b7945b299e06624c922fe33 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 7 Mar 2023 20:36:50 +0100 Subject: [PATCH 2/9] adapt to changes in `gix-transport` and add V2 test for unshallow fetching --- gix-protocol/src/fetch/response/async_io.rs | 13 ++-- .../src/fetch/response/blocking_io.rs | 8 +-- gix-protocol/src/handshake/refs/tests.rs | 27 ++++++++ gix-protocol/tests/fetch/response.rs | 65 +++++++++++++++++- .../fixtures/v1/fetch-unshallow.response | Bin 0 -> 3079 bytes .../fixtures/v2/fetch-unshallow.response | Bin 0 -> 3083 bytes 6 files changed, 103 insertions(+), 10 deletions(-) create mode 100644 gix-protocol/tests/fixtures/v1/fetch-unshallow.response create mode 100644 gix-protocol/tests/fixtures/v2/fetch-unshallow.response diff --git a/gix-protocol/src/fetch/response/async_io.rs b/gix-protocol/src/fetch/response/async_io.rs index 7b00d843c82..550ed46b66f 100644 --- a/gix-protocol/src/fetch/response/async_io.rs +++ b/gix-protocol/src/fetch/response/async_io.rs @@ -1,6 +1,5 @@ use std::io; -use futures_lite::AsyncBufReadExt; use gix_transport::{client, Protocol}; use crate::fetch::{ @@ -16,7 +15,7 @@ async fn parse_v2_section( parse: impl Fn(&str) -> Result, ) -> Result { line.clear(); - while reader.read_line(line).await? != 0 { + while reader.readline_str(line).await? != 0 { res.push(parse(line)?); line.clear(); } @@ -62,7 +61,7 @@ impl Response { Some(client::MessageKind::Flush), "If this isn't a flush packet, we don't know what's going on" ); - reader.read_line(&mut line).await?; + reader.readline_str(&mut line).await?; reader.reset(Protocol::V1); match reader.peek_data_line().await { Some(Ok(Ok(line))) => String::from_utf8_lossy(line), @@ -76,7 +75,11 @@ impl Response { if Response::parse_v1_ack_or_shallow_or_assume_pack(&mut acks, &mut shallows, &peeked_line) { break 'lines true; } - assert_ne!(reader.read_line(&mut line).await?, 0, "consuming a peeked line works"); + assert_ne!( + reader.readline_str(&mut line).await?, + 0, + "consuming a peeked line works" + ); }; Ok(Response { acks, @@ -94,7 +97,7 @@ impl Response { let mut wanted_refs = Vec::::new(); let has_pack = 'section: loop { line.clear(); - if reader.read_line(&mut line).await? == 0 { + if reader.readline_str(&mut line).await? == 0 { return Err(response::Error::Io(io::Error::new( io::ErrorKind::UnexpectedEof, "Could not read message headline", diff --git a/gix-protocol/src/fetch/response/blocking_io.rs b/gix-protocol/src/fetch/response/blocking_io.rs index ca79724e266..7a3f2deb3bb 100644 --- a/gix-protocol/src/fetch/response/blocking_io.rs +++ b/gix-protocol/src/fetch/response/blocking_io.rs @@ -15,7 +15,7 @@ fn parse_v2_section( parse: impl Fn(&str) -> Result, ) -> Result { line.clear(); - while reader.read_line(line)? != 0 { + while reader.readline_str(line)? != 0 { res.push(parse(line)?); line.clear(); } @@ -61,7 +61,7 @@ impl Response { Some(client::MessageKind::Flush), "If this isn't a flush packet, we don't know what's going on" ); - reader.read_line(&mut line)?; + reader.readline_str(&mut line)?; reader.reset(Protocol::V1); match reader.peek_data_line() { Some(Ok(Ok(line))) => String::from_utf8_lossy(line), @@ -75,7 +75,7 @@ impl Response { if Response::parse_v1_ack_or_shallow_or_assume_pack(&mut acks, &mut shallows, &peeked_line) { break 'lines true; } - assert_ne!(reader.read_line(&mut line)?, 0, "consuming a peeked line works"); + assert_ne!(reader.readline_str(&mut line)?, 0, "consuming a peeked line works"); }; Ok(Response { acks, @@ -93,7 +93,7 @@ impl Response { let mut wanted_refs = Vec::::new(); let has_pack = 'section: loop { line.clear(); - if reader.read_line(&mut line)? == 0 { + if reader.readline_str(&mut line)? == 0 { return Err(response::Error::Io(io::Error::new( io::ErrorKind::UnexpectedEof, "Could not read message headline", diff --git a/gix-protocol/src/handshake/refs/tests.rs b/gix-protocol/src/handshake/refs/tests.rs index a7c9171a576..7d995da5cba 100644 --- a/gix-protocol/src/handshake/refs/tests.rs +++ b/gix-protocol/src/handshake/refs/tests.rs @@ -170,6 +170,20 @@ impl<'a> gix_transport::client::ReadlineBufRead for Fixture<'a> { self.0 = lines.as_bytes(); Some(Ok(Ok(gix_packetline::PacketLineRef::Data(res)))) } + + fn readline_str(&mut self, line: &mut String) -> std::io::Result { + use bstr::{BStr, ByteSlice}; + let bytes: &BStr = self.0.into(); + let mut lines = bytes.lines(); + let res = match lines.next() { + None => return Ok(0), + Some(line) => line, + }; + self.0 = lines.as_bytes(); + let len = res.len(); + line.push_str(res.to_str().expect("valid UTF8 in fixture")); + Ok(len) + } } #[cfg(feature = "async-client")] @@ -220,4 +234,17 @@ impl<'a> gix_transport::client::ReadlineBufRead for Fixture<'a> { self.0 = lines.as_bytes(); Some(Ok(Ok(gix_packetline::PacketLineRef::Data(res)))) } + async fn readline_str(&mut self, line: &mut String) -> std::io::Result { + use bstr::{BStr, ByteSlice}; + let bytes: &BStr = self.0.into(); + let mut lines = bytes.lines(); + let res = match lines.next() { + None => return Ok(0), + Some(line) => line, + }; + self.0 = lines.as_bytes(); + let len = res.len(); + line.push_str(res.to_str().expect("valid UTF8 in fixture")); + Ok(len) + } } diff --git a/gix-protocol/tests/fetch/response.rs b/gix-protocol/tests/fetch/response.rs index 2aca1b5fbd0..47cc00da5b4 100644 --- a/gix-protocol/tests/fetch/response.rs +++ b/gix-protocol/tests/fetch/response.rs @@ -69,6 +69,37 @@ mod v1 { Ok(()) } + #[maybe_async::test(feature = "blocking-client", async(feature = "async-client", async_std::test))] + async fn unshallow_fetch() -> crate::Result { + let mut provider = mock_reader("v1/fetch-unshallow.response"); + let mut reader = provider.as_read_without_sidebands(); + let r = fetch::Response::from_line_reader(Protocol::V1, &mut reader).await?; + assert_eq!( + r.acknowledgements(), + &[ + Acknowledgement::Common(id("f99771fe6a1b535783af3163eba95a927aae21d5")), + Acknowledgement::Common(id("2d9d136fb0765f2e24c44a0f91984318d580d03b")), + Acknowledgement::Common(id("dfd0954dabef3b64f458321ef15571cc1a46d552")), + ] + ); + assert_eq!( + r.shallow_updates(), + &[ + ShallowUpdate::Unshallow(id("2d9d136fb0765f2e24c44a0f91984318d580d03b")), + ShallowUpdate::Unshallow(id("dfd0954dabef3b64f458321ef15571cc1a46d552")) + ] + ); + assert!(r.has_pack()); + let mut pack = Vec::new(); + reader.read_to_end(&mut pack).await?; + assert_eq!( + pack.len(), + 2662, + "should be able to read the whole pack (and progress info)" + ); + Ok(()) + } + #[maybe_async::test(feature = "blocking-client", async(feature = "async-client", async_std::test))] async fn fetch_acks_without_pack() -> crate::Result { let mut provider = mock_reader("v1/fetch-no-pack.response"); @@ -159,6 +190,38 @@ mod v2 { Ok(()) } + #[maybe_async::test(feature = "blocking-client", async(feature = "async-client", async_std::test))] + async fn unshallow_fetch() -> crate::Result { + let mut provider = mock_reader("v2/fetch-unshallow.response"); + let mut reader = provider.as_read_without_sidebands(); + let r = fetch::Response::from_line_reader(Protocol::V2, &mut reader).await?; + assert_eq!( + r.acknowledgements(), + &[ + Acknowledgement::Common(id("f99771fe6a1b535783af3163eba95a927aae21d5")), + Acknowledgement::Common(id("2d9d136fb0765f2e24c44a0f91984318d580d03b")), + Acknowledgement::Common(id("dfd0954dabef3b64f458321ef15571cc1a46d552")), + Acknowledgement::Ready, + ] + ); + assert_eq!( + r.shallow_updates(), + &[ + ShallowUpdate::Unshallow(id("2d9d136fb0765f2e24c44a0f91984318d580d03b")), + ShallowUpdate::Unshallow(id("dfd0954dabef3b64f458321ef15571cc1a46d552")) + ] + ); + assert!(r.has_pack()); + let mut pack = Vec::new(); + reader.read_to_end(&mut pack).await?; + assert_eq!( + pack.len(), + 2664, + "should be able to read the whole pack (and progress info)" + ); + Ok(()) + } + #[maybe_async::test(feature = "blocking-client", async(feature = "async-client", async_std::test))] async fn empty_shallow_clone() -> crate::Result { let mut provider = mock_reader("v2/clone-deepen-5.response"); @@ -199,7 +262,7 @@ mod v2 { async fn fetch_acks_without_pack() -> crate::Result { let mut provider = mock_reader("v2/fetch-no-pack.response"); let r = fetch::Response::from_line_reader(Protocol::V2, &mut provider.as_read_without_sidebands()).await?; - assert_eq!(r.acknowledgements(), &[Acknowledgement::Nak,]); + assert_eq!(r.acknowledgements(), &[Acknowledgement::Nak]); Ok(()) } diff --git a/gix-protocol/tests/fixtures/v1/fetch-unshallow.response b/gix-protocol/tests/fixtures/v1/fetch-unshallow.response new file mode 100644 index 0000000000000000000000000000000000000000..ca74bfddea0b1ccd7b6c06319388349ba69b2489 GIT binary patch literal 3079 zcma);c~nzZ9>w#!*I|fMCUjf`JRn@*|k>!301Q#Z4}lU?2dq1WAT3 zm_TSQ1xYe7WJa+$hVP1xYm%|}2RUi!v4w3- z^JjG;aS6e;Yq7?CgBhp1YCc=|_ragGjty9>Zc#wY2lc{(tJ7D=_I#%Dc(gySkO)!9 zLuT8KM8#|lzu^;&Uc6PIT0kO(k&W5Iu9bUY`(}Q*Wx2ng(KV~>8|_}frOy)*y{lf^ zaUx{Z&%Rb3~26)O>%i>&9HW?RUYUUe)(EyV#k8VhU%vyafhz+s%~J-H~zQx zNZ|POp62>_6+ekCFvu2r3nkHKYfzE`0*MInAsS9p^Uv`iA4h9&fdCguFo{@zokOXx zumPcuukMO}WMY(cv^5*VBb=F)&bu2dd6nt9(S}VnSy@@32eU$twYAxtw;HsvS`*aS zdo4fz>jjqUEi*0GS$A3a34^FZE-tL-`2c-h9;Hx;nEXO4l2nVqSw6&20K#8^j;rzy zNLC$~?JM@NkMxZcH#mw!@L{ggT<7J^%Y*FPvOfrp3z4apg`HTd-Fo`d?WLuqc@?F3 zyBn&upWfE6jqMVFQ`O}jwfvU1TGq9=wAO6Pb?o@hfX#H}Au1f=34{23Npu)ngGgQPv`ft3|^Y z78tg;>Ef*K{LeP#tRFw6AJw~$Z%Etjz!3)7%yM6n@iW=D`Kt|!n_Ahw#Pt5!6HuFx zlv#OEeDgUqK8Z;%Dol(4D5|T$`8XvMLnwrRs6pvBOf6rGofG4H@!-oXt?k&KFKvbk z%T|vbK4km+ngwdJL{5KeL4Bu%_)f{THlbU~os&+z=f{^kONsh-xz4J+ znB(a<=y0p`yfEwXieDCi6qg>?{p*|U7pfn8bXmvwo>?4Y+t_rk6dGX0hxuj+lZ znc-JDgmp7mm!H`4$kZbJ4_EtzcQ`CdA$#?7=lxedU03C;x%;(#Nc<%+R8YLMtNQ6Y z*2M#_Innj>{d7G8D`mRN(0r!(^!?>Wv>_v*I}6j!yk6H%6; zu4f9}qQR1e`ZH}8PPPT!jeL?n6n=?a_BXp2x=GQ{RfF(*x=F~6oNl~w~Y*zV6TP0J@K$q)fB~5q5D()pbIJ|Mg!+!fW6UuV?T#K7-Zna~T z=h_7_D2Hg*ZI<@Z!5PtC+}hvX@vr>&75CJ0pY$})+mLgjG|AF<1OmiMp^Q(KBB-|y eqE)6ODHYRxOq9ys^ZiXh>Gvo~vR#>1`Tqe+6GF=X literal 0 HcmV?d00001 diff --git a/gix-protocol/tests/fixtures/v2/fetch-unshallow.response b/gix-protocol/tests/fixtures/v2/fetch-unshallow.response new file mode 100644 index 0000000000000000000000000000000000000000..2035abc8cd5689a2284aed65a26bdb7bfcfa9fdc GIT binary patch literal 3083 zcma);3s6&M7RQrNkmX?!LCWG{RB&PBalh}rl0_Gk1@i<%C~ntL^0=YUkYEDJ!$;MM z2=xILElLq6=%V8Tc@zY!pbN&h6{KP-(4kmhrIq@u0@Izn7u&2d3FAzXxpU9|{LlG* zU(UTBf&fgW_&}rGtfrI+$&@BlZ-*c#SP?4bs`z}81S*P<0XdH1BoCFTP(YxRT*k*` zd`QY<6a-4#lmL}{B|r(493cr@1u2LrFieK1_<+yDP{32-JVc40a#Mm*r9}8Rrj*Gk z6)Gn%6^8Rr2q+c6aS|vLK!y=Y9EWrQBGXYaWd?Hv=nv4pFH@_voBb0tDlOwjq-lP; z#eLH@Ue9{dUS2kUC{t)`szfzK7l#-pOp}&O>13&ingp&^o{NV66=!x~s5 zi8CO838i@ph6J$4l3+*!6HK!f1i@%5(j=!?%#4#{SU`(7$*F(b%s6=r3uqB1k6{64 z#>r!lLyI{13=23jPCml|TEa;`Cz$Z>IHBCkc)hB5Wms`qnw+Ae^!nEZnmaX1AaH72 zd6q{lCbB2Imv( zDEj5VVzD^%*R{|ty?D2AgA$xt&OYS&uDyHqRC5dBAZy{*0J6dS7xgQEx%ag@|{Z6B7!?GwlQbay=q^4|LiZft_&15xo5Y3quVdM^3UWH-|Dw_ zoeEp?^JvDl`Ehr#UxQv?S@UcFJ4(z)pAK+W^9q$4-^)wj@=TmRjE zEO_!}Z%f01%AdrS7-UO*Mbg;wwFp51p;Qb75Q)TV1Q!HQfFZS*P>6}7s8k|EFCb(@ z#E{6(-*7E3Iwi(7)|L&_T+Zw&*S(F_{HjdDc;hC!?Ck9D!`a~{+S~0e+6>#+tPAPt zyHQZ^^&;y8>+RMXY`bj&L?Pr6H#b)7LV%dcBP1e~5MM~d(i#akFMufhR*ifMI zBwcf8uD`_3G1@;`(&#J}!^e3p^IccEt_*ST$oVidDNL?e5pin0Zrj<+I)G=cY)`dauX)c7{`sClZhESoj9TpyE&fAvVJC-wtSViw7HG_OI+Wty+L(Z z`t4PhCAUq~1OzHY$p{Gwpt!yk6JVrB0-*>3ycVI)NOb}UdO?B-B*SmEY3(Ow-n3b6 zEPEqz^oYG_%tE?;EtUtAdby(@DtEB0u%XLJa<6npyU3&U-f5S~?!Gspa#hOPSpVVTg`lLe>SwX8K6B&~>BYt$t`Cataah(O_S)&05C8DlhH78!{jZH9k}vU*!jk3PHP08Y zE+2Z^g|25Dr0Wf^(xzt(w47@>`*7tkUD#Oo?xKuy@6>mYMOLgcEe@_s)(>>ncdXRu zSud1^CBvnS4d>b~oo)}lAN{OgB=QQo{ErTCbd%zd>jR?i=_VmNdV1ouV>g>R*X#0( z9;f;B8HKBlDAtC~W3wtw+fz)r0lHitOFum;Uil#T(b0{Gj|Uy!O)k&tcQ0waz0HAD zk>?Q1pd6vI?y_`O4$q4H;?BX2&c79?S3S_of7aVb?}maCqfM2mxe#!Dl$1JE#zlPn bxH>9LPbsI}WDc(WzeNz{JWa6O>0ji3w!cIo literal 0 HcmV?d00001 From 8e8c71ba1238f0c8177f84810c2d9ed165010d7e Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 8 Mar 2023 10:12:54 +0100 Subject: [PATCH 3/9] fix: implement `Clone` for `PartialName`. Also improve documentation for `FullName*` and `PartialName*` types. --- gix-ref/src/lib.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/gix-ref/src/lib.rs b/gix-ref/src/lib.rs index b18d67c4a33..8ef6a8c9d11 100644 --- a/gix-ref/src/lib.rs +++ b/gix-ref/src/lib.rs @@ -104,28 +104,27 @@ pub(crate) struct Store { inner: store::State, } -/// Indicate that the given BString is a validate reference name or path that can be used as path on disk or written as target -/// of a symbolic reference +/// A validated complete and fully qualified referenced reference name, safe to use for all operations. #[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] #[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] pub struct FullName(pub(crate) BString); -/// A validated and potentially partial reference name - it can safely be used for common operations. +/// A validated complete and fully qualified referenced reference name, safe to use for all operations. #[derive(Hash, Debug, PartialEq, Eq, Ord, PartialOrd)] #[repr(transparent)] pub struct FullNameRef(BStr); -/// A validated complete and fully qualified reference name, safe to use for all operations. +/// A validated and potentially partial reference name, safe to use for common operations. #[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] pub struct PartialNameCow<'a>(Cow<'a, BStr>); -/// A validated complete and fully qualified referenced reference name, safe to use for all operations. +/// A validated and potentially partial reference name, safe to use for common operations. #[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd)] #[repr(transparent)] pub struct PartialNameRef(BStr); -/// A validated complete and fully qualified owned reference name, safe to use for all operations. -#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd)] +/// A validated and potentially partial reference name, safe to use for common operations. +#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] pub struct PartialName(BString); /// A _validated_ prefix for references to act as a namespace. From ac34d2e96d7904fdf0c654a64260be0ec106ae0f Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Thu, 9 Mar 2023 09:15:32 +0100 Subject: [PATCH 4/9] add more fetch::Arguments tests That way can be sure the all capabilities are working like we think they do. --- gix-protocol/tests/fetch/response.rs | 97 ++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/gix-protocol/tests/fetch/response.rs b/gix-protocol/tests/fetch/response.rs index 47cc00da5b4..68d49e23578 100644 --- a/gix-protocol/tests/fetch/response.rs +++ b/gix-protocol/tests/fetch/response.rs @@ -137,6 +137,54 @@ mod v1 { Ok(()) } } + + mod arguments { + use crate::fetch::response::id; + use crate::fetch::transport; + use bstr::ByteSlice; + use gix_protocol::{fetch, Command}; + use gix_transport::client::Capabilities; + use gix_transport::Protocol; + + #[maybe_async::test(feature = "blocking-client", async(feature = "async-client", async_std::test))] + async fn all() -> crate::Result { + let (caps, _) = Capabilities::from_bytes(&b"7814e8a05a59c0cf5fb186661d1551c75d1299b5 HEAD\0multi_ack thin-pack filter side-band side-band-64k ofs-delta shallow deepen-since deepen-not deepen-relative no-progress include-tag multi_ack_detailed symref=HEAD:refs/heads/master object-format=sha1 agent=git/2.28.0"[..])?; + let mut args = fetch::Arguments::new(Protocol::V1, Command::Fetch.default_features(Protocol::V1, &caps)); + assert!(args.can_use_shallow()); + assert!(args.can_use_deepen()); + assert!(args.can_use_deepen_not()); + assert!(args.can_use_deepen_relative()); + assert!(args.can_use_deepen_since()); + assert!(args.can_use_filter()); + assert!(args.can_use_include_tag()); + assert!( + !args.can_use_ref_in_want(), + "V2 only feature, and we initialize capabilities with V1 for convenience" + ); + assert!(args.is_empty()); + + args.shallow(id("97c5a932b3940a09683e924ef6a92b31a6f7c6de")); + args.deepen(1); + args.deepen_relative(); + args.deepen_since(123456); + args.deepen_not("tag".into()); + args.want(id("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")); + args.have(id("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb")); + + let mut out = Vec::new(); + let mut transport = transport( + &mut out, + "v1/clone.response", + Protocol::V2, + gix_transport::client::git::ConnectMode::Daemon, + ); + + let _response = args.send(&mut transport, true).await?; + drop(_response); + assert_eq!(out.as_slice().as_bstr(), "00aawant aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa thin-pack side-band-64k ofs-delta shallow deepen-since deepen-not deepen-relative include-tag multi_ack_detailed filter\n000ddeepen 1\n0014deepen-relative\n0018deepen-since 123456\n0013deepen-not tag\n0035shallow 97c5a932b3940a09683e924ef6a92b31a6f7c6de\n00000032have bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\n0009done\n"); + Ok(()) + } + } } mod v2 { mod from_line_reader { @@ -305,4 +353,53 @@ mod v2 { Ok(()) } } + + mod arguments { + use crate::fetch::response::id; + use crate::fetch::transport; + use bstr::ByteSlice; + use gix_protocol::{fetch, Command}; + use gix_transport::client::Capabilities; + use gix_transport::Protocol; + + #[maybe_async::test(feature = "blocking-client", async(feature = "async-client", async_std::test))] + async fn all() -> crate::Result { + let (caps, _) = Capabilities::from_bytes(&b"7814e8a05a59c0cf5fb186661d1551c75d1299b5 HEAD\0multi_ack thin-pack filter side-band side-band-64k ofs-delta shallow deepen-since deepen-not deepen-relative no-progress include-tag multi_ack_detailed symref=HEAD:refs/heads/master object-format=sha1 agent=git/2.28.0"[..])?; + let mut args = fetch::Arguments::new(Protocol::V2, Command::Fetch.default_features(Protocol::V1, &caps)); + assert!(args.can_use_shallow()); + assert!(args.can_use_deepen()); + assert!(args.can_use_deepen_not()); + assert!(args.can_use_deepen_relative()); + assert!(args.can_use_deepen_since()); + assert!(args.can_use_filter()); + assert!(args.can_use_include_tag()); + assert!( + !args.can_use_ref_in_want(), + "V2 only feature, and we initialize capabilities with V1 for convenience" + ); + assert!(args.is_empty()); + + args.shallow(id("97c5a932b3940a09683e924ef6a92b31a6f7c6de")); + args.deepen(1); + args.deepen_relative(); + args.deepen_since(123456); + args.deepen_not("tag".into()); + args.want(id("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")); + args.have(id("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb")); + + let mut out = Vec::new(); + let mut transport = transport( + &mut out, + "v1/clone.response", + Protocol::V2, + gix_transport::client::git::ConnectMode::Daemon, + ); + + let _response = args.send(&mut transport, true).await?; + drop(_response); + assert_eq!(out.as_slice().as_bstr(), "0012command=fetch\n0001000ethin-pack\n000eofs-delta\n0035shallow 97c5a932b3940a09683e924ef6a92b31a6f7c6de\n000ddeepen 1\n0014deepen-relative\n0018deepen-since 123456\n0013deepen-not tag\n0032want aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +0032have bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\n0009done\n0000"); + Ok(()) + } + } } From bfd8c04d95ddf2e028cc937a26f1a0d03360b403 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Thu, 9 Mar 2023 11:07:17 +0100 Subject: [PATCH 5/9] fix!: rename `WithSidebands::read_line()` to `*::read_line_to_string()`. Also, we remove the existing blocking `read_line()` override as it's not representable in the async implementation anyway, to unify both implementations once again. The new name also makes sure that we are not accidentally calling a built-in implementation on the `BufRead` trait as it's clear our method has to be called under a different name. --- gix-packetline/src/read/sidebands/async_io.rs | 2 +- .../src/read/sidebands/blocking_io.rs | 28 +++++++++++-------- gix-packetline/tests/read/sideband.rs | 20 ++++++------- 3 files changed, 27 insertions(+), 23 deletions(-) diff --git a/gix-packetline/src/read/sidebands/async_io.rs b/gix-packetline/src/read/sidebands/async_io.rs index c8d009ec4a1..b2a91102360 100644 --- a/gix-packetline/src/read/sidebands/async_io.rs +++ b/gix-packetline/src/read/sidebands/async_io.rs @@ -170,7 +170,7 @@ where } /// Read a packet line as string line. - pub fn read_line<'b>(&'b mut self, buf: &'b mut String) -> ReadLineFuture<'a, 'b, T, F> { + pub fn read_line_to_string<'b>(&'b mut self, buf: &'b mut String) -> ReadLineFuture<'a, 'b, T, F> { ReadLineFuture { parent: self, buf } } diff --git a/gix-packetline/src/read/sidebands/blocking_io.rs b/gix-packetline/src/read/sidebands/blocking_io.rs index ea67dc09cef..ee1ce32f29e 100644 --- a/gix-packetline/src/read/sidebands/blocking_io.rs +++ b/gix-packetline/src/read/sidebands/blocking_io.rs @@ -109,6 +109,22 @@ where ); self.parent.read_line() } + + /// Like `BufRead::read_line()`, but will only read one packetline at a time. + /// + /// It will also be easier to call as sometimes it's unclear which implementation we get on a type like this with + /// plenty of generic parameters. + pub fn read_line_to_string(&mut self, buf: &mut String) -> io::Result { + assert_eq!( + self.cap, 0, + "we don't support partial buffers right now - read-line must be used consistently" + ); + let line = std::str::from_utf8(self.fill_buf()?).map_err(|err| io::Error::new(io::ErrorKind::Other, err))?; + buf.push_str(line); + let bytes = line.len(); + self.cap = 0; + Ok(bytes) + } } impl<'a, T, F> BufRead for WithSidebands<'a, T, F> @@ -168,18 +184,6 @@ where fn consume(&mut self, amt: usize) { self.pos = std::cmp::min(self.pos + amt, self.cap); } - - fn read_line(&mut self, buf: &mut String) -> io::Result { - assert_eq!( - self.cap, 0, - "we don't support partial buffers right now - read-line must be used consistently" - ); - let line = std::str::from_utf8(self.fill_buf()?).map_err(|err| io::Error::new(io::ErrorKind::Other, err))?; - buf.push_str(line); - let bytes = line.len(); - self.cap = 0; - Ok(bytes) - } } impl<'a, T, F> io::Read for WithSidebands<'a, T, F> diff --git a/gix-packetline/tests/read/sideband.rs b/gix-packetline/tests/read/sideband.rs index 6345b015ea6..8c685a9dda4 100644 --- a/gix-packetline/tests/read/sideband.rs +++ b/gix-packetline/tests/read/sideband.rs @@ -1,5 +1,5 @@ #[cfg(feature = "blocking-io")] -use std::io::{BufRead, Read}; +use std::io::Read; use bstr::{BString, ByteSlice}; #[cfg(all(not(feature = "blocking-io"), feature = "async-io"))] @@ -106,16 +106,16 @@ async fn read_line_trait_method_reads_one_packet_line_at_a_time() -> crate::Resu let mut out = String::new(); let mut r = rd.as_read(); - r.read_line(&mut out).await?; + r.read_line_to_string(&mut out).await?; assert_eq!(out, "808e50d724f604f69ab93c6da2919c014667bedb HEAD\0multi_ack thin-pack side-band side-band-64k ofs-delta shallow deepen-since deepen-not deepen-relative no-progress include-tag multi_ack_detailed symref=HEAD:refs/heads/master object-format=sha1 agent=git/2.28.0\n"); out.clear(); - r.read_line(&mut out).await?; + r.read_line_to_string(&mut out).await?; assert_eq!(out, "808e50d724f604f69ab93c6da2919c014667bedb refs/heads/master\n"); out.clear(); - r.read_line(&mut out).await?; + r.read_line_to_string(&mut out).await?; assert_eq!(out, "", "flush means empty lines…"); out.clear(); - r.read_line(&mut out).await?; + r.read_line_to_string(&mut out).await?; assert_eq!(out, "", "…which can't be overcome unless the reader is reset"); assert_eq!( r.stopped_at(), @@ -127,18 +127,18 @@ async fn read_line_trait_method_reads_one_packet_line_at_a_time() -> crate::Resu rd.reset(); let mut r = rd.as_read(); - r.read_line(&mut out).await?; + r.read_line_to_string(&mut out).await?; assert_eq!(out, "NAK\n"); drop(r); let mut r = rd.as_read_with_sidebands(|_, _| ()); out.clear(); - r.read_line(&mut out).await?; + r.read_line_to_string(&mut out).await?; assert_eq!(out, "&"); out.clear(); - r.read_line(&mut out).await?; + r.read_line_to_string(&mut out).await?; assert_eq!(out, ""); Ok(()) @@ -199,7 +199,7 @@ async fn peek_past_an_actual_eof_is_an_error() -> crate::Result { assert_eq!(res.expect("one line")??, b"ERR e"); let mut buf = String::new(); - reader.read_line(&mut buf).await?; + reader.read_line_to_string(&mut buf).await?; assert_eq!( buf, "ERR e", "by default ERR lines won't propagate as failure but are merely text" @@ -223,7 +223,7 @@ async fn peek_past_a_delimiter_is_no_error() -> crate::Result { assert_eq!(res.expect("one line")??, b"hello"); let mut buf = String::new(); - reader.read_line(&mut buf).await?; + reader.read_line_to_string(&mut buf).await?; assert_eq!(buf, "hello"); let res = reader.peek_data_line().await; From 397ed90a36c0b86e2c9022b2787656746045c5a3 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Thu, 9 Mar 2023 11:09:57 +0100 Subject: [PATCH 6/9] fix: Assure `ReadlineBufRead::readline_str()` calls our implementation for `Box` as well. This fixes a bug where the HTTP implementation would incorrectly concatenate packetlines without newlines as sent by the server for shallow info lines. --- gix-transport/src/client/async_io/bufread_ext.rs | 4 ++-- gix-transport/src/client/blocking_io/bufread_ext.rs | 7 +++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/gix-transport/src/client/async_io/bufread_ext.rs b/gix-transport/src/client/async_io/bufread_ext.rs index 002af7b8bbf..ce6cb93e6b8 100644 --- a/gix-transport/src/client/async_io/bufread_ext.rs +++ b/gix-transport/src/client/async_io/bufread_ext.rs @@ -94,7 +94,7 @@ impl ReadlineBufRead for gix_packetline::read::WithSideban self.read_data_line().await } async fn readline_str(&mut self, line: &mut String) -> io::Result { - self.read_line(line).await + self.read_line_to_string(line).await } } @@ -104,7 +104,7 @@ impl<'a, T: AsyncRead + Unpin> ReadlineBufRead for gix_packetline::read::WithSid self.read_data_line().await } async fn readline_str(&mut self, line: &mut String) -> io::Result { - self.read_line(line).await + self.read_line_to_string(line).await } } diff --git a/gix-transport/src/client/blocking_io/bufread_ext.rs b/gix-transport/src/client/blocking_io/bufread_ext.rs index 53105d8919d..5dbf26d5306 100644 --- a/gix-transport/src/client/blocking_io/bufread_ext.rs +++ b/gix-transport/src/client/blocking_io/bufread_ext.rs @@ -1,4 +1,3 @@ -use std::io::BufRead; use std::{ io, ops::{Deref, DerefMut}, @@ -58,7 +57,7 @@ impl<'a, T: ReadlineBufRead + ?Sized + 'a> ReadlineBufRead for Box { ReadlineBufRead::readline(self.deref_mut()) } fn readline_str(&mut self, line: &mut String) -> io::Result { - self.read_line(line) + ReadlineBufRead::readline_str(self.deref_mut(), line) } } @@ -86,7 +85,7 @@ impl ReadlineBufRead for gix_packetline::read::WithSidebands<'_, T, } fn readline_str(&mut self, line: &mut String) -> io::Result { - self.read_line(line) + self.read_line_to_string(line) } } @@ -96,7 +95,7 @@ impl<'a, T: io::Read> ReadlineBufRead for gix_packetline::read::WithSidebands<'a } fn readline_str(&mut self, line: &mut String) -> io::Result { - self.read_line(line) + self.read_line_to_string(line) } } From 4e89c19d7656a96bd512dafbc9669011487671f5 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 8 Mar 2023 10:43:54 +0100 Subject: [PATCH 7/9] feat: shallow support for `clone` operations. TODO: more elaborate docs --- crate-status.md | 6 +- gix/src/clone/fetch/mod.rs | 7 + gix/src/clone/mod.rs | 6 +- gix/src/config/tree/sections/clone.rs | 4 +- gix/src/remote/connection/fetch/error.rs | 15 ++ gix/src/remote/connection/fetch/mod.rs | 10 + gix/src/remote/connection/fetch/negotiate.rs | 20 +- .../remote/connection/fetch/receive_pack.rs | 88 +++++++- gix/src/remote/fetch.rs | 46 ++++ gix/src/repository/shallow.rs | 8 +- gix/src/shallow.rs | 67 ++++++ gix/tests/clone/mod.rs | 197 +++++++++++++++++- .../generated-archives/make_diff_repo.tar.xz | 4 +- gix/tests/fixtures/make_fetch_repos.sh | 2 +- gix/tests/fixtures/make_remote_repos.sh | 3 + gix/tests/object/tree/diff.rs | 5 +- gix/tests/remote/fetch.rs | 12 ++ 17 files changed, 479 insertions(+), 21 deletions(-) diff --git a/crate-status.md b/crate-status.md index 1ea98e94bb2..0536784762f 100644 --- a/crate-status.md +++ b/crate-status.md @@ -637,10 +637,14 @@ See its [README.md](https://github.com/Byron/gitoxide/blob/main/gix-lock/README. * [x] find remote itself - [ ] respect `branch..merge` in the returned remote. * **remotes** - * [ ] clone + * [x] clone * [ ] shallow + * [ ] include-tags when shallow is used (needs separate fetch) + * [ ] prune non-existing shallow commits * [ ] [bundles](https://git-scm.com/docs/git-bundle) * [x] fetch + * [ ] 'ref-in-want' + * [ ] standard negotiation algorithms (right now we only have a 'naive' one) * [ ] push * [x] ls-refs * [x] ls-refs with ref-spec filter diff --git a/gix/src/clone/fetch/mod.rs b/gix/src/clone/fetch/mod.rs index d663b47eafe..ea2743812e5 100644 --- a/gix/src/clone/fetch/mod.rs +++ b/gix/src/clone/fetch/mod.rs @@ -121,6 +121,7 @@ impl PrepareFetch { .with_reflog_message(RefLogMessage::Override { message: reflog_message.clone(), }) + .with_shallow(self.shallow.clone()) .receive(should_interrupt)?; util::append_config_to_repo_config(repo, config); @@ -184,6 +185,12 @@ impl PrepareFetch { self.remote_name = Some(crate::remote::name::validated(name)?); Ok(self) } + + /// Make this clone a shallow one with the respective choice of shallow-ness. + pub fn with_shallow(mut self, shallow: crate::remote::fetch::Shallow) -> Self { + self.shallow = shallow; + self + } } /// Consumption diff --git a/gix/src/clone/mod.rs b/gix/src/clone/mod.rs index 249a66a42f0..146bf8fa64d 100644 --- a/gix/src/clone/mod.rs +++ b/gix/src/clone/mod.rs @@ -1,7 +1,7 @@ #![allow(clippy::result_large_err)] use std::convert::TryInto; -use crate::{bstr::BString, config::tree::gitoxide}; +use crate::{bstr::BString, config::tree::gitoxide, remote}; type ConfigureRemoteFn = Box) -> Result, Box>>; @@ -22,6 +22,9 @@ pub struct PrepareFetch { /// The url to clone from #[cfg_attr(not(feature = "blocking-network-client"), allow(dead_code))] url: gix_url::Url, + /// How to handle shallow clones + #[cfg_attr(not(feature = "blocking-network-client"), allow(dead_code))] + shallow: remote::fetch::Shallow, } /// The error returned by [`PrepareFetch::new()`]. @@ -99,6 +102,7 @@ impl PrepareFetch { repo: Some(repo), remote_name: None, configure_remote: None, + shallow: remote::fetch::Shallow::NoChange, }) } } diff --git a/gix/src/config/tree/sections/clone.rs b/gix/src/config/tree/sections/clone.rs index 616185a0b71..6cb274e7dc7 100644 --- a/gix/src/config/tree/sections/clone.rs +++ b/gix/src/config/tree/sections/clone.rs @@ -7,6 +7,8 @@ impl Clone { /// The `clone.defaultRemoteName` key. pub const DEFAULT_REMOTE_NAME: keys::RemoteName = keys::RemoteName::new_remote_name("defaultRemoteName", &config::Tree::CLONE); + /// The `clone.rejectShallow` key. + pub const REJECT_SHALLOW: keys::Boolean = keys::Boolean::new_boolean("rejectShallow", &config::Tree::CLONE); } impl Section for Clone { @@ -15,6 +17,6 @@ impl Section for Clone { } fn keys(&self) -> &[&dyn Key] { - &[&Self::DEFAULT_REMOTE_NAME] + &[&Self::DEFAULT_REMOTE_NAME, &Self::REJECT_SHALLOW] } } diff --git a/gix/src/remote/connection/fetch/error.rs b/gix/src/remote/connection/fetch/error.rs index 0e6a4b84098..afcacca1379 100644 --- a/gix/src/remote/connection/fetch/error.rs +++ b/gix/src/remote/connection/fetch/error.rs @@ -28,6 +28,21 @@ pub enum Error { path: std::path::PathBuf, source: std::io::Error, }, + #[error(transparent)] + ShallowOpen(#[from] crate::shallow::open::Error), + #[error("Server lack feature {feature:?}: {description}")] + MissingServerFeature { + feature: &'static str, + description: &'static str, + }, + #[error("Could not write 'shallow' file to incorporate remote updates after fetching")] + WriteShallowFile(#[from] crate::shallow::write::Error), + #[error("'shallow' file could not be locked in preparation for writing changes")] + LockShallowFile(#[from] gix_lock::acquire::Error), + #[error("Could not obtain configuration to learn if shallow remotes should be rejected")] + RejectShallowRemoteConfig(#[from] config::boolean::Error), + #[error("Receiving objects from shallow remotes is prohibited due to the value of `clone.rejectShallow`")] + RejectShallowRemote, } impl gix_protocol::transport::IsSpuriousError for Error { diff --git a/gix/src/remote/connection/fetch/mod.rs b/gix/src/remote/connection/fetch/mod.rs index 4ce631b1ef5..b205b1341a0 100644 --- a/gix/src/remote/connection/fetch/mod.rs +++ b/gix/src/remote/connection/fetch/mod.rs @@ -149,6 +149,7 @@ where dry_run: DryRun::No, reflog_message: None, write_packed_refs: WritePackedRefs::Never, + shallow: Default::default(), }) } } @@ -179,6 +180,7 @@ where dry_run: DryRun, reflog_message: Option, write_packed_refs: WritePackedRefs, + shallow: remote::fetch::Shallow, } /// Builder @@ -212,6 +214,14 @@ where self.reflog_message = reflog_message.into(); self } + + /// Define what to do when the current repository is a shallow clone. + /// + /// *Has no effect if the current repository is not as shallow clone.* + pub fn with_shallow(mut self, shallow: remote::fetch::Shallow) -> Self { + self.shallow = shallow; + self + } } impl<'remote, 'repo, T, P> Drop for Prepare<'remote, 'repo, T, P> diff --git a/gix/src/remote/connection/fetch/negotiate.rs b/gix/src/remote/connection/fetch/negotiate.rs index f5051ec72ff..605f442a3d4 100644 --- a/gix/src/remote/connection/fetch/negotiate.rs +++ b/gix/src/remote/connection/fetch/negotiate.rs @@ -1,3 +1,5 @@ +use gix_refspec::RefSpec; + /// The way the negotiation is performed #[derive(Copy, Clone)] pub(crate) enum Algorithm { @@ -16,6 +18,7 @@ pub enum Error { /// Negotiate one round with `algo` by looking at `ref_map` and adjust `arguments` to contain the haves and wants. /// If this is not the first round, the `previous_response` is set with the last recorded server response. /// Returns `true` if the negotiation is done from our side so the server won't keep asking. +#[allow(clippy::too_many_arguments)] pub(crate) fn one_round( algo: Algorithm, round: usize, @@ -24,13 +27,15 @@ pub(crate) fn one_round( fetch_tags: crate::remote::fetch::Tags, arguments: &mut gix_protocol::fetch::Arguments, _previous_response: Option<&gix_protocol::fetch::Response>, + wants_shallow_change: Option<&[RefSpec]>, ) -> Result { let tag_refspec_to_ignore = fetch_tags .to_refspec() .filter(|_| matches!(fetch_tags, crate::remote::fetch::Tags::Included)); + let non_wildcard_specs_only = wants_shallow_change; match algo { Algorithm::Naive => { - assert_eq!(round, 1, "Naive always finishes after the first round, and claims."); + assert_eq!(round, 1, "Naive always finishes after the first round, it claims."); let mut has_missing_tracking_branch = false; for mapping in &ref_map.mappings { if tag_refspec_to_ignore.map_or(false, |tag_spec| { @@ -42,6 +47,14 @@ pub(crate) fn one_round( }) { continue; } + if non_wildcard_specs_only + .and_then(|refspecs| mapping.spec_index.get(refspecs, &ref_map.extra_refspecs)) + .map_or(false, |spec| { + spec.to_ref().local().map_or(false, |ref_| ref_.contains(&b'*')) + }) + { + continue; + } let have_id = mapping.local.as_ref().and_then(|name| { repo.find_reference(name) .ok() @@ -50,7 +63,7 @@ pub(crate) fn one_round( match have_id { Some(have_id) => { if let Some(want_id) = mapping.remote.as_id() { - if want_id != have_id { + if want_id != have_id || wants_shallow_change.is_some() { arguments.want(want_id); arguments.have(have_id); } @@ -65,10 +78,11 @@ pub(crate) fn one_round( } } - if has_missing_tracking_branch { + if has_missing_tracking_branch || (wants_shallow_change.is_some() && arguments.is_empty()) { if let Ok(Some(r)) = repo.head_ref() { if let Some(id) = r.target().try_id() { arguments.have(id); + arguments.want(id); } } } diff --git a/gix/src/remote/connection/fetch/receive_pack.rs b/gix/src/remote/connection/fetch/receive_pack.rs index 686de599919..c67f8a95570 100644 --- a/gix/src/remote/connection/fetch/receive_pack.rs +++ b/gix/src/remote/connection/fetch/receive_pack.rs @@ -1,16 +1,18 @@ use std::sync::atomic::AtomicBool; use gix_odb::FindExt; +use gix_protocol::fetch::Arguments; use gix_protocol::transport::client::Transport; +use crate::config::tree::Clone; use crate::{ remote, remote::{ connection::fetch::config, fetch, - fetch::{negotiate, refs, Error, Outcome, Prepare, ProgressId, RefLogMessage, Status}, + fetch::{negotiate, refs, Error, Outcome, Prepare, ProgressId, RefLogMessage, Shallow, Status}, }, - Progress, + Progress, Repository, }; impl<'remote, 'repo, T, P> Prepare<'remote, 'repo, T, P> @@ -82,10 +84,17 @@ where let mut arguments = gix_protocol::fetch::Arguments::new(protocol_version, fetch_features); if matches!(con.remote.fetch_tags, crate::remote::fetch::Tags::Included) { if !arguments.can_use_include_tag() { - unimplemented!("we expect servers to support 'include-tag', otherwise we have to implement another pass to fetch attached tags separately"); + return Err(Error::MissingServerFeature { + feature: "include-tag", + description: + // NOTE: if this is an issue, we could probably do what's proposed here. + "To make this work we would have to implement another pass to fetch attached tags separately", + }); } arguments.use_include_tag(); } + let (shallow_commits, mut shallow_lock) = add_shallow_args(&mut arguments, &self.shallow, repo)?; + let mut previous_response = None::; let mut round = 1; @@ -108,6 +117,7 @@ where con.remote.fetch_tags, &mut arguments, previous_response.as_ref(), + (self.shallow != Shallow::NoChange).then(|| con.remote.refspecs(remote::Direction::Fetch)), ) { Ok(_) if arguments.is_empty() => { gix_protocol::indicate_end_of_interaction(&mut con.transport).await.ok(); @@ -146,11 +156,26 @@ where if !sideband_all { setup_remote_progress(progress, &mut reader); } + previous_response = Some(response); break 'negotiation reader; } else { previous_response = Some(response); } }; + let previous_response = previous_response.expect("knowledge of a pack means a response was received"); + if !previous_response.shallow_updates().is_empty() && shallow_lock.is_none() { + let reject_shallow_remote = repo + .config + .resolved + .boolean_filter_by_key("clone.rejectShallow", &mut repo.filter_config_section()) + .map(|val| Clone::REJECT_SHALLOW.enrich_error(val)) + .transpose()? + .unwrap_or(false); + if reject_shallow_remote { + return Err(Error::RejectShallowRemote); + } + shallow_lock = acquire_shallow_lock(repo).map(Some)?; + } let options = gix_pack::bundle::write::Options { thread_limit: config::index_threads(repo)?, @@ -187,6 +212,12 @@ where gix_protocol::indicate_end_of_interaction(&mut con.transport).await.ok(); } + if let Some(shallow_lock) = shallow_lock { + if !previous_response.shallow_updates().is_empty() { + crate::shallow::write(shallow_lock, shallow_commits, previous_response.shallow_updates())?; + } + } + let update_refs = refs::update( repo, self.reflog_message @@ -221,6 +252,57 @@ where } } +fn acquire_shallow_lock(repo: &Repository) -> Result { + gix_lock::File::acquire_to_update_resource(repo.shallow_file(), gix_lock::acquire::Fail::Immediately, None) + .map_err(Into::into) +} + +fn add_shallow_args( + args: &mut Arguments, + shallow: &Shallow, + repo: &Repository, +) -> Result<(Option, Option), Error> { + let expect_change = *shallow != Shallow::NoChange; + let shallow_lock = expect_change.then(|| acquire_shallow_lock(repo)).transpose()?; + + let shallow_commits = repo.shallow_commits()?; + if (shallow_commits.is_some() || expect_change) && !args.can_use_shallow() { + // NOTE: if this is an issue, we can always unshallow the repo ourselves. + return Err(Error::MissingServerFeature { + feature: "shallow", + description: "shallow clones need server support to remain shallow, otherwise bigger than expected packs are sent effectively unshallowing the repository", + }); + } + if let Some(shallow_commits) = &shallow_commits { + for commit in shallow_commits.iter() { + args.shallow(commit); + } + } + match shallow { + Shallow::NoChange => {} + Shallow::DepthAtRemote(commits) => args.deepen(commits.get() as usize), + Shallow::Deepen(commits) => { + args.deepen(*commits as usize); + args.deepen_relative(); + } + Shallow::Since { cutoff } => { + args.deepen_since(cutoff.seconds_since_unix_epoch as usize); + } + Shallow::Exclude { + remote_refs, + since_cutoff, + } => { + if let Some(cutoff) = since_cutoff { + args.deepen_since(cutoff.seconds_since_unix_epoch as usize); + } + for ref_ in remote_refs { + args.deepen_not(ref_.as_ref().as_bstr()); + } + } + } + Ok((shallow_commits, shallow_lock)) +} + fn setup_remote_progress

( progress: &mut P, reader: &mut Box, diff --git a/gix/src/remote/fetch.rs b/gix/src/remote/fetch.rs index 4add96a6518..f8a58c19755 100644 --- a/gix/src/remote/fetch.rs +++ b/gix/src/remote/fetch.rs @@ -52,6 +52,52 @@ impl Tags { } } +/// Describe how shallow clones are handled when fetching, with variants defining how the *shallow boundary* is handled. +/// +/// The *shallow boundary* is a set of commits whose parents are not present in the repository. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Shallow { + /// Fetch all changes from the remote without affecting the shallow boundary at all. + NoChange, + /// Receive update to `depth` commits in the history of the refs to fetch (from the viewpoint of the remote), + /// with the value of `1` meaning to receive only the commit a ref is pointing to. + /// + /// This may update the shallow boundary to increase or decrease the amount of available history. + DepthAtRemote(std::num::NonZeroU32), + /// Increase the number of commits and thus expand the shallow boundary by `depth` commits as seen from our local + /// shallow boundary, with a value of `0` having no effect. + Deepen(u32), + /// Set the shallow boundary at the `cutoff` time, meaning that there will be no commits beyond that time. + Since { + /// The date beyond which there will be no history. + cutoff: gix_date::Time, + }, + /// Receive all history excluding all commits reachable from `remote_refs`. These can be long or short + /// ref names or tag names. + Exclude { + /// The ref names to exclude, short or long. Note that ambiguous short names will cause the remote to abort + /// without an error message being transferred (because the protocol does not support it) + remote_refs: Vec, + /// If some, this field has the same meaning as [`Shallow::Since`] which can be used in combination + /// with excluded references. + since_cutoff: Option, + }, +} + +impl Default for Shallow { + fn default() -> Self { + Shallow::NoChange + } +} + +impl Shallow { + /// Produce a variant that causes the repository to loose its shallow boundary, effectively by extending it + /// beyond all limits. + pub fn undo() -> Self { + Shallow::DepthAtRemote((i32::MAX as u32).try_into().expect("valid at compile time")) + } +} + /// Information about the relationship between our refspecs, and remote references with their local counterparts. #[derive(Default, Debug, Clone)] #[cfg(any(feature = "blocking-network-client", feature = "async-network-client"))] diff --git a/gix/src/repository/shallow.rs b/gix/src/repository/shallow.rs index d6b8562ca5a..22d2d5f955e 100644 --- a/gix/src/repository/shallow.rs +++ b/gix/src/repository/shallow.rs @@ -12,8 +12,12 @@ impl Repository { .map_or(false, |m| m.is_file() && m.len() > 0) } - /// Return a shared list of shallow commits which is updated automatically if the in-memory snapshot has become stale as the underlying file - /// on disk has changed. + /// Return a shared list of shallow commits which is updated automatically if the in-memory snapshot has become stale + /// as the underlying file on disk has changed. + /// + /// The list of shallow commits represents the shallow boundary, beyond which we are lacking all (parent) commits. + /// Note that the list is never empty, as `Ok(None)` is returned in that case indicating the repository + /// isn't a shallow clone. /// /// The shared list is shared across all clones of this repository. pub fn shallow_commits(&self) -> Result, crate::shallow::open::Error> { diff --git a/gix/src/shallow.rs b/gix/src/shallow.rs index 62e20e732ef..0d8a61d5fd0 100644 --- a/gix/src/shallow.rs +++ b/gix/src/shallow.rs @@ -4,6 +4,73 @@ pub(crate) type CommitsStorage = /// sorted to allow bisecting. pub type Commits = gix_features::fs::SharedSnapshot>; +/// +#[cfg(any(feature = "blocking-network-client", feature = "async-network-client"))] +pub mod write { + pub(crate) mod function { + use crate::shallow::{write::Error, Commits}; + use gix_protocol::fetch::response::ShallowUpdate; + use std::io::Write; + + /// Write the previously obtained (possibly non-existing) `shallow_commits` to the shallow `file` + /// after applying all `updates`. + /// + /// If this leaves the list of shallow commits empty, the file is removed. + /// + /// ### Deviation + /// + /// Git also prunes the set of shallow commits while writing, we don't until we support some sort of pruning. + pub fn write( + mut file: gix_lock::File, + shallow_commits: Option, + updates: &[ShallowUpdate], + ) -> Result<(), Error> { + let mut shallow_commits = shallow_commits.map(|sc| (**sc).to_owned()).unwrap_or_default(); + for update in updates { + match update { + ShallowUpdate::Shallow(id) => { + shallow_commits.push(*id); + } + ShallowUpdate::Unshallow(id) => shallow_commits.retain(|oid| oid != id), + } + } + + if shallow_commits.is_empty() { + if let Err(err) = std::fs::remove_file(file.resource_path()) { + if err.kind() != std::io::ErrorKind::NotFound { + return Err(err.into()); + } + } + } else { + shallow_commits.sort(); + let mut buf = Vec::::new(); + for commit in shallow_commits { + commit.write_hex_to(&mut buf).map_err(Error::Io)?; + buf.push(b'\n'); + } + file.write_all(&buf).map_err(Error::Io)?; + file.flush()?; + } + file.commit()?; + Ok(()) + } + } + + /// The error returned by [`write()`][crate::shallow::write()]. + #[derive(Debug, thiserror::Error)] + #[allow(missing_docs)] + pub enum Error { + #[error(transparent)] + Commit(#[from] gix_lock::commit::Error), + #[error("Could not remove an empty shallow file")] + RemoveEmpty(#[from] std::io::Error), + #[error("Failed to write object id to shallow file")] + Io(std::io::Error), + } +} +#[cfg(any(feature = "blocking-network-client", feature = "async-network-client"))] +pub use write::function::write; + /// pub mod open { /// The error returned by [`Repository::shallow_commits()`][crate::Repository::shallow_commits()]. diff --git a/gix/tests/clone/mod.rs b/gix/tests/clone/mod.rs index 821b8d8f618..77a82c58d5d 100644 --- a/gix/tests/clone/mod.rs +++ b/gix/tests/clone/mod.rs @@ -2,12 +2,203 @@ use crate::{remote, util::restricted}; #[cfg(feature = "blocking-network-client")] mod blocking_io { - use gix::remote::fetch::SpecIndex; + use gix::config::tree::{Clone, Core, Init, Key}; + use gix::remote::Direction; use gix_object::bstr::ByteSlice; use gix_ref::TargetRef; + use std::sync::atomic::AtomicBool; use crate::{remote, util::restricted}; + use crate::util::hex_to_id; + use gix::remote::fetch::{Shallow, SpecIndex}; + + #[test] + fn fetch_shallow_no_checkout_then_unshallow() -> crate::Result { + let tmp = gix_testtools::tempfile::TempDir::new()?; + let called_configure_remote = std::sync::Arc::new(std::sync::atomic::AtomicBool::default()); + let remote_name = "special"; + let desired_fetch_tags = gix::remote::fetch::Tags::Included; + let mut prepare = gix::prepare_clone_bare(remote::repo("base").path(), tmp.path())? + .with_remote_name(remote_name)? + .configure_remote({ + let called_configure_remote = called_configure_remote; + move |r| { + called_configure_remote.store(true, std::sync::atomic::Ordering::Relaxed); + let r = r + .with_refspecs(Some("+refs/tags/b-tag:refs/tags/b-tag"), gix::remote::Direction::Fetch)? + .with_fetch_tags(desired_fetch_tags); + Ok(r) + } + }) + .with_shallow(Shallow::DepthAtRemote(2.try_into().expect("non-zero"))); + let (repo, _out) = prepare.fetch_only(gix::progress::Discard, &std::sync::atomic::AtomicBool::default())?; + drop(prepare); + + assert_eq!( + repo.shallow_commits()?.expect("shallow").as_slice(), + [ + hex_to_id("27e71576a6335294aa6073ab767f8b36bdba81d0"), + hex_to_id("2d9d136fb0765f2e24c44a0f91984318d580d03b"), + hex_to_id("82024b2ef7858273337471cbd1ca1cedbdfd5616"), + hex_to_id("b5152869aedeb21e55696bb81de71ea1bb880c85") + ], + "shallow information is written" + ); + + let shallow_commit_count = repo.head_id()?.ancestors().all()?.count(); + let remote = repo.head()?.into_remote(Direction::Fetch).expect("present")?; + + remote + .connect(Direction::Fetch, gix::progress::Discard)? + .prepare_fetch(Default::default())? + .with_shallow(Shallow::undo()) + .receive(&AtomicBool::default())?; + + assert!(repo.shallow_commits()?.is_none(), "the repo isn't shallow anymore"); + assert!( + !repo.is_shallow(), + "both methods agree - if there are no shallow commits, it shouldn't think the repo is shallow" + ); + assert!( + repo.head_id()?.ancestors().all()?.count() > shallow_commit_count, + "there are more commits now as the history is complete" + ); + + Ok(()) + } + + #[test] + fn from_shallow_prohibited_with_option() -> crate::Result { + let tmp = gix_testtools::tempfile::TempDir::new()?; + let err = gix::clone::PrepareFetch::new( + remote::repo("base.shallow").path(), + tmp.path(), + gix::create::Kind::Bare, + Default::default(), + gix::open::Options::isolated().config_overrides([Clone::REJECT_SHALLOW.validated_assignment_fmt(&true)?]), + )? + .fetch_only(gix::progress::Discard, &std::sync::atomic::AtomicBool::default()) + .unwrap_err(); + assert!( + matches!( + err, + gix::clone::fetch::Error::Fetch(gix::remote::fetch::Error::RejectShallowRemote) + ), + "we can avoid fetching from remotes with this setting" + ); + Ok(()) + } + + #[test] + fn from_shallow_allowed_by_default() -> crate::Result { + let tmp = gix_testtools::tempfile::TempDir::new()?; + let (repo, _change) = gix::prepare_clone_bare(remote::repo("base.shallow").path(), tmp.path())? + .fetch_only(gix::progress::Discard, &std::sync::atomic::AtomicBool::default())?; + assert_eq!( + repo.shallow_commits()?.expect("present").as_slice(), + vec![ + hex_to_id("2d9d136fb0765f2e24c44a0f91984318d580d03b"), + hex_to_id("dfd0954dabef3b64f458321ef15571cc1a46d552"), + hex_to_id("dfd0954dabef3b64f458321ef15571cc1a46d552"), + ] + ); + Ok(()) + } + + #[test] + fn from_non_shallow_then_deepen_then_deepen_since_to_unshallow() -> crate::Result { + let tmp = gix_testtools::tempfile::TempDir::new()?; + let (repo, _change) = gix::prepare_clone_bare(remote::repo("base").path(), tmp.path())? + .with_shallow(Shallow::DepthAtRemote(2.try_into()?)) + .fetch_only(gix::progress::Discard, &std::sync::atomic::AtomicBool::default())?; + + assert!(repo.is_shallow()); + assert_eq!( + repo.shallow_commits()?.expect("present").as_slice(), + vec![ + hex_to_id("2d9d136fb0765f2e24c44a0f91984318d580d03b"), + hex_to_id("dfd0954dabef3b64f458321ef15571cc1a46d552"), + ] + ); + + let shallow_commit_count = repo.head_id()?.ancestors().all()?.count(); + + let remote = repo.head()?.into_remote(Direction::Fetch).expect("present")?; + remote + .connect(Direction::Fetch, gix::progress::Discard)? + .prepare_fetch(Default::default())? + .with_shallow(Shallow::Deepen(1)) + .receive(&AtomicBool::default())?; + + assert_eq!( + repo.shallow_commits()?.expect("present").as_slice(), + vec![ + hex_to_id("27e71576a6335294aa6073ab767f8b36bdba81d0"), + hex_to_id("82024b2ef7858273337471cbd1ca1cedbdfd5616"), + hex_to_id("b5152869aedeb21e55696bb81de71ea1bb880c85"), + ], + "the shallow boundary was changed" + ); + assert!( + repo.head_id()?.ancestors().all()?.count() > shallow_commit_count, + "there are more commits now as the history was deepened" + ); + + let shallow_commit_count = repo.head_id()?.ancestors().all()?.count(); + remote + .connect(Direction::Fetch, gix::progress::Discard)? + .prepare_fetch(Default::default())? + .with_shallow(Shallow::Since { + cutoff: gix::date::Time::new(1112354053, 0), + }) + .receive(&AtomicBool::default())?; + + assert!( + !repo.is_shallow(), + "the cutoff date is before the first commit, effectively unshallowing" + ); + assert!( + repo.head_id()?.ancestors().all()?.count() > shallow_commit_count, + "there is even more commits than previously" + ); + Ok(()) + } + + #[test] + fn from_non_shallow_by_deepen_exclude_then_deepen_to_unshallow() -> crate::Result { + let tmp = gix_testtools::tempfile::TempDir::new()?; + let excluded_leaf_refs = ["g", "h", "j"]; + let (repo, _change) = gix::prepare_clone_bare(remote::repo("base").path(), tmp.path())? + .with_shallow(Shallow::Exclude { + remote_refs: excluded_leaf_refs + .into_iter() + .map(|n| n.try_into().expect("valid")) + .collect(), + since_cutoff: None, + }) + .fetch_only(gix::progress::Discard, &std::sync::atomic::AtomicBool::default())?; + + assert!(repo.is_shallow()); + assert_eq!( + repo.shallow_commits()?.expect("present").as_slice(), + vec![ + hex_to_id("27e71576a6335294aa6073ab767f8b36bdba81d0"), + hex_to_id("82024b2ef7858273337471cbd1ca1cedbdfd5616"), + ] + ); + + let remote = repo.head()?.into_remote(Direction::Fetch).expect("present")?; + remote + .connect(Direction::Fetch, gix::progress::Discard)? + .prepare_fetch(Default::default())? + .with_shallow(Shallow::Deepen(2)) + .receive(&AtomicBool::default())?; + + assert!(!repo.is_shallow(), "one is just enough to unshallow it"); + Ok(()) + } + #[test] fn fetch_only_with_configuration() -> crate::Result { let tmp = gix_testtools::tempfile::TempDir::new()?; @@ -20,8 +211,8 @@ mod blocking_io { gix::create::Kind::Bare, Default::default(), gix::open::Options::isolated().config_overrides([ - "init.defaultBranch=unused-as-overridden-by-remote", - "core.logAllRefUpdates", + Init::DEFAULT_BRANCH.validated_assignment_fmt(&"unused-as-overridden-by-remote")?, + Core::LOG_ALL_REF_UPDATES.logical_name().into(), // missing user and email is acceptable in this special case, i.e. `git` also doesn't mind filling it in. ]), )? diff --git a/gix/tests/fixtures/generated-archives/make_diff_repo.tar.xz b/gix/tests/fixtures/generated-archives/make_diff_repo.tar.xz index d31727f442c..0d6b76452b7 100644 --- a/gix/tests/fixtures/generated-archives/make_diff_repo.tar.xz +++ b/gix/tests/fixtures/generated-archives/make_diff_repo.tar.xz @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:f0ffd6adef1a5755c2013dd7e793ca8e239308448872681a36a2216ea56623b2 -size 32032 +oid sha256:dceeedc3ee706632030f8317f2245c327456ec58a6ac690d09099557a5aa23b0 +size 32336 diff --git a/gix/tests/fixtures/make_fetch_repos.sh b/gix/tests/fixtures/make_fetch_repos.sh index 9bb598ea918..af77c24d025 100644 --- a/gix/tests/fixtures/make_fetch_repos.sh +++ b/gix/tests/fixtures/make_fetch_repos.sh @@ -1,7 +1,7 @@ set -eu -o pipefail # IMPORTANT: keep this repo small as it's used for writes, hence will be executed for each writer! -git clone --bare "${1:?First argument is the complex base repo from make_remote_repos.sh/base}" base +git clone ${2:-} --bare "${1:?First argument is the complex base repo from make_remote_repos.sh/base}" base git clone --shared base clone-as-base-with-changes (cd clone-as-base-with-changes diff --git a/gix/tests/fixtures/make_remote_repos.sh b/gix/tests/fixtures/make_remote_repos.sh index e32f45e1a28..85d2775d7b6 100644 --- a/gix/tests/fixtures/make_remote_repos.sh +++ b/gix/tests/fixtures/make_remote_repos.sh @@ -107,6 +107,9 @@ git init base git branch -D tmp ) +git clone --shared --depth 2 file://$PWD/base base.shallow + + git clone --shared base clone (cd clone git remote add myself . diff --git a/gix/tests/object/tree/diff.rs b/gix/tests/object/tree/diff.rs index 936ed3b0e72..6efa203f9b8 100644 --- a/gix/tests/object/tree/diff.rs +++ b/gix/tests/object/tree/diff.rs @@ -591,10 +591,7 @@ mod track_rewrites { out.num_similarity_checks_skipped_for_rename_tracking_due_to_limit, 0, "no limit configured" ); - assert_eq!( - out.num_similarity_checks_skipped_for_copy_tracking_due_to_limit, - if cfg!(windows) { 63 } else { 57 } - ); + assert_eq!(out.num_similarity_checks_skipped_for_copy_tracking_due_to_limit, 57); Ok(()) } diff --git a/gix/tests/remote/fetch.rs b/gix/tests/remote/fetch.rs index 240c158686f..1f75116aeba 100644 --- a/gix/tests/remote/fetch.rs +++ b/gix/tests/remote/fetch.rs @@ -1,3 +1,15 @@ +mod shallow { + use gix::remote::fetch::Shallow; + + #[test] + fn undo() { + assert_eq!( + Shallow::undo(), + Shallow::DepthAtRemote(2147483647u32.try_into().expect("known at compile time")) + ); + } +} + #[cfg(any(feature = "blocking-network-client", feature = "async-network-client-async-std"))] mod blocking_and_async_io { use std::sync::atomic::AtomicBool; From 93d412c54833d822e5369644226c6fd3b888c89c Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 8 Mar 2023 10:13:42 +0100 Subject: [PATCH 8/9] feat: shallow support for `fetch` operations. TBD: more elaborate docs --- gix/tests/remote/fetch.rs | 77 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-) diff --git a/gix/tests/remote/fetch.rs b/gix/tests/remote/fetch.rs index 1f75116aeba..7b492517cc9 100644 --- a/gix/tests/remote/fetch.rs +++ b/gix/tests/remote/fetch.rs @@ -14,6 +14,7 @@ mod shallow { mod blocking_and_async_io { use std::sync::atomic::AtomicBool; + use gix::remote::fetch::Status; use gix::remote::{fetch, Direction::Fetch}; use gix_features::progress; use gix_protocol::maybe_async; @@ -43,10 +44,32 @@ mod blocking_and_async_io { #[allow(clippy::result_large_err)] pub(crate) fn try_repo_rw( name: &str, + ) -> Result<(gix::Repository, gix_testtools::tempfile::TempDir), gix::open::Error> { + try_repo_rw_args(name, Vec::::new(), Mode::FastClone) + } + + pub(crate) enum Mode { + FastClone, + CloneWithShallowSupport, + } + + #[allow(clippy::result_large_err)] + pub(crate) fn try_repo_rw_args>( + name: &str, + args: impl IntoIterator, + mode: Mode, ) -> Result<(gix::Repository, gix_testtools::tempfile::TempDir), gix::open::Error> { let dir = gix_testtools::scripted_fixture_writable_with_args( "make_fetch_repos.sh", - [base_repo_path()], + [{ + let mut url = base_repo_path(); + if matches!(mode, Mode::CloneWithShallowSupport) { + url.insert_str(0, "file://"); + } + url + }] + .into_iter() + .chain(args.into_iter().map(Into::into)), gix_testtools::Creation::ExecuteScript, ) .unwrap(); @@ -84,6 +107,58 @@ mod blocking_and_async_io { Ok(()) } + #[maybe_async::test( + feature = "blocking-network-client", + async(feature = "async-network-client-async-std", async_std::test) + )] + async fn fetch_shallow_deepen_not_possible() -> crate::Result { + let (repo, _tmp) = try_repo_rw_args("two-origins", ["--depth=2"], Mode::CloneWithShallowSupport)?; + let remote = repo + .head()? + .into_remote(Fetch) + .expect("present")? + .with_fetch_tags(fetch::Tags::Included); + + assert_eq!( + repo.shallow_commits()?.expect("shallow clone").as_slice(), + [ + hex_to_id("2d9d136fb0765f2e24c44a0f91984318d580d03b"), + hex_to_id("dfd0954dabef3b64f458321ef15571cc1a46d552"), + hex_to_id("dfd0954dabef3b64f458321ef15571cc1a46d552") + ] + ); + let prev_commits = repo.head_id()?.ancestors().all()?.count(); + let changes = remote + .connect(Fetch, gix::progress::Discard) + .await? + .prepare_fetch(Default::default()) + .await? + .with_shallow(fetch::Shallow::Deepen(1)) + .receive(&AtomicBool::default()) + .await?; + + assert!( + matches!(changes.status, Status::Change {write_pack_bundle, ..} if write_pack_bundle.index.num_objects == 0), + "we get an empty pack as there is nothing to do" + ); + + assert_eq!( + repo.shallow_commits()?.expect("shallow clone").as_slice(), + [ + hex_to_id("2d9d136fb0765f2e24c44a0f91984318d580d03b"), + hex_to_id("dfd0954dabef3b64f458321ef15571cc1a46d552"), + hex_to_id("dfd0954dabef3b64f458321ef15571cc1a46d552") + ], + "the base is shallow, and so is the clone, and we can't extend further" + ); + assert_eq!( + repo.head_id()?.ancestors().all()?.count(), + prev_commits, + "no more commits are available - there simply isn't more information" + ); + Ok(()) + } + #[maybe_async::test( feature = "blocking-network-client", async(feature = "async-network-client-async-std", async_std::test) From 9723e1addf52cc336d59322de039ea0537cdca36 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 8 Mar 2023 10:14:39 +0100 Subject: [PATCH 9/9] feat: `gix clone` and `gix fetch` with controls for shallow repositories. TBD: more elaborate documentation --- gitoxide-core/src/repository/clone.rs | 7 ++- gitoxide-core/src/repository/fetch.rs | 3 + src/plumbing/main.rs | 4 ++ src/plumbing/options/mod.rs | 87 +++++++++++++++++++++++++++ src/plumbing/progress.rs | 8 ++- src/shared.rs | 30 ++++++++- 6 files changed, 134 insertions(+), 5 deletions(-) diff --git a/gitoxide-core/src/repository/clone.rs b/gitoxide-core/src/repository/clone.rs index e5a0bc55d0b..789087b62f2 100644 --- a/gitoxide-core/src/repository/clone.rs +++ b/gitoxide-core/src/repository/clone.rs @@ -5,6 +5,7 @@ pub struct Options { pub bare: bool, pub handshake_info: bool, pub no_tags: bool, + pub shallow: gix::remote::fetch::Shallow, } pub const PROGRESS_RANGE: std::ops::RangeInclusive = 1..=3; @@ -30,6 +31,7 @@ pub(crate) mod function { handshake_info, bare, no_tags, + shallow, }: Options, ) -> anyhow::Result<()> where @@ -66,8 +68,9 @@ pub(crate) mod function { if no_tags { prepare = prepare.configure_remote(|r| Ok(r.with_fetch_tags(gix::remote::fetch::Tags::None))); } - let (mut checkout, fetch_outcome) = - prepare.fetch_then_checkout(&mut progress, &gix::interrupt::IS_INTERRUPTED)?; + let (mut checkout, fetch_outcome) = prepare + .with_shallow(shallow) + .fetch_then_checkout(&mut progress, &gix::interrupt::IS_INTERRUPTED)?; let (repo, outcome) = if bare { (checkout.persist(), None) diff --git a/gitoxide-core/src/repository/fetch.rs b/gitoxide-core/src/repository/fetch.rs index 580cc4d9193..e942074cf88 100644 --- a/gitoxide-core/src/repository/fetch.rs +++ b/gitoxide-core/src/repository/fetch.rs @@ -8,6 +8,7 @@ pub struct Options { pub remote: Option, /// If non-empty, override all ref-specs otherwise configured in the remote pub ref_specs: Vec, + pub shallow: gix::remote::fetch::Shallow, pub handshake_info: bool, } @@ -30,6 +31,7 @@ pub(crate) mod function { dry_run, remote, handshake_info, + shallow, ref_specs, }: Options, ) -> anyhow::Result<()> @@ -50,6 +52,7 @@ pub(crate) mod function { .connect(gix::remote::Direction::Fetch, progress)? .prepare_fetch(Default::default())? .with_dry_run(dry_run) + .with_shallow(shallow) .receive(&gix::interrupt::IS_INTERRUPTED)?; if handshake_info { diff --git a/src/plumbing/main.rs b/src/plumbing/main.rs index 733022fb7c8..3c0af9d0549 100644 --- a/src/plumbing/main.rs +++ b/src/plumbing/main.rs @@ -130,6 +130,7 @@ pub fn main() -> Result<()> { bare, no_tags, remote, + shallow, directory, }) => { let opts = core::repository::clone::Options { @@ -137,6 +138,7 @@ pub fn main() -> Result<()> { bare, handshake_info, no_tags, + shallow: shallow.into(), }; prepare_and_run( "clone", @@ -152,6 +154,7 @@ pub fn main() -> Result<()> { dry_run, handshake_info, remote, + shallow, ref_spec, }) => { let opts = core::repository::fetch::Options { @@ -159,6 +162,7 @@ pub fn main() -> Result<()> { dry_run, remote, handshake_info, + shallow: shallow.into(), ref_specs: ref_spec, }; prepare_and_run( diff --git a/src/plumbing/options/mod.rs b/src/plumbing/options/mod.rs index 54335f70d43..d22d024dbb0 100644 --- a/src/plumbing/options/mod.rs +++ b/src/plumbing/options/mod.rs @@ -135,6 +135,9 @@ pub mod config { #[cfg(feature = "gitoxide-core-blocking-client")] pub mod fetch { + use gix::remote::fetch::Shallow; + use std::num::NonZeroU32; + #[derive(Debug, clap::Parser)] pub struct Platform { /// Don't change the local repository, but otherwise try to be as accurate as possible. @@ -145,6 +148,9 @@ pub mod fetch { #[clap(long, short = 'H')] pub handshake_info: bool, + #[clap(flatten)] + pub shallow: ShallowOptions, + /// The name of the remote to connect to, or the url of the remote to connect to directly. /// /// If unset, the current branch will determine the remote. @@ -155,10 +161,56 @@ pub mod fetch { #[clap(value_parser = crate::shared::AsBString)] pub ref_spec: Vec, } + + #[derive(Debug, clap::Parser)] + pub struct ShallowOptions { + /// Fetch with the history truncated to the given number of commits as seen from the remote. + #[clap(long, conflicts_with_all = ["shallow_since", "shallow_exclude", "deepen", "unshallow"])] + pub depth: Option, + + /// Extend the current shallow boundary by the given amount of commits, with 0 meaning no change. + #[clap(long, value_name = "DEPTH", conflicts_with_all = ["depth", "shallow_since", "shallow_exclude", "unshallow"])] + pub deepen: Option, + + /// Cutoff all history past the given date. Can be combined with shallow-exclude. + #[clap(long, value_parser = crate::shared::AsTime, value_name = "DATE", conflicts_with_all = ["depth", "deepen", "unshallow"])] + pub shallow_since: Option, + + /// Cutoff all history past the tag-name or ref-name. Can be combined with shallow-since. + #[clap(long, value_parser = crate::shared::AsPartialRefName, value_name = "REF_NAME", conflicts_with_all = ["depth", "deepen", "unshallow"])] + pub shallow_exclude: Vec, + + /// Remove the shallow boundary and fetch the entire history available on the remote. + #[clap(long, conflicts_with_all = ["shallow_since", "shallow_exclude", "depth", "deepen", "unshallow"])] + pub unshallow: bool, + } + + impl From for Shallow { + fn from(opts: ShallowOptions) -> Self { + if let Some(depth) = opts.depth { + Shallow::DepthAtRemote(depth) + } else if !opts.shallow_exclude.is_empty() { + Shallow::Exclude { + remote_refs: opts.shallow_exclude, + since_cutoff: opts.shallow_since, + } + } else if let Some(cutoff) = opts.shallow_since { + Shallow::Since { cutoff } + } else if let Some(depth) = opts.deepen { + Shallow::Deepen(depth) + } else if opts.unshallow { + Shallow::undo() + } else { + Shallow::default() + } + } + } } #[cfg(feature = "gitoxide-core-blocking-client")] pub mod clone { + use gix::remote::fetch::Shallow; + use std::num::NonZeroU32; use std::{ffi::OsString, path::PathBuf}; #[derive(Debug, clap::Parser)] @@ -175,12 +227,47 @@ pub mod clone { #[clap(long)] pub no_tags: bool, + #[clap(flatten)] + pub shallow: ShallowOptions, + /// The url of the remote to connect to, like `https://github.com/byron/gitoxide`. pub remote: OsString, /// The directory to initialize with the new repository and to which all data should be written. pub directory: Option, } + + #[derive(Debug, clap::Parser)] + pub struct ShallowOptions { + /// Create a shallow clone with the history truncated to the given number of commits. + #[clap(long, conflicts_with_all = ["shallow_since", "shallow_exclude"])] + pub depth: Option, + + /// Cutoff all history past the given date. Can be combined with shallow-exclude. + #[clap(long, value_parser = crate::shared::AsTime, value_name = "DATE")] + pub shallow_since: Option, + + /// Cutoff all history past the tag-name or ref-name. Can be combined with shallow-since. + #[clap(long, value_parser = crate::shared::AsPartialRefName, value_name = "REF_NAME")] + pub shallow_exclude: Vec, + } + + impl From for Shallow { + fn from(opts: ShallowOptions) -> Self { + if let Some(depth) = opts.depth { + Shallow::DepthAtRemote(depth) + } else if !opts.shallow_exclude.is_empty() { + Shallow::Exclude { + remote_refs: opts.shallow_exclude, + since_cutoff: opts.shallow_since, + } + } else if let Some(cutoff) = opts.shallow_since { + Shallow::Since { cutoff } + } else { + Shallow::default() + } + } + } } #[cfg(any(feature = "gitoxide-core-async-client", feature = "gitoxide-core-blocking-client"))] diff --git a/src/plumbing/progress.rs b/src/plumbing/progress.rs index dde6a10a091..a6f6d28b66d 100644 --- a/src/plumbing/progress.rs +++ b/src/plumbing/progress.rs @@ -214,8 +214,14 @@ static GIT_CONFIG: &[Record] = &[ }, Record { config: "clone.rejectShallow", + usage: Planned { + note: Some("probably trivial to implement once there is protocol support for shallow clones"), + }, + }, + Record { + config: "receive.shallowUpdate", usage: NotPlanned { - reason: "it's not a use-case we consider important now, but once that changes it can be implemented", + reason: "it looks like a server-only setting that allows boundaries to change if refs are pushed that are outside of the boundary.", }, }, Record { diff --git a/src/shared.rs b/src/shared.rs index ee16f0c34e0..64c602b3f95 100644 --- a/src/shared.rs +++ b/src/shared.rs @@ -292,7 +292,7 @@ mod clap { } } - use clap::builder::{OsStringValueParser, TypedValueParser}; + use clap::builder::{OsStringValueParser, StringValueParser, TypedValueParser}; #[derive(Clone)] pub struct AsPathSpec; @@ -306,5 +306,31 @@ mod clap { .parse_ref(cmd, arg, value) } } + + #[derive(Clone)] + pub struct AsTime; + + impl TypedValueParser for AsTime { + type Value = gix::date::Time; + + fn parse_ref(&self, cmd: &Command, arg: Option<&Arg>, value: &OsStr) -> Result { + StringValueParser::new() + .try_map(|arg| gix::date::parse(&arg, Some(std::time::SystemTime::now()))) + .parse_ref(cmd, arg, value) + } + } + + #[derive(Clone)] + pub struct AsPartialRefName; + + impl TypedValueParser for AsPartialRefName { + type Value = gix::refs::PartialName; + + fn parse_ref(&self, cmd: &Command, arg: Option<&Arg>, value: &OsStr) -> Result { + AsBString + .try_map(gix::refs::PartialName::try_from) + .parse_ref(cmd, arg, value) + } + } } -pub use self::clap::{AsBString, AsHashKind, AsOutputFormat, AsPathSpec}; +pub use self::clap::{AsBString, AsHashKind, AsOutputFormat, AsPartialRefName, AsPathSpec, AsTime};