D7798: rust-nodemap: special case for prefixes of NULL_NODE

2020-01-06 Thread gracinet (Georges Racinet) 
gracinet created this revision.
Herald added subscribers: mercurial-devel, kevincox, durin42.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  We have to behave as though NULL_NODE was stored in the node tree,
  although we don't store it.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  rust/hg-core/src/revlog/nodemap.rs

CHANGE DETAILS

diff --git a/rust/hg-core/src/revlog/nodemap.rs 
b/rust/hg-core/src/revlog/nodemap.rs
--- a/rust/hg-core/src/revlog/nodemap.rs
+++ b/rust/hg-core/src/revlog/nodemap.rs
@@ -13,8 +13,8 @@
 //! is used in a more abstract context.
 
 use super::{
-node::get_nybble, Node, NodeError, NodePrefix, NodePrefixRef, Revision,
-RevlogIndex,
+node::get_nybble, node::NULL_NODE, Node, NodeError, NodePrefix,
+NodePrefixRef, Revision, RevlogIndex, NULL_REVISION,
 };
 use std::fmt;
 use std::mem;
@@ -209,6 +209,31 @@
 })
 }
 
+/// validate that the candidate's node starts indeed with given prefix,
+/// and treat ambiguities related to `NULL_REVISION`.
+///
+/// From the data in the NodeTree, one can only conclude that some
+/// revision is the only one for a *subprefix* of the one being looked up.
+fn validate_candidate<'p>(
+idx: &impl RevlogIndex,
+prefix: NodePrefixRef<'p>,
+rev: Option,
+) -> Result, NodeMapError> {
+if prefix.is_prefix_of(&NULL_NODE) {
+// NULL_REVISION always matches a prefix made only of zeros
+// and any other *valid* result is an ambiguity
+match rev {
+None => Ok(Some(NULL_REVISION)),
+Some(r) => match has_prefix_or_none(idx, prefix, r)? {
+None => Ok(Some(NULL_REVISION)),
+_ => Err(NodeMapError::MultipleResults),
+},
+}
+} else {
+rev.map_or(Ok(None), |r| has_prefix_or_none(idx, prefix, r))
+}
+}
+
 impl NodeTree {
 /// Initiate a NodeTree from an immutable slice-like of `Block`
 ///
@@ -282,9 +307,6 @@
 }
 
 /// Main working method for `NodeTree` searches
-///
-/// This partial implementation lacks
-/// - special cases for NULL_REVISION
 fn lookup<'p>(
 &self,
 prefix: NodePrefixRef<'p>,
@@ -519,9 +541,7 @@
 idx: &impl RevlogIndex,
 prefix: NodePrefixRef<'a>,
 ) -> Result, NodeMapError> {
-self.lookup(prefix.clone()).and_then(|opt| {
-opt.map_or(Ok(None), |rev| has_prefix_or_none(idx, prefix, rev))
-})
+validate_candidate(idx, prefix.clone(), self.lookup(prefix)?)
 }
 }
 
@@ -645,8 +665,9 @@
 
 assert_eq!(nt.find_hex(&idx, "0"), Err(MultipleResults));
 assert_eq!(nt.find_hex(&idx, "01"), Ok(Some(9)));
-assert_eq!(nt.find_hex(&idx, "00"), Ok(Some(0)));
+assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults));
 assert_eq!(nt.find_hex(&idx, "00a"), Ok(Some(0)));
+assert_eq!(nt.find_hex(&idx, "000"), Ok(Some(NULL_REVISION)));
 }
 
 #[test]
@@ -665,7 +686,8 @@
 };
 assert_eq!(nt.find_hex(&idx, "10")?, Some(1));
 assert_eq!(nt.find_hex(&idx, "c")?, Some(2));
-assert_eq!(nt.find_hex(&idx, "00")?, Some(0));
+assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults));
+assert_eq!(nt.find_hex(&idx, "000")?, Some(NULL_REVISION));
 assert_eq!(nt.find_hex(&idx, "01")?, Some(9));
 Ok(())
 }



To: gracinet, #hg-reviewers
Cc: durin42, kevincox, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7798: rust-nodemap: special case for prefixes of NULL_NODE

2020-01-10 Thread gracinet (Georges Racinet) 
gracinet updated this revision to Diff 19138.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7798?vs=19047&id=19138

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7798/new/

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

AFFECTED FILES
  rust/hg-core/src/revlog/nodemap.rs

CHANGE DETAILS

