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

Jump to bottom

dnf5: Run transaction test for offline transactions #1672

Merged
merged 2 commits into from
Sep 17, 2024
Merged

dnf5: Run transaction test for offline transactions #1672

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
514d52c
dnf5: Run transaction test for offline transactions
m-blaha Sep 3, 2024
c171eb6
dnf5: Check offline transaction state before download
m-blaha Sep 5, 2024
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
72 changes: 34 additions & 38 deletions dnf5/context.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,10 +325,30 @@ void Context::Impl::load_repos(bool load_system) {
}

void Context::Impl::store_offline(libdnf5::base::Transaction & transaction) {
// Test the transaction
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scenario:

  1. a new transaction is tested
  2. get_status() of the offline transaction status is tested - if there is already an offline transaction in the queue and an AbortedbyUserError exception is thrown, the tested transaction is discarded
  3. serialize the transaction under test

Can we change the order? Test new transaction only if get_status() test passes?

  1. get_status() of offline transaction status is tested
  2. a new transaction is tested
  3. serialize the transaction under test

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point. Maybe we should do the status test even before downloading transaction packages.

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 added a commit with such optimization. Now the offline transaction status is checked before the download phase.

base.get_config().get_tsflags_option().set(libdnf5::Option::Priority::RUNTIME, "test");
print_info(_("Testing offline transaction"));
auto result = transaction.run();
if (result != libdnf5::base::Transaction::TransactionRunResult::SUCCESS) {
print_error(libdnf5::utils::sformat(
_("Transaction failed: {}"), libdnf5::base::Transaction::transaction_result_to_string(result)));
for (auto const & entry : transaction.get_gpg_signature_problems()) {
print_error(entry);
}
for (auto & problem : transaction.get_transaction_problems()) {
print_error(libdnf5::utils::sformat(_(" - {}"), problem));
}
throw libdnf5::cli::SilentCommandExitError(1);
}

for (auto const & entry : transaction.get_gpg_signature_problems()) {
print_error(entry);
}

// Serialize the transaction
const auto & installroot = base.get_config().get_installroot_option().get_value();
const auto & offline_datadir = installroot / libdnf5::offline::DEFAULT_DATADIR.relative_path();
std::filesystem::create_directories(offline_datadir);

constexpr const char * packages_in_trans_dir{"./packages"};
constexpr const char * comps_in_trans_dir{"./comps"};
const auto & comps_location = offline_datadir / comps_in_trans_dir;
Expand All @@ -337,53 +357,16 @@ void Context::Impl::store_offline(libdnf5::base::Transaction & transaction) {
libdnf5::offline::OfflineTransactionState state{state_path};

auto & offline_data = state.get_data();
if (offline_data.get_status() != libdnf5::offline::STATUS_DOWNLOAD_INCOMPLETE) {
print_error(_("There is already an offline transaction queued, initiated by the following command:"));
print_error(fmt::format("\t{}", offline_data.get_cmd_line()));
print_error(_("Continuing will cancel the old offline transaction and replace it with this one."));
if (!libdnf5::cli::utils::userconfirm::userconfirm(base.get_config())) {
throw libdnf5::cli::AbortedByUserError();
}
}

offline_data.set_status(libdnf5::offline::STATUS_DOWNLOAD_INCOMPLETE);
state.write();

// First, serialize the transaction
transaction.store_comps(comps_location);

const auto transaction_json_path = offline_datadir / TRANSACTION_JSON;
libdnf5::utils::fs::File transaction_json_file{transaction_json_path, "w"};
transaction_json_file.write(transaction.serialize(packages_in_trans_dir, comps_in_trans_dir));
transaction_json_file.close();

// Then, test the serialized transaction
const auto & goal = std::make_unique<libdnf5::Goal>(base);
goal->add_serialized_transaction(transaction_json_path);
auto test_transaction = goal->resolve();
if (test_transaction.get_problems() != libdnf5::GoalProblem::NO_PROBLEM) {
throw libdnf5::cli::GoalResolveError(transaction);
}
base.get_config().get_tsflags_option().set(libdnf5::Option::Priority::RUNTIME, "test");

print_info(_("Testing offline transaction"));
auto result = test_transaction.run();
if (result != libdnf5::base::Transaction::TransactionRunResult::SUCCESS) {
print_error(libdnf5::utils::sformat(
_("Transaction failed: {}"), libdnf5::base::Transaction::transaction_result_to_string(result)));
for (auto const & entry : transaction.get_gpg_signature_problems()) {
print_error(entry);
}
for (auto & problem : test_transaction.get_transaction_problems()) {
print_error(libdnf5::utils::sformat(_(" - {}"), problem));
}
throw libdnf5::cli::SilentCommandExitError(1);
}

for (auto const & entry : test_transaction.get_gpg_signature_problems()) {
print_error(entry);
}

// Download and transaction test complete. Fill out entries in offline
// transaction state file.
offline_data.set_status(libdnf5::offline::STATUS_DOWNLOAD_COMPLETE);
Expand Down Expand Up @@ -443,6 +426,19 @@ void Context::Impl::download_and_run(libdnf5::base::Transaction & transaction) {
const auto & installroot = base.get_config().get_installroot_option().get_value();
const auto & offline_datadir = installroot / libdnf5::offline::DEFAULT_DATADIR.relative_path();
std::filesystem::create_directories(offline_datadir);
const std::filesystem::path state_path{offline_datadir / libdnf5::offline::TRANSACTION_STATE_FILENAME};
libdnf5::offline::OfflineTransactionState state{state_path};

// Check whether there is another pending offline transaction present
auto & offline_data = state.get_data();
if (offline_data.get_status() != libdnf5::offline::STATUS_DOWNLOAD_INCOMPLETE) {
print_error(_("There is already an offline transaction queued, initiated by the following command:"));
print_error(fmt::format("\t{}", offline_data.get_cmd_line()));
print_error(_("Continuing will cancel the old offline transaction and replace it with this one."));
if (!libdnf5::cli::utils::userconfirm::userconfirm(base.get_config())) {
throw libdnf5::cli::AbortedByUserError();
}
}

base.get_config().get_destdir_option().set(offline_datadir / "packages");
transaction.set_download_local_pkgs(true);
Expand Down
Loading