[bug#68935] [PATCH 0/3] Add 'put' option to guix import.

[bug#68935] [PATCH 0/3] Add 'put' option to guix import.

[Herman Rimm via Guix-patches via]

Hi all,

The first patch makes (svn-fetch) invoke svn from a native package
instead of one from the current PATH, if any. The third patch adds the
'put' option to 'guix import', so new package definitions get inserted
among existing definitions into e.g. gnu/packages/crates-io.scm. 

Cheers,
Herman

Herman Rimm (3):
  svn-fetch: Require svn-command argument.
  guix: import: Wrap package expressions with define-public.
  guix: import: Put packages into modules in alphabetical order.

 doc/guix.texi           | 14 +++++-
 guix/build/svn.scm      |  5 +--
 guix/scripts/import.scm | 96 ++++++++++++++++++++++++++++++-----------
 guix/svn-download.scm   | 35 +++++++++++----
 4 files changed, 111 insertions(+), 39 deletions(-)


base-commit: a3df20db8c3b9c819db7ca29bf014e2544e389d4
-- 
2.41.0

[bug#68935] [PATCH 1/3] svn-fetch: Require svn-command argument.

[Herman Rimm via Guix-patches via]

* guix/build/svn.scm (svn-fetch): Require svn-command argument.
* guix/svn-download.scm: Pass svn-command to each (svn-fetch).

Change-Id: Ibc7d3a79e45374588f71ca4d4ac2685c60ff3c00
---
 guix/build/svn.scm    |  5 ++---
 guix/svn-download.scm | 35 ++++++++++++++++++++++++++---------
 2 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/guix/build/svn.scm b/guix/build/svn.scm
index 875d3c50ca..e8f168abc6 100644
--- a/guix/build/svn.scm
+++ b/guix/build/svn.scm
@@ -31,9 +31,8 @@ (define-module (guix build svn)
 ;;;
 ;;; Code:
 
-(define* (svn-fetch url revision directory
-                    #:key (svn-command "svn")
-                    (recursive? #t)
+(define* (svn-fetch url revision directory svn-command
+                    #:key (recursive? #t)
                     (user-name #f)
                     (password #f))
   "Fetch REVISION from URL into DIRECTORY.  REVISION must be an integer, and a
diff --git a/guix/svn-download.scm b/guix/svn-download.scm
index c6688908de..c4eeed3563 100644
--- a/guix/svn-download.scm
+++ b/guix/svn-download.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2014-2016, 2019, 2021-2023 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2014 Sree Harsha Totakura <sreeharsha@totakura.in>
 ;;; Copyright © 2017, 2019, 2021 Ricardo Wurmus <rekado@elephly.net>
+;;; Copyright © 2024 Herman Rimm <herman@rimm.ee>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -105,7 +106,7 @@ (define* (svn-fetch ref hash-algo hash
             (or (svn-fetch (getenv "svn url")
                            (string->number (getenv "svn revision"))
                            #$output
-                           #:svn-command #+(file-append svn "/bin/svn")
+                           #+(file-append svn "/bin/svn")
                            #:recursive? (match (getenv "svn recursive?")
                                           ("yes" #t)
                                           (_ #f))
@@ -195,7 +196,7 @@ (define* (svn-multi-fetch ref hash-algo hash
                               (if (string-suffix? "/" location)
                                   (string-append #$output "/" location)
                                   (string-append #$output "/" (dirname location)))
-                              #:svn-command #+(file-append svn "/bin/svn")
+                              #+(file-append svn "/bin/svn")
                               #:recursive? (match (getenv "svn recursive?")
                                              ("yes" #t)
                                              (_ #f))
@@ -245,25 +246,34 @@ (define* (svn-multi-fetch ref hash-algo hash
 
 (define* (download-svn-to-store store ref
                                 #:optional (name (basename (svn-reference-url ref)))
-                                #:key (log (current-error-port)))
+                                #:key (log (current-error-port))
+                                (svn (subversion-package)))
   "Download from REF, a <svn-reference> object to STORE.  Write progress
 reports to LOG."
   (call-with-temporary-directory
    (lambda (temp)
      (let ((result
             (parameterize ((current-output-port log))
-              (build:svn-fetch (svn-reference-url ref)
+              (with-imported-modules
+                  (source-module-closure '((guix build svn)
+                                           (guix build utils)))
+                #~(begin
+                    (use-modules (guix build svn)
+                                 (guix build utils))
+                    (svn-fetch (svn-reference-url ref)
                                (svn-reference-revision ref)
                                (string-append temp "/svn")
+                               #+(file-append svn "/bin/svn")
                                #:user-name (svn-reference-user-name ref)
-                               #:password (svn-reference-password ref)))))
+                               #:password (svn-reference-password ref)))))))
        (and result
             (add-to-store store name #t "sha256"
                           (string-append temp "/svn")))))))
 
 (define* (download-multi-svn-to-store store ref
                                       #:optional (name (basename (svn-multi-reference-url ref)))
-                                      #:key (log (current-error-port)))
+                                      #:key (log (current-error-port))
+                                      (svn (subversion-package)))
   "Download from REF, a <svn-multi-reference> object to STORE.  Write progress
 reports to LOG."
   (call-with-temporary-directory
@@ -272,16 +282,23 @@ (define* (download-multi-svn-to-store store ref
                    (let ((dir (string-append temp "/" (dirname location))))
                      (mkdir-p dir))
                    (parameterize ((current-output-port log))
-                     (build:svn-fetch (string-append (svn-multi-reference-url ref)
-                                                     "/" location)
+                     (with-imported-modules
+                         (source-module-closure '((guix build svn)
+                                                  (guix build utils)))
+                       #~(begin
+                           (use-modules (guix build svn)
+                                        (guix build utils))
+                           (svn-fetch (string-append (svn-multi-reference-url ref)
+                                                           "/" location)
                                       (svn-multi-reference-revision ref)
                                       (if (string-suffix? "/" location)
                                           (string-append temp "/" location)
                                           (string-append temp "/" (dirname location)))
+                                      #+(file-append svn "/bin/svn")
                                       #:recursive?
                                       (svn-multi-reference-recursive? ref)
                                       #:user-name (svn-multi-reference-user-name ref)
-                                      #:password (svn-multi-reference-password ref))))
+                                      #:password (svn-multi-reference-password ref))))))
                  (svn-multi-reference-locations ref))
           (add-to-store store name #t "sha256" temp)))))
 
-- 
2.41.0

[bug#68935] [PATCH 1/3] svn-fetch: Require svn-command argument.

[Ludovic Courtès]

Hi Herman,

Herman Rimm <herman@rimm.ee> skribis:

> * guix/build/svn.scm (svn-fetch): Require svn-command argument.
> * guix/svn-download.scm: Pass svn-command to each (svn-fetch).
>
> Change-Id: Ibc7d3a79e45374588f71ca4d4ac2685c60ff3c00

Usually we write all the entities (procedures, variables, etc.)
modified in the commit log to help review.

> -(define* (svn-fetch url revision directory
> -                    #:key (svn-command "svn")
> -                    (recursive? #t)
> +(define* (svn-fetch url revision directory svn-command
> +                    #:key (recursive? #t)
>                      (user-name #f)
>                      (password #f))

What’s the rationale for this change?

It’s a relatively common pattern in Guix build-side code to assume that
commands can be found in $PATH while still providing a way to specify
the command’s absolute file name.

>  (define* (download-svn-to-store store ref
>                                  #:optional (name (basename (svn-reference-url ref)))
> -                                #:key (log (current-error-port)))
> +                                #:key (log (current-error-port))
> +                                (svn (subversion-package)))
>    "Download from REF, a <svn-reference> object to STORE.  Write progress
>  reports to LOG."
>    (call-with-temporary-directory
>     (lambda (temp)
>       (let ((result
>              (parameterize ((current-output-port log))
> -              (build:svn-fetch (svn-reference-url ref)
> +              (with-imported-modules
> +                  (source-module-closure '((guix build svn)
> +                                           (guix build utils)))
> +                #~(begin
> +                    (use-modules (guix build svn)
> +                                 (guix build utils))
> +                    (svn-fetch (svn-reference-url ref)
>                                 (svn-reference-revision ref)
>                                 (string-append temp "/svn")
> +                               #+(file-append svn "/bin/svn")
>                                 #:user-name (svn-reference-user-name ref)
> -                               #:password (svn-reference-password ref)))))
> +                               #:password (svn-reference-password ref)))))))
>         (and result
>              (add-to-store store name #t "sha256"
>                            (string-append temp "/svn")))))))

This doesn’t have the desired effect: ‘download-svn-to-store’ will now
always return a gexp instead of returning either #f or a store file
name.

This procedure and ‘download-multi-svn-to-store’ (used by the texlive
importer) really expect “svn” to be in $PATH.  It cannot really be
avoided AFAICS.

Thanks,
Ludo’.

[bug#68935] [PATCH 2/3] guix: import: Wrap package expressions with define-public.

[Herman Rimm via Guix-patches via]

* guix/scripts/import.scm (guix-import): Wrap package expressions.

Change-Id: Ic4d986a4706a692b2fecd6fded8ac72ab6311687
---
 guix/scripts/import.scm | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/guix/scripts/import.scm b/guix/scripts/import.scm
index d2a1cee56e..80b1bec7fc 100644
--- a/guix/scripts/import.scm
+++ b/guix/scripts/import.scm
@@ -6,6 +6,7 @@
 ;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
 ;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
 ;;; Copyright © 2022 Philip McGrath <philip@philipmcgrath.com>
+;;; Copyright © 2024 Herman Rimm <herman@rimm.ee>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -23,6 +24,7 @@
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 (define-module (guix scripts import)
+  #:use-module (guix import utils)
   #:use-module (guix ui)
   #:use-module (guix scripts)
   #:use-module (guix read-print)
@@ -88,17 +90,23 @@ (define-command (guix-import . args)
                         (leave-on-EPIPE
                          (pretty-print-with-comments (current-output-port) expr)))))
            (match (apply (resolve-importer importer) args)
-             ((and expr (or ('package _ ...)
-                            ('let _ ...)
+             ((and expr ('package _ ...))
+              (print (package->definition expr)))
+             ((and expr (or ('let _ ...)
                             ('define-public _ ...)))
               (print expr))
              ((? list? expressions)
               (for-each (lambda (expr)
-                          (print expr)
-                          ;; Two newlines: one after the closing paren, and
-                          ;; one to leave a blank line.
-                          (newline) (newline))
-                        expressions))
+                  (match expr
+                         ((and expr ('package _ ...))
+                          (print (package->definition expr)))
+                         ((and expr (or ('let _ ...)
+                                        ('define-public _ ...)))
+                          (print expr)))
+                  ;; Two newlines: one after the closing paren, and
+                  ;; one to leave a blank line.
+                  (newline) (newline))
+                expressions))
              (x
               (leave (G_ "'~a' import failed~%") importer))))
          (let ((hint (string-closest importer importers #:threshold 3)))
-- 
2.41.0

[bug#68935] [PATCH 2/3] guix: import: Wrap package expressions with define-public.

[Ludovic Courtès]

Herman Rimm <herman@rimm.ee> skribis:

> * guix/scripts/import.scm (guix-import): Wrap package expressions.
>
> Change-Id: Ic4d986a4706a692b2fecd6fded8ac72ab6311687

Probably a good idea!

>             (match (apply (resolve-importer importer) args)
> -             ((and expr (or ('package _ ...)
> -                            ('let _ ...)
> +             ((and expr ('package _ ...))
> +              (print (package->definition expr)))
> +             ((and expr (or ('let _ ...)
>                              ('define-public _ ...)))
>                (print expr))

The (let …) case should be treated like the (package …) case.  It
corresponds to things like:

  (let ((commit "abcde"))
    (package
      …))

> +                  (match expr
> +                         ((and expr ('package _ ...))
> +                          (print (package->definition expr)))
> +                         ((and expr (or ('let _ ...)
> +                                        ('define-public _ ...)))
> +                          (print expr)))

Same here.

Also, please indent ‘match’ the same way as elsewhere in the code:

  (match lst
    ((x y z)
     …))

Ludo’.

[bug#68935] [PATCH 3/3] guix: import: Put packages into modules in alphabetical order.

[Herman Rimm via Guix-patches via]

* guix/scripts/import.scm (guix-import): Add 'put' option.
(try-import): Add variable.
* doc/guix.texi (Invoking guix import): Describe 'put' option.

Change-Id: Ied4fc95899c31fb2523a01139780fc9744aa63cb
---
 doc/guix.texi           |  14 +++++-
 guix/scripts/import.scm | 100 +++++++++++++++++++++++++++-------------
 2 files changed, 81 insertions(+), 33 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index b76df868f8..5ab4c0d2a8 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -124,6 +124,7 @@
 Copyright @copyright{} 2023 Saku Laesvuori@*
 Copyright @copyright{} 2023 Graham James Addis@*
 Copyright @copyright{} 2023 Tomas Volf@*
+Copyright @copyright{} 2024 Herman Rimm@*
 
 Permission is granted to copy, distribute and/or modify this document
 under the terms of the GNU Free Documentation License, Version 1.3 or
@@ -14178,12 +14179,21 @@ Invoking guix import
 The general syntax is:
 
 @example
-guix import @var{importer} @var{options}@dots{}
+guix import @var{option} @var{importer} @var{options}@dots{}
 @end example
 
 @var{importer} specifies the source from which to import package
 metadata, and @var{options} specifies a package identifier and other
-options specific to @var{importer}.
+options specific to @var{importer}. @command{guix import} itself has the
+following options:
+
+@table @code
+@item --put=@var{file}
+@itemx -p @var{file}
+Insert the package definition(s) that the @var{importer} generated into the
+specified @var{file}, either in alphabetical order among existing package
+definitions, or at the end of the file otherwise.
+@end table
 
 Some of the importers rely on the ability to run the @command{gpgv} command.
 For these, GnuPG must be installed and in @code{$PATH}; run @code{guix install
diff --git a/guix/scripts/import.scm b/guix/scripts/import.scm
index 80b1bec7fc..0d83483c4e 100644
--- a/guix/scripts/import.scm
+++ b/guix/scripts/import.scm
@@ -24,6 +24,7 @@
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 (define-module (guix scripts import)
+  #:use-module (guix build utils)
   #:use-module (guix import utils)
   #:use-module (guix ui)
   #:use-module (guix scripts)
@@ -67,10 +68,37 @@ (define (show-help)
   (display (G_ "
   -h, --help             display this help and exit"))
   (display (G_ "
+  -p, --put              put the import in a package module"))
+  (display (G_ "
   -V, --version          display version information and exit"))
   (newline)
   (show-bug-report-information))
 
+(define (try-import importer args finish)
+  (if (member importer importers)
+        (match (apply (resolve-importer importer) args)
+          ((and expr ('package _ ...))
+           (finish (package->definition expr)))
+          ((and expr (or ('let _ ...)
+                         ('define-public _ ...)))
+           (finish expr))
+          ((? list? expressions)
+           (for-each (lambda (expr)
+               (match expr
+                      ((and expr ('package _ ...))
+                       (finish (package->definition expr)))
+                      ((and expr (or ('let _ ...)
+                                     ('define-public _ ...)))
+                       (finish expr))))
+             expressions))
+          (x
+           (leave (G_ "'~a' import failed~%") importer)))
+      (let ((hint (string-closest importer importers #:threshold 3)))
+        (report-error (G_ "~a: invalid importer~%") importer)
+        (when hint
+          (display-hint (G_ "Did you mean @code{~a}?~%") hint))
+        (exit 1))))
+
 (define-command (guix-import . args)
   (category packaging)
   (synopsis "import a package definition from an external repository")
@@ -79,38 +107,48 @@ (define-command (guix-import . args)
     (()
      (format (current-error-port)
              (G_ "guix import: missing importer name~%")))
-    ((or ("-h") ("--help"))
+    ((or ("-h" _ ...) ("--help" _ ...))
      (leave-on-EPIPE (show-help))
      (exit 0))
-    ((or ("-V") ("--version"))
+    ((or ("-V" _ ...) ("--version" _ ...))
      (show-version-and-exit "guix import"))
+    ((or ("-p" module importer args ...)
+         ("--put" module importer args ...))
+     (let ((put (lambda (expr)
+                  (let ((line
+                    (call-with-input-file module
+                      (lambda (port)
+                        (do ((se (read-syntax port)
+                                 (read-syntax port)))
+                          ((match (syntax->datum se)
+                             (('define-public symbol _ ...)
+                              (string> (symbol->string symbol)
+                                       (symbol->string (cadr expr))))
+                             ((? eof-object?) #t)
+                             (_ #f))
+                           (if (eof-object? se)
+                             #f
+                             (car (string-split
+                                    (object->string*
+                                      (syntax->datum se) 0)
+                                    #\newline)))))))))
+                    (if line
+                        (substitute* module
+                          (((string-append "\\" line))
+                           (string-append
+                             (object->string* expr 0)
+                             "\n\n" line)))
+                        (let ((port (open-file module "a")))
+                          (pretty-print-with-comments port expr)
+                          (newline port)
+                          (close-port port)))))))
+       (try-import importer args put)))
     ((importer args ...)
-     (if (member importer importers)
-         (let ((print (lambda (expr)
-                        (leave-on-EPIPE
-                         (pretty-print-with-comments (current-output-port) expr)))))
-           (match (apply (resolve-importer importer) args)
-             ((and expr ('package _ ...))
-              (print (package->definition expr)))
-             ((and expr (or ('let _ ...)
-                            ('define-public _ ...)))
-              (print expr))
-             ((? list? expressions)
-              (for-each (lambda (expr)
-                  (match expr
-                         ((and expr ('package _ ...))
-                          (print (package->definition expr)))
-                         ((and expr (or ('let _ ...)
-                                        ('define-public _ ...)))
-                          (print expr)))
-                  ;; Two newlines: one after the closing paren, and
-                  ;; one to leave a blank line.
-                  (newline) (newline))
-                expressions))
-             (x
-              (leave (G_ "'~a' import failed~%") importer))))
-         (let ((hint (string-closest importer importers #:threshold 3)))
-           (report-error (G_ "~a: invalid importer~%") importer)
-           (when hint
-             (display-hint (G_ "Did you mean @code{~a}?~%") hint))
-           (exit 1))))))
+     (let ((print (lambda (expr)
+                    (leave-on-EPIPE
+                      (pretty-print-with-comments
+                        (current-output-port) expr)
+                      ;; Two newlines: one after the closing paren,
+                      ;; and one to leave a blank line.
+                      (newline) (newline)))))
+       (try-import importer args print)))))
-- 
2.41.0

[bug#68935] [PATCH 3/3] guix: import: Put packages into modules in alphabetical order.

[Ludovic Courtès]

Herman Rimm <herman@rimm.ee> skribis:

> * guix/scripts/import.scm (guix-import): Add 'put' option.
> (try-import): Add variable.
> * doc/guix.texi (Invoking guix import): Describe 'put' option.
>
> Change-Id: Ied4fc95899c31fb2523a01139780fc9744aa63cb

Nice!  This can certainly simplify people’s work!

>  @example
> -guix import @var{importer} @var{options}@dots{}
> +guix import @var{option} @var{importer} @var{options}@dots{}

Uh?  It looks weird at first.  :-)

For clarity, we should change it to something like:

  guix import [@var{global-options}@dots{}] @var{importer} @var{package} [@var{options}@dots{}]

… though I admit this doesn’t look great either.

> +@item --put=@var{file}
> +@itemx -p @var{file}
> +Insert the package definition(s) that the @var{importer} generated into the

How about calling it ‘--insert’?  It seems more natural.

> +(define (try-import importer args finish)

Please add a docstring for top-level procedures like this one:

  https://guix.gnu.org/manual/devel/en/html_node/Formatting-Code.html

> +  (if (member importer importers)
> +        (match (apply (resolve-importer importer) args)

Indentation of the second line is off.

> +          ((and expr ('package _ ...))
> +           (finish (package->definition expr)))
> +          ((and expr (or ('let _ ...)
> +                         ('define-public _ ...)))
> +           (finish expr))

Again (let …) should be treated like (package …).

> +          ((? list? expressions)
> +           (for-each (lambda (expr)
> +               (match expr
> +                      ((and expr ('package _ ...))
> +                       (finish (package->definition expr)))
> +                      ((and expr (or ('let _ ...)
> +                                     ('define-public _ ...)))
> +                       (finish expr))))

Likewise.  Also indentation of ‘match’ and that of the subsequent
clauses is off.

> -    ((or ("-h") ("--help"))
> +    ((or ("-h" _ ...) ("--help" _ ...))
>       (leave-on-EPIPE (show-help))
>       (exit 0))
> -    ((or ("-V") ("--version"))
> +    ((or ("-V" _ ...) ("--version" _ ...))
>       (show-version-and-exit "guix import"))

This change seems to be unrelated, right?

> +    ((or ("-p" module importer args ...)
> +         ("--put" module importer args ...))
> +     (let ((put (lambda (expr)
> +                  (let ((line
> +                    (call-with-input-file module
> +                      (lambda (port)
> +                        (do ((se (read-syntax port)
> +                                 (read-syntax port)))
> +                          ((match (syntax->datum se)
> +                             (('define-public symbol _ ...)
> +                              (string> (symbol->string symbol)
> +                                       (symbol->string (cadr expr))))
> +                             ((? eof-object?) #t)
> +                             (_ #f))
> +                           (if (eof-object? se)
> +                             #f
> +                             (car (string-split
> +                                    (object->string*
> +                                      (syntax->datum se) 0)
> +                                    #\newline)))))))))
> +                    (if line
> +                        (substitute* module
> +                          (((string-append "\\" line))
> +                           (string-append
> +                             (object->string* expr 0)
> +                             "\n\n" line)))
> +                        (let ((port (open-file module "a")))
> +                          (pretty-print-with-comments port expr)
> +                          (newline port)
> +                          (close-port port)))))))

This whole thing should be in a separate procedure, probably in (guix
utils), close to ‘edit-expression’, and we could call it
‘insert-expression’ and it would take the location where the caller
wants to insert an expression.

Then, separately, we need a procedure to determine that location based
on the alphabetical order to top-level ‘define-public’ occurrences.

Does that make sense?  We should try and add tests for these in
‘tests/utils.scm’.

Also please check out the coding style regarding use of ‘car’ etc.:

  https://guix.gnu.org/manual/devel/en/html_node/Data-Types-and-Pattern-Matching.html

Could you send an updated series?

Please let us know if anything’s unclear or if you need guidance.

Thank you!

Ludo’.

[bug#68935] [PATCH v2 1/6] doc: Note SVN dependency of texlive importer.

[Herman Rimm via Guix-patches via]

* doc/guix.texi (Invoking guix import): Note dependency and remove
  duplicated words.

Change-Id: I94320db8c8ab3569aa461876522d5560e37a19ea
---
 doc/guix.texi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index b76df868f8..2908085c36 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -14334,7 +14334,7 @@ Invoking guix import
 
 Information is extracted from the @file{DESCRIPTION} file of the package.
 
-The command command below imports metadata for the Cairo R package:
+The command below imports metadata for the Cairo R package:
 
 @example
 guix import cran Cairo
@@ -14394,10 +14394,10 @@ Invoking guix import
 database, a plain text file that is included in the
 @code{texlive-scripts} package.  The source code is downloaded from
 possibly multiple locations in the SVN repository of the Tex Live
-project.
+project.  Note that therefore SVN must be installed and in @code{$PATH};
+run @code{guix install subversion} if needed.
 
-The command command below imports metadata for the @code{fontspec}
-TeX package:
+The command below imports metadata for the @code{fontspec} TeX package:
 
 @example
 guix import texlive fontspec

base-commit: 513755d64debb44096f21e323a5b89a7a597d2ca
-- 
2.41.0

[bug#68935] [PATCH v2 2/6] import: Wrap package expressions with define-public.

[Herman Rimm via Guix-patches via]

* guix/scripts/import.scm (guix-import): Wrap package expressions.

Change-Id: Ic4d986a4706a692b2fecd6fded8ac72ab6311687
---
 guix/scripts/import.scm | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/guix/scripts/import.scm b/guix/scripts/import.scm
index d2a1cee56e..77fcfe3990 100644
--- a/guix/scripts/import.scm
+++ b/guix/scripts/import.scm
@@ -6,6 +6,7 @@
 ;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
 ;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
 ;;; Copyright © 2022 Philip McGrath <philip@philipmcgrath.com>
+;;; Copyright © 2024 Herman Rimm <herman@rimm.ee>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -23,6 +24,7 @@
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 (define-module (guix scripts import)
+  #:use-module (guix import utils)
   #:use-module (guix ui)
   #:use-module (guix scripts)
   #:use-module (guix read-print)
@@ -89,12 +91,18 @@ (define-command (guix-import . args)
                          (pretty-print-with-comments (current-output-port) expr)))))
            (match (apply (resolve-importer importer) args)
              ((and expr (or ('package _ ...)
-                            ('let _ ...)
-                            ('define-public _ ...)))
+                            ('let _ ...)))
+              (print (package->definition expr)))
+             ((and expr ('define-public _ ...))
               (print expr))
              ((? list? expressions)
               (for-each (lambda (expr)
-                          (print expr)
+                          (match expr
+                            ((and expr (or ('package _ ...)
+                                           ('let _ ...)))
+                             (print (package->definition expr)))
+                            ((and expr ('define-public _ ...))
+                             (print expr)))
                           ;; Two newlines: one after the closing paren, and
                           ;; one to leave a blank line.
                           (newline) (newline))
-- 
2.41.0

[bug#68935] [PATCH v2 3/6] utils: Add insert-expression procedure.

[Herman Rimm via Guix-patches via]

* guix/utils.scm (define-module): Use (guix read-print) and export
(insert-expression).
(insert-expression): Add procedure.
* tests/utils.scm ("insert-expression"): Add test.

Change-Id: I971a43a78aa6ecaaef33c1a7a0db4b287eb85036
---
 guix/utils.scm  | 11 +++++++++++
 tests/utils.scm | 14 ++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/guix/utils.scm b/guix/utils.scm
index e4e9d922e7..8f0bc2399e 100644
--- a/guix/utils.scm
+++ b/guix/utils.scm
@@ -20,6 +20,7 @@
 ;;; Copyright © 2023 Janneke Nieuwenhuizen <janneke@gnu.org>
 ;;; Copyright © 2023 Zheng Junjie <873216071@qq.com>
 ;;; Copyright © 2023 Foundation Devices, Inc. <hello@foundationdevices.com>
+;;; Copyright © 2024 Herman Rimm <herman@rimm.ee>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -38,6 +39,7 @@
 
 (define-module (guix utils)
   #:use-module (guix config)
+  #:use-module (guix read-print)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-11)
   #:use-module (srfi srfi-26)
@@ -145,6 +147,7 @@ (define-module (guix utils)
             go-to-location
             edit-expression
             delete-expression
+            insert-expression
 
             filtered-port
             decompressed-port
@@ -502,6 +505,14 @@ (define (delete-expression source-properties)
   "Delete the expression specified by SOURCE-PROPERTIES."
   (edit-expression source-properties (const "") #:include-trailing-newline? #t))
 
+(define (insert-expression source-properties expr)
+  "Insert EXPR before the top-level expression specified by
+SOURCE-PROPERTIES."
+  (let* ((expr (object->string* expr 0))
+         (insert (lambda (str)
+                   (string-append expr "\n\n" str))))
+    (edit-expression source-properties insert)))
+
 \f
 ;;;
 ;;; Keyword arguments.
diff --git a/tests/utils.scm b/tests/utils.scm
index 5664165c85..cd54112846 100644
--- a/tests/utils.scm
+++ b/tests/utils.scm
@@ -5,6 +5,7 @@
 ;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
 ;;; Copyright © 2021 Maxime Devos <maximedevos@telenet.be>
 ;;; Copyright © 2023 Foundation Devices, Inc. <hello@foundationdevices.com>
+;;; Copyright © 2024 Herman Rimm <herman@rimm.ee>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -274,6 +275,19 @@ (define (test-compression/decompression method run?)
                      string-reverse)
     (call-with-input-file temp-file get-string-all)))
 
+(test-equal "insert-expression"
+  "(define-public package-1\n  'package)\n
+(define-public package-2\n  'package)\n"
+  (begin
+    (call-with-output-file temp-file
+      (lambda (port)
+        (display "(define-public package-2\n  'package)\n" port)))
+    (insert-expression `((filename . ,temp-file)
+                         (line     . 0)
+                         (column   . 0))
+                       `(define-public package-1 'package))
+    (call-with-input-file temp-file get-string-all)))
+
 (test-equal "string-distance"
   '(0 1 1 5 5)
   (list
-- 
2.41.0

[bug#68935] [PATCH v2 3/6] utils: Add insert-expression procedure.

[Ludovic Courtès]

Herman Rimm <herman@rimm.ee> skribis:

> * guix/utils.scm (define-module): Use (guix read-print) and export
> (insert-expression).
> (insert-expression): Add procedure.
> * tests/utils.scm ("insert-expression"): Add test.
>
> Change-Id: I971a43a78aa6ecaaef33c1a7a0db4b287eb85036

Nice!

[bug#68935] [PATCH v2 4/6] utils: Add find-expression procedure.

[Herman Rimm via Guix-patches via]

* guix/utils.scm (find-expression): Add and export procedure.
* tests/utils.scm ("find-expression"): Add test.

Change-Id: Ie209df39c1f006b20aa6436fb1aef4c84b1694ee
---
 guix/utils.scm  | 24 ++++++++++++++++++++++++
 tests/utils.scm | 16 ++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/guix/utils.scm b/guix/utils.scm
index 8f0bc2399e..315feeb232 100644
--- a/guix/utils.scm
+++ b/guix/utils.scm
@@ -148,6 +148,7 @@ (define-module (guix utils)
             edit-expression
             delete-expression
             insert-expression
+            find-expression
 
             filtered-port
             decompressed-port
@@ -513,6 +514,29 @@ (define (insert-expression source-properties expr)
                    (string-append expr "\n\n" str))))
     (edit-expression source-properties insert)))
 
+(define (find-expression file expr proc)
+  "Search in FILE for a top-level expression which alphabetically
+succeeds EXPR. Call PROC with the location if found, or with #f
+otherwise."
+  (let* ((name (match expr
+                 (('define-public symbol _ ...)
+                  (symbol->string symbol))))
+         (source-properties
+           (call-with-input-file
+             file
+             (lambda (port)
+               (do ((syntax (read-syntax port)
+                            (read-syntax port)))
+                 ((match (syntax->datum syntax)
+                    (('define-public symbol _ ...)
+                     (string> (symbol->string symbol)
+                              name))
+                    ((? eof-object?) #t)
+                    (_ #f))
+                  (if (eof-object? syntax)
+                    #f (syntax-source syntax))))))))
+    (proc source-properties)))
+
 \f
 ;;;
 ;;; Keyword arguments.
diff --git a/tests/utils.scm b/tests/utils.scm
index cd54112846..feaed4b561 100644
--- a/tests/utils.scm
+++ b/tests/utils.scm
@@ -288,6 +288,22 @@ (define-public package-2\n  'package)\n"
                        `(define-public package-1 'package))
     (call-with-input-file temp-file get-string-all)))
 
+(test-equal "find-expression"
+  (list `((filename . ,temp-file) (line . 0) (column . 0))
+        `((filename . ,temp-file) (line . 5) (column . 0))
+        #f)
+  (begin
+    (call-with-output-file temp-file
+      (lambda (port)
+        (display "(define-public package-1\n  'foo)\n\n" port)
+        (display "(define foo 'bar)\n\n" port)
+        (display "(define-public package-2\n  'baz)\n" port)))
+    (map (lambda (expr)
+           (find-expression temp-file expr identity))
+         (list `(define-public package 'foo)
+               `(define-public package-1 'bar)
+               `(define-public package-2 'baz)))))
+
 (test-equal "string-distance"
   '(0 1 1 5 5)
   (list
-- 
2.41.0

[bug#68935] [PATCH v2 4/6] utils: Add find-expression procedure.

[Ludovic Courtès]

Herman Rimm <herman@rimm.ee> skribis:

> * guix/utils.scm (find-expression): Add and export procedure.
> * tests/utils.scm ("find-expression"): Add test.
>
> Change-Id: Ie209df39c1f006b20aa6436fb1aef4c84b1694ee

[...]

> +(define (find-expression file expr proc)
> +  "Search in FILE for a top-level expression which alphabetically
> +succeeds EXPR. Call PROC with the location if found, or with #f
> +otherwise."
> +  (let* ((name (match expr
> +                 (('define-public symbol _ ...)
> +                  (symbol->string symbol))))
> +         (source-properties
> +           (call-with-input-file
> +             file
> +             (lambda (port)
> +               (do ((syntax (read-syntax port)
> +                            (read-syntax port)))
> +                 ((match (syntax->datum syntax)
> +                    (('define-public symbol _ ...)
> +                     (string> (symbol->string symbol)
> +                              name))
> +                    ((? eof-object?) #t)
> +                    (_ #f))
> +                  (if (eof-object? syntax)
> +                    #f (syntax-source syntax))))))))
> +    (proc source-properties)))

I think it’d be clearer to:

  1. Omit ‘proc’ and always return the source properties of the thing
     that has been found.

  2. Pass a symbol instead of ‘expr’.

  3. Call it ‘find-definition-insertion-location’ to clarify that it’s
     really about finding where we want to insert a definition, hence
     alphabetical sorting.

BTW, the formatting above is unusual; for instance, ‘file’ would
normally appear on the same line as ‘call-with-input-file’.

Also, write:

  (and (not (eof-object? syntax))
       (syntax-source-syntax))

… instead of using ‘if’.

Thanks,
Ludo’.

[bug#68935] [PATCH v2 5/6] import: Insert packages into modules alphabetically.

[Herman Rimm via Guix-patches via]

* guix/scripts/import.scm (guix-import): Add 'insert' option.
(define-import): Add procedure.
* doc/guix.texi (Invoking guix import): Describe 'insert' option.

Change-Id: Id021095355283ade79acde120890eb2ce9173cf7
---
 doc/guix.texi           | 14 ++++++--
 guix/scripts/import.scm | 80 ++++++++++++++++++++++++++---------------
 2 files changed, 63 insertions(+), 31 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 2908085c36..c73db5cbbf 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -124,6 +124,7 @@
 Copyright @copyright{} 2023 Saku Laesvuori@*
 Copyright @copyright{} 2023 Graham James Addis@*
 Copyright @copyright{} 2023 Tomas Volf@*
+Copyright @copyright{} 2024 Herman Rimm@*
 
 Permission is granted to copy, distribute and/or modify this document
 under the terms of the GNU Free Documentation License, Version 1.3 or
@@ -14178,12 +14179,21 @@ Invoking guix import
 The general syntax is:
 
 @example
-guix import @var{importer} @var{options}@dots{}
+guix import [@var{global-options}@dots{}] @var{importer} @var{package} [@var{options}@dots{}]
 @end example
 
 @var{importer} specifies the source from which to import package
 metadata, and @var{options} specifies a package identifier and other
-options specific to @var{importer}.
+options specific to @var{importer}. @command{guix import} itself has the
+following @var{global-options}:
+
+@table @code
+@item --insert=@var{file}
+@itemx -i @var{file}
+Insert the package definition(s) that the @var{importer} generated into the
+specified @var{file}, either in alphabetical order among existing package
+definitions, or at the end of the file otherwise.
+@end table
 
 Some of the importers rely on the ability to run the @command{gpgv} command.
 For these, GnuPG must be installed and in @code{$PATH}; run @code{guix install
diff --git a/guix/scripts/import.scm b/guix/scripts/import.scm
index 77fcfe3990..850887e720 100644
--- a/guix/scripts/import.scm
+++ b/guix/scripts/import.scm
@@ -67,10 +67,39 @@ (define (show-help)
   (display (G_ "
   -h, --help             display this help and exit"))
   (display (G_ "
+  -i, --insert           insert into package module alphabetically"))
+  (display (G_ "
   -V, --version          display version information and exit"))
   (newline)
   (show-bug-report-information))
 
+(define (define-import importer args proc)
+  "Wrap package expressions from IMPORTER with 'define-public and invoke
+PROC callback."
+  (if (member importer importers)
+      (match (apply (resolve-importer importer) args)
+        ((and expr (or ('package _ ...)
+                       ('let _ ...)))
+         (proc (package->definition expr)))
+        ((and expr ('define-public _ ...))
+         (proc expr))
+        ((? list? expressions)
+         (for-each (lambda (expr)
+                     (match expr
+                       ((and expr (or ('package _ ...)
+                                      ('let _ ...)))
+                        (proc (package->definition expr)))
+                       ((and expr ('define-public _ ...))
+                        (proc expr))))
+                   expressions))
+        (x
+         (leave (G_ "'~a' import failed~%") importer)))
+      (let ((hint (string-closest importer importers #:threshold 3)))
+        (report-error (G_ "~a: invalid importer~%") importer)
+        (when hint
+          (display-hint (G_ "Did you mean @code{~a}?~%") hint))
+        (exit 1))))
+
 (define-command (guix-import . args)
   (category packaging)
   (synopsis "import a package definition from an external repository")
@@ -84,33 +113,26 @@ (define-command (guix-import . args)
      (exit 0))
     ((or ("-V") ("--version"))
      (show-version-and-exit "guix import"))
+    ((or ("-i" module importer args ...)
+         ("--insert" module importer args ...))
+     (let ((find-and-insert
+             (lambda (expr)
+               (let ((insert
+                       (lambda (source-properties)
+                         (if source-properties
+                          (insert-expression source-properties expr)
+                          (let ((port (open-file file "a")))
+                            (pretty-print-with-comments port expr)
+                            (newline port)
+                            (close-port port))))))
+                 (find-expression module expr insert)))))
+       (define-import importer args find-and-insert)))
     ((importer args ...)
-     (if (member importer importers)
-         (let ((print (lambda (expr)
-                        (leave-on-EPIPE
-                         (pretty-print-with-comments (current-output-port) expr)))))
-           (match (apply (resolve-importer importer) args)
-             ((and expr (or ('package _ ...)
-                            ('let _ ...)))
-              (print (package->definition expr)))
-             ((and expr ('define-public _ ...))
-              (print expr))
-             ((? list? expressions)
-              (for-each (lambda (expr)
-                          (match expr
-                            ((and expr (or ('package _ ...)
-                                           ('let _ ...)))
-                             (print (package->definition expr)))
-                            ((and expr ('define-public _ ...))
-                             (print expr)))
-                          ;; Two newlines: one after the closing paren, and
-                          ;; one to leave a blank line.
-                          (newline) (newline))
-                        expressions))
-             (x
-              (leave (G_ "'~a' import failed~%") importer))))
-         (let ((hint (string-closest importer importers #:threshold 3)))
-           (report-error (G_ "~a: invalid importer~%") importer)
-           (when hint
-             (display-hint (G_ "Did you mean @code{~a}?~%") hint))
-           (exit 1))))))
+     (let ((print (lambda (expr)
+                    (leave-on-EPIPE
+                      (pretty-print-with-comments
+                        (current-output-port) expr)
+                      ;; Two newlines: one after the closing paren, and
+                      ;; one to leave a blank line.
+                      (newline) (newline)))))
+       (define-import importer args print)))))
-- 
2.41.0

[bug#68935] [PATCH v2 5/6] import: Insert packages into modules alphabetically.

[Herman Rimm via Guix-patches via]

Hi,

On Fri, Feb 09, 2024 at 08:25:17PM +0100, Herman Rimm wrote:
> diff --git a/guix/scripts/import.scm b/guix/scripts/import.scm
> index 77fcfe3990..850887e720 100644
> --- a/guix/scripts/import.scm
> +++ b/guix/scripts/import.scm
> @@ -67,10 +67,39 @@ (define (show-help)
>    (display (G_ "
>    -h, --help             display this help and exit"))
>    (display (G_ "
> +  -i, --insert           insert into package module alphabetically"))
> +  (display (G_ "
>    -V, --version          display version information and exit"))
>    (newline)
>    (show-bug-report-information))
>  
> +(define (define-import importer args proc)
> +  "Wrap package expressions from IMPORTER with 'define-public and invoke
> +PROC callback."
> +  (if (member importer importers)
> +      (match (apply (resolve-importer importer) args)
> +        ((and expr (or ('package _ ...)
> +                       ('let _ ...)))
> +         (proc (package->definition expr)))
> +        ((and expr ('define-public _ ...))

With doc/package-hello.json file:

  [
    {
      "name": "myhello",
      "version": "2.10",
      "source": "mirror://gnu/hello/hello-2.10.tar.gz",
      "build-system": "gnu",
      "arguments": {
        "tests?": false
      },
      "home-page": "https://www.gnu.org/software/hello/",
      "synopsis": "Hello, GNU world: An example GNU package",
      "description": "GNU Hello prints a greeting.",
      "license": "GPL-3.0+",
      "native-inputs": ["gettext"]
    }
  ]

'guix import json doc/package-hello.json' produces:

  Starting download of /tmp/guix-file.BLTipY
  From https://ftpmirror.gnu.org/gnu/hello/hello-2.10.tar.gz...
  following redirection to `https://mirrors.ibiblio.org/gnu/hello/hello-2.10.tar.gz'...
   …10.tar.gz  709KiB                   1.6MiB/s 00:00 ▕██████████████████▏ 100.0%
  (define-public myhello
    (package
      (name "myhello")
      (version "2.10")
      (source
       (origin
         (method url-fetch)
         (uri (string-append "mirror://gnu/hello/hello-" version ".tar.gz"))
         (sha256
          (base32 "0ssi1wpaf7plaswqqjwigppsg5fyh99vdlb9kzl7c9lng89ndq1i"))))
      (build-system (@ (guix build-system gnu) gnu-build-system))
      (arguments
       `(#:tests? #f))
      (native-inputs (list (@ (gnu packages gettext) gnu-gettext)))
      (home-page "https://www.gnu.org/software/hello/")
      (synopsis "Hello, GNU world: An example GNU package")
      (description "GNU Hello prints a greeting.")
      (license license:gpl3+)))
  
  Backtrace:
             4 (primitive-load "/home/herman/.cache/guix/inferiors/umi…")
  In guix/ui.scm:
     2324:7  3 (run-guix . _)
    2287:10  2 (run-guix-command _ . _)
  In srfi/srfi-1.scm:
      634:9  1 (for-each #<procedure 7f31b8f81380 at guix/scripts/imp…> …)
  In guix/scripts/import.scm:
      88:21  0 (_ _)
  
  guix/scripts/import.scm:88:21: Throw to key `match-error' with args `("match" "no matching pattern" myhello)'.

I think I can remove the (myhello) variable expression after definition
from the JSON importer in a future revision. Just a heads up.

Also I provided a reduced package-hello.json, because after fixing the
comma syntax, importing the file gives:

  Starting download of /tmp/guix-file.dVsYSt
  From https://ftpmirror.gnu.org/gnu/hello/hello-2.10.tar.gz...
  following redirection to `https://mirrors.ibiblio.org/gnu/hello/hello-2.10.tar.gz'...
   …10.tar.gz  709KiB                   1.5MiB/s 00:00 ▕██████████████████▏ 100.0%
  
  Starting download of /tmp/guix-file.B2wURN
  From https://example.com/greeter-1.0.tar.gz...
  download failed "https://example.com/greeter-1.0.tar.gz" 404 "Not Found"
  
  Starting download of /tmp/guix-file.B2wURN
  From https://web.archive.org/web/20240210155936/https://example.com/greeter-1.0.tar.gz...
  following redirection to `https://web.archive.org/web/20221219143408/http://example.com/greeter-1.0.tar.gz'...
  download failed "https://web.archive.org/web/20221219143408/http://example.com/greeter-1.0.tar.gz" 404 "Not Found"
  Trying to use Disarchive to assemble /tmp/guix-file.B2wURN...
  could not find its Disarchive specification
  failed to download "/tmp/guix-file.B2wURN" from "https://example.com/greeter-1.0.tar.gz"
  Backtrace:
            13 (primitive-load "/home/herman/.cache/guix/inferiors/umi…")
  In guix/ui.scm:
     2324:7 12 (run-guix . _)
    2287:10 11 (run-guix-command _ . _)
  In guix/scripts/import.scm:
       80:6 10 (guix-import . _)
  In ice-9/boot-9.scm:
    1747:15  9 (with-exception-handler #<procedure 7f890505d3f0 at ic…> …)
  In guix/scripts/import/json.scm:
      91:16  8 (_)
  In ice-9/boot-9.scm:
    1747:15  7 (with-exception-handler #<procedure 7f890505d3c0 at ic…> …)
  In guix/import/json.scm:
      72:19  6 (_)
  In srfi/srfi-1.scm:
     460:18  5 (fold #<procedure 7f890424dd60 at guix/import/json.scm…> …)
  In guix/import/json.scm:
      81:55  4 (_ (("inputs" . #("myhello" "hello")) ("license" . #) …) …)
  In guix/import/utils.scm:
     543:12  3 (alist->package (("inputs" . #("myhello" "hello")) # …) _)
     162:33  2 (source-spec->object "https://example.com/greeter-1.0.t…")
  In ice-9/ports.scm:
     450:11  1 (call-with-input-file #f #<procedure 7f88f0ba9b40 at g…> …)
  In unknown file:
             0 (open-file #f "r" #:encoding #f #:guess-encoding #f)

Can greeter have a real source? What should it be?

Cheers,
Herman Rimm

[bug#68935] [PATCH v2 5/6] import: Insert packages into modules alphabetically.

[Herman Rimm via Guix-patches via]

Hi,

On Fri, Feb 09, 2024 at 08:25:17PM +0100, Herman Rimm wrote:
> +    ((or ("-i" module importer args ...)
> +         ("--insert" module importer args ...))
> +     (let ((find-and-insert
> +             (lambda (expr)
> +               (let ((insert
> +                       (lambda (source-properties)
> +                         (if source-properties
> +                          (insert-expression source-properties expr)
> +                          (let ((port (open-file file "a")))
It should be 'module' instead of 'file' here. I will rename 'module' to
'file' in the next revision though.

Cheers,
Herman

[bug#68935] [PATCH v2 5/6] import: Insert packages into modules alphabetically.

[Ludovic Courtès]

Herman Rimm <herman@rimm.ee> skribis:

> * guix/scripts/import.scm (guix-import): Add 'insert' option.
> (define-import): Add procedure.
> * doc/guix.texi (Invoking guix import): Describe 'insert' option.
>
> Change-Id: Id021095355283ade79acde120890eb2ce9173cf7

[...]

> +(define (define-import importer args proc)
> +  "Wrap package expressions from IMPORTER with 'define-public and invoke
> +PROC callback."

s/define-import/import-as-definitions/ for clarity?

> +        ((? list? expressions)

Rather: (expressions ...).

(It’s equivalent but more customary.)

Otherwise LGTM, thanks!

Ludo’.

[bug#68935] [PATCH v2 6/6] import: Discard args after --version and --help.

[Herman Rimm via Guix-patches via]

* guix/scripts/import.scm (guix-import): Discard args.

Change-Id: Icce5cd0daf9011f7ddde7904113b31b547f063ef
---
 guix/scripts/import.scm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/guix/scripts/import.scm b/guix/scripts/import.scm
index 850887e720..3d77d1af55 100644
--- a/guix/scripts/import.scm
+++ b/guix/scripts/import.scm
@@ -108,10 +108,10 @@ (define-command (guix-import . args)
     (()
      (format (current-error-port)
              (G_ "guix import: missing importer name~%")))
-    ((or ("-h") ("--help"))
+    ((or ("-h" _ ...) ("--help" _ ...))
      (leave-on-EPIPE (show-help))
      (exit 0))
-    ((or ("-V") ("--version"))
+    ((or ("-V" _ ...) ("--version" _ ...))
      (show-version-and-exit "guix import"))
     ((or ("-i" module importer args ...)
          ("--insert" module importer args ...))
-- 
2.41.0

[bug#68935] [PATCH v3 0/7] Add insert option to guix import.

[Herman Rimm via Guix-patches via]

Hi,

Thanks for the feedback. Biggest issue I see is, e.g.:

  (define-public package-bar)
  ;; Unlikely comment about package-bar.
  
  (define for-package-foo)
  ;; Comment related to package-foo.
  (define-public package-foo)

after inserting package-baz becomes:

  (define-public package-bar)
  ;; Unlikely comment about package-bar.
  
  (define for-package-foo)
  ;; Comment related to package-foo.
+ (define-public package-baz)
+
  (define-public package-foo)

but for a language read from top-to-bottom I would rather want:

  (define-public package-bar)
+  
+ (define for-package-baz)
  ;; Unlikely comment about package-bar.

  (define for-package-foo)
  ;; Comment related to package-foo.
  (define-public package-foo)

This would be the case if package-baz is inserted after package-bar,
rather than before package-foo. I don't intend to implement this though,
and for the large gnu/packages/crates-*.scm files, where alphabetical
--insert is most useful, I believe this issue is the least likely to
occur. So despite the issue, I think these patches can be upstreamed.

Cheers,
Herman

Herman Rimm (7):
  doc: Note SVN dependency of texlive importer.
  import: Wrap package expressions with define-public.
  utils: Add insert-expression procedure.
  utils: Add find-definition-insertion-location procedure.
  import: Insert packages into modules alphabetically.
  import: Discard args after --version and --help.
  import: Do not return package name with json importer.

 doc/guix.texi           | 22 ++++++++---
 doc/package-hello.json  |  6 +--
 guix/import/json.scm    | 13 +++----
 guix/scripts/import.scm | 82 ++++++++++++++++++++++++++++-------------
 guix/utils.scm          | 30 +++++++++++++++
 tests/utils.scm         | 28 ++++++++++++++
 6 files changed, 140 insertions(+), 41 deletions(-)


base-commit: e3c612a7de679c96b9eafdb0da500dcc18d9a101
-- 
2.41.0

[bug#68935] [PATCH v3 1/7] doc: Note SVN dependency of texlive importer.

[Herman Rimm via Guix-patches via]

* doc/guix.texi (Invoking guix import): Note dependency and remove
  duplicated words.

Change-Id: I94320db8c8ab3569aa461876522d5560e37a19ea
---
 doc/guix.texi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 9966a8e697..6bf358e762 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -14339,7 +14339,7 @@ Invoking guix import
 
 Information is extracted from the @file{DESCRIPTION} file of the package.
 
-The command command below imports metadata for the Cairo R package:
+The command below imports metadata for the Cairo R package:
 
 @example
 guix import cran Cairo
@@ -14399,10 +14399,10 @@ Invoking guix import
 database, a plain text file that is included in the
 @code{texlive-scripts} package.  The source code is downloaded from
 possibly multiple locations in the SVN repository of the Tex Live
-project.
+project.  Note that therefore SVN must be installed and in @code{$PATH};
+run @code{guix install subversion} if needed.
 
-The command command below imports metadata for the @code{fontspec}
-TeX package:
+The command below imports metadata for the @code{fontspec} TeX package:
 
 @example
 guix import texlive fontspec
-- 
2.41.0

[bug#68935] [PATCH v3 2/7] import: Wrap package expressions with define-public.

[Herman Rimm via Guix-patches via]

* guix/scripts/import.scm (guix-import): Wrap package expressions.

Change-Id: Ic4d986a4706a692b2fecd6fded8ac72ab6311687
---
 guix/scripts/import.scm | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/guix/scripts/import.scm b/guix/scripts/import.scm
index d2a1cee56e..77fcfe3990 100644
--- a/guix/scripts/import.scm
+++ b/guix/scripts/import.scm
@@ -6,6 +6,7 @@
 ;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
 ;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
 ;;; Copyright © 2022 Philip McGrath <philip@philipmcgrath.com>
+;;; Copyright © 2024 Herman Rimm <herman@rimm.ee>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -23,6 +24,7 @@
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 (define-module (guix scripts import)
+  #:use-module (guix import utils)
   #:use-module (guix ui)
   #:use-module (guix scripts)
   #:use-module (guix read-print)
@@ -89,12 +91,18 @@ (define-command (guix-import . args)
                          (pretty-print-with-comments (current-output-port) expr)))))
            (match (apply (resolve-importer importer) args)
              ((and expr (or ('package _ ...)
-                            ('let _ ...)
-                            ('define-public _ ...)))
+                            ('let _ ...)))
+              (print (package->definition expr)))
+             ((and expr ('define-public _ ...))
               (print expr))
              ((? list? expressions)
               (for-each (lambda (expr)
-                          (print expr)
+                          (match expr
+                            ((and expr (or ('package _ ...)
+                                           ('let _ ...)))
+                             (print (package->definition expr)))
+                            ((and expr ('define-public _ ...))
+                             (print expr)))
                           ;; Two newlines: one after the closing paren, and
                           ;; one to leave a blank line.
                           (newline) (newline))
-- 
2.41.0

[bug#68935] [PATCH v3 3/7] utils: Add insert-expression procedure.

[Herman Rimm via Guix-patches via]

* guix/utils.scm (define-module): Use (guix read-print) and export
(insert-expression).
(insert-expression): Add procedure.
* tests/utils.scm ("insert-expression"): Add test.

Change-Id: I971a43a78aa6ecaaef33c1a7a0db4b287eb85036
---
 guix/utils.scm  | 11 +++++++++++
 tests/utils.scm | 14 ++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/guix/utils.scm b/guix/utils.scm
index e4e9d922e7..8f0bc2399e 100644
--- a/guix/utils.scm
+++ b/guix/utils.scm
@@ -20,6 +20,7 @@
 ;;; Copyright © 2023 Janneke Nieuwenhuizen <janneke@gnu.org>
 ;;; Copyright © 2023 Zheng Junjie <873216071@qq.com>
 ;;; Copyright © 2023 Foundation Devices, Inc. <hello@foundationdevices.com>
+;;; Copyright © 2024 Herman Rimm <herman@rimm.ee>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -38,6 +39,7 @@
 
 (define-module (guix utils)
   #:use-module (guix config)
+  #:use-module (guix read-print)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-11)
   #:use-module (srfi srfi-26)
@@ -145,6 +147,7 @@ (define-module (guix utils)
             go-to-location
             edit-expression
             delete-expression
+            insert-expression
 
             filtered-port
             decompressed-port
@@ -502,6 +505,14 @@ (define (delete-expression source-properties)
   "Delete the expression specified by SOURCE-PROPERTIES."
   (edit-expression source-properties (const "") #:include-trailing-newline? #t))
 
+(define (insert-expression source-properties expr)
+  "Insert EXPR before the top-level expression specified by
+SOURCE-PROPERTIES."
+  (let* ((expr (object->string* expr 0))
+         (insert (lambda (str)
+                   (string-append expr "\n\n" str))))
+    (edit-expression source-properties insert)))
+
 \f
 ;;;
 ;;; Keyword arguments.
diff --git a/tests/utils.scm b/tests/utils.scm
index 5664165c85..cd54112846 100644
--- a/tests/utils.scm
+++ b/tests/utils.scm
@@ -5,6 +5,7 @@
 ;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
 ;;; Copyright © 2021 Maxime Devos <maximedevos@telenet.be>
 ;;; Copyright © 2023 Foundation Devices, Inc. <hello@foundationdevices.com>
+;;; Copyright © 2024 Herman Rimm <herman@rimm.ee>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -274,6 +275,19 @@ (define (test-compression/decompression method run?)
                      string-reverse)
     (call-with-input-file temp-file get-string-all)))
 
+(test-equal "insert-expression"
+  "(define-public package-1\n  'package)\n
+(define-public package-2\n  'package)\n"
+  (begin
+    (call-with-output-file temp-file
+      (lambda (port)
+        (display "(define-public package-2\n  'package)\n" port)))
+    (insert-expression `((filename . ,temp-file)
+                         (line     . 0)
+                         (column   . 0))
+                       `(define-public package-1 'package))
+    (call-with-input-file temp-file get-string-all)))
+
 (test-equal "string-distance"
   '(0 1 1 5 5)
   (list
-- 
2.41.0

[bug#68935] [PATCH v3 3/7] utils: Add insert-expression procedure.

[Ludovic Courtès]

Herman Rimm <herman@rimm.ee> skribis:

> * guix/utils.scm (define-module): Use (guix read-print) and export
> (insert-expression).
> (insert-expression): Add procedure.
> * tests/utils.scm ("insert-expression"): Add test.
>
> Change-Id: I971a43a78aa6ecaaef33c1a7a0db4b287eb85036

[...]

>  (define-module (guix utils)
>    #:use-module (guix config)
> +  #:use-module (guix read-print)

I took the liberty to change that to #:autoload so users of (guix utils)
don’t load (guix read-print) if they don’t actually need it.

Ludo’.

[bug#68935] [PATCH v3 4/7] utils: Add find-definition-insertion-location procedure.

[Herman Rimm via Guix-patches via]

* guix/utils.scm (find-definition-insertion-location): Add and export
procedure.
* tests/utils.scm ("find-definition-insertion-location"): Add test.

Change-Id: Ie17e1b4a94790f58518ce121411a38d357f49feb
---
 guix/utils.scm  | 19 +++++++++++++++++++
 tests/utils.scm | 14 ++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/guix/utils.scm b/guix/utils.scm
index 8f0bc2399e..82a78e2dfb 100644
--- a/guix/utils.scm
+++ b/guix/utils.scm
@@ -148,6 +148,7 @@ (define-module (guix utils)
             edit-expression
             delete-expression
             insert-expression
+            find-definition-insertion-location
 
             filtered-port
             decompressed-port
@@ -513,6 +514,24 @@ (define (insert-expression source-properties expr)
                    (string-append expr "\n\n" str))))
     (edit-expression source-properties insert)))
 
+(define (find-definition-insertion-location file term)
+  "Search in FILE for a top-level public definition whose defined term
+alphabetically succeeds TERM. Return the location if found, or #f
+otherwise."
+  (let ((search-term (symbol->string term)))
+    (call-with-input-file file
+      (lambda (port)
+        (do ((syntax (read-syntax port)
+                     (read-syntax port)))
+          ((match (syntax->datum syntax)
+             (('define-public current-term _ ...)
+              (string> (symbol->string current-term)
+                       search-term))
+             ((? eof-object?) #t)
+             (_ #f))
+           (and (not (eof-object? syntax))
+                (syntax-source syntax))))))))
+
 \f
 ;;;
 ;;; Keyword arguments.
diff --git a/tests/utils.scm b/tests/utils.scm
index cd54112846..52f3b58ede 100644
--- a/tests/utils.scm
+++ b/tests/utils.scm
@@ -288,6 +288,20 @@ (define-public package-2\n  'package)\n"
                        `(define-public package-1 'package))
     (call-with-input-file temp-file get-string-all)))
 
+(test-equal "find-definition-insertion-location"
+  (list `((filename . ,temp-file) (line . 0) (column . 0))
+        `((filename . ,temp-file) (line . 5) (column . 0))
+        #f)
+  (begin
+    (call-with-output-file temp-file
+      (lambda (port)
+        (display "(define-public package-1\n  'foo)\n\n" port)
+        (display "(define foo 'bar)\n\n" port)
+        (display "(define-public package-2\n  'baz)\n" port)))
+    (map (lambda (term)
+           (find-definition-insertion-location temp-file term))
+         (list 'package 'package-1 'package-2))))
+
 (test-equal "string-distance"
   '(0 1 1 5 5)
   (list
-- 
2.41.0

[bug#68935] [PATCH v3 5/7] import: Insert packages into modules alphabetically.

[Herman Rimm via Guix-patches via]

* guix/scripts/import.scm (guix-import): Add 'insert' option.
(import-as-definitions): Add procedure.
* doc/guix.texi (Invoking guix import): Describe 'insert' option.

Change-Id: Id87ea707123630e12bcb6788599acac6895b26c4
---
 doc/guix.texi           | 14 ++++++-
 guix/scripts/import.scm | 82 ++++++++++++++++++++++++++---------------
 2 files changed, 65 insertions(+), 31 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 6bf358e762..59838c5a17 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -124,6 +124,7 @@
 Copyright @copyright{} 2023 Saku Laesvuori@*
 Copyright @copyright{} 2023 Graham James Addis@*
 Copyright @copyright{} 2023 Tomas Volf@*
+Copyright @copyright{} 2024 Herman Rimm@*
 
 Permission is granted to copy, distribute and/or modify this document
 under the terms of the GNU Free Documentation License, Version 1.3 or
@@ -14183,12 +14184,21 @@ Invoking guix import
 The general syntax is:
 
 @example
-guix import @var{importer} @var{options}@dots{}
+guix import [@var{global-options}@dots{}] @var{importer} @var{package} [@var{options}@dots{}]
 @end example
 
 @var{importer} specifies the source from which to import package
 metadata, and @var{options} specifies a package identifier and other
-options specific to @var{importer}.
+options specific to @var{importer}. @command{guix import} itself has the
+following @var{global-options}:
+
+@table @code
+@item --insert=@var{file}
+@itemx -i @var{file}
+Insert the package definition(s) that the @var{importer} generated into the
+specified @var{file}, either in alphabetical order among existing package
+definitions, or at the end of the file otherwise.
+@end table
 
 Some of the importers rely on the ability to run the @command{gpgv} command.
 For these, GnuPG must be installed and in @code{$PATH}; run @code{guix install
diff --git a/guix/scripts/import.scm b/guix/scripts/import.scm
index 77fcfe3990..aca4e61f26 100644
--- a/guix/scripts/import.scm
+++ b/guix/scripts/import.scm
@@ -67,10 +67,39 @@ (define (show-help)
   (display (G_ "
   -h, --help             display this help and exit"))
   (display (G_ "
+  -i, --insert           insert packages into file alphabetically"))
+  (display (G_ "
   -V, --version          display version information and exit"))
   (newline)
   (show-bug-report-information))
 
+(define (import-as-definitions importer args proc)
+  "Wrap package expressions from IMPORTER with 'define-public and invoke
+PROC callback."
+  (if (member importer importers)
+      (match (apply (resolve-importer importer) args)
+        ((and expr (or ('package _ ...)
+                       ('let _ ...)))
+         (proc (package->definition expr)))
+        ((and expr ('define-public _ ...))
+         (proc expr))
+        ((expressions ...)
+         (for-each (lambda (expr)
+                     (match expr
+                       ((and expr (or ('package _ ...)
+                                      ('let _ ...)))
+                        (proc (package->definition expr)))
+                       ((and expr ('define-public _ ...))
+                        (proc expr))))
+                   expressions))
+        (x
+         (leave (G_ "'~a' import failed~%") importer)))
+      (let ((hint (string-closest importer importers #:threshold 3)))
+        (report-error (G_ "~a: invalid importer~%") importer)
+        (when hint
+          (display-hint (G_ "Did you mean @code{~a}?~%") hint))
+        (exit 1))))
+
 (define-command (guix-import . args)
   (category packaging)
   (synopsis "import a package definition from an external repository")
@@ -84,33 +113,28 @@ (define-command (guix-import . args)
      (exit 0))
     ((or ("-V") ("--version"))
      (show-version-and-exit "guix import"))
+    ((or ("-i" file importer args ...)
+         ("--insert" file importer args ...))
+     (let ((find-and-insert
+             (lambda (expr)
+               (match expr
+                 (('define-public term _ ...)
+                  (let ((source-properties
+                          (find-definition-insertion-location
+                            file term)))
+                    (if source-properties
+                      (insert-expression source-properties expr)
+                      (let ((port (open-file file "a")))
+                        (pretty-print-with-comments port expr)
+                        (newline port)
+                        (close-port port)))))))))
+       (import-as-definitions importer args find-and-insert)))
     ((importer args ...)
-     (if (member importer importers)
-         (let ((print (lambda (expr)
-                        (leave-on-EPIPE
-                         (pretty-print-with-comments (current-output-port) expr)))))
-           (match (apply (resolve-importer importer) args)
-             ((and expr (or ('package _ ...)
-                            ('let _ ...)))
-              (print (package->definition expr)))
-             ((and expr ('define-public _ ...))
-              (print expr))
-             ((? list? expressions)
-              (for-each (lambda (expr)
-                          (match expr
-                            ((and expr (or ('package _ ...)
-                                           ('let _ ...)))
-                             (print (package->definition expr)))
-                            ((and expr ('define-public _ ...))
-                             (print expr)))
-                          ;; Two newlines: one after the closing paren, and
-                          ;; one to leave a blank line.
-                          (newline) (newline))
-                        expressions))
-             (x
-              (leave (G_ "'~a' import failed~%") importer))))
-         (let ((hint (string-closest importer importers #:threshold 3)))
-           (report-error (G_ "~a: invalid importer~%") importer)
-           (when hint
-             (display-hint (G_ "Did you mean @code{~a}?~%") hint))
-           (exit 1))))))
+     (let ((print (lambda (expr)
+                    (leave-on-EPIPE
+                      (pretty-print-with-comments
+                        (current-output-port) expr)
+                      ;; Two newlines: one after the closing paren, and
+                      ;; one to leave a blank line.
+                      (newline) (newline)))))
+       (import-as-definitions importer args print)))))
-- 
2.41.0

[bug#68935] [PATCH v3 6/7] import: Discard args after --version and --help.

[Herman Rimm via Guix-patches via]

* guix/scripts/import.scm (guix-import): Discard args.

Change-Id: Icce5cd0daf9011f7ddde7904113b31b547f063ef
---
 guix/scripts/import.scm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/guix/scripts/import.scm b/guix/scripts/import.scm
index aca4e61f26..1f34cab088 100644
--- a/guix/scripts/import.scm
+++ b/guix/scripts/import.scm
@@ -108,10 +108,10 @@ (define-command (guix-import . args)
     (()
      (format (current-error-port)
              (G_ "guix import: missing importer name~%")))
-    ((or ("-h") ("--help"))
+    ((or ("-h" _ ...) ("--help" _ ...))
      (leave-on-EPIPE (show-help))
      (exit 0))
-    ((or ("-V") ("--version"))
+    ((or ("-V" _ ...) ("--version" _ ...))
      (show-version-and-exit "guix import"))
     ((or ("-i" file importer args ...)
          ("--insert" file importer args ...))
-- 
2.41.0

[bug#68935] [PATCH v3 7/7] import: Do not return package name with json importer.

[Herman Rimm via Guix-patches via]

* guix/import/json.scm (json->code): Do not return package names after
package expressions.
* doc/package-hello.json: Fix comma errors and use valid greeter URL.

Change-Id: Id71924e72f690a9bda5fbfdb65a443029adfd158
---
 doc/package-hello.json |  6 +++---
 guix/import/json.scm   | 13 ++++++-------
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/doc/package-hello.json b/doc/package-hello.json
index a47e266e4b..60193e97e6 100644
--- a/doc/package-hello.json
+++ b/doc/package-hello.json
@@ -6,7 +6,7 @@
     "build-system": "gnu",
     "arguments": {
       "tests?": false
-    }
+    },
     "home-page": "https://www.gnu.org/software/hello/",
     "synopsis": "Hello, GNU world: An example GNU package",
     "description": "GNU Hello prints a greeting.",
@@ -16,11 +16,11 @@
   {
     "name": "greeter",
     "version": "1.0",
-    "source": "https://example.com/greeter-1.0.tar.gz",
+    "source": "mirror://gnu/hello/hello-2.10.tar.gz",
     "build-system": "gnu",
     "arguments": {
       "test-target": "foo",
-      "parallel-build?": false,
+      "parallel-build?": false
     },
     "home-page": "https://example.com/",
     "synopsis": "Greeter using GNU Hello",
diff --git a/guix/import/json.scm b/guix/import/json.scm
index b87e9918c5..bf346a1bef 100644
--- a/guix/import/json.scm
+++ b/guix/import/json.scm
@@ -78,14 +78,13 @@ (define (json->code file-name)
                               #:result
                               (append result
                                       (list
-                                       (package->code (alist->package pkg names))
-                                       (string->symbol (assoc-ref pkg "name"))))))))
-                        (list #:names '()
-                              #:result '())
-                        packages))))
+                                       (package->code
+                                         (alist->package pkg names))))))))
+                   (list #:names '()
+                         #:result '())
+                   packages))))
           (package
-            (list (package->code (alist->package json))
-                  (string->symbol (assoc-ref json "name")))))))
+            (list (package->code (alist->package json)))))))
     (const #f)))
 
 (define (json->scheme-file file)
-- 
2.41.0

[bug#68935] [PATCH v3 0/7] Add insert option to guix import.

[Ludovic Courtès]

Hi,

Herman Rimm <herman@rimm.ee> skribis:

> after inserting package-baz becomes:
>
>   (define-public package-bar)
>   ;; Unlikely comment about package-bar.
>   
>   (define for-package-foo)
>   ;; Comment related to package-foo.
> + (define-public package-baz)
> +
>   (define-public package-foo)

I think that’s an acceptable limitation.  Also, usually one would write:

  (define-public foo
    ;; Comment related to foo.
    (package …))

This case is correctly handled.

Ludo’.

bug#68935: [PATCH v3 0/7] Add insert option to guix import.

[Ludovic Courtès]

Hi!

Herman Rimm <herman@rimm.ee> skribis:

>   doc: Note SVN dependency of texlive importer.
>   import: Wrap package expressions with define-public.
>   utils: Add insert-expression procedure.
>   utils: Add find-definition-insertion-location procedure.
>   import: Insert packages into modules alphabetically.
>   import: Discard args after --version and --help.
>   import: Do not return package name with json importer.

Pushed as b386c11e7804e0b577411d930b60f1e0a4a0382c, thanks!

Ludo’.