From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS3215 2.6.0.0/16 X-Spam-Status: No, score=-3.9 required=3.0 tests=AWL,BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20]) by dcvr.yhbt.net (Postfix) with ESMTP id 5331A1F54E for ; Wed, 31 Aug 2022 15:36:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231815AbiHaPgY (ORCPT ); Wed, 31 Aug 2022 11:36:24 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59368 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231282AbiHaPgW (ORCPT ); Wed, 31 Aug 2022 11:36:22 -0400 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F3F91C2775 for ; Wed, 31 Aug 2022 08:36:20 -0700 (PDT) Received: (qmail 14965 invoked by uid 109); 31 Aug 2022 15:36:20 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Wed, 31 Aug 2022 15:36:20 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 14717 invoked by uid 111); 31 Aug 2022 15:36:20 -0000 Received: from coredump.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Wed, 31 Aug 2022 11:36:20 -0400 Authentication-Results: peff.net; auth=none Date: Wed, 31 Aug 2022 11:36:19 -0400 From: Jeff King To: phillip.wood@dunelm.org.uk Cc: Junio C Hamano , Johannes Schindelin via GitGitGadget , git@vger.kernel.org, Philippe Blain , Johannes Schindelin Subject: Re: [PATCH v3 1/5] t3701: redefine what is "bogus" output of a diff filter Message-ID: References: <9261de42-3287-6ccb-6cf5-21c0a8ee1f17@gmail.com> <742b42af-a298-219d-a35f-1fa8ba985aed@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <742b42af-a298-219d-a35f-1fa8ba985aed@gmail.com> Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On Wed, Aug 31, 2022 at 10:26:17AM +0100, Phillip Wood wrote: > > Is it that all it cares about is that the output has the same number > > of lines as the input? If so, I agree that it is much less realistic, > > but it may not matter in practice. Even an "exit 0" might do ;-) > > Yes I think it only cares that there are the same number of lines which begs > the question "why are we changing this test in the first place?". The commit > message talks about the being unable to parse the hunk header but that's > only true because we would be looking in the wrong place for it - the output > does in fact contain a valid hunk header. With this change there is no hunk > header in the filtered output at all. In practice if the number of lines > differs it is normally because the filter messed with the diff header and > removed some lines from it which is what the existing test simulates. > > I'm struggling to understand the need for this change from the explanation > in the commit message as it seems to me to assume the line being deleted is > the hunk header when in fact it is deleting the diff header. FWIW, as the author of the original test, I'm also confused about why it needs to be changed. The filter I wrote in the original test was just "echo too-short". It was switched to "sed 1d" because the original did not read the input at all, which racily caused Git to see SIGPIPE: https://lore.kernel.org/git/20200113170417.GK32750@szeder.dev/ Switching to "exit 0" would bring that problem back. But I think "sed q" potentially does, too, because sed will quit without reading all of the input. We really do want something like "sed 1d" that changes the number of lines, but ensures that the filter reads to EOF. -Peff