Re: [PATCH 1/4] fast-import: add 'abort-if-invalid' mode to '--signed-commits=<mode>'

[Junio C Hamano <gitster@pobox.com>]

Justin Tobler <jltobler@gmail.com> writes:

> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> index 13621b0d6a..dcbc5bc82d 100644
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -822,6 +822,9 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
>  			die(_("encountered signed commit %s; use "
>  			      "--signed-commits=<mode> to handle it"),
>  			    oid_to_hex(&commit->object.oid));
> +		case SIGN_ABORT_IF_INVALID:
> +			die(_("'abort-if-invalid' is not a valid mode for "
> +			      "git fast-export with --signed-commits=<mode>"));
>  		case SIGN_STRIP_IF_INVALID:
>  			die(_("'strip-if-invalid' is not a valid mode for "
>  			      "git fast-export with --signed-commits=<mode>"));

There are a few similar hunks in this patch to fast-export, but I am
not sure what is going on here.

I may be misreading the code, but this error, and the similar one
for strip-if-invalid that is already there, trigger if the command
is given "--signed-commits=abort-if-invalid" on its command line,
and when we see a signed commit in the range we walk to export the
commits.  Why shouldn't the user get the error immediately while the
command is parsing the command line options?  You may be sharing the
underlying parse_sign_mode() with import side that may support more
variants, but that is not a good excuse to make these two

    git fast-export --signed-commits=abort-if-invalid
    git fast-export --signed-commits=i-dont-know-what-i-am-doing

behave completely differently, no?

You can either move these "no, these subset of options are not
available here" to fast-export.c::parse_opt_sign_mode(), or even
better yet, teach parse_sign_mode() an option to say "hey, you are
being called from fast-export, so pretend that you have never heard
of options that are only available on fast-import" and error out
right there, can't you?  Or would it be too _early_ to give errors
to users?

Thanks.