diff mbox series

[bug#69780,1/4] git authenticate: Record introduction and keyring in ‘.git/config’.

Message ID 40858e56cf55b27711d23add5f3cd2ccc6ea5c58.1710351278.git.ludo@gnu.org
State New
Headers show
Series Simplify 'guix git authenticate' usage | expand

Commit Message

Ludovic Courtès March 13, 2024, 5:42 p.m. UTC
* guix/scripts/git/authenticate.scm (%default-options): Remove
‘keyring-reference’.
(config-value, configured-introduction, configured-keyring-reference)
(configured?, record-configuration): New procedures.
(guix-git-authenticate)[missing-arguments]: New procedure.
Use ‘configured-introduction’ when zero arguments are given.
Use ‘configured-keyring-reference’ when ‘-k’ is not passed.  Add call to
‘record-configuration’.
* doc/guix.texi (Invoking guix git authenticate): Document it.

Change-Id: I66e111a83f50407b52da71662629947f83a78bbc
---
 doc/guix.texi                     |  12 ++-
 guix/scripts/git/authenticate.scm | 128 ++++++++++++++++++++++--------
 tests/guix-git-authenticate.sh    |   9 ++-
 3 files changed, 112 insertions(+), 37 deletions(-)

Comments

Tomas Volf March 16, 2024, 9 p.m. UTC | #1
On 2024-03-13 18:42:19 +0100, Ludovic Courtès wrote:
> * guix/scripts/git/authenticate.scm (%default-options): Remove
> ‘keyring-reference’.
> (config-value, configured-introduction, configured-keyring-reference)
> (configured?, record-configuration): New procedures.
> (guix-git-authenticate)[missing-arguments]: New procedure.
> Use ‘configured-introduction’ when zero arguments are given.
> Use ‘configured-keyring-reference’ when ‘-k’ is not passed.  Add call to
> ‘record-configuration’.
> * doc/guix.texi (Invoking guix git authenticate): Document it.
>
> Change-Id: I66e111a83f50407b52da71662629947f83a78bbc
> ---
>  doc/guix.texi                     |  12 ++-
>  guix/scripts/git/authenticate.scm | 128 ++++++++++++++++++++++--------
>  tests/guix-git-authenticate.sh    |   9 ++-
>  3 files changed, 112 insertions(+), 37 deletions(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 858d5751bf..ac0766b98c 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -7615,8 +7615,16 @@ Invoking guix git authenticate
>  and non-zero on failure.  @var{commit} above denotes the first commit
>  where authentication takes place, and @var{signer} is the OpenPGP
>  fingerprint of public key used to sign @var{commit}.  Together, they
> -form a ``channel introduction'' (@pxref{channel-authentication, channel
> -introduction}).  The options below allow you to fine-tune the process.
> +form a @dfn{channel introduction} (@pxref{channel-authentication, channel
> +introduction}).  On your first successful run, the introduction is
> +recorded in the @file{.git/config} file of your checkout, allowing you
> +to omit them from subsequent invocations:
> +
> +@example
> +guix git authenticate [@var{options}@dots{}]
> +@end example
> +
> +The options below allow you to fine-tune the process.
>
>  @table @code
>  @item --repository=@var{directory}
> diff --git a/guix/scripts/git/authenticate.scm b/guix/scripts/git/authenticate.scm
> index 6ff5cee682..d3cc4065df 100644
> --- a/guix/scripts/git/authenticate.scm
> +++ b/guix/scripts/git/authenticate.scm
> @@ -31,6 +31,7 @@ (define-module (guix scripts git authenticate)
>    #:use-module (srfi srfi-1)
>    #:use-module (srfi srfi-26)
>    #:use-module (srfi srfi-37)
> +  #:use-module (srfi srfi-71)
>    #:use-module (ice-9 format)
>    #:use-module (ice-9 match)
>    #:export (guix-git-authenticate))
> @@ -73,8 +74,60 @@ (define %options
>                    (alist-cons 'show-stats? #t result)))))
>
>  (define %default-options
> -  '((directory . ".")
> -    (keyring-reference . "keyring")))
> +  '((directory . ".")))
> +
> +(define (config-value config key)
> +  "Return the config value associated with KEY, or #f if no such config was
> +found."
> +  (catch 'git-error
> +    (lambda ()
> +      (config-entry-value (config-get-entry config key)))
> +    (const #f)))
> +
> +(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)))
> +
> +(define (configured-keyring-reference repository)
> +  "Return the keyring reference configured in REPOSITORY or #f if missing."
> +  (let ((config (repository-config repository)))
> +    (config-value config "guix.authentication.keyring")))
> +
> +(define (configured? repository)
> +  "Return true if REPOSITORY already container introduction info in its
> +'config' file."
> +  (let ((config (repository-config repository)))
> +    (and (config-value config "guix.authentication.introduction-commit")
> +         (config-value config "guix.authentication.introduction-signer"))))
> +
> +(define* (record-configuration repository
> +                               #:key commit signer keyring-reference)
> +  "Record COMMIT, SIGNER, and KEYRING-REFERENCE in the 'config' file of
> +REPOSITORY."
> +  (define directory
> +    (repository-directory repository))
> +
> +  (define config-file
> +    (in-vicinity directory "config"))

I do not think this will work with worktrees.  It will create the config file in
the worktree's git directory, but that file will be ignored by git.

    scheme@(guile-user)> (repository-discover "/home/xx/src/guix-wt/patch-1")
    $7 = "/home/xx/src/guix/.git/worktrees/orig/"
    scheme@(guile-user)> (repository-open $7)
    $8 = #<git-repository 128cbe0>
    scheme@(guile-user)> (repository-directory $8)
    $9 = "/home/xx/src/guix/.git/worktrees/orig/"
    scheme@(guile-user)> (in-vicinity $9 "config")
    $10 = "/home/xx/src/guix/.git/worktrees/orig/config"

The $10 should be "/home/xx/src/guix/.git/config" instead.

> +
> +  (call-with-port (open-file config-file "a")
> +    (lambda (port)
> +      (format port "
> +# Added by 'guix git authenticate'.
> +[guix \"authentication\"]
> +        introduction-commit = ~a
> +        introduction-signer = ~a
> +        keyring = ~a~%"
> +              commit signer keyring-reference)))

I guess these specific values might not need any escaping?  But the escaping
(and the previous problem) would be solved by just shelling out to the `git
config --local ...' to set the value.  Something to consider.

> +
> +  (info (G_ "introduction and keyring configuration recorded in '~a'~%")
> +        config-file))
>
>  (define (show-stats stats)
>    "Display STATS, an alist containing commit signing stats as returned by
> @@ -156,35 +209,48 @@ (define (guix-git-authenticate . args)
>          (progress-reporter/bar (length commits))
>          progress-reporter/silent))
>
> +  (define (missing-arguments)
> +    (leave (G_ "wrong number of arguments; \
> +expected COMMIT and SIGNER~%")))
> +
>    (with-error-handling
>      (with-git-error-handling
> -     (match (command-line-arguments options)
> -       ((commit signer)
> -        (let* ((directory   (assoc-ref options 'directory))
> -               (show-stats? (assoc-ref options 'show-stats?))
> -               (keyring     (assoc-ref options 'keyring-reference))
> -               (repository  (repository-open directory))
> -               (end         (match (assoc-ref options 'end-commit)
> -                              (#f  (reference-target
> -                                    (repository-head repository)))
> -                              (oid oid)))
> -               (history     (match (assoc-ref options 'historical-authorizations)
> -                              (#f '())
> -                              (file (call-with-input-file file
> -                                      read-authorizations))))
> -               (cache-key   (or (assoc-ref options 'cache-key)
> -                                (repository-cache-key repository))))
> -          (define stats
> -            (authenticate-repository repository (string->oid commit)
> -                                     (openpgp-fingerprint* signer)
> -                                     #:end end
> -                                     #:keyring-reference keyring
> -                                     #:historical-authorizations history
> -                                     #:cache-key cache-key
> -                                     #:make-reporter make-reporter))
> +     (let* ((directory   (assoc-ref options 'directory))
> +            (show-stats? (assoc-ref options 'show-stats?))
> +            (repository  (repository-open directory))
> +            (commit signer (match (command-line-arguments options)
> +                             ((commit signer)
> +                              (values commit signer))
> +                             (()
> +                              (configured-introduction repository))
> +                             (_
> +                              (missing-arguments))))
> +            (keyring     (or (assoc-ref options 'keyring-reference)
> +                             (configured-keyring-reference repository)
> +                             "keyring"))
> +            (end         (match (assoc-ref options 'end-commit)
> +                           (#f  (reference-target
> +                                 (repository-head repository)))
> +                           (oid oid)))
> +            (history     (match (assoc-ref options 'historical-authorizations)
> +                           (#f '())
> +                           (file (call-with-input-file file
> +                                   read-authorizations))))
> +            (cache-key   (or (assoc-ref options 'cache-key)
> +                             (repository-cache-key repository))))
> +       (define stats
> +         (authenticate-repository repository (string->oid commit)
> +                                  (openpgp-fingerprint* signer)
> +                                  #:end end
> +                                  #:keyring-reference keyring
> +                                  #:historical-authorizations history
> +                                  #:cache-key cache-key
> +                                  #:make-reporter make-reporter))
>
> -          (when (and show-stats? (not (null? stats)))
> -            (show-stats stats))))
> -       (_
> -        (leave (G_ "wrong number of arguments; \
> -expected COMMIT and SIGNER~%")))))))
> +       (unless (configured? repository)
> +         (record-configuration repository
> +                               #:commit commit #:signer signer
> +                               #:keyring-reference keyring))

Hm, so this records the information only on the very first successful
authentication?  So if I re-run with different values (e.g. I am creating a new
channel and `git commit --amend'-ed after checking the authorization), I am
stuck with the (now) invalid values forever until I edit the .git/config by
hand?

Also (as Skyler Ferris already mentioned) this ignores the case of multiple
branches with different authentication origins (I actually do have such use
case), so the suggested option of storing it per-branch might be nice.  Not sure
how to deal with pruning of old values though (if at all?).

> +
> +       (when (and show-stats? (not (null? stats)))
> +         (show-stats stats))))))
> diff --git a/tests/guix-git-authenticate.sh b/tests/guix-git-authenticate.sh
> index ec89f941e6..db60816d45 100644
> --- a/tests/guix-git-authenticate.sh
> +++ b/tests/guix-git-authenticate.sh
> @@ -1,5 +1,5 @@
>  # GNU Guix --- Functional package management for GNU
> -# Copyright © 2020, 2022 Ludovic Courtès <ludo@gnu.org>
> +# Copyright © 2020, 2022, 2024 Ludovic Courtès <ludo@gnu.org>
>  #
>  # This file is part of GNU Guix.
>  #
> @@ -40,10 +40,11 @@ guix git authenticate "$intro_commit" "$intro_signer"	\
>       --end=9549f0283a78fe36f2d4ff2a04ef8ad6b0c02604 && false
>
>  # The v1.2.0 commit is a descendant of $intro_commit and it satisfies the
> -# authorization invariant.
> +# authorization invariant.  No need to repeat $intro_commit and $intro_signer
> +# because it should have been recorded in '.git/config'.
>  v1_2_0_commit="a099685659b4bfa6b3218f84953cbb7ff9e88063"
> -guix git authenticate "$intro_commit" "$intro_signer"	\
> -     --cache-key="$cache_key" --stats			\
> +guix git authenticate				\
> +     --cache-key="$cache_key" --stats		\
>       --end="$v1_2_0_commit"
>
>  rm "$XDG_CACHE_HOME/guix/authentication/$cache_key"
> --
> 2.41.0

Have a nice day,
Tomas Volf

--
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.
Ludovic Courtès March 19, 2024, 1:32 p.m. UTC | #2
Hi Tomas!

Tomas Volf <~@wolfsden.cz> skribis:

>> +(define* (record-configuration repository
>> +                               #:key commit signer keyring-reference)
>> +  "Record COMMIT, SIGNER, and KEYRING-REFERENCE in the 'config' file of
>> +REPOSITORY."
>> +  (define directory
>> +    (repository-directory repository))
>> +
>> +  (define config-file
>> +    (in-vicinity directory "config"))
>
> I do not think this will work with worktrees.  It will create the config file in
> the worktree's git directory, but that file will be ignored by git.
>
>     scheme@(guile-user)> (repository-discover "/home/xx/src/guix-wt/patch-1")
>     $7 = "/home/xx/src/guix/.git/worktrees/orig/"
>     scheme@(guile-user)> (repository-open $7)
>     $8 = #<git-repository 128cbe0>
>     scheme@(guile-user)> (repository-directory $8)
>     $9 = "/home/xx/src/guix/.git/worktrees/orig/"
>     scheme@(guile-user)> (in-vicinity $9 "config")
>     $10 = "/home/xx/src/guix/.git/worktrees/orig/config"
>
> The $10 should be "/home/xx/src/guix/.git/config" instead.

Damn it.  So hmm, I can see two options:

  1. Add more bindings to (git config) in Guile-Git so we can populate
     that file “the right way”.  But then we’ll have to require that
     newer version of Guile-Git.

  2. Bail out when the ‘.git/config’ isn’t found, as in the case of
     worktrees; we can change that to use the proper (git config)
     eventually.

Maybe we should go straight to #1 though.  Thoughts?

>> +  (call-with-port (open-file config-file "a")
>> +    (lambda (port)
>> +      (format port "
>> +# Added by 'guix git authenticate'.
>> +[guix \"authentication\"]
>> +        introduction-commit = ~a
>> +        introduction-signer = ~a
>> +        keyring = ~a~%"
>> +              commit signer keyring-reference)))
>
> I guess these specific values might not need any escaping?  But the escaping
> (and the previous problem) would be solved by just shelling out to the `git
> config --local ...' to set the value.  Something to consider.

No escaping is needed in this case.  Things will be even clearer if/when
we switch to (git config).

>> +       (unless (configured? repository)
>> +         (record-configuration repository
>> +                               #:commit commit #:signer signer
>> +                               #:keyring-reference keyring))
>
> Hm, so this records the information only on the very first successful
> authentication?

Right.

> So if I re-run with different values (e.g. I am creating a new channel
> and `git commit --amend'-ed after checking the authorization), I am
> stuck with the (now) invalid values forever until I edit the
> .git/config by hand?

You mean if you change the introduction?  Yes.

> Also (as Skyler Ferris already mentioned) this ignores the case of multiple
> branches with different authentication origins (I actually do have such use
> case), so the suggested option of storing it per-branch might be nice.  Not sure
> how to deal with pruning of old values though (if at all?).

Oh right.  Needs some thought.

Thanks for your feedback!

Ludo’.
Ludovic Courtès March 20, 2024, 4:03 p.m. UTC | #3
Hi,

Ludovic Courtès <ludo@gnu.org> skribis:

> Tomas Volf <~@wolfsden.cz> skribis:
>
>>> +(define* (record-configuration repository
>>> +                               #:key commit signer keyring-reference)
>>> +  "Record COMMIT, SIGNER, and KEYRING-REFERENCE in the 'config' file of
>>> +REPOSITORY."
>>> +  (define directory
>>> +    (repository-directory repository))
>>> +
>>> +  (define config-file
>>> +    (in-vicinity directory "config"))
>>
>> I do not think this will work with worktrees.  It will create the config file in
>> the worktree's git directory, but that file will be ignored by git.
>>
>>     scheme@(guile-user)> (repository-discover "/home/xx/src/guix-wt/patch-1")
>>     $7 = "/home/xx/src/guix/.git/worktrees/orig/"
>>     scheme@(guile-user)> (repository-open $7)
>>     $8 = #<git-repository 128cbe0>
>>     scheme@(guile-user)> (repository-directory $8)
>>     $9 = "/home/xx/src/guix/.git/worktrees/orig/"
>>     scheme@(guile-user)> (in-vicinity $9 "config")
>>     $10 = "/home/xx/src/guix/.git/worktrees/orig/config"
>>
>> The $10 should be "/home/xx/src/guix/.git/config" instead.
>
> Damn it.  So hmm, I can see two options:
>
>   1. Add more bindings to (git config) in Guile-Git so we can populate
>      that file “the right way”.  But then we’ll have to require that
>      newer version of Guile-Git.
>
>   2. Bail out when the ‘.git/config’ isn’t found, as in the case of
>      worktrees; we can change that to use the proper (git config)
>      eventually.
>
> Maybe we should go straight to #1 though.  Thoughts?

Done:

  https://gitlab.com/guile-git/guile-git/-/commit/b3be1dd752682b2b6c9a7c11ccdbfc0f0b5cf4e7
  https://gitlab.com/guile-git/guile-git/-/commit/d38c09230467ca5cca7faabb0c3a43c61a1e2c05

I can cut a new Guile-Git release soonish and we’d use these new
bindings.

The only open issue left is branches: how to configure different
introductions for different branches.  I’m all ears!

Ludo’.
Tomas Volf March 20, 2024, 10:13 p.m. UTC | #4
On 2024-03-20 17:03:23 +0100, Ludovic Courtès wrote:
> Hi,
>
> Ludovic Courtès <ludo@gnu.org> skribis:
>
> > Tomas Volf <~@wolfsden.cz> skribis:
> >
> >>> +(define* (record-configuration repository
> >>> +                               #:key commit signer keyring-reference)
> >>> +  "Record COMMIT, SIGNER, and KEYRING-REFERENCE in the 'config' file of
> >>> +REPOSITORY."
> >>> +  (define directory
> >>> +    (repository-directory repository))
> >>> +
> >>> +  (define config-file
> >>> +    (in-vicinity directory "config"))
> >>
> >> I do not think this will work with worktrees.  It will create the config file in
> >> the worktree's git directory, but that file will be ignored by git.
> >>
> >>     scheme@(guile-user)> (repository-discover "/home/xx/src/guix-wt/patch-1")
> >>     $7 = "/home/xx/src/guix/.git/worktrees/orig/"
> >>     scheme@(guile-user)> (repository-open $7)
> >>     $8 = #<git-repository 128cbe0>
> >>     scheme@(guile-user)> (repository-directory $8)
> >>     $9 = "/home/xx/src/guix/.git/worktrees/orig/"
> >>     scheme@(guile-user)> (in-vicinity $9 "config")
> >>     $10 = "/home/xx/src/guix/.git/worktrees/orig/config"
> >>
> >> The $10 should be "/home/xx/src/guix/.git/config" instead.
> >
> > Damn it.  So hmm, I can see two options:
> >
> >   1. Add more bindings to (git config) in Guile-Git so we can populate
> >      that file “the right way”.  But then we’ll have to require that
> >      newer version of Guile-Git.
> >
> >   2. Bail out when the ‘.git/config’ isn’t found, as in the case of
> >      worktrees; we can change that to use the proper (git config)
> >      eventually.
> >
> > Maybe we should go straight to #1 though.  Thoughts?
>
> Done:
>
>   https://gitlab.com/guile-git/guile-git/-/commit/b3be1dd752682b2b6c9a7c11ccdbfc0f0b5cf4e7
>   https://gitlab.com/guile-git/guile-git/-/commit/d38c09230467ca5cca7faabb0c3a43c61a1e2c05

Oh, that looks really nice.

>
> I can cut a new Guile-Git release soonish and we’d use these new
> bindings.
>
> The only open issue left is branches: how to configure different
> introductions for different branches.  I’m all ears!

Right, so I have few ideas, not sure if they are any good though.  And I myself
am not really sure which one I like the most, so...

0. Not care about it.  Since the explicit values override the stored ones, my
scripting will still work.

1. Record the success only when on the master branch.  That assumes that *most*
branches will share authentication parameters with the master.  That holds for
my repository (only orig-master branch differs), not sure if generally.

2. Store authentication per branch (guix.authentication.BRANCH. prefix) and
periodically clean up stale configuration (if BRANCH no longer exist, delete all
config for it).

3. Add guix.authenticate.do-not-record config defaulting to false.  That would
allow people like me to just turn if off.

4. Store the authentication on *each* success, so last-wins approach.

5. Expand the 2. to allow pattern (regex? or at least list of branches), so I
(as a user) could configure (using git config) which branch should use different
authentication from the globally recorded "default" from master branch (see 1.).

The problem here is that any possible "smart" solution leaves something to be
desired.  Either it is not automated enough (new branches are unconfigured), or
it is too magical (new branches inherit the config from... something).  5. is
probably the most flexible and covering edge cases, but will not be exactly
one-line change.

Hm, now when I read it after myself, maybe it is just not a problem worth
solving.  All of these are likely to complicate the code quite a bit.  Not sure.

Dunno, was this of any help? :)

Tomas

--
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.
Ludovic Courtès March 29, 2024, 10:34 a.m. UTC | #5
Hi!

Tomas Volf <~@wolfsden.cz> skribis:

>> The only open issue left is branches: how to configure different
>> introductions for different branches.  I’m all ears!
>
> Right, so I have few ideas, not sure if they are any good though.  And I myself
> am not really sure which one I like the most, so...

[...]

> The problem here is that any possible "smart" solution leaves something to be
> desired.  Either it is not automated enough (new branches are unconfigured), or
> it is too magical (new branches inherit the config from... something).  5. is
> probably the most flexible and covering edge cases, but will not be exactly
> one-line change.
>
> Hm, now when I read it after myself, maybe it is just not a problem worth
> solving.  All of these are likely to complicate the code quite a bit.  Not sure.
>
> Dunno, was this of any help? :)

Yes, in a way.  :-)

For me the takeaway is that we shouldn’t go to far in attempting to
address this.  The proposal we came up with Skyler, where
‘introduction-commit’ is taken as the default but may be overridden by
‘introduction-commit-BRANCH’ sounds reasonable:

  https://issues.guix.gnu.org/69780#17

It does mean that people who need this would have to manually edit their
‘.git/config’, but I think that’s acceptable: that’s an advanced use
case.

Thoughts?

Ludo’.
Tomas Volf March 31, 2024, 12:24 p.m. UTC | #6
On 2024-03-29 11:34:06 +0100, Ludovic Courtès wrote:
> > [..]
> >
> > Dunno, was this of any help? :)
>
> Yes, in a way.  :-)
>
> For me the takeaway is that we shouldn’t go to far in attempting to
> address this.  The proposal we came up with Skyler, where
> ‘introduction-commit’ is taken as the default but may be overridden by
> ‘introduction-commit-BRANCH’ sounds reasonable:
>
>   https://issues.guix.gnu.org/69780#17
>
> It does mean that people who need this would have to manually edit their
> ‘.git/config’, but I think that’s acceptable: that’s an advanced use
> case.
>
> Thoughts?

Yes, that does sound like a reasonable approach :)

Tomas

--
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.
Ludovic Courtès April 12, 2024, 2:52 p.m. UTC | #7
Hey Tomas & Skyler,

Looks like I forgot to Cc: you (or maybe the ‘send-email’ magic overrode
my Cc: or X-Debbugs-Cc: header, not sure.)  So: here’s a friendly ping.

Ludo’.

Ludovic Courtès <ludo@gnu.org> skribis:

> This is an update on <https://issues.guix.gnu.org/69780>.
>
> Changes since v1:
>
>   • Write config to the right file using the new
>     ‘set-config-string’ procedure of Guile-Git.
>
>   • Write hooks to the right place (again, accounting for
>     worktrees) using the new ‘repository-common-directory’
>     procedure of Guile-Git.
>
>   • Support branch-specific configuration, as suggested by
>     Skyler and Tomas.
>
>   • Create a post-merge hook rather than post-checkout, as
>     suggested by Skyler.
>
>   • Add German translation by Florian and French translation
>     of the news entry.
>
> Since this requires the latest Guile-Git, you can use this
> trick to test locally:
>
>   guix shell -CPW -m manifest.scm \
>     guile-git --with-branch=guile-git=master
>
> If we agree on the patch set, I’ll tag Guile-Git 0.7.0 and
> update it in Guix before applying these patches.
>
> Feedback welcome!
>
> Ludo’.
>
> Ludovic Courtès (5):
>   git authenticate: Record introduction and keyring in ‘.git/config’.
>   git authenticate: Discover the repository.
>   git authenticate: Print something upon success.
>   git authenticate: Install pre-push and post-checkout hooks.
>   DRAFT news: Add entry for ‘guix git authenticate’ changes.
>
>  doc/guix.texi                     |  33 ++++-
>  etc/news.scm                      |  44 +++++++
>  guix/scripts/git/authenticate.scm | 197 +++++++++++++++++++++++++-----
>  tests/guix-git-authenticate.sh    |   9 +-
>  4 files changed, 249 insertions(+), 34 deletions(-)
>
>
> base-commit: 9b50a35fc0764f48688974af106fd7a6809f33ee
Ludovic Courtès May 1, 2024, 3:52 p.m. UTC | #8
Hello,

Ludovic Courtès <ludo@gnu.org> skribis:

> This is an update on <https://issues.guix.gnu.org/69780>.
>
> Changes since v1:
>
>   • Write config to the right file using the new
>     ‘set-config-string’ procedure of Guile-Git.
>
>   • Write hooks to the right place (again, accounting for
>     worktrees) using the new ‘repository-common-directory’
>     procedure of Guile-Git.
>
>   • Support branch-specific configuration, as suggested by
>     Skyler and Tomas.
>
>   • Create a post-merge hook rather than post-checkout, as
>     suggested by Skyler.
>
>   • Add German translation by Florian and French translation
>     of the news entry.

[...]

> If we agree on the patch set, I’ll tag Guile-Git 0.7.0 and
> update it in Guix before applying these patches.

I went ahead and pushed this patch series:

  5c13ab50b9 news: Add entry for ‘guix git authenticate’ changes.
  8d1d98a3aa git authenticate: Install pre-push and post-checkout hooks.
  1a5041a502 git authenticate: Print something upon success.
  88573dd928 git authenticate: Discover the repository.
  7b4bf4ee88 git authenticate: Record introduction and keyring in ‘.git/config’.
  10aa88ea01 gnu: guile-git: Update to 0.7.0.

Thanks again for your insightful feedback!

Ludo’.
diff mbox series

Patch

diff --git a/doc/guix.texi b/doc/guix.texi
index 858d5751bf..ac0766b98c 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -7615,8 +7615,16 @@  Invoking guix git authenticate
 and non-zero on failure.  @var{commit} above denotes the first commit
 where authentication takes place, and @var{signer} is the OpenPGP
 fingerprint of public key used to sign @var{commit}.  Together, they
-form a ``channel introduction'' (@pxref{channel-authentication, channel
-introduction}).  The options below allow you to fine-tune the process.
+form a @dfn{channel introduction} (@pxref{channel-authentication, channel
+introduction}).  On your first successful run, the introduction is
+recorded in the @file{.git/config} file of your checkout, allowing you
+to omit them from subsequent invocations:
+
+@example
+guix git authenticate [@var{options}@dots{}]
+@end example
+
+The options below allow you to fine-tune the process.
 
 @table @code
 @item --repository=@var{directory}
diff --git a/guix/scripts/git/authenticate.scm b/guix/scripts/git/authenticate.scm
index 6ff5cee682..d3cc4065df 100644
--- a/guix/scripts/git/authenticate.scm
+++ b/guix/scripts/git/authenticate.scm
@@ -31,6 +31,7 @@  (define-module (guix scripts git authenticate)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-26)
   #:use-module (srfi srfi-37)
+  #:use-module (srfi srfi-71)
   #:use-module (ice-9 format)
   #:use-module (ice-9 match)
   #:export (guix-git-authenticate))
@@ -73,8 +74,60 @@  (define %options
                   (alist-cons 'show-stats? #t result)))))
 
 (define %default-options
-  '((directory . ".")
-    (keyring-reference . "keyring")))
+  '((directory . ".")))
+
+(define (config-value config key)
+  "Return the config value associated with KEY, or #f if no such config was
+found."
+  (catch 'git-error
+    (lambda ()
+      (config-entry-value (config-get-entry config key)))
+    (const #f)))
+
+(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)))
+
+(define (configured-keyring-reference repository)
+  "Return the keyring reference configured in REPOSITORY or #f if missing."
+  (let ((config (repository-config repository)))
+    (config-value config "guix.authentication.keyring")))
+
+(define (configured? repository)
+  "Return true if REPOSITORY already container introduction info in its
+'config' file."
+  (let ((config (repository-config repository)))
+    (and (config-value config "guix.authentication.introduction-commit")
+         (config-value config "guix.authentication.introduction-signer"))))
+
+(define* (record-configuration repository
+                               #:key commit signer keyring-reference)
+  "Record COMMIT, SIGNER, and KEYRING-REFERENCE in the 'config' file of
+REPOSITORY."
+  (define directory
+    (repository-directory repository))
+
+  (define config-file
+    (in-vicinity directory "config"))
+
+  (call-with-port (open-file config-file "a")
+    (lambda (port)
+      (format port "
+# Added by 'guix git authenticate'.
+[guix \"authentication\"]
+        introduction-commit = ~a
+        introduction-signer = ~a
+        keyring = ~a~%"
+              commit signer keyring-reference)))
+
+  (info (G_ "introduction and keyring configuration recorded in '~a'~%")
+        config-file))
 
 (define (show-stats stats)
   "Display STATS, an alist containing commit signing stats as returned by
@@ -156,35 +209,48 @@  (define (guix-git-authenticate . args)
         (progress-reporter/bar (length commits))
         progress-reporter/silent))
 
+  (define (missing-arguments)
+    (leave (G_ "wrong number of arguments; \
+expected COMMIT and SIGNER~%")))
+
   (with-error-handling
     (with-git-error-handling
-     (match (command-line-arguments options)
-       ((commit signer)
-        (let* ((directory   (assoc-ref options 'directory))
-               (show-stats? (assoc-ref options 'show-stats?))
-               (keyring     (assoc-ref options 'keyring-reference))
-               (repository  (repository-open directory))
-               (end         (match (assoc-ref options 'end-commit)
-                              (#f  (reference-target
-                                    (repository-head repository)))
-                              (oid oid)))
-               (history     (match (assoc-ref options 'historical-authorizations)
-                              (#f '())
-                              (file (call-with-input-file file
-                                      read-authorizations))))
-               (cache-key   (or (assoc-ref options 'cache-key)
-                                (repository-cache-key repository))))
-          (define stats
-            (authenticate-repository repository (string->oid commit)
-                                     (openpgp-fingerprint* signer)
-                                     #:end end
-                                     #:keyring-reference keyring
-                                     #:historical-authorizations history
-                                     #:cache-key cache-key
-                                     #:make-reporter make-reporter))
+     (let* ((directory   (assoc-ref options 'directory))
+            (show-stats? (assoc-ref options 'show-stats?))
+            (repository  (repository-open directory))
+            (commit signer (match (command-line-arguments options)
+                             ((commit signer)
+                              (values commit signer))
+                             (()
+                              (configured-introduction repository))
+                             (_
+                              (missing-arguments))))
+            (keyring     (or (assoc-ref options 'keyring-reference)
+                             (configured-keyring-reference repository)
+                             "keyring"))
+            (end         (match (assoc-ref options 'end-commit)
+                           (#f  (reference-target
+                                 (repository-head repository)))
+                           (oid oid)))
+            (history     (match (assoc-ref options 'historical-authorizations)
+                           (#f '())
+                           (file (call-with-input-file file
+                                   read-authorizations))))
+            (cache-key   (or (assoc-ref options 'cache-key)
+                             (repository-cache-key repository))))
+       (define stats
+         (authenticate-repository repository (string->oid commit)
+                                  (openpgp-fingerprint* signer)
+                                  #:end end
+                                  #:keyring-reference keyring
+                                  #:historical-authorizations history
+                                  #:cache-key cache-key
+                                  #:make-reporter make-reporter))
 
-          (when (and show-stats? (not (null? stats)))
-            (show-stats stats))))
-       (_
-        (leave (G_ "wrong number of arguments; \
-expected COMMIT and SIGNER~%")))))))
+       (unless (configured? repository)
+         (record-configuration repository
+                               #:commit commit #:signer signer
+                               #:keyring-reference keyring))
+
+       (when (and show-stats? (not (null? stats)))
+         (show-stats stats))))))
diff --git a/tests/guix-git-authenticate.sh b/tests/guix-git-authenticate.sh
index ec89f941e6..db60816d45 100644
--- a/tests/guix-git-authenticate.sh
+++ b/tests/guix-git-authenticate.sh
@@ -1,5 +1,5 @@ 
 # GNU Guix --- Functional package management for GNU
-# Copyright © 2020, 2022 Ludovic Courtès <ludo@gnu.org>
+# Copyright © 2020, 2022, 2024 Ludovic Courtès <ludo@gnu.org>
 #
 # This file is part of GNU Guix.
 #
@@ -40,10 +40,11 @@  guix git authenticate "$intro_commit" "$intro_signer"	\
      --end=9549f0283a78fe36f2d4ff2a04ef8ad6b0c02604 && false
 
 # The v1.2.0 commit is a descendant of $intro_commit and it satisfies the
-# authorization invariant.
+# authorization invariant.  No need to repeat $intro_commit and $intro_signer
+# because it should have been recorded in '.git/config'.
 v1_2_0_commit="a099685659b4bfa6b3218f84953cbb7ff9e88063"
-guix git authenticate "$intro_commit" "$intro_signer"	\
-     --cache-key="$cache_key" --stats			\
+guix git authenticate				\
+     --cache-key="$cache_key" --stats		\
      --end="$v1_2_0_commit"
 
 rm "$XDG_CACHE_HOME/guix/authentication/$cache_key"