-
Notifications
You must be signed in to change notification settings - Fork 99
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use actual repository ID in stored transactions #2061
base: main
Are you sure you want to change the base?
Conversation
460a416
to
544667e
Compare
Previously, the `Package::get_package_path()` method worked correctly only for packages in repositories of type `Repo::Type::COMMANDLINE`. When an RPM file was added to a regular `Repo::Type::AVAILABLE` repository, its package path incorrectly pointed to the cache directory instead of the local RPM file. This fix ensures the correct path is used.
Currently, when replaying a stored transaction, all RPM files are placed into a single `@stored_transaction` repository. The user can see this artifitial repository when checking from which repositories the installed packages came: ❯ dnf repoquery --installed --queryformat="%{full_nevra} %{from_repo}\n" vlc-plugin-gstreamer-1:3.0.21-15.fc40.x86_64 @stored_transaction This can be confusing, especially during a system upgrade, which also uses stored transactions. With this patch, local RPM files from stored transactions are placed to the repository they originally came from: ❯ dnf repoquery --installed --queryformat="%{full_nevra} %{from_repo}\n" vlc-plugin-gstreamer-1:3.0.21-15.fc40.x86_64 updates Fixes: #1851
544667e
to
36d5e1d
Compare
Use explicit std::vector<std::string> constructor instead of uniform initialization to allow range based construction.
36d5e1d
to
e05b785
Compare
Use prepared connection to the System Bus. Otherwise the default connection to Session Bus would be created.
@@ -161,6 +164,9 @@ class PackageSack::Impl { | |||
int cached_solvables_size{0}; | |||
PackageId running_kernel; | |||
|
|||
// ids of packages created from local rpm files | |||
std::unordered_set<Id> local_rpm_ids; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not really sure about this.
In my mind the Repo::Type::AVAILABLE
repos are not meant for locally added rpms. I would be interested in an opinion from @jrohel.
On the other hand I understand that our options are limited here.
Given that we are interested only in the repo ids would it be possible to perhaps not load the repo definitions at all and create only Repo::Type::COMMANDLINE
repos with the specified id when needed by the add_stored_transaction_package(...)
?
This would have the advantage that package replays with repo ids that are no longer defined on the system would still have the original repoid instead of @stored_transaction
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is also possibility. Let me experiment with this approach a bit...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've created an alternative PR which does not create repositories from the system configuration and creates COMMANDLINE
type repos on the fly as they are needed: #2093
Currently, when replaying a stored transaction, all RPM files are placed into a single
@stored_transaction
repository. This can be confusing, especially during a system upgrade, which also uses stored transactions.With this patch, local RPM files from stored transactions are placed to the repository they originally came from.
Fixes: #1851