Patchwork D10076: rhg: Check .hg/requires for absence of required features

login
register
mail settings
Submitter phabricator
Date Feb. 25, 2021, 8:32 p.m.
Message ID <differential-rev-PHID-DREV-do5s7dxuqxu4k6oielgi-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/48391/
State Superseded
Headers show

Comments

phabricator - Feb. 25, 2021, 8:32 p.m.
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  Some old repository layouts are not supported.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  rust/hg-core/src/requirements.rs
  rust/hg-core/src/utils.rs

CHANGE DETAILS




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

Patch

diff --git a/rust/hg-core/src/utils.rs b/rust/hg-core/src/utils.rs
--- a/rust/hg-core/src/utils.rs
+++ b/rust/hg-core/src/utils.rs
@@ -11,6 +11,8 @@ 
 use crate::utils::hg_path::HgPath;
 use im_rc::ordmap::DiffItem;
 use im_rc::ordmap::OrdMap;
+use std::cell::Cell;
+use std::fmt;
 use std::{io::Write, ops::Deref};
 
 pub mod files;
@@ -378,3 +380,43 @@ 
         right
     }
 }
+
+/// Join items of the iterable with the given separator, similar to Python’s
+/// `separator.join(iter)`.
+///
+/// Formatting the return value consumes the iterator.
+/// Formatting it again will produce an empty string.
+pub fn join_display(
+    iter: impl IntoIterator<Item = impl fmt::Display>,
+    separator: impl fmt::Display,
+) -> impl fmt::Display {
+    JoinDisplay {
+        iter: Cell::new(Some(iter.into_iter())),
+        separator,
+    }
+}
+
+struct JoinDisplay<I, S> {
+    iter: Cell<Option<I>>,
+    separator: S,
+}
+
+impl<I, T, S> fmt::Display for JoinDisplay<I, S>
+where
+    I: Iterator<Item = T>,
+    T: fmt::Display,
+    S: fmt::Display,
+{
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        if let Some(mut iter) = self.iter.take() {
+            if let Some(first) = iter.next() {
+                first.fmt(f)?;
+            }
+            for value in iter {
+                self.separator.fmt(f)?;
+                value.fmt(f)?;
+            }
+        }
+        Ok(())
+    }
+}
diff --git a/rust/hg-core/src/requirements.rs b/rust/hg-core/src/requirements.rs
--- a/rust/hg-core/src/requirements.rs
+++ b/rust/hg-core/src/requirements.rs
@@ -1,5 +1,6 @@ 
 use crate::errors::{HgError, HgResultExt};
 use crate::repo::{Repo, Vfs};
+use crate::utils::join_display;
 use std::collections::HashSet;
 
 fn parse(bytes: &[u8]) -> Result<HashSet<String>, HgError> {
@@ -42,34 +43,48 @@ 
 }
 
 pub(crate) fn check(repo: &Repo) -> Result<(), HgError> {
-    for feature in repo.requirements() {
-        if !SUPPORTED.contains(&feature.as_str()) {
-            // TODO: collect and all unknown features and include them in the
-            // error message?
-            return Err(HgError::UnsupportedFeature(format!(
-                "repository requires feature unknown to this Mercurial: {}",
-                feature
-            )));
-        }
+    let unknown: Vec<_> = repo
+        .requirements()
+        .iter()
+        .map(String::as_str)
+        // .filter(|feature| !ALL_SUPPORTED.contains(feature.as_str()))
+        .filter(|feature| {
+            !REQUIRED.contains(feature) && !SUPPORTED.contains(feature)
+        })
+        .collect();
+    if !unknown.is_empty() {
+        return Err(HgError::unsupported(format!(
+            "repository requires feature unknown to this Mercurial: {}",
+            join_display(&unknown, ", ")
+        )));
+    }
+    let missing: Vec<_> = REQUIRED
+        .iter()
+        .filter(|&&feature| !repo.requirements().contains(feature))
+        .collect();
+    if !missing.is_empty() {
+        return Err(HgError::unsupported(format!(
+            "repository is missing feature required by this Mercurial: {}",
+            join_display(&missing, ", ")
+        )));
     }
     Ok(())
 }
 
-// TODO: set this to actually-supported features
+/// rhg does not support repositories that are *missing* any of these features
+const REQUIRED: &[&str] = &["revlogv1", "store", "fncache", "dotencode"];
+
+/// rhg supports repository with or without these
 const SUPPORTED: &[&str] = &[
-    "dotencode",
-    "fncache",
     "generaldelta",
-    "revlogv1",
     SHARED_REQUIREMENT,
     SHARESAFE_REQUIREMENT,
     SPARSEREVLOG_REQUIREMENT,
     RELATIVE_SHARED_REQUIREMENT,
-    "store",
     // As of this writing everything rhg does is read-only.
     // When it starts writing to the repository, it’ll need to either keep the
     // persistent nodemap up to date or remove this entry:
-    "persistent-nodemap",
+    NODEMAP_REQUIREMENT,
 ];
 
 // Copied from mercurial/requirements.py: