diff --git a/base/cvd/cuttlefish/common/libs/utils/flag_parser.cpp b/base/cvd/cuttlefish/common/libs/utils/flag_parser.cpp index 4ec049e3ecd..b64e206dd5d 100644 --- a/base/cvd/cuttlefish/common/libs/utils/flag_parser.cpp +++ b/base/cvd/cuttlefish/common/libs/utils/flag_parser.cpp @@ -425,10 +425,10 @@ std::ostream& operator<<(std::ostream& out, const Flag& flag) { } out << "\n"; if (flag.help_) { - out << " " << *flag.help_ << "\n"; + out << *flag.help_ << "\n"; } if (flag.getter_) { - out << " Current value: \"" << (*flag.getter_)() << "\"\n"; + out << "Current value: \"" << (*flag.getter_)() << "\"\n"; } return out; } diff --git a/base/cvd/cuttlefish/common/libs/utils/flag_parser.h b/base/cvd/cuttlefish/common/libs/utils/flag_parser.h index 1baea558ae3..836dbe26861 100644 --- a/base/cvd/cuttlefish/common/libs/utils/flag_parser.h +++ b/base/cvd/cuttlefish/common/libs/utils/flag_parser.h @@ -95,6 +95,7 @@ class Flag { Flag AddValidator(std::function()>) &&; const std::string& Name() const { return name_; } + bool operator<(const Flag& other) const { return Name() < other.Name(); } /* Examines a list of arguments, removing any matches from the list and * invoking the `Setter` for every match. Returns `false` if the callback ever diff --git a/base/cvd/cuttlefish/host/commands/cvd/cli/BUILD.bazel b/base/cvd/cuttlefish/host/commands/cvd/cli/BUILD.bazel index 4ce19f4e81f..269d16489cd 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/cli/BUILD.bazel +++ b/base/cvd/cuttlefish/host/commands/cvd/cli/BUILD.bazel @@ -40,6 +40,18 @@ cf_cc_test( ], ) +cf_cc_library( + name = "help_format", + srcs = ["help_format.cpp"], + hdrs = ["help_format.h"], + deps = [ + "//cuttlefish/common/libs/utils:flag_parser", + "@abseil-cpp//absl/log", + "@abseil-cpp//absl/log:check", + "@abseil-cpp//absl/strings", + ], +) + cf_cc_library( name = "interruptible_terminal", srcs = ["interruptible_terminal.cpp"], @@ -104,6 +116,7 @@ cf_cc_library( "//cuttlefish/common/libs/utils:subprocess", "//cuttlefish/common/libs/utils:users", "//cuttlefish/host/commands/cvd/cli:command_request", + "//cuttlefish/host/commands/cvd/cli:help_format", "//cuttlefish/host/commands/cvd/cli:types", "//cuttlefish/host/commands/cvd/cli/commands:bugreport", "//cuttlefish/host/commands/cvd/cli/commands:cache", diff --git a/base/cvd/cuttlefish/host/commands/cvd/cli/commands/BUILD.bazel b/base/cvd/cuttlefish/host/commands/cvd/cli/commands/BUILD.bazel index 1aa127cab42..e27d3cf39db 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/cli/commands/BUILD.bazel +++ b/base/cvd/cuttlefish/host/commands/cvd/cli/commands/BUILD.bazel @@ -82,9 +82,11 @@ cf_cc_library( srcs = ["command_handler.cpp"], hdrs = ["command_handler.h"], deps = [ - "//cuttlefish/common/libs/utils:contains", + "//cuttlefish/common/libs/utils:flag_parser", "//cuttlefish/host/commands/cvd/cli:command_request", + "//cuttlefish/host/commands/cvd/cli:help_format", "//cuttlefish/host/commands/cvd/cli:types", + "//cuttlefish/host/commands/cvd/cli/selector:selector_common_parser", "//cuttlefish/result", ], ) @@ -196,7 +198,7 @@ cf_cc_library( srcs = ["lint.cpp"], hdrs = ["lint.h"], deps = [ - "//cuttlefish/common/libs/utils:files", + "//cuttlefish/common/libs/utils:flag_parser", "//cuttlefish/host/commands/cvd/cli:command_request", "//cuttlefish/host/commands/cvd/cli:types", "//cuttlefish/host/commands/cvd/cli/commands:command_handler", @@ -425,12 +427,12 @@ cf_cc_library( "//cuttlefish/common/libs/utils:subprocess_managed_stdio", "//cuttlefish/common/libs/utils:users", "//cuttlefish/host/commands/cvd/cli:command_request", + "//cuttlefish/host/commands/cvd/cli:help_format", "//cuttlefish/host/commands/cvd/cli:types", "//cuttlefish/host/commands/cvd/cli:utils", "//cuttlefish/host/commands/cvd/cli/commands:command_handler", "//cuttlefish/host/commands/cvd/cli/commands:host_tool_target", "//cuttlefish/host/commands/cvd/cli/selector", - "//cuttlefish/host/commands/cvd/cli/selector:selector_common_parser", "//cuttlefish/host/commands/cvd/fetch:substitute", "//cuttlefish/host/commands/cvd/instances", "//cuttlefish/host/commands/cvd/instances:cvd_persistent_data", diff --git a/base/cvd/cuttlefish/host/commands/cvd/cli/commands/command_handler.cpp b/base/cvd/cuttlefish/host/commands/cvd/cli/commands/command_handler.cpp index f50d2be2795..fab53fcac7a 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/cli/commands/command_handler.cpp +++ b/base/cvd/cuttlefish/host/commands/cvd/cli/commands/command_handler.cpp @@ -16,8 +16,62 @@ #include "cuttlefish/host/commands/cvd/cli/commands/command_handler.h" +#include + +#include +#include +#include +#include + +#include "cuttlefish/common/libs/utils/flag_parser.h" +#include "cuttlefish/host/commands/cvd/cli/command_request.h" +#include "cuttlefish/host/commands/cvd/cli/help_format.h" +#include "cuttlefish/host/commands/cvd/cli/selector/selector_common_parser.h" +#include "cuttlefish/host/commands/cvd/cli/types.h" +#include "cuttlefish/result/result.h" + namespace cuttlefish { bool CvdCommandHandler::RequiresDeviceExists() const { return false; } +std::vector CvdCommandHandler::Description() const { return {}; } + +Result> CvdCommandHandler::Flags(const CommandRequest&) { + return {}; +} + +Result CvdCommandHandler::DetailedHelp( + const CommandRequest& request) { + std::stringstream ss; + std::vector cmd_list = CmdList(); + CF_EXPECT(!cmd_list.empty(), "Command aliases list is empty"); + + ss << "cvd " << cmd_list[0] << " - " << SummaryHelp() << "\n"; + ss << "\n"; + ss << FormatHelpText(Description()); + + std::vector flags = CF_EXPECT(Flags(request)); + // Consume flags to ensure "current value" is accurate + cvd_common::Args args = request.SubcommandArguments(); + CF_EXPECT(ConsumeFlags(flags, args)); + + selector::SelectorOptions selector_options = request.Selectors(); + if (RequiresDeviceExists()) { + // Add the common selector flags if the command supports them. This doesn't + // need to hapen before consuming as the selector flags were consumed + // already. Using the selectors from the request ensures the flags's + // "current value" is correct. + std::vector selector_flags = + selector::BuildCommonSelectorFlags(selector_options); + flags.insert(flags.end(), selector_flags.begin(), selector_flags.end()); + } + + // Make sure the flags are in alphabetical order + std::sort(flags.begin(), flags.end()); + + ss << FormatFlagsHelp(flags); + + return ss.str(); +} + } // namespace cuttlefish diff --git a/base/cvd/cuttlefish/host/commands/cvd/cli/commands/command_handler.h b/base/cvd/cuttlefish/host/commands/cvd/cli/commands/command_handler.h index 53f86d0a986..e70803354a0 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/cli/commands/command_handler.h +++ b/base/cvd/cuttlefish/host/commands/cvd/cli/commands/command_handler.h @@ -18,7 +18,9 @@ #include +#include "cuttlefish/common/libs/utils/flag_parser.h" #include "cuttlefish/host/commands/cvd/cli/command_request.h" +#include "cuttlefish/host/commands/cvd/cli/help_format.h" #include "cuttlefish/host/commands/cvd/cli/types.h" #include "cuttlefish/result/result.h" @@ -34,8 +36,11 @@ class CvdCommandHandler { // used for command help text virtual std::string SummaryHelp() const = 0; virtual bool RequiresDeviceExists() const; + virtual Result DetailedHelp(const CommandRequest&); + virtual std::vector Description() const; + virtual Result> Flags(const CommandRequest&); + virtual bool RequiresHostConfiguration() const { return true; } - virtual Result DetailedHelp(const CommandRequest&) = 0; }; } // namespace cuttlefish diff --git a/base/cvd/cuttlefish/host/commands/cvd/cli/commands/lint.cpp b/base/cvd/cuttlefish/host/commands/cvd/cli/commands/lint.cpp index c15d3440fae..565a4d319f4 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/cli/commands/lint.cpp +++ b/base/cvd/cuttlefish/host/commands/cvd/cli/commands/lint.cpp @@ -21,7 +21,7 @@ #include #include -#include "cuttlefish/common/libs/utils/files.h" +#include "cuttlefish/common/libs/utils/flag_parser.h" #include "cuttlefish/host/commands/cvd/cli/command_request.h" #include "cuttlefish/host/commands/cvd/cli/commands/command_handler.h" #include "cuttlefish/host/commands/cvd/cli/parser/load_configs_parser.h" @@ -47,8 +47,7 @@ Usage: cvd lint /path/to/input.json Result LintCommandHandler::Handle(const CommandRequest& request) { std::vector args = request.SubcommandArguments(); - auto working_directory = CurrentDirectory(); - const auto config_path = CF_EXPECT(ValidateConfig(args, working_directory)); + const auto config_path = CF_EXPECT(ValidateConfig(args)); std::cout << "Lint of flags and config \"" << config_path << "\" succeeded\n"; @@ -65,10 +64,16 @@ Result LintCommandHandler::DetailedHelp( } Result LintCommandHandler::ValidateConfig( - std::vector& args, const std::string& working_directory) { - const LoadFlags flags = CF_EXPECT(GetFlags(args, working_directory)); - CF_EXPECT(GetEnvironmentSpecification(flags)); - return flags.config_path; + std::vector& args) { + LoadFlags load_flags; + std::vector flags = BuildCvdLoadFlags(load_flags); + CF_EXPECT(ConsumeFlags(flags, args)); + CF_EXPECT( + !args.empty(), + "No arguments provided to cvd command, please provide path to json file"); + std::string& config_path = args.front(); + CF_EXPECT(GetEnvironmentSpecification(config_path, load_flags.overrides)); + return config_path; } std::unique_ptr NewLintCommand() { diff --git a/base/cvd/cuttlefish/host/commands/cvd/cli/commands/lint.h b/base/cvd/cuttlefish/host/commands/cvd/cli/commands/lint.h index 7d6e0c7c1e9..9fab5de2252 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/cli/commands/lint.h +++ b/base/cvd/cuttlefish/host/commands/cvd/cli/commands/lint.h @@ -34,8 +34,7 @@ class LintCommandHandler : public CvdCommandHandler { Result DetailedHelp(const CommandRequest& request) override; private: - Result ValidateConfig(std::vector& args, - const std::string& working_directory); + Result ValidateConfig(std::vector& args); }; std::unique_ptr NewLintCommand(); diff --git a/base/cvd/cuttlefish/host/commands/cvd/cli/commands/load_configs.cpp b/base/cvd/cuttlefish/host/commands/cvd/cli/commands/load_configs.cpp index e86f98c8c06..f7786dd8bb7 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/cli/commands/load_configs.cpp +++ b/base/cvd/cuttlefish/host/commands/cvd/cli/commands/load_configs.cpp @@ -27,10 +27,12 @@ #include "absl/log/log.h" #include "cuttlefish/common/libs/utils/files.h" +#include "cuttlefish/common/libs/utils/flag_parser.h" #include "cuttlefish/host/commands/cvd/cli/command_request.h" #include "cuttlefish/host/commands/cvd/cli/commands/command_handler.h" #include "cuttlefish/host/commands/cvd/cli/commands/fetch.h" #include "cuttlefish/host/commands/cvd/cli/commands/start.h" +#include "cuttlefish/host/commands/cvd/cli/help_format.h" #include "cuttlefish/host/commands/cvd/cli/parser/load_config.pb.h" #include "cuttlefish/host/commands/cvd/cli/parser/load_configs_parser.h" #include "cuttlefish/host/commands/cvd/cli/selector/selector_common_parser.h" @@ -52,26 +54,8 @@ namespace { constexpr char kLoadSubCmd[] = "load"; constexpr char kSummaryHelpText[] = - R"(Loads the given JSON configuration file and launches devices based on the options provided)"; + "Creates and starts an instance group from a JSON configuration file"; -constexpr char kDetailedHelpText[] = R"( -Warning: This command is deprecated, use cvd create --config_file instead. - -Usage: -cvd load [--override=:] - -Reads the fields in the JSON configuration file and translates them to corresponding start command and flags. - -Optionally fetches remote artifacts prior to launching the cuttlefish environment. - -The --override flag can be used to give new values for properties in the config file without needing to edit the file directly. Convenient for one-off invocations. -)"; - -Result GetLoadFlags(const CommandRequest& request) { - std::vector args = request.SubcommandArguments(); - auto working_directory = CurrentDirectory(); - return CF_EXPECT(GetFlags(args, working_directory)); -} Result BuildFetchCmd(const CommandRequest& request, const CvdFlags& cvd_flags) { @@ -133,9 +117,16 @@ LoadConfigsCommand::LoadConfigsCommand(InstanceManager& instance_manager) : instance_manager_(instance_manager) {} Result LoadConfigsCommand::Handle(const CommandRequest& request) { - LoadFlags load_flags = CF_EXPECT(GetLoadFlags(request)); + std::vector args = request.SubcommandArguments(); + std::vector flags = CF_EXPECT(Flags(request)); + CF_EXPECT(ConsumeFlags(flags, args)); + CF_EXPECT( + !args.empty(), + "No arguments provided to cvd command, please provide path to json file"); + std::string& config_path = args.front(); + EnvironmentSpecification env_spec = - CF_EXPECT(GetEnvironmentSpecification(load_flags)); + CF_EXPECT(GetEnvironmentSpecification(config_path, flags_.overrides)); std::mutex group_creation_mtx; // Have to use the group name because LocalInstanceGroup can't be default @@ -181,7 +172,7 @@ Result LoadConfigsCommand::Handle(const CommandRequest& request) { group_creation_mtx.lock(); // Don't use CF_EXPECT here or the mutex will be left locked. auto group_res = - CreateGroup(instance_manager_, load_flags.base_dir, env_spec); + CreateGroup(instance_manager_, flags_.base_dir, env_spec); if (group_res.ok()) { // Have to initialize the group_name variable before releasing the mutex. group_name = (*group_res).GroupName(); @@ -250,9 +241,79 @@ cvd_common::Args LoadConfigsCommand::CmdList() const { return {kLoadSubCmd}; } std::string LoadConfigsCommand::SummaryHelp() const { return kSummaryHelpText; } -Result LoadConfigsCommand::DetailedHelp( +std::vector LoadConfigsCommand::Description() const { + std::vector description; + description.emplace_back( + "This command is an alias of `cvd create --config_file= " + "[--override=:]...`, provided for convenience and backwards " + "compatibility."); + + description.emplace_back("Usage:"); + + description.emplace_back( + " cvd load [--override=:]"); + + description.emplace_back( + "Creates and starts a new instance group from a specification file. An " + "example specification file looks like:"); + + description.emplace_back(HelpParagraph::Raw(R"( { + "instances": [ + { + "name": "ins-1", + "disk": { + "default_build": "@ab/aosp-android-latest-release/aosp_cf_x86_64_only_phone-userdebug" + }, + "vm": { + "cpus": 8, + "memory_mb": 2048 + } + }, + { + "name": "ins-2", + "disk": { + "default_build": "/path/to/android/build" + } + } + ] + })")); + + description.emplace_back( + "A complete reference of the specification file format can be found in " + "https://github.com/google/android-cuttlefish/blob/main/base/cvd/" + "cuttlefish/host/commands/cvd/cli/parser/load_config.proto."); + + description.emplace_back( + "While most config file properties are self explanatory, the build " + "properties (default_build, kernel.build, bootloader.build, etc) require " + "more explanation. These properties support two types of values:"); + + description.emplace_back(HelpParagraph::Raw( + R"( - "@ab/[/[{}]]" + - "")")); + + description.emplace_back( + "If the build value starts with \"@ab\", cvd will fetch the specified " + "Android build target from the Android build servers. By default it will " + "download the cuttlefish host package archive or the images zip as " + "needed, but for more advanced use cases the file to download from the " + "server can be specified with the optional parameter in curly " + "braces. For more information on build fetching and caching operations " + "refer to `cvd help fetch`."); + + description.emplace_back( + "Alternatively, the build value may point to an absolute path (starts " + "with '/') in the filesystem where the Android source code has been " + "checked out and a Cuttlefish target has been built. This is " + "particularly useful for rapid iteration during development in " + "combination with incremental builds and the `cvd stop` and `cvd start` " + "subcommands."); + return description; +} + +Result> LoadConfigsCommand::Flags( const CommandRequest& request) { - return kDetailedHelpText; + return BuildCvdLoadFlags(flags_); } std::unique_ptr NewLoadConfigsCommand( diff --git a/base/cvd/cuttlefish/host/commands/cvd/cli/commands/load_configs.h b/base/cvd/cuttlefish/host/commands/cvd/cli/commands/load_configs.h index 99aed1213a1..5c1c46ecc01 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/cli/commands/load_configs.h +++ b/base/cvd/cuttlefish/host/commands/cvd/cli/commands/load_configs.h @@ -20,6 +20,7 @@ #include "cuttlefish/host/commands/cvd/cli/command_request.h" #include "cuttlefish/host/commands/cvd/cli/commands/command_handler.h" +#include "cuttlefish/host/commands/cvd/cli/help_format.h" #include "cuttlefish/host/commands/cvd/cli/parser/load_configs_parser.h" #include "cuttlefish/host/commands/cvd/instances/instance_manager.h" #include "cuttlefish/host/commands/cvd/instances/local_instance_group.h" @@ -35,13 +36,15 @@ class LoadConfigsCommand : public CvdCommandHandler { Result Handle(const CommandRequest& request) override; cvd_common::Args CmdList() const override; std::string SummaryHelp() const override; - Result DetailedHelp(const CommandRequest& request) override; + std::vector Description() const override; + Result> Flags(const CommandRequest&) override; private: Result LoadGroup(const CommandRequest& request, LocalInstanceGroup& group, CvdFlags cvd_flags); InstanceManager& instance_manager_; + LoadFlags flags_; }; /* diff --git a/base/cvd/cuttlefish/host/commands/cvd/cli/commands/start.cpp b/base/cvd/cuttlefish/host/commands/cvd/cli/commands/start.cpp index 307e82b10e1..a1f32dea81f 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/cli/commands/start.cpp +++ b/base/cvd/cuttlefish/host/commands/cvd/cli/commands/start.cpp @@ -30,7 +30,6 @@ #include #include #include -#include #include #include #include @@ -52,8 +51,8 @@ #include "cuttlefish/host/commands/cvd/cli/command_request.h" #include "cuttlefish/host/commands/cvd/cli/commands/command_handler.h" #include "cuttlefish/host/commands/cvd/cli/commands/host_tool_target.h" +#include "cuttlefish/host/commands/cvd/cli/help_format.h" #include "cuttlefish/host/commands/cvd/cli/selector/selector.h" -#include "cuttlefish/host/commands/cvd/cli/selector/selector_common_parser.h" #include "cuttlefish/host/commands/cvd/cli/types.h" #include "cuttlefish/host/commands/cvd/cli/utils.h" #include "cuttlefish/host/commands/cvd/fetch/substitute.h" @@ -75,18 +74,6 @@ namespace cuttlefish { namespace { -constexpr char kCommandDescription[] = - R"(The `cvd start` command applies to the instance group, not specific instances -because Cuttlefish instances in the same group must all be started (and stopped) -in unisom. The group to be started is chosen using the standar selector flags. - -Flags that modify individual instances accept a comma separated list of values. -If the number of values in one of these flags is less than the number of -instances, then the last instances in the group will default to the first value -in the list (as a result a single value provided applies to all instances). The -empty string is interpreted as a valid value, to force a particular instance to -use its intended default value the special value "unset" must be given.)"; - std::optional GetConfigPath(cvd_common::Args& args) { size_t initial_size = args.size(); std::string config_file; @@ -521,34 +508,35 @@ Result CvdStartCommandHandler::LaunchDeviceInterruptible( return {}; } -Result CvdStartCommandHandler::DetailedHelp( +std::vector CvdStartCommandHandler::Description() const { + std::vector description; + description.emplace_back( + "The `cvd start` command applies to the instance group, not specific " + "instances because Cuttlefish instances in the same group must all be " + "started (and stopped) in unisom. The group to be started is chosen " + "using the standard selector flags."); + description.emplace_back( + "Flags that modify individual instances accept a comma separated list of " + "values. If the number of values in one of these flags is less than the " + "number of instances, then the last instances in the group will default " + "to the first value in the list (as a result a single value provided " + "applies to all instances). The empty string is interpreted as a valid " + "value, to force a particular instance to use its intended default value " + "the special value \"unset\" must be given."); + return description; +} + +Result> CvdStartCommandHandler::Flags( const CommandRequest& request) { - cvd_common::Args args = request.SubcommandArguments(); std::vector own_flags = BuildOwnFlags(); - CF_EXPECT(ConsumeFlags(own_flags, args)); std::vector internal_flags = CF_EXPECT(GetCvdInternalStartFlags( request.SubcommandArguments(), cvd_common::Envs())); - selector::SelectorOptions selector_options = request.Selectors(); - std::vector selector_flags = - selector::BuildCommonSelectorFlags(selector_options); - - std::vector flags = std::move(selector_flags); - flags.insert(flags.end(), own_flags.begin(), own_flags.end()); + std::vector flags = std::move(own_flags); flags.insert(flags.end(), internal_flags.begin(), internal_flags.end()); - // Make sure the flags are in alphabetical order - std::sort(flags.begin(), flags.end(), - [](auto f1, auto f2) { return f1.Name() < f2.Name(); }); - - std::stringstream ss; - ss << SummaryHelp() << "\n\n"; - ss << kCommandDescription << "\n\n"; - for (const Flag& flag : flags) { - ss << flag << std::endl; - } - return ss.str(); + return flags; } std::vector CvdStartCommandHandler::BuildOwnFlags() { diff --git a/base/cvd/cuttlefish/host/commands/cvd/cli/commands/start.h b/base/cvd/cuttlefish/host/commands/cvd/cli/commands/start.h index 96f4b840351..8091f4306e3 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/cli/commands/start.h +++ b/base/cvd/cuttlefish/host/commands/cvd/cli/commands/start.h @@ -37,9 +37,10 @@ class CvdStartCommandHandler : public CvdCommandHandler { std::string SummaryHelp() const override { return "Start all Cuttlefish Instances in a group"; } + std::vector Description() const override; + Result> Flags(const CommandRequest&) override; bool RequiresDeviceExists() const override { return true; } - Result DetailedHelp(const CommandRequest& request) override; private: Result LaunchDevice(Command command, LocalInstanceGroup& group, diff --git a/base/cvd/cuttlefish/host/commands/cvd/cli/help_format.cpp b/base/cvd/cuttlefish/host/commands/cvd/cli/help_format.cpp new file mode 100644 index 00000000000..0e649d74c2c --- /dev/null +++ b/base/cvd/cuttlefish/host/commands/cvd/cli/help_format.cpp @@ -0,0 +1,115 @@ +/* + * Copyright (C) 2026 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "cuttlefish/host/commands/cvd/cli/help_format.h" + +#include +#include +#include + +#include "absl/log/check.h" +#include "absl/strings/str_join.h" +#include "absl/strings/str_split.h" + +#include "cuttlefish/common/libs/utils/flag_parser.h" + +namespace cuttlefish { +namespace { + +std::vector WrapAroundLine(std::string_view str, + size_t max_line_length) { + std::vector ret; + size_t total_word_sizes = 0; + std::vector line; + for (std::string_view word : absl::StrSplit(str, absl::ByAnyChar(" \t\r\n"))) { + // line.size() accounts for the spaces added when joining the words. + if (total_word_sizes + word.size() + line.size() >= max_line_length) { + // If the line is empty at this point it means the current word is too + // long, it will be added to the line anyways and printed on its own in + // the next iteration. + if (!line.empty()) { + ret.push_back(absl::StrJoin(line, " ")); + } + total_word_sizes = 0; + line.clear(); + } + total_word_sizes += word.size(); + line.push_back(word); + } + if (!line.empty()) { + // Print the last line + ret.push_back(absl::StrJoin(line, " ")); + } + return ret; +} + +std::vector GetFlagHelpMessage(const Flag& flag) { + std::stringstream ss; + ss << flag; + std::vector flag_help_lines = + absl::StrSplit(ss.str(), '\n', absl::SkipWhitespace()); + return flag_help_lines; +} + +} // namespace + +HelpParagraph HelpParagraph::Raw(std::string text) { + return HelpParagraph(std::move(text), Style::Raw); +} + +HelpParagraph::HelpParagraph(std::string text) + : text_(std::move(text)), style_(Style::Wrapped) {} +HelpParagraph::HelpParagraph(std::string text, Style style) + : text_(std::move(text)), style_(style) {} + +std::string HelpParagraph::Formatted(size_t max_line_width) const { + switch(style_) { + case Style::Wrapped: + return absl::StrJoin(WrapAroundLine(text_, max_line_width), "\n"); + case Style::Raw: + return text_; + } +} + +std::string FormatHelpText(const std::vector& text, + size_t max_line_width) { + std::stringstream ss; + for (const HelpParagraph& paragraph : text) { + // Empty line after paragraphs + ss << paragraph.Formatted(max_line_width) << "\n\n"; + } + return ss.str(); +} + +std::string FormatFlagsHelp(const std::vector& flags, + size_t max_line_width) { + std::stringstream ss; + for (const Flag& flag : flags) { + std::vector help_lines = GetFlagHelpMessage(flag); + CHECK(!help_lines.empty()) + << "Flag produced empty help message: " << flag.Name(); + for (std::string_view line : help_lines) { + for (std::string_view wrapped_line : + WrapAroundLine(line, max_line_width - 4)) { + ss << " " << wrapped_line << "\n"; + } + } + ss << "\n"; + } + return ss.str(); +} + +} // namespace cuttlefish diff --git a/base/cvd/cuttlefish/host/commands/cvd/cli/help_format.h b/base/cvd/cuttlefish/host/commands/cvd/cli/help_format.h new file mode 100644 index 00000000000..fffca57455d --- /dev/null +++ b/base/cvd/cuttlefish/host/commands/cvd/cli/help_format.h @@ -0,0 +1,63 @@ +/* + * Copyright (C) 2026 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include + +#include +#include + +#include "cuttlefish/common/libs/utils/flag_parser.h" + +namespace cuttlefish { + +constexpr size_t kDefaultMaxLineWidth = 80; + +class HelpParagraph { + public: + static HelpParagraph Raw(std::string text); + + explicit HelpParagraph(std::string); + + std::string Formatted(size_t max_line_width) const; + + private: + enum class Style { + Wrapped, + Raw, + }; + + HelpParagraph(std::string, Style style); + + std::string text_; + Style style_; +}; + +// Formats text (typically a command description) to be displayed in the +// terminal. Each string in the input is considered to be a different paragraph +// and should not contain line changes. Each paragraph will be broken into lines +// of at most max_line_width columns without splitting individual words. An +// empty line will be added after each paragraph. +std::string FormatHelpText(const std::vector& text, + size_t max_line_width = kDefaultMaxLineWidth); + +// Formats the help messages of a list of flags to be displayed in the terminal. +// The return string will contain lines of at most max_line_width characters +// except when necessary to avoid splitting a word. +std::string FormatFlagsHelp(const std::vector& flags, + size_t max_line_width = kDefaultMaxLineWidth); +} // namespace cuttlefish diff --git a/base/cvd/cuttlefish/host/commands/cvd/cli/parser/BUILD.bazel b/base/cvd/cuttlefish/host/commands/cvd/cli/parser/BUILD.bazel index 9844bff6dec..e65d56d1f7d 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/cli/parser/BUILD.bazel +++ b/base/cvd/cuttlefish/host/commands/cvd/cli/parser/BUILD.bazel @@ -89,7 +89,10 @@ cf_cc_test( name = "flags_parser_test", srcs = ["flags_parser_test.cc"], deps = [ + "//cuttlefish/common/libs/utils:flag_parser", + "//cuttlefish/host/commands/cvd/cli/parser:load_configs_parser", "//cuttlefish/host/commands/cvd/cli/parser:test_common", + "//cuttlefish/result:result_matchers", "//libbase", ], ) diff --git a/base/cvd/cuttlefish/host/commands/cvd/cli/parser/flags_parser_test.cc b/base/cvd/cuttlefish/host/commands/cvd/cli/parser/flags_parser_test.cc index 77c4f144dca..894dff6f2b9 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/cli/parser/flags_parser_test.cc +++ b/base/cvd/cuttlefish/host/commands/cvd/cli/parser/flags_parser_test.cc @@ -14,12 +14,17 @@ * limitations under the License. */ +#include +#include #include #include #include +#include "cuttlefish/common/libs/utils/flag_parser.h" +#include "cuttlefish/host/commands/cvd/cli/parser/load_configs_parser.h" #include "cuttlefish/host/commands/cvd/cli/parser/test_common.h" +#include "cuttlefish/result/result_matchers.h" namespace cuttlefish { @@ -170,4 +175,131 @@ TEST(BootFlagsParserTest, ParseNetSimFlagEnabled) { << "netsim_uwb flag is missing or wrongly formatted"; } +TEST(CvdLoadFlagsTest, CredentialSourceSetter) { + LoadFlags load_flags; + + auto flags = BuildCvdLoadFlags(load_flags); + std::vector args = {"--credential_source=foo"}; + auto result = ConsumeFlags(flags, args); + ASSERT_TRUE(result.ok()) << result.error().Trace(); + + ASSERT_EQ(load_flags.overrides.size(), 1); + EXPECT_EQ(load_flags.overrides.count("fetch.credential_source"), 1); + EXPECT_EQ(load_flags.overrides["fetch.credential_source"], "foo"); +} + +TEST(CvdLoadFlagsTest, CredentialSourceGetter) { + LoadFlags load_flags; + load_flags.overrides["fetch.credential_source"] = "bar"; + + auto flags = BuildCvdLoadFlags(load_flags); + auto flag_it = std::find_if(flags.begin(), flags.end(), [](const Flag& f) { + return f.Name() == "credential_source"; + }); + ASSERT_NE(flag_it, flags.end()); + + std::stringstream ss; + ss << *flag_it; + EXPECT_NE(ss.str().find("Current value: \"bar\""), std::string::npos) + << "Help text was: " << ss.str(); +} + +TEST(CvdLoadFlagsTest, CredentialSourceDuplicated) { + LoadFlags load_flags; + + auto flags = BuildCvdLoadFlags(load_flags); + std::vector args = {"--credential_source=first_val", + "--credential_source=second_val"}; + auto result = ConsumeFlags(flags, args); + EXPECT_THAT(result, IsError()); +} + +TEST(CvdLoadFlagsTest, ProjectIDSetter) { + LoadFlags load_flags; + + auto flags = BuildCvdLoadFlags(load_flags); + std::vector args = {"--project_id=foo"}; + auto result = ConsumeFlags(flags, args); + ASSERT_TRUE(result.ok()) << result.error().Trace(); + + ASSERT_EQ(load_flags.overrides.size(), 1); + EXPECT_EQ(load_flags.overrides.count("fetch.project_id"), 1); + EXPECT_EQ(load_flags.overrides["fetch.project_id"], "foo"); +} + +TEST(CvdLoadFlagsTest, ProjectIDGetter) { + LoadFlags load_flags; + load_flags.overrides["fetch.project_id"] = "bar"; + + auto flags = BuildCvdLoadFlags(load_flags); + auto flag_it = std::find_if(flags.begin(), flags.end(), [](const Flag& f) { + return f.Name() == "project_id"; + }); + ASSERT_NE(flag_it, flags.end()); + + std::stringstream ss; + ss << *flag_it; + EXPECT_NE(ss.str().find("Current value: \"bar\""), + std::string::npos) + << "Help text was: " << ss.str(); +} + +TEST(CvdLoadFlagsTest, ProjectIDDuplicated) { + LoadFlags load_flags; + + auto flags = BuildCvdLoadFlags(load_flags); + std::vector args = {"--project_id=first_project", + "--project_id=second_project"}; + auto result = ConsumeFlags(flags, args); + EXPECT_FALSE(result.ok()) << "Expected duplicate flag to fail"; +} + +TEST(CvdLoadFlagsTest, CredentialSourceConflictWithOverride) { + LoadFlags load_flags; + auto flags = BuildCvdLoadFlags(load_flags); + std::vector args = { + "--override=fetch.credential_source:override_val", + "--credential_source=flag_val"}; + auto result = ConsumeFlags(flags, args); + EXPECT_FALSE(result.ok()) << "Expected override followed by flag to fail"; +} + +TEST(CvdLoadFlagsTest, OverrideConflictWithCredentialSource) { + LoadFlags load_flags; + auto flags = BuildCvdLoadFlags(load_flags); + std::vector args = { + "--credential_source=flag_val", + "--override=fetch.credential_source:override_val"}; + auto result = ConsumeFlags(flags, args); + EXPECT_FALSE(result.ok()) << "Expected flag followed by override to fail"; +} + +TEST(CvdLoadFlagsTest, ProjectIDConflictWithOverride) { + LoadFlags load_flags; + auto flags = BuildCvdLoadFlags(load_flags); + std::vector args = { + "--override=fetch.project_id:override_project", + "--project_id=flag_project"}; + auto result = ConsumeFlags(flags, args); + EXPECT_FALSE(result.ok()) << "Expected override followed by flag to fail"; +} + +TEST(CvdLoadFlagsTest, OverrideConflictWithProjectID) { + LoadFlags load_flags; + auto flags = BuildCvdLoadFlags(load_flags); + std::vector args = { + "--project_id=flag_project", + "--override=fetch.project_id:override_project"}; + auto result = ConsumeFlags(flags, args); + EXPECT_FALSE(result.ok()) << "Expected flag followed by override to fail"; +} + +TEST(CvdLoadFlagsTest, DuplicateOverridesFail) { + LoadFlags load_flags; + auto flags = BuildCvdLoadFlags(load_flags); + std::vector args = {"--override=fetch.credential_source:val1", "--override=fetch.credential_source:val2"}; + auto result = ConsumeFlags(flags, args); + EXPECT_FALSE(result.ok()) << "Expected duplicate overrides to fail"; +} + } // namespace cuttlefish diff --git a/base/cvd/cuttlefish/host/commands/cvd/cli/parser/load_configs_parser.cpp b/base/cvd/cuttlefish/host/commands/cvd/cli/parser/load_configs_parser.cpp index 13becd1989b..edb452250b4 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/cli/parser/load_configs_parser.cpp +++ b/base/cvd/cuttlefish/host/commands/cvd/cli/parser/load_configs_parser.cpp @@ -23,8 +23,8 @@ #include #include #include +#include #include -#include #include #include #include @@ -70,36 +70,41 @@ bool IsLocalBuild(std::string path) { return absl::StartsWith(path, "/"); } -Flag GflagsCompatFlagOverride(const std::string& name, - std::vector& values) { +Flag GflagsCompatFlagOverride( + const std::string& name, + std::map>& overrides) { return GflagsCompatFlag(name) - .Getter([&values]() { - return absl::StrJoin(values, ",", absl::StreamFormatter()); + .Getter([&overrides]() { + std::vector formatted; + for (const auto& [k, v] : overrides) { + formatted.push_back(fmt::format("({}=\"{}\")", k, v)); + } + return absl::StrJoin(formatted, ","); }) - .Setter([&values](const FlagMatch& match) -> Result { + .Setter([&overrides](const FlagMatch& match) -> Result { size_t separator_index = match.value.find(kOverrideSeparator); CF_EXPECTF(separator_index != std::string::npos, "Unable to find separator \"{}\" in input \"{}\"", kOverrideSeparator, match.value); - auto result = - Override{.config_path = match.value.substr(0, separator_index), - .new_value = match.value.substr(separator_index + 1)}; - CF_EXPECTF(!result.config_path.empty(), + auto property_path = match.value.substr(0, separator_index); + auto new_value = match.value.substr(separator_index + 1); + CF_EXPECTF(overrides.count(property_path) == 0, + "Property \"{}\" is already overridden", property_path); + CF_EXPECTF(!property_path.empty(), "Config path before the separator \"{}\" cannot be empty in " "input \"{}\"", kOverrideSeparator, match.value); - CF_EXPECTF(!result.new_value.empty(), + CF_EXPECTF(!new_value.empty(), "New value after the separator \"{}\" cannot be empty in " "input \"{}\"", kOverrideSeparator, match.value); - CF_EXPECTF(result.config_path.front() != '.' && - result.config_path.back() != '.', + CF_EXPECTF(property_path.front() != '.' && property_path.back() != '.', "Config path \"{}\" must not start or end with dot", - result.config_path); - CF_EXPECTF(result.config_path.find("..") == std::string::npos, + property_path); + CF_EXPECTF(property_path.find("..") == std::string::npos, "Config path \"{}\" cannot contain two consecutive dots", - result.config_path); - values.emplace_back(result); + property_path); + overrides[property_path] = new_value; return {}; }); } @@ -172,29 +177,6 @@ Json::Value OverrideToJson(const std::string& key, return leaf; } -std::vector GetFlagsVector(LoadFlags& load_flags) { - std::vector flags; - flags.emplace_back( - GflagsCompatFlag("credential_source", load_flags.credential_source)); - flags.emplace_back(GflagsCompatFlag("project_id", load_flags.project_id)); - flags.emplace_back( - GflagsCompatFlag("base_directory", load_flags.base_dir) - .Help( - "Parent directory for artifacts and runtime files. Defaults to " + - CvdDir() + "/.")); - flags.emplace_back(GflagsCompatFlagOverride("override", load_flags.overrides) - .Help("Use --override=: " - "to override config values")); - return flags; -} - -void MakeAbsolute(std::string& path, const std::string& working_dir) { - if (!path.empty() && path[0] == '/') { - return; - } - path.insert(0, working_dir + "/"); -} - Result ParseJsonFile(const std::string& file_path) { CF_EXPECTF(FileExists(file_path), "Provided file \"{}\" to cvd command does not exist", file_path); @@ -227,14 +209,11 @@ std::optional GetSystemHostPath( Result GetOverriddenConfig( const std::string& config_path, - const std::vector& override_flags) { + const std::map>& override_flags) { Json::Value result = CF_EXPECT(ParseJsonFile(config_path)); - if (!override_flags.empty()) { - for (const auto& flag : override_flags) { - MergeTwoJsonObjs(result, - OverrideToJson(flag.config_path, flag.new_value)); - } + for (const auto& [key, val] : override_flags) { + MergeTwoJsonObjs(result, OverrideToJson(key, val)); } return result; @@ -266,6 +245,66 @@ EnvironmentSpecification FillEmptyInstanceNames( } // namespace +std::vector BuildCvdLoadFlags(LoadFlags& load_flags) { + std::vector flags; + flags.emplace_back( + GflagsCompatFlagOverride("override", load_flags.overrides) + .Help( + "Override or add new properties to the group specification. This " + "flag may appear multiple times to affect different properties. " + "The format is `--override=:`. " + "For example: `--override=instance.0.vm.cpus=16`.")); + flags.emplace_back( + GflagsCompatFlag("credential_source") + .Help("Source of credentials to access the Android Build Server API. " + "Can be left empty in most cases, see the help for `login` and " + "`fetch` for details.") + .Setter([&load_flags](const FlagMatch& match) -> Result { + CF_EXPECTF( + load_flags.overrides.count(kCredentialSourceOverride) == 0, + "Specifying both --override={} and the " + "--credential_source flag is not allowed.", + kCredentialSourceOverride); + load_flags.overrides.emplace(kCredentialSourceOverride, + match.value); + return {}; + }) + .Getter([&load_flags]() -> std::string { + auto it = load_flags.overrides.find(kCredentialSourceOverride); + if (it != load_flags.overrides.end()) { + return it->second; + } + return ""; + })); + flags.emplace_back( + GflagsCompatFlag("project_id") + .Help("Google Cloud Project ID for Android Build " + "Server API access and quotas.") + .Setter([&load_flags](const FlagMatch& match) -> Result { + CF_EXPECTF(load_flags.overrides.count(kProjectIDOverride) == 0, + "Specifying both --override={} and the --project_id " + "flag is not allowed.", + kProjectIDOverride); + load_flags.overrides.emplace(kProjectIDOverride, match.value); + return {}; + }) + .Getter([&load_flags]() -> std::string { + auto it = + load_flags.overrides.find(kProjectIDOverride); + if (it != load_flags.overrides.end()) { + return it->second; + } + return ""; + })); + flags.emplace_back( + GflagsCompatFlag("base_directory", load_flags.base_dir) + .Help(fmt::format( + "Parent directory for artifacts and runtime files. When not " + "provided a new directory under {}/{} will be created.", + CvdDir(), getuid()))); + return flags; +} + Result GetGroupCreationDirectories( const std::string& parent_directory, const EnvironmentSpecification& env_spec) { @@ -329,57 +368,11 @@ Result ParseCvdConfigs(const EnvironmentSpecification& env_spec, return flags; } -std::ostream& operator<<(std::ostream& out, const Override& override) { - fmt::print(out, "(config_path=\"{}\", new_value=\"{}\")", - override.config_path, override.new_value); - return out; -} - -Result GetFlags(std::vector& args, - const std::string& working_directory) { - LoadFlags load_flags; - auto flags = GetFlagsVector(load_flags); - CF_EXPECT(ConsumeFlags(flags, args)); - CF_EXPECT( - !args.empty(), - "No arguments provided to cvd command, please provide path to json file"); - - if (!load_flags.base_dir.empty()) { - MakeAbsolute(load_flags.base_dir, working_directory); - } - - load_flags.config_path = args.front(); - MakeAbsolute(load_flags.config_path, working_directory); - - if (!load_flags.credential_source.empty()) { - for (const auto& flag : load_flags.overrides) { - CF_EXPECT(!absl::StartsWith(flag.config_path, - kCredentialSourceOverride), - "Specifying both --override=fetch.credential_source and the " - "--credential_source flag is not allowed."); - } - load_flags.overrides.emplace_back( - Override{.config_path = std::string(kCredentialSourceOverride), - .new_value = load_flags.credential_source}); - } - if (!load_flags.project_id.empty()) { - for (const auto& flag : load_flags.overrides) { - CF_EXPECT( - !absl::StartsWith(flag.config_path, kProjectIDOverride), - "Specifying both --override=fetch.project_id and the " - "--project_id flag is not allowed."); - } - load_flags.overrides.emplace_back( - Override{.config_path = std::string(kProjectIDOverride), - .new_value = load_flags.project_id}); - } - return load_flags; -} - Result GetEnvironmentSpecification( - const LoadFlags& flags) { + const std::string& config_path, + const std::map>& overrides) { Json::Value json_configs = - CF_EXPECT(GetOverriddenConfig(flags.config_path, flags.overrides)); + CF_EXPECT(GetOverriddenConfig(config_path, overrides)); EnvironmentSpecification env_spec = CF_EXPECT(ValidateCfConfigs(json_configs)); diff --git a/base/cvd/cuttlefish/host/commands/cvd/cli/parser/load_configs_parser.h b/base/cvd/cuttlefish/host/commands/cvd/cli/parser/load_configs_parser.h index faefdd12e25..7a68bea7894 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/cli/parser/load_configs_parser.h +++ b/base/cvd/cuttlefish/host/commands/cvd/cli/parser/load_configs_parser.h @@ -16,7 +16,8 @@ #pragma once -#include +#include +#include #include #include @@ -35,26 +36,16 @@ struct CvdFlags { std::string target_directory; }; -struct Override { - std::string config_path; - std::string new_value; -}; - -std::ostream& operator<<(std::ostream& out, const Override& override); - struct LoadFlags { - std::vector overrides; - std::string config_path; - std::string credential_source; - std::string project_id; + std::map> overrides; std::string base_dir; }; -Result GetFlags(std::vector& args, - const std::string& working_directory); +std::vector BuildCvdLoadFlags(LoadFlags& load_flags); Result GetEnvironmentSpecification( - const LoadFlags& flags); + const std::string& config_path, + const std::map>& overrides); Result GetGroupCreationDirectories( const std::string& parent_directory,