Patchwork D12586: rhg: fix dirstate-v2 data file removal system

login
register
mail settings
Submitter phabricator
Date April 25, 2022, 4:15 p.m.
Message ID <differential-rev-PHID-DREV-3gqttddkh5ao3hpbe5ay-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/50934/
State New
Headers show

Comments

phabricator - April 25, 2022, 4:15 p.m.
Alphare created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  In D12581 <https://phab.mercurial-scm.org/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.

REPOSITORY
  rHG Mercurial

BRANCH
  stable

REVISION DETAIL
  https://phab.mercurial-scm.org/D12586

AFFECTED FILES
  rust/hg-core/src/repo.rs
  tests/test-dirstate.t

CHANGE DETAILS




To: Alphare, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/tests/test-dirstate.t b/tests/test-dirstate.t
--- a/tests/test-dirstate.t
+++ b/tests/test-dirstate.t
@@ -120,4 +120,58 @@ 
   C hgext3rd/__init__.py
 
   $ cd ..
+
+Check that the old dirstate data file is removed correctly and the new one is
+valid.
+
+  $ number_of_dirstate_data_files () {
+  >   find .hg -maxdepth 1 -name "dirstate.*" | wc -l
+  > } 
+
+  $ find_dirstate_uuid () {
+  >   find .hg -maxdepth 1 -name "dirstate.*" | sed 's#.hg/dirstate.##'
+  > }
+
+  $ dirstate_uuid_has_not_changed () {
+  >   # Python always rewrites the whole dirstate
+  >   if [ -n $1 ] || [ "$HGMODULEPOLICY" = *"rust"* ] || [ -n "$RHG_INSTALLED_AS_HG" ]; then
+  >     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
+  $ number_of_dirstate_data_files
+  1
+  $ current_uid=$(find_dirstate_uuid)
+
+Nothing changes here
+
+  $ hg st
+  $ number_of_dirstate_data_files
+  1
+  $ dirstate_uuid_has_not_changed
+
+Trigger an append with a small change
+
+  $ echo "modified" > dir2/f
+  $ hg st
+  M dir2/f
+  $ number_of_dirstate_data_files
+  1
+  $ 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
+  $ number_of_dirstate_data_files
+  1
+  $ dirstate_uuid_has_not_changed also-if-python
+  [1]
+
+  $ cd ..
 #endif
diff --git a/rust/hg-core/src/repo.rs b/rust/hg-core/src/repo.rs
--- a/rust/hg-core/src/repo.rs
+++ b/rust/hg-core/src/repo.rs
@@ -435,25 +435,32 @@ 
         // it’s unset
         let parents = self.dirstate_parents()?;
         let (packed_dirstate, old_uuid_to_remove) = 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 uuid_opt = self.dirstate_data_file_uuid.get_or_init(self)?;
+            let uuid_opt = uuid_opt.as_ref();
+            let can_append = uuid_opt.is_some();
             let (data, tree_metadata, append, old_data_size) =
                 map.pack_v2(can_append)?;
-            if !append {
-                uuid = None
-            }
-            let (uuid, old_uuid) = if let Some(uuid) = uuid {
-                let as_str = std::str::from_utf8(uuid)
-                    .map_err(|_| {
-                        HgError::corrupted("non-UTF-8 dirstate data file ID")
-                    })?
-                    .to_owned();
-                let old_uuid_to_remove = Some(as_str.to_owned());
-                (as_str, old_uuid_to_remove)
-            } else {
-                (DirstateDocket::new_uid(), None)
+
+            // Reuse the uuid, or generate a new one, keeping the old for
+            // deletion.
+            let (uuid, old_uuid) = match uuid_opt {
+                Some(uuid) => {
+                    let as_str = std::str::from_utf8(uuid)
+                        .map_err(|_| {
+                            HgError::corrupted(
+                                "non-UTF-8 dirstate data file ID",
+                            )
+                        })?
+                        .to_owned();
+                    if append {
+                        (as_str, None)
+                    } else {
+                        (DirstateDocket::new_uid(), Some(as_str))
+                    }
+                }
+                None => (DirstateDocket::new_uid(), None),
             };
+
             let data_filename = format!("dirstate.{}", uuid);
             let data_filename = self.hg_vfs().join(data_filename);
             let mut options = std::fs::OpenOptions::new();