diff --git a/rust/hg-core/src/revlog/nodemap.rs 
b/rust/hg-core/src/revlog/nodemap.rs
--- a/rust/hg-core/src/revlog/nodemap.rs
+++ b/rust/hg-core/src/revlog/nodemap.rs
@@ -13,8 +13,8 @@
 //! is used in a more abstract context.
 
 use super::{
-node::get_nybble, Node, NodeError, NodePrefix, NodePrefixRef, Revision,
-RevlogIndex,
+node::get_nybble, node::NULL_NODE, Node, NodeError, NodePrefix,
+NodePrefixRef, Revision, RevlogIndex, NULL_REVISION,
 };
 use std::fmt;
 use std::mem;
@@ -209,6 +209,31 @@
 })
 }
 
+/// validate that the candidate's node starts indeed with given prefix,
+/// and treat ambiguities related to `NULL_REVISION`.
+///
+/// From the data in the NodeTree, one can only conclude that some
+/// revision is the only one for a *subprefix* of the one being looked up.
+fn validate_candidate<'p>(
+idx: &impl RevlogIndex,
+prefix: NodePrefixRef<'p>,
+rev: Option,
+) -> Result, NodeMapError> {
+if prefix.is_prefix_of(&NULL_NODE) {
+// NULL_REVISION always matches a prefix made only of zeros
+// and any other *valid* result is an ambiguity
+match rev {
+None => Ok(Some(NULL_REVISION)),
+Some(r) => match has_prefix_or_none(idx, prefix, r)? {
+None => Ok(Some(NULL_REVISION)),
+_ => Err(NodeMapError::MultipleResults),
+},
+}
+} else {
+rev.map_or(Ok(None), |r| has_prefix_or_none(idx, prefix, r))
+}
+}
+
 impl NodeTree {
 /// Initiate a NodeTree from an immutable slice-like of `Block`
 ///
@@ -282,9 +307,6 @@
 }
 
 /// Main working method for `NodeTree` searches
-///
-/// This partial implementation lacks
-/// - special cases for NULL_REVISION
 fn lookup<'p>(
 &self,
 prefix: NodePrefixRef<'p>,
@@ -519,9 +541,7 @@
 idx: &impl RevlogIndex,
 prefix: NodePrefixRef<'a>,
 ) -> Result, NodeMapError> {
-self.lookup(prefix.clone()).and_then(|opt| {
-opt.map_or(Ok(None), |rev| has_prefix_or_none(idx, prefix, rev))
-})
+validate_candidate(idx, prefix.clone(), self.lookup(prefix)?)
 }
 }
 
@@ -645,8 +665,9 @@
 
 assert_eq!(nt.find_hex(&idx, "0"), Err(MultipleResults));
 assert_eq!(nt.find_hex(&idx, "01"), Ok(Some(9)));
-assert_eq!(nt.find_hex(&idx, "00"), Ok(Some(0)));
+assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults));
 assert_eq!(nt.find_hex(&idx, "00a"), Ok(Some(0)));
+assert_eq!(nt.find_hex(&idx, "000"), Ok(Some(NULL_REVISION)));
 }
 
 #[test]
@@ -665,7 +686,8 @@
 };
 assert_eq!(nt.find_hex(&idx, "10")?, Some(1));
 assert_eq!(nt.find_hex(&idx, "c")?, Some(2));
-assert_eq!(nt.find_hex(&idx, "00")?, Some(0));
+assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults));
+assert_eq!(nt.find_hex(&idx, "000")?, Some(NULL_REVISION));
 assert_eq!(nt.find_hex(&idx, "01")?, Some(9));
 Ok(())
 }



To: gracinet, #hg-reviewers
Cc: durin42, kevincox, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7798: rust-nodemap: special case for prefixes of NULL_NODE

2020-01-15 Thread gracinet (Georges Racinet) 
gracinet updated this revision to Diff 19330.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7798?vs=19138&id=19330

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7798/new/

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

AFFECTED FILES
  rust/hg-core/src/revlog/nodemap.rs

CHANGE DETAILS

diff --git a/rust/hg-core/src/revlog/nodemap.rs 
b/rust/hg-core/src/revlog/nodemap.rs
--- a/rust/hg-core/src/revlog/nodemap.rs
+++ b/rust/hg-core/src/revlog/nodemap.rs
@@ -13,8 +13,8 @@
 //! is used in a more abstract context.
 
 use super::{
-node::get_nybble, Node, NodeError, NodePrefix, NodePrefixRef, Revision,
-RevlogIndex,
+node::get_nybble, node::NULL_NODE, Node, NodeError, NodePrefix,
+NodePrefixRef, Revision, RevlogIndex, NULL_REVISION,
 };
 use std::fmt;
 use std::mem;
