[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#69780] [PATCH 4/4] DRAFT news: Add entry for ‘guix git authenticate
From: |
Ludovic Courtès |
Subject: |
[bug#69780] [PATCH 4/4] DRAFT news: Add entry for ‘guix git authenticate’ changes. |
Date: |
Tue, 19 Mar 2024 15:12:46 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) |
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’.
- [bug#69780] [PATCH 2/4] git authenticate: Discover the repository., (continued)
[bug#69780] [PATCH 4/4] DRAFT news: Add entry for ‘guix git authenticate’ changes., Ludovic Courtès, 2024/03/13
[bug#69780] [PATCH 3/4] git authenticate: Install pre-push and post-checkout hooks., Ludovic Courtès, 2024/03/13