Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

status improvements #1746

Merged
merged 7 commits into from
Jan 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions gitoxide-core/src/repository/status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,6 @@ pub fn show(
)?;
continue;
}
gix::diff::index::Change::Unmerged { .. } => {
// Unmerged entries from the worktree-index are displayed as part of the index-worktree comparison.
// Here we have nothing to do with them and can ignore.
continue;
}
};
writeln!(
out,
Expand Down
31 changes: 3 additions & 28 deletions gix-diff/src/index/change.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,19 +70,6 @@ impl ChangeRef<'_, '_> {
id: Cow::Owned(id.into_owned()),
copy,
},
ChangeRef::Unmerged {
location,
stage,
index,
entry_mode,
id,
} => ChangeRef::Unmerged {
location: Cow::Owned(location.into_owned()),
stage,
index,
entry_mode,
id: Cow::Owned(id.into_owned()),
},
}
}
}
Expand Down Expand Up @@ -120,13 +107,6 @@ impl ChangeRef<'_, '_> {
entry_mode,
id,
..
}
| ChangeRef::Unmerged {
location,
index,
entry_mode,
id,
..
} => (location.as_ref(), *index, *entry_mode, id),
}
}
Expand All @@ -138,7 +118,7 @@ impl rewrites::tracker::Change for ChangeRef<'_, '_> {
ChangeRef::Addition { id, .. } | ChangeRef::Deletion { id, .. } | ChangeRef::Modification { id, .. } => {
id.as_ref()
}
ChangeRef::Rewrite { .. } | ChangeRef::Unmerged { .. } => {
ChangeRef::Rewrite { .. } => {
unreachable!("BUG")
}
}
Expand All @@ -156,9 +136,6 @@ impl rewrites::tracker::Change for ChangeRef<'_, '_> {
ChangeRef::Rewrite { .. } => {
unreachable!("BUG: rewrites can't be determined ahead of time")
}
ChangeRef::Unmerged { .. } => {
unreachable!("BUG: unmerged don't participate in rename tracking")
}
}
}