@@ -207,6 +207,31 @@
 })
 }
 
+/// validate that the candidate's node starts indeed with given prefix,
+/// and treat ambiguities related to `NULL_REVISION`.
+///
+/// From the data in the NodeTree, one can only conclude that some
+/// revision is the only one for a *subprefix* of the one being looked up.
+fn validate_candidate<'p>(
+idx: &impl RevlogIndex,
+prefix: NodePrefixRef<'p>,
+rev: Option,
+) -> Result, NodeMapError> {
+if prefix.is_prefix_of(&NULL_NODE) {
+// NULL_REVISION always matches a prefix made only of zeros
+// and any other *valid* result is an ambiguity
+match rev {
+None => Ok(Some(NULL_REVISION)),
+Some(r) => match has_prefix_or_none(idx, prefix, r)? {
+None => Ok(Some(NULL_REVISION)),
+_ => Err(NodeMapError::MultipleResults),
+},
+}
+} else {
+rev.map_or(Ok(None), |r| has_prefix_or_none(idx, prefix, r))
+}
+}
+
 impl NodeTree {
 /// Initiate a NodeTree from an immutable slice-like of `Block`
 ///
@@ -280,9 +305,6 @@
 }
 
 /// Main working method for `NodeTree` searches
-///
-/// This partial implementation lacks
-/// - special cases for NULL_REVISION
 fn lookup<'p>(
 &self,
 prefix: NodePrefixRef<'p>,
@@ -517,9 +539,7 @@
 idx: &impl RevlogIndex,
 prefix: NodePrefixRef<'a>,
 ) -> Result, NodeMapError> {
-self.lookup(prefix.clone()).and_then(|opt| {
-opt.map_or(Ok(None), |rev| has_prefix_or_none(idx, prefix, rev))
-})
+validate_candidate(idx, prefix.clone(), self.lookup(prefix)?)
 }
 }
 
@@ -643,8 +663,9 @@
 
 assert_eq!(nt.find_hex(&idx, "0"), Err(MultipleResults));
 assert_eq!(nt.find_hex(&idx, "01"), Ok(Some(9)));
-assert_eq!(nt.find_hex(&idx, "00"), Ok(Some(0)));
+assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults));
 assert_eq!(nt.find_hex(&idx, "00a"), Ok(Some(0)));
+assert_eq!(nt.find_hex(&idx, "000"), Ok(Some(NULL_REVISION)));
 }
 
 #[test]
@@ -663,7 +684,8 @@
 };
 assert_eq!(nt.find_hex(&idx, "10")?, Some(1));
 assert_eq!(nt.find_hex(&idx, "c")?, Some(2));
-assert_eq!(nt.find_hex(&idx, "00")?, Some(0));
+assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults));
+assert_eq!(nt.find_hex(&idx, "000")?, Some(NULL_REVISION));
 assert_eq!(nt.find_hex(&idx, "01")?, Some(9));
 Ok(())
 }



To: gracinet, #hg-reviewers
Cc: durin42, kevincox, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7798: rust-nodemap: special case for prefixes of NULL_NODE

2020-01-27 Thread gracinet (Georges Racinet) 
gracinet updated this revision to Diff 19638.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7798?vs=19330&id=19638

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7798/new/

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

AFFECTED FILES
  rust/hg-core/src/revlog/nodemap.rs

CHANGE DETAILS

diff --git a/rust/hg-core/src/revlog/nodemap.rs 
b/rust/hg-core/src/revlog/nodemap.rs
--- a/rust/hg-core/src/revlog/nodemap.rs
+++ b/rust/hg-core/src/revlog/nodemap.rs
@@ -13,7 +13,8 @@
 //! is used in a more abstract context.
 
 use super::{
-Node, NodeError, NodePrefix, NodePrefixRef, Revision, RevlogIndex,
+node::NULL_NODE, Node, NodeError, NodePrefix, NodePrefixRef, Revision,
+RevlogIndex, NULL_REVISION,
 };
 
 use std::fmt;
@@ -250,6 +251,31 @@
 })
 }
 
