[bug#69780] [PATCH 4/4] DRAFT news: Add entry for ‘guix git authenticate’ changes.

["Ludovic Courtès" <ludo@gnu.org>]

Hi Skyler,

Skyler Ferris <skyvine@protonmail.com> skribis:

>> +  (if (or (file-exists? pre-push-hook)
>> +          (file-exists? fpost-checkout-hook))
>> +      (begin
>> +        (warning (G_ "not overriding pre-existing hooks '~a' and '~a'~%")
>> +                 pre-push-hook post-checkout-hook)
>> +        (display-hint (G_ "Consider running @command{guix git authenticate}
>> +from your pre-push and update hooks so your repository is automatically
>> +authenticated before you push or receive updates.")))
>
> When the developer builds guix a pre-push hook is already installed,
> from etc/git/pre-push.

Right.  Like I wrote when replying to Tomas, I view this as a helper
primarily for people outside Guix itself, because Guix already has its
own hooks installed as you write.

We could discuss what to do with Guix’s own hooks, but to me that’s a
separate issue.

>> +  (define post-checkout-hook
>> +    (in-vicinity directory "hooks/post-checkout"))
>
> The post-checkout hook does not run when `git pull` is called. Instead, the post-merge hook is called. Note that post-merge does not receive the same set of arguments as post-checkout. I had success replacing "$newrev" with "$(git rev-parse HEAD)". We could leave out the value completely for post-merge because the script will use HEAD by default if no end is given. But maybe we don't want to rely on default behavior for a script that will not be automatically updated with the tool.
>
> I can think of more ways that a developer could end up on a new commit. For example, running `git fetch` followed by `git reset --hard`. I'm not sure if it makes to support every possible case because that could get complicated very quickly. But git pull is the most common way to get updates (at least in my experience, which could have a sample bias) so I think it makes sense to at least support that.

I spent time looking for the “right” hook and couldn’t find anything
really satisfying.  Ideally, I’d want a hook that runs on ‘fetch’, for
each new reference.

Is post-merge better than post-checkout?  githooks(5) says ‘post-merge’
“is invoked by git-merge(1), which happens when a git pull is done on a
local repository.”  Is it actually invoked when ‘git pull’ does *not*
trigger a merge?

>> +while read local_ref local_oid remote_ref remote_oid
>> +do
>> +  guix git authenticate --end=\"$local_ref\"
>> +done\n")
>
> I believe this should be "$local_oid", not "$local_ref".

Oops, noted.

>> +(define (configured-introduction repository)
>> +  "Return two values: the commit and signer fingerprint (strings) as
>> +configured in REPOSITORY.  Error out if one or both were missing."
>> +  (let* ((config (repository-config repository))
>> +         (commit (config-value config "guix.authentication.introduction-commit"))
>> +         (signer (config-value config "guix.authentication.introduction-signer")))
>> +    (unless (and commit signer)
>> +      (leave (G_ "unknown introductory commit and signer~%")))
>> +    (values commit signer)))
>
> I am wondering how this would work if somebody is working with multiple branches, in particular if they do not all use the same introduction. Maybe that doesn't need to be addressed in this patch series but it might be easier to address it in the future if the branch name was included in the config file instead assuming that a specific introduction applies to every branch in a given checkout (for example, by using "guix.authentication.introduction-commit.branch-name"). This is probably more relevant to external users of the tool than to the guix repository itself. The logistics of using unrelated branches simultaneously seems complicated and not very helpful, especially when channels are such an appealing alternative. But it could be useful for other projects.

Very good point.  Now, what would it look like?

Currently we have:

--8<---------------cut here---------------start------------->8---
[guix "authentication"]
        introduction-commit = 9edb3f66fd807b096b48283debdcddccfea34bad
        introduction-signer = BBB0 2DDF 2CEA F6A8 0D1D  E643 A2A0 6DF2 A33A 54FA
        keyring = keyring
--8<---------------cut here---------------end--------------->8---

Using this configuration format, it seems there’s no room left for a
branch name, or am I overlooking something?

Or we could take the risk of removing ‘guix’ and make it:

--8<---------------cut here---------------start------------->8---
[authentication "master"]
        introduction-commit = 9edb3f66fd807b096b48283debdcddccfea34bad
        introduction-signer = BBB0 2DDF 2CEA F6A8 0D1D  E643 A2A0 6DF2 A33A 54FA
        keyring = keyring
--8<---------------cut here---------------end--------------->8---

WDYT?

Thanks for your feedback!

Ludo’.