History

feat!: add `remote::Connection::with_transport_config()`, change the way `*::transport_mut()` is used.

Previously `transport_mut()` was supposed to be used for calling `configure()`, but that doesn't work anymore as `configure()` can only effectivey be called once the initialization of the Connection is complete, as it may depend on the Remote name AND the credential provider for proxy auth credential acquisition. Thus we allow callers to set the transport options they need in advance for it to be used when needed.

Sebastian Thiel committed 4 years ago

Commit db9040f1 parent b46e3bb7 files · +45 −14Browse files

Unified · Split

7 files
  1. git-repository/src/remote/connect.rs +1 −0
  2. git-repository/src/remote/connection/access.rs +16 −7
  3. git-repository/src/remote/connection/mod.rs +1 −0
  4. git-repository/src/remote/connection/ref_map.rs +23 −3
  5. git-repository/src/repository/config/transport.rs +2 −2
  6. git-repository/tests/repository/config/mod.rs +1 −1
  7. git-repository/tests/repository/config/transport_options.rs +1 −1
git-repository/src/remote/connect.rs+1−0
@@ -39,6 +39,7 @@ impl<'repo> Remote<'repo> { · 38 unmodified lines
3939 Connection {
4040 remote: self,
4141 authenticate: None,
42+ transport_config: None,
4243 transport,
4344 progress,
4445 }
git-repository/src/remote/connection/access.rs+16−7
@@ -19,6 +19,18 @@ impl<'a, 'repo, T, P> Connection<'a, 'repo, T, P> { · 18 unmodified lines
1919 self.authenticate = Some(Box::new(helper));
2020 self
2121 }
22+
23+ /// Provide configuration to be used before the first handshake is conducted.
24+ /// It's typically created by initializing it with [`Repository::transport_options()`][crate::Repository::transport_options()], which
25+ /// is also the default if this isn't set explicitly. Note that all of the default configuration is created from `git`
26+ /// configuration, which can also be manipulated through overrides to affect the default configuration.
27+ ///
28+ /// Use this method to provide transport configuration with custom backend configuration that is not configurable by other means and
29+ /// custom to the application at hand.
30+ pub fn with_transport_options(mut self, config: Box<dyn std::any::Any>) -> Self {
31+ self.transport_config = Some(config);
32+ self
33+ }
2234 }
2335
2436 /// Access
@@ -41,13 +53,10 @@ impl<'a, 'repo, T, P> Connection<'a, 'repo, T, P> { · 16 unmodified lines
4153 self.remote
4254 }
4355
44− /// Provide a mutable transport to allow configuring it with [`configure()`][git_protocol::transport::client::TransportWithoutIO::configure()].
45− ///
46− /// Please note that it is pre-configured, but can be configured again right after the [`Connection`] was instantiated without
47− /// the previous configuration having ever been used.
48− ///
49− /// Also note that all of the default configuration is created from `git` configuration, which can also be manipulated through overrides
50− /// to affect the default configuration.
56+ /// Provide a mutable transport to allow interacting with it according to its actual type.
57+ /// Note that the caller _should not_ call [`configure()`][git_protocol::transport::client::TransportWithoutIO::configure()]
58+ /// as we will call it automatically before performing the handshake. Instead, to bring in custom configuration,
59+ /// call [`with_transport_options()`][Connection::with_transport_options()].
5160 pub fn transport_mut(&mut self) -> &mut T {
5261 &mut self.transport
5362 }
git-repository/src/remote/connection/mod.rs+1−0
@@ -15,6 +15,7 @@ pub type AuthenticateFn<'a> = Box<dyn FnMut(git_credentials::helper::Action) -> · 14 unmodified lines
1515 pub struct Connection<'a, 'repo, T, P> {
1616 pub(crate) remote: &'a Remote<'repo>,
1717 pub(crate) authenticate: Option<AuthenticateFn<'a>>,
18+ pub(crate) transport_config: Option<Box<dyn std::any::Any>>,
1819 pub(crate) transport: T,
1920 pub(crate) progress: P,
2021 }
git-repository/src/remote/connection/ref_map.rs+23−3
@@ -13,6 +13,13 @@ use crate::{ · 12 unmodified lines
1313 #[derive(Debug, thiserror::Error)]
1414 #[allow(missing_docs)]
1515 pub enum Error {
16+ #[error("Failed to configure the transport before connecting to {url:?}")]
17+ GatherTransportConfig {
18+ url: BString,
19+ source: crate::config::transport::Error,
20+ },
21+ #[error("Failed to configure the transport layer")]
22+ ConfigureTransport(#[from] Box<dyn std::error::Error + Send + Sync + 'static>),
1623 #[error(transparent)]
1724 Handshake(#[from] git_protocol::fetch::handshake::Error),
1825 #[error("The object format {format:?} as used by the remote is unsupported")]
@@ -139,6 +146,7 @@ where · 120 unmodified lines
139146 extra_parameters: Vec<(String, Option<String>)>,
140147 ) -> Result<HandshakeWithRefs, Error> {
141148 let mut credentials_storage;
149+ let url = self.transport.to_url();
142150 let authenticate = match self.authenticate.as_mut() {
143151 Some(f) => f,
144152 None => {
@@ -146,13 +154,25 @@ where · 1 unmodified line
146154 .remote
147155 .url(Direction::Fetch)
148156 .map(ToOwned::to_owned)
149− .unwrap_or_else(|| {
150− git_url::parse(self.transport.to_url().as_ref()).expect("valid URL to be provided by transport")
151− });
157+ .unwrap_or_else(|| git_url::parse(url.as_ref()).expect("valid URL to be provided by transport"));
152158 credentials_storage = self.configured_credentials(url)?;
153159 &mut credentials_storage
154160 }
155161 };
162+
163+ if self.transport_config.is_none() {
164+ self.transport_config =
165+ self.remote
166+ .repo
167+ .transport_options(url.as_ref())
168+ .map_err(|err| Error::GatherTransportConfig {
169+ source: err,
170+ url: url.into_owned(),
171+ })?;
172+ }
173+ if let Some(config) = self.transport_config.as_ref() {
174+ self.transport.configure(&**config)?;
175+ }
156176 let mut outcome =
157177 git_protocol::fetch::handshake(&mut self.transport, authenticate, extra_parameters, &mut self.progress)
158178 .await?;
git-repository/src/repository/config/transport.rs+2−2
@@ -6,11 +6,11 @@ use std::convert::{TryFrom, TryInto}; · 5 unmodified lines
66
77 impl crate::Repository {
88 /// Produce configuration suitable for `url`, as differentiated by its protocol/scheme, to be passed to a transport instance via
9− /// [configure()][git_transport::client::TransportWithoutIO::configure()].
9+ /// [configure()][git_transport::client::TransportWithoutIO::configure()] (via `&**config` to pass the contained `Any` and not the `Box`).
1010 /// `None` is returned if there is no known configuration.
1111 ///
1212 /// Note that the caller may cast the instance themselves to modify it before passing it on.
13− pub fn transport_config<'a>(
13+ pub fn transport_options<'a>(
1414 &self,
1515 url: impl Into<&'a BStr>,
1616 ) -> Result<Option<Box<dyn Any>>, crate::config::transport::Error> {
git-repository/tests/repository/config/mod.rs+1−1
@@ -2,4 +2,4 @@ mod config_snapshot; · 1 unmodified line
22 mod identity;
33 mod remote;
44 #[cfg(any(feature = "blocking-network-client", feature = "async-network-client"))]
5−mod transport_config;
5+mod transport_options;
git-repository/tests/repository/config/transport_options.rsrenamed from git-repository/tests/repository/config/transport_config.rs+1−1
@@ -23,7 +23,7 @@ mod http { · 22 unmodified lines
2323 fn simple_configuration() {
2424 let repo = repo("http-config");
2525 let http_config = repo
26− .transport_config("https://example.com/does/not/matter")
26+ .transport_options("https://example.com/does/not/matter")
2727 .expect("valid configuration")
2828 .expect("configuration available for http");
2929 let git_transport::client::http::Options {

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t