From: Thomas Bertschinger Date: Wed, 21 Dec 2022 16:52:50 +0000 (-0500) Subject: LU-16392 utils: use --list-commands for bash completion X-Git-Url: https://git.whamcloud.com/?a=commitdiff_plain;h=5329631e93e999a60dfc6d99bc7224b061e21550;p=fs%2Flustre-release.git LU-16392 utils: use --list-commands for bash completion The CLI utils lctl and lfs currently use a pseudo option --non-existent-option to generate a list of completions. However, this was broken when the help output for an invalid command was changed. Using --list-commands instead means that the format of the help output can be kept succinct. However, currently there are 2 issues that make --list-commands unsuitable. First, --list-commands truncates long commands. This commit resolves this by not truncating long commands, and removing the fixed-length char buffer and writing directly to stdout so that the line length can overflow slightly if needed. Second, --list-commands recursively displays sub-commands. For example, for `lctl`, it will display `pcc add`, `pcc del`, etc in additon to just `pcc`. The bash completion tools would view these as separate tokens and thus would inappropriately suggest `add`, `del`, etc. as completions for `lctl`. This commit removes the recursive behavior. Removing the recursive behavior resolves an unrelated bug with the recursion that can be observed for `lctl`, where a number of top-level commands are skipped following recursion into a previous sub-command, equal to the number of subcommands processed in the recursive call. Specifically, the commands in the section "device setup", e.g. `attach`, `detach`, were not displayed following the recursive call into `pcc`. Finally, this commit changes the command parser to recognize --help and print the list of commands when this argument is seen. Lustre-change: https://review.whamcloud.com/49484 Lustre-commit: b4cc570ad11c1c07a6e1d825787ccc62c1245ca1 Fixes: bc69a8d058 ("LU-8621 utils: cmd help to stdout or short cmd error") Signed-off-by: Thomas Bertschinger Change-Id: Ib6e139402b9cd18e5a54b8fd3d6a2652d301e736 Reviewed-by: Andreas Dilger Reviewed-by: Vitaliy Kuznetsov Reviewed-on: https://review.whamcloud.com/c/ex/lustre-release/+/53337 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Alex Deiter --- diff --git a/libcfs/include/libcfs/util/parser.h b/libcfs/include/libcfs/util/parser.h index 7bae839..fc9d42d 100644 --- a/libcfs/include/libcfs/util/parser.h +++ b/libcfs/include/libcfs/util/parser.h @@ -76,10 +76,8 @@ void Parser_ignore_errors(int ignore); /* Set the ignore errors flag */ void Parser_printhelp(char *); /* Detailed help routine */ void Parser_exit(int, char **); /* Shuts down command parser */ int Parser_execarg(int argc, char **argv, command_t cmds[]); -int execute_line(char * line); -int Parser_list_commands(const command_t *cmdlist, char *buffer, - size_t buf_size, const char *parent_cmd, - int col_start, int col_num); +int Parser_list_commands(const command_t *cmdlist, int line_len, + int col_num); /* Converts a string to an integer */ int Parser_int(char *, int *); diff --git a/libcfs/libcfs/util/parser.c b/libcfs/libcfs/util/parser.c index c5c8947..4e4b0cd 100644 --- a/libcfs/libcfs/util/parser.c +++ b/libcfs/libcfs/util/parser.c @@ -121,10 +121,21 @@ int Parser_execarg(int argc, char **argv, command_t cmds[]) fprintf(stderr, "%s\n", cmd->pc_help); return rc; } - printf("Try interactive use without arguments or use one of:\n"); - for (cmd = cmds; cmd->pc_name; cmd++) - printf("\"%s\"\n", cmd->pc_name); - printf("as argument.\n"); + /* + * handle --help here as a synonym of --list-commands so that it can be + * applied to all commands without needing to individually add a --help + * option to each command. + * This does not apply to lctl which has its own --help option because + * `lctl --list-commands` uses a unique format. + */ + if (strcmp(argv[0], "--help") == 0) { + Parser_list_commands(cmds, 80, 4); + return 0; + } + fprintf(stderr, + "%s: '%s' is not a valid command. See '%s --list-commands'.\n", + program_invocation_short_name, argv[0], + program_invocation_short_name); return -1; } @@ -539,72 +550,49 @@ void Parser_printhelp(char *cmd) /** * Parser_list_commands() - Output a list of the supported commands. * @cmdlist: Array of structures describing the commands. - * @buffer: String buffer used to temporarily store the output text. - * @buf_size: Length of the string buffer. - * @parent_cmd: When called recursively, contains the name of the parent cmd. - * @col_start: Column where printing should begin. + * @line_len: Length of output line. * @col_num: The number of commands printed in a single row. * * The commands and subcommands supported by the utility are printed, arranged - * into several columns for readability. If a command supports subcommands, the - * function is called recursively, and the name of the parent command is - * supplied so that it can be prepended to the names of the subcommands. + * into several columns for readability. * * Return: The number of items that were printed. */ -int Parser_list_commands(const command_t *cmdlist, char *buffer, - size_t buf_size, const char *parent_cmd, - int col_start, int col_num) +int Parser_list_commands(const command_t *cmdlist, int line_len, + int col_num) { - int col = col_start; int char_max; - int len; int count = 0; - int rc; + int col = 0; - if (col_start >= col_num) - return 0; + int nprinted = 0; + int offset = 0; - char_max = (buf_size - 1) / col_num; /* Reserve 1 char for NUL */ + char_max = line_len / col_num; for (; cmdlist->pc_name; cmdlist++) { if (!cmdlist->pc_func && !cmdlist->pc_sub_cmd) break; count++; - if (parent_cmd) - len = snprintf(&buffer[col * char_max], - char_max + 1, "%s %s", parent_cmd, - cmdlist->pc_name); - else - len = snprintf(&buffer[col * char_max], - char_max + 1, "%s", cmdlist->pc_name); - - /* Add trailing spaces to pad the entry to the column size */ - if (len < char_max) { - snprintf(&buffer[col * char_max] + len, - char_max - len + 1, "%*s", char_max - len, - " "); - } else { - buffer[(col + 1) * char_max - 1] = ' '; - } + + nprinted = fprintf(stdout, "%-*s ", char_max - offset - 1, + cmdlist->pc_name); + /* + * when a column is too wide, save offset so subsequent columns + * can be aligned properly + */ + offset = offset + nprinted - char_max; + offset = offset > 0 ? offset : 0; col++; if (col >= col_num) { - fprintf(stdout, "%s\n", buffer); + fprintf(stdout, "\n"); col = 0; - buffer[0] = '\0'; - } - - if (cmdlist->pc_sub_cmd) { - rc = Parser_list_commands(cmdlist->pc_sub_cmd, buffer, - buf_size, cmdlist->pc_name, - col, col_num); - col = (col + rc) % col_num; - count += rc; + offset = 0; } } - if (!parent_cmd && col != 0) - fprintf(stdout, "%s\n", buffer); + if (col != 0) + fprintf(stdout, "\n"); return count; } diff --git a/lnet/utils/lnetctl.c b/lnet/utils/lnetctl.c index 720c4fe..2528db0 100644 --- a/lnet/utils/lnetctl.c +++ b/lnet/utils/lnetctl.c @@ -2295,9 +2295,7 @@ static int jt_discover(int argc, char **argv) static int lnetctl_list_commands(int argc, char **argv) { - char buffer[81] = ""; /* 80 printable chars + terminating NUL */ - - Parser_list_commands(cmd_list, buffer, sizeof(buffer), NULL, 0, 4); + Parser_list_commands(cmd_list, 80, 4); return 0; } diff --git a/lnet/utils/lst.c b/lnet/utils/lst.c index 3509137..3ea8834 100644 --- a/lnet/utils/lst.c +++ b/lnet/utils/lst.c @@ -3289,9 +3289,7 @@ lst_initialize(void) static int lst_list_commands(int argc, char **argv) { - char buffer[81] = ""; /* 80 printable chars + terminating NUL */ - - Parser_list_commands(lst_cmdlist, buffer, sizeof(buffer), NULL, 0, 4); + Parser_list_commands(lst_cmdlist, 80, 4); return 0; } diff --git a/lustre/scripts/bash-completion/lustre b/lustre/scripts/bash-completion/lustre index 7e5c31c..51afc07 100644 --- a/lustre/scripts/bash-completion/lustre +++ b/lustre/scripts/bash-completion/lustre @@ -3,9 +3,8 @@ _lustre_cmds() local cmd="$1" local sub="$2" - # "--list-command" prints commands in columns and truncates long ones - "$cmd" "$sub" --non-existent-option | - sed -e 1d -e '$d' -e 's/"//g' -e /=/d -e /exit/d -e /quit/d + $cmd $sub --list-commands | + sed -e '/=/d' -e s/exit// -e s/quit// } _lustre_long_opts() diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh index be1f811..b066f1a 100755 --- a/lustre/tests/sanity.sh +++ b/lustre/tests/sanity.sh @@ -28087,6 +28087,30 @@ test_434() { } run_test 434 "Client should not send RPCs for security.selinux with SElinux disabled" +test_440() { + if [[ -f $LUSTRE/scripts/bash-completion/lustre ]]; then + source $LUSTRE/scripts/bash-completion/lustre + elif [[ -f /usr/share/bash-completion/completions/lustre ]]; then + source /usr/share/bash-completion/completions/lustre + else + skip "bash completion scripts not found" + fi + + local lctl_completions + local lfs_completions + + lctl_completions=$(_lustre_cmds lctl) + if [[ ! $lctl_completions =~ "get_param" ]]; then + error "lctl bash completion failed" + fi + + lfs_completions=$(_lustre_cmds lfs) + if [[ ! $lfs_completions =~ "setstripe" ]]; then + error "lfs bash completion failed" + fi +} +run_test 440 "bash completion for lfs, lctl" + test_450() { remote_ost_nodsh && skip "remote OST with nodsh" && return local mntdev diff --git a/lustre/utils/lctl.c b/lustre/utils/lctl.c index c88288e..01c50b6 100644 --- a/lustre/utils/lctl.c +++ b/lustre/utils/lctl.c @@ -77,7 +77,7 @@ command_t pccdev_cmdlist[] = { { .pc_name = "list", .pc_func = jt_pcc_list, .pc_help = "List all PCC backends on a client.\n" "usage: lctl pcc list \n" }, - { .pc_name = "list-commands", .pc_func = jt_pcc_list_commands, + { .pc_name = "--list-commands", .pc_func = jt_pcc_list_commands, .pc_help = "list commands supported by lctl pcc"}, { .pc_name = "help", .pc_func = Parser_help, .pc_help = "help" }, { .pc_name = "exit", .pc_func = Parser_quit, .pc_help = "quit" }, @@ -110,6 +110,9 @@ command_t cmdlist[] = { "print build version of this utility and exit"}, {"--list-commands", lctl_list_commands, 0, "list commands supported by this utility and exit"}, + /* help is a synonym of --list-commands */ + {"--help", lctl_list_commands, 0, + "list commands supported by this utility and exit"}, /* Network configuration commands */ {"===== network config =====", NULL, 0, "network config"}, @@ -603,10 +606,7 @@ command_t cmdlist[] = { */ static int jt_pcc_list_commands(int argc, char **argv) { - char buffer[81] = ""; - - Parser_list_commands(pccdev_cmdlist, buffer, sizeof(buffer), - NULL, 0, 4); + Parser_list_commands(pccdev_cmdlist, 80, 4); return 0; } @@ -670,7 +670,6 @@ int lctl_main(int argc, char **argv) static int lctl_list_commands(int argc, char **argv) { - char buffer[81] = ""; /* 80 printable chars + terminating NUL */ command_t *cmd; int rc; @@ -678,8 +677,7 @@ static int lctl_list_commands(int argc, char **argv) while (cmd->pc_name != NULL) { printf("\n%s\n", cmd->pc_name); /* Command category */ cmd++; - rc = Parser_list_commands(cmd, buffer, sizeof(buffer), NULL, - 0, 4); + rc = Parser_list_commands(cmd, 80, 4); cmd += rc; } diff --git a/lustre/utils/lfs.c b/lustre/utils/lfs.c index fd8e9e1..d211034 100644 --- a/lustre/utils/lfs.c +++ b/lustre/utils/lfs.c @@ -384,10 +384,9 @@ command_t mirror_cmdlist[] = { " [...]\n"}, { .pc_name = "verify", .pc_func = lfs_mirror_verify, .pc_help = "Verify mirrored file(s).\n" - "usage: lfs mirror verify " - "[--only ] " - "[--verbose|-v] [ ...]\n"}, - { .pc_name = "list-commands", .pc_func = lfs_mirror_list_commands, + "usage: lfs mirror verify [--only MIRROR_ID[,...]]\n" + "\t\t[--verbose|-v] [ ...]\n" }, + { .pc_name = "--list-commands", .pc_func = lfs_mirror_list_commands, .pc_help = "list commands supported by lfs mirror"}, { .pc_name = "help", .pc_func = Parser_help, .pc_help = "help" }, { .pc_name = "exit", .pc_func = Parser_quit, .pc_help = "quit" }, @@ -439,7 +438,7 @@ command_t pcc_cmdlist[] = { { .pc_name = "unpin", .pc_func = lfs_pcc_unpin, .pc_help = "Un-pin files so that they can be removed from PCC by lpcc_purge.\n" "usage: lfs pcc unpin [--id|-i ID] FILE ...\n"}, - { .pc_name = "list-commands", .pc_func = lfs_pcc_list_commands, + { .pc_name = "--list-commands", .pc_func = lfs_pcc_list_commands, .pc_help = "list commands supported by lfs pcc"}, { .pc_name = "help", .pc_func = Parser_help, .pc_help = "help" }, { .pc_name = "exit", .pc_func = Parser_quit, .pc_help = "quit" }, @@ -13015,10 +13014,7 @@ static int lfs_somsync(int argc, char **argv) */ static int lfs_mirror_list_commands(int argc, char **argv) { - char buffer[81] = ""; - - Parser_list_commands(mirror_cmdlist, buffer, sizeof(buffer), - NULL, 0, 4); + Parser_list_commands(mirror_cmdlist, 80, 4); return 0; } @@ -13702,10 +13698,7 @@ static int lfs_pcc_unpin(int argc, char **argv) */ static int lfs_pcc_list_commands(int argc, char **argv) { - char buffer[81] = ""; - - Parser_list_commands(pcc_cmdlist, buffer, sizeof(buffer), - NULL, 0, 4); + Parser_list_commands(pcc_cmdlist, 80, 4); return 0; } @@ -13742,9 +13735,7 @@ static int lfs_pcc(int argc, char **argv) static int lfs_list_commands(int argc, char **argv) { - char buffer[81] = ""; /* 80 printable chars + terminating NUL */ - - Parser_list_commands(cmdlist, buffer, sizeof(buffer), NULL, 0, 4); + Parser_list_commands(cmdlist, 80, 4); return 0; }