Indexes 6: Changes version_filter field in index schema#1344
Open
dcookspi wants to merge 1 commit intoindex-5-repo-cmds-config-and-cli-flagsfrom
Open
Indexes 6: Changes version_filter field in index schema#1344dcookspi wants to merge 1 commit intoindex-5-repo-cmds-config-and-cli-flagsfrom
version_filter field in index schema#1344dcookspi wants to merge 1 commit intoindex-5-repo-cmds-config-and-cli-flagsfrom
Conversation
dcookspi
added
enhancement
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
dcookspi
force-pushed
the
index-5-repo-cmds-config-and-cli-flags
branch
2 times, most recently
from
83c8d12 to
3dcf4e3
Compare
dcookspi
force-pushed
the
index-5-zplus-1-versionfilters-not-strings
branch
2 times, most recently
from
95154e5 to
6e12345
Compare
jrray
reviewed
Apr 2, 2026
crates/spk-proto/schema/spk.fbs
Outdated
| // LessThanOrEqualToRange, LessThanRange, LowestSpecifiedRange, | ||
| // NotEqualsVersion, SemverRange WildCardRange | ||
| version: Version; | ||
| // Used for: Compat, |
Collaborator
Author
There was a problem hiding this comment.
No, I've taken it out.
jrray
requested changes
Apr 2, 2026
| let base = fb_rule | ||
| .version() | ||
| .map(|v| fb_version_to_version(v)) | ||
| .expect("A CompatRange should have a version in an index"); |
Collaborator
There was a problem hiding this comment.
It's hurting my brain that the type names mentioned in these expect strings are aligned with the version_range types, but they don't match the VersionRangeOperator enum arm names, and it makes this code look like there's "random" inconsistencies. But now I see they do match the VersionRange enum names.
How about changing the names used in these strings to match the enum names?
Collaborator
Author
There was a problem hiding this comment.
Todo:
- Update the names in the messages to match the enum names
jrray
reviewed
Apr 2, 2026
| None | ||
| } else { | ||
| // A version filter is a bunch of version ranges, e.g. | ||
| // [kg/3.10 |
Collaborator
There was a problem hiding this comment.
Do I need to hunt down the PR where this [ came from? 😅
dcookspi
force-pushed
the
index-5-repo-cmds-config-and-cli-flags
branch
3 times, most recently
from
39f3a57 to
50c68f1
Compare
list/table representation instead of a string. The field no longer needs to be parsed when extracted from the index. Signed-off-by: David Gilligan-Cook <dcook@imageworks.com>
dcookspi
force-pushed
the
index-5-zplus-1-versionfilters-not-strings
branch
from
6e12345 to
c3fb021
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This changes the
version_filterfield in the index schema to a flatbuffers list and table representation, instead of a string. This allows the version filter rules (VersionRange entries) to be extracted from the index without string parsing.The resulting index is slightly larger at 122 MB (see Indexes PR 5: #1340). Index generation time is about the same. But the solve times improve for the larger solves (marked with *):
This is the 6th of the chained PRs for adding indexes to spk solves:
new_unchecked()constructors to spk schema objects #1337spk repo indexsubcommand for index generation and updates #1340