Expand All @@ -167,8 +144,7 @@ impl rewrites::tracker::Change for ChangeRef<'_, '_> {
ChangeRef::Addition { entry_mode, .. }
| ChangeRef::Deletion { entry_mode, .. }
| ChangeRef::Modification { entry_mode, .. }
| ChangeRef::Rewrite { entry_mode, .. }
| ChangeRef::Unmerged { entry_mode, .. } => {
| ChangeRef::Rewrite { entry_mode, .. } => {
entry_mode
.to_tree_entry_mode()
// Default is for the impossible case - just don't let it participate in rename tracking.
Expand All @@ -182,8 +158,7 @@ impl rewrites::tracker::Change for ChangeRef<'_, '_> {
ChangeRef::Addition { id, entry_mode, .. }
| ChangeRef::Deletion { id, entry_mode, .. }
| ChangeRef::Modification { id, entry_mode, .. }
| ChangeRef::Rewrite { id, entry_mode, .. }
| ChangeRef::Unmerged { id, entry_mode, .. } => {
| ChangeRef::Rewrite { id, entry_mode, .. } => {
(
id,
entry_mode
Expand Down
68 changes: 14 additions & 54 deletions gix-diff/src/index/function.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::{Action, ChangeRef, Error, RewriteOptions};
use crate::rewrites;
use bstr::{BStr, BString, ByteSlice};
use bstr::BStr;
use gix_filter::attributes::glob::pattern::Case;
use std::borrow::Cow;
use std::cell::RefCell;
Expand All @@ -20,7 +20,8 @@ use std::cmp::Ordering;
/// Return the outcome of the rewrite tracker if it was enabled.
///
/// Note that only `rhs` may contain unmerged entries, as `rhs` is expected to be the index read from `.git/index`.
/// Unmerged entries are always provided as changes, one stage at a time, up to three stages for *base*, *ours* and *theirs*.
/// Unmerged entries are skipped entirely.
///
/// Conceptually, `rhs` is *ours*, and `lhs` is *theirs*.
/// The entries in `lhs` and `rhs` are both expected to be sorted like index entries are typically sorted.
///
Expand Down Expand Up @@ -74,23 +75,6 @@ where
.filter(|(_, path, e)| pattern_matches.borrow_mut()(path, e)),
);

let mut conflicting_paths = Vec::<BString>::new();
let mut cb = move |change: ChangeRef<'lhs, 'rhs>| {
let (location, ..) = change.fields();
if let ChangeRef::Unmerged { .. } = &change {
if let Err(insert_idx) = conflicting_paths.binary_search_by(|p| p.as_bstr().cmp(location)) {
conflicting_paths.insert(insert_idx, location.to_owned());
}
cb(change)
} else if conflicting_paths
.binary_search_by(|p| p.as_bstr().cmp(location))
.is_err()
{
cb(change)
} else {
Ok(Action::Continue)
}
};
let mut resource_cache_storage = None;
let mut tracker = rewrite_options.map(
|RewriteOptions {
Expand All @@ -107,15 +91,6 @@ where
loop {
match (lhs_storage, rhs_storage) {
(Some(lhs), Some(rhs)) => {
match emit_unmerged_ignore_intent_to_add(rhs, &mut cb)? {
None => {}
Some(Action::Cancel) => return Ok(None),
Some(Action::Continue) => {
rhs_storage = rhs_iter.next();
continue;
}
};

let (lhs_idx, lhs_path, lhs_entry) = lhs;
let (rhs_idx, rhs_path, rhs_entry) = rhs;
match lhs_path.cmp(rhs_path) {
Expand All @@ -126,6 +101,11 @@ where
Action::Cancel => return Ok(None),
},
Ordering::Equal => {
if ignore_unmerged_and_intent_to_add(rhs) {
rhs_storage = rhs_iter.next();
lhs_storage = lhs_iter.next();
continue;
}
if lhs_entry.id != rhs_entry.id || lhs_entry.mode != rhs_entry.mode {
let change = ChangeRef::Modification {
location: Cow::Borrowed(rhs_path),
Expand Down Expand Up @@ -274,8 +254,8 @@ fn emit_addition<'rhs, 'lhs: 'rhs, E>(
where
E: Into<Box<dyn std::error::Error + Send + Sync>>,
{
if let Some(action) = emit_unmerged_ignore_intent_to_add((idx, path, entry), &mut cb)? {
return Ok(action);
if ignore_unmerged_and_intent_to_add((idx, path, entry)) {
return Ok(Action::Continue);
}

let change = ChangeRef::Addition {
Expand All @@ -296,29 +276,9 @@ where
cb(change).map_err(|err| Error::Callback(err.into()))
}

fn emit_unmerged_ignore_intent_to_add<'rhs, 'lhs: 'rhs, E>(
(idx, path, entry): (usize, &'rhs BStr, &'rhs gix_index::Entry),
cb: &mut impl FnMut(ChangeRef<'lhs, 'rhs>) -> Result<Action, E>,
) -> Result<Option<Action>, Error>
where
E: Into<Box<dyn std::error::Error + Send + Sync>>,
{
if entry.flags.contains(gix_index::entry::Flags::INTENT_TO_ADD) {
return Ok(Some(Action::Continue));
}
fn ignore_unmerged_and_intent_to_add<'rhs, 'lhs: 'rhs>(
(_idx, _path, entry): (usize, &'rhs BStr, &'rhs gix_index::Entry),
) -> bool {
let stage = entry.stage();
if stage == gix_index::entry::Stage::Unconflicted {
return Ok(None);
}

Ok(Some(
cb(ChangeRef::Unmerged {
location: Cow::Borrowed(path),
stage,
index: idx,
entry_mode: entry.mode,
id: Cow::Borrowed(entry.id.as_ref()),
})
.map_err(|err| Error::Callback(err.into()))?,
))
entry.flags.contains(gix_index::entry::Flags::INTENT_TO_ADD) || stage != gix_index::entry::Stage::Unconflicted
}
18 changes: 0 additions & 18 deletions gix-diff/src/index/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@ where
}

/// Identify a change that would have to be applied to `lhs` to obtain `rhs`, as provided in [`index()`](crate::index()).
///
/// Note that all variants are unconflicted entries, unless it's the [`Self::Unmerged`] one.
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum ChangeRef<'lhs, 'rhs> {
/// An entry was added to `rhs`.
Expand Down Expand Up @@ -116,22 +114,6 @@ pub enum ChangeRef<'lhs, 'rhs> {
/// or similar content.
copy: bool,
},
/// One of up to three unmerged entries that are provided in order, one for each stage, ordered
/// by `location` and `stage`.
///
/// Unmerged entries also don't participate in rename tracking, and they are never present in `lhs`.
Unmerged {
/// The current location of the entry in `rhs`.
location: Cow<'rhs, BStr>,
/// The stage of the entry, either *base*, *ours*, or *theirs*.
stage: gix_index::entry::Stage,
/// The index into the entries array of `rhs` for full access.
index: usize,
/// The mode of the entry in `rhs`.
entry_mode: gix_index::entry::Mode,
/// The object id of the entry in `rhs`.
id: Cow<'rhs, gix_hash::oid>,
},
}

/// The fully-owned version of [`ChangeRef`].
Expand Down
50 changes: 4 additions & 46 deletions gix-diff/tests/diff/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1191,56 +1191,14 @@ fn unmerged_entries_and_intent_to_add() -> crate::Result {
}),
)?;

// each unmerged entry is emitted separately, and no entry is emitted for
// paths that are mentioned there. Intent-to-add is transparent.
// Intent-to-add is transparent. And unmerged entries aren't emitted either, along with
// their sibling paths.
// All that with rename tracking…
insta::assert_debug_snapshot!(changes.into_iter().collect::<Vec<_>>(), @r#"
[
Unmerged {
location: "src/plumbing-renamed/main.rs",
stage: Base,
index: 2,
entry_mode: Mode(
FILE,
),
id: Sha1(d00491fd7e5bb6fa28c517a0bb32b8b506539d4d),
},
Unmerged {
location: "src/plumbing-renamed/main.rs",
stage: Ours,
index: 3,
entry_mode: Mode(
FILE,
),
id: Sha1(d00491fd7e5bb6fa28c517a0bb32b8b506539d4d),
},
]
"#);
insta::assert_debug_snapshot!(changes.into_iter().collect::<Vec<_>>(), @"[]");

let changes = collect_changes_no_renames("r4-dir-rename-non-identity", ".git/index")?;
// …or without
insta::assert_debug_snapshot!(changes.into_iter().collect::<Vec<_>>(), @r#"
[
Unmerged {
location: "src/plumbing-renamed/main.rs",
stage: Base,
index: 2,
entry_mode: Mode(
FILE,
),
id: Sha1(d00491fd7e5bb6fa28c517a0bb32b8b506539d4d),
},
Unmerged {
location: "src/plumbing-renamed/main.rs",
stage: Ours,
index: 3,
entry_mode: Mode(
FILE,
),
id: Sha1(d00491fd7e5bb6fa28c517a0bb32b8b506539d4d),
},
]
"#);
insta::assert_debug_snapshot!(changes.into_iter().collect::<Vec<_>>(), @"[]");

let (index, _, _, _, _) = repo_with_indices(".git/index", ".git/index", None)?;
assert_eq!(
Expand Down
6 changes: 3 additions & 3 deletions gix-filter/src/pipeline/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,15 +76,15 @@ impl Pipeline {
where
R: std::io::Read,
{
let bstr_path = gix_path::into_bstr(rela_path);
let bstr_rela_path = gix_path::to_unix_separators_on_windows(gix_path::into_bstr(rela_path));
let Configuration {
driver,
digest,
_attr_digest: _,
encoding,
apply_ident_filter,
} = Configuration::at_path(
bstr_path.as_ref(),
bstr_rela_path.as_ref(),
&self.options.drivers,
&mut self.attrs,
attributes,
Expand All @@ -109,7 +109,7 @@ impl Pipeline {
driver,
&mut src,
driver::Operation::Clean,
self.context.with_path(bstr_path.as_ref()),
self.context.with_path(bstr_rela_path.as_ref()),
)? {
if !apply_ident_filter && encoding.is_none() && !would_convert_eol {
// Note that this is not typically a benefit in terms of saving memory as most filters
Expand Down
11 changes: 11 additions & 0 deletions gix-object/src/object/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,17 @@ impl From<tree::EntryRef<'_>> for tree::Entry {
}
}

impl<'a> From<&'a tree::Entry> for tree::EntryRef<'a> {
fn from(other: &'a tree::Entry) -> tree::EntryRef<'a> {
let tree::Entry { mode, filename, oid } = other;
tree::EntryRef {
mode: *mode,
filename: filename.as_ref(),
oid,
}
}
}

impl From<ObjectRef<'_>> for Object {
fn from(v: ObjectRef<'_>) -> Self {
match v {
Expand Down
1 change: 1 addition & 0 deletions gix-object/tests/object/tree/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,7 @@ fn from_existing_add() -> crate::Result {
let root_tree = find_tree(&odb, root_tree_id)?;
odb.access_count_and_clear();
let mut edit = gix_object::tree::Editor::new(root_tree.clone(), &odb, gix_hash::Kind::Sha1);
assert!(edit.get(["bin"]).is_some(), "the root is immediately available");

let actual = edit.write(&mut write).expect("no changes are fine");
assert_eq!(actual, root_tree_id, "it rewrites the same tree");
Expand Down
30 changes: 30 additions & 0 deletions gix/src/object/tree/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,21 @@ impl<'repo> Cursor<'_, 'repo> {
pub fn write(&mut self) -> Result<Id<'repo>, write::Error> {
write_cursor(self)
}

/// Obtain the entry at `rela_path` or return `None` if none was found, or the tree wasn't yet written
/// to that point.
/// The root tree is always available.
/// Note that after [writing](Self::write) only the root path remains, all other intermediate trees are removed.
/// The entry can be anything that can be stored in a tree, but may have a null-id if it's a newly
/// inserted tree. Also, ids of trees might not be accurate as they may have been changed in memory.
pub fn get(&self, rela_path: impl ToComponents) -> Option<crate::object::tree::EntryRef<'repo, '_>> {
self.inner
.get(rela_path.to_components())
.map(|entry| crate::object::tree::EntryRef {
inner: entry.into(),
repo: self.repo,
})
}
}

/// Operations
Expand Down Expand Up @@ -242,6 +257,21 @@ impl<'repo> super::Editor<'repo> {
pub fn write(&mut self) -> Result<Id<'repo>, write::Error> {
write_cursor(&mut self.to_cursor())
}

/// Obtain the entry at `rela_path` or return `None` if none was found, or the tree wasn't yet written
/// to that point.
/// The root tree is always available.
/// Note that after [writing](Self::write) only the root path remains, all other intermediate trees are removed.
/// The entry can be anything that can be stored in a tree, but may have a null-id if it's a newly
/// inserted tree. Also, ids of trees might not be accurate as they may have been changed in memory.
pub fn get(&self, rela_path: impl ToComponents) -> Option<crate::object::tree::EntryRef<'repo, '_>> {
self.inner
.get(rela_path.to_components())
.map(|entry| crate::object::tree::EntryRef {
inner: entry.into(),
repo: self.repo,
})
}
}

fn write_cursor<'repo>(cursor: &mut Cursor<'_, 'repo>) -> Result<Id<'repo>, write::Error> {
Expand Down
Loading
Loading