Skip to content
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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
+51 −16
Closed

Use actual repository ID in stored transactions #2061

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4e8669d
package: Fix package path for packages from RPM files
m-blaha Feb 19, 2025
0894497
repo: Invalidate provides after adding a comps xml
m-blaha Feb 19, 2025
7f3a61c
Use actual repository ID in stored transactions
m-blaha Feb 19, 2025
e05b785
offline: Fix constructing vector from D-Bus value
m-blaha Feb 19, 2025
1c1e925
offline: Use connection for creating D-Bus proxies
m-blaha Feb 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion libdnf5/repo/repo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,9 @@ rpm::Package Repo::add_rpm_package(const std::string & path, bool with_hdrid) {
}

p_impl->solv_repo->set_needs_internalizing();
p_impl->base->get_rpm_package_sack()->p_impl->invalidate_provides();
auto pkg_sack = p_impl->base->get_rpm_package_sack();
pkg_sack->p_impl->register_local_rpm_id(new_id);
pkg_sack->p_impl->invalidate_provides();

return rpm::Package(p_impl->base, rpm::PackageId(new_id));
}
Expand Down
3 changes: 2 additions & 1 deletion libdnf5/rpm/package.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,8 @@ std::vector<std::string> Package::get_remote_locations(const std::set<std::strin
std::string Package::get_package_path() const {
Solvable * solvable = get_rpm_pool(p_impl->base).id2solvable(p_impl->id.id);
if (auto repo = static_cast<repo::Repo *>(solvable->repo->appdata)) {
if (repo->get_type() == repo::Repo::Type::COMMANDLINE) {
if (repo->get_type() == repo::Repo::Type::COMMANDLINE ||
p_impl->base->get_rpm_package_sack()->p_impl->is_local_rpm_id(p_impl->id.id)) {
// Command line packages are used from their original location.
return get_location();
}
Expand Down
8 changes: 8 additions & 0 deletions libdnf5/rpm/package_sack.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -637,4 +637,12 @@ rpm::Package PackageSack::get_running_kernel() {
return rpm::Package(p_impl->base, p_impl->get_running_kernel_id());
}

void PackageSack::Impl::register_local_rpm_id(const Id id) {
local_rpm_ids.emplace(id);
}

bool PackageSack::Impl::is_local_rpm_id(const Id id) {
return local_rpm_ids.contains(id);
}

} // namespace libdnf5::rpm
6 changes: 6 additions & 0 deletions libdnf5/rpm/package_sack_impl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,9 @@ class PackageSack::Impl {
/// And sets `considered_uptodate` to` true`.
void recompute_considered_in_pool();

void register_local_rpm_id(const Id id);
bool is_local_rpm_id(const Id id);

private:
bool provides_ready{false};

Expand Down Expand Up @@ -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;
Copy link
Contributor

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.

Copy link
Member Author

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...

Copy link
Member Author

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


friend PackageSack;
friend Package;
friend PackageSet;
Expand Down