rpm: versioning fixes #1510
Merged
rpm: versioning fixes #1510
Conversation
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
hdonnay
force-pushed
the
hack/rpm-version
branch
from
8b7ac06 to
c24c26d
Compare
hdonnay
force-pushed
the
hack/rpm-version
branch
2 times, most recently
from
bade1c7 to
b766d77
Compare
hdonnay
force-pushed
the
hack/rpm-version
branch
from
8d14b55 to
c67ac62
Compare
RTann
reviewed
Apr 23, 2025
Contributor
RTann
left a comment
RTann
left a comment
There was a problem hiding this comment.
I skipped everything under test/ as well as rpm/packagescanner_test.go. I can go over rpm/packagescanner_test.go another time, if desired, but anything under test/ is very unknown to me. I can look into it, if needed, otherwise I'll leave it for someone who knows more about that than I do
This changes the custom packed-string format into URL query encoding. In the original conception, this field wasn't meant to hold structured information. It's grown to do it, though, so we should use some format that's not hand-rolled. Signed-off-by: Hank Donnay <hdonnay@redhat.com>
The BDB test fixture lost the "version" field in the `rpm` objects. This
re-adds them. Done via `jq`:
```
<bdb.rpm-manifest.json jq '.rpms[] |= . + (.nerva | capture(".+-(?<version>[^-]+)-.+"))' >out
mv out bdb.rpm-manifest.json
```
Signed-off-by: Hank Donnay <hdonnay@redhat.com>
Previously, the test output just printed the diff of the lists, but this makes it very hard to figure out what's being compared. This now prints the name, then prints the diff if necesarry. Signed-off-by: Hank Donnay <hdonnay@redhat.com>
This implements rpm versioning in a package under our control. I wrote this to make sure we understand and control the procedure. Due to issues with test fixures, I wanted to make sure we weren't breaking our tools with our tools. Signed-off-by: Hank Donnay <hdonnay@redhat.com>
Signed-off-by: Hank Donnay <hdonnay@redhat.com>
Signed-off-by: Hank Donnay <hdonnay@redhat.com>
Using these should give us a nice "todo" list when the time comes. Signed-off-by: Hank Donnay <hdonnay@redhat.com>
Port `rpmtest` over to use the `rpmver` package internally for its version comparison. Signed-off-by: Hank Donnay <hdonnay@redhat.com>
Sort the Packages for easier-to-interpret output on test failure. Signed-off-by: Hank Donnay <hdonnay@redhat.com>
Use `rpmver` for a structured comparison rather than just string comparison. Signed-off-by: Hank Donnay <hdonnay@redhat.com>
This tool is based on extracting portions of the `test/periodic` test, repuroposing it to create test fixtures. This outputs `txtar` formatted files that should have enough information to track the manifests' provenance. This has become a pain point when investigating the correctness of test fixures. Signed-off-by: Hank Donnay <hdonnay@redhat.com>
Signed-off-by: Hank Donnay <hdonnay@redhat.com>
Signed-off-by: Hank Donnay <hdonnay@redhat.com>
Signed-off-by: Hank Donnay <hdonnay@redhat.com>
Signed-off-by: Hank Donnay <hdonnay@redhat.com>
hdonnay
force-pushed
the
hack/rpm-version
branch
from
2b11c86 to
8be64d9
Compare
RTann
reviewed
Apr 29, 2025
| // 1: a is newer than b | ||
| // 0: a and b are the same version | ||
| // -1: b is newer than a | ||
| func rpmvercmp(a, b string) int { |
Contributor
There was a problem hiding this comment.
It's hard to tell, but I don't think this has changed since I last reviewed. When I last reviewed, this all looked good to me after I did a comparison between the C version and this version
RTann
approved these changes
Apr 29, 2025
Contributor
RTann
left a comment
RTann
left a comment
There was a problem hiding this comment.
From what I can tell, everything outside test/ looks good. I haven't checked there, but I can if you need. Curious to see how this'll be used for the dnf PR
Member
Author
|
/fast-forward |
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.
While working on #869, I got to a point where I wasn't sure if I could trust the test suite, which is an untenable state of affairs.
This is splitting out that work so it can be applied first.