Skip to content

Commit c9ff885

Browse files
committed
produce only a ref-map as it contains all data somebody would want. (#450)
It does more work as well, but it seems unnecessary to maintian a bigger API surface and it might be key to turning this into a step-by-step transition to fetching packs and updating local references.
1 parent 0f9833a commit c9ff885

File tree

5 files changed

+129
-165
lines changed

5 files changed

+129
-165
lines changed

Diff for: git-repository/src/remote/connection/list_refs.rs

-148
This file was deleted.

Diff for: git-repository/src/remote/connection/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,4 @@ mod access {
6666
}
6767

6868
///
69-
pub mod list_refs;
69+
pub mod ref_map;

Diff for: git-repository/src/remote/connection/ref_map.rs

+114
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
use git_features::progress::Progress;
2+
use git_protocol::transport::client::Transport;
3+
4+
use crate::remote::{connection::HandshakeWithRefs, fetch, Connection, Direction};
5+
6+
/// The error returned by [`Connection::list_refs_to_map()`].
7+
#[derive(Debug, thiserror::Error)]
8+
#[allow(missing_docs)]
9+
pub enum Error {
10+
#[error(transparent)]
11+
Handshake(#[from] git_protocol::fetch::handshake::Error),
12+
#[error(transparent)]
13+
ListRefs(#[from] git_protocol::fetch::refs::Error),
14+
#[error(transparent)]
15+
Transport(#[from] git_protocol::transport::client::Error),
16+
#[error(transparent)]
17+
ConfigureCredentials(#[from] crate::config::credential_helpers::Error),
18+
#[error(transparent)]
19+
MappingValidation(#[from] git_refspec::match_group::validate::Error),
20+
}
21+
22+
impl<'a, 'repo, T, P> Connection<'a, 'repo, T, P>
23+
where
24+
T: Transport,
25+
P: Progress,
26+
{
27+
/// List all references on the remote that have been filtered through our remote's [`refspecs`][crate::Remote::refspecs()]
28+
/// for _fetching_.
29+
///
30+
/// This comes in the form of all matching tips on the remote and the object they point to, along with
31+
/// with the local tracking branch of these tips (if available).
32+
///
33+
/// Note that this doesn't fetch the objects mentioned in the tips nor does it make any change to underlying repository.
34+
#[git_protocol::maybe_async::maybe_async]
35+
pub async fn ref_map(mut self) -> Result<fetch::RefMap, Error> {
36+
let res = self.ref_map_inner().await;
37+
git_protocol::fetch::indicate_end_of_interaction(&mut self.transport).await?;
38+
res
39+
}
40+
41+
#[git_protocol::maybe_async::maybe_async]
42+
async fn ref_map_inner(&mut self) -> Result<fetch::RefMap, Error> {
43+
let remote = self.fetch_refs().await?;
44+
let group = git_refspec::MatchGroup::from_fetch_specs(self.remote.fetch_specs.iter().map(|s| s.to_ref()));
45+
let (res, fixes) = group
46+
.match_remotes(remote.refs.iter().map(|r| {
47+
let (full_ref_name, target, object) = r.unpack();
48+
git_refspec::match_group::Item {
49+
full_ref_name,
50+
target,
51+
object,
52+
}
53+
}))
54+
.validated()?;
55+
let mappings = res.mappings;
56+
let mappings = mappings
57+
.into_iter()
58+
.map(|m| fetch::Mapping {
59+
remote: m
60+
.item_index
61+
.map(|idx| fetch::Source::Ref(remote.refs[idx].clone()))
62+
.unwrap_or_else(|| {
63+
fetch::Source::ObjectId(match m.lhs {
64+
git_refspec::match_group::SourceRef::ObjectId(id) => id,
65+
_ => unreachable!("no item index implies having an object id"),
66+
})
67+
}),
68+
local: m.rhs.map(|c| c.into_owned()),
69+
spec_index: m.spec_index,
70+
})
71+
.collect();
72+
Ok(fetch::RefMap {
73+
mappings,
74+
fixes,
75+
remote_refs: remote.refs,
76+
handshake: remote.outcome,
77+
})
78+
}
79+
#[git_protocol::maybe_async::maybe_async]
80+
async fn fetch_refs(&mut self) -> Result<HandshakeWithRefs, Error> {
81+
let mut credentials_storage;
82+
let authenticate = match self.credentials.as_mut() {
83+
Some(f) => f,
84+
None => {
85+
let url = self
86+
.remote
87+
.url(Direction::Fetch)
88+
.map(ToOwned::to_owned)
89+
.unwrap_or_else(|| {
90+
git_url::parse(self.transport.to_url().as_bytes().into())
91+
.expect("valid URL to be provided by transport")
92+
});
93+
credentials_storage = self.configured_credentials(url)?;
94+
&mut credentials_storage
95+
}
96+
};
97+
let mut outcome =
98+
git_protocol::fetch::handshake(&mut self.transport, authenticate, Vec::new(), &mut self.progress).await?;
99+
let refs = match outcome.refs.take() {
100+
Some(refs) => refs,
101+
None => {
102+
git_protocol::fetch::refs(
103+
&mut self.transport,
104+
outcome.server_protocol_version,
105+
&outcome.capabilities,
106+
|_a, _b, _c| Ok(git_protocol::fetch::delegate::LsRefsAction::Continue),
107+
&mut self.progress,
108+
)
109+
.await?
110+
}
111+
};
112+
Ok(HandshakeWithRefs { outcome, refs })
113+
}
114+
}

Diff for: git-repository/src/remote/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ pub mod connect;
7373
#[cfg(any(feature = "async-network-client", feature = "blocking-network-client"))]
7474
mod connection;
7575
#[cfg(any(feature = "async-network-client", feature = "blocking-network-client"))]
76-
pub use connection::{list_refs, Connection};
76+
pub use connection::{ref_map, Connection};
7777

7878
mod access;
7979
pub(crate) mod url;

Diff for: git-repository/tests/remote/list_refs.rs

+13-15
Original file line numberDiff line numberDiff line change
@@ -24,22 +24,20 @@ mod blocking_io {
2424
}
2525

2626
let remote = repo.find_remote("origin")?;
27-
{
28-
let connection = remote.connect(Fetch, progress::Discard)?;
29-
let refs = connection.list_refs()?;
30-
assert_eq!(refs.len(), 14, "it gets all remote refs, independently of the refspec.");
31-
}
27+
let connection = remote.connect(Fetch, progress::Discard)?;
28+
let map = connection.ref_map()?;
29+
assert_eq!(
30+
map.remote_refs.len(),
31+
14,
32+
"it gets all remote refs, independently of the refspec."
33+
);
3234

33-
{
34-
let connection = remote.connect(Fetch, progress::Discard)?;
35-
let map = connection.list_refs_to_map()?;
36-
assert_eq!(map.fixes.len(), 0);
37-
assert_eq!(
38-
map.mappings.len(),
39-
11,
40-
"mappings are only a sub-set of all remotes due to refspec matching"
41-
);
42-
}
35+
assert_eq!(map.fixes.len(), 0);
36+
assert_eq!(
37+
map.mappings.len(),
38+
11,
39+
"mappings are only a sub-set of all remotes due to refspec matching"
40+
);
4341
}
4442
Ok(())
4543
}

0 commit comments

Comments
 (0)