Add --destdir option support to all commands with --downloadonly#18
Draft
Add --destdir option support to all commands with --downloadonly#18
Conversation
When rebooting machine on an off-line upgrade, the system could be rebooted without unmouning file systems because org.freedesktop.systemd1.Manager.Reboot() D-Bus call performs an immediate reboot. This patch fixes it by using org.freedesktop.login1.Manger interface which performs graceful shutdown. That means stopping services and unmounting file systems. The logind method also employs inhibition locks preventing e.g. inadverternt reboot when another users are logged in. The true argument enables interactive authorization with polkit. That's useless after applying the updates, but useful before applying them when a user issues "dnf5 offline reboot". Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2350947
…ervice
In my test system there is no D-Bus server running in systemd updates
environment. Therefore rebooting by calling a D-Bus method fails:
čen 30 13:17:52 fedora-43 dnf5[642]: Couldn't connect to D-Bus: [org.freedesktop.DBus.Error.FileNotFound] Failed to open bus (No such file or directory)
čen 30 13:17:52 fedora-43 systemd[1]: dnf5-offline-transaction.service: Main process exited, code=exited, status=1/FAILURE
čen 30 13:17:52 fedora-43 systemd[1]: dnf5-offline-transaction.service: Failed with result 'exit-code'.
čen 30 13:17:52 fedora-43 systemd[1]: Failed to start dnf5-offline-transaction.service - Offline upgrades/transactions using DNF 5.
I tried to exececute "shutdown -r now" command or "systemctl reboot"
command instead to avoid starting D-Bus and logind daemons. The
commands indeed initiated the reboot, but still complained on missing
D-Bus.
Let's swallow the bitter pill and accept that systemd requires D-Bus:
This patch adds a dependency on dbus.socket to
dnf5-offline-transaction.service.
(Neveretheless, sometimes systemd is persuaded that dnf5 program finished
with a failure:
čen 30 16:03:17 fedora-43 systemd-logind[647]: The system will reboot now!
[...]
čen 30 16:03:17 fedora-43 dnf5[637]: Complete!
[...]
čen 30 16:03:18 fedora-43 systemd[1]: dnf5-offline-transaction.service: Main process exited, code=exited, status=1/FAILURE
but that's a different issue rpm-software-management#2327).
We need to call server methods without blocking the current thread. Otherwise, the dnf5daemon-client cannot handle signals reporting progress of repository/package downloading and RPM transactions. Unfortunately, the repository key import confirmation workflow also depends on signal handling. Curiously, blocking calls used to work with sdbus-cpp v1, but with v2 any signal handler was blocked until the call finished.
The POT files are actually generated by GitHub and pushed to dnf5-l10n and the translations are commited into this repository.
Add the base package dependency for the dnf5daemon-server-polkit subpackage to ensure that dnf5daemon-server also gets updated when the polkit rule changes. Resolves: rpm-software-management#2313
…wnload() The compiler decides when the variables are freed at the end of the function and when the `user_data` is freed before the `downloader`, then the `downloader` uses just freed `user_data` in the callbacks. The free order can be different when the function is left due to an exception. Fixes rpm-software-management#2015
The purpose is to enable multithread support in python. With the %thread directive, the Python GIL is released before entering the methods from repo_sack.hpp and acquired when exiting them. In case the methods are called in a thread, another thread is not blocked by it. This is especially important in case of loading repositories which can take a lot of time.
The `--from-repo` argument allows the user to run distro-sync on packages in the specified repositories. However, any dependency resolution takes into account packages from all allowed repositories. Multiple repository ids can be specified, separated by commas, and globs can be used. Usage is similar to the `install` command.
When we want distrosync to only use specified repositories for certain packages, we shouldn't include their already installed versions. If we did, distr-sync wouldn't perform the necessary downgrades to the desired package versions, leaving the currently installed versions in place instead.
It was reported that selecting a package name from an output of "dnf5
search" command was difficult because of a colon adjacent to the
pacakge name:
$ dnf5 search dontpanic
Updating and loading repositories:
Repositories loaded.
Matched fields: name (exact)
dontpanic.i686: Very simple library and executable used in testing Alien::Base
Double-clicking on "dontpanic" word selected "dontpanic.i686:",
including the colon character. That made dificult to copy and paste
the found packages.
While this is a problem of the terminal emulator and later the reporter
found a setttings of his terminal not to do it, I think we should use
a pure white-space separator to help people with not so advanced
terminals.
This patch replaces the ": " separator with a single tabulation
character:
$ dnf5 search dontpanic
Updating and loading repositories:
Repositories loaded.
Matched fields: name (exact)
dontpanic.i686 Very simple library and executable used in testing Alien::Base
Resolves rpm-software-management#2166
Similar to autodetect_unsatisfied_installed_weak_dependencies (in fact I stole the code from there). repoclosure can give bogus failures for dependencies like (foo if bar). If nothing provides foo in the repo(s) under test, but also bar is not present, that should not be considered a repoclosure failure, but repoclosure reports it as an "unresolved dep". See: rpm-software-management/dnf-plugins-core#549 Ideally we should handle rich dependencies correctly, but I cannot see how to do that, so I'm sending this as an alternative that's at least less-bad than the current state. Signed-off-by: Adam Williamson <awilliam@redhat.com>
For: rpm-software-management#2323 Unfortunaly the errors from librepo parallel downloading API are reported only as strings (to improve that we would have to break librepo ABI). https://github.com/rpm-software-management/librepo/blob/master/librepo/metadata_downloader.h#L61-L62 This approach also works with the nested exceptions (still used by local repo loading) because we check the suffix (the inner most exception).
The actions UPGRADE_ALL, UPGRADE_ALL_MINIMAL, and DISTRO_SYNC_ALL resolve package IDs directly (they do not call additional functions). During resolution, the to_repo_ids setting was ignored. This fix ensures that the setting is now taken into account.
The actions UPGRADE_ALL, UPGRADE_ALL_MINIMAL, and DISTRO_SYNC_ALL unnecessarily processed an empty queries, creating an empty IdQueue and passing it to the functions `rpm_goal.add_upgrade` and `rpm_goal.add_distro_sync`.
This change ensures that `--from-repo` is applied only when specific packages are provided on the command line. I am preparing support for filtering from-repo in the DISTRO_SYNC_ALL, UPGRADE_ALL, and UPGRADE_ALL_MINILMAL actions in libdnf5. This adjustment in the dnf5 application ensures its behavior won't change unexpectedly once the full support is added.
If our package installation request includes a restriction to only repositories defined in `to_repo_ids`, we must retain only the corresponding installed packages, and do so before checking if the packages requested for installation are already installed.
We need to ensure repositories for requested packages are enabled. During the enabling process, a check will also be performed to verify that the requested repositories exist.
This filter can be used toimplement GoalJobsSettings::set_from_repo_ids. Within the dnf5 application, this filter will be utilized to filter installed packages using the command-line option "--installed-from-repo=".
Adds the `--installed-from-repo=` option to `dnf5 repoquery` `dnf5 list` and `dnf5 info` commands. This new filter allows users to narrow down the displayed installed packages to only those that originated from a specified repositories ID.
The `add_remove_to_goal` and `add_reinstall_to_goal` functions now properly account for `GoalJobsSettings::set_from_repo_ids`. This ensures that goal operations respect the specified repository origins.
The `add_up_down_distrosync_to_goal` function now properly takes `GoalJobsSettings::set_from_repo_ids` into account. This ensures that goal operations respect the specified repository origins.
When `from_repo_ids` is specified, we want to perform `upgrade` and `distro-sync` only for packages installed from the specified repositories. Using UPGRADE_ALL, UPGRADE_ALL_MINIMAL, and DISTROSYNC_ALL is not possible in this case.
With /bin/sh that is not bash:
test/data/build-rpms-and-repos.sh: 4: Syntax error: "(" unexpected
See the comment for more info. For: https://bugzilla.redhat.com/show_bug.cgi?id=2381859
This will reduce the number of secrets we have to manage. We already use the RSM_CI_APP_ID and RSM_CI_APP_PRIVATE_KEY in `do-release.yaml`.
Use toml::find_or to safely read file paths from temp files toml, returning an empty vector if the "files" key is missing. Resolves: rpm-software-management#1001 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2330306
In tests distinguish between toml missing the "files" key and invalid toml format. The former is expected to return an empty vector, the latter to throw an error.
…an() The operation can require root privileges, for which it can ask the user through polkit. It's okay when the operation is interactive, aka issued by a user, but when the operation is ran by a background service, then the password prompt coming out of blue is wrong. This change allows to tell the daemon server whether it can ask for the root password or not.
…transaction() The operation can require root privileges, for which it can ask the user through polkit. It's okay when the operation is interactive, aka issued by a user, but when the operation is ran by a background service, then the password prompt coming out of blue is wrong. This change allows to tell the daemon server whether it can ask for the root password or not.
…firm_key(), Repo::enable(), Repo::disable() The operation can require root privileges, for which it can ask the user through polkit. It's okay when the operation is interactive, aka issued by a user, but when the operation is ran by a background service, then the password prompt coming out of blue is wrong. This change allows to tell the daemon server whether it can ask for the root password or not.
…cancel(), Offline::clean(), Offline::set_finish_action() The operation can require root privileges, for which it can ask the user through polkit. It's okay when the operation is interactive, aka issued by a user, but when the operation is ran by a background service, then the password prompt coming out of blue is wrong. This change allows to tell the daemon server whether it can ask for the root password or not.
…em_upgrade() The operation can require root privileges, for which it can ask the user through polkit. It's okay when the operation is interactive, aka issued by a user, but when the operation is ran by a background service, then the password prompt coming out of blue is wrong. This change allows to tell the daemon server whether it can ask for the root password or not.
With the new "downloadonly" option passed to the "do_transaction" method, packages are now only downloaded to the cache, but the RPM transaction is not executed. No special permissions are required for download-only transactions.
m-blaha
force-pushed
the
install-destdir
branch
from
b43462a to
0633329
Compare
While the DownloadCB::create_signal_download() may not always have set the `user_data`, the D-Bus signal signature is not that flexible and requires to be always the same, with all the declared arguments, thus pass an empty string as the `download_id` when it's not available. Fixes rpm-software-management#2338
This enables the reuse of the code in other parts of libdnf5.
The function takes a URL-encoded string and returns it in decoded form.
The function accepts a URL parameter and returns its URL-encoded version while preserving unencoded path separators. Additionally, it can prevent the double-encoding of already encoded characters.
librepo expects that the download locations are properly URL-encoded. Unfortunately, createrepo_c leaves the URL in the location unencoded, so we need to encode it before passing it to librepo. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2325962
Instead, use interval checks to prevent incorrect behavior in certain locales.
Some doc pages were referring to dnf5-conf(5), which doesn't exist. Fix those references to point to dnf5.conf(5) instead.
Closes: rpm-software-management#2160 The `keepcache` option controls whether downloaded packages are written by `TempFilesMemory`. If they are prensent the next transaciton can find them and potentially remove them. We shouldn't automatically remove stored transactions packages.
Extends --destdir option support from upgrade and download commands to all other commands that support --downloadonly option. When --destdir is specified, --downloadonly is automatically enabled to ensure packages are downloaded to the specified directory instead of being installed. 🤖 Generated with [Claude Code](https://claude.ai/code)
Documents the newly added --destdir option support for package management commands. 🤖 Generated with [Claude Code](https://claude.ai/code)
m-blaha
force-pushed
the
install-destdir
branch
from
0633329 to
624e34b
Compare
Packages downloaded during a download-only transaction into an explicitly specified --destdir were still tracked by TempFilesMemory for cleanup after the next successful transaction. The fix introduces a check for the use of --destdir during the transaction download. When --destdir is configured, the downloaded packages will not be tracked for removal. This resolves the unexpected behavior that occurs when --destdir, --store, or --offline options are used, as all of these options override the default package download destination.
m-blaha
force-pushed
the
install-destdir
branch
from
624e34b to
5420100
Compare
Document DNF5 behavior in case the destdir option is overriden (either in the configuration or using the command line option).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
10 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extends
--destdiroption support fromupgradeanddownloadcommands to all other commands that support--downloadonlyoption. This provides consistent behavior across all package management operations.Changes
Code Implementation
--destdiroption with automatic--downloadonlyactivation--downloadonlywhen--destdiris specifiedDocumentation
Behavior
When
--destdiris specified,--downloadonlyis automatically enabled to ensure packages are downloaded to the specified directory instead of being installed. This maintains consistency with the existingupgradecommand behavior.Default Download Locations
Test Plan
--destdiroption appears in help for all relevant commands--destdirautomatically enables--downloadonlyFiles Changed
Code: 10 files
Documentation: 7 files
Total: +59 insertions, -7 deletions
🤖 Generated with Claude Code