+/// validate that the candidate's node starts indeed with given prefix,
+/// and treat ambiguities related to `NULL_REVISION`.
+///
+/// From the data in the NodeTree, one can only conclude that some
+/// revision is the only one for a *subprefix* of the one being looked up.
+fn validate_candidate<'p>(
+idx: &impl RevlogIndex,
+prefix: NodePrefixRef<'p>,
+rev: Option,
+) -> Result, NodeMapError> {
+if prefix.is_prefix_of(&NULL_NODE) {
+// NULL_REVISION always matches a prefix made only of zeros
+// and any other *valid* result is an ambiguity
+match rev {
+None => Ok(Some(NULL_REVISION)),
+Some(r) => match has_prefix_or_none(idx, prefix, r)? {
+None => Ok(Some(NULL_REVISION)),
+_ => Err(NodeMapError::MultipleResults),
+},
+}
+} else {
+rev.map_or(Ok(None), |r| has_prefix_or_none(idx, prefix, r))
+}
+}
+
 impl NodeTree {
 /// Initiate a NodeTree from an immutable slice-like of `Block`
 ///
@@ -330,8 +356,6 @@
 }
 
 /// Main working method for `NodeTree` searches
-///
-/// This partial implementation lacks special cases for NULL_REVISION
 fn lookup<'p>(
 &self,
 prefix: NodePrefixRef<'p>,
@@ -582,9 +606,7 @@
 idx: &impl RevlogIndex,
 prefix: NodePrefixRef<'a>,
 ) -> Result, NodeMapError> {
-self.lookup(prefix.clone()).and_then(|opt| {
-opt.map_or(Ok(None), |rev| has_prefix_or_none(idx, prefix, rev))
-})
+validate_candidate(idx, prefix.clone(), self.lookup(prefix)?)
 }
 }
 
@@ -713,8 +735,9 @@
 
 assert_eq!(nt.find_hex(&idx, "0"), Err(MultipleResults));
 assert_eq!(nt.find_hex(&idx, "01"), Ok(Some(9)));
-assert_eq!(nt.find_hex(&idx, "00"), Ok(Some(0)));
+assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults));
 assert_eq!(nt.find_hex(&idx, "00a"), Ok(Some(0)));
+assert_eq!(nt.find_hex(&idx, "000"), Ok(Some(NULL_REVISION)));
 }
 
 #[test]
@@ -733,7 +756,8 @@
 };
 assert_eq!(nt.find_hex(&idx, "10")?, Some(1));
 assert_eq!(nt.find_hex(&idx, "c")?, Some(2));
-assert_eq!(nt.find_hex(&idx, "00")?, Some(0));
+assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults));
+assert_eq!(nt.find_hex(&idx, "000")?, Some(NULL_REVISION));
 assert_eq!(nt.find_hex(&idx, "01")?, Some(9));
 Ok(())
 }



To: gracinet, #hg-reviewers, kevincox
Cc: durin42, kevincox, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7798: rust-nodemap: special case for prefixes of NULL_NODE

2020-02-07 Thread marmoute (Pierre-Yves David) 
marmoute added a comment.
marmoute accepted this revision.


  looks good to me too.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7798/new/

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

To: gracinet, #hg-reviewers, kevincox, marmoute
Cc: marmoute, durin42, kevincox, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7798: rust-nodemap: special case for prefixes of NULL_NODE

2020-02-08 Thread marmoute (Pierre-Yves David) 
marmoute updated this revision to Diff 20027.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7798?vs=19638&id=20027

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7798/new/

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

AFFECTED FILES
  rust/hg-core/src/revlog/nodemap.rs

CHANGE DETAILS

diff --git a/rust/hg-core/src/revlog/nodemap.rs 
b/rust/hg-core/src/revlog/nodemap.rs
--- a/rust/hg-core/src/revlog/nodemap.rs
+++ b/rust/hg-core/src/revlog/nodemap.rs
@@ -13,7 +13,8 @@
 //! is used in a more abstract context.
 
 use super::{
-Node, NodeError, NodePrefix, NodePrefixRef, Revision, RevlogIndex,
+node::NULL_NODE, Node, NodeError, NodePrefix, NodePrefixRef, Revision,
+RevlogIndex, NULL_REVISION,
 };
 
 use std::fmt;
@@ -250,6 +251,31 @@
 })
 }
 
+/// validate that the candidate's node starts indeed with given prefix,
+/// and treat ambiguities related to `NULL_REVISION`.
+///
+/// From the data in the NodeTree, one can only conclude that some
+/// revision is the only one for a *subprefix* of the one being looked up.
+fn validate_candidate<'p>(
+idx: &impl RevlogIndex,
+prefix: NodePrefixRef<'p>,
+rev: Option,
+) -> Result, NodeMapError> {
+if prefix.is_prefix_of(&NULL_NODE) {
+// NULL_REVISION always matches a prefix made only of zeros
+// and any other *valid* result is an ambiguity
+match rev {
+None => Ok(Some(NULL_REVISION)),
+Some(r) => match has_prefix_or_none(idx, prefix, r)? {
+None => Ok(Some(NULL_REVISION)),
+_ => Err(NodeMapError::MultipleResults),
+},
+}
+} else {
+rev.map_or(Ok(None), |r| has_prefix_or_none(idx, prefix, r))
+}
+}
+
 impl NodeTree {
 /// Initiate a NodeTree from an immutable slice-like of `Block`
 ///
@@ -330,8 +356,6 @@
 }
 
 /// Main working method for `NodeTree` searches
