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-Status: No, score=-2.9 required=3.0 tests=AWL,BAYES_00,BODY_8BITS, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by dcvr.yhbt.net (Postfix) with ESMTP id 62BD61F4B4 for ; Wed, 20 Jan 2021 13:44:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388858AbhATM7h (ORCPT ); Wed, 20 Jan 2021 07:59:37 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55322 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729591AbhATM3m (ORCPT ); Wed, 20 Jan 2021 07:29:42 -0500 Received: from mail-ed1-x530.google.com (mail-ed1-x530.google.com [IPv6:2a00:1450:4864:20::530]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CB775C061757 for ; Wed, 20 Jan 2021 04:29:01 -0800 (PST) Received: by mail-ed1-x530.google.com with SMTP id f1so11411935edr.12 for ; Wed, 20 Jan 2021 04:29:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=6tPkfF+fAC4z9aK8l7TYPrUaqdTYCDWiQVpvYgDKBE4=; b=nL67HWMEa4d9HgjrsOT0F5oo5/aMLpTWK/rJDGOQFko6OGUxWXwoedYX8yITlItVoO PHtD0O3cgvfK0gQMolcjSAn2vUfNyF7+K1OISBS9cDamiqk+jgVeN2YJj3/IxBciqyp0 NU0i2MGfy/tPSG9W6ghIyNOsYoeXIcK0m9ixoGYIgcE3pmmMto2Es0sQVBe+H0FbToSE Xqbzkjb2tjp2LhIQoZ6bh8LKtwPYE/KCPyoYExuL8G2aPyI6JsrE9G1n5bfB1FFSF/id iBCCKl3ZtbZ6bEy/y1WyeVBuBYqhRA+nEvS8zg/tZz4ITJ1DwFpQ+fDdORdk6QnmbAcK 1Yyg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=6tPkfF+fAC4z9aK8l7TYPrUaqdTYCDWiQVpvYgDKBE4=; b=CsFy/fqnPMdi25Yr6PSfiQLKhkxXThX2axPQMgUWCAnV0Otri5w4e2p9UWZh37KlGo iQGmV/1G7YGgM5RQhuwe7MqA68LtQA2zh/lRXqG0NpPc5yZ9Do+S+zK+ClAWeCvYbqfF G/m5qxrWrmehB/BXF/mdx0GQEJRTINY2w+gcBcWoFnhEwTa7OiPpapW+nTxQPG1pxf+U cSMwqSIBs78ugdSdoc9it4OTBraUyJya8ZRPrCD0+NrYMLi4JzEsu6vbs+YW46qMpSMU zTqQPapYnMGcjFou7G8HRRK6tcGw52DxAEq6v4DzSQmc7ntcmix1a9D/NFOzkYi3l3S0 caoA== X-Gm-Message-State: AOAM5313T7KggriUWKrLZy8oReHbpmKN36qxuXOcUgKIAUuAk4X1hnqL 8XQtTx2i9FyUOzTszBhcAumN4Kto3u8= X-Google-Smtp-Source: ABdhPJwc9DpdhhPOvnEOYTNC8omGVVNkuUjNHeFwcfus3B1trLP1F8dGpRZyMXNL7SXLrP/ln7Ajdg== X-Received: by 2002:a05:6402:4c1:: with SMTP id n1mr7156842edw.66.1611145740593; Wed, 20 Jan 2021 04:29:00 -0800 (PST) Received: from szeder.dev (92-249-246-25.pool.digikabel.hu. [92.249.246.25]) by smtp.gmail.com with ESMTPSA id l17sm844714ejc.60.2021.01.20.04.28.59 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 20 Jan 2021 04:29:00 -0800 (PST) Date: Wed, 20 Jan 2021 13:28:58 +0100 From: SZEDER =?utf-8?B?R8OhYm9y?= To: Jiang Xin Cc: Junio C Hamano , Git List , Jiang Xin Subject: Re: [PATCH v19 03/10] receive-pack: add new proc-receive hook Message-ID: <20210120122858.GZ8396@szeder.dev> References: <20200824174202.11710-1-worldhello.net@gmail.com> <20200827154551.5966-4-worldhello.net@gmail.com> <20210117222151.GY8396@szeder.dev> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On Mon, Jan 18, 2021 at 04:24:11PM +0800, Jiang Xin wrote: > SZEDER Gábor 于2021年1月18日周一 上午6:21写道： > > > > > > This patch added a whole lot of test cases, and two of them '86 - > > proc-receive: not support push options (builtin protocol)' and '95 - > > proc-receive: not support push options (builtin protocol/porcelain)' > > are prone to rare failures. > > > > On Thu, Aug 27, 2020 at 11:45:44AM -0400, Jiang Xin wrote: > > > diff --git a/t/t5411/test-0026-push-options.sh b/t/t5411/test-0026-push-options.sh > > > new file mode 100644 > > > index 0000000000..d0c4da8b23 > > > --- /dev/null > > > +++ b/t/t5411/test-0026-push-options.sh > > > > > +# Refs of upstream : master(A) > > > +# Refs of workbench: master(A) tags/v123 > > > +# git push -o ... : refs/for/master/topic > > > +test_expect_success "proc-receive: not support push options ($PROTOCOL)" ' > > > + test_must_fail git -C workbench push \ > > > + -o issue=123 \ > > > + -o reviewer=user1 \ > > > + origin \ > > > + HEAD:refs/for/master/topic \ > > > + >out 2>&1 && > > > > Three relevant things are happening here: > > > > - 'git push' is executed with its standard output and error > > redirected to the file 'out'. > > > > - 'git push' executes 'git receive-pack' internally, which inherits > > the open file descriptors, so its output and error goes into that > > same 'out' file. > > > > - 'git push' is expected to fail when it finds out that the other > > side doesn't support push options, but it does so with a simple > > die() right away, without waiting for its child 'git receive-pack' > > process to finish. > > > > > + make_user_friendly_and_stable_output actual && > > > + test_i18ngrep "fatal: the receiving end does not support push options" \ > > > + actual && > > > + git -C "$upstream" show-ref >out && > > > > Here the shell opens and truncates the file 'out' to write 'git > > show-ref's output, i.e. it is still the same 'out' file that was used > > earlier. > > > > Consequently, it is possible that 'git receive-pack' is still running, > > its open file descriptors to 'out' are still valid, and its "fatal: > > the remote end hung up unexpectedly" error message about the suddenly > > disappeared 'git push' can partially overwrite the output from 'git > > show-ref'. > > I think these are the only two tests that can cause this racy > > behavior: by instrumenting finish_command() I found that in all other > > tests where 'git push' is expected to fail it errors out gracefully > > and waits for its 'git receive-pack' child process. > > Atomic push may have the same problem. I don't think so, because send_pack() doesn't die() when a ref is rejected in an atomic push, but returns, and lets its caller terminate in an usual way, including waiting for 'git receive-pack'.