git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 3/3] fetch: die on invalid --negotiation-tip hash

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Jul 13, 2021, 23:34 UTC
Message-ID
<878s293htx.fsf@evledraar.gmail.com>
In-Reply-To
<YO4Vc58rl2Ble31u@google.com>
On Tue, Jul 13 2021, Emily Shaffer wrote:
Show 32 quoted lines
> 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@google.com>
>> ---
>>  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.

Generally I'd agree, but aren't we explicitly conflating cases where something is a valid way no name an object v.s. being certain that such an object does not exist? I.e. this should be something like:

    if can't get_get():
        error "couldn't get the OID of revision '%s'"
    if can't look up fully-qualified OID:
        error "the OID '%s' does not exist"
Or something...
Previous: Emily ShafferNext: Jonathan Tan
Message 13 of 24 in “Push negotiation fixes”
  1. 0/3 Push negotiation fixesJonathan Tan, Jun 23, 2021
  2. 2/3 send-pack: fix push nego. when remote has refsJonathan Tan, Jun 23, 2021
  3. Emily ShafferJul 13, 2021
  4. Jonathan TanJul 14, 2021
  5. 1/3 send-pack: fix push.negotiate with remote helperJonathan Tan, Jun 23, 2021
  6. Emily ShafferJul 13, 2021
  7. Jonathan TanJul 14, 2021
  8. Ævar Arnfjörð BjarmasonJul 13, 2021
  9. Jonathan TanJul 14, 2021
  10. Ævar Arnfjörð BjarmasonJul 14, 2021
  11. 3/3 fetch: die on invalid --negotiation-tip hashJonathan Tan, Jun 23, 2021
  12. Emily ShafferJul 13, 2021
  13. Ævar Arnfjörð BjarmasonJul 13, 2021
  14. Jonathan TanJul 14, 2021
  15. Ævar Arnfjörð BjarmasonJul 14, 2021
  16. 0/3 Push negotiation fixesJonathan Tan, Jul 15, 2021
  17. 1/3 send-pack: fix push.negotiate with remote helperJonathan Tan, Jul 15, 2021
  18. Ævar Arnfjörð BjarmasonJul 27, 2021
  19. 2/3 send-pack: fix push nego. when remote has refsJonathan Tan, Jul 15, 2021
  20. Ævar Arnfjörð BjarmasonJul 27, 2021
  21. Jeff KingJul 27, 2021
  22. Jonathan TanJul 27, 2021
  23. 3/3 fetch: die on invalid --negotiation-tip hashJonathan Tan, Jul 15, 2021
  24. Junio C HamanoJul 15, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.