This is an archive of the discontinued Mercurial Phabricator instance.

Differential D12586

rhg: fix dirstate-v2 data file removal system
ClosedPublic

Authored by Alphare on Apr 25 2022, 12:15 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHGf2ef6a4f918f: rhg: fix dirstate-v2 data file removal system
Summary

In D12581 I introduced logic to remove the previous dirstate-v2 data file
after a new one is created (and its corresponding docket), but the logic was
flawed. I fixed it and made it simpler to understand by gather all logic in
a single expression.

Diff Detail

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

Event Timeline

Alphare created this revision.Apr 25 2022, 12:15 PM
Alphare planned changes to this revision.Apr 25 2022, 12:51 PM

There are some minor issues with this patch, I'll update it soon

baymax updated this revision to Diff 33314.Apr 26 2022, 10:30 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)
⚠ This patch is intended for stable ⚠

Alphare added a commit: rHGf2ef6a4f918f: rhg: fix dirstate-v2 data file removal system.Apr 27 2022, 9:50 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHGf2ef6a4f918f: rhg: fix dirstate-v2 data file removal system (authored by Alphare). · Explain Why
This revision was automatically updated to reflect the committed changes.
baymax updated this revision to Diff 33319.Apr 27 2022, 10:33 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)
⚠ This patch is intended for stable ⚠

av6 added a subscriber: av6.Wed, May 4, 7:30 AM
av6 added inline comments.
tests/test-dirstate.t
137

I know this already landed, but I don't think bash or dash allow you to match variables with globs like that. In my limited testing this was always echoing no:

P=rust+c
if [ "$P" = *"rust"* ]; then echo yes; else echo no; fi

Revision Contents
Changeset List

PathPackages
Mrust/hg-core/src/repo.rs (39 lines)
Mtests/test-dirstate.t (54 lines)

Diff 33319

rust/hg-core/src/repo.rs

/// TODO: have a `WritableRepo` type only accessible while holding the /// TODO: have a `WritableRepo` type only accessible while holding the
/// lock? /// lock?
pub fn write_dirstate(&self) -> Result<(), DirstateError> { pub fn write_dirstate(&self) -> Result<(), DirstateError> {
let map = self.dirstate_map()?; let map = self.dirstate_map()?;
// TODO: Maintain a `DirstateMap::dirty` flag, and return early here if // TODO: Maintain a `DirstateMap::dirty` flag, and return early here if
// it’s unset // it’s unset
let parents = self.dirstate_parents()?; let parents = self.dirstate_parents()?;
let (packed_dirstate, old_uuid_to_remove) = if self.has_dirstate_v2() { let (packed_dirstate, old_uuid_to_remove) = if self.has_dirstate_v2() {
let uuid = self.dirstate_data_file_uuid.get_or_init(self)?; let uuid_opt = self.dirstate_data_file_uuid.get_or_init(self)?;
let mut uuid = uuid.as_ref(); let uuid_opt = uuid_opt.as_ref();
let can_append = uuid.is_some(); let can_append = uuid_opt.is_some();
let (data, tree_metadata, append, old_data_size) = let (data, tree_metadata, append, old_data_size) =
map.pack_v2(can_append)?; map.pack_v2(can_append)?;
if !append {
uuid = None // Reuse the uuid, or generate a new one, keeping the old for
} // deletion.
let (uuid, old_uuid) = if let Some(uuid) = uuid { let (uuid, old_uuid) = match uuid_opt {
Some(uuid) => {
let as_str = std::str::from_utf8(uuid) let as_str = std::str::from_utf8(uuid)
.map_err(|_| { .map_err(|_| {
HgError::corrupted("non-UTF-8 dirstate data file ID") HgError::corrupted(
"non-UTF-8 dirstate data file ID",
)
})? })?
.to_owned(); .to_owned();
let old_uuid_to_remove = Some(as_str.to_owned()); if append {
(as_str, old_uuid_to_remove) (as_str, None)
} else { } else {
(DirstateDocket::new_uid(), None) (DirstateDocket::new_uid(), Some(as_str))
}
}
None => (DirstateDocket::new_uid(), None),
}; };
let data_filename = format!("dirstate.{}", uuid); let data_filename = format!("dirstate.{}", uuid);
let data_filename = self.hg_vfs().join(data_filename); let data_filename = self.hg_vfs().join(data_filename);
let mut options = std::fs::OpenOptions::new(); let mut options = std::fs::OpenOptions::new();
if append { if append {
options.append(true); options.append(true);
} else { } else {
options.write(true).create_new(true); options.write(true).create_new(true);
} }

tests/test-dirstate.t

$ touch hgext3rd/__init__.py hgext/zeroconf.py $ touch hgext3rd/__init__.py hgext/zeroconf.py
$ hg commit -Aqm0 $ hg commit -Aqm0
$ hg st -c $ hg st -c
C hgext/zeroconf.py C hgext/zeroconf.py
C hgext3rd/__init__.py C hgext3rd/__init__.py
$ cd .. $ cd ..
Check that the old dirstate data file is removed correctly and the new one is
valid.
$ dirstate_data_files () {
> find .hg -maxdepth 1 -name "dirstate.*"
> }
$ find_dirstate_uuid () {
> dirstate_data_files | sed 's#.hg/dirstate.##'
> }
$ dirstate_uuid_has_not_changed () {
> # Pure Python always rewrites the whole dirstate
> if [ $# -eq 1 ] || [ "$HGMODULEPOLICY" = *"rust"* ] || [ -n "$RHG_INSTALLED_AS_HG" ]; then
av6Unsubmitted
Not Done

I know this already landed, but I don't think bash or dash allow you to match variables with globs like that. In my limited testing this was always echoing no:

P=rust+c
if [ "$P" = *"rust"* ]; then echo yes; else echo no; fi
av6: I know this already landed, but I don't think bash or dash allow you to match variables with…
> test $current_uid = $(find_dirstate_uuid)
> fi
> }
$ cd ..
$ hg init append-mostly
$ cd append-mostly
$ mkdir dir dir2
$ touch dir/a dir/b dir/c dir/d dir/e dir2/f
$ hg commit -Aqm initial
$ hg st
$ dirstate_data_files | wc -l
*1 (re)
$ current_uid=$(find_dirstate_uuid)
Nothing changes here
$ hg st
$ dirstate_data_files | wc -l
*1 (re)
$ dirstate_uuid_has_not_changed
Trigger an append with a small change
$ echo "modified" > dir2/f
$ hg st
M dir2/f
$ dirstate_data_files | wc -l
*1 (re)
$ dirstate_uuid_has_not_changed
Delete most of the dirstate to trigger a non-append
$ hg rm dir/a dir/b dir/c dir/d
$ dirstate_data_files | wc -l
*1 (re)
$ dirstate_uuid_has_not_changed also-if-python
[1]
$ cd ..
#endif #endif