-///
-/// This partial implementation lacks special cases for NULL_REVISION
 fn lookup<'p>(
 &self,
 prefix: NodePrefixRef<'p>,
@@ -582,9 +606,7 @@
 idx: &impl RevlogIndex,
 prefix: NodePrefixRef<'a>,
 ) -> Result, NodeMapError> {
-self.lookup(prefix.clone()).and_then(|opt| {
-opt.map_or(Ok(None), |rev| has_prefix_or_none(idx, prefix, rev))
-})
+validate_candidate(idx, prefix.clone(), self.lookup(prefix)?)
 }
 }
 
@@ -713,8 +735,9 @@
 
 assert_eq!(nt.find_hex(&idx, "0"), Err(MultipleResults));
 assert_eq!(nt.find_hex(&idx, "01"), Ok(Some(9)));
-assert_eq!(nt.find_hex(&idx, "00"), Ok(Some(0)));
+assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults));
 assert_eq!(nt.find_hex(&idx, "00a"), Ok(Some(0)));
+assert_eq!(nt.find_hex(&idx, "000"), Ok(Some(NULL_REVISION)));
 }
 
 #[test]
@@ -733,7 +756,8 @@
 };
 assert_eq!(nt.find_hex(&idx, "10")?, Some(1));
 assert_eq!(nt.find_hex(&idx, "c")?, Some(2));
-assert_eq!(nt.find_hex(&idx, "00")?, Some(0));
+assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults));
+assert_eq!(nt.find_hex(&idx, "000")?, Some(NULL_REVISION));
 assert_eq!(nt.find_hex(&idx, "01")?, Some(9));
 Ok(())
 }



To: gracinet, #hg-reviewers, kevincox, marmoute
Cc: marmoute, durin42, kevincox, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7798: rust-nodemap: special case for prefixes of NULL_NODE

2020-02-14 Thread Raphaël Gomès 
Alphare updated this revision to Diff 20217.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7798?vs=20027&id=20217

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7798/new/

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

AFFECTED FILES
  rust/hg-core/src/revlog/nodemap.rs

CHANGE DETAILS

diff --git a/rust/hg-core/src/revlog/nodemap.rs 
b/rust/hg-core/src/revlog/nodemap.rs
--- a/rust/hg-core/src/revlog/nodemap.rs
+++ b/rust/hg-core/src/revlog/nodemap.rs
@@ -13,7 +13,8 @@
 //! is used in a more abstract context.
 
 use super::{
-Node, NodeError, NodePrefix, NodePrefixRef, Revision, RevlogIndex,
+node::NULL_NODE, Node, NodeError, NodePrefix, NodePrefixRef, Revision,
+RevlogIndex, NULL_REVISION,
 };
 
 use std::fmt;
@@ -250,6 +251,31 @@
 })
 }
 
+/// validate that the candidate's node starts indeed with given prefix,
+/// and treat ambiguities related to `NULL_REVISION`.
+///
+/// From the data in the NodeTree, one can only conclude that some
+/// revision is the only one for a *subprefix* of the one being looked up.
+fn validate_candidate<'p>(
+idx: &impl RevlogIndex,
+prefix: NodePrefixRef<'p>,
+rev: Option,
+) -> Result, NodeMapError> {
+if prefix.is_prefix_of(&NULL_NODE) {
+// NULL_REVISION always matches a prefix made only of zeros
+// and any other *valid* result is an ambiguity
+match rev {
+None => Ok(Some(NULL_REVISION)),
+Some(r) => match has_prefix_or_none(idx, prefix, r)? {
+None => Ok(Some(NULL_REVISION)),
+_ => Err(NodeMapError::MultipleResults),
+},
+}
+} else {
+rev.map_or(Ok(None), |r| has_prefix_or_none(idx, prefix, r))
+}
+}
+
 impl NodeTree {
 /// Initiate a NodeTree from an immutable slice-like of `Block`
 ///
@@ -339,8 +365,6 @@
 }
 
 /// Main working method for `NodeTree` searches
-///
-/// This partial implementation lacks special cases for NULL_REVISION
 fn lookup<'p>(
 &self,
 prefix: NodePrefixRef<'p>,
