This is an archive of the discontinued Mercurial Phabricator instance.

Differential D11839

rhg: Add Repo::write_dirstate
ClosedPublic

Authored by SimonSapin on Dec 2 2021, 11:48 AM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG2097f63575a5: rhg: Add Repo::write_dirstate
Summary

This method is not used yet. It saves to disk any mutation that was done to
the Repo’s dirstate through Repo::dirstate_map_mut. It takes care of
dirstate-v1 v.s. dirstate-v2, dockets, data files, appending when possible,
etc.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

SimonSapin created this revision.Dec 2 2021, 11:48 AM
Alphare requested changes to this revision.Dec 3 2021, 8:18 AM
Alphare added a subscriber: Alphare.
Alphare added inline comments.
rust/hg-core/src/repo.rs
431

+1 for a WritableRepo that requires the lock in a future patch

453

This should support HGTEST_UUIDFILE (see mercurial/revlogutils/docket.py) otherwise some tests will end up failing. Possibly this chould just be a TODO, but I'd rather at least detect the env variable and print a warning to stderr that its support isn't implemented, so that it's more obvious down the line.

rust/hg-core/src/revlog/node.rs
178

Cool method

This revision now requires changes to proceed.Dec 3 2021, 8:18 AM
SimonSapin updated this revision to Diff 31320.Dec 6 2021, 5:03 AM
SimonSapin added inline comments.Dec 7 2021, 4:06 AM
rust/hg-core/src/repo.rs
453

https://foss.heptapod.net/mercurial/mercurial-devel/-/jobs/271544 shows that no test fail because of this.

As far as I understand, this point of HGTEST_UUIDFILE is to have deterministic data filenames for nodemap dockets so it’s less annoying to add glob patterns everywhere in expected test ouptput what it includes these names. But this new Rust code is only used to generate dirstate data filenames which are not printed in tests.

To port the current exact behavior of docket.py we’d need to have the same PRNG algorithm as Python’s random.Random(). Instead we could probably change docket.py to use a known, fixed PRNG algorithm.

I’ll probably stick with the warning for now. By stderr do you mean literally using std::eprintln!, or should we start passing around a &Ui value everywhere?

SimonSapin updated this revision to Diff 31343.Dec 7 2021, 7:52 AM
SimonSapin added inline comments.Dec 7 2021, 7:52 AM
rust/hg-core/src/repo.rs
453

I'd rather at least detect the env variable and print a warning

The env variable is always set by run-tests.py, so a warning would affect the output of every single test where hg status writes dirstate updates. So if a warning exists the only practical thing for tests would be to silence it, but that’s the only place where it would happen.

I’ll add a TODO comment and no warning.

Alphare accepted this revision.Dec 7 2021, 9:55 AM
This revision is now accepted and ready to land.Dec 7 2021, 9:55 AM
SimonSapin added a commit: rHG2097f63575a5: rhg: Add Repo::write_dirstate.Dec 7 2021, 10:05 AM
Closed by commit rHG2097f63575a5: rhg: Add Repo::write_dirstate (authored by SimonSapin). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 31320

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

