From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x335.google.com (mail-ot1-x335.google.com [IPv6:2607:f8b0:4864:20::335]) by sourceware.org (Postfix) with ESMTPS id 84CB13857801 for ; Thu, 17 Jun 2021 01:01:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 84CB13857801 Received: by mail-ot1-x335.google.com with SMTP id w23-20020a9d5a970000b02903d0ef989477so4434958oth.9 for ; Wed, 16 Jun 2021 18:01:34 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=94lTDalmSe4Mk5xJz6gQ/GTSBN0ThrpiGl8CSsCw1DI=; b=It4zglgv/Cp4mfI+2oCHIctfzsG+UHJBPJQGLNeuetdH1/UqtsZKVzoWJjX7lfoAXw 6hneU36dv9aYiwkWwqyKas0nvC2FLAVFIoLmw38ZUF1n3SIRIUQUnVPdMzUXuFZ+fFg6 chb16mWaUR+ICeqOjwSJCfIOKvUQe9BBMyeNBTtrkh45ROZjySaR0bgz4LvzV5v9SYtL Qdpp1TWpuJ2wGA0652IGfYETwfZlkBC3u1nSJ2u64MrmCudDvIX+6UtG81opnCnZz3+t OCXl/F9NSGuaB1k8aWC5wTUVHss/ArNLA4S18ESm7stQOzy/9kLd6bwqLVir8bcHCQDT aHLw== X-Gm-Message-State: AOAM5304Nh9XCq6a2RoLN+a6HrXkA68LCbiVf/HfQQP3h574Lb0DiWes L5cwbJrrEJNL5TdEWsSQjJ8= X-Google-Smtp-Source: ABdhPJw/emRxgsJ/0ISLJItVQa1p8csIWB9+wLj73b7ILS062ihpATswuSfM67HfF27huL1r/aKLkQ== X-Received: by 2002:a9d:2034:: with SMTP id n49mr2297102ota.231.1623891693728; Wed, 16 Jun 2021 18:01:33 -0700 (PDT) Received: from [192.168.0.41] (97-118-105-195.hlrn.qwest.net. [97.118.105.195]) by smtp.gmail.com with ESMTPSA id z14sm923453oti.29.2021.06.16.18.01.31 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 16 Jun 2021 18:01:33 -0700 (PDT) Subject: Re: git gcc-commit-mklog doesn't extract PR number to ChangeLog To: Jason Merrill Cc: Hans-Peter Nilsson , Jakub Jelinek , gcc Mailing List , Jonathan Wakely References: <3228435b-aba0-6157-3266-c0f025822829@gmail.com> <5f89ddc0-aed4-2c20-0979-dfafb29046ee@gmail.com> <20210610173005.GI7746@tucnak> <20210610190941.GJ7746@tucnak> <58b63929-01f5-038c-931c-9ff8349d9f95@gmail.com> <71b4a023-efb2-6c6a-9ced-93cce7c96540@gmail.com> <013d6727-4008-b4b5-b793-c782a5ba8e10@redhat.com> From: Martin Sebor Message-ID: <5b2bace3-2f6f-ce1c-98ef-726c65686656@gmail.com> Date: Wed, 16 Jun 2021 19:01:31 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 In-Reply-To: <013d6727-4008-b4b5-b793-c782a5ba8e10@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00, BODY_8BITS, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 17 Jun 2021 01:01:36 -0000 On 6/16/21 6:40 PM, Jason Merrill wrote: > On 6/16/21 8:17 PM, Martin Sebor wrote: >> On 6/16/21 5:45 PM, Jason Merrill wrote: >>> On Wed, Jun 16, 2021 at 5:46 PM Martin Sebor >> > wrote: >>> >>>     On 6/16/21 2:49 PM, Jason Merrill wrote: >>>      > On 6/15/21 11:42 PM, Jason Merrill wrote: >>>      >> On Tue, Jun 15, 2021 at 10:04 PM Martin Sebor via Gcc >>>     >>>      >> >> wrote: >>>      >> >>>      >>     On 6/15/21 6:56 PM, Hans-Peter Nilsson wrote: >>>      >>      > On Fri, 11 Jun 2021, Martin Sebor via Gcc wrote: >>>      >>      > >>>      >>      >> On 6/11/21 11:32 AM, Jonathan Wakely wrote: >>>      >>      >>> On Fri, 11 Jun 2021 at 18:02, Martin Sebor wrote: >>>      >>      >>>> My objection is to making our policies and tools more >>>      >> restrictive >>>      >>      >>>> than they need to be.  We shouldn't expect everyone to >>>     study >>>      >> whole >>>      >>      >>>> manuals just to figure out how to successfully >>> commit a >>>      >> change (or >>>      >>      >>>> learn how to format it just the right way).  It should >>>     be easy. >>>      >>      >>> >>>      >>      >>> I agree, to some extent. But consistency is also >>> good. The >>>      >>     conventions >>>      >>      >>> for GNU ChangeLog formatting exist for a reason, and so >>>     do the >>>      >>      >>> conventions for good Git commit messages. >>>      >>      >>> >>>      >>      >>>> Setting this discussion aside for a moment and using a >>>      >> different >>>      >>      >>>> example, the commit hook rejects commit messages that >>>     don't >>>      >> start >>>      >>      >>>> ChangeLog entries with tabs.  It also rejects commit >>>      >> messages that >>>      >>      >>>> don't list all the same test files as those changed by >>>     the >>>      >> commit >>>      >>      >>>> (and probably some others as well).  That's in my view >>>      >> unnecessary >>>      >>      >>>> when the hook could just replace the leading spaces >>> with >>>      >> tabs and >>>      >>      >>>> automatically mention all the tests. >>>      >>      >>>> >>>      >>      >>>> I see this proposal as heading in the same direction. >>>      >> Rather than >>>      >>      >>>> making the script fix things up if we get them wrong >>>     it would >>>      >>     reject >>>      >>      >>>> the commit, requiring the user to massage the >>> ChangeLog by >>>      >>     hand into >>>      >>      >>>> an unnecessarily rigid format. >>>      >>      >>> >>>      >>      >>> You cannot "fix things up" in a server-side receive >>> hook, >>>      >> because >>>      >>      >>> changing the commit message would alter the commit >>>     hash, which >>>      >>     would >>>      >>      >>> require the committer to do a rebase to proceed. That >>>     breaks the >>>      >>      >>> expected behaviour and workflow of a git repo. >>>      >>      >>> >>>      >>      >>> You can use the scripts on the client side to verify >>>     your commit >>>      >>      >>> message before pushing, so you don't have to be >>> surprised >>>      >> when the >>>      >>      >>> server rejects it. >>>      >>      >> >>>      >>      >> That sounds like a killer argument.  Do we have shared >>>      >> client-side >>>      >>      >> scripts that could fix things up for us, or are we each >>>     on our >>>      >> own >>>      >>      >> to write them? >>>      >>      > >>>      >>      > I hope I got your view wrong.  If not: the "scripts >>> fixing >>>      >>      > things up for us" direction is flawed (compared to the >>>     "scripts >>>      >>      > rejecting bad formats"), unless offered as a non-default >>>     option; >>>      >>      > please don't proceed. >>>      >>      > >>>      >>      > Why?  For one, there'll always be bugs in the scripting. >>>      >>      > Mitigate those situations: while wrongly rejecting a >>>     commit is >>>      >>      > bad, wrongly "fixing things up" is worse, as a general >>> rule. >>>      >>      > Better avoid that.  (There's probably a popular "pattern >>>     name" >>>      >>      > for what I try to describe.) >>>      >> >>>      >>     The word that comes to mind is Technophobia.  Is it wise to >>>     trust >>>      >>     compilers to transform programs from their source form into >>>      >>     executables?  What if there are bugs in either?  What about >>>     the OS? >>>      >>     The whole computer, or the Internet?  Our cars? >>> Fortunately, there's >>>      >>     more to gain than to lose by trusting automation.  If there >>>     weren't >>>      >>     human progress would be stuck sometime in the 1700's. >>>      >> >>>      >>     But we're not talking about anything anywhere that >>> sophisticated >>>      >>     here: a sed script to copy and paste a piece of text in >>>      >>     the description of a change from one place to another.  It's >>>     been >>>      >>     done a few times before with more important data than >>>     ChangeLogs. >>>      >> >>>      >> >>>      >> git gcc-commit-mklog already automates most of the process.  It >>>     could >>>      >> also automate adding [PRxxxxx] to the first line.  Is that what >>>     you're >>>      >> asking for? >>>      > >>>      > Like, say: >>> >>>     I don't think this solves the problem Xionghu Luo was asking about: >>>     https://gcc.gnu.org/pipermail/gcc/2021-June/236346.html >>> >>> >>> Indeed, their problem was not mentioning the PR in the testcase, >>> which a script isn't going to fix. >>> >>>     i.e., they did have a [PRnnnn] in the one line subject but not in >>>     their ChangeLog entries.  It also not clear if they used mklog.py >>>     at all.  IME, mklog.py already puts in a [PRnnnn] near the top of >>>     a patch if it finds in one of the tests.  Though it doesn't seem >>>     to put in the ChangeLog entries.  Odd. >>> >>> >>> It currently puts in >>> >>>   PR comp/nnnnn >>> >>> at the beginning of the ChangeLog entries; it used to put them in the >>> entries for each ChangeLog file, but that changed in r12-771.  My >>> patch also adds the [PRnnnn] to the subject line. >> >> To say I'm not good at Python would be an understatement but I hacked >> up the attached patch that: >> >> 1) extracts PR numbers from test names, >> 2) gets the component for each PR from Bugzilla, > > That seems useful for testcases like the OP's that put the PR number in > the filename rather than a comment.  Maybe submit it as a patch? Will do. > >> 3) adds the PR component/nnnnn to each ChangeLog > > This would be reverting the r12-771 change, which seems both unrelated > and undesirable. Now I'm confused. Isn't that just what caused the problem to begin with? (The bug not being updated with the commit because it's not in the ChangeLog entries?) > >> Running the hacked up script with -p on the patch for PR 100085 prints >> the following: >> >> Resolves: >> PR 100085/target - Bad code for union transfer from __float128 to >> vector types > > You have the number and component switched. Doh! > >>> We could do various cleanup in the 'commit-msg' hook on the user >>> side. Or, probably better, git gcc-verify could clean up some of the >>> issues it finds. >> >> I'm not familiar with these.  Where should I look for them to learn >> how to do this? > > We currently have no commit-msg hook; see git help hooks for a > description, or .git/hooks/commit-msg.sample for an example hook. > > git gcc-verify is an alias to run > contrib/gcc-changelog/git_check_commit.py , which is the same checker > that runs on the server.  I don't know how hard it would be to adjust it > to have a fixup mode as well, for when it is run from git gcc-verify. Thanks, I'll see if I can find some time to look at this. Martin > > Jason >