@@ -591,9 +615,7 @@
 idx: &impl RevlogIndex,
 prefix: NodePrefixRef<'a>,
 ) -> Result, NodeMapError> {
-self.lookup(prefix.clone()).and_then(|opt| {
-opt.map_or(Ok(None), |rev| has_prefix_or_none(idx, prefix, rev))
-})
+validate_candidate(idx, prefix.clone(), self.lookup(prefix)?)
 }
 }
 
@@ -723,8 +745,9 @@
 
 assert_eq!(nt.find_hex(&idx, "0"), Err(MultipleResults));
 assert_eq!(nt.find_hex(&idx, "01"), Ok(Some(9)));
-assert_eq!(nt.find_hex(&idx, "00"), Ok(Some(0)));
+assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults));
 assert_eq!(nt.find_hex(&idx, "00a"), Ok(Some(0)));
+assert_eq!(nt.find_hex(&idx, "000"), Ok(Some(NULL_REVISION)));
 }
 
 #[test]
@@ -743,7 +766,8 @@
 };
 assert_eq!(nt.find_hex(&idx, "10")?, Some(1));
 assert_eq!(nt.find_hex(&idx, "c")?, Some(2));
-assert_eq!(nt.find_hex(&idx, "00")?, Some(0));
+assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults));
+assert_eq!(nt.find_hex(&idx, "000")?, Some(NULL_REVISION));
 assert_eq!(nt.find_hex(&idx, "01")?, Some(9));
 Ok(())
 }



To: gracinet, #hg-reviewers, kevincox, marmoute
Cc: marmoute, durin42, kevincox, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7798: rust-nodemap: special case for prefixes of NULL_NODE

2020-02-15 Thread Raphaël Gomès 
Alphare updated this revision to Diff 20236.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7798?vs=20217&id=20236

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7798/new/

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

AFFECTED FILES
  rust/hg-core/src/revlog/nodemap.rs

CHANGE DETAILS

diff --git a/rust/hg-core/src/revlog/nodemap.rs 
b/rust/hg-core/src/revlog/nodemap.rs
--- a/rust/hg-core/src/revlog/nodemap.rs
+++ b/rust/hg-core/src/revlog/nodemap.rs
@@ -13,7 +13,8 @@
 //! is used in a more abstract context.
 
 use super::{
-Node, NodeError, NodePrefix, NodePrefixRef, Revision, RevlogIndex,
+node::NULL_NODE, Node, NodeError, NodePrefix, NodePrefixRef, Revision,
+RevlogIndex, NULL_REVISION,
 };
 
 use std::fmt;
@@ -268,6 +269,31 @@
 })
 }
 
+/// validate that the candidate's node starts indeed with given prefix,
+/// and treat ambiguities related to `NULL_REVISION`.
+///
+/// From the data in the NodeTree, one can only conclude that some
+/// revision is the only one for a *subprefix* of the one being looked up.
+fn validate_candidate(
+idx: &impl RevlogIndex,
+prefix: NodePrefixRef,
+rev: Option,
+) -> Result, NodeMapError> {
+if prefix.is_prefix_of(&NULL_NODE) {
+// NULL_REVISION always matches a prefix made only of zeros
+// and any other *valid* result is an ambiguity
+match rev {
+None => Ok(Some(NULL_REVISION)),
+Some(r) => match has_prefix_or_none(idx, prefix, r)? {
+None => Ok(Some(NULL_REVISION)),
+_ => Err(NodeMapError::MultipleResults),
+},
+}
+} else {
+rev.map_or(Ok(None), |r| has_prefix_or_none(idx, prefix, r))
+}
+}
+
 impl NodeTree {
 /// Initiate a NodeTree from an immutable slice-like of `Block`
 ///
@@ -358,8 +384,6 @@
 }
 
 /// Main working method for `NodeTree` searches
-///
-/// This partial implementation lacks special cases for NULL_REVISION
 fn lookup<'p>(
 &self,
 prefix: NodePrefixRef<'p>,
@@ -610,9 +634,7 @@
 idx: &impl RevlogIndex,
 prefix: NodePrefixRef<'a>,
 ) -> Result, NodeMapError> {
-self.lookup(prefix.clone()).and_then(|opt| {
-opt.map_or(Ok(None), |rev| has_prefix_or_none(idx, prefix, rev))
-})
+validate_candidate(idx, prefix.clone(), self.lookup(prefix)?)
 }
 }
 
@@ -745,8 +767,9 @@
 
 assert_eq!(nt.find_hex(&idx, "0"), Err(MultipleResults));
 assert_eq!(nt.find_hex(&idx, "01"), Ok(Some(9)));
