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
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

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 30339

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,
); );