From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Jim Porter Newsgroups: gmane.emacs.devel Subject: Re: New Git hooks for checking file names in commit messages Date: Sun, 23 Apr 2023 12:15:37 -0700 Message-ID: References: <838rel46ou.fsf@gnu.org> <865y9ptfa3.fsf@aarsen.me> <83354t44kn.fsf@gnu.org> <875y9p2h3u.fsf@thaodan.de> <83h6t82yao.fsf@gnu.org> <0a76fcd4-df69-56e0-ba90-30dc211ad56e@gmail.com> <83v8hnyvou.fsf@gnu.org> <10f5fa75-f076-228d-5ad7-1aaba8b98d47@gmail.com> <83pm7vyrp4.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="40302"; mail-complaints-to="usenet@ciao.gmane.io" Cc: bjorn.bidar@thaodan.de, arsen@aarsen.me, acm@muc.de, emacs-devel@gnu.org To: Eli Zaretskii Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Sun Apr 23 21:16:24 2023 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1pqfCB-000AJi-Qx for ged-emacs-devel@m.gmane-mx.org; Sun, 23 Apr 2023 21:16:23 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pqfBX-00014I-4F; Sun, 23 Apr 2023 15:15:43 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pqfBV-000149-Kw for emacs-devel@gnu.org; Sun, 23 Apr 2023 15:15:41 -0400 Original-Received: from mail-pg1-x530.google.com ([2607:f8b0:4864:20::530]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1pqfBU-0006tI-0K; Sun, 23 Apr 2023 15:15:41 -0400 Original-Received: by mail-pg1-x530.google.com with SMTP id 41be03b00d2f7-517bfdf55c3so2213779a12.2; Sun, 23 Apr 2023 12:15:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1682277338; x=1684869338; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:mime-version:date:message-id:from:to:cc :subject:date:message-id:reply-to; bh=nDbvcJAVjK5YFSIzu+njFWHbKjl7pnT9mbKR966uBVc=; b=PWGz7GgsUTWDSKEFr/YxFUiyh06a8WkvLXTMxA+VGlmlFEVtfJSVRU6tu3be12316E s2oxv/Q64RbfmaxlFQ1i0UNEgLGO+SnHYXWOOiS2tw5DWM5JsLYlajVzsTMxa2vSSkHr W+d+Ifp3BDJBneU18vNQoMjtWonEh0rjijDo4RCyilZI2l8aQ3yd2qe+lopUW6hR4m3B vuOwVm0CpbfGQSsI5YLeEyUvbyt7C61hOgNJj/lM1Vo6QUjCPig9sPo7Wkk3ghZaanIk eearFGHTEJv4GdqBR5is8F7P4ryiGoefcI4ptzFHiqcezjdekS2uaBMdZDTFVSr8PudD LFlw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682277338; x=1684869338; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=nDbvcJAVjK5YFSIzu+njFWHbKjl7pnT9mbKR966uBVc=; b=deFUxmSmWDrjNqJf0nOJ87wgYD/wlA6Tgi/BUbr5Yg7ql+1yFgldPiTc/H58uMV1U7 Bc7opFQo9Zv6rVqHOoYyPn0BdngeO7a7frf8pA+qtYfwWpkpFbLLSUivuc0kBOMCmxDf 8l9cAlIbu5ow/XNNfMt6dnlAsiewfT+HDJugfQ0kHEJevsSInq+rSTWn+zhmqgyvfkvg xIk1eKiN6ioHN+s6cVqrg8CligqdwrrjejsrBN9Hbsu2XumNdOPRajogSytHKbsth++U UAq9WgeF1n+EYFE+L+0TaIaFQ544ZmBEHcfNOLoaSS9DaYHQkGP8u2HsKEfEfkSvUWaL /llg== X-Gm-Message-State: AAQBX9fuX+zc+Yntnf0t7kgPiYpAztxnRE5+Ay1deK1+hKzN0eFaQ2QA 7rj0v8uNAIQuTDvgGoyozimoe6ZmWc35qg== X-Google-Smtp-Source: AKy350ao0OAZ7b/FlT22DMyoUhMoaQ/ZjLw2atQUptW1o5C0a8eelWoL+DKgRfZrrtT2/L911elGgg== X-Received: by 2002:a17:90a:7004:b0:247:1e30:5880 with SMTP id f4-20020a17090a700400b002471e305880mr10759800pjk.38.1682277337935; Sun, 23 Apr 2023 12:15:37 -0700 (PDT) Original-Received: from [192.168.1.2] (cpe-76-168-148-233.socal.res.rr.com. [76.168.148.233]) by smtp.googlemail.com with ESMTPSA id x15-20020a17090a46cf00b00247164c1947sm8801416pjg.0.2023.04.23.12.15.37 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 23 Apr 2023 12:15:37 -0700 (PDT) Content-Language: en-US In-Reply-To: <83pm7vyrp4.fsf@gnu.org> Received-SPF: pass client-ip=2607:f8b0:4864:20::530; envelope-from=jporterbugs@gmail.com; helo=mail-pg1-x530.google.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:305609 Archived-At: I've pushed a couple of fixes for the hooks (described in more detail below). I *think* this should resolve all the issues, though I'll still do more testing locally, especially with merge commits. More generally, if people find these hooks to be annoying or unhelpful, I truly won't be offended if they get backed out. I wrote them in the hopes that it will make it easier to maintain Emacs, but if they can't meet that goal, then they shouldn't stick around causing problems. On 4/23/2023 12:37 AM, Eli Zaretskii wrote: >> Date: Sun, 23 Apr 2023 00:07:49 -0700 >> Cc: bjorn.bidar@thaodan.de, arsen@aarsen.me, acm@muc.de, emacs-devel@gnu.org >> From: Jim Porter >> >> Ok, it should be easy to restrict the pre-push hook to master/emacs-NN >> branches. ... > > It's a compromise (ideally, other mistakes in file names should also > be detected), but I think it's the best we can do without too many > false positives. Ok, done. >> This part should be easy enough to handle: if a commit is a merge >> commit, we just won't validate any file names listed in the log message. > > This is not what I meant, though. If a merge-commit does have a > meaningful log message, its file names should be validated. It turns out this is actually very easy to do, now that I've learned about the "--first-parent" option in Git. Using that on a merge commit, I can get a list of all the changes that were merged into the target branch (i.e. everything from the feature branch that's now on, say, master). With this fix, commit 289b457cac1 (the merge of 'abort-redisplay') now passes the tests, as it should. >> Suppose I make commits A, B, and C on a branch, and then merge to master >> with merge-commit D. Does your message mean that the only commit that >> ends up in the ChangeLog file is commit D? (Or that commit D is the only >> one that authors.el looks at?) > > The only one that it is reasonable to validate is commit D, yes. The > other commits should not be rejected due to their log messages, at > least not ideally. I believe "--first-parent" also solves this case. In particular, by using this option, the hook correctly ignores the commits from the Eglot branch that were merged in. This means that any merge like that in the future won't run afoul of these hooks. I'll have to do a bit more testing here to try out other edge cases, but this should only result in false negatives, not false positives. (False negatives would just require some fixup when making the tarball.)