Add transaction.test()
#251
Conversation
|
Please call it "test" or "dry-run" such, but not "verify". Verify is already overloaded as a term (in rpm context) and we do not need more confusion in that area. |
|
I'd say when this PR will be merged, execute_transaction function from dnf-behave-tests/dnf/steps/cmd.py should be updated. |
959a516 to
58a3a9d
Compare
|
Changed to |
58a3a9d to
2e537ff
Compare
|
@evan-goode May I ask you to resolve the merge conflicts? |
2e537ff to
2c59d63
Compare
|
Done. |
include/libdnf/base/transaction.hpp
Outdated
| @@ -81,6 +81,12 @@ class Transaction { | |||
| /// @return the transaction groups. | |||
| std::vector<libdnf::base::TransactionGroup> & get_transaction_groups() const; | |||
|
|
|||
| /// Check the transaction by running it with RPMTRANS_FLAG_TEST. This check | |||
| /// is performed automatically by run(); it is redundant to call test() | |||
| /// before calling run(). | |||
There was a problem hiding this comment.
There is one thing that deserves to be documented - import of GPG keys. If I am correct - test in DNF5 and in DNF also imports GPG keys. It is let say incorrect - test is not supposed to do permanent changes. On the other hand - we need that behavior for workflow in system upgrade and offline upgrades.
There was a problem hiding this comment.
Thanks, it's a good idea to make that explicit. Is the language correct here?
/// Check the transaction by running it with RPMTRANS_FLAG_TEST. Packages
/// will be downloaded, public keys will be installed, and transaction
/// checks will be performed, but no changes to the package set will be
/// made. These checks are performed automatically by run(); it is
/// redundant to call test() before calling run().
There was a problem hiding this comment.
I believe that this is incorrect - Packages will be downloaded. The method does not download packages.
There was a problem hiding this comment.
About public keys will be installed - this is not wrong, but it performs complex tasks related to signed rpm keys verification. First of all - the test of keys is only applied when configuration requires it. The workflow - it tries to verify signature using already imported keys in rpm-db. If it fails then it downloads keys, request approval for import (callback), than it imports keys and request the verification of the RPM signature.
install vs. import keys into RPM DB - I somehow do not prefer install.
There was a problem hiding this comment.
to the package set - what about to add installed in the statement - to the installed package set`.
There was a problem hiding this comment.
I believe that this is incorrect - Packages will be downloaded. The method does not download packages.
Thanks, yes of course, oops!
About public keys will be installed - this is not wrong...
Changed to "the import of any necessary public keys will be requested".
to the package set - what about to add installed in the statement - to the installed package set`.
Done.
3a28716 to
a34e6eb
Compare
Allows dry running a transaction using RPMTRANS_FLAG_TEST by calling
transaction.test(), regardless of what tsflags are set on the base
config. So you could run:
libdnf::Goal goal(base);
auto transaction = goal.resolve();
auto result = transaction.test()
// check result of test, possibly change the transaction
transaction.run(...)
without creating a whole new `base` with the RPMTRANS_FLAG_TEST flag
set.
a34e6eb to
6402d8a
Compare
|
LGTM |
Requires rpm-software-management/dnf5#251 Signed-off-by: Evan Goode <mail@evangoo.de>
Requires rpm-software-management/dnf5#251 Signed-off-by: Evan Goode <mail@evangoo.de>
Allows dry running a transaction using
RPMTRANS_FLAG_TESTby callingtransaction.verify(), regardless of what tsflags are set on the base config. So you could run:without creating a whole new
basewith the RPMTRANS_FLAG_TEST flag set.Example use case: https://bugzilla.redhat.com/show_bug.cgi?id=2109660