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

repolist: Implement JSON output #1522

Merged
merged 2 commits into from
Jun 5, 2024
+64 −2
Merged

repolist: Implement JSON output #1522

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
03b0b49
context: Prepare shared json option
jan-kolarik May 31, 2024
edeb153
repolist: Implement json output
jan-kolarik May 31, 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
10 changes: 9 additions & 1 deletion dnf5/commands/repo/repo_list.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ along with libdnf. If not, see <https://www.gnu.org/licenses/>.

#include "repo_list.hpp"

#include <dnf5/shared_options.hpp>
#include <libdnf5-cli/output/adapters/repo.hpp>
#include <libdnf5-cli/output/repolist.hpp>

Expand All @@ -39,6 +40,8 @@ void RepoListCommand::set_argument_parser() {
all->get_arg()->add_conflict_argument(*enabled->get_arg());
all->get_arg()->add_conflict_argument(*disabled->get_arg());
enabled->get_arg()->add_conflict_argument(*disabled->get_arg());

create_json_option(*this);
}

void RepoListCommand::run() {
Expand Down Expand Up @@ -82,7 +85,12 @@ void RepoListCommand::print(const libdnf5::repo::RepoQuery & query, bool with_st
cli_repos.emplace_back(new libdnf5::cli::output::RepoAdapter<libdnf5::repo::RepoWeakPtr>(repo));
}

libdnf5::cli::output::print_repolist_table(cli_repos, with_status, libdnf5::cli::output::COL_REPO_ID);
auto & ctx = get_context();
if (ctx.get_json_output_requested()) {
libdnf5::cli::output::print_repolist_json(cli_repos);
} else {
libdnf5::cli::output::print_repolist_table(cli_repos, with_status, libdnf5::cli::output::COL_REPO_ID);
}
}

} // namespace dnf5
12 changes: 12 additions & 0 deletions dnf5/context.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,9 @@ class Context::Impl {
void set_should_store_offline(bool should_store_offline) { this->should_store_offline = should_store_offline; }
bool get_should_store_offline() const { return should_store_offline; }

void set_json_output_requested(bool json_output) { this->json_output = json_output; }
bool get_json_output_requested() const { return json_output; }

libdnf5::Base & get_base() { return base; };

std::vector<std::pair<std::string, std::string>> & get_setopts() { return setopts; }
Expand Down Expand Up @@ -206,6 +209,7 @@ class Context::Impl {
const char * comment{nullptr};

bool should_store_offline = false;
bool json_output = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to be stored in the Context?

It seems the value is used only in the commands, to make it simpler I would store it there.
It could use libdnf5::cli::session::BoolOption, for example like advisory SecurityOption?

Copy link
Member Author

@jan-kolarik jan-kolarik Jun 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I've been already working on the follow-up repoinfo implementation and there I am calling the set_quiet(true) when json output is requested to have really just the JSON on the output, so it's also parsable. In this regard, this place is handy.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, however doesn't that have the same issue as mentioned in #1524?
That is dnf5 repo info --quiet --refresh prints REPOSYNC to stdout which would ruin the json.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, you're right this is happening, though I'd think it's suppressed by this line: https://github.com/rpm-software-management/dnf5/blob/5.2.3.0/dnf5/main.cpp#L1192. So it seems like a bug to me right now. I am still planning to do such binding to quiet there...

Copy link
Member Author

Choose a reason for hiding this comment

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

So the issue is that the ^ mentioned callbacks setup should be done after parsing the command line arguments. Then the quiet is taken into account. Tested locally and seems working 🙂

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for looking into it.


bool quiet{false};
bool dump_main_config{false};
Expand Down Expand Up @@ -658,6 +662,14 @@ bool Context::get_should_store_offline() const {
return p_impl->get_should_store_offline();
}

void Context::set_json_output_requested(bool json_output) {
p_impl->set_json_output_requested(json_output);
}

bool Context::get_json_output_requested() const {
return p_impl->get_json_output_requested();
}

libdnf5::Base & Context::get_base() {
return p_impl->get_base();
};
Expand Down
3 changes: 3 additions & 0 deletions dnf5/include/dnf5/context.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,9 @@ class Context : public libdnf5::cli::session::Session {
void set_should_store_offline(bool should_store_offline);
bool get_should_store_offline() const;

void set_json_output_requested(bool json_output);
bool get_json_output_requested() const;

libdnf5::Base & get_base();

std::vector<std::pair<std::string, std::string>> & get_setopts();
Expand Down
4 changes: 3 additions & 1 deletion dnf5/include/dnf5/shared_options.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,12 @@ void create_downloadonly_option(dnf5::Command & command);
/// The value is stored in Context::transaction_store_path.
void create_store_option(dnf5::Command & command);


/// Create the `--offline` option for a command provided as an argument.
void create_offline_option(dnf5::Command & command);

/// Create the `--json` option for a command provided as an argument.
void create_json_option(dnf5::Command & command);

} // namespace dnf5

#endif // DNF5_COMMANDS_SHARED_OPTIONS_HPP
17 changes: 17 additions & 0 deletions dnf5/shared_options.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,5 +139,22 @@ void create_store_option(dnf5::Command & command) {
});
}

