[git-transport] refactor

DEVELOPMENT.md

@@ -29,7 +29,7 @@
   * **library client-side**
     * ~~Don't use it client side, as operations there are usually bound by the CPU and ultra-fast access to memory mapped files.
       It's no problem to saturate either CPU or the IO system.~~
-      * Provide `async` clients as opt-in using feature toggles to help integrating into an exisging async codebase.
+      * Provide `async` clients as opt-in using feature toggles to help integrating into an existing async codebase.
   * **User Interfaces**
     * User interfaces can greatly benefit from using async as it's much easier to maintain a responsive UI thread that way thanks
       to the wonderful future combinators.
@@ -42,10 +42,16 @@
   * **server-side**
     * ~~Building a pack is CPU and at some point, IO bound, and it makes no sense to use async to handle more connections - git
       needs a lot of resources and threads will do just fine.~~
-      * Support async out of the box without looking it into particular traits using conditional complication. This will make integrating
+      * Support async out of the box without locking it into particular traits using conditional complication. This will make integrating
         into an async codebase easier, which we assume is given on the server side _these days_.
+  * **usage of `maybe_async`**
+    * Right not we intentionally only use it in tests to allow one set of test cases to test both blocking and async implementations. This is the
+      only way to prevent drift of otherwise distinct implementations.
+    * **Why not use it to generate blocking versions of traits automatically?**
+      * This would require `maybe_async` and its dependencies to always be present, increasing compile times. For now we chose a little more code to handle
+        over increasing compile times for everyone. This stance may change later once compile times don't matter that much anymore to allow the removal of code.
 
-* **`Default` implementations**
+* **`Default` trait implementations**
   * These can change only if the effect is contained within the callers process.
     This means **changing the default of a file version** is a **breaking change**.
 * **Using the `Progress` trait**

git-transport/src/client/async_io.rs

@@ -0,0 +1,11 @@
+use crate::{client::Capabilities, Protocol};
+
+/// The response of the [`handshake()`][Transport::handshake()] method.
+pub struct SetServiceResponse<'a> {
+    /// The protocol the service can provide. May be different from the requested one
+    pub actual_protocol: Protocol,
+    /// The capabilities parsed from the server response.
+    pub capabilities: Capabilities,
+    /// In protocol version one, this is set to a list of refs and their peeled counterparts.
+    pub refs: Option<Box<dyn futures_io::AsyncBufRead + 'a>>,
+}

git-transport/src/client/mod.rs

@@ -13,110 +13,12 @@ pub use blocking_io::{
 pub use connect::connect;
 
 #[cfg(all(not(feature = "blocking-client"), feature = "async-client"))]
-mod async_io {
-    use crate::{client::Capabilities, Protocol};
-
-    /// The response of the [`handshake()`][Transport::handshake()] method.
-    pub struct SetServiceResponse<'a> {
-        /// The protocol the service can provide. May be different from the requested one
-        pub actual_protocol: Protocol,
-        /// The capabilities parsed from the server response.
-        pub capabilities: Capabilities,
-        /// In protocol version one, this is set to a list of refs and their peeled counterparts.
-        pub refs: Option<Box<dyn futures_io::AsyncBufRead + 'a>>,
-    }
-}
+mod async_io;
 #[cfg(all(not(feature = "blocking-client"), feature = "async-client"))]
 pub use async_io::SetServiceResponse;
 
