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.8 required=3.0 tests=AWL,BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,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 343B81F54E for ; Wed, 31 Aug 2022 15:49:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232181AbiHaPsb (ORCPT ); Wed, 31 Aug 2022 11:48:31 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52960 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231868AbiHaPsB (ORCPT ); Wed, 31 Aug 2022 11:48:01 -0400 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C1106491C5 for ; Wed, 31 Aug 2022 08:47:40 -0700 (PDT) Received: (qmail 15001 invoked by uid 109); 31 Aug 2022 15:47:40 -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:47:40 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 14914 invoked by uid 111); 31 Aug 2022 15:47:40 -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:47:40 -0400 Authentication-Results: peff.net; auth=none Date: Wed, 31 Aug 2022 11:47:39 -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: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On Wed, Aug 31, 2022 at 11:36:19AM -0400, Jeff King wrote: > > 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. By the way, the test change is needed for it to pass with the change in patch 2, where we become more lenient about parsing the hunk header. That implies to me that the builtin version's check for one-to-one line correspondence is broken, but we didn't notice. In the perl version, that test fails because it triggers the mismatched-output check: $ GIT_TEST_ADD_I_USE_BUILTIN=false ./t3701-add-interactive.sh --verbose-only=56 [...] fatal: mismatched output from interactive.diffFilter hint: Your filter must maintain a one-to-one correspondence hint: between its input and output lines. ok 56 - detect bogus diffFilter output but the builtin version complains about the hunk header: $ GIT_TEST_ADD_I_USE_BUILTIN=true ./t3701-add-interactive.sh --verbose-only=56 [...] error: could not parse colored hunk header '?[31m-10?[m' ok 56 - detect bogus diffFilter output Once patch 2 is applied, we stop complaining there, and we _should_ complain that the number of lines isn't the same. But we don't: $ GIT_TEST_ADD_I_USE_BUILTIN=true ./t3701-add-interactive.sh --verbose-only=56 test_must_fail: command succeeded: git add -p not ok 56 - detect bogus diffFilter output -Peff