-assert_eq!(nt.find_hex(&idx, "00"), Ok(Some(0)));
+assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults));
 assert_eq!(nt.find_hex(&idx, "00a"), Ok(Some(0)));
+assert_eq!(nt.find_hex(&idx, "000"), Ok(Some(NULL_REVISION)));
 }
 
 #[test]
@@ -765,7 +788,8 @@
 };
 assert_eq!(nt.find_hex(&idx, "10")?, Some(1));
 assert_eq!(nt.find_hex(&idx, "c")?, Some(2));
-assert_eq!(nt.find_hex(&idx, "00")?, Some(0));
+assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults));
+assert_eq!(nt.find_hex(&idx, "000")?, Some(NULL_REVISION));
 assert_eq!(nt.find_hex(&idx, "01")?, Some(9));
 Ok(())
 }



To: gracinet, #hg-reviewers, kevincox, marmoute
Cc: marmoute, durin42, kevincox, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7798: rust-nodemap: special case for prefixes of NULL_NODE

2020-02-24 Thread Raphaël Gomès 
Alphare updated this revision to Diff 20282.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7798?vs=20236&id=20282

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7798/new/

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

AFFECTED FILES
  rust/hg-core/src/revlog/nodemap.rs

CHANGE DETAILS

diff --git a/rust/hg-core/src/revlog/nodemap.rs 
b/rust/hg-core/src/revlog/nodemap.rs
--- a/rust/hg-core/src/revlog/nodemap.rs
+++ b/rust/hg-core/src/revlog/nodemap.rs
@@ -13,7 +13,8 @@
 //! is used in a more abstract context.
 
 use super::{
-Node, NodeError, NodePrefix, NodePrefixRef, Revision, RevlogIndex,
+node::NULL_NODE, Node, NodeError, NodePrefix, NodePrefixRef, Revision,
+RevlogIndex, NULL_REVISION,
 };
 
 use std::fmt;
@@ -270,6 +271,31 @@
 })
 }
 
+/// validate that the candidate's node starts indeed with given prefix,
+/// and treat ambiguities related to `NULL_REVISION`.
+///
+/// From the data in the NodeTree, one can only conclude that some
+/// revision is the only one for a *subprefix* of the one being looked up.
+fn validate_candidate(
+idx: &impl RevlogIndex,
+prefix: NodePrefixRef,
+rev: Option,
+) -> Result, NodeMapError> {
+if prefix.is_prefix_of(&NULL_NODE) {
+// NULL_REVISION always matches a prefix made only of zeros
+// and any other *valid* result is an ambiguity
+match rev {
+None => Ok(Some(NULL_REVISION)),
+Some(r) => match has_prefix_or_none(idx, prefix, r)? {
+None => Ok(Some(NULL_REVISION)),
+_ => Err(NodeMapError::MultipleResults),
+},
+}
+} else {
+rev.map_or(Ok(None), |r| has_prefix_or_none(idx, prefix, r))
+}
+}
+
 impl NodeTree {
 /// Initiate a NodeTree from an immutable slice-like of `Block`
 ///
@@ -361,8 +387,6 @@
 }
 
 /// Main working method for `NodeTree` searches
-///
-/// This partial implementation lacks special cases for NULL_REVISION
 fn lookup<'p>(
 &self,
 prefix: NodePrefixRef<'p>,
@@ -613,9 +637,7 @@
 idx: &impl RevlogIndex,
 prefix: NodePrefixRef<'a>,
 ) -> Result, NodeMapError> {
-self.lookup(prefix.clone()).and_then(|opt| {
-opt.map_or(Ok(None), |rev| has_prefix_or_none(idx, prefix, rev))
-})
+validate_candidate(idx, prefix.clone(), self.lookup(prefix)?)
 }
 }
 
@@ -748,8 +770,9 @@
 
 assert_eq!(nt.find_hex(&idx, "0"), Err(MultipleResults));
 assert_eq!(nt.find_hex(&idx, "01"), Ok(Some(9)));
-assert_eq!(nt.find_hex(&idx, "00"), Ok(Some(0)));
+assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults));
 assert_eq!(nt.find_hex(&idx, "00a"), Ok(Some(0)));
+assert_eq!(nt.find_hex(&idx, "000"), Ok(Some(NULL_REVISION)));
 }
 
 #[test]
@@ -768,7 +791,8 @@
 };
 assert_eq!(nt.find_hex(&idx, "10")?, Some(1));
 assert_eq!(nt.find_hex(&idx, "c")?, Some(2));
