Skip to content

Commit b7fcee1

Browse files
committed
fix!: assure that rename tracking can be turned off.
Previously it was impossible to tell if rename tracking was disabled, or if it was unset, which leads to incorrect logic. This changes the signature of `diff::new_rewrites()` to also provide information about whether or not it was configured.
1 parent ee6e503 commit b7fcee1

File tree

4 files changed

+43
-33
lines changed

4 files changed

+43
-33
lines changed

gix/src/config/cache/access.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ impl Cache {
156156
}
157157

158158
#[cfg(feature = "blob-diff")]
159-
pub(crate) fn diff_renames(&self) -> Result<Option<gix_diff::Rewrites>, crate::diff::new_rewrites::Error> {
159+
pub(crate) fn diff_renames(&self) -> Result<(Option<gix_diff::Rewrites>, bool), crate::diff::new_rewrites::Error> {
160160
self.diff_renames
161161
.get_or_try_init(|| crate::diff::new_rewrites(&self.resolved, self.lenient_config))
162162
.copied()

gix/src/config/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -605,7 +605,7 @@ pub(crate) struct Cache {
605605
pub(crate) url_rewrite: OnceCell<crate::remote::url::Rewrite>,
606606
/// The lazy-loaded rename information for diffs.
607607
#[cfg(feature = "blob-diff")]
608-
pub(crate) diff_renames: OnceCell<Option<crate::diff::Rewrites>>,
608+
pub(crate) diff_renames: OnceCell<(Option<crate::diff::Rewrites>, bool)>,
609609
/// A lazily loaded mapping to know which url schemes to allow
610610
#[cfg(any(feature = "blocking-network-client", feature = "async-network-client"))]
611611
pub(crate) url_scheme: OnceCell<crate::remote::url::SchemePermission>,

gix/src/diff.rs

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,14 @@ impl Options {
5151
pub(crate) fn from_configuration(config: &crate::config::Cache) -> Result<Self, options::init::Error> {
5252
Ok(Options {
5353
location: Some(Location::Path),
54-
rewrites: config.diff_renames()?.unwrap_or_default().into(),
54+
rewrites: {
55+
let (rewrites, is_configured) = config.diff_renames()?;
56+
if is_configured {
57+
rewrites
58+
} else {
59+
Some(Default::default())
60+
}
61+
},
5562
})
5663
}
5764
}
@@ -164,14 +171,14 @@ pub(crate) mod utils {
164171
}
165172

166173
/// Create an instance by reading all relevant information from the `config`uration, while being `lenient` or not.
167-
/// Returns `Ok(None)` if nothing is configured.
174+
/// Returns `Ok((None, false))` if nothing is configured, or `Ok((None, true))` if it's configured and disabled.
168175
///
169176
/// Note that missing values will be defaulted similar to what git does.
170177
#[allow(clippy::result_large_err)]
171178
pub fn new_rewrites(
172179
config: &gix_config::File<'static>,
173180
lenient: bool,
174-
) -> Result<Option<Rewrites>, new_rewrites::Error> {
181+
) -> Result<(Option<Rewrites>, bool), new_rewrites::Error> {
175182
new_rewrites_inner(config, lenient, &Diff::RENAMES, &Diff::RENAME_LIMIT)
176183
}
177184

@@ -180,33 +187,36 @@ pub(crate) mod utils {
180187
lenient: bool,
181188
renames: &'static crate::config::tree::diff::Renames,
182189
rename_limit: &'static crate::config::tree::keys::UnsignedInteger,
183-
) -> Result<Option<Rewrites>, new_rewrites::Error> {
190+
) -> Result<(Option<Rewrites>, bool), new_rewrites::Error> {
184191
let copies = match config
185192
.boolean(renames)
186193
.map(|value| renames.try_into_renames(value))
187194
.transpose()
188195
.with_leniency(lenient)?
189196
{
190197
Some(renames) => match renames {
191-
Tracking::Disabled => return Ok(None),
198+
Tracking::Disabled => return Ok((None, true)),
192199
Tracking::Renames => None,
193200
Tracking::RenamesAndCopies => Some(Copies::default()),
194201
},
195-
None => return Ok(None),
202+
None => return Ok((None, false)),
196203
};
197204

198205
let default = Rewrites::default();
199-
Ok(Rewrites {
200-
copies,
201-
limit: config
202-
.integer(rename_limit)
203-
.map(|value| rename_limit.try_into_usize(value))
204-
.transpose()
205-
.with_leniency(lenient)?
206-
.unwrap_or(default.limit),
207-
..default
208-
}
209-
.into())
206+
Ok((
207+
Rewrites {
208+
copies,
209+
limit: config
210+
.integer(rename_limit)
211+
.map(|value| rename_limit.try_into_usize(value))
212+
.transpose()
213+
.with_leniency(lenient)?
214+
.unwrap_or(default.limit),
215+
..default
216+
}
217+
.into(),
218+
true,
219+
))
210220
}
211221

212222
/// Return a low-level utility to efficiently prepare a blob-level diff operation between two resources,

gix/src/repository/merge.rs

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -88,21 +88,21 @@ impl Repository {
8888

8989
/// Read all relevant configuration options to instantiate options for use in [`merge_trees()`](Self::merge_trees).
9090
pub fn tree_merge_options(&self) -> Result<crate::merge::tree::Options, tree_merge_options::Error> {
91+
let (mut rewrites, mut is_configured) = crate::diff::utils::new_rewrites_inner(
92+
&self.config.resolved,
93+
self.config.lenient_config,
94+
&tree::Merge::RENAMES,
95+
&tree::Merge::RENAME_LIMIT,
96+
)?;
97+
if !is_configured {
98+
(rewrites, is_configured) =
99+
crate::diff::utils::new_rewrites(&self.config.resolved, self.config.lenient_config)?;
100+
}
101+
if !is_configured {
102+
rewrites = Some(Default::default());
103+
}
91104
Ok(gix_merge::tree::Options {
92-
rewrites: Some(
93-
crate::diff::utils::new_rewrites_inner(
94-
&self.config.resolved,
95-
self.config.lenient_config,
96-
&tree::Merge::RENAMES,
97-
&tree::Merge::RENAME_LIMIT,
98-
)?
99-
.map(Ok)
100-
.or_else(|| {
101-
crate::diff::utils::new_rewrites(&self.config.resolved, self.config.lenient_config).transpose()
102-
})
103-
.transpose()?
104-
.unwrap_or_default(),
105-
),
105+
rewrites,
106106
blob_merge: self.blob_merge_options()?,
107107
blob_merge_command_ctx: self.command_context()?,
108108
fail_on_conflict: None,

0 commit comments

Comments
 (0)