[bug#69780] [PATCH 0/4] Simplify 'guix git authenticate' usage

[bug#69780] [PATCH 0/4] Simplify 'guix git authenticate' usage

[Ludovic Courtès]

Hello Git! :-)

‘guix git authenticate’ has always been inconvenient because one
has to provide the introduction (commit and signer) on the command
line.

Only recently did I realize that we could store that info in
‘.git/config’.  This is the main goal of this patch set.

The rest further simplifies its use by discovering the repo and
installing pre-push and post-checkout hooks.

Thoughts?

Ludo’.

Ludovic Courtès (4):
  git authenticate: Record introduction and keyring in ‘.git/config’.
  git authenticate: Discover the repository.
  git authenticate: Install pre-push and post-checkout hooks.
  DRAFT news: Add entry for ‘guix git authenticate’ changes.

 doc/guix.texi                     |  17 ++-
 etc/news.scm                      |  16 +++
 guix/scripts/git/authenticate.scm | 169 ++++++++++++++++++++++++------
 tests/guix-git-authenticate.sh    |   9 +-
 4 files changed, 174 insertions(+), 37 deletions(-)


base-commit: 7b5c030684020282a690322b558f86718eb148a7
-- 
2.41.0

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

[Ludovic Courtès]

* 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"))
+
+  (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"
-- 
2.41.0

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

[Tomas Volf]

[-- 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 --]

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

[Ludovic Courtès]

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’.

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

[Ludovic Courtès]

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’.

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

[Tomas Volf]

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

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.

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

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

[Ludovic Courtès]

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’.

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

[Tomas Volf]

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

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.

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

[bug#69780] [PATCH v2 0/5] Simplify 'guix git authenticate' usage

[Ludovic Courtès]

Hello comrades,

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
-- 
2.41.0

[bug#69780] [PATCH v2 0/5] Simplify 'guix git authenticate' usage

[Ludovic Courtès]

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

bug#69780: [PATCH v2 0/5] Simplify 'guix git authenticate' usage

[Ludovic Courtès]

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’.

[bug#69780] [PATCH v2 1/5] git authenticate: Record introduction and keyring in ‘.git/config’.

[Ludovic Courtès]

* guix/scripts/git/authenticate.scm (%default-options): Remove
‘keyring-reference’.
(config-value, configured-introduction, configured-keyring-reference)
(configured?, record-configuration, current-branch): 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                     |  28 +++++-
 guix/scripts/git/authenticate.scm | 147 +++++++++++++++++++++++-------
 tests/guix-git-authenticate.sh    |   9 +-
 3 files changed, 150 insertions(+), 34 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index acfe60b47a..6ff0e76d97 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -7667,6 +7667,8 @@ Invoking guix git authenticate
 @section Invoking @command{guix git authenticate}
 
 @cindex @command{guix git authenticate}
+@cindex authentication, of Git checkouts
+@cindex Git checkout authentication
 
 The @command{guix git authenticate} command authenticates a Git checkout
 following the same rule as for channels (@pxref{channel-authentication,
@@ -7686,13 +7688,35 @@ Invoking guix git authenticate
 guix git authenticate @var{commit} @var{signer} [@var{options}@dots{}]
 @end example
 
+@cindex introduction, for Git authentication
 By default, this command authenticates the Git checkout in the current
 directory; it outputs nothing and exits with exit code zero on success
 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
+
+Should you have branches that require different introductions, you can
+specify them directly in @file{.git/config}.  For example, if the branch
+called @code{personal-fork} has a different introduction than other
+branches, you can extend @file{.git/config} along these lines:
+
+@smallexample
+[guix "authentication-personal-fork"]
+	introduction-commit = cabba936fd807b096b48283debdcddccfea3900d
+	introduction-signer = C0FF EECA BBA9 E6A8 0D1D  E643 A2A0 6DF2 A33A 54FA
+	keyring = keyring
+@end smallexample
+
+The command-line options described 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 def4879e96..a606f1c146 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,79 @@ (define %options
                   (alist-cons 'show-stats? #t result)))))
 
 (define %default-options
-  '((directory . ".")
-    (keyring-reference . "keyring")))
+  '((directory . ".")))
+
+(define (current-branch repository)
+  "Return the name of the checked out branch of REPOSITORY or #f if it could
+not be determined."
+  (and (not (repository-head-detached? repository))
+       (let* ((head (repository-head repository))
+              (name (reference-name head)))
+         (and (string-prefix? "refs/heads/" name)
+              (string-drop name (string-length "refs/heads/"))))))
+
+(define (config-value repository key)
+  "Return the config value associated with KEY in the 'guix.authentication' or
+'guix.authentication-BRANCH' name space in REPOSITORY, or #f if no such config
+was found."
+  (let-syntax ((false-if-git-error
+                (syntax-rules ()
+                  ((_ exp)
+                   (catch 'git-error (lambda () exp) (const #f))))))
+    (let* ((config (repository-config repository))
+           (branch (current-branch repository)))
+      ;; First try the BRANCH-specific value, then the generic one.`
+      (or (and branch
+               (false-if-git-error
+                (config-entry-value
+                 (config-get-entry config
+                                   (string-append "guix.authentication-"
+                                                  branch "." key)))))
+          (false-if-git-error
+           (config-entry-value
+            (config-get-entry config
+                              (string-append "guix.authentication."
+                                             key))))))))
+
+(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* ((commit (config-value repository "introduction-commit"))
+         (signer (config-value repository "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."
+  (config-value repository "keyring"))
+
+(define (configured? repository)
+  "Return true if REPOSITORY already container introduction info in its
+'config' file."
+  (and (config-value repository "introduction-commit")
+       (config-value repository "introduction-signer")))
+
+(define* (record-configuration repository
+                               #:key commit signer keyring-reference)
+  "Record COMMIT, SIGNER, and KEYRING-REFERENCE in the 'config' file of
+REPOSITORY."
+  (define config
+    (repository-config repository))
+
+  ;; Guile-Git < 0.7.0 lacks 'set-config-string'.
+  (if (module-defined? (resolve-interface '(git)) 'set-config-string)
+      (begin
+        (set-config-string config "guix.authentication.introduction-commit"
+                           commit)
+        (set-config-string config "guix.authentication.introduction-signer"
+                           signer)
+        (set-config-string config "guix.authentication.keyring"
+                           keyring-reference)
+        (info (G_ "introduction and keyring recorded \
+in repository configuration file~%")))
+      (warning (G_ "could not record introduction and keyring configuration\
+ (Guile-Git too old?)~%"))))
 
 (define (show-stats stats)
   "Display STATS, an alist containing commit signing stats as returned by
@@ -158,35 +230,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..7b8951b9aa 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.
 #
@@ -46,6 +46,13 @@ guix git authenticate "$intro_commit" "$intro_signer"	\
      --cache-key="$cache_key" --stats			\
      --end="$v1_2_0_commit"
 
+# Check a commit that came soon after v1.2.0.  No need to repeat $intro_commit
+# and $intro_signer because it should have been recorded in '.git/config'.
+after_v1_2_0="be4d9527b55b6829e33a6e0727496af25927a786"
+guix git authenticate				\
+     --cache-key="$cache_key" --stats		\
+     --end="$v1_2_0_commit"
+
 rm "$XDG_CACHE_HOME/guix/authentication/$cache_key"
 
 # Commit and signer of the 'v1.0.0' tag.
-- 
2.41.0

[bug#69780] [PATCH v2 2/5] git authenticate: Discover the repository.

[Ludovic Courtès]

This allows one to run ‘guix git authenticate’ from a sub-directory of
the checkout.

* guix/scripts/git/authenticate.scm (%default-options): Remove
‘directory’ key.
(guix-git-authenticate): Use ‘repository-discover’ when ‘directory’
option is missing.

Change-Id: Ifada00d559254971ed7eeb8c0a8d4ae74ff3defc
---
 guix/scripts/git/authenticate.scm | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/guix/scripts/git/authenticate.scm b/guix/scripts/git/authenticate.scm
index a606f1c146..d81a0e1ffb 100644
--- a/guix/scripts/git/authenticate.scm
+++ b/guix/scripts/git/authenticate.scm
@@ -74,7 +74,7 @@ (define %options
                   (alist-cons 'show-stats? #t result)))))
 
 (define %default-options
-  '((directory . ".")))
+  '())
 
 (define (current-branch repository)
   "Return the name of the checked out branch of REPOSITORY or #f if it could
@@ -236,9 +236,9 @@ (define (guix-git-authenticate . args)
 
   (with-error-handling
     (with-git-error-handling
-     (let* ((directory   (assoc-ref options 'directory))
-            (show-stats? (assoc-ref options 'show-stats?))
-            (repository  (repository-open directory))
+     (let* ((show-stats? (assoc-ref options 'show-stats?))
+            (repository  (repository-open (or (assoc-ref options 'directory)
+                                              (repository-discover "."))))
             (commit signer (match (command-line-arguments options)
                              ((commit signer)
                               (values commit signer))
-- 
2.41.0

[bug#69780] [PATCH v2 3/5] git authenticate: Print something upon success.

[Ludovic Courtès]

Until now the command would be silent and exit with 0.

* guix/scripts/git/authenticate.scm (guix-git-authenticate): Print
something upon success.

Change-Id: I08d086c35df6ac74ee847df0479660293c68987d
---
 guix/scripts/git/authenticate.scm | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/guix/scripts/git/authenticate.scm b/guix/scripts/git/authenticate.scm
index d81a0e1ffb..0797cba0b6 100644
--- a/guix/scripts/git/authenticate.scm
+++ b/guix/scripts/git/authenticate.scm
@@ -274,4 +274,7 @@ (define (guix-git-authenticate . args)
                                #:keyring-reference keyring))
 
        (when (and show-stats? (not (null? stats)))
-         (show-stats stats))))))
+         (show-stats stats))
+
+       (info (G_ "successfully authenticated commit ~a~%")
+             (oid->string end))))))
-- 
2.41.0

[bug#69780] [PATCH v2 4/5] git authenticate: Install pre-push and post-checkout hooks.

[Ludovic Courtès]

* guix/scripts/git/authenticate.scm (install-hooks): New procedure.
(guix-git-authenticate): Use it.
* doc/guix.texi (Invoking guix git authenticate): Document it.

Change-Id: I4464a33193186e85b476a12740e54412bd58429c
---
 doc/guix.texi                     |  5 ++++
 guix/scripts/git/authenticate.scm | 49 ++++++++++++++++++++++++++++++-
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 6ff0e76d97..9db0ff865d 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -7715,6 +7715,11 @@ Invoking guix git authenticate
 	keyring = keyring
 @end smallexample
 
+The first run also attempts to install pre-push and post-merge hooks,
+such that @command{guix git authenticate} is invoked as soon as you run
+@command{git push}, @command{git pull}, and related commands; it does
+not overwrite preexisting hooks though.
+
 The command-line options described below allow you to fine-tune the
 process.
 
diff --git a/guix/scripts/git/authenticate.scm b/guix/scripts/git/authenticate.scm
index 0797cba0b6..e3ecb67c89 100644
--- a/guix/scripts/git/authenticate.scm
+++ b/guix/scripts/git/authenticate.scm
@@ -148,6 +148,52 @@ (define* (record-configuration repository
       (warning (G_ "could not record introduction and keyring configuration\
  (Guile-Git too old?)~%"))))
 
+(define (install-hooks repository)
+  "Attempt to install in REPOSITORY hooks that invoke 'guix git authenticate'.
+Bail out if one of these already exists."
+  ;; Guile-Git < 0.7.0 lacks 'repository-common-directory'.
+  (if (module-defined? (resolve-interface '(git))
+                       'repository-common-directory)
+      (let ()
+        (define directory
+          (repository-common-directory repository))
+
+        (define pre-push-hook
+          (in-vicinity directory "hooks/pre-push"))
+
+        (define post-merge-hook
+          (in-vicinity directory "hooks/post-merge"))
+
+        (if (or (file-exists? pre-push-hook)
+                (file-exists? post-merge-hook))
+            (begin
+              (warning (G_ "not overriding pre-existing hooks '~a' and '~a'~%")
+                       pre-push-hook post-merge-hook)
+              (display-hint (G_ "Consider running @command{guix git authenticate}
+from your pre-push and post-merge hooks so your repository is automatically
+authenticated before you push and when you pull updates.")))
+            (begin
+              (call-with-output-file pre-push-hook
+                (lambda (port)
+                  (format port "#!/bin/sh
+# Installed by 'guix git authenticate'.
+set -e
+while read local_ref local_oid remote_ref remote_oid
+do
+  guix git authenticate --end=\"$local_oid\"
+done\n")
+                  (chmod port #o755)))
+              (call-with-output-file post-merge-hook
+                (lambda (port)
+                  (format port "#!/bin/sh
+# Installed by 'guix git authenticate'.
+exec guix git authenticate\n")
+                  (chmod port #o755)))
+              (info (G_ "installed hooks '~a' and '~a'~%")
+                    pre-push-hook post-merge-hook))))
+      (warning (G_ "cannot determine where to install hooks\
+ (Guile-Git too old?)~%"))))
+
 (define (show-stats stats)
   "Display STATS, an alist containing commit signing stats as returned by
 'authenticate-repository'."
@@ -271,7 +317,8 @@ (define (guix-git-authenticate . args)
        (unless (configured? repository)
          (record-configuration repository
                                #:commit commit #:signer signer
-                               #:keyring-reference keyring))
+                               #:keyring-reference keyring)
+         (install-hooks repository))
 
        (when (and show-stats? (not (null? stats)))
          (show-stats stats))
-- 
2.41.0

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

[Ludovic Courtès]

* etc/news.scm: Add entry.

Change-Id: I661a0c0bfc373b87a70508ad9a735315c96ba4a5

Co-authored-by: Florian Pelz <pelzflorian@pelzflorian.de>
Change-Id: Ibafef9d432df163948c4884279d4ce2579ed0312
---
 etc/news.scm | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/etc/news.scm b/etc/news.scm
index 9443475455..e9fde35346 100644
--- a/etc/news.scm
+++ b/etc/news.scm
@@ -31,6 +31,50 @@
 (channel-news
  (version 0)
 
+ (entry (commit "TODO")
+        (title
+         (en "@command{guix git authenticate} usage simplified")
+         (de "@command{guix git authenticate} ist leichter nutzbar")
+         (fr "@command{guix git authenticate} simplifiée"))
+        (body
+         (en "Usage of the @command{guix git authenticate} command has been
+simplified.  The command is useful to channel authors and to developers
+willing to validate the provenance of their code.
+
+On your first use, @command{guix git authenticate} will now record the commit
+and signer (the @dfn{introduction}) in the @file{.git/config} file of your
+repository so that you don't have to pass them on the command line in
+subsequent runs.  It will also install pre-push and post-merge hooks,
+unless preexisting hooks are found.
+
+Run @command{info \"(guix) Invoking guix authenticate\"} for more info.")
+         (de "Der Befehl @command{guix git authenticate} kann jetzt einfacher
+benutzt werden.  Mit dem Befehl können Kanalautoren und Entwickler die
+Provenienz ihres Codes überprüfen.
+
+Beim ersten Gebrauch speichert @command{guix git authenticate} Commit und
+Unterzeichner (wie in der @dfn{Kanaleinführung}) in der Datei
+@file{.git/config} Ihres Repositorys, so dass Sie sie bei späteren
+Ausführungen nicht mehr auf der Befehlszeile angeben müssen.  Auch werden
+Git-Hooks für pre-push und post-merge installiert, wenn es bisher keine
+Hooks dieser Art gibt.
+
+Führen Sie @command{info \"(guix.de) Aufruf von guix git authenticate\"}
+aus, wenn Sie mehr wissen wollen.")
+         (fr "L'utilisation de la commande @command{guix git authenticate} a
+été simplifiée.  Cette commande est utile aux auteur·rices de canaux et aux
+développeur·euses souhaitant pouvoir valider l'origine de leur code.
+
+À la première utilisation, @command{guix git authenticate} enregistre
+désormais le commit et signataire (l'@dfn{introduction}) dans le fichier
+@file{.git/config} du dépôt, ce qui permet de ne pas avoir à les spécifier sur
+la ligne de commande les fois suivantes.  La commande installe aussi des
+crochets « pre-push » et « post-merge », sauf si des crochets préexistants
+sont trouvés.
+
+Lancer @command{info \"(guix.fr) Invoquer guix git authenticate\"} pour en
+savoir plus.")))
+
  (entry (commit "523f3def65ab061a87f4fc9e6f9008e6a78fafb5")
         (title
          (en "GNOME updated to version 44 with a more modular desktop service")
-- 
2.41.0

[bug#69780] [PATCH 2/4] git authenticate: Discover the repository.

[Ludovic Courtès]

This allows one to run ‘guix git authenticate’ from a sub-directory of
the checkout.

* guix/scripts/git/authenticate.scm (%default-options): Remove
‘directory’ key.
(guix-git-authenticate): Use ‘repository-discover’ when ‘directory’
option is missing.

Change-Id: Ifada00d559254971ed7eeb8c0a8d4ae74ff3defc
---
 guix/scripts/git/authenticate.scm | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/guix/scripts/git/authenticate.scm b/guix/scripts/git/authenticate.scm
index d3cc4065df..36e1aa6228 100644
--- a/guix/scripts/git/authenticate.scm
+++ b/guix/scripts/git/authenticate.scm
@@ -74,7 +74,7 @@ (define %options
                   (alist-cons 'show-stats? #t result)))))
 
 (define %default-options
-  '((directory . ".")))
+  '())
 
 (define (config-value config key)
   "Return the config value associated with KEY, or #f if no such config was
@@ -215,9 +215,9 @@ (define (guix-git-authenticate . args)
 
   (with-error-handling
     (with-git-error-handling
-     (let* ((directory   (assoc-ref options 'directory))
-            (show-stats? (assoc-ref options 'show-stats?))
-            (repository  (repository-open directory))
+     (let* ((show-stats? (assoc-ref options 'show-stats?))
+            (repository  (repository-open (or (assoc-ref options 'directory)
+                                              (repository-discover "."))))
             (commit signer (match (command-line-arguments options)
                              ((commit signer)
                               (values commit signer))
-- 
2.41.0

[bug#69780] [PATCH 3/4] git authenticate: Install pre-push and post-checkout hooks.

[Ludovic Courtès]

* guix/scripts/git/authenticate.scm (install-hooks): New procedure.
(guix-git-authenticate): Use it.
* doc/guix.texi (Invoking guix git authenticate): Document it.

Change-Id: I4464a33193186e85b476a12740e54412bd58429c
---
 doc/guix.texi                     |  5 ++++
 guix/scripts/git/authenticate.scm | 43 ++++++++++++++++++++++++++++++-
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index ac0766b98c..b1672803c0 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -7624,6 +7624,11 @@ Invoking guix git authenticate
 guix git authenticate [@var{options}@dots{}]
 @end example
 
+The first run also attempts to install pre-push and post-checkout hooks,
+such that @command{guix git authenticate} is invoked as soon as you run
+@command{git push}, @command{git checkout}, and related commands; it
+does not overwrite preexisting hooks though.
+
 The options below allow you to fine-tune the process.
 
 @table @code
diff --git a/guix/scripts/git/authenticate.scm b/guix/scripts/git/authenticate.scm
index 36e1aa6228..13e1de3099 100644
--- a/guix/scripts/git/authenticate.scm
+++ b/guix/scripts/git/authenticate.scm
@@ -129,6 +129,46 @@ (define* (record-configuration repository
   (info (G_ "introduction and keyring configuration recorded in '~a'~%")
         config-file))
 
+(define (install-hooks repository)
+  "Attempt to install in REPOSITORY pre-push and update hooks that invoke
+'guix git authenticate'.  Bail out if one of these already exists."
+  (define directory
+    (repository-directory repository))
+
+  (define pre-push-hook
+    (in-vicinity directory "hooks/pre-push"))
+
+  (define post-checkout-hook
+    (in-vicinity directory "hooks/post-checkout"))
+
+  (if (or (file-exists? pre-push-hook)
+          (file-exists? post-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.")))
+      (begin
+        (call-with-output-file pre-push-hook
+          (lambda (port)
+            (format port "#!/bin/sh
+set -e
+while read local_ref local_oid remote_ref remote_oid
+do
+  guix git authenticate --end=\"$local_ref\"
+done\n")
+            (chmod port #o755)))
+        (call-with-output-file post-checkout-hook
+          (lambda (port)
+            (format port "#!/bin/sh
+oldrev=\"$1\"
+newrev=\"$2\"
+exec guix git authenticate --end=\"$newrev\"\n")
+            (chmod port #o755)))
+        (info (G_ "installed hooks '~a' and '~a'~%")
+              pre-push-hook post-checkout-hook))))
+
 (define (show-stats stats)
   "Display STATS, an alist containing commit signing stats as returned by
 'authenticate-repository'."
@@ -250,7 +290,8 @@ (define (guix-git-authenticate . args)
        (unless (configured? repository)
          (record-configuration repository
                                #:commit commit #:signer signer
-                               #:keyring-reference keyring))
+                               #:keyring-reference keyring)
+         (install-hooks repository))
 
        (when (and show-stats? (not (null? stats)))
          (show-stats stats))))))
-- 
2.41.0

[bug#69780] [PATCH 3/4] git authenticate: Install pre-push and post-checkout hooks.

[Tomas Volf]

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

On 2024-03-13 18:42:21 +0100, Ludovic Courtès wrote:
> * guix/scripts/git/authenticate.scm (install-hooks): New procedure.
> (guix-git-authenticate): Use it.
> * doc/guix.texi (Invoking guix git authenticate): Document it.
>
> Change-Id: I4464a33193186e85b476a12740e54412bd58429c
> ---
>  doc/guix.texi                     |  5 ++++
>  guix/scripts/git/authenticate.scm | 43 ++++++++++++++++++++++++++++++-
>  2 files changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index ac0766b98c..b1672803c0 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -7624,6 +7624,11 @@ Invoking guix git authenticate
>  guix git authenticate [@var{options}@dots{}]
>  @end example
>
> +The first run also attempts to install pre-push and post-checkout hooks,
> +such that @command{guix git authenticate} is invoked as soon as you run
> +@command{git push}, @command{git checkout}, and related commands; it
> +does not overwrite preexisting hooks though.
> +
>  The options below allow you to fine-tune the process.
>
>  @table @code
> diff --git a/guix/scripts/git/authenticate.scm b/guix/scripts/git/authenticate.scm
> index 36e1aa6228..13e1de3099 100644
> --- a/guix/scripts/git/authenticate.scm
> +++ b/guix/scripts/git/authenticate.scm
> @@ -129,6 +129,46 @@ (define* (record-configuration repository
>    (info (G_ "introduction and keyring configuration recorded in '~a'~%")
>          config-file))
>
> +(define (install-hooks repository)
> +  "Attempt to install in REPOSITORY pre-push and update hooks that invoke
> +'guix git authenticate'.  Bail out if one of these already exists."
> +  (define directory
> +    (repository-directory repository))
> +
> +  (define pre-push-hook
> +    (in-vicinity directory "hooks/pre-push"))
> +
> +  (define post-checkout-hook
> +    (in-vicinity directory "hooks/post-checkout"))

I think these will not work with worktrees.

> +
> +  (if (or (file-exists? pre-push-hook)
> +          (file-exists? post-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.")))
> +      (begin
> +        (call-with-output-file pre-push-hook
> +          (lambda (port)
> +            (format port "#!/bin/sh
> +set -e
> +while read local_ref local_oid remote_ref remote_oid
> +do
> +  guix git authenticate --end=\"$local_ref\"

Am I right in believing that the --end does solve #69541?  Shame it (--end) is
not documented in the --help.

> +done\n")
> +            (chmod port #o755)))

What is role of etc/git/pre-push now?  Should it be removed?  Should it be
updated to run this code instead?

> +        (call-with-output-file post-checkout-hook
> +          (lambda (port)
> +            (format port "#!/bin/sh
> +oldrev=\"$1\"
> +newrev=\"$2\"
> +exec guix git authenticate --end=\"$newrev\"\n")
> +            (chmod port #o755)))
> +        (info (G_ "installed hooks '~a' and '~a'~%")
> +              pre-push-hook post-checkout-hook))))
> +
>  (define (show-stats stats)
>    "Display STATS, an alist containing commit signing stats as returned by
>  'authenticate-repository'."
> @@ -250,7 +290,8 @@ (define (guix-git-authenticate . args)
>         (unless (configured? repository)
>           (record-configuration repository
>                                 #:commit commit #:signer signer
> -                               #:keyring-reference keyring))
> +                               #:keyring-reference keyring)
> +         (install-hooks repository))
>
>         (when (and show-stats? (not (null? stats)))
>           (show-stats stats))))))
> --
> 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 --]

[bug#69780] [PATCH 3/4] git authenticate: Install pre-push and post-checkout hooks.

[Ludovic Courtès]

Tomas Volf <~@wolfsden.cz> skribis:

> On 2024-03-13 18:42:21 +0100, Ludovic Courtès wrote:
>> * guix/scripts/git/authenticate.scm (install-hooks): New procedure.
>> (guix-git-authenticate): Use it.
>> * doc/guix.texi (Invoking guix git authenticate): Document it.
>>
>> Change-Id: I4464a33193186e85b476a12740e54412bd58429c

[...]

>> +  (define directory
>> +    (repository-directory repository))
>> +
>> +  (define pre-push-hook
>> +    (in-vicinity directory "hooks/pre-push"))
>> +
>> +  (define post-checkout-hook
>> +    (in-vicinity directory "hooks/post-checkout"))
>
> I think these will not work with worktrees.

Right.  I’m not sure Guile-Git can help here.

>> +while read local_ref local_oid remote_ref remote_oid
>> +do
>> +  guix git authenticate --end=\"$local_ref\"
>
> Am I right in believing that the --end does solve #69541?  Shame it (--end) is
> not documented in the --help.

Indeed, I’ll add it.

>> +done\n")
>> +            (chmod port #o755)))
>
> What is role of etc/git/pre-push now?  Should it be removed?  Should it be
> updated to run this code instead?

We could discuss it, but this is orthogonal.  The changes here are meant
for broad usage of ‘guix git authenticate’, not (or not just) within
Guix.

Ludo’.

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

[Ludovic Courtès]

* etc/news.scm: Add entry.

Change-Id: I661a0c0bfc373b87a70508ad9a735315c96ba4a5
---
 etc/news.scm | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/etc/news.scm b/etc/news.scm
index ab7fa4c0d5..ff0428be29 100644
--- a/etc/news.scm
+++ b/etc/news.scm
@@ -28,6 +28,22 @@
 (channel-news
  (version 0)
 
+ (entry (commit "TODO")
+        (title
+         (en "@command{guix git authenticate} usage simplified"))
+        (body
+         (en "Usage of the @command{guix git authenticate} command has been
+simplified.  The command is useful to channel authors and to developers
+willing to validate the provenance of their code.
+
+On your first use, @command{guix git authenticate} will now record the commit
+and signer (the @dfn{introduction}) in the @file{.git/config} file of your
+repository so that you don't have to pass them on the command line in
+subsequent runs.  It will also install pre-push and post-checkout hooks,
+unless preexisting hooks are found.
+
+Run @command{info \"(guix) Invoking guix authenticate\"} for more info.")))
+
  (entry (commit "ff1251de0bc327ec478fc66a562430fbf35aef42")
         (title
          (en "Daemon vulnerability allowing store corruption has been fixed")
-- 
2.41.0

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

[pelzflorian (Florian Pelz)]

Hi there.

Haven’t tested, but looks like now I won’t get confused which guix git
authenticate command from my bash history is the right one for which
repository. :)

Could you add this German translation:

Ludovic Courtès <ludo@gnu.org> writes:
> +        (title
> +         (en "@command{guix git authenticate} usage simplified")

          (de "@command{guix git authenticate} ist leichter nutzbar"))


> )
> +        (body
> +         (en "Usage of the @command{guix git authenticate} command

          (de "Der Befehl @command{guix git authenticate} kann jetzt einfacher
benutzt werden.  Mit dem Befehl können Kanalautoren und Entwickler die
Provenienz ihres Codes überprüfen.

Beim ersten Gebrauch speichert @command{guix git authenticate} Commit und
Unterzeichner (wie in der @dfn{Kanaleinführung}) in der Datei
@file{.git/config} Ihres Repositorys, so dass Sie sie bei späteren
Ausführungen nicht mehr auf der Befehlszeile angeben müssen.  Auch werden
Git-Hooks für pre-push und post-checkout installiert, wenn es bisher keine
Hooks dieser Art gibt.

Führen Sie @command{info \"(guix.de) Aufruf von guix git authenticate\"}
aus, wenn Sie mehr wissen wollen.")))




Regards,
Florian

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

[Ludovic Courtès]

Hi!

"pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> skribis:

> Haven’t tested, but looks like now I won’t get confused which guix git
> authenticate command from my bash history is the right one for which
> repository. :)

Heheh.  :-)

> Could you add this German translation:

Will do, thanks!

Ludo’.

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

[Skyler Ferris via Guix-patches via]

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

Hello Ludo’,
This looks like a great way to improve protection for guix developers. Saving the input, which still initially comes from the developer, and using it automatically in the future means less overhead for developers and less chances for something to go wrong in the process. I'm trying it locally on my machine and there are a few things I noticed.

> +  (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. This runs `make authenticate` itself but also runs `make check-channel-news`. I don't think we can just get rid of that file because then people would lose check-channel-news, but it seems useful to have this functionality built into the authenticate script so that other projects can use it. Unfortunately, unconditionally appending to the script could cause problems because the developer could have added their own hook which could have been written in any language. Perhaps we could update etc/git/pre-push to call the authenticate script in the new way and install any hooks that do not clobber (eg, if pre-push exists then we skip that hook but install the rest)?

> +  (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.

> +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".

> +(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.

> +(define (configured? repository)
> +  "Return true if REPOSITORY already container introduction info in its
> +'config' file."

Typo: this should be "contains", not "container"

Regards,
Skyler

[-- Attachment #2: Type: text/html, Size: 5486 bytes --]

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

[Ludovic Courtès]

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 4/4] DRAFT news: Add entry for ‘guix git authenticate’ changes.

[Skyler Ferris via Guix-patches via]

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

On 3/19/24 07:12, Ludovic Courtès wrote:

> 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.

Fair enough, that can be addressed in a separate patch. On a related note, this patch reminded me of an idea I mentioned in a private thread with the security team about writing a package that provides `guix git authenticate` as a standalone script, which can be done by extracting the dependent modules and building just them (I have successfully done this for the "(guix build utils)" library, for shell-like guile scripts that have a lighter footprint than all of guix). This will help people who are installing guix from source for the first time. They will not have guix itself available, but it would be easier for them to use the standalone script than to install all of guix (which they will presumably be replacing anyway). Is there anything else around `guix git authenticate` you are working on that might conflict with this? I am starting to look at it now that I have been reminded.

> 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?

That gave me pause too, but I tested it with a pull that caused a fast-forward (no separate merge commit) and it still ran the hook. I looked for a `post-fetch` hook but couldn't find one, I agree that would be ideal.

> 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?

Hmm, I didn't realize that git limited the number of components available in config names, but it looks like you're correct - my suggestion of appending `.branch-name` caused git to produce an error that the config was invalid.

But we don't necessarily need git to be aware of the semantic meaning of the branch name since the value is calculated by the guile script. So we could just use a hyphen as a separator and have "introduction-commit-master" and "introduction-commit-feature" as separate values unrelated to each other (aside from being in the "guix authentication" namespace).

Regards,
Skyler

[-- Attachment #2: Type: text/html, Size: 4502 bytes --]

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

[Skyler Ferris via Guix-patches via]

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

On 3/20/24 18:43, Skyler Ferris wrote:

> That gave me pause too, but I tested it with a pull that caused a fast-forward (no separate merge commit) and it still ran the hook. I looked for a `post-fetch` hook but couldn't find one, I agree that would be ideal.

It turns out that a post-fetch hook has been discussed on the git mailing list. Notably, someone asked about it for a very similar use-case starting at (1). Much of the discussion is about the way this person implemented the hook, because they wanted to prevent refs from being updated instead of simply alerting the user of the potentially bad commits. There is also a categorical objection to the idea of implementing a post-fetch hook (2), in part because modifying refs could lead to problems for the environment (such as clients re-fetching, then rejecting, the same commits repeatedly leading to excess bandwidth use) (3). In spite of this, there are WIP patches at the end of the thread implementing a tweak-fetch hook at the end of the thread (4), but I don't think they were ever completed... haven't looked at those too closely yet but maybe there's something somewhere else about them.

(1) https://lore.kernel.org/git/5898be69-4211-d441-494d-93477179cf0e@gaspard.io/
(2) https://lore.kernel.org/git/25bd770c-6a48-5b5d-04cc-6d02784ea3e7@gaspard.io/
(3) https://lore.kernel.org/git/87lgg1bwmi.fsf@evledraar.gmail.com/
(4) https://lore.kernel.org/git/30753d19-d77d-1a1a-ba42-afcd6fbb4223@gaspard.io/

[-- Attachment #2: Type: text/html, Size: 2480 bytes --]

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

[Ludovic Courtès]

Hi!

Skyler Ferris <skyvine@protonmail.com> skribis:

> Fair enough, that can be addressed in a separate patch. On a related note, this patch reminded me of an idea I mentioned in a private thread with the security team about writing a package that provides `guix git authenticate` as a standalone script, which can be done by extracting the dependent modules and building just them (I have successfully done this for the "(guix build utils)" library, for shell-like guile scripts that have a lighter footprint than all of guix). This will help people who are installing guix from source for the first time. They will not have guix itself available, but it would be easier for them to use the standalone script than to install all of guix (which they will presumably be replacing anyway). Is there anything else around `guix git authenticate` you are working on that might conflict with this? I am starting to look at it now that I have been reminded.

I must say I’m usually focusing on the use case where people building
Guix from source already have a working Guix installation.  In that
case, it’s that installation that provides a source of trust.

For someone building entirely from source, I don’t know what the right
solution is.  It could be extracting ‘guix git authenticate’ as you say,
but then we’re just offsetting the problem.  We could just as well
recommend building from a signed tag (or signed source tarball) after
verifying it.  Dunno what a good bootstrapping story might be.

>> 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?
>
> That gave me pause too, but I tested it with a pull that caused a fast-forward (no separate merge commit) and it still ran the hook. I looked for a `post-fetch` hook but couldn't find one, I agree that would be ideal.

OK, thanks for checking.

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

[...]

> Hmm, I didn't realize that git limited the number of components available in config names, but it looks like you're correct - my suggestion of appending `.branch-name` caused git to produce an error that the config was invalid.
>
> But we don't necessarily need git to be aware of the semantic meaning of the branch name since the value is calculated by the guile script. So we could just use a hyphen as a separator and have "introduction-commit-master" and "introduction-commit-feature" as separate values unrelated to each other (aside from being in the "guix authentication" namespace).

Hmm right.  Not pretty, but why not.

We could still have ‘introduction-commit’ as a default, and
‘introduction-commit-BRANCH’ would take precedence over it when present.

Thanks for your feedback!

Ludo’.