Use a correct EVR sort by dralley · Pull Request #4136 · pulp/pulp_rpm

dralley · 2025-09-18T04:16:53Z

No description provided.

mdellweg · 2025-09-19T11:48:58Z

+        curr_vers.get_content(Package.objects)
+        .order_by(
+            "name", "epoch", "version", "release", "arch"
+        )  # why sort by all of these values?  lexicographical sort is not "accurate"


I have been doing experiments with the "CREATE COLLATION" feature in postgresql.
I think it could be tailored to define a proper "lexicographical" order for these version components.

related pr: pulp/pulp_deb#1335

RPM versioning has a special ~ and ^ character behavior - is collation flexible enough to handle that?

see:

https://slashterix.wordpress.com/2016/08/06/rpm-version-comparison/

https://github.com/pulp/pulp_rpm/blob/main/pulp_rpm/app/rpm_version.py

I have been doing experiments with the "CREATE COLLATION" feature in postgresql. I think it could be tailored to define a proper "lexicographical" order for these version components.

related pr: pulp/pulp_deb#1335

Isn't that also dependent on pgres-16?

Yes it is pg 16 and for the special ~ handling (sorting even before the end of the segment) i needed to trick a little.
I'd love to do a little less of that. But i found no way to either add a custom normalize function to the collation or a way to provide a search_key function to a text field.

dralley · 2025-10-08T16:43:37Z

        return "pulp_evr_t"


-class PackageManager(ContentManager):


The rationale for getting rid of this is that different implementations of with_age() are not stable when applying additional filters to the queryset. You want the annotation to be the very last thing you do post-filtering to guarantee correctness.

Also I recall running into issues because we didn't inherit from the Pulpcore-overridden content manager properly. Easier to avoid conflicts this way.

dralley · 2025-10-08T17:18:23Z

+
+    when_clauses = [When(pk=pk, then=age) for pk, age in age_mapping.items()]
+
+    return qs.annotate(age=Case(*when_clauses, output_field=IntegerField()))


As mentioned previously I don't really love this, but it should be "fine" and hopefully it's just a temporary implementation

Remind us again why this is a temporary implementation?

Because ideally we have a fix in the database layer that can be shared with Katello. But that's not backportable.

dralley · 2025-10-08T17:38:37Z

        self.assertEqual(packages[2].release, "1.el8")  # age=3
        self.assertEqual(packages[2].age, 3)

-    @skip("The implementation of package age is broken w/r/t '^' and '~' characters")


This test passes now

dralley · 2025-10-08T17:38:45Z

        self.assertIn("4_0", remaining_versions)
        self.assertIn("4+0", remaining_versions)

-    @skip("Non-ASCII character handling do not work correctly with current implementation")


This test passes now

Copilot

Pull Request Overview

This PR fixes a bug in RPM package version comparison by replacing PostgreSQL's built-in EVR (Epoch-Version-Release) sorting with a proper Python-based implementation using the RpmVersion class.

Removes the PostgreSQL window function approach from PackageManager.with_age()
Implements a new annotate_with_age() function in shared_utils.py that uses proper RPM version sorting
Updates all code locations to use the new annotation function instead of the manager method

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
pulp_rpm/app/shared_utils.py	Adds new `annotate_with_age()` function with proper EVR sorting using RpmVersion
pulp_rpm/app/models/package.py	Removes the broken `PackageManager.with_age()` method and custom manager
pulp_rpm/tests/unit/test_package_age.py	Updates all test calls to use new annotation function and removes skip decorators
pulp_rpm/app/tasks/prune.py	Updates to use new annotation function
pulp_rpm/app/tasks/copy.py	Updates to use new annotation function
pulp_rpm/app/models/repository.py	Updates to use new annotation function
CHANGES/4124.bugfix	Adds changelog entry describing the bug fix

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-08T20:45:35Z

+    newest age=2, and so on.
+
+    A second partition by architecture is important because there can be packages with
+    the same name and verison numbers but they are not interchangeable because they have


Corrected spelling of 'verison' to 'version'.

Suggested change

the same name and verison numbers but they are not interchangeable because they have

the same name and version numbers but they are not interchangeable because they have

ggainey

Looks like this should address the issue(s) - great work!

closes pulp#4124

patchback · 2025-10-10T16:29:51Z

Backport to 3.26: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply c7bbe48 on top of patchback/backports/3.26/c7bbe48ba247518fe0cd5e840ec2357811fe49bb/pr-4136

Backporting merged PR #4136 into main

Ensure you have a local repo clone of your fork. Unless you cloned it
from the upstream, this would be your origin remote.
Make sure you have an upstream repo added as a remote too. In these
instructions you'll refer to it by the name upstream. If you don't
have it, here's how you can add it:
```
$ git remote add upstream https://github.com/pulp/pulp_rpm.git
```

Ensure you have the latest copy of upstream and prepare a branch
that will hold the backported code:

$ git fetch upstream
$ git checkout -b patchback/backports/3.26/c7bbe48ba247518fe0cd5e840ec2357811fe49bb/pr-4136 upstream/3.26

Now, cherry-pick PR Use a correct EVR sort #4136 contents into that branch:
```
$ git cherry-pick -x c7bbe48ba247518fe0cd5e840ec2357811fe49bb
```
If it'll yell at you with something like fatal: Commit c7bbe48ba247518fe0cd5e840ec2357811fe49bb is a merge but no -m option was given., add -m 1 as follows instead:
```
$ git cherry-pick -m1 -x c7bbe48ba247518fe0cd5e840ec2357811fe49bb
```
At this point, you'll probably encounter some merge conflicts. You must
resolve them in to preserve the patch from PR Use a correct EVR sort #4136 as close to the
original as possible.

