Skip to content

Commit 404fde5

Browse files
ByronByron
committed
fix!: rename Repository::branch_remote_ref() to Repository::branch_remote_ref_name(), add direction argument (also to Repository::branch_remote_name() and Repository::branch_remote()).
This better differentiates the return value from the corresponding ref objects, which would require the named ref to exist in the repository. The `direction` argument allows to get the reference to push to as well. Further, it now takes a full ref name to support deriving the name of branches to push to. Regarding `Repository::branch_remote()`, previously, this functionality was only available from a `Reference`, but now it's more generally available with just a branch name. The method was also adjusted to permit looking up non-symbolic remote names, like remotes that are specified by their URL.
1 parent 5c07c76 commit 404fde5

10 files changed

Lines changed: 380 additions & 87 deletions

File tree

‎gix/src/reference/remote.rs‎

Lines changed: 6 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
use crate::{config, config::tree::Branch, remote, Reference};
1+
use crate::{remote, Reference};
22

33
/// Remotes
44
impl<'repo> Reference<'repo> {
5-
/// Find the unvalidated name of our remote for `direction` as configured in `branch.<name>.remote|pushRemote` respectively.
5+
/// Find the name of our remote for `direction` as configured in `branch.<name>.remote|pushRemote` respectively.
66
/// If `Some(<name>)` it can be used in [`Repository::find_remote(…)`][crate::Repository::find_remote()], or if `None` then
77
/// [`Repository::remote_default_name()`][crate::Repository::remote_default_name()] could be used in its place.
88
///
@@ -14,36 +14,15 @@ impl<'repo> Reference<'repo> {
1414
/// information.
1515
/// - `branch.<name>.pushRemote` falls back to `branch.<name>.remote`.
1616
pub fn remote_name(&self, direction: remote::Direction) -> Option<remote::Name<'repo>> {
17-
let name = self.name().shorten();
18-
let config = &self.repo.config.resolved;
19-
(direction == remote::Direction::Push)
20-
.then(|| {
21-
config
22-
.string("branch", Some(name), Branch::PUSH_REMOTE.name)
23-
.or_else(|| config.string("remote", None, config::tree::Remote::PUSH_DEFAULT.name))
24-
})
25-
.flatten()
26-
.or_else(|| config.string("branch", Some(name), Branch::REMOTE.name))
27-
.and_then(|name| name.try_into().ok())
17+
self.repo.branch_remote_name(self.name().shorten(), direction)
2818
}
2919

30-
/// Like [`remote_name(…)`][Self::remote_name()], but configures the returned `Remote` with additional information like
31-
///
32-
/// - `branch.<name>.merge` to know which branch on the remote side corresponds to this one for merging when pulling.
33-
///
34-
/// It also handles if the remote is a configured URL, which has no name.
20+
/// Like [`branch_remote(…)`](crate::Repository::branch_remote()), but automatically provides the reference name
21+
/// for configuration lookup.
3522
pub fn remote(
3623
&self,
3724
direction: remote::Direction,
3825
) -> Option<Result<crate::Remote<'repo>, remote::find::existing::Error>> {
39-
// TODO: use `branch.<name>.merge`
40-
self.remote_name(direction).map(|name| match name {
41-
remote::Name::Symbol(name) => self.repo.find_remote(name.as_ref()).map_err(Into::into),
42-
remote::Name::Url(url) => gix_url::parse(url.as_ref()).map_err(Into::into).and_then(|url| {
43-
self.repo
44-
.remote_at(url)
45-
.map_err(|err| remote::find::existing::Error::Find(remote::find::Error::Init(err)))
46-
}),
47-
})
26+
self.repo.branch_remote(self.name().shorten(), direction)
4827
}
4928
}

‎gix/src/repository/config/mod.rs‎

