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 all commits
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
20 changes: 11 additions & 9 deletions dnf5/commands/offline/offline.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,11 +245,13 @@ void reboot(bool poweroff = false) {
const std::string error_message{ex.what()};
throw libdnf5::cli::CommandExitError(1, M_("Couldn't connect to D-Bus: {}"), error_message);
}
auto proxy = sdbus::createProxy(SYSTEMD_DESTINATION_NAME, SYSTEMD_OBJECT_PATH);
if (poweroff) {
proxy->callMethod("PowerOff").onInterface(SYSTEMD_MANAGER_INTERFACE);
} else {
proxy->callMethod("Reboot").onInterface(SYSTEMD_MANAGER_INTERFACE);
if (connection != nullptr) {
auto proxy = sdbus::createProxy(*connection, SYSTEMD_DESTINATION_NAME, SYSTEMD_OBJECT_PATH);
if (poweroff) {
proxy->callMethod("PowerOff").onInterface(SYSTEMD_MANAGER_INTERFACE);
} else {
proxy->callMethod("Reboot").onInterface(SYSTEMD_MANAGER_INTERFACE);
}
}
#else
std::cerr << "Can't connect to D-Bus; this build of DNF 5 does not support D-Bus." << std::endl;
Expand Down Expand Up @@ -313,17 +315,17 @@ void OfflineRebootCommand::run() {
std::cerr << "Warning: couldn't connect to D-Bus: " << ex.what() << std::endl;
}
if (connection != nullptr) {
auto systemd_proxy = sdbus::createProxy(SYSTEMD_DESTINATION_NAME, SYSTEMD_OBJECT_PATH);
auto systemd_proxy = sdbus::createProxy(*connection, SYSTEMD_DESTINATION_NAME, SYSTEMD_OBJECT_PATH);

sdbus::ObjectPath unit_object_path;
systemd_proxy->callMethod("LoadUnit")
.onInterface(SYSTEMD_MANAGER_INTERFACE)
.withArguments("system-update.target")
.storeResultsTo(unit_object_path);

auto unit_proxy = sdbus::createProxy(SYSTEMD_DESTINATION_NAME, unit_object_path);
const auto & wants =
std::vector<std::string>{unit_proxy->getProperty("Wants").onInterface(SYSTEMD_UNIT_INTERFACE)};
auto unit_proxy = sdbus::createProxy(*connection, SYSTEMD_DESTINATION_NAME, unit_object_path);
const auto wants =
std::vector<std::string>(unit_proxy->getProperty("Wants").onInterface(SYSTEMD_UNIT_INTERFACE));
if (std::find(wants.begin(), wants.end(), SYSTEMD_SERVICE_NAME) == wants.end()) {
throw libdnf5::cli::CommandExitError(
1, M_("{} is not wanted by system-update.target."), SYSTEMD_SERVICE_NAME);
Expand Down
2 changes: 1 addition & 1 deletion include/libdnf5/repo/repo_sack.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ class LIBDNF_API RepoSack : public sack::Sack<Repo> {
/// @param calculate_checksum Whether libsolv should calculate and store checksum of added packages. Setting to true significantly reduces performance.
/// @return Newly created rpm::Package object in cmdline repo
LIBDNF_LOCAL libdnf5::rpm::Package add_stored_transaction_package(
const std::string & path, bool calculate_checksum = false);
const std::string & path, const std::string & repo_id, bool calculate_checksum = false);

LIBDNF_LOCAL void internalize_repos();

Expand Down
4 changes: 2 additions & 2 deletions libdnf5/base/goal.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -791,8 +791,8 @@ GoalProblem Goal::Impl::add_replay_to_goal(
std::optional<libdnf5::rpm::Package> local_pkg;
if (!package_replay.package_path.empty()) {
// Package paths are relative to replay location
local_pkg =
base->get_repo_sack()->add_stored_transaction_package(replay_location / package_replay.package_path);
local_pkg = base->get_repo_sack()->add_stored_transaction_package(
replay_location / package_replay.package_path, package_replay.repo_id);
}

const auto nevras = rpm::Nevra::parse(package_replay.nevra, {rpm::Nevra::Form::NEVRA});
Expand Down
8 changes: 7 additions & 1 deletion libdnf5/repo/repo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,7 @@ void Repo::add_xml_comps(const std::string & path) {
throw RepoCompsError(
M_("Failed to load xml Comps \"{}\": {}"), path, std::string(pool_errstr(p_impl->solv_repo->repo->pool)));
}
p_impl->base->get_rpm_package_sack()->p_impl->invalidate_provides();
}

rpm::Package Repo::add_rpm_package(const std::string & path, bool with_hdrid) {
Expand All @@ -500,8 +501,13 @@ rpm::Package Repo::add_rpm_package(const std::string & path, bool with_hdrid) {
M_("Failed to load RPM \"{}\": {}"), path, std::string(pool_errstr(p_impl->solv_repo->repo->pool)));
}

// repo altered by adding an rpm package should not be written back to the cache
get_config().get_build_cache_option().set(libdnf5::Option::Priority::RUNTIME, false);

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
16 changes: 14 additions & 2 deletions libdnf5/repo/repo_sack.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,20 @@ void RepoSack::add_stored_transaction_comps(const std::string & path) {
}


libdnf5::rpm::Package RepoSack::add_stored_transaction_package(const std::string & path, bool calculate_checksum) {
auto stored_repo = get_stored_transaction_repo();
libdnf5::rpm::Package RepoSack::add_stored_transaction_package(
const std::string & path, const std::string & repo_id, bool calculate_checksum) {
RepoWeakPtr stored_repo;
if (!repo_id.empty()) {
for (const auto & existing_repo : get_data()) {
if (existing_repo->get_id() == repo_id) {
stored_repo = existing_repo->get_weak_ptr();
break;
}
}
}
if (!stored_repo.is_valid()) {
stored_repo = get_stored_transaction_repo();
}

auto pkg = stored_repo->add_rpm_package(path, calculate_checksum);

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