Push this branch to your fork on GitHub:

$ git push origin patchback/backports/3.26/c7bbe48ba247518fe0cd5e840ec2357811fe49bb/pr-4136

Create a PR, ensure that the CI is green. If it's not — update it so that
the tests and any other checks pass. This is it!
Now relax and wait for the maintainers to process your pull request
when they have some cycles to do reviews. Don't worry — they'll tell you if
any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback · 2025-10-10T16:29:57Z

Backport to 3.29: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.29/c7bbe48ba247518fe0cd5e840ec2357811fe49bb/pr-4136

Backported as #4166

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback · 2025-10-10T16:30:06Z

Backport to 3.27: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.27/c7bbe48ba247518fe0cd5e840ec2357811fe49bb/pr-4136

Backported as #4167

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback · 2025-10-10T16:30:14Z

Backport to 3.32: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.32/c7bbe48ba247518fe0cd5e840ec2357811fe49bb/pr-4136

Backported as #4168

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

github-actions Bot added no-changelog no-issue labels Sep 18, 2025

dralley force-pushed the evr-sorting branch 5 times, most recently from 53934f7 to 5b6a3be Compare September 18, 2025 13:15

mdellweg reviewed Sep 19, 2025

View reviewed changes

dralley force-pushed the evr-sorting branch 3 times, most recently from 2486a36 to cb6241a Compare September 23, 2025 19:07

github-actions Bot added the multi-commit Added when a PR consists of more than one commit label Sep 23, 2025

dralley force-pushed the evr-sorting branch 2 times, most recently from 9c5de07 to ef509ef Compare September 23, 2025 19:54

dralley commented Sep 23, 2025

View reviewed changes

Comment thread pulp_rpm/app/models/package.py Outdated

pedro-psb mentioned this pull request Sep 23, 2025

Add tests for and fix RPM version sorting #4146

Merged

dralley force-pushed the evr-sorting branch from ef509ef to 2b656f4 Compare September 24, 2025 13:34

github-actions Bot removed the multi-commit Added when a PR consists of more than one commit label Sep 24, 2025

dralley added backport-3.26 backport-3.29 backport-3.27 backport-3.32 labels Sep 24, 2025

dralley force-pushed the evr-sorting branch 3 times, most recently from 8899d12 to 561b378 Compare September 24, 2025 19:44

github-actions Bot added the multi-commit Added when a PR consists of more than one commit label Sep 24, 2025

dralley force-pushed the evr-sorting branch from 561b378 to 6056f17 Compare September 25, 2025 16:37

github-actions Bot removed multi-commit Added when a PR consists of more than one commit no-issue no-changelog labels Sep 25, 2025

dralley force-pushed the evr-sorting branch 4 times, most recently from 75788be to 5f13c10 Compare October 8, 2025 16:16

github-actions Bot removed the multi-commit Added when a PR consists of more than one commit label Oct 8, 2025

dralley commented Oct 8, 2025

View reviewed changes

Comment thread pulp_rpm/app/models/repository.py

dralley force-pushed the evr-sorting branch from 5f13c10 to 877108a Compare October 8, 2025 17:15

dralley marked this pull request as ready for review October 8, 2025 17:15

github-actions Bot removed no-issue no-changelog labels Oct 8, 2025

dralley commented Oct 8, 2025

View reviewed changes

dralley requested a review from Copilot October 8, 2025 20:45

Copilot AI reviewed Oct 8, 2025

View reviewed changes

dralley requested a review from ggainey October 9, 2025 21:16

ggainey approved these changes Oct 10, 2025

View reviewed changes

Use a correct package EVR sort

558baf1

closes pulp#4124

dralley force-pushed the evr-sorting branch from 877108a to 558baf1 Compare October 10, 2025 16:01

dralley enabled auto-merge (rebase) October 10, 2025 16:07

dralley merged commit c7bbe48 into pulp:main Oct 10, 2025
14 checks passed

patchback Bot mentioned this pull request Oct 10, 2025

[PR #4136/c7bbe48b backport][3.29] Use a correct EVR sort #4166

Merged

This was referenced Oct 10, 2025

[PR #4136/c7bbe48b backport][3.27] Use a correct EVR sort #4167

Merged

[PR #4136/c7bbe48b backport][3.32] Use a correct EVR sort #4168

Merged

dralley deleted the evr-sorting branch October 10, 2025 16:52


		when_clauses = [When(pk=pk, then=age) for pk, age in age_mapping.items()]

		return qs.annotate(age=Case(*when_clauses, output_field=IntegerField()))

	the same name and verison numbers but they are not interchangeable because they have
	the same name and version numbers but they are not interchangeable because they have

Conversation

dralley commented Sep 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

ggainey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

patchback Bot commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Backport to 3.26: 💔 cherry-picking failed — conflicts found

Backporting merged PR #4136 into main

Uh oh!

patchback Bot commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Backport to 3.29: 💚 backport PR created

Uh oh!

patchback Bot commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Backport to 3.27: 💚 backport PR created

Uh oh!

patchback Bot commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Backport to 3.32: 💚 backport PR created

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

patchback Bot commented Oct 10, 2025 •

edited

Loading

patchback Bot commented Oct 10, 2025 •

edited

Loading

patchback Bot commented Oct 10, 2025 •

edited

Loading

patchback Bot commented Oct 10, 2025 •

edited

Loading