Lines changed: 136 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -138,15 +138,17 @@ mod remote {
138138
use crate::bstr::BStr;
139139
use std::{borrow::Cow, collections::BTreeSet};
140140

141+
use crate::config::tree::{Remote, Section};
141142
use crate::remote;
142143

144+
/// Query configuration related to remotes.
143145
impl crate::Repository {
144146
/// Returns a sorted list unique of symbolic names of remotes that
145147
/// we deem [trustworthy][crate::open::Options::filter_config_section()].
146148
pub fn remote_names(&self) -> BTreeSet<Cow<'_, BStr>> {
147149
self.config
148150
.resolved
149-
.sections_by_name("remote")
151+
.sections_by_name(Remote.name())
150152
.map(|it| {
151153
let filter = self.filter_config_section();
152154
it.filter(move |s| filter(s.meta()))
@@ -167,9 +169,12 @@ mod remote {
167169
pub fn remote_default_name(&self, direction: remote::Direction) -> Option<Cow<'_, BStr>> {
168170
let name = (direction == remote::Direction::Push)
169171
.then(|| {
170-
self.config
171-
.resolved
172-
.string_filter("remote", None, "pushDefault", &mut self.filter_config_section())
172+
self.config.resolved.string_filter(
173+
Remote.name(),
174+
None,
175+
Remote::PUSH_DEFAULT.name,
176+
&mut self.filter_config_section(),
177+
)
173178
})
174179
.flatten();
175180
name.or_else(|| {
@@ -190,11 +195,15 @@ mod remote {
190195
mod branch {
191196
use std::{borrow::Cow, collections::BTreeSet, convert::TryInto};
192197

193-
use gix_ref::FullNameRef;
194-
use gix_validate::reference::name::Error as ValidateNameError;
198+
use gix_ref::{FullName, FullNameRef};
195199

196200
use crate::bstr::BStr;
201+
use crate::config::cache::util::ApplyLeniencyDefault;
202+
use crate::config::tree::{Branch, Push, Section};
203+
use crate::repository::branch_remote_ref_name;
204+
use crate::{push, remote};
197205

206+
/// Query configuration related to branches.
198207
impl crate::Repository {
199208
/// Return a set of unique short branch names for which custom configuration exists in the configuration,
200209
/// if we deem them [trustworthy][crate::open::Options::filter_config_section()].
@@ -206,38 +215,143 @@ mod branch {
206215
self.subsection_str_names_of("branch")
207216
}
208217

209-
/// Returns the validated reference on the remote associated with the given `short_branch_name`,
210-
/// always `main` instead of `refs/heads/main`.
218+
/// Returns the validated reference on the remote associated with the given `name`,
219+
/// which will be used when *merging*.
220+
/// The returned value corresponds to the `branch.<short_branch_name>.merge` configuration key.
221+
///
222+
/// Returns `None` if there is no value at the given key, or if no remote or remote ref is configured.
223+
/// May return an error if the reference name to be returned is invalid.
211224
///
212-
/// The returned reference is the one we track on the remote side for merging and pushing.
213-
/// Returns `None` if the remote reference was not found.
214-
/// May return an error if the reference is invalid.
215-
pub fn branch_remote_ref<'a>(
225+
/// ### Note
226+
///
227+
/// This name refers to what Git calls upstream branch (as opposed to upstream *tracking* branch).
228+
#[doc(alias = "branch_upstream_name", alias = "git2")]
229+
pub fn branch_remote_ref_name(
216230
&self,
217-
short_branch_name: impl Into<&'a BStr>,
218-
) -> Option<Result<Cow<'_, FullNameRef>, ValidateNameError>> {
219-
self.config
220-
.resolved
221-
.string("branch", Some(short_branch_name.into()), "merge")
222-
.map(crate::config::tree::branch::Merge::try_into_fullrefname)
231+
name: &FullNameRef,
232+
direction: remote::Direction,
233+
) -> Option<Result<Cow<'_, FullNameRef>, branch_remote_ref_name::Error>> {
234+
match direction {
235+
remote::Direction::Fetch => {
236+
let short_name = name.shorten();
237+
self.config
238+
.resolved
239+
.string("branch", Some(short_name), Branch::MERGE.name)
240+
.map(|name| crate::config::tree::branch::Merge::try_into_fullrefname(name).map_err(Into::into))
241+
}
242+
remote::Direction::Push => {
243+
let remote = match self.branch_remote(name.shorten(), direction)? {
244+
Ok(r) => r,
245+
Err(err) => return Some(Err(err.into())),
246+
};
247+
if remote.push_specs.is_empty() {
248+
let push_default = match self
249+
.config
250+
.resolved
251+
.string(Push.name(), None, Push::DEFAULT.name)
252+
.map_or(Ok(Default::default()), |v| {
253+
Push::DEFAULT
254+
.try_into_default(v)
255+
.with_lenient_default(self.config.lenient_config)
256+
}) {
257+
Ok(v) => v,
258+
Err(err) => return Some(Err(err.into())),
259+
};
260+
match push_default {
261+
push::Default::Nothing => None,
262+
push::Default::Current | push::Default::Matching => Some(Ok(Cow::Owned(name.to_owned()))),
263+
push::Default::Upstream => self.branch_remote_ref_name(name, remote::Direction::Fetch),
264+
push::Default::Simple => {
265+
match self.branch_remote_ref_name(name, remote::Direction::Fetch)? {
266+
Ok(fetch_ref) if fetch_ref.as_ref() == name => Some(Ok(fetch_ref)),
267+
Err(err) => Some(Err(err)),
268+
Ok(_different_fetch_ref) => None,
269+
}
270+
}
271+
}
272+
} else {
273+
let search = gix_refspec::MatchGroup::from_push_specs(
274+
remote
275+
.push_specs
276+
.iter()
277+
.map(gix_refspec::RefSpec::to_ref)
278+
.filter(|spec| spec.destination().is_some()),
279+
);
280+
let null_id = self.object_hash().null();
281+
let out = search.match_remotes(
282+
Some(gix_refspec::match_group::Item {
283+
full_ref_name: name.as_bstr(),
284+
target: &null_id,
285+
object: None,
286+
})
287+
.into_iter(),
288+
);
289+
out.mappings.into_iter().next().and_then(|m| {
290+
m.rhs.map(|name| {
291+
FullName::try_from(name.into_owned())
292+
.map(Cow::Owned)
293+
.map_err(Into::into)
294+
})
295+
})
296+
}
297+
}
298+
}
223299
}
224300

225301
/// Returns the unvalidated name of the remote associated with the given `short_branch_name`,
226302
/// typically `main` instead of `refs/heads/main`.
227303
/// In some cases, the returned name will be an URL.
228304
/// Returns `None` if the remote was not found or if the name contained illformed UTF-8.
229305
///
306+
/// * if `direction` is [remote::Direction::Fetch], we will query the `branch.<short_name>.remote` configuration.
307+
/// * if `direction` is [remote::Direction::Push], the push remote will be queried by means of `branch.<short_name>.pushRemote`
308+
/// or `remote.pushDefault` as fallback.
309+
///
230310
/// See also [`Reference::remote_name()`][crate::Reference::remote_name()] for a more typesafe version
231311
/// to be used when a `Reference` is available.
312+
///
313+
/// `short_branch_name` can typically be obtained by [shortening a full branch name](FullNameRef::shorten()).
314+
#[doc(alias = "branch_upstream_remote", alias = "git2")]
232315
pub fn branch_remote_name<'a>(
233316
&self,
234317
short_branch_name: impl Into<&'a BStr>,
235-
) -> Option<crate::remote::Name<'_>> {
236-
self.config
237-
.resolved
238-
.string("branch", Some(short_branch_name.into()), "remote")
318+
direction: remote::Direction,
319+
) -> Option<remote::Name<'_>> {
320+
let name = short_branch_name.into();
321+
let config = &self.config.resolved;
322+
(direction == remote::Direction::Push)
323+
.then(|| {
324+
config
325+
.string("branch", Some(name), Branch::PUSH_REMOTE.name)
326+
.or_else(|| config.string("remote", None, crate::config::tree::Remote::PUSH_DEFAULT.name))
327+
})
328+
.flatten()
329+
.or_else(|| config.string("branch", Some(name), Branch::REMOTE.name))
239330
.and_then(|name| name.try_into().ok())
240331
}
332+
333+
/// Like [`branch_remote_name(…)`](Self::branch_remote_name()), but returns a [Remote](crate::Remote).
334+
/// `short_branch_name` is the name to use for looking up `branch.<short_branch_name>.*` values in the
335+
/// configuration.
336+
pub fn branch_remote<'a>(
337+
&self,
338+
short_branch_name: impl Into<&'a BStr>,
339+
direction: remote::Direction,
340+
) -> Option<Result<crate::Remote<'_>, remote::find::existing::Error>> {
341+
let name = self.branch_remote_name(short_branch_name, direction)?;
342+
self.try_find_remote(name.as_bstr())
343+
.map(|res| res.map_err(Into::into))
344+
.or_else(|| match name {
345+
remote::Name::Url(url) => gix_url::parse(url.as_ref())
346+
.map_err(Into::into)
347+
.and_then(|url| {
348+
self.remote_at(url)
349+
.map_err(|err| remote::find::existing::Error::Find(remote::find::Error::Init(err)))
350+
})
351+
.into(),
352+
remote::Name::Symbol(_) => None,
353+
})
354+
}
241355
}
242356
}
243357

‎gix/src/repository/mod.rs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,22 @@ mod submodule;
6969
mod thread_safe;
7070
mod worktree;
7171

72+
///
73+
pub mod branch_remote_ref_name {
74+
75+
/// The error returned by [Repository::branch_remote_ref_name()](crate::Repository::branch_remote_ref_name()).
76+
#[derive(Debug, thiserror::Error)]
77+
#[allow(missing_docs)]
78+
pub enum Error {
79+
#[error("The configured name of the remote ref to merge wasn't valid")]
80+
ValidateFetchRemoteRefName(#[from] gix_validate::reference::name::Error),
81+
#[error(transparent)]
82+
PushDefault(#[from] crate::config::key::GenericErrorWithValue),
83+
#[error(transparent)]
84+
FindPushRemote(#[from] crate::remote::find::existing::Error),
85+
}
86+
}
87+
7288
/// A type to represent an index which either was loaded from disk as it was persisted there, or created on the fly in memory.
7389
#[cfg(feature = "index")]
7490
pub enum IndexPersistedOrInMemory {

‎gix/tests/clone/mod.rs‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -443,14 +443,19 @@ mod blocking_io {
443443
"local clone always adopts the name of the remote"
444444
);
445445

446-
let short_name = referent.name().shorten();
446+
let ref_name = referent.name();
447447
assert_eq!(
448-
repo.branch_remote_name(short_name).expect("remote is set").as_ref(),
448+
referent
449+
.remote_name(gix::remote::Direction::Fetch)
450+
.expect("remote is set")
451+
.as_ref(),
449452
remote_name,
450453
"the remote branch information is fully configured"
451454
);
452455
assert_eq!(
453-
repo.branch_remote_ref(short_name).expect("present")?.as_bstr(),
456+
repo.branch_remote_ref_name(ref_name, gix::remote::Direction::Fetch)
457+
.expect("present")?
458+
.as_bstr(),
454459
"refs/heads/main"
455460
);
456461

‎gix/tests/fixtures/generated-archives/make_remote_config_repos.tar.xz‎

11.3 KB
Binary file not shown.

‎gix/tests/fixtures/generated-archives/make_remote_repo.tar.xz‎

-9.6 KB
Binary file not shown.

‎gix/tests/fixtures/make_remote_config_repos.sh‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
#!/bin/bash
2+
set -eu -o pipefail
3+
4+
(mkdir fetch && cd fetch
5+
git init -q
6+
7+
git checkout -b main
8+
9+
git commit --allow-empty -q -m c1
10+
11+
git remote add --fetch remote_repo .
12+
git branch --set-upstream-to remote_repo/main
13+
14+
git config branch.broken.merge not_a_valid_merge_ref
15+
git config push.default simple
16+
)
17+
18+
(mkdir push-mapped && cd push-mapped
19+
git init -q
20+
21+
git checkout -b main
22+
git commit --allow-empty -q -m c1
23+
24+
cat<<EOF >.git/config
25+
[remote "origin"]
26+
url = .
27+
fetch = +refs/heads/*:refs/remotes/origin/*
28+
push = refs/heads/main ; this should be ignored
29+
push = refs/heads/main:refs/heads/remapped-main
30+
push = refs/heads/main:refs/heads/skipped ; skipped as it's not the first matching one
31+
push = refs/heads/feature:refs/heads/remapped-feature ; this is picked up before going to push.default (which would fail)
32+
33+
[branch "main"]
34+
remote = "origin"
35+
merge = refs/heads/main
36+
37+
[push]
38+
default = simple
39+
40+
[branch "feature"]
41+
remote = "origin"
42+
merge = refs/heads/main ; this one is remapped to merge from main, which doesn't affect the push remote.
43+
EOF
44+
)
45+
46+
(mkdir push-missing && cd push-missing
47+
git init -q
48+
49+
git checkout -b main
50+
git commit --allow-empty -q -m c1
51+
52+
cat<<EOF >.git/config
53+
[remote "origin"]
54+
url = .
55+
fetch = +refs/heads/*:refs/remotes/origin/*
56+
push = refs/heads/main ; there is a match, but no destination is available
57+
58+
[push]
59+
default = current ; this could work, but the default isn't looked at if there are any push specs
60+
61+
[branch "main"]
62+
remote = "origin"
63+
merge = refs/heads/main
64+
EOF
65+
)
66+
67+
(mkdir push-default-current && cd push-default-current
68+
git init -q
69+
70+
git checkout -b main
71+
git commit --allow-empty -q -m c1
72+
73+
cat<<EOF >.git/config
74+
[remote "origin"]
75+
url = .
76+
fetch = +refs/heads/*:refs/remotes/origin/*
77+
78+
[push]
79+
default = current ; this would be the one setting that works as it ignores 'branch.main.merge'
80+
81+
[branch "main"]
82+
remote = "origin"
83+
merge = refs/heads/other
84+
EOF
85+
)
86+

0 commit comments

Comments
 (0)