void create_json_option(dnf5::Command & command) {
auto & ctx = command.get_context();
auto & parser = command.get_context().get_argument_parser();
auto json = parser.add_new_named_arg("json");
json->set_long_name("json");
json->set_description("Request json output format");
json->set_const_value("true");
json->set_parse_hook_func([&ctx](
[[maybe_unused]] libdnf5::cli::ArgumentParser::NamedArg * arg,
[[maybe_unused]] const char * option,
[[maybe_unused]] const char * value) {
ctx.set_json_output_requested(true);
return true;
});
command.get_argument_parser_command()->register_named_arg(json);
}


} // namespace dnf5
1 change: 1 addition & 0 deletions include/libdnf5-cli/output/repolist.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ namespace libdnf5::cli::output {
enum { COL_REPO_ID, COL_REPO_NAME, COL_REPO_STATUS };

void print_repolist_table(const std::vector<std::unique_ptr<IRepo>> & repos, bool with_status, size_t sort_column);
void print_repolist_json(const std::vector<std::unique_ptr<IRepo>> & repos);

} // namespace libdnf5::cli::output

Expand Down
4 changes: 4 additions & 0 deletions libdnf5-cli/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ pkg_check_modules(SMARTCOLS REQUIRED smartcols)
list(APPEND LIBDNF5_CLI_PC_REQUIRES_PRIVATE "${SMARTCOLS_MODULE_NAME}")
target_link_libraries(libdnf5-cli PRIVATE ${SMARTCOLS_LIBRARIES})

pkg_check_modules(JSONC REQUIRED json-c)
include_directories(${JSONC_INCLUDE_DIRS})
target_link_libraries(libdnf5-cli PRIVATE ${JSONC_LIBRARIES})


# sort the pkg-config requires and concatenate them into a string
list(SORT LIBDNF5_CLI_PC_REQUIRES)
Expand Down
15 changes: 15 additions & 0 deletions libdnf5-cli/output/repolist.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ along with libdnf. If not, see <https://www.gnu.org/licenses/>.

#include "libdnf5-cli/tty.hpp"

#include <json.h>
#include <libsmartcols/libsmartcols.h>

namespace libdnf5::cli::output {
Expand Down Expand Up @@ -75,4 +76,18 @@ void print_repolist_table(const std::vector<std::unique_ptr<IRepo>> & repos, boo
scols_unref_table(table);
}


void print_repolist_json([[maybe_unused]] const std::vector<std::unique_ptr<IRepo>> & repos) {
json_object * json_repos = json_object_new_array();
for (const auto & repo : repos) {
json_object * json_repo = json_object_new_object();
json_object_object_add(json_repo, "id", json_object_new_string(repo->get_id().c_str()));
json_object_object_add(json_repo, "name", json_object_new_string(repo->get_name().c_str()));
json_object_object_add(json_repo, "is_enabled", json_object_new_boolean(repo->is_enabled()));
json_object_array_add(json_repos, json_repo);
}
std::cout << json_object_to_json_string_ext(json_repos, JSON_C_TO_STRING_PRETTY) << std::endl;
json_object_put(json_repos);
}

} // namespace libdnf5::cli::output
Loading