public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Junio C Hamano <gitster@pobox•com>
To: Matt McCutchen <matt@mattmccutchen•net>
Cc: git@vger•kernel.org
Subject: Re: [PATCH] fetch: print an error when declining to request an unadvertised object
Date: Mon, 20 Feb 2017 22:36:14 -0800	[thread overview]
Message-ID: <xmqqvas4gie9.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <1487470080.3570.8.camel@mattmccutchen.net> (Matt McCutchen's message of "Sat, 18 Feb 2017 20:55:08 -0500")

Matt McCutchen <matt@mattmccutchen•net> writes:

> Currently "git fetch REMOTE SHA1" silently exits 1 if the server doesn't
> allow requests for unadvertised objects by sha1.  The more common case
> of requesting a nonexistent ref normally triggers a die() in
> get_fetch_map, so "git fetch" wasn't bothering to check after the fetch
> that it got all the refs it sought, like "git fetch-pack" does near the
> end of cmd_fetch_pack.
>
> Move the code from cmd_fetch_pack to a new function,
> report_unmatched_refs, that is called by fetch_refs_via_pack as part of
> "git fetch".  Also, change filter_refs (which checks whether a request
> for an unadvertised object should be sent to the server) to set a new
> match status on the "struct ref" when the request is not allowed, and
> have report_unmatched_refs check for this status and print a special
> error message, "Server does not allow request for unadvertised object".
>
> Finally, add a simple test case for "git fetch REMOTE SHA1".
>
> Signed-off-by: Matt McCutchen <matt@mattmccutchen•net>
> ---

Hmph, I would have expected this to be done as a three-patch series,

 * move the loop at the end of cmd_fetch_pack() to a separate helper
   function report_unmatched_refs() and call it;

 * add a call to report_unmatched_refs() to the transport layer;

 * enhance report_unmatched_refs() by introducing match_status
   field and adding new code to filter_refs() to diagnose other
   kinds of errors.

The result looks reasonable from a cursory read, though.

Thanks for following it up to the completion.

next prev parent reply	other threads:[~2017-02-21  6:37 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-10 17:26 [PATCH] fetch: print an error when declining to request an unadvertised object Matt McCutchen
2017-02-10 18:36 ` Junio C Hamano
2017-02-12 21:13   ` Matt McCutchen
2017-02-12 23:49     ` Junio C Hamano
2017-02-19  1:55       ` Matt McCutchen
2017-02-21  6:36         ` Junio C Hamano [this message]
2017-02-22 16:01           ` [PATCH 1/3] fetch-pack: move code to report unmatched refs to a function Matt McCutchen
2017-02-22 17:11             ` Junio C Hamano
2017-02-22 16:01               ` [PATCH v2 " Matt McCutchen
2017-02-22 16:02               ` [PATCH v2 2/3] fetch_refs_via_pack: call report_unmatched_refs Matt McCutchen
2017-02-22 16:05               ` [PATCH v2 3/3] fetch-pack: add specific error for fetching an unadvertised object Matt McCutchen
2017-02-22 17:26               ` [PATCH 1/3] fetch-pack: move code to report unmatched refs to a function Matt McCutchen
2017-02-22 16:02           ` [PATCH 2/3] fetch_refs_via_pack: call report_unmatched_refs Matt McCutchen
2017-02-22 16:05           ` [PATCH 3/3] fetch-pack: add specific error for fetching an unadvertised object Matt McCutchen
2017-02-22 16:17           ` [PATCH] fetch: print an error when declining to request " Matt McCutchen
2017-02-22 17:07             ` Junio C Hamano
2017-02-19  2:07       ` Matt McCutchen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqvas4gie9.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=matt@mattmccutchen$(echo .)net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox