Skip to content

Commit

Permalink
git.c: correct NO_PARSEOPT, test --help-all
Browse files Browse the repository at this point in the history
Remove the NO_PARSEOPT flag from various built-ins which have used
parse_options() for a while, but hadn't had their listing here
updated.

Let's make sure that this doesn't drift out of sync in the future by
asserting in our tests that anything with NO_PARSEOPT will fail to
handle --git-completion-helper.

The "diff" case is special, there we handle --git-completion-helper
but not --help-all. Since the NO_PARSEOPT setting is intended for
git-completion.bash we should remove the flag in those cases. Let's
handle the edge case in our tests.

For "receive-pack" and "difftool" we've been segfaulting on
"--help-all" ever since 1b68387 (builtin/receive-pack.c: use
parse_options API, 2016-03-02) and 03831ef (difftool: implement
the functionality in the builtin, 2017-01-19). Let's mark the tests as
failing for now, and handle that in a subsequent commit.

For "fsmonitor--daemon" it has Windows/OSX-only handling of
"--git-completion-helper", so let's skip it for now.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
  • Loading branch information
avar committed Jul 19, 2022
1 parent 9466639 commit f0076de
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 10 deletions.
20 changes: 10 additions & 10 deletions git.c
Original file line number Diff line number Diff line change
Expand Up @@ -480,14 +480,14 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
static struct cmd_struct commands[] = {
{ "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
{ "am", cmd_am, RUN_SETUP | NEED_WORK_TREE },
{ "annotate", cmd_annotate, RUN_SETUP | NO_PARSEOPT },
{ "annotate", cmd_annotate, RUN_SETUP },
{ "apply", cmd_apply, RUN_SETUP_GENTLY },
{ "archive", cmd_archive, RUN_SETUP_GENTLY },
{ "bisect--helper", cmd_bisect__helper, RUN_SETUP },
{ "blame", cmd_blame, RUN_SETUP },
{ "branch", cmd_branch, RUN_SETUP | DELAY_PAGER_CONFIG },
{ "bugreport", cmd_bugreport, RUN_SETUP_GENTLY },
{ "bundle", cmd_bundle, RUN_SETUP_GENTLY | NO_PARSEOPT },
{ "bundle", cmd_bundle, RUN_SETUP_GENTLY },
{ "cat-file", cmd_cat_file, RUN_SETUP },
{ "check-attr", cmd_check_attr, RUN_SETUP },
{ "check-ignore", cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE },
Expand All @@ -505,18 +505,18 @@ static struct cmd_struct commands[] = {
{ "column", cmd_column, RUN_SETUP_GENTLY },
{ "commit", cmd_commit, RUN_SETUP | NEED_WORK_TREE },
{ "commit-graph", cmd_commit_graph, RUN_SETUP },
{ "commit-tree", cmd_commit_tree, RUN_SETUP | NO_PARSEOPT },
{ "commit-tree", cmd_commit_tree, RUN_SETUP },
{ "config", cmd_config, RUN_SETUP_GENTLY | DELAY_PAGER_CONFIG },
{ "count-objects", cmd_count_objects, RUN_SETUP },
{ "credential", cmd_credential, RUN_SETUP_GENTLY | NO_PARSEOPT },
{ "credential-cache", cmd_credential_cache },
{ "credential-cache--daemon", cmd_credential_cache_daemon },
{ "credential-store", cmd_credential_store },
{ "describe", cmd_describe, RUN_SETUP },
{ "diff", cmd_diff, NO_PARSEOPT },
{ "diff-files", cmd_diff_files, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT },
{ "diff-index", cmd_diff_index, RUN_SETUP | NO_PARSEOPT },
{ "diff-tree", cmd_diff_tree, RUN_SETUP | NO_PARSEOPT },
{ "diff", cmd_diff },
{ "diff-files", cmd_diff_files, RUN_SETUP | NEED_WORK_TREE },
{ "diff-index", cmd_diff_index, RUN_SETUP },
{ "diff-tree", cmd_diff_tree, RUN_SETUP },
{ "difftool", cmd_difftool, RUN_SETUP_GENTLY },
{ "env--helper", cmd_env__helper },
{ "fast-export", cmd_fast_export, RUN_SETUP },
Expand Down Expand Up @@ -544,7 +544,7 @@ static struct cmd_struct commands[] = {
{ "ls-files", cmd_ls_files, RUN_SETUP },
{ "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY },
{ "ls-tree", cmd_ls_tree, RUN_SETUP },
{ "mailinfo", cmd_mailinfo, RUN_SETUP_GENTLY | NO_PARSEOPT },
{ "mailinfo", cmd_mailinfo, RUN_SETUP_GENTLY },
{ "mailsplit", cmd_mailsplit, NO_PARSEOPT },
{ "maintenance", cmd_maintenance, RUN_SETUP | NO_PARSEOPT },
{ "merge", cmd_merge, RUN_SETUP | NEED_WORK_TREE },
Expand All @@ -557,7 +557,7 @@ static struct cmd_struct commands[] = {
{ "merge-recursive-theirs", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT },
{ "merge-subtree", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT },
{ "merge-tree", cmd_merge_tree, RUN_SETUP },
{ "mktag", cmd_mktag, RUN_SETUP | NO_PARSEOPT },
{ "mktag", cmd_mktag, RUN_SETUP },
{ "mktree", cmd_mktree, RUN_SETUP },
{ "multi-pack-index", cmd_multi_pack_index, RUN_SETUP },
{ "mv", cmd_mv, RUN_SETUP | NEED_WORK_TREE },
Expand Down Expand Up @@ -585,7 +585,7 @@ static struct cmd_struct commands[] = {
{ "rerere", cmd_rerere, RUN_SETUP },
{ "reset", cmd_reset, RUN_SETUP },
{ "restore", cmd_restore, RUN_SETUP | NEED_WORK_TREE },
{ "rev-list", cmd_rev_list, RUN_SETUP | NO_PARSEOPT },
{ "rev-list", cmd_rev_list, RUN_SETUP },
{ "rev-parse", cmd_rev_parse, NO_PARSEOPT },
{ "revert", cmd_revert, RUN_SETUP | NEED_WORK_TREE },
{ "rm", cmd_rm, RUN_SETUP },
Expand Down
46 changes: 46 additions & 0 deletions t/t0012-help.sh
Original file line number Diff line number Diff line change
Expand Up @@ -257,4 +257,50 @@ do
'
done <builtins

test_expect_success 'generate parseopt list' '
git --list-cmds=parseopt >parseopt
'

test_expect_success 'generate no-parseopt list' '
git --list-cmds=parseopt >parseopt &&
comm -2 -3 builtins parseopt >no-parseopt
'

while read cmd
do
result=success &&
case "$cmd" in
diff| \
difftool| \
fsmonitor--daemon|\
receive-pack)
result=failure
;;
esac &&
test_expect_$result "$cmd --help-all output" '
test_expect_code 129 git -C sub $cmd --help-all &&
test_i18ngrep usage output
'
done <parseopt

while read cmd
do
case "$cmd" in
rev-parse)
test_expect_success "$cmd has NO_PARSEOPT, treats --git-completion-helper as a normal argument" '
cat >expect <<-\EOF &&
--git-completion-helper
EOF
git $cmd --git-completion-helper >actual &&
test_cmp expect actual
'
;;
*)
test_expect_success "$cmd has NO_PARSEOPT, does not handle --git-completion-helper" '
test_must_fail git $cmd --git-completion-helper
'
;;
esac
done <no-parseopt

test_done

0 comments on commit f0076de

Please sign in to comment.