From e4925487726fc71122f0ff474e35dbb4746e6397 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 10 Apr 2023 06:40:18 -0700 Subject: [PATCH] revset: bump generation numbers in API to 64 bits A chain of 4 billion commits is a lot, but it's not out of the question, so let's support it. The current default index will not be able to handle that many commits, so I let that still use 32-bit integers. --- lib/src/default_revset_engine.rs | 16 ++++++++++-- lib/src/revset.rs | 44 +++++++++++++++++--------------- 2 files changed, 37 insertions(+), 23 deletions(-) diff --git a/lib/src/default_revset_engine.rs b/lib/src/default_revset_engine.rs index 74663fd9d..f658a73f4 100644 --- a/lib/src/default_revset_engine.rs +++ b/lib/src/default_revset_engine.rs @@ -16,6 +16,7 @@ use std::cmp::{Ordering, Reverse}; use std::collections::{BinaryHeap, HashSet}; use std::fmt; use std::iter::Peekable; +use std::ops::Range; use std::sync::Arc; use itertools::Itertools; @@ -565,6 +566,17 @@ struct EvaluationContext<'index> { composite_index: CompositeIndex<'index>, } +fn to_u32_generation_range(range: &Range) -> Result, RevsetEvaluationError> { + let start = range.start.try_into().map_err(|_| { + RevsetEvaluationError::Other(format!( + "Lower bound of generation ({}) is too large", + range.start + )) + })?; + let end = range.end.try_into().unwrap_or(u32::MAX); + Ok(start..end) +} + impl<'index> EvaluationContext<'index> { fn evaluate( &self, @@ -600,7 +612,7 @@ impl<'index> EvaluationContext<'index> { if generation == &GENERATION_RANGE_FULL { Ok(Box::new(RevWalkRevset { walk })) } else { - let walk = walk.filter_by_generation(generation.clone()); + let walk = walk.filter_by_generation(to_u32_generation_range(generation)?); Ok(Box::new(RevWalkRevset { walk })) } } @@ -617,7 +629,7 @@ impl<'index> EvaluationContext<'index> { if generation == &GENERATION_RANGE_FULL { Ok(Box::new(RevWalkRevset { walk })) } else { - let walk = walk.filter_by_generation(generation.clone()); + let walk = walk.filter_by_generation(to_u32_generation_range(generation)?); Ok(Box::new(RevWalkRevset { walk })) } } diff --git a/lib/src/revset.rs b/lib/src/revset.rs index 3cd10503c..7c0feec9e 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -54,6 +54,8 @@ pub enum RevsetResolutionError { pub enum RevsetEvaluationError { #[error("Unexpected error from store: {0}")] StoreError(#[source] BackendError), + #[error("{0}")] + Other(String), } #[derive(Parser)] @@ -189,9 +191,9 @@ impl error::Error for RevsetParseError { } } -// assumes index has less than u32::MAX entries. -pub const GENERATION_RANGE_FULL: Range = 0..u32::MAX; -pub const GENERATION_RANGE_EMPTY: Range = 0..0; +// assumes index has less than u64::MAX entries. +pub const GENERATION_RANGE_FULL: Range = 0..u64::MAX; +pub const GENERATION_RANGE_EMPTY: Range = 0..0; /// Symbol or function to be resolved to `CommitId`s. #[derive(Clone, Debug, Eq, PartialEq)] @@ -233,13 +235,13 @@ pub enum RevsetExpression { Children(Rc), Ancestors { heads: Rc, - generation: Range, + generation: Range, }, // Commits that are ancestors of "heads" but not ancestors of "roots" Range { roots: Rc, heads: Rc, - generation: Range, + generation: Range, }, // Commits that are descendants of "roots" and ancestors of "heads" DagRange { @@ -466,13 +468,13 @@ pub enum ResolvedExpression { }, Ancestors { heads: Box, - generation: Range, + generation: Range, }, /// Commits that are ancestors of `heads` but not ancestors of `roots`. Range { roots: Box, heads: Box, - generation: Range, + generation: Range, }, /// Commits that are descendants of `roots` and ancestors of `heads`. DagRange { @@ -1524,8 +1526,8 @@ fn fold_ancestors(expression: &Rc) -> TransformedExpression { let generation = if generation1.is_empty() || generation2.is_empty() { GENERATION_RANGE_EMPTY } else { - let start = u32::saturating_add(generation1.start, generation2.start); - let end = u32::saturating_add(generation1.end, generation2.end - 1); + let start = u64::saturating_add(generation1.start, generation2.start); + let end = u64::saturating_add(generation1.end, generation2.end - 1); start..end }; Some(Rc::new(RevsetExpression::Ancestors { @@ -2954,7 +2956,7 @@ mod tests { "foo", ), ), - generation: 0..4294967295, + generation: 0..18446744073709551615, } "###); insta::assert_debug_snapshot!(optimize(parse("~:foo & :bar").unwrap()), @r###" @@ -2969,7 +2971,7 @@ mod tests { "bar", ), ), - generation: 0..4294967295, + generation: 0..18446744073709551615, } "###); insta::assert_debug_snapshot!(optimize(parse("foo..").unwrap()), @r###" @@ -2982,7 +2984,7 @@ mod tests { heads: CommitRef( VisibleHeads, ), - generation: 0..4294967295, + generation: 0..18446744073709551615, } "###); insta::assert_debug_snapshot!(optimize(parse("foo..bar").unwrap()), @r###" @@ -2997,7 +2999,7 @@ mod tests { "bar", ), ), - generation: 0..4294967295, + generation: 0..18446744073709551615, } "###); @@ -3750,7 +3752,7 @@ mod tests { "foo", ), ), - generation: 3..4294967295, + generation: 3..18446744073709551615, } "###); insta::assert_debug_snapshot!(optimize(parse("(:foo)---").unwrap()), @r###" @@ -3760,7 +3762,7 @@ mod tests { "foo", ), ), - generation: 3..4294967295, + generation: 3..18446744073709551615, } "###); @@ -3791,7 +3793,7 @@ mod tests { "bar", ), ), - generation: 2..4294967295, + generation: 2..18446744073709551615, } "###); // roots can also be folded, but range expression cannot be reconstructed. @@ -3804,7 +3806,7 @@ mod tests { "bar", ), ), - generation: 3..4294967295, + generation: 3..18446744073709551615, }, Ancestors { heads: CommitRef( @@ -3812,7 +3814,7 @@ mod tests { "foo", ), ), - generation: 2..4294967295, + generation: 2..18446744073709551615, }, ) "###); @@ -3832,7 +3834,7 @@ mod tests { "bar", ), ), - generation: 0..4294967295, + generation: 0..18446744073709551615, }, generation: 2..3, } @@ -3855,9 +3857,9 @@ mod tests { "baz", ), ), - generation: 0..4294967295, + generation: 0..18446744073709551615, }, - generation: 0..4294967295, + generation: 0..18446744073709551615, } "###);