-mod error {
-    use crate::client::capabilities;
-    #[cfg(feature = "http-client-curl")]
-    use crate::client::http;
-    #[cfg(feature = "http-client-curl")]
-    type HttpError = http::Error;
-    #[cfg(not(feature = "http-client-curl"))]
-    type HttpError = std::convert::Infallible;
-
-    /// The error used in most methods of the [`client`][crate::client] module
-    #[derive(thiserror::Error, Debug)]
-    #[allow(missing_docs)]
-    pub enum Error {
-        #[error("An IO error occurred when talking to the server")]
-        Io {
-            #[from]
-            err: std::io::Error,
-        },
-        #[error("Capabilities could not be parsed")]
-        Capabilities {
-            #[from]
-            err: capabilities::Error,
-        },
-        #[error("A packet line could not be decoded")]
-        LineDecode {
-            #[from]
-            err: git_packetline::decode::Error,
-        },
-        #[error("A {0} line was expected, but there was none")]
-        ExpectedLine(&'static str),
-        #[error("Expected a data line, but got a delimiter")]
-        ExpectedDataLine,
-        #[error("The transport layer does not support authentication")]
-        AuthenticationUnsupported,
-        #[error("The transport layer refuses to use a given identity: {0}")]
-        AuthenticationRefused(&'static str),
-        #[error(transparent)]
-        Http(#[from] HttpError),
-    }
-}
-pub use error::Error;
-
-mod non_io_types {
-    /// Configure how the [`RequestWriter`][crate::client::RequestWriter] behaves when writing bytes.
-    #[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
-    #[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))]
-    pub enum WriteMode {
-        /// Each [write()][Write::write()] call writes the bytes verbatim as one or more packet lines.
-        Binary,
-        /// Each [write()][Write::write()] call assumes text in the input, assures a trailing newline and writes it as single packet line.
-        OneLfTerminatedLinePerWriteCall,
-    }
-
-    impl Default for WriteMode {
-        fn default() -> Self {
-            WriteMode::OneLfTerminatedLinePerWriteCall
-        }
-    }
-
-    /// The kind of packet line to write when transforming a [`RequestWriter`][crate::client::RequestWriter] into an
-    /// [`ExtendedBufRead`][crate::client::ExtendedBufRead].
-    #[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
-    #[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))]
-    pub enum MessageKind {
-        /// A `flush` packet.
-        Flush,
-        /// A V2 delimiter.
-        Delimiter,
-        /// The end of a response.
-        ResponseEnd,
-        /// The given text.
-        Text(&'static [u8]),
-    }
-
-    #[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)]
-    #[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))]
-    /// An identity for use when authenticating the transport layer.
-    pub enum Identity {
-        /// An account based identity
-        Account {
-            /// The user's name
-            username: String,
-            /// The user's password
-            password: String,
-        },
-    }
-}
-pub use non_io_types::{Identity, MessageKind, WriteMode};
+mod non_io_types;
+pub use non_io_types::{Error, Identity, MessageKind, WriteMode};
 
 ///
 pub mod capabilities;

git-transport/src/client/non_io_types.rs

@@ -0,0 +1,86 @@
+/// Configure how the [`RequestWriter`][crate::client::RequestWriter] behaves when writing bytes.
+#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
+#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))]
+pub enum WriteMode {
+    /// Each [write()][Write::write()] call writes the bytes verbatim as one or more packet lines.
+    Binary,
+    /// Each [write()][Write::write()] call assumes text in the input, assures a trailing newline and writes it as single packet line.
+    OneLfTerminatedLinePerWriteCall,
+}
+
+impl Default for WriteMode {
+    fn default() -> Self {
+        WriteMode::OneLfTerminatedLinePerWriteCall
+    }
+}
+
+/// The kind of packet line to write when transforming a [`RequestWriter`][crate::client::RequestWriter] into an
+/// [`ExtendedBufRead`][crate::client::ExtendedBufRead].
+#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
+#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))]
+pub enum MessageKind {
+    /// A `flush` packet.
+    Flush,
+    /// A V2 delimiter.
+    Delimiter,
+    /// The end of a response.
+    ResponseEnd,
+    /// The given text.
+    Text(&'static [u8]),
+}
+
+#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)]
+#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))]
+/// An identity for use when authenticating the transport layer.
+pub enum Identity {
+    /// An account based identity
+    Account {
+        /// The user's name
+        username: String,
+        /// The user's password
+        password: String,
+    },
+}
+
+mod error {
+    use crate::client::capabilities;
+    #[cfg(feature = "http-client-curl")]
+    use crate::client::http;
+    #[cfg(feature = "http-client-curl")]
+    type HttpError = http::Error;
+    #[cfg(not(feature = "http-client-curl"))]
+    type HttpError = std::convert::Infallible;
+
+    /// The error used in most methods of the [`client`][crate::client] module
+    #[derive(thiserror::Error, Debug)]
+    #[allow(missing_docs)]
+    pub enum Error {
+        #[error("An IO error occurred when talking to the server")]
+        Io {
+            #[from]
+            err: std::io::Error,
+        },
+        #[error("Capabilities could not be parsed")]
+        Capabilities {
+            #[from]
+            err: capabilities::Error,
+        },
+        #[error("A packet line could not be decoded")]
+        LineDecode {
+            #[from]
+            err: git_packetline::decode::Error,
+        },
+        #[error("A {0} line was expected, but there was none")]
+        ExpectedLine(&'static str),
+        #[error("Expected a data line, but got a delimiter")]
+        ExpectedDataLine,
+        #[error("The transport layer does not support authentication")]
+        AuthenticationUnsupported,
+        #[error("The transport layer refuses to use a given identity: {0}")]
+        AuthenticationRefused(&'static str),
+        #[error(transparent)]
+        Http(#[from] HttpError),
+    }
+}
+
+pub use error::Error;