diff --git a/crates/spk-proto/schema/spk.fbs b/crates/spk-proto/schema/spk.fbs index 847834433..24f95bf20 100644 --- a/crates/spk-proto/schema/spk.fbs +++ b/crates/spk-proto/schema/spk.fbs @@ -160,6 +160,44 @@ table PkgRequestOptionValue { is_complete: bool; } +enum VersionRangeOperator: uint8 { + // From: VersionRange enum in spk + Compat = 0, + DoubleEquals, + DoubleNotEquals, + Equals, + // Note, Filter (or VersionFilter) is not actually supported in an + // index. This operator is flattened out into other operators before + // the index is generated. It has an entry here for completeness. + Filter, + GreaterThan, + GreaterThanOrEqualTo, + LessThan, + LessThanOrEqualTo, + LowestSpecified, + NotEquals, + Semver, + Wildcard, +} + + +// A single rule in a VersionFilter +table VersionFilterRule { + filter_op: VersionRangeOperator; + // Used for: Compat, DoubleEqualsVersion, DoubleNotEqualsVersion, + // EqualsVersion, GreaterThanOrEqualToRange, GreaterThanRange, + // LessThanOrEqualToRange, LessThanRange, LowestSpecifiedRange, + // NotEqualsVersion, SemverRange WildCardRange + version: Version; + // Used for: Compat + required: LoneCompatRule; + // WildCardRange - ::MAX means *, aka None, and <::MAX means Some(number) + parts: [uint32]; + // Some VersionRange(objects) have a 'specified' field. This is + // not stored here because it can be regenerated in the + // constructor when extracting the object from the index. +} + // A package request with options table PkgRequestWithOptions { // from pkg_request: PkgRequest. In spk, a PkgRequest contains a @@ -174,7 +212,7 @@ table PkgRequestWithOptions { // string, e.g. '>=1.2.3,<4,5,6' and parsed when needed. // TODO: this will likely change in a future version because // processing this is showing up in current profiling. - version_filter: string; + version_filter: [VersionFilterRule]; build: Build; prerelease_policy: PreReleasePolicy; inclusion_policy: InclusionPolicy; diff --git a/crates/spk-schema/crates/foundation/src/version/mod.rs b/crates/spk-schema/crates/foundation/src/version/mod.rs index 43e304497..221102fe2 100644 --- a/crates/spk-schema/crates/foundation/src/version/mod.rs +++ b/crates/spk-schema/crates/foundation/src/version/mod.rs @@ -392,12 +392,6 @@ impl Version { } } - /// Make a new Version from a string without checking it. - /// - pub fn new_from_version_str(version_str: &str) -> Result { - Version::try_from(version_str) - } - /// The major version number (first component) pub fn major(&self) -> u32 { self.parts.first().copied().unwrap_or_default() diff --git a/crates/spk-schema/crates/foundation/src/version_range/mod.rs b/crates/spk-schema/crates/foundation/src/version_range/mod.rs index 3998845b8..2addf3328 100644 --- a/crates/spk-schema/crates/foundation/src/version_range/mod.rs +++ b/crates/spk-schema/crates/foundation/src/version_range/mod.rs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 // https://github.com/spkenv/spk +use std::borrow::Cow; use std::collections::BTreeSet; use std::convert::{TryFrom, TryInto}; use std::fmt::{Display, Write}; @@ -393,6 +394,10 @@ impl SemverRange { minimum: minimum.try_into()?, })) } + + pub fn version(&self) -> Cow<'_, Version> { + Cow::Borrowed(&self.minimum) + } } impl Ranged for SemverRange { @@ -478,6 +483,10 @@ impl WildcardRange { } Ok(VersionRange::Wildcard(range)) } + + pub fn parts(&self) -> Cow<'_, Vec>> { + Cow::Borrowed(&self.parts) + } } impl Ranged for WildcardRange { @@ -585,6 +594,10 @@ impl LowestSpecifiedRange { base, } } + + pub fn version(&self) -> Cow<'_, Version> { + Cow::Borrowed(&self.base) + } } impl TryFrom for LowestSpecifiedRange { @@ -647,6 +660,10 @@ impl GreaterThanRange { bound: boundary.try_into()?, })) } + + pub fn version(&self) -> Cow<'_, Version> { + Cow::Borrowed(&self.bound) + } } impl Ranged for GreaterThanRange { @@ -695,6 +712,10 @@ impl LessThanRange { bound: boundary.try_into()?, })) } + + pub fn version(&self) -> Cow<'_, Version> { + Cow::Borrowed(&self.bound) + } } impl Ranged for LessThanRange { @@ -743,6 +764,10 @@ impl GreaterThanOrEqualToRange { bound: boundary.try_into()?, })) } + + pub fn version(&self) -> Cow<'_, Version> { + Cow::Borrowed(&self.bound) + } } impl Ranged for GreaterThanOrEqualToRange { @@ -791,6 +816,10 @@ impl LessThanOrEqualToRange { bound: boundary.try_into()?, })) } + + pub fn version(&self) -> Cow<'_, Version> { + Cow::Borrowed(&self.bound) + } } impl Ranged for LessThanOrEqualToRange { @@ -835,6 +864,10 @@ impl EqualsVersion { pub fn version_range(version: Version) -> VersionRange { VersionRange::Equals(Self { version }) } + + pub fn version(&self) -> Cow<'_, Version> { + Cow::Borrowed(&self.version) + } } impl From for EqualsVersion { @@ -904,6 +937,10 @@ impl NotEqualsVersion { pub fn new(specified: usize, base: Version) -> Self { Self { specified, base } } + + pub fn version(&self) -> Cow<'_, Version> { + Cow::Borrowed(&self.base) + } } impl From for NotEqualsVersion { @@ -977,6 +1014,10 @@ impl DoubleEqualsVersion { pub fn version_range(version: Version) -> VersionRange { VersionRange::DoubleEquals(Self { version }) } + + pub fn version(&self) -> Cow<'_, Version> { + Cow::Borrowed(&self.version) + } } impl From for DoubleEqualsVersion { @@ -1043,6 +1084,10 @@ impl DoubleNotEqualsVersion { pub fn new(specified: usize, base: Version) -> Self { Self { specified, base } } + + pub fn version(&self) -> Cow<'_, Version> { + Cow::Borrowed(&self.base) + } } impl From for DoubleNotEqualsVersion { @@ -1131,6 +1176,14 @@ impl CompatRange { }; Ok(VersionRange::Compat(compat_range)) } + + pub fn version(&self) -> Cow<'_, Version> { + Cow::Borrowed(&self.base) + } + + pub fn required(&self) -> Option { + self.required + } } impl Ranged for CompatRange { @@ -1208,19 +1261,6 @@ impl VersionFilter { } } - /// Makes a VersionFilter from the given string, without checking - /// that it is valid. This is used by indexing. - /// - /// # Safety - /// - /// The caller must make sure the string parses ad a valid - /// VersionFilter. - pub unsafe fn new_unchecked(filter_string: &str) -> Self { - VersionFilter::from_str(filter_string).expect( - "A filter string given to VersionFilter::new_unchecked() should be a valid VersionFilter when parsed", - ) - } - pub fn single(item: VersionRange) -> Self { let mut filter = Self::default(); filter.rules.insert(item); diff --git a/crates/spk-schema/src/fb_converter.rs b/crates/spk-schema/src/fb_converter.rs index 31d4aa4fc..e9f2081fc 100644 --- a/crates/spk-schema/src/fb_converter.rs +++ b/crates/spk-schema/src/fb_converter.rs @@ -25,7 +25,23 @@ use spk_schema_foundation::ident_ops::parsing::NormalizedVersionString; use spk_schema_foundation::name::{OptNameBuf, PkgNameBuf, RepositoryNameBuf}; use spk_schema_foundation::option_map::OptionMap; use spk_schema_foundation::spec_ops::FileMatcher; -use spk_schema_foundation::version_range::VersionFilter; +use spk_schema_foundation::version_range::{ + CompatRange, + DoubleEqualsVersion, + DoubleNotEqualsVersion, + EqualsVersion, + GreaterThanOrEqualToRange, + GreaterThanRange, + LessThanOrEqualToRange, + LessThanRange, + LowestSpecifiedRange, + NotEqualsVersion, + Ranged, + SemverRange, + VersionFilter, + VersionRange, + WildcardRange, +}; use crate::component_embedded_packages::ComponentEmbeddedPackage; use crate::deprecate::Deprecate; @@ -45,6 +61,8 @@ use crate::{ RequirementsList, }; +const WILDCARD_MARKER: u32 = u32::MAX; + /// Helper for turning a Vec into to flatbuffer Some(list) or None, if /// the list is empty. #[macro_export] @@ -231,10 +249,6 @@ pub fn fb_requirements_to_requirements( let components = fb_component_names_to_component_names_set(&fb_pkg_req.components()); - // TODO: this will be the next thing to get a - // proper flatbuffer representation because it - // is now showing up as the next significant - // chunk on the large solve flamegraphs let version_filter = fb_version_filter_to_version_filter(fb_pkg_req.version_filter()); @@ -744,18 +758,223 @@ pub fn fb_pkg_request_option_values_to_pkg_request_options( options } -// TODO: this will change if the version filter gets a proper -// flatbuffer representation to move it away from a string. This look -// like it is advisable to do in future, based on current profiling. -#[inline] -pub fn fb_version_filter_to_version_filter(version_filter: Option<&str>) -> VersionFilter { - if let Some(filter_string) = version_filter { - unsafe { VersionFilter::new_unchecked(filter_string) } +pub fn fb_version_filter_to_version_filter( + fb_version_filter: Option< + flatbuffers::Vector<'_, flatbuffers::ForwardsUOffset>>, + >, +) -> VersionFilter { + if let Some(filter) = fb_version_filter { + let mut rules = Vec::new(); + + for fb_rule in filter.iter() { + let rule = match fb_rule.filter_op() { + spk_proto::VersionRangeOperator::Compat => { + let base = fb_rule + .version() + .map(|v| fb_version_to_version(v)) + .expect("A VersionRangeOperator::Compat should have a version in an index"); + let required = fb_lone_compat_rule_to_lone_compat_rule(fb_rule.required()); + VersionRange::Compat(CompatRange::new(base, required)) + } + spk_proto::VersionRangeOperator::DoubleEquals => { + let version = fb_rule.version().map(|v| fb_version_to_version(v)).expect( + "A VersionRangeOperator::DoubleEquals should have a version in an index", + ); + VersionRange::DoubleEquals(DoubleEqualsVersion::from(version)) + } + spk_proto::VersionRangeOperator::DoubleNotEquals => { + let base = fb_rule + .version() + .map(|v| fb_version_to_version(v)) + .expect("A VersionRangeOperator::DoubleNotEquals should have a base version in an index"); + VersionRange::DoubleNotEquals(DoubleNotEqualsVersion::from(base)) + } + spk_proto::VersionRangeOperator::Equals => { + let version = fb_rule.version().map(|v| fb_version_to_version(v)).expect( + "An VersionRangeOperator::Equals should have a version in an index", + ); + VersionRange::Equals(EqualsVersion::new(version)) + } + spk_proto::VersionRangeOperator::Filter => { + unreachable!( + "A version range Filter should not be in the index. It should have been flattened beforehand into other VersionRange objects" + ) + } + spk_proto::VersionRangeOperator::GreaterThan => { + let version = fb_rule + .version() + .map(|v| fb_version_to_version(v)) + .expect("A GreaterThanRange should have a version in an index"); + VersionRange::GreaterThan(GreaterThanRange::new(version)) + } + spk_proto::VersionRangeOperator::GreaterThanOrEqualTo => { + let version = fb_rule + .version() + .map(|v| fb_version_to_version(v)) + .expect("A VersionRangeOperator::GreaterThanOrEqualTo should have a version in an index"); + VersionRange::GreaterThanOrEqualTo(GreaterThanOrEqualToRange::new(version)) + } + spk_proto::VersionRangeOperator::LessThan => { + let version = fb_rule.version().map(|v| fb_version_to_version(v)).expect( + "A VersionRangeOperator::LessThan should have a version in an index", + ); + VersionRange::LessThan(LessThanRange::new(version)) + } + spk_proto::VersionRangeOperator::LessThanOrEqualTo => { + let version = fb_rule + .version() + .map(|v| fb_version_to_version(v)) + .expect("A VersionRangeOperator::LessThanOrEqualTo should have a version in an index"); + VersionRange::LessThanOrEqualTo(LessThanOrEqualToRange::new(version)) + } + spk_proto::VersionRangeOperator::LowestSpecified => { + let base = fb_rule.version().map(|v| fb_version_to_version(v)).expect( + "A VersionRangeOperator::LowestSpecified should have a base in an index", + ); + VersionRange::LowestSpecified(LowestSpecifiedRange::new(base)) + } + spk_proto::VersionRangeOperator::NotEquals => { + let base = fb_rule + .version() + .map(|v| fb_version_to_version(v)) + .expect("A VersionRangeOperator::NotEquals should have a base in an index"); + VersionRange::NotEquals(NotEqualsVersion::from(base)) + } + spk_proto::VersionRangeOperator::Semver => { + let minimum = fb_rule + .version() + .map(|v| fb_version_to_version(v)) + .expect("A VersionRangeOperator::Semver should have a version in an index"); + VersionRange::Semver(SemverRange::new(minimum)) + } + spk_proto::VersionRangeOperator::Wildcard => { + if let Some(parts_list) = fb_rule.parts() { + let parts = parts_list + .iter() + .map(|p| if p != WILDCARD_MARKER { Some(p) } else { None }) + .collect(); + VersionRange::Wildcard(unsafe { WildcardRange::new_unchecked(parts) }) + } else { + // This should not happen? + VersionRange::Wildcard(WildcardRange::any_version()) + } + } + _ => { + // coverage for the ::MAX values + unreachable!( + "filter_op field from a VersionFilterRule in an index should not be outside the valid VersionRangeOperator entries" + ); + } + }; + + rules.push(rule); + } + + VersionFilter::new(rules) } else { Default::default() } } +fn version_filter_to_fb_version_filter<'a>( + builder: &mut flatbuffers::FlatBufferBuilder<'a>, + version_filter: &VersionFilter, +) -> Option< + flatbuffers::WIPOffset< + flatbuffers::Vector<'a, flatbuffers::ForwardsUOffset>>, + >, +> { + let mut converted_rules = Vec::new(); + for rule in version_filter.rules() { + let mut args = spk_proto::VersionFilterRuleArgs { + filter_op: spk_proto::VersionRangeOperator::Compat, + version: None, + required: spk_proto::LoneCompatRule::None, + parts: None, + }; + + match rule { + VersionRange::Compat(value) => { + let fb_version = version_to_fb_version(builder, &value.version()); + let fb_required = lone_compat_rule_to_fb_lone_compat_rule(value.required()); + args.filter_op = spk_proto::VersionRangeOperator::Compat; + args.version = Some(fb_version); + args.required = fb_required; + } + VersionRange::DoubleEquals(value) => { + let fb_version = version_to_fb_version(builder, &value.version()); + args.filter_op = spk_proto::VersionRangeOperator::DoubleEquals; + args.version = Some(fb_version); + } + VersionRange::DoubleNotEquals(value) => { + let fb_version = version_to_fb_version(builder, &value.version()); + args.filter_op = spk_proto::VersionRangeOperator::DoubleNotEquals; + args.version = Some(fb_version); + } + VersionRange::Equals(value) => { + let fb_version = version_to_fb_version(builder, &value.version()); + args.filter_op = spk_proto::VersionRangeOperator::Equals; + args.version = Some(fb_version); + } + VersionRange::Filter(value) => { + unreachable!( + "VersionRange::Filter is not allowed in the index. It should have been flattened out: {value}" + ) + } + VersionRange::GreaterThan(value) => { + let fb_version = version_to_fb_version(builder, &value.version()); + args.filter_op = spk_proto::VersionRangeOperator::GreaterThan; + args.version = Some(fb_version); + } + VersionRange::GreaterThanOrEqualTo(value) => { + let fb_version = version_to_fb_version(builder, &value.version()); + args.filter_op = spk_proto::VersionRangeOperator::GreaterThanOrEqualTo; + args.version = Some(fb_version); + } + VersionRange::LessThan(value) => { + let fb_version = version_to_fb_version(builder, &value.version()); + args.filter_op = spk_proto::VersionRangeOperator::LessThan; + args.version = Some(fb_version); + } + VersionRange::LessThanOrEqualTo(value) => { + let fb_version = version_to_fb_version(builder, &value.version()); + args.filter_op = spk_proto::VersionRangeOperator::LessThanOrEqualTo; + args.version = Some(fb_version); + } + VersionRange::LowestSpecified(value) => { + let fb_version = version_to_fb_version(builder, &value.version()); + args.filter_op = spk_proto::VersionRangeOperator::LowestSpecified; + args.version = Some(fb_version); + } + VersionRange::NotEquals(value) => { + let fb_version = version_to_fb_version(builder, &value.version()); + args.filter_op = spk_proto::VersionRangeOperator::NotEquals; + args.version = Some(fb_version); + } + VersionRange::Semver(value) => { + let fb_version = version_to_fb_version(builder, &value.version()); + args.filter_op = spk_proto::VersionRangeOperator::Semver; + args.version = Some(fb_version); + } + VersionRange::Wildcard(value) => { + let parts: Vec = value + .parts() + .iter() + .map(|p| if let Some(n) = p { *n } else { WILDCARD_MARKER }) + .collect(); + let fb_parts = flatbuffer_vector!(builder, parts); + args.filter_op = spk_proto::VersionRangeOperator::Wildcard; + args.parts = fb_parts; + } + }; + + let fb_rule = spk_proto::VersionFilterRule::create(builder, &args); + converted_rules.push(fb_rule); + } + + flatbuffer_vector!(builder, converted_rules) +} + #[inline] fn component_to_fb_component<'a>( builder: &mut flatbuffers::FlatBufferBuilder<'a>, @@ -1185,17 +1404,10 @@ pub fn requirements_with_options_to_fb_requirements_with_options<'a>( let fb_components = components_set_to_fb_components(builder, &pr.pkg.components); - // TODO: for now version filters strings, but in future a proper - // version filter will breakdown into pieces in the flatbuffer let fb_version_filter = if pr.pkg.version.is_empty() { None } else { - // A version filter is a bunch of version ranges, e.g. - // [kg/3.10 - // pkg/3.10.0 - // so need to use the alternate format here for version - // filters to ensure extra .0's don't get baked into requests. - Some(builder.create_string(&format!("{:#}", pr.pkg.version))) + version_filter_to_fb_version_filter(builder, &pr.pkg.version) }; let (fb_build, fb_build_type) = if let Some(build) = &pr.pkg.build {