-assert_eq!(nt.find_hex(&idx, "00")?, Some(0));
+assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults));
+assert_eq!(nt.find_hex(&idx, "000")?, Some(NULL_REVISION));
 assert_eq!(nt.find_hex(&idx, "01")?, Some(9));
 Ok(())
 }



To: gracinet, #hg-reviewers, kevincox, marmoute
Cc: marmoute, durin42, kevincox, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7798: rust-nodemap: special case for prefixes of NULL_NODE

2020-02-26 Thread gracinet (Georges Racinet) 
Closed by commit rHG895934342631: rust-nodemap: special case for prefixes of 
NULL_NODE (authored by gracinet).
This revision was automatically updated to reflect the committed changes.
This revision was not accepted when it landed; it landed in state "Needs 
Review".

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7798?vs=20282&id=20312

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7798/new/

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

AFFECTED FILES
  rust/hg-core/src/revlog/nodemap.rs

CHANGE DETAILS

diff --git a/rust/hg-core/src/revlog/nodemap.rs 
b/rust/hg-core/src/revlog/nodemap.rs
--- a/rust/hg-core/src/revlog/nodemap.rs
+++ b/rust/hg-core/src/revlog/nodemap.rs
@@ -13,7 +13,8 @@
 //! is used in a more abstract context.
 
 use super::{
-Node, NodeError, NodePrefix, NodePrefixRef, Revision, RevlogIndex,
+node::NULL_NODE, Node, NodeError, NodePrefix, NodePrefixRef, Revision,
+RevlogIndex, NULL_REVISION,
 };
 
 use std::fmt;
@@ -270,6 +271,31 @@
 })
 }
 
+/// validate that the candidate's node starts indeed with given prefix,
+/// and treat ambiguities related to `NULL_REVISION`.
+///
+/// From the data in the NodeTree, one can only conclude that some
+/// revision is the only one for a *subprefix* of the one being looked up.
+fn validate_candidate(
+idx: &impl RevlogIndex,
+prefix: NodePrefixRef,
+rev: Option,
+) -> Result, NodeMapError> {
+if prefix.is_prefix_of(&NULL_NODE) {
+// NULL_REVISION always matches a prefix made only of zeros
+// and any other *valid* result is an ambiguity
+match rev {
+None => Ok(Some(NULL_REVISION)),
+Some(r) => match has_prefix_or_none(idx, prefix, r)? {
+None => Ok(Some(NULL_REVISION)),
+_ => Err(NodeMapError::MultipleResults),
+},
+}
+} else {
+rev.map_or(Ok(None), |r| has_prefix_or_none(idx, prefix, r))
+}
+}
+
 impl NodeTree {
 /// Initiate a NodeTree from an immutable slice-like of `Block`
 ///
@@ -361,8 +387,6 @@
 }
 
 /// Main working method for `NodeTree` searches
-///
-/// This partial implementation lacks special cases for NULL_REVISION
 fn lookup<'p>(
 &self,
 prefix: NodePrefixRef<'p>,
@@ -613,9 +637,7 @@
 idx: &impl RevlogIndex,
 prefix: NodePrefixRef<'a>,
 ) -> Result, NodeMapError> {
-self.lookup(prefix.clone()).and_then(|opt| {
-opt.map_or(Ok(None), |rev| has_prefix_or_none(idx, prefix, rev))
-})
+validate_candidate(idx, prefix.clone(), self.lookup(prefix)?)
 }
 }
 
@@ -748,8 +770,9 @@
 
 assert_eq!(nt.find_hex(&idx, "0"), Err(MultipleResults));
 assert_eq!(nt.find_hex(&idx, "01"), Ok(Some(9)));
-assert_eq!(nt.find_hex(&idx, "00"), Ok(Some(0)));
+assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults));
 assert_eq!(nt.find_hex(&idx, "00a"), Ok(Some(0)));
+assert_eq!(nt.find_hex(&idx, "000"), Ok(Some(NULL_REVISION)));
 }
 
 #[test]
@@ -768,7 +791,8 @@
 };
 assert_eq!(nt.find_hex(&idx, "10")?, Some(1));
 assert_eq!(nt.find_hex(&idx, "c")?, Some(2));
-assert_eq!(nt.find_hex(&idx, "00")?, Some(0));
+assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults));
+assert_eq!(nt.find_hex(&idx, "000")?, Some(NULL_REVISION));
 assert_eq!(nt.find_hex(&idx, "01")?, Some(9));
 Ok(())
 }



To: gracinet, #hg-reviewers, kevincox, marmoute
Cc: marmoute, durin42, kevincox, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel