This is an archive of the discontinued Mercurial Phabricator instance.

Differential D11465

rust: Remove EntryState::Unknown
ClosedPublic

Authored by SimonSapin on Sep 20 2021, 6:17 PM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG1b2ee68e85f9: rust: Remove EntryState::Unknown
Summary

This enum variant represented the state == '?' case, which was used
to represent the absence of a dirstate entry/item (and therefore of that
entry’s state).

Now that previous refactors have removed this use in the Python/Rust
FFI APIs, the remaining uses can be removed by replacing EntryState
by Option<EntryState> where appropriate, using None to represent
the absence of an entry.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

SimonSapin created this revision.Sep 20 2021, 6:17 PM
Alphare accepted this revision.Sep 22 2021, 4:10 AM
This revision is now accepted and ready to land.Sep 22 2021, 4:10 AM
SimonSapin added a commit: rHG1b2ee68e85f9: rust: Remove EntryState::Unknown.Sep 22 2021, 4:55 AM
Closed by commit rHG1b2ee68e85f9: rust: Remove EntryState::Unknown (authored by SimonSapin). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 30377

rust/hg-core/src/dirstate/dirstate_map.rs

} else { } else {
state = EntryState::Normal; state = EntryState::Normal;
size = entry.size() & V1_RANGEMASK; size = entry.size() & V1_RANGEMASK;
mtime = entry.mtime() & V1_RANGEMASK; mtime = entry.mtime() & V1_RANGEMASK;
} }
let mode = entry.mode(); let mode = entry.mode();
let entry = DirstateEntry::from_v1_data(state, mode, size, mtime); let entry = DirstateEntry::from_v1_data(state, mode, size, mtime);
let old_state = match self.get(filename) { let old_state = self.get(filename).map(|e| e.state());
Some(e) => e.state(), if old_state.is_none() || old_state == Some(EntryState::Removed) {
None => EntryState::Unknown,
};
if old_state == EntryState::Unknown || old_state == EntryState::Removed
{
if let Some(ref mut dirs) = self.dirs { if let Some(ref mut dirs) = self.dirs {
dirs.add_path(filename)?; dirs.add_path(filename)?;
} }
} }
if old_state == EntryState::Unknown { if old_state.is_none() {
if let Some(ref mut all_dirs) = self.all_dirs { if let Some(ref mut all_dirs) = self.all_dirs {
all_dirs.add_path(filename)?; all_dirs.add_path(filename)?;
} }
} }
self.state_map.insert(filename.to_owned(), entry.to_owned()); self.state_map.insert(filename.to_owned(), entry.to_owned());
if entry.is_non_normal() { if entry.is_non_normal() {
self.get_non_normal_other_parent_entries() self.get_non_normal_other_parent_entries()
/// the file's previous state. In the future, we should refactor this /// the file's previous state. In the future, we should refactor this
/// to be more explicit about what that state is. /// to be more explicit about what that state is.
pub fn remove_file( pub fn remove_file(
&mut self, &mut self,
filename: &HgPath, filename: &HgPath,
in_merge: bool, in_merge: bool,
) -> Result<(), DirstateError> { ) -> Result<(), DirstateError> {
let old_entry_opt = self.get(filename); let old_entry_opt = self.get(filename);
let old_state = match old_entry_opt { let old_state = old_entry_opt.map(|e| e.state());
Some(e) => e.state(),
None => EntryState::Unknown,
};
let mut size = 0; let mut size = 0;
if in_merge { if in_merge {
// XXX we should not be able to have 'm' state and 'FROM_P2' if not // XXX we should not be able to have 'm' state and 'FROM_P2' if not
// during a merge. So I (marmoute) am not sure we need the // during a merge. So I (marmoute) am not sure we need the
// conditionnal at all. Adding double checking this with assert // conditionnal at all. Adding double checking this with assert
// would be nice. // would be nice.
if let Some(old_entry) = old_entry_opt { if let Some(old_entry) = old_entry_opt {
// backup the previous state // backup the previous state
if old_entry.state() == EntryState::Merged { if old_entry.state() == EntryState::Merged {
size = SIZE_NON_NORMAL; size = SIZE_NON_NORMAL;
} else if old_entry.state() == EntryState::Normal } else if old_entry.state() == EntryState::Normal
&& old_entry.size() == SIZE_FROM_OTHER_PARENT && old_entry.size() == SIZE_FROM_OTHER_PARENT
{ {
// other parent // other parent
size = SIZE_FROM_OTHER_PARENT; size = SIZE_FROM_OTHER_PARENT;
self.get_non_normal_other_parent_entries() self.get_non_normal_other_parent_entries()
.1 .1
.insert(filename.to_owned()); .insert(filename.to_owned());
} }
} }
} }
if old_state != EntryState::Unknown && old_state != EntryState::Removed if old_state.is_some() && old_state != Some(EntryState::Removed) {
{
if let Some(ref mut dirs) = self.dirs { if let Some(ref mut dirs) = self.dirs {
dirs.delete_path(filename)?; dirs.delete_path(filename)?;
} }
} }
if old_state == EntryState::Unknown { if old_state.is_none() {
if let Some(ref mut all_dirs) = self.all_dirs { if let Some(ref mut all_dirs) = self.all_dirs {
all_dirs.add_path(filename)?; all_dirs.add_path(filename)?;
} }
} }
if size == 0 { if size == 0 {
self.copy_map.remove(filename); self.copy_map.remove(filename);
} }
self.state_map self.state_map
.insert(filename.to_owned(), DirstateEntry::new_removed(size)); .insert(filename.to_owned(), DirstateEntry::new_removed(size));
self.get_non_normal_other_parent_entries() self.get_non_normal_other_parent_entries()
.0 .0
.insert(filename.to_owned()); .insert(filename.to_owned());
Ok(()) Ok(())
} }
/// Remove a file from the dirstate. /// Remove a file from the dirstate.
/// Returns `true` if the file was previously recorded. /// Returns `true` if the file was previously recorded.
pub fn drop_file( pub fn drop_file(
&mut self, &mut self,
filename: &HgPath, filename: &HgPath,
) -> Result<bool, DirstateError> { ) -> Result<bool, DirstateError> {
let old_state = match self.get(filename) { let old_state = self.get(filename).map(|e| e.state());
Some(e) => e.state(),
None => EntryState::Unknown,
};
let exists = self.state_map.remove(filename).is_some(); let exists = self.state_map.remove(filename).is_some();
if exists { if exists {
if old_state != EntryState::Removed { if old_state != Some(EntryState::Removed) {
if let Some(ref mut dirs) = self.dirs { if let Some(ref mut dirs) = self.dirs {
dirs.delete_path(filename)?; dirs.delete_path(filename)?;
} }
} }
if let Some(ref mut all_dirs) = self.all_dirs { if let Some(ref mut all_dirs) = self.all_dirs {
all_dirs.delete_path(filename)?; all_dirs.delete_path(filename)?;
} }
} }

rust/hg-core/src/dirstate/entry.rs

use crate::errors::HgError; use crate::errors::HgError;
use std::convert::TryFrom; use std::convert::TryFrom;
#[derive(Copy, Clone, Debug, Eq, PartialEq)] #[derive(Copy, Clone, Debug, Eq, PartialEq)]
pub enum EntryState { pub enum EntryState {
Normal, Normal,
Added, Added,
Removed, Removed,
Merged, Merged,
Unknown,
} }
/// The C implementation uses all signed types. This will be an issue /// The C implementation uses all signed types. This will be an issue
/// either when 4GB+ source files are commonplace or in 2038, whichever /// either when 4GB+ source files are commonplace or in 2038, whichever
/// comes first. /// comes first.
#[derive(Debug, PartialEq, Copy, Clone)] #[derive(Debug, PartialEq, Copy, Clone)]
pub struct DirstateEntry { pub struct DirstateEntry {
state: EntryState, state: EntryState,
} }
} }
impl EntryState { impl EntryState {
pub fn is_tracked(self) -> bool { pub fn is_tracked(self) -> bool {
use EntryState::*; use EntryState::*;
match self { match self {
Normal | Added | Merged => true, Normal | Added | Merged => true,
Removed | Unknown => false, Removed => false,
} }
} }
} }
impl TryFrom<u8> for EntryState { impl TryFrom<u8> for EntryState {
type Error = HgError; type Error = HgError;
fn try_from(value: u8) -> Result<Self, Self::Error> { fn try_from(value: u8) -> Result<Self, Self::Error> {
match value { match value {
b'n' => Ok(EntryState::Normal), b'n' => Ok(EntryState::Normal),
b'a' => Ok(EntryState::Added), b'a' => Ok(EntryState::Added),
b'r' => Ok(EntryState::Removed), b'r' => Ok(EntryState::Removed),
b'm' => Ok(EntryState::Merged), b'm' => Ok(EntryState::Merged),
b'?' => Ok(EntryState::Unknown),
_ => Err(HgError::CorruptedRepository(format!( _ => Err(HgError::CorruptedRepository(format!(
"Incorrect dirstate entry state {}", "Incorrect dirstate entry state {}",
value value
))), ))),
} }
} }
} }
impl Into<u8> for EntryState { impl Into<u8> for EntryState {
fn into(self) -> u8 { fn into(self) -> u8 {
match self { match self {
EntryState::Normal => b'n', EntryState::Normal => b'n',
EntryState::Added => b'a', EntryState::Added => b'a',
EntryState::Removed => b'r', EntryState::Removed => b'r',
EntryState::Merged => b'm', EntryState::Merged => b'm',
EntryState::Unknown => b'?',
} }
} }
} }

rust/hg-core/src/dirstate/status.rs

Dispatch::Clean Dispatch::Clean
} else { } else {
Dispatch::None Dispatch::None
} }
} }
EntryState::Merged => Dispatch::Modified, EntryState::Merged => Dispatch::Modified,
EntryState::Added => Dispatch::Added, EntryState::Added => Dispatch::Added,
EntryState::Removed => Dispatch::Removed, EntryState::Removed => Dispatch::Removed,
EntryState::Unknown => Dispatch::Unknown,
} }
} }
/// The file corresponding to this Dirstate entry is missing. /// The file corresponding to this Dirstate entry is missing.
fn dispatch_missing(state: EntryState) -> Dispatch { fn dispatch_missing(state: EntryState) -> Dispatch {
match state { match state {
// File was removed from the filesystem during commands // File was removed from the filesystem during commands
EntryState::Normal | EntryState::Merged | EntryState::Added => { EntryState::Normal | EntryState::Merged | EntryState::Added => {
Dispatch::Deleted Dispatch::Deleted
} }
// File was removed, everything is normal // File was removed, everything is normal
EntryState::Removed => Dispatch::Removed, EntryState::Removed => Dispatch::Removed,
// File is unknown to Mercurial, everything is normal
EntryState::Unknown => Dispatch::Unknown,
} }
} }
fn dispatch_os_error(e: &std::io::Error) -> Dispatch { fn dispatch_os_error(e: &std::io::Error) -> Dispatch {
Dispatch::Bad(BadMatch::OsError( Dispatch::Bad(BadMatch::OsError(
e.raw_os_error().expect("expected real OS error"), e.raw_os_error().expect("expected real OS error"),
)) ))
} }

rust/hg-core/src/dirstate_tree/dirstate_map.rs

return Ok(child_node); return Ok(child_node);
} }
} }
} }
fn add_or_remove_file( fn add_or_remove_file(
&mut self, &mut self,
path: &HgPath, path: &HgPath,
old_state: EntryState, old_state: Option<EntryState>,
new_entry: DirstateEntry, new_entry: DirstateEntry,
) -> Result<(), DirstateV2ParseError> { ) -> Result<(), DirstateV2ParseError> {
let had_entry = old_state != EntryState::Unknown; let had_entry = old_state.is_some();
let was_tracked = old_state.map_or(false, |s| s.is_tracked());
let tracked_count_increment = let tracked_count_increment =
match (old_state.is_tracked(), new_entry.state().is_tracked()) { match (was_tracked, new_entry.state().is_tracked()) {
(false, true) => 1, (false, true) => 1,
(true, false) => -1, (true, false) => -1,
_ => 0, _ => 0,
}; };
let node = Self::get_or_insert_node( let node = Self::get_or_insert_node(
self.on_disk, self.on_disk,
&mut self.unreachable_bytes, &mut self.unreachable_bytes,
} else { } else {
state = EntryState::Normal; state = EntryState::Normal;
size = entry.size() & V1_RANGEMASK; size = entry.size() & V1_RANGEMASK;
mtime = entry.mtime() & V1_RANGEMASK; mtime = entry.mtime() & V1_RANGEMASK;
} }
let mode = entry.mode(); let mode = entry.mode();
let entry = DirstateEntry::from_v1_data(state, mode, size, mtime); let entry = DirstateEntry::from_v1_data(state, mode, size, mtime);
let old_state = match self.get(filename)? { let old_state = self.get(filename)?.map(|e| e.state());
Some(e) => e.state(),
None => EntryState::Unknown,
};
Ok(self.add_or_remove_file(filename, old_state, entry)?) Ok(self.add_or_remove_file(filename, old_state, entry)?)
} }
fn remove_file( fn remove_file(
&mut self, &mut self,
filename: &HgPath, filename: &HgPath,
in_merge: bool, in_merge: bool,
) -> Result<(), DirstateError> { ) -> Result<(), DirstateError> {
let old_entry_opt = self.get(filename)?; let old_entry_opt = self.get(filename)?;
let old_state = match old_entry_opt { let old_state = old_entry_opt.map(|e| e.state());
Some(e) => e.state(),
None => EntryState::Unknown,
};
let mut size = 0; let mut size = 0;
if in_merge { if in_merge {
// XXX we should not be able to have 'm' state and 'FROM_P2' if not // XXX we should not be able to have 'm' state and 'FROM_P2' if not
// during a merge. So I (marmoute) am not sure we need the // during a merge. So I (marmoute) am not sure we need the
// conditionnal at all. Adding double checking this with assert // conditionnal at all. Adding double checking this with assert
// would be nice. // would be nice.
if let Some(old_entry) = old_entry_opt { if let Some(old_entry) = old_entry_opt {
// backup the previous state // backup the previous state
if size == 0 { if size == 0 {
self.copy_map_remove(filename)?; self.copy_map_remove(filename)?;
} }
let entry = DirstateEntry::new_removed(size); let entry = DirstateEntry::new_removed(size);
Ok(self.add_or_remove_file(filename, old_state, entry)?) Ok(self.add_or_remove_file(filename, old_state, entry)?)
} }
fn drop_file(&mut self, filename: &HgPath) -> Result<bool, DirstateError> { fn drop_file(&mut self, filename: &HgPath) -> Result<bool, DirstateError> {
let old_state = match self.get(filename)? { let was_tracked = self
Some(e) => e.state(), .get(filename)?
None => EntryState::Unknown, .map_or(false, |e| e.state().is_tracked());
};
struct Dropped { struct Dropped {
was_tracked: bool, was_tracked: bool,
had_entry: bool, had_entry: bool,
had_copy_source: bool, had_copy_source: bool,
} }
/// If this returns `Ok(Some((dropped, removed)))`, then /// If this returns `Ok(Some((dropped, removed)))`, then
/// ///
if dropped.had_entry { if dropped.had_entry {
self.nodes_with_entry_count -= 1 self.nodes_with_entry_count -= 1
} }
if dropped.had_copy_source { if dropped.had_copy_source {
self.nodes_with_copy_source_count -= 1 self.nodes_with_copy_source_count -= 1
} }
Ok(dropped.had_entry) Ok(dropped.had_entry)
} else { } else {
debug_assert!(!old_state.is_tracked()); debug_assert!(!was_tracked);
Ok(false) Ok(false)
} }
} }
fn clear_ambiguous_times( fn clear_ambiguous_times(
&mut self, &mut self,
filenames: Vec<HgPathBuf>, filenames: Vec<HgPathBuf>,
now: i32, now: i32,

rust/hg-core/src/dirstate_tree/status.rs

EntryState::Merged => self EntryState::Merged => self
.outcome .outcome
.lock() .lock()
.unwrap() .unwrap()
.modified .modified
.push(hg_path.detach_from_tree()), .push(hg_path.detach_from_tree()),
EntryState::Normal => self EntryState::Normal => self
.handle_normal_file(&dirstate_node, fs_metadata)?, .handle_normal_file(&dirstate_node, fs_metadata)?,
// This variant is not used in DirstateMap
// nodes
EntryState::Unknown => unreachable!(),
} }
} else { } else {
// `node.entry.is_none()` indicates a "directory" // `node.entry.is_none()` indicates a "directory"
// node, but the filesystem has a file // node, but the filesystem has a file
self.mark_unknown_or_ignored( self.mark_unknown_or_ignored(
has_ignored_ancestor, has_ignored_ancestor,
hg_path, hg_path,
); );