Maxim Cournoyer <[email protected]> writes: > Hi Tomas, > > Tomas Volf <[email protected]> writes: > >> Maxim Cournoyer <[email protected]> writes: >> >>> Hi, >>> >>> Tomas Volf <[email protected]> writes: >>> >>>> Carlo Zancanaro <[email protected]> writes: >>>> >>>>> On Thu, May 14 2026, Tomas Volf wrote: >>>>>> I have encountered bug with the hook we install for adding Change-Id >>>>>> trailer. It for some reason inserts it as a first line of the commit >>>>>> message instead of at the end. >>> >>> Would you mind to share how to reproduce the issue? It isn't obvious >>> to me. >> >> Right, sorry, that was in the initial bug report. Copy&pasting here: >> >> $ git commit -m '--- test' --allow-empty >> [master f07a209fb1] Change-Id: I02675f1053c80563402f1acddd0a64a537f4da55 --- >> test >> $ git log -1 --format=%B >> Change-Id: I02675f1053c80563402f1acddd0a64a537f4da55 >> --- test >> >> Notice that Change-Id line is *before* `--- test' line > > I see; is there a practical use case for this?
Well, I reported this because I was bitten by this bug :) This is git log from my personal Guix tree: --8<---------------cut here---------------start------------->8--- 924956928f6 etc: Add update-fork. e70d44706e9 services: Make nftables-service-type extensible. 61c9add20fa scripts: Use better SRFI-37 implementation. 4b38c620f54 etc: Add a script to update home-mpv-configuration. 28e0a409a02 etc: Add send-email-to script. 1efa81f85b0 git: Do not auto-configure. 3e103a09d96 git-authenticate: Do not install hooks. c36c4fa63a7 git-authenticate: Switch to list of always authorized parties. a76dfdec29a -- custom cf8e86f9d9f -- cherry-picked from upstream 69e6b810553 -- unsure 03e07fc67c1 environment: Allow directly setting environment variables. c65aeb1deb4 --- 81043 8e1924a8216 machine: ssh: Allow skipping kexec. 3d404b2b18c --- 80796 58afaa895fd services: Add svcgssd-service-type. 54bc6103cc6 --- 77419 b108cf06678 services: kerberos: Add my copyright. d2cba1c1e8e services: krb5-service-type: Support launching KDC daemon. bcaf60562b9 services: kerberos: Fix order of definitions. 74c520b67c9 services: krb5-configuration: Add dns-lookup-realm? field. 2adeeea69a4 services: krb5-configuration: Unify style of documentation strings. 1a7b8ced37c services: krb5-configuration: Fix indentation. 26aac2bc6e8 services: krb5-realm: Add default-principal-flags field. baafb45e092 services: krb5-realm: Unify style of documentation strings. f99241bcf8e services: krb5-realm: Delete trailing whitespace. 33f41fc31c3 --- 77001 eb26e30ef08 services: configuration: Produce doc even if package->symbol fails. dcdd659dc69 --- 71981 2d2e1c33837 monad-repl: Add "build-mode" command. a0e96027060 --- 80034 c787c252ff1 -- upstream 76d6300ee0e .guix-channel: Point the URL to my server. ef81a966ef5 COPYING: Change the license. 09505bb008e -- prequel --8<---------------cut here---------------end--------------->8--- As you can see, I am using -- and --- for separating groups of commits to make it more organized. I am trying hard not to use the Guix provided hooks (every time I do, it usually ends up in a bug report), but sometimes they just slip through. -_- > This only happens when the message starts with `---`, or when there is > a `---` used anywhere in the message? Seems to be whenever there is a line starting with ---: --8<---------------cut here---------------start------------->8--- $ git commit -m "$(printf 'aa\n\n- a\n-- b\n--- c')" --allow-empty [master 5099284fc1] aa $ g show commit 5099284fc1d75c3099d3782441a67f0d3a3eddca (HEAD -> master) Author: Portable Git <[email protected]> Date: Tue May 26 23:18:54 2026 +0000 aa - a -- b Change-Id: I56e915d54180d5d5d001711246681e1a825533a7 --- c --8<---------------cut here---------------end--------------->8--- > >>> >>> [...] >>> >>>>> I'm not sure why we're limiting ourselves to git versions and features >>>>> from 2017. >>>> >>>> Given that the commit-msg hook has this header >>>> >>>> # From Gerrit Code Review 3.11.1. >>>> # >>>> # Part of Gerrit Code Review (https://www.gerritcodereview.com/) >>>> # >>>> # Copyright (C) 2009 The Android Open Source Project >>>> >>>> I would not rule out the option that Maxim just added it without >>>> considering this particular detail. I took the liberty of CC-ing him, >>>> maybe he could answer this bit. >>> >>> It's, as the copyright suggests, a script borrowed from the Gerrit >>> project :-). It hasn't been modified. About the ways we call it, if it >>> resolves a clear issue, we do not need to constrain ourselves to an old >>> git version the same the gerrit project might; using the git version >>> which is in our current master tree would be fine, I think. >> >> Great, since adding the --no-divider seems to do the trick, that could >> be a fix. > > If it doesn't change the normal behavior otherwise, fine by me! > >> At the same time, since we are just setting a single trailer >> to a random value, it might be worth it to simplify the script a lot. > > I wouldn't want to have to maintain that Gerrit script; they update it > sometimes and it's nice to just be able to pull their fixes/updates in > our tree. > > Note that as hinted in GCD 002, its originally intended purpose is not > fulfilled, and I'd personally like to do away with it, to simplify > things a bit. I think if we want to continue having some stable > identifiers for commits we should look into the change-id stored in git > commit headers, an effort that Jujutsu, Gerrit and others are pursuing > [0]. This as the benefit of keeping a stable identifier available for > each commit, without the downside of cluttering the commit message. > > [0] > https://lore.kernel.org/git/[email protected]/T/#m2e6a57d8aeefd3146c7632e89dc36e2a0a8f68ba That looks interesting, thanks for the link. How would we go about removing the current message hook? There was no GCD to put it in, so can any committer just delete it? Tomas -- There are only two hard things in Computer Science: cache invalidation, naming things and off-by-one errors.
signature.asc
Description: PGP signature
