All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Jacob Vosmaer <jacob@gitlab.com>,
	gitster@pobox.com, git@vger.kernel.org, jeffhost@microsoft.com,
	jonathantanmy@google.com
Subject: Re: [PATCH v3 1/1] upload-pack.c: fix filter spec quoting bug
Date: Tue, 26 Jan 2021 16:07:37 -0500	[thread overview]
Message-ID: <YBCEmZHapifbVqRy@coredump.intra.peff.net> (raw)
In-Reply-To: <874kj46mwf.fsf@evledraar.gmail.com>

On Tue, Jan 26, 2021 at 10:57:36AM +0100, Ævar Arnfjörð Bjarmason wrote:

> > This fixes a bug that occurs when you combine partial clone and
> > uploadpack.packobjectshook. You can reproduce it as follows:
> 
> Let's: 
> 
>  * Refer to the commit we're fixing a bug in, i.e. Junio's mention of
>    10ac85c7 (upload-pack: add object filtering for partial clone,
>    2017-12-08) upthread.
> 
>  * See also "imperative-mood" in SubmittingPatches. I.e. say "Fix a bug
>    in ..." not "This fixes ... can be reproduced as"
> 
>  * uploadpack.packObjectsHook not uploadpack.packobjectshook except in C
>    code.

Generally good advice (the imperative stuff IMHO is less important
outside of the subject line, but a reasonable default way of writing).

> > The problem is an unnecessary and harmful layer of quoting. I tried
> > digging through the history of this function and I think this quoting
> > was there from the start.
> 
> 
> ...So looked at "git log" but didn't try to check out 10ac85c7 and see
> if it had the same issue? If we're going to leave a note about this at
> all probably better to help future source spelunkers by being able to
> say the issue was there from the start.

I don't think it could be tested easily at that point. It implemented
the server side, but not the client side. And later when the client side
was added, the non-hook code path was fixed. (More discussion earlier in
the thread).

-Peff

  parent reply	other threads:[~2021-01-26 22:19 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-22 14:21 [PATCH 0/1] upload-pack.c: fix filter spec quoting bug Jacob Vosmaer
2021-01-22 14:21 ` [PATCH 1/1] " Jacob Vosmaer
2021-01-22 20:32   ` Jeff King
2021-01-22 21:03     ` [PATCH] run-command: document use_shell option Jeff King
2021-01-22 21:32       ` Taylor Blau
2021-01-22 22:21       ` Junio C Hamano
2021-01-23  0:04         ` Jeff King
2021-01-22 22:10     ` [PATCH 1/1] upload-pack.c: fix filter spec quoting bug Junio C Hamano
2021-01-25 17:09     ` [PATCH v2] " Jacob Vosmaer
2021-01-25 19:48       ` Junio C Hamano
2021-01-25 21:16         ` Jeff King
2021-01-25 23:09           ` [PATCH v3 0/1] " Jacob Vosmaer
2021-01-25 23:09             ` [PATCH v3 1/1] " Jacob Vosmaer
2021-01-26  9:57               ` Ævar Arnfjörð Bjarmason
2021-01-26 10:29                 ` Jacob Vosmaer
2021-01-26 17:46                   ` Junio C Hamano
2021-01-26 21:09                   ` Jeff King
2021-01-28 16:04                     ` [PATCH v4] " Jacob Vosmaer
     [not found]                       ` <xmqqmtwsx4d9.fsf@gitster.c.googlers.com>
2021-01-28 21:12                         ` Jacob Vosmaer
2021-01-28 21:40                           ` Jacob Vosmaer
2021-01-28 21:51                           ` Jeff King
2021-02-01 20:31                             ` Jacob Vosmaer
2021-01-28 21:58                           ` Junio C Hamano
2021-02-01 20:29                             ` [PATCH v5 0/1] " Jacob Vosmaer
2021-02-01 20:29                               ` [PATCH v5 1/1] " Jacob Vosmaer
2021-02-02  5:49                               ` [PATCH v5 0/1] " Junio C Hamano
2021-02-02 10:37                                 ` [PATCH 1/1] t5544: clarify 'hook works with partial clone' test Jacob Vosmaer
2021-02-02 17:22                                   ` Eric Sunshine
2021-02-02 19:24                                     ` [PATCH v2] " Jacob Vosmaer
2021-02-02 20:21                                       ` Junio C Hamano
2021-01-26 17:51                 ` [PATCH v3 1/1] upload-pack.c: fix filter spec quoting bug Junio C Hamano
2021-01-26 21:07                 ` Jeff King [this message]
2021-01-26  0:01             ` [PATCH v3 0/1] " Junio C Hamano
2021-01-26  2:25           ` [PATCH v2] " Junio C Hamano
2021-01-25 21:16       ` Jeff King
2021-01-25 17:14     ` [PATCH 1/1] " Jacob Vosmaer
2021-01-25 17:41     ` Jacob Vosmaer

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=YBCEmZHapifbVqRy@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jacob@gitlab.com \
    --cc=jeffhost@microsoft.com \
    --cc=jonathantanmy@google.com \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.