all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Tomas Volf <~@wolfsden.cz>
To: "Ludovic Courtès" <ludo@gnu.org>
Cc: 69780@debbugs.gnu.org
Subject: [bug#69780] [PATCH 1/4] git authenticate: Record introduction and keyring in ‘.git/config’.
Date: Sat, 16 Mar 2024 22:00:04 +0100	[thread overview]
Message-ID: <ZfYIVJRAq3-8VG-N@ws> (raw)
In-Reply-To: <40858e56cf55b27711d23add5f3cd2ccc6ea5c58.1710351278.git.ludo@gnu.org>

[-- Attachment #1: Type: text/plain, Size: 12046 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2024-03-16 21:00 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-13 17:40 [bug#69780] [PATCH 0/4] Simplify 'guix git authenticate' usage Ludovic Courtès
2024-03-13 17:42 ` [bug#69780] [PATCH 1/4] git authenticate: Record introduction and keyring in ‘.git/config’ Ludovic Courtès
2024-03-16 21:00   ` Tomas Volf [this message]
2024-03-19 13:32     ` Ludovic Courtès
2024-03-20 16:03       ` Ludovic Courtès
2024-03-20 22:13         ` Tomas Volf
2024-03-29 10:34           ` Ludovic Courtès
2024-03-31 12:24             ` Tomas Volf
2024-04-07 20:38               ` [bug#69780] [PATCH v2 0/5] Simplify 'guix git authenticate' usage Ludovic Courtès
2024-04-12 14:52                 ` Ludovic Courtès
2024-05-01 15:52                 ` bug#69780: " Ludovic Courtès
2024-04-07 20:38               ` [bug#69780] [PATCH v2 1/5] git authenticate: Record introduction and keyring in ‘.git/config’ Ludovic Courtès
2024-04-07 20:38               ` [bug#69780] [PATCH v2 2/5] git authenticate: Discover the repository Ludovic Courtès
2024-04-07 20:38               ` [bug#69780] [PATCH v2 3/5] git authenticate: Print something upon success Ludovic Courtès
2024-04-07 20:38               ` [bug#69780] [PATCH v2 4/5] git authenticate: Install pre-push and post-checkout hooks Ludovic Courtès
2024-04-07 20:38               ` [bug#69780] [PATCH v2 5/5] DRAFT news: Add entry for ‘guix git authenticate’ changes Ludovic Courtès
2024-03-13 17:42 ` [bug#69780] [PATCH 2/4] git authenticate: Discover the repository Ludovic Courtès
2024-03-13 17:42 ` [bug#69780] [PATCH 3/4] git authenticate: Install pre-push and post-checkout hooks Ludovic Courtès
2024-03-16 21:09   ` Tomas Volf
2024-03-19 14:02     ` Ludovic Courtès
2024-03-13 17:42 ` [bug#69780] [PATCH 4/4] DRAFT news: Add entry for ‘guix git authenticate’ changes Ludovic Courtès
2024-03-14 14:51   ` pelzflorian (Florian Pelz)
2024-03-19 14:02     ` Ludovic Courtès
2024-03-15  0:58   ` Skyler Ferris via Guix-patches via
2024-03-19 14:12     ` Ludovic Courtès
2024-03-21  1:43       ` Skyler Ferris via Guix-patches via
2024-03-21  2:14         ` Skyler Ferris via Guix-patches via
2024-03-21 14:13         ` Ludovic Courtès

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZfYIVJRAq3-8VG-N@ws \
    --to=~@wolfsden.cz \
    --cc=69780@debbugs.gnu.org \
    --cc=ludo@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.