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.

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

Print diagnostic messages on stderr, not stdout #1641

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions dnf5-plugins/automatic_plugin/automatic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,7 @@ void AutomaticCommand::pre_configure() {
}

context.set_output_stream(output_stream);
context.set_error_stream(output_stream);
}

void AutomaticCommand::configure() {
Expand Down
8 changes: 4 additions & 4 deletions dnf5-plugins/copr_plugin/copr_repo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ std::string CoprRepo::get_projectname() {


void CoprRepo::save_interactive() {
std::cout << COPR_THIRD_PARTY_WARNING;
std::cerr << COPR_THIRD_PARTY_WARNING;
if (!libdnf5::cli::utils::userconfirm::userconfirm(base->get_config()))
return;

Expand All @@ -416,9 +416,9 @@ void CoprRepo::save_interactive() {
the_list << " baseurl=" << repo.get_baseurl() << std::endl;
}

std::cout << std::endl;
std::cout << libdnf5::utils::sformat(COPR_EXTERNAL_DEPS_WARNING, the_list.str());
std::cout << std::endl;
std::cerr << std::endl;
std::cerr << libdnf5::utils::sformat(COPR_EXTERNAL_DEPS_WARNING, the_list.str());
std::cerr << std::endl;
if (!libdnf5::cli::utils::userconfirm::userconfirm(base->get_config())) {
for (auto & p : repositories) {
auto & repo = p.second;
Expand Down
2 changes: 1 addition & 1 deletion dnf5/commands/history/history_store.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ void HistoryStoreCommand::run() {
trans_file_path /= TRANSACTION_JSON;

if (std::filesystem::exists(trans_file_path)) {
std::cout << libdnf5::utils::sformat(
std::cerr << libdnf5::utils::sformat(
_("File \"{}\" already exists, it will be overwritten.\n"), trans_file_path.string());
// ask user for the file overwrite confirmation
if (!libdnf5::cli::utils::userconfirm::userconfirm(get_context().get_base().get_config())) {
Expand Down
77 changes: 48 additions & 29 deletions dnf5/context.cpp
evan-goode marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -70,18 +70,18 @@ class KeyImportRepoCB : public libdnf5::repo::RepoCallbacks {
return false;
}

std::cout << "Importing PGP key 0x" << key_info.get_short_key_id() << ":\n";
std::cerr << "Importing PGP key 0x" << key_info.get_short_key_id() << ":\n";
for (auto & user_id : key_info.get_user_ids()) {
std::cout << " Userid : \"" << user_id << "\"\n";
std::cerr << " Userid : \"" << user_id << "\"\n";
}
std::cout << " Fingerprint: " << key_info.get_fingerprint() << "\n";
std::cout << " From : " << key_info.get_url() << std::endl;
std::cerr << " Fingerprint: " << key_info.get_fingerprint() << "\n";
std::cerr << " From : " << key_info.get_url() << std::endl;

return libdnf5::cli::utils::userconfirm::userconfirm(*config);
}

void repokey_imported([[maybe_unused]] const libdnf5::rpm::KeyInfo & key_info) override {
std::cout << _("The key was successfully imported.") << std::endl;
std::cerr << _("The key was successfully imported.") << std::endl;
}

private:
Expand Down Expand Up @@ -163,9 +163,12 @@ class Context::Impl {
void set_load_available_repos(LoadAvailableRepos which) { load_available_repos = which; }
LoadAvailableRepos get_load_available_repos() const noexcept { return load_available_repos; }

void print_output(std::string_view msg) const;
void print_info(std::string_view msg) const;
void print_error(std::string_view msg) const;

void set_output_stream(std::ostream & new_output_stream) { output_stream = new_output_stream; }
void set_output_stream(std::ostream & new_output_stream) { out_stream = new_output_stream; }
void set_error_stream(std::ostream & new_error_stream) { err_stream = new_error_stream; }

void set_transaction_store_path(std::filesystem::path path) { transaction_store_path = path; }
const std::filesystem::path & get_transaction_store_path() const { return transaction_store_path; }
Expand Down Expand Up @@ -223,7 +226,8 @@ class Context::Impl {
bool show_new_leaves{false};
std::string get_cmd_line();

std::reference_wrapper<std::ostream> output_stream = std::cout;
std::reference_wrapper<std::ostream> out_stream = std::cout;
std::reference_wrapper<std::ostream> err_stream = std::cerr;

std::unique_ptr<Plugins> plugins;
std::unique_ptr<libdnf5::Goal> goal;
Expand Down Expand Up @@ -251,7 +255,7 @@ void Context::Impl::apply_repository_setopts() {
repo->get_config().opt_binds().at(key).new_string(
libdnf5::Option::Priority::COMMANDLINE, setopt.second);
} catch (const std::exception & ex) {
std::cout << "setopt: \"" + setopt.first + "." + setopt.second + "\": " + ex.what() << std::endl;
print_error("setopt: \"" + setopt.first + "." + setopt.second + "\": " + ex.what());
}
}
}
Expand Down Expand Up @@ -330,9 +334,9 @@ void Context::Impl::store_offline(libdnf5::base::Transaction & transaction) {

auto & offline_data = state.get_data();
if (offline_data.get_status() != libdnf5::offline::STATUS_DOWNLOAD_INCOMPLETE) {
std::cout << "There is already an offline transaction queued, initiated by the following command:" << std::endl
<< "\t" << offline_data.get_cmd_line() << std::endl
<< "Continuing will cancel the old offline transaction and replace it with this one." << std::endl;
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();
}
Expand Down Expand Up @@ -361,19 +365,19 @@ void Context::Impl::store_offline(libdnf5::base::Transaction & transaction) {
print_info("Testing offline transaction");
auto result = test_transaction.run();
if (result != libdnf5::base::Transaction::TransactionRunResult::SUCCESS) {
std::cerr << "Transaction failed: " << libdnf5::base::Transaction::transaction_result_to_string(result)
<< std::endl;
print_error(
fmt::format("Transaction failed: {}", libdnf5::base::Transaction::transaction_result_to_string(result)));
for (auto const & entry : transaction.get_gpg_signature_problems()) {
std::cerr << entry << std::endl;
print_error(entry);
}
for (auto & problem : test_transaction.get_transaction_problems()) {
std::cerr << " - " << problem << std::endl;
print_error(fmt::format(" - {}", problem));
}
throw libdnf5::cli::SilentCommandExitError(1);
}

for (auto const & entry : test_transaction.get_gpg_signature_problems()) {
std::cerr << entry << std::endl;
print_error(entry);
}

// Download and transaction test complete. Fill out entries in offline
Expand Down Expand Up @@ -411,9 +415,9 @@ void Context::Impl::download_and_run(libdnf5::base::Transaction & transaction) {
constexpr const char * comps_in_trans_dir{"./comps"};
auto comps_location = transaction_store_path / comps_in_trans_dir;
if (std::filesystem::exists(transaction_location)) {
std::cout << libdnf5::utils::sformat(
_("Location \"{}\" already contains a stored transaction, it will be overwritten.\n"),
transaction_store_path.string());
print_error(libdnf5::utils::sformat(
_("Location \"{}\" already contains a stored transaction, it will be overwritten."),
transaction_store_path.string()));
if (libdnf5::cli::utils::userconfirm::userconfirm(base.get_config())) {
std::filesystem::remove_all(packages_location);
std::filesystem::remove_all(comps_location);
Expand Down Expand Up @@ -448,10 +452,10 @@ void Context::Impl::download_and_run(libdnf5::base::Transaction & transaction) {

if (should_store_offline) {
store_offline(transaction);
std::cout << "Transaction stored to be performed offline. Run `dnf5 offline reboot` to reboot and run the "
"transaction. To cancel the transaction and delete the downloaded files, use `dnf5 "
"offline clean`."
<< std::endl;
print_info(
"Transaction stored to be performed offline. Run `dnf5 offline reboot` to reboot and run the "
"transaction. To cancel the transaction and delete the downloaded files, use `dnf5 "
"offline clean`.");
return;
}

Expand Down Expand Up @@ -483,19 +487,19 @@ void Context::Impl::download_and_run(libdnf5::base::Transaction & transaction) {
auto result = transaction.run();
print_info("");
if (result != libdnf5::base::Transaction::TransactionRunResult::SUCCESS) {
std::cerr << "Transaction failed: " << libdnf5::base::Transaction::transaction_result_to_string(result)
<< std::endl;
print_error(
fmt::format("Transaction failed: {}", libdnf5::base::Transaction::transaction_result_to_string(result)));
for (auto const & entry : transaction.get_gpg_signature_problems()) {
std::cerr << entry << std::endl;
print_error(entry);
}
for (auto & problem : transaction.get_transaction_problems()) {
std::cerr << " - " << problem << std::endl;
print_error(problem);
}
throw libdnf5::cli::SilentCommandExitError(1);
}

for (auto const & entry : transaction.get_gpg_signature_problems()) {
std::cerr << entry << std::endl;
print_error(entry);
}
// TODO(mblaha): print a summary of successful transaction
}
Expand All @@ -507,11 +511,17 @@ libdnf5::Goal * Context::Impl::get_goal(bool new_if_not_exist) {
return goal.get();
}

void Context::Impl::print_output(std::string_view msg) const {
out_stream.get() << msg << std::endl;
}
void Context::Impl::print_info(std::string_view msg) const {
if (!quiet) {
output_stream.get() << msg << std::endl;
err_stream.get() << msg << std::endl;
}
}
void Context::Impl::print_error(std::string_view msg) const {
err_stream.get() << msg << std::endl;
}


Context::Context(std::vector<std::unique_ptr<libdnf5::Logger>> && loggers)
Expand Down Expand Up @@ -645,12 +655,21 @@ Context::LoadAvailableRepos Context::get_load_available_repos() const noexcept {
return p_impl->get_load_available_repos();
}

void Context::print_output(std::string_view msg) const {
p_impl->print_output(msg);
}
void Context::print_info(std::string_view msg) const {
p_impl->print_info(msg);
}
void Context::print_error(std::string_view msg) const {
p_impl->print_error(msg);
}
void Context::set_output_stream(std::ostream & new_output_stream) {
p_impl->set_output_stream(new_output_stream);
}
void Context::set_error_stream(std::ostream & new_error_stream) {
p_impl->set_error_stream(new_error_stream);
}

void Context::set_transaction_store_path(std::filesystem::path path) {
p_impl->set_transaction_store_path(path);
Expand Down
2 changes: 1 addition & 1 deletion dnf5/download_callbacks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ int DownloadCallbacks::mirror_failure(void * user_cb_data, const char * msg, con
void DownloadCallbacks::reset_progress_bar() {
multi_progress_bar.reset();
if (printed) {
std::cout << std::endl;
std::cerr << std::endl;
printed = false;
}
}
Expand Down
10 changes: 9 additions & 1 deletion dnf5/include/dnf5/context.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,19 @@ class DNF_API Context : public libdnf5::cli::session::Session {
void set_load_available_repos(LoadAvailableRepos which);
LoadAvailableRepos get_load_available_repos() const noexcept;

/// If quiet mode is not active, it will print `msg` to standard output.
/// Print `msg` to the output stream, which by default is stdout.
void print_output(std::string_view msg) const;

/// If quiet mode is not active, it will print `msg` to the error stream, which by default is stderr.
void print_info(std::string_view msg) const;

/// Print `msg` to the error stream, which by default is stderr.
void print_error(std::string_view msg) const;

void set_output_stream(std::ostream & new_output_stream);

void set_error_stream(std::ostream & new_error_stream);

// When set current transaction is not executed but rather stored to the specified path.
void set_transaction_store_path(std::filesystem::path path);
const std::filesystem::path & get_transaction_store_path() const;
Expand Down
Loading
Loading