/// Returns new data and metadata together with whether that data should be /// Returns new data and metadata together with whether that data should be
/// appended to the existing data file whose content is at /// appended to the existing data file whose content is at
/// `map.on_disk` (true), instead of written to a new data file /// `map.on_disk` (true), instead of written to a new data file
/// (false). /// (false).
#[timed] #[timed]
pub fn pack_v2( pub fn pack_v2(
&self, &self,
can_append: bool, can_append: bool,
) -> Result<(Vec<u8>, Vec<u8>, bool), DirstateError> { ) -> Result<(Vec<u8>, on_disk::TreeMetadata, bool), DirstateError> {
let map = self.get_map(); let map = self.get_map();
on_disk::write(map, can_append) on_disk::write(map, can_append)
} }
pub fn status<'a>( pub fn status<'a>(
&'a mut self, &'a mut self,
matcher: &'a (dyn Matcher + Sync), matcher: &'a (dyn Matcher + Sync),
root_dir: PathBuf, root_dir: PathBuf,

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

header: &'on_disk DocketHeader, header: &'on_disk DocketHeader,
pub uuid: &'on_disk [u8], pub uuid: &'on_disk [u8],
} }
/// Fields are documented in the *Tree metadata in the docket file* /// Fields are documented in the *Tree metadata in the docket file*
/// section of `mercurial/helptext/internals/dirstate-v2.txt` /// section of `mercurial/helptext/internals/dirstate-v2.txt`
#[derive(BytesCast)] #[derive(BytesCast)]
#[repr(C)] #[repr(C)]
struct TreeMetadata { pub struct TreeMetadata {
root_nodes: ChildNodes, root_nodes: ChildNodes,
nodes_with_entry_count: Size, nodes_with_entry_count: Size,
nodes_with_copy_source_count: Size, nodes_with_copy_source_count: Size,
unreachable_bytes: Size, unreachable_bytes: Size,
unused: [u8; 4], unused: [u8; 4],
/// See *Optional hash of ignore patterns* section of /// See *Optional hash of ignore patterns* section of
/// `mercurial/helptext/internals/dirstate-v2.txt` /// `mercurial/helptext/internals/dirstate-v2.txt`
} }
impl From<DirstateV2ParseError> for crate::DirstateError { impl From<DirstateV2ParseError> for crate::DirstateError {
fn from(error: DirstateV2ParseError) -> Self { fn from(error: DirstateV2ParseError) -> Self {
HgError::from(error).into() HgError::from(error).into()
} }
} }
impl TreeMetadata {
pub fn as_bytes(&self) -> &[u8] {
BytesCast::as_bytes(self)
}
}
impl<'on_disk> Docket<'on_disk> { impl<'on_disk> Docket<'on_disk> {
pub fn serialize(
parents: DirstateParents,
tree_metadata: TreeMetadata,
data_size: u64,
uuid: &[u8],
) -> Result<Vec<u8>, std::num::TryFromIntError> {
let header = DocketHeader {
marker: *V2_FORMAT_MARKER,
parent_1: parents.p1.pad_to_256_bits(),
parent_2: parents.p2.pad_to_256_bits(),
metadata: tree_metadata,
data_size: u32::try_from(data_size)?.into(),
uuid_size: uuid.len().try_into()?,
};
let header = header.as_bytes();
let mut docket = Vec::with_capacity(header.len() + uuid.len());
docket.extend_from_slice(header);
docket.extend_from_slice(uuid);
Ok(docket)
}
pub fn parents(&self) -> DirstateParents { pub fn parents(&self) -> DirstateParents {
use crate::Node; use crate::Node;
let p1 = Node::try_from(&self.header.parent_1[..USED_NODE_ID_BYTES]) let p1 = Node::try_from(&self.header.parent_1[..USED_NODE_ID_BYTES])
.unwrap() .unwrap()
.clone(); .clone();
let p2 = Node::try_from(&self.header.parent_2[..USED_NODE_ID_BYTES]) let p2 = Node::try_from(&self.header.parent_2[..USED_NODE_ID_BYTES])
.unwrap() .unwrap()
.clone(); .clone();
/// Returns new data and metadata, together with whether that data should be /// Returns new data and metadata, together with whether that data should be
/// appended to the existing data file whose content is at /// appended to the existing data file whose content is at
/// `dirstate_map.on_disk` (true), instead of written to a new data file /// `dirstate_map.on_disk` (true), instead of written to a new data file
/// (false). /// (false).
pub(super) fn write( pub(super) fn write(
dirstate_map: &DirstateMap, dirstate_map: &DirstateMap,
can_append: bool, can_append: bool,
) -> Result<(Vec<u8>, Vec<u8>, bool), DirstateError> { ) -> Result<(Vec<u8>, TreeMetadata, bool), DirstateError> {
let append = can_append && dirstate_map.write_should_append(); let append = can_append && dirstate_map.write_should_append();
// This ignores the space for paths, and for nodes without an entry. // This ignores the space for paths, and for nodes without an entry.
// TODO: better estimate? Skip the `Vec` and write to a file directly? // TODO: better estimate? Skip the `Vec` and write to a file directly?
let size_guess = std::mem::size_of::<Node>() let size_guess = std::mem::size_of::<Node>()
* dirstate_map.nodes_with_entry_count as usize; * dirstate_map.nodes_with_entry_count as usize;
let mut writer = Writer { let mut writer = Writer {
nodes_with_entry_count: dirstate_map.nodes_with_entry_count.into(), nodes_with_entry_count: dirstate_map.nodes_with_entry_count.into(),
nodes_with_copy_source_count: dirstate_map nodes_with_copy_source_count: dirstate_map
.nodes_with_copy_source_count .nodes_with_copy_source_count
.into(), .into(),
unreachable_bytes: dirstate_map.unreachable_bytes.into(), unreachable_bytes: dirstate_map.unreachable_bytes.into(),
unused: [0; 4], unused: [0; 4],
ignore_patterns_hash: dirstate_map.ignore_patterns_hash, ignore_patterns_hash: dirstate_map.ignore_patterns_hash,
}; };
Ok((writer.out, meta.as_bytes().to_vec(), append)) Ok((writer.out, meta, append))
} }
struct Writer<'dmap, 'on_disk> { struct Writer<'dmap, 'on_disk> {
dirstate_map: &'dmap DirstateMap<'on_disk>, dirstate_map: &'dmap DirstateMap<'on_disk>,
append: bool, append: bool,
out: Vec<u8>, out: Vec<u8>,
} }

rust/hg-core/src/repo.rs

use crate::changelog::Changelog; use crate::changelog::Changelog;
use crate::config::{Config, ConfigError, ConfigParseError}; use crate::config::{Config, ConfigError, ConfigParseError};
use crate::dirstate::DirstateParents; use crate::dirstate::DirstateParents;
use crate::dirstate_tree::dirstate_map::DirstateMap; use crate::dirstate_tree::dirstate_map::DirstateMap;
use crate::dirstate_tree::on_disk::Docket as DirstateDocket;
use crate::dirstate_tree::owning::OwningDirstateMap; use crate::dirstate_tree::owning::OwningDirstateMap;
use crate::errors::HgError;
use crate::errors::HgResultExt; use crate::errors::HgResultExt;
use crate::errors::{HgError, IoResultExt};
use crate::exit_codes; use crate::exit_codes;
use crate::lock::{try_with_lock_no_wait, LockError}; use crate::lock::{try_with_lock_no_wait, LockError};
use crate::manifest::{Manifest, Manifestlog}; use crate::manifest::{Manifest, Manifestlog};
use crate::revlog::filelog::Filelog; use crate::revlog::filelog::Filelog;
use crate::revlog::revlog::RevlogError; use crate::revlog::revlog::RevlogError;
use crate::utils::files::get_path_from_bytes; use crate::utils::files::get_path_from_bytes;
use crate::utils::hg_path::HgPath; use crate::utils::hg_path::HgPath;
use crate::utils::SliceExt; use crate::utils::SliceExt;
use crate::vfs::{is_dir, is_file, Vfs}; use crate::vfs::{is_dir, is_file, Vfs};
use crate::{requirements, NodePrefix}; use crate::{requirements, NodePrefix};
use crate::{DirstateError, Revision}; use crate::{DirstateError, Revision};
use rand::Rng;
use std::cell::{Ref, RefCell, RefMut}; use std::cell::{Ref, RefCell, RefMut};
use std::collections::HashSet; use std::collections::HashSet;
use std::fmt::Write;
use std::io::Seek;
use std::io::SeekFrom;
use std::io::Write as IoWrite;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
/// A repository on disk /// A repository on disk
pub struct Repo { pub struct Repo {
working_directory: PathBuf, working_directory: PathBuf,
dot_hg: PathBuf, dot_hg: PathBuf,
store: PathBuf, store: PathBuf,
requirements: HashSet<String>, requirements: HashSet<String>,
.manifest_node()? .manifest_node()?
.into(), .into(),
) )
} }
pub fn filelog(&self, path: &HgPath) -> Result<Filelog, HgError> { pub fn filelog(&self, path: &HgPath) -> Result<Filelog, HgError> {
Filelog::open(self, path) Filelog::open(self, path)
} }
/// Write to disk any updates that were made through `dirstate_map_mut`.
///
/// The "wlock" must be held while calling this.
/// See for example `try_with_wlock_no_wait`.
///
/// TODO: have a `WritableRepo` type only accessible while holding the
AlphareUnsubmitted
Not Done

+1 for a WritableRepo that requires the lock in a future patch

Alphare: +1 for a WritableRepo that requires the lock in a future patch
/// lock?
pub fn write_dirstate(&self) -> Result<(), DirstateError> {
let map = self.dirstate_map()?;
// TODO: Maintain a `DirstateMap::dirty` flag, and return early here if
// it’s unset
let parents = self.dirstate_parents()?;
let packed_dirstate = if self.has_dirstate_v2() {
let uuid = self.dirstate_data_file_uuid.get_or_init(self)?;
let mut uuid = uuid.as_ref();
let can_append = uuid.is_some();
let (data, tree_metadata, append) = map.pack_v2(can_append)?;
if !append {
uuid = None
}
let uuid = if let Some(uuid) = uuid {
std::str::from_utf8(uuid)
.map_err(|_| {
HgError::corrupted("non-UTF-8 dirstate data file ID")
})?
.to_owned()
} else {
const ID_LENGTH: usize = 8;
AlphareUnsubmitted
Not Done

This should support HGTEST_UUIDFILE (see mercurial/revlogutils/docket.py) otherwise some tests will end up failing. Possibly this chould just be a TODO, but I'd rather at least detect the env variable and print a warning to stderr that its support isn't implemented, so that it's more obvious down the line.

Alphare: This should support `HGTEST_UUIDFILE` (see `mercurial/revlogutils/docket.py`) otherwise some…
SimonSapinAuthorUnsubmitted
Done

https://foss.heptapod.net/mercurial/mercurial-devel/-/jobs/271544 shows that no test fail because of this.

As far as I understand, this point of HGTEST_UUIDFILE is to have deterministic data filenames for nodemap dockets so it’s less annoying to add glob patterns everywhere in expected test ouptput what it includes these names. But this new Rust code is only used to generate dirstate data filenames which are not printed in tests.

To port the current exact behavior of docket.py we’d need to have the same PRNG algorithm as Python’s random.Random(). Instead we could probably change docket.py to use a known, fixed PRNG algorithm.

I’ll probably stick with the warning for now. By stderr do you mean literally using std::eprintln!, or should we start passing around a &Ui value everywhere?

SimonSapin: https://foss.heptapod.net/mercurial/mercurial-devel/-/jobs/271544 shows that no test fail…
SimonSapinAuthorUnsubmitted
Done

I'd rather at least detect the env variable and print a warning

The env variable is always set by run-tests.py, so a warning would affect the output of every single test where hg status writes dirstate updates. So if a warning exists the only practical thing for tests would be to silence it, but that’s the only place where it would happen.

I’ll add a TODO comment and no warning.

SimonSapin: > I'd rather at least detect the env variable and print a warning The env variable is always…
let mut id = String::with_capacity(ID_LENGTH);
let mut rng = rand::thread_rng();
for _ in 0..ID_LENGTH {
// One random hexadecimal digit.
// `unwrap` never panics because `impl Write for String`
// never returns an error.
write!(&mut id, "{:x}", rng.gen_range(0, 16)).unwrap();
}
id
};
let data_filename = format!("dirstate.{}", uuid);
let data_filename = self.hg_vfs().join(data_filename);
let mut options = std::fs::OpenOptions::new();
if append {
options.append(true);
} else {
options.write(true).create_new(true);
}
let data_size = (|| {
// TODO: loop and try another random ID if !append and this
// returns `ErrorKind::AlreadyExists`? Collision chance of two
// random IDs is one in 2**32
let mut file = options.open(&data_filename)?;
file.write_all(&data)?;
file.flush()?;
// TODO: use https://doc.rust-lang.org/std/io/trait.Seek.html#method.stream_position when we require Rust 1.51+
file.seek(SeekFrom::Current(0))
})()
.when_writing_file(&data_filename)?;
DirstateDocket::serialize(
parents,
tree_metadata,
data_size,
uuid.as_bytes(),
)
.map_err(|_: std::num::TryFromIntError| {
HgError::corrupted("overflow in dirstate docket serialization")
})?
} else {
map.pack_v1(parents)?
};
self.hg_vfs().atomic_write("dirstate", &packed_dirstate)?;
Ok(())
}
} }
/// Lazily-initialized component of `Repo` with interior mutability /// Lazily-initialized component of `Repo` with interior mutability
/// ///
/// This differs from `OnceCell` in that the value can still be "deinitialized" /// This differs from `OnceCell` in that the value can still be "deinitialized"
/// later by setting its inner `Option` to `None`. /// later by setting its inner `Option` to `None`.
struct LazyCell<T, E> { struct LazyCell<T, E> {
value: RefCell<Option<T>>, value: RefCell<Option<T>>,

rust/hg-core/src/revlog/node.rs

} }
pub fn short(&self) -> NodePrefix { pub fn short(&self) -> NodePrefix {
NodePrefix { NodePrefix {
nybbles_len: SHORT_PREFIX_DEFAULT_NYBBLES_LENGTH, nybbles_len: SHORT_PREFIX_DEFAULT_NYBBLES_LENGTH,
data: self.data, data: self.data,
} }
} }
pub fn pad_to_256_bits(&self) -> [u8; 32] {
AlphareUnsubmitted
Not Done

Cool method

Alphare: Cool method
let mut bits = [0; 32];
bits[..NODE_BYTES_LENGTH].copy_from_slice(&self.data);
bits
}
} }
/// The beginning of a binary revision SHA. /// The beginning of a binary revision SHA.
/// ///
/// Since it can potentially come from an hexadecimal representation with /// Since it can potentially come from an hexadecimal representation with
/// odd length, it needs to carry around whether the last 4 bits are relevant /// odd length, it needs to carry around whether the last 4 bits are relevant
/// or not. /// or not.
#[derive(Debug, PartialEq, Copy, Clone)] #[derive(Debug, PartialEq, Copy, Clone)]

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

&self, &self,
can_append: bool, can_append: bool,
) -> PyResult<PyObject> { ) -> PyResult<PyObject> {
let inner = self.inner(py).borrow(); let inner = self.inner(py).borrow();
let result = inner.pack_v2(can_append); let result = inner.pack_v2(can_append);
match result { match result {
Ok((packed, tree_metadata, append)) => { Ok((packed, tree_metadata, append)) => {
let packed = PyBytes::new(py, &packed); let packed = PyBytes::new(py, &packed);
let tree_metadata = PyBytes::new(py, &tree_metadata); let tree_metadata = PyBytes::new(py, tree_metadata.as_bytes());
let tuple = (packed, tree_metadata, append); let tuple = (packed, tree_metadata, append);
Ok(tuple.to_py_object(py).into_object()) Ok(tuple.to_py_object(py).into_object())
}, },
Err(_) => Err(PyErr::new::<exc::OSError, _>( Err(_) => Err(PyErr::new::<exc::OSError, _>(
py, py,
"Dirstate error".to_string(), "Dirstate error".to_string(),
)), )),
} }