Skip to content

Commit e4d78e1

Browse files
committed
feat: Remote knows about its tagOpt configuration.
That way it's clear if it should or shouldn't fetch included/reachable tags automatically. The default setting for this is to include tags, similar to `git`. The `fetch_tags()` accessor allows to query this information, and the `with_fetch_tags()` builder method allows to set the value comfortably right after creating the `Remote` instance. The `tagOpt` key will also be written as part of the remote's git configuration. Clone operations can set the `Tags` setting when configuring the remote in a callback. This also comes with a fix to assure that ref-updates aren't skipped just because there was no pack to receive. That way, locally missing refs or tags will automatically be put back.
1 parent a872782 commit e4d78e1

File tree

25 files changed

+516
-172
lines changed

25 files changed

+516
-172
lines changed

git-repository/src/clone/fetch/mod.rs

+27-18
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@ pub enum Error {
1212
#[error(transparent)]
1313
Fetch(#[from] crate::remote::fetch::Error),
1414
#[error(transparent)]
15-
RemoteConfiguration(#[from] crate::remote::init::Error),
15+
RemoteInit(#[from] crate::remote::init::Error),
16+
#[error("Custom configuration of remote to clone from failed")]
17+
RemoteConfiguration(#[source] Box<dyn std::error::Error + Send + Sync>),
1618
#[error("Default remote configured at `clone.defaultRemoteName` is invalid")]
1719
RemoteName(#[from] crate::remote::name::Error),
1820
#[error("Failed to load repo-local git configuration before writing")]
@@ -24,7 +26,7 @@ pub enum Error {
2426
#[error("The remote HEAD points to a reference named {head_ref_name:?} which is invalid.")]
2527
InvalidHeadRef {
2628
source: git_validate::refname::Error,
27-
head_ref_name: crate::bstr::BString,
29+
head_ref_name: BString,
2830
},
2931
#[error("Failed to update HEAD with values from remote")]
3032
HeadUpdate(#[from] crate::reference::edit::Error),
@@ -51,6 +53,7 @@ impl PrepareFetch {
5153
P: crate::Progress,
5254
P::SubProgress: 'static,
5355
{
56+
use crate::remote;
5457
use crate::{bstr::ByteVec, remote::fetch::RefLogMessage};
5558

5659
let repo = self
@@ -64,36 +67,39 @@ impl PrepareFetch {
6467
.config
6568
.resolved
6669
.string_by_key("clone.defaultRemoteName")
67-
.map(|n| crate::remote::name::validated(n.into_owned()))
70+
.map(|n| remote::name::validated(n.into_owned()))
6871
.unwrap_or_else(|| Ok("origin".into()))?,
6972
};
7073

7174
let mut remote = repo
7275
.remote_at(self.url.clone())?
7376
.with_refspecs(
7477
Some(format!("+refs/heads/*:refs/remotes/{remote_name}/*").as_str()),
75-
crate::remote::Direction::Fetch,
78+
remote::Direction::Fetch,
7679
)
7780
.expect("valid static spec");
7881
if let Some(f) = self.configure_remote.as_mut() {
79-
remote = f(remote)?;
82+
remote = f(remote).map_err(|err| Error::RemoteConfiguration(err))?;
8083
}
8184

8285
let config = util::write_remote_to_local_config_file(&mut remote, remote_name.clone())?;
8386

8487
// Add HEAD after the remote was written to config, we need it to know what to checkout later, and assure
8588
// the ref that HEAD points to is present no matter what.
86-
remote.fetch_specs.push(
87-
git_refspec::parse(
88-
format!("HEAD:refs/remotes/{remote_name}/HEAD").as_str().into(),
89-
git_refspec::parse::Operation::Fetch,
90-
)
91-
.expect("valid")
92-
.to_owned(),
93-
);
94-
let pending_pack: crate::remote::fetch::Prepare<'_, '_, _, _> = remote
95-
.connect(crate::remote::Direction::Fetch, progress)?
96-
.prepare_fetch(self.fetch_options.clone())?;
89+
let head_refspec = git_refspec::parse(
90+
format!("HEAD:refs/remotes/{remote_name}/HEAD").as_str().into(),
91+
git_refspec::parse::Operation::Fetch,
92+
)
93+
.expect("valid")
94+
.to_owned();
95+
let pending_pack: remote::fetch::Prepare<'_, '_, _, _> =
96+
remote.connect(remote::Direction::Fetch, progress)?.prepare_fetch({
97+
let mut opts = self.fetch_options.clone();
98+
if !opts.extra_refspecs.contains(&head_refspec) {
99+
opts.extra_refspecs.push(head_refspec)
100+
}
101+
opts
102+
})?;
97103
if pending_pack.ref_map().object_hash != repo.object_hash() {
98104
unimplemented!("configure repository to expect a different object hash as advertised by the server")
99105
}
@@ -148,10 +154,13 @@ impl PrepareFetch {
148154
///
149155
/// The passed in `remote` will be un-named and pre-configured to be a default remote as we know it from git-clone.
150156
/// It is not yet present in the configuration of the repository,
151-
/// but each change it will eventually be written to the configuration prior to performing a the fetch operation.
157+
/// but each change it will eventually be written to the configuration prior to performing a the fetch operation,
158+
/// _all changes done in `f()` will be persisted_.
159+
///
160+
/// It can also be used to configure additional options, like those for fetching tags.
152161
pub fn configure_remote(
153162
mut self,
154-
f: impl FnMut(crate::Remote<'_>) -> Result<crate::Remote<'_>, crate::remote::init::Error> + 'static,
163+
f: impl FnMut(crate::Remote<'_>) -> Result<crate::Remote<'_>, Box<dyn std::error::Error + Send + Sync>> + 'static,
155164
) -> Self {
156165
self.configure_remote = Some(Box::new(f));
157166
self

git-repository/src/clone/mod.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ use std::convert::TryInto;
22

33
use crate::bstr::BString;
44

5-
type ConfigureRemoteFn = Box<dyn FnMut(crate::Remote<'_>) -> Result<crate::Remote<'_>, crate::remote::init::Error>>;
5+
type ConfigureRemoteFn =
6+
Box<dyn FnMut(crate::Remote<'_>) -> Result<crate::Remote<'_>, Box<dyn std::error::Error + Send + Sync>>>;
67

78
/// A utility to collect configuration on how to fetch from a remote and initiate a fetch operation. It will delete the newly
89
/// created repository on when dropped without successfully finishing a fetch.

git-repository/src/remote/access.rs

+5
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ impl<'repo> Remote<'repo> {
2222
}
2323
}
2424

25+
/// Return how we handle tags when fetching the remote.
26+
pub fn fetch_tags(&self) -> remote::fetch::Tags {
27+
self.fetch_tags
28+
}
29+
2530
/// Return the url used for the given `direction` with rewrites from `url.<base>.insteadOf|pushInsteadOf`, unless the instance
2631
/// was created with one of the `_without_url_rewrite()` methods.
2732
/// For pushing, this is the `remote.<name>.pushUrl` or the `remote.<name>.url` used for fetching, and for fetching it's

git-repository/src/remote/build.rs

+6
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@ impl Remote<'_> {
2323
self.push_url_inner(url, false)
2424
}
2525

26+
/// Configure how tags should be handled when fetching from the remote.
27+
pub fn with_fetch_tags(mut self, tags: remote::fetch::Tags) -> Self {
28+
self.fetch_tags = tags;
29+
self
30+
}
31+
2632
fn push_url_inner<Url, E>(mut self, push_url: Url, should_rewrite_urls: bool) -> Result<Self, remote::init::Error>
2733
where
2834
Url: TryInto<git_url::Url, Error = E>,

git-repository/src/remote/connection/fetch/mod.rs

+7-2
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,13 @@ impl RefLogMessage {
4141
/// The status of the repository after the fetch operation
4242
#[derive(Debug, Clone)]
4343
pub enum Status {
44-
/// Nothing changed as the remote didn't have anything new compared to our tracking branches.
45-
NoChange,
44+
/// Nothing changed as the remote didn't have anything new compared to our tracking branches, thus no pack was received
45+
/// and no new object was added.
46+
NoPackReceived {
47+
/// However, depending on the refspecs, references might have been updated nonetheless to point to objects as
48+
/// reported by the remote.
49+
update_refs: refs::update::Outcome,
50+
},
4651
/// There was at least one tip with a new object which we received.
4752
Change {
4853
/// Information collected while writing the pack and its index.

git-repository/src/remote/connection/fetch/negotiate.rs

+15-2
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,27 @@ pub(crate) fn one_round(
2121
round: usize,
2222
repo: &crate::Repository,
2323
ref_map: &crate::remote::fetch::RefMap,
24+
fetch_tags: crate::remote::fetch::Tags,
2425
arguments: &mut git_protocol::fetch::Arguments,
2526
_previous_response: Option<&git_protocol::fetch::Response>,
2627
) -> Result<bool, Error> {
28+
let tag_refspec_to_ignore = fetch_tags
29+
.to_refspec()
30+
.filter(|_| matches!(fetch_tags, crate::remote::fetch::Tags::Included));
2731
match algo {
2832
Algorithm::Naive => {
2933
assert_eq!(round, 1, "Naive always finishes after the first round, and claims.");
3034
let mut has_missing_tracking_branch = false;
3135
for mapping in &ref_map.mappings {
36+
if tag_refspec_to_ignore.map_or(false, |tag_spec| {
37+
mapping
38+
.spec_index
39+
.implicit_index()
40+
.and_then(|idx| ref_map.extra_refspecs.get(idx))
41+
.map_or(false, |spec| spec.to_ref() == tag_spec)
42+
}) {
43+
continue;
44+
}
3245
let have_id = mapping.local.as_ref().and_then(|name| {
3346
repo.find_reference(name)
3447
.ok()
@@ -44,8 +57,8 @@ pub(crate) fn one_round(
4457
}
4558
}
4659
None => {
47-
if let Some(have_id) = mapping.remote.as_id() {
48-
arguments.want(have_id);
60+
if let Some(want_id) = mapping.remote.as_id() {
61+
arguments.want(want_id);
4962
has_missing_tracking_branch = true;
5063
}
5164
}

git-repository/src/remote/connection/fetch/receive_pack.rs

+16-1
Original file line numberDiff line numberDiff line change
@@ -99,14 +99,27 @@ where
9999
round,
100100
repo,
101101
&self.ref_map,
102+
con.remote.fetch_tags,
102103
&mut arguments,
103104
previous_response.as_ref(),
104105
) {
105106
Ok(_) if arguments.is_empty() => {
106107
git_protocol::indicate_end_of_interaction(&mut con.transport).await.ok();
108+
let update_refs = refs::update(
109+
repo,
110+
self.reflog_message
111+
.take()
112+
.unwrap_or_else(|| RefLogMessage::Prefixed { action: "fetch".into() }),
113+
&self.ref_map.mappings,
114+
con.remote.refspecs(remote::Direction::Fetch),
115+
&self.ref_map.extra_refspecs,
116+
con.remote.fetch_tags,
117+
self.dry_run,
118+
self.write_packed_refs,
119+
)?;
107120
return Ok(Outcome {
108121
ref_map: std::mem::take(&mut self.ref_map),
109-
status: Status::NoChange,
122+
status: Status::NoPackReceived { update_refs },
110123
});
111124
}
112125
Ok(is_done) => is_done,
@@ -175,6 +188,8 @@ where
175188
.unwrap_or_else(|| RefLogMessage::Prefixed { action: "fetch".into() }),
176189
&self.ref_map.mappings,
177190
con.remote.refspecs(remote::Direction::Fetch),
191+
&self.ref_map.extra_refspecs,
192+
con.remote.fetch_tags,
178193
self.dry_run,
179194
self.write_packed_refs,
180195
)?;

git-repository/src/remote/connection/fetch/update_refs/mod.rs

+24-4
Original file line numberDiff line numberDiff line change
@@ -33,38 +33,58 @@ impl From<update::Mode> for Update {
3333
}
3434
}
3535

36-
/// Update all refs as derived from `mappings` and produce an `Outcome` informing about all applied changes in detail, with each
36+
/// Update all refs as derived from `refmap.mappings` and produce an `Outcome` informing about all applied changes in detail, with each
3737
/// [`update`][Update] corresponding to the [`fetch::Mapping`] of at the same index.
3838
/// If `dry_run` is true, ref transactions won't actually be applied, but are assumed to work without error so the underlying
3939
/// `repo` is not actually changed. Also it won't perform an 'object exists' check as these are likely not to exist as the pack
4040
/// wasn't fetched either.
4141
/// `action` is the prefix used for reflog entries, and is typically "fetch".
4242
///
4343
/// It can be used to produce typical information that one is used to from `git fetch`.
44+
#[allow(clippy::too_many_arguments)]
4445
pub(crate) fn update(
4546
repo: &Repository,
4647
message: RefLogMessage,
4748
mappings: &[fetch::Mapping],
4849
refspecs: &[git_refspec::RefSpec],
50+
extra_refspecs: &[git_refspec::RefSpec],
51+
fetch_tags: fetch::Tags,
4952
dry_run: fetch::DryRun,
5053
write_packed_refs: fetch::WritePackedRefs,
5154
) -> Result<update::Outcome, update::Error> {
5255
let mut edits = Vec::new();
5356
let mut updates = Vec::new();
5457

55-
for (remote, local, spec) in mappings.iter().filter_map(
58+
let implicit_tag_refspec = fetch_tags
59+
.to_refspec()
60+
.filter(|_| matches!(fetch_tags, crate::remote::fetch::Tags::Included));
61+
for (remote, local, spec, is_implicit_tag) in mappings.iter().filter_map(
5662
|fetch::Mapping {
5763
remote,
5864
local,
5965
spec_index,
60-
}| refspecs.get(*spec_index).map(|spec| (remote, local, spec)),
66+
}| {
67+
spec_index.get(refspecs, extra_refspecs).map(|spec| {
68+
(
69+
remote,
70+
local,
71+
spec,
72+
implicit_tag_refspec.map_or(false, |tag_spec| spec.to_ref() == tag_spec),
73+
)
74+
})
75+
},
6176
) {
6277
let remote_id = match remote.as_id() {
6378
Some(id) => id,
6479
None => continue,
6580
};
6681
if dry_run == fetch::DryRun::No && !repo.objects.contains(remote_id) {
67-
updates.push(update::Mode::RejectedSourceObjectNotFound { id: remote_id.into() }.into());
82+
let update = if is_implicit_tag {
83+
update::Mode::ImplicitTagNotSentByRemote.into()
84+
} else {
85+
update::Mode::RejectedSourceObjectNotFound { id: remote_id.into() }.into()
86+
};
87+
updates.push(update);
6888
continue;
6989
}
7090
let checked_out_branches = worktree_branches(repo)?;

0 commit comments

Comments
 (0)