goal: Do not add lower versions to upgrade jobs #878
Conversation
m-blaha
force-pushed
the
mblaha/upgrade-versions
branch
from
f62ab7f to
3423249
Compare
libdnf5/base/goal.cpp
Outdated
| // Remove packages of versions lower than the installed ones. | ||
| // Substract downgrades instead of filter_upgrades() to keep | ||
| // installed packages in the query. | ||
| rpm::PackageQuery tmp_downgrades(query); |
There was a problem hiding this comment.
I am quite curious, why you want to keep packages with the same version as installed in the query?
There was a problem hiding this comment.
Well, I don't :) I've just overlooked it. Thanks for noticing.
There was a problem hiding this comment.
I recall that "dnf upgrade" reinstalls packages which were originally installed unsigned from a local build and then they appeared properly signed in a repository. That could warrant leaving same versions in the query.
There was a problem hiding this comment.
I recall that "dnf upgrade" reinstalls packages which were originally installed unsigned from a local build and then they appeared properly signed in a repository. That could warrant leaving same versions in the query.
I need to check this. We do not have a test for it, I suppose?
There was a problem hiding this comment.
@ppisar it seems that you are refering to behaviour described in this libsolv issue: openSUSE/libsolv#287
The trigger for re-installation was not whether the package was or was not signed, but more generally that the package was changed (for example has different build time).
This behavior was changed in dnf4 here: rpm-software-management/libdnf#786 (and the same behavior we have in dnf5).
So dnf4 upgrade does not reinstall package with the same nevra but different buildtime for quite a long time. But distrosync command should trigger the reinstallation.
There was a problem hiding this comment.
Yes, that libsolv ticket describes my experience. If DNF stopped doing it, the then j-mracek's proposal is fine.
m-blaha
force-pushed
the
mblaha/upgrade-versions
branch
from
3423249 to
bbcab54
Compare
libdnf5/base/goal.cpp
Outdated
| /// to the installed ones. Similar to filter_upgrades(), but retains also | ||
| /// installed packages and packages whose names are not installed. | ||
| static void remove_older_versions(libdnf5::rpm::PackageQuery & query) { | ||
| libdnf5::rpm::PackageQuery installed(query.get_base()); |
There was a problem hiding this comment.
I would suggest to initialize query without excluded packages
libdnf5::rpm::PackageQuery installed(query.get_base(), rpm::PackageQuery::ExcludeFlags::IGNORE_EXCLUDES)
| /// installed packages and packages whose names are not installed. | ||
| static void remove_older_versions(libdnf5::rpm::PackageQuery & query) { | ||
| libdnf5::rpm::PackageQuery installed(query.get_base()); | ||
| installed.filter_installed(); |
There was a problem hiding this comment.
This is very weak suggestion because it does not change the result.
What about to add additional filter - latest()? Installonly packages will be handled in the next query only once.
There was a problem hiding this comment.
Added. (although I did not measure impact on performance here).
m-blaha
force-pushed
the
mblaha/upgrade-versions
branch
2 times, most recently
from
05935ea to
c39f7fc
Compare
|
Looks like some tests with repo priorities started failing. I'll investigate. |
|
We have a problem, if we do this we will have a problem with repo priority test cases. Basically we have two options, we can keep all packages from proiority repo and then apply removal of old packages (logic from check update) |
For UPGRADE* actions, do not add packages that are older than the currently installed versions to the libsolv job. This decreases the number of potential solutions and helps resolve some edge cases. See https://issues.redhat.com/browse/RHEL-1448 for one of those edge cases.
The `minimal` parameter makes sense only for upgrade* actions, and filtering of the earliest EVR needs to be performed after the versions lower than the installed ones have been removed.
m-blaha
force-pushed
the
mblaha/upgrade-versions
branch
from
c39f7fc to
a87ab64
Compare
I've added code to pre-select upgrade candidates considering also repository priorities. However, I'm starting to feel that we might be exceeding our responsibilities by intervening in matters that should be handled by libsolv. To be honest, I'm leaning towards closing this PR and simply documenting in the upgrade code why including all versions of the package in the libsolv job is important. |
|
I agree we are stepping on libsolve's toes. Maybe --best can provide a similar user experience. |
|
@ppisar unfortunately, the |
For UPGRADE* actions, do not add packages that are older than the
currently installed versions to the libsolv job. This decreases the
number of potential solutions and helps resolve some edge cases.
See https://issues.redhat.com/browse/RHEL-1448 for one of those edge
cases.
Test: rpm-software-management/ci-dnf-stack#1380