On Wed, Jun 23, 2021 at 03:30:53PM -0700, Jonathan Tan wrote:
>
> If a full hexadecimal hash is given as a --negotiation-tip to "git
> fetch", and that hash does not correspond to an object, "git fetch" will
> segfault if --negotiate-only is given and will silently ignore that hash
> otherwise. Make these cases fatal errors, just like the case when an
> invalid ref name or abbreviated hash is given.
>
> While at it, mark the error messages as translatable.
I don't usually like this kind of "while we're at it" change, but in
this case it's very very small, so it doesn't bother me much.
>
> Signed-off-by: Jonathan Tan <jonathantanmy@xxxxxxxxxx>
> ---
> builtin/fetch.c | 4 +++-
> t/t5510-fetch.sh | 9 +++++++++
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 9191620e50..2c50465cff 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1428,7 +1428,9 @@ static void add_negotiation_tips(struct git_transport_options *smart_options)
> if (!has_glob_specials(s)) {
> struct object_id oid;
> if (get_oid(s, &oid))
> - die("%s is not a valid object", s);
> + die(_("%s is not a valid object"), s);
> + if (!has_object(the_repository, &oid, 0))
> + die(_("%s is not a valid object"), s);
Any reason not to consolidate these, e.g. if (get_oid || !has_object)?
Then we wouldn't dup the err string.
> oid_array_append(oids, &oid);
> continue;
> }
> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
> index e83b2a6506..5fc5750d8d 100755
> --- a/t/t5510-fetch.sh
> +++ b/t/t5510-fetch.sh
> @@ -1214,6 +1214,15 @@ test_expect_success '--negotiation-tip understands abbreviated SHA-1' '
> check_negotiation_tip
> '
>
> +test_expect_success '--negotiation-tip rejects missing OIDs' '
> + setup_negotiation_tip server server 0 &&
> + test_must_fail git -C client fetch \
> + --negotiation-tip=alpha_1 \
This one's okay...
> + --negotiation-tip=$(test_oid zero) \
...and this one's junk. Ok.
> + origin alpha_s beta_s 2>err &&
> + test_i18ngrep "is not a valid object" err
> +'
> +
> . "$TEST_DIRECTORY"/lib-httpd.sh
> start_httpd
>
> --
> 2.32.0.288.g62a8d224e6-goog
Other than the little nit, this seems OK to me.
- Emily
