Codifying/Documenting Guix commit message conventions?

Codifying/Documenting Guix commit message conventions?

[Richard Sent]

Hi Guix,

I noticed that there seems to be discrepencies between the GNU Changelog
format and Guix's commit message convention. For example, see these
lines from [1].

>    Our convention for indicating conditional changes is to use _square
> brackets around the name of the condition_.
> 
>    Conditional changes can happen in numerous scenarios and with many
> variations, so here are some examples to help clarify. This first
> example describes changes in C, Perl, and Python files which are
> conditional but do not have an associated function or entity name:
> 
>      * xterm.c [SOLARIS2]: Include <string.h>.

Meanwhile in Guix commit messages, [foo] seems to be used to refer to a
subset of a larger part [2]:

> * doc/guix.texi (Base Services)[extra-special-file]: Add warning regarding
> files in /etc.

From what I'm seeing, the GNU Changelog convention is to indicate
subsets using <> [3].

> * progmodes/sh-script.el (sh-while-getopts) <sh>: Handle case that
> user-specified option string is empty.

Guix states that commit messages should follow the ChangeLog format
and—as far as I can tell—leaves it at that with no mention of
discrepencies or quirks [4].

>    Please write commit logs in the ChangeLog format (*note
> (standards)Change Logs::); you can check the commit history for
> examples.

My questions to the group are thus:

1. Is this in fact a discrepency between the GNU ChangeLog format and
Guix convention or am I missing something?

2. Are there other discrepencies out there that people know of?

3. How should we go about documenting Guix-specific conventions?

I suspect another discrepency not mentioned is Guix's tendency to prefix
header lines with e.g. "doc:" or "gnu:". I haven't found a better way to
identify what to put for those besides viewing commits touching X file.
And if a commit evenly spans multiple categories it can sometimes be
blurry determining what fits best.

Another one seems to be the [security fixes], [fixes CVE-...], and
[fixes TROVE-...] blocks added to certain header lines. What other tags
exist? There seems to be inconsistency here when referring to multiple
CVEs. For example, when a fixes tag references multiple CVEs you can
find.

[fixes CVE-2020-10700, CVE-2020-10704]  [5]
[fixes CVE-2020-3898 & CVE-2019-8842]   [6]
[fixes CVE-2023-{28755, 28756}]         [7]

I'm happy to write up documentation on best practices, but I figure a
general post on guix-devel is a good idea to make sure nothing's missed.
I'm not advocating for a new French revolution to overthrow the
ChangeLog aristocracy.

[8] seems like a very interesting commit to analyze in terms of Guix
conventions since it deals with a dense, nontrivial package change and
refers to "sub-sub elements", which don't seem to be a thing in GNU
ChangeLog land.

[1]: (standards) Conditional Changes
[2]: Guix commit 398393187cc48f449215b14cf18b13fefb228558
[3]: (standards) Indicating the Part Changed
[4]: (guix) Submitting Patches
[5]: 62881ad61c
[6]: a9ca7998f7
[7]: cd0a8950e4
[8]: 77d949c812

-- 
Take it easy,
Richard Sent
Making my computer weirder one commit at a time.

Re: Codifying/Documenting Guix commit message conventions?

[Andreas Enge]

Hello,

thanks for bringing up the question!

Am Thu, Jun 27, 2024 at 07:04:49PM -0400 schrieb Richard Sent:
> Meanwhile in Guix commit messages, [foo] seems to be used to refer to a
> subset of a larger part [2]:
> From what I'm seeing, the GNU Changelog convention is to indicate
> subsets using <> [3].

I also just follow what I see in other commit messages; so probably some
of my (and other people's) commit messages are "wrong"...

We seem to use <> for a second level of subdivision after [].

> 1. Is this in fact a discrepency between the GNU ChangeLog format and
> Guix convention or am I missing something?

I think it is a difference.

> [fixes TROVE-...] blocks added to certain header lines. What other tags
> exist? There seems to be inconsistency here when referring to multiple
> CVEs. For example, when a fixes tag references multiple CVEs you can
> find.
> 
> [fixes CVE-2020-10700, CVE-2020-10704]  [5]
> [fixes CVE-2020-3898 & CVE-2019-8842]   [6]
> [fixes CVE-2023-{28755, 28756}]         [7]

This probably happens rarely enough that no single convention has
mendelled out yet.

> I'm happy to write up documentation on best practices, but I figure a
> general post on guix-devel is a good idea to make sure nothing's missed.

From my point of view, it would be nice if you could try to come up with
a set of rules that capture the observed practice, and propose a patch to
add them as a section to the manual (somewhere in the chapter that speaks
about contributing).

Andreas

Re: Codifying/Documenting Guix commit message conventions?

[Maxim Cournoyer]

Hi Richard,

Richard Sent <richard@freakingpenguin.com> writes:

> Hi Guix,
>
> I noticed that there seems to be discrepencies between the GNU Changelog
> format and Guix's commit message convention. For example, see these
> lines from [1].
>
>>    Our convention for indicating conditional changes is to use _square
>> brackets around the name of the condition_.
>> 
>>    Conditional changes can happen in numerous scenarios and with many
>> variations, so here are some examples to help clarify. This first
>> example describes changes in C, Perl, and Python files which are
>> conditional but do not have an associated function or entity name:
>> 
>>      * xterm.c [SOLARIS2]: Include <string.h>.
>
> Meanwhile in Guix commit messages, [foo] seems to be used to refer to a
> subset of a larger part [2]:
>
>> * doc/guix.texi (Base Services)[extra-special-file]: Add warning regarding
>> files in /etc.
>
> From what I'm seeing, the GNU Changelog convention is to indicate
> subsets using <> [3].
>
>> * progmodes/sh-script.el (sh-while-getopts) <sh>: Handle case that
>> user-specified option string is empty.
>

Brackets ([]) are often used in our ChangeLog commit messages to specify
the field of a record.  Arguably that should be enclosed in <>, perhaps.
One gripe I have with <> is that Emacs/company-mode treat it as part of
some symbol which makes completion (M-/) useless.  That may be fixable
with some configuration for commit message buffers.  Another Emacs tip
for those who don't know it yet: pressing 'C' (capital C) on a diff hunk
while authoring a commit message using Magit will create some ChangeLog
entry stub in the commit message buffer, which saves a lot of typing.
Perhaps we should mention this in our doc, as it makes writing GNU
ChangeLog less painful.

> Guix states that commit messages should follow the ChangeLog format
> and—as far as I can tell—leaves it at that with no mention of
> discrepencies or quirks [4].
>
>>    Please write commit logs in the ChangeLog format (*note
>> (standards)Change Logs::); you can check the commit history for
>> examples.
>
> My questions to the group are thus:
>
> 1. Is this in fact a discrepency between the GNU ChangeLog format and
> Guix convention or am I missing something?
>
> 2. Are there other discrepencies out there that people know of?
>
> 3. How should we go about documenting Guix-specific conventions?

I think we shouldn't create our own dialect of GNU ChangeLog if
possible.  Ideally we'd adopt it as-is and be able to simply continue
pointing people to it.  One problem is that it evolved through the
years, so someone who read it 10 years ago may be doing it a bit
differently.

> I suspect another discrepency not mentioned is Guix's tendency to prefix
> header lines with e.g. "doc:" or "gnu:". I haven't found a better way to
> identify what to put for those besides viewing commits touching X file.
> And if a commit evenly spans multiple categories it can sometimes be
> blurry determining what fits best.

These are "domain" prefixes; it denotes the part of the code base where
the change occurred.  Some are more obvious, some others less so.
Documentation is tagged with a 'doc' prefix.  Packages are tagged with
'gnu' as they form the GNU system distribution.  Someone may use 'maint'
to denote maintenance work or 'build' for build-system related changes,
etc.  These unfortunately no science behind these so it can be difficult
for newcomers to learn what to use.  It'd be nice to document a few of
the most used ones in our contributing section.

> Another one seems to be the [security fixes], [fixes CVE-...], and
> [fixes TROVE-...] blocks added to certain header lines. What other tags
> exist? There seems to be inconsistency here when referring to multiple
> CVEs. For example, when a fixes tag references multiple CVEs you can
> find.
>
> [fixes CVE-2020-10700, CVE-2020-10704]  [5]
> [fixes CVE-2020-3898 & CVE-2019-8842]   [6]
> [fixes CVE-2023-{28755, 28756}]         [7]

I think these are likely to bust the 70 characters limit for a git
commit summary line, so perhaps we could standardize on [fixes CVE-XXX]
for single CVEs or [security fixes] when there are more than one
(listing the CVEs in the commit message body instead then).

> I'm happy to write up documentation on best practices, but I figure a
> general post on guix-devel is a good idea to make sure nothing's missed.
> I'm not advocating for a new French revolution to overthrow the
> ChangeLog aristocracy.

That sounds like a good, incremental, non-controversial approach to the
issues found (good observation, by the way!); I encourage you to pursue
it.

> [8] seems like a very interesting commit to analyze in terms of Guix
> conventions since it deals with a dense, nontrivial package change and
> refers to "sub-sub elements", which don't seem to be a thing in GNU
> ChangeLog land.

Eh, except for the duplicated Change-Id git trailer which slipped in (my
bad!).

-- 
Thanks,
Maxim

Re: Codifying/Documenting Guix commit message conventions?

[Richard Sent]

Hi Maxim,

Thanks for the detailed response! (And Andreas as well)

> Another Emacs tip for those who don't know it yet: pressing 'C'
> (capital C) on a diff hunk while authoring a commit message using
> Magit will create some ChangeLog entry stub in the commit message
> buffer, which saves a lot of typing. Perhaps we should mention this in
> our doc, as it makes writing GNU ChangeLog less painful.

Bonus points, there's a magit-generate-changelog [1] function that can
be run from the commit message buffer that'll insert changelog entries
for every diff hunk. I use this snippet to bind it to the commit entry
keymap:

--8<---------------cut here---------------start------------->8---
(use-package magit
  ;; ...
  :bind (:map git-commit-mode-map
              ("C-c m" . magit-generate-changelog)))
--8<---------------cut here---------------end--------------->8---

> That sounds like a good, incremental, non-controversial approach to the
> issues found (good observation, by the way!); I encourage you to pursue
> it.

Got it, I'll take a crack at it at some point. I'll approach it from the
perspective of documenting current practices (standardizing on one
option when there are alternatives). Then if we don't like those
practices ([] vs <>), we can discuss if we should push for matching GNU
ChangeLog or leave it as is.

The joys of Texinfo await me. For some values of joy. 🙂

(If I recall correctly there's been discussion of an RFC process in the
past. I don't recall the details but depending on how expansive this
becomes perhaps that's relevant.)

[1]: https://github.com/magit/magit/pull/3928

-- 
Take it easy,
Richard Sent
Making my computer weirder one commit at a time.

Re: Codifying/Documenting Guix commit message conventions?

[Liliana Marie Prikler]

Hi Guix,

Am Freitag, dem 28.06.2024 um 21:11 -0400 schrieb Maxim Cournoyer:
> Richard Sent <richard@freakingpenguin.com> writes:
> 
> > Another one seems to be the [security fixes], [fixes CVE-...], and
> > [fixes TROVE-...] blocks added to certain header lines. What other
> > tags exist? There seems to be inconsistency here when referring to
> > multiple CVEs. For example, when a fixes tag references multiple
> > CVEs you can find.
> > 
> > [fixes CVE-2020-10700, CVE-2020-10704]  [5]
> > [fixes CVE-2020-3898 & CVE-2019-8842]   [6]
> > [fixes CVE-2023-{28755, 28756}]         [7]
> 
> I think these are likely to bust the 70 characters limit for a git
> commit summary line, so perhaps we could standardize on [fixes CVE-
> XXX] for single CVEs or [security fixes] when there are more than one
> (listing the CVEs in the commit message body instead then).

I think we should use a "Fixes: [short description] <URI>" footer for
both Guix and upstream bugs, that can easily be parsed – hopefully by
both humans and machines.  That would give the interested reader the
(contextual) information they need, while also leaving the main body to
a more thorough description of the patch itself.

Cheers
> 

Re: Codifying/Documenting Guix commit message conventions?

[Maxim Cournoyer]

Hi Liliana,

Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

> Hi Guix,
>
> Am Freitag, dem 28.06.2024 um 21:11 -0400 schrieb Maxim Cournoyer:
>> Richard Sent <richard@freakingpenguin.com> writes:
>> 
>> > Another one seems to be the [security fixes], [fixes CVE-...], and
>> > [fixes TROVE-...] blocks added to certain header lines. What other
>> > tags exist? There seems to be inconsistency here when referring to
>> > multiple CVEs. For example, when a fixes tag references multiple
>> > CVEs you can find.
>> > 
>> > [fixes CVE-2020-10700, CVE-2020-10704]  [5]
>> > [fixes CVE-2020-3898 & CVE-2019-8842]   [6]
>> > [fixes CVE-2023-{28755, 28756}]         [7]
>> 
>> I think these are likely to bust the 70 characters limit for a git
>> commit summary line, so perhaps we could standardize on [fixes CVE-
>> XXX] for single CVEs or [security fixes] when there are more than one
>> (listing the CVEs in the commit message body instead then).
>
> I think we should use a "Fixes: [short description] <URI>" footer for
> both Guix and upstream bugs, that can easily be parsed – hopefully by
> both humans and machines.  That would give the interested reader the
> (contextual) information they need, while also leaving the main body to
> a more thorough description of the patch itself.

That's a good idea, and I already use a "Fixes:" git trailer for fixed
bugs, but I also like to be able to see from the 'git log' output which
commits were security related (I see value in the summary [security
fixes] "tag").

-- 
Thanks,
Maxim

Re: Codifying/Documenting Guix commit message conventions?

[Liliana Marie Prikler]

Am Samstag, dem 29.06.2024 um 21:11 -0400 schrieb Maxim Cournoyer:
> Hi Liliana,
> 
> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
> > I think we should use a "Fixes: [short description] <URI>" footer
> > for both Guix and upstream bugs, that can easily be parsed –
> > hopefully by both humans and machines.  That would give the
> > interested reader the (contextual) information they need, while
> > also leaving the main body to a more thorough description of the
> > patch itself.
> 
> That's a good idea, and I already use a "Fixes:" git trailer for
> fixed bugs, but I also like to be able to see from the 'git log'
> output which commits were security related (I see value in the
> summary [security fixes] "tag").
Oh, sure, my preference w.r.t. that would be to have [security fix] or
[security fixes] in the header, and the particular CVEs in the trailer.

Cheers

Re: Codifying/Documenting Guix commit message conventions?

[Maxim Cournoyer]

Hi Liliana,

Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

> Am Samstag, dem 29.06.2024 um 21:11 -0400 schrieb Maxim Cournoyer:
>> Hi Liliana,
>> 
>> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
>> > I think we should use a "Fixes: [short description] <URI>" footer
>> > for both Guix and upstream bugs, that can easily be parsed –
>> > hopefully by both humans and machines.  That would give the
>> > interested reader the (contextual) information they need, while
>> > also leaving the main body to a more thorough description of the
>> > patch itself.
>> 
>> That's a good idea, and I already use a "Fixes:" git trailer for
>> fixed bugs, but I also like to be able to see from the 'git log'
>> output which commits were security related (I see value in the
>> summary [security fixes] "tag").
> Oh, sure, my preference w.r.t. that would be to have [security fix] or
> [security fixes] in the header, and the particular CVEs in the trailer.

That sounds like a good idea to me.

Would someone be available/motivated to prepare a patch with these new
suggested guidelines, added to our 'Contributing' section?

-- 
Thanks,
Maxim

Re: Codifying/Documenting Guix commit message conventions?

[Richard Sent]

Hi Maxim,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Would someone be available/motivated to prepare a patch with these new
> suggested guidelines, added to our 'Contributing' section?

Heads up to all that I plan on taking a crack at this as time allows
(hopefully a week or so).

If anyone starts on it first feel free! I will appreciate a heads up if
that's the case. :)

-- 
Take it easy,
Richard Sent
Making my computer weirder one commit at a time.

Re: Codifying/Documenting Guix commit message conventions?

[Maxim Cournoyer]

Hi Richard,

Richard Sent <richard@freakingpenguin.com> writes:

> Hi Maxim,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>
>> Would someone be available/motivated to prepare a patch with these new
>> suggested guidelines, added to our 'Contributing' section?
>
> Heads up to all that I plan on taking a crack at this as time allows
> (hopefully a week or so).
>
> If anyone starts on it first feel free! I will appreciate a heads up if
> that's the case. :)

Yay!  It's nice to see such energy :-).

-- 
Thanks,
Maxim