More cookie keys

[Fred Wright]

On Mon, 19 Dec 2022, James Browning via devel wrote:
>> On 12/18/2022 6:02 PM PST Hal Murray via devel <devel at ntpsec.org> wrote:
>>
>>> The commit message for that is lacking the blank line after the summary
>>> line. This means that some git tools treat the entire commit message as the
>>> summary, creating obnoxiously long lines in their output. It's too late to
>>> fix the existing commit message without a forced update, but it would be
>>> good to avoid that in the future.
>>
>> Thanks for the heads-up.
>>
>> This seems like a bug in git. Is there a reason they haven't added a check
>> for this?

I suspect that this is because it was never considered a sufficiently 
rigid requirement to warrant a built-in check.

Some third-party git tools consider the summary to be just the first line, 
though the git suite itself uses the blank line as the delimiter. 
Conceivably this was to allow for the possibility of multiline summaries, 
though a summary that uses multiple lines while still being adequately 
short for single-line formats seems unlikely.  And in any case, separating 
the summary from the rest of the message makes it more readable.

This issue has been showing up more and more in the Linux kernel sources 
of late.  Perhaps many developers are using third-party git tools that 
cover up the issue.

Aside from the appearance in certain one-line formats, the presence or 
absence of the blank line affects the split between the %s (summary) and 
%b (body) interpolations in format strings:

------------------------------------------------------------------------
MacPro:ntpsec fw$ git show -s --format=%s 87358bc90
Update NTS cookie code to save 10 days of cookie keys. This will let NTS 
clients that only probe once a day work without needing to hammer on the 
NTS-KE server. Previous code only saved 1 old key so clients probing once 
a day would have to get new keys every other day.
MacPro:ntpsec fw$ git show -s --format=%b 87358bc90

MacPro:ntpsec fw$
------------------------------------------------------------------------

> I think it should go in a hook that gets called before the
> commit happens.

Yes, there's such a thing as a pre-commit hook, though I don't know the 
details well enough for a specific recommendation.

>> What is a forced update?

The general idea of git is that all commits are immutable, and *past* 
histories should be as close to immutable as possible.  A commit ID is a 
hash of everything in the commit, including its parentage, its file state, 
its message, its date, etc., and as such cannot be preserved while 
changing *anything* about the commit.  This is good from a reproducibility 
perspective, since it means that a single commit ID is a handle on an 
immutable state of the current files, the associated metadata, and the 
complete history thereof.

There are tools which give the appearance of modifying one or more 
commits, but what they actually do is create new commits with some or most 
of the properties of the old ones, and then (typically) update the branch 
head to point to the latest replacement commit.  So from the perspective 
of that branch, it looks like some recent commits have been "modified", as 
long as you ignore the fact that they all now have new IDs.  But 
references from other branches and/or tags still point to the old ones 
unless they're explicitly updated.

The simplest example would be commit --amend, which "amends" the head 
commit on the current branch by replacing it with a new version and 
updating the branch head accordingly.  Although it appears superficially 
that the head commit on the branch has been modified, in reality it's as 
if the old head commit were deleted and then its updated version appended. 
Since this is not a strictly append-only operation (sometimes referred to 
as "fast-forward"), it's "rewriting history", which is highly discouraged 
in *published* branches.  It's perfectly OK to do this all you want in 
your local repo, but once you've pushed a branch to a public repo, then 
pushing that sort of change to it is disallowed by default.  The push 
command itself can override this with the -f option, but if the remote 
repo has the branch protected against forced updates then even that won't 
work.

It's also possible to "modify" commits other than the head commit, but for 
that you use interactive rebase, which allows a wide variety of changes to 
be applied to any of the commits mrer recent than a specified starting 
point.  Again this is perfectly OK locally, but you should be done with it 
before "publishing" the results.

The reason forced updates are disallowed by default is that rewriting 
history causes problems for anything already referring to the previous 
commit IDs.  This is even a potential problem within a local repo, but at 
least there you can deal with any such issues yourself.  When branches in 
*shared* repos get forced updates, that potentially causes issues for 
anyone who'd already obtained the earlier version, and hence "git fetch" 
similarly refuses to perform non-fast-forward updates to local branches by 
default.  Also, "git rebase" may have to work harder amd have more 
potential conflicts when the point of divergence between the branches has 
been moved back by a forced update.  So even after you've managed to get 
your forced update "published", others may have to contend with its 
consequences.

Eric *once* did a forced update of the GPSD master, to get rid of some 
"bubbles" in the history (probably caused by someone's misuse of "git 
pull"; personally I never use "git pull"), but he announced this on the 
dev list in advance so that devs could be prepared for the possible 
consequences.  It actually turned out that "git rebase" handled this 
surprisingly well in this case, though that could have been different.

>> Is there any way to delete commits from gitlab? If it were local, I could
>> just nuke the top two and resubmit.

No need to "nuke" anything.  *Locally* in this particular case, you'd 
just:
 	$ git rebase -i HEAD~2

Then you'd change "pick" to "reword" in the first commit in the sequence 
editor, write and exit, then edit the commit message in the next editor
appearance.

Or, even better, you'd instead change "pick" to "squash" for the second 
commit, and then fix up the combined commit message in the message editor. 
That merges the two commits into one and avoids the intermediate broken 
state in the history.  But it's still too late to do this after pushing 
the previous state to the public repo.

> Well, no, but actually, yes. It involves locking,
> unprotecting, and forced pushing, followed by re-protecting
> and unlocking the 'master' branch of the remote repository.

That's only explaining mechanism, not policy. :-) The *reason* for the 
"protection" mechanism is that forced updates are highly undesirable.

> Quite possibly with a side of garbage collecting
> housekeeping/maintenance commands. IIRC.

Most likely not.  A manual GC just to clean up two orphaned commits isn't 
really justifiable.

Fred Wright