diff mbox series

[bug#61255,4/5] tests: pack: Fix indentation.

Message ID 20230203221409.15886-5-maxim.cournoyer@gmail.com
State New
Headers show
Series None | expand

Commit Message

Maxim Cournoyer Feb. 3, 2023, 10:14 p.m. UTC
* tests/pack.scm: Fix indentation.
---

 tests/pack.scm | 274 ++++++++++++++++++++++++-------------------------
 1 file changed, 137 insertions(+), 137 deletions(-)

Comments

Ludovic Courtès Feb. 12, 2023, 6:20 p.m. UTC | #1
Hi,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> * tests/pack.scm: Fix indentation.

[...]

>         (check   (gexp->derivation
> -                 "check-tarball"
> -                 (with-imported-modules '((guix build utils))

[...]

> +                    "check-tarball"
> +                  (with-imported-modules '((guix build utils))

I’m not convinced by the indentation rule for ‘gexp->derivation’ added
in 82daab42811a2e3c7684ebdf12af75ff0fa67b99: there’s no reason to treat
‘gexp->derivation’ differently from other procedures.

What about removing that rule from ‘.dir-locals.el’ and keeping
‘tests/pack.scm’ unchanged?

(Also not sure about the ‘computed-file’ rule.  For all these things,
we’ll want to keep Emacs and (guix read-print) in sync, too.)

Thanks,
Ludo’.
Maxim Cournoyer Feb. 16, 2023, 3:22 p.m. UTC | #2
Hi Ludovic,

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

> Hi,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> * tests/pack.scm: Fix indentation.
>
> [...]
>
>>         (check   (gexp->derivation
>> -                 "check-tarball"
>> -                 (with-imported-modules '((guix build utils))
>
> [...]
>
>> +                    "check-tarball"
>> +                  (with-imported-modules '((guix build utils))
>
> I’m not convinced by the indentation rule for ‘gexp->derivation’ added
> in 82daab42811a2e3c7684ebdf12af75ff0fa67b99: there’s no reason to treat
> ‘gexp->derivation’ differently from other procedures.

The benefit I saw was that writing

--8<---------------cut here---------------start------------->8---
 (gexp->derivation the-name
  #~(begin
     (the
       (multi-line
        (gexp)))))
--8<---------------cut here---------------end--------------->8---

Seemed most readable/natural.  I saw some code in our code base was
already indented that way, but Emacs wasn't happy about it then.

> What about removing that rule from ‘.dir-locals.el’ and keeping
> ‘tests/pack.scm’ unchanged?

I don't feel too strongly about it, but I thought being able to indent
as above was neater than having to drop the-name on the second like, or
have it indented like under:

--8<---------------cut here---------------start------------->8---
(gexp->derivation the-name
                   #~(begin
                      (the
                        (multi-line
                         (gexp)))))
--8<---------------cut here---------------end--------------->8---

> (Also not sure about the ‘computed-file’ rule.  For all these things,
> we’ll want to keep Emacs and (guix read-print) in sync, too.)

Ah!  Thanks for pointing that out.  I can address this separately.  It'd
be nice if (guix read-print) understood the rules under .dir-locals.el
:-).
Ludovic Courtès Feb. 23, 2023, 3:47 p.m. UTC | #3
Hi,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Hi,
>>
>> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>>
>>> * tests/pack.scm: Fix indentation.
>>
>> [...]
>>
>>>         (check   (gexp->derivation
>>> -                 "check-tarball"
>>> -                 (with-imported-modules '((guix build utils))
>>
>> [...]
>>
>>> +                    "check-tarball"
>>> +                  (with-imported-modules '((guix build utils))
>>
>> I’m not convinced by the indentation rule for ‘gexp->derivation’ added
>> in 82daab42811a2e3c7684ebdf12af75ff0fa67b99: there’s no reason to treat
>> ‘gexp->derivation’ differently from other procedures.
>
> The benefit I saw was that writing
>
>  (gexp->derivation the-name
>   #~(begin
>      (the
>        (multi-line
>         (gexp)))))

I understand, but you know, it’s best to avoid unilaterally changing
established conventions.  :-)

If and when there’s consensus about this change, (guix read-print)
should be updated.

Ludo’.
diff mbox series

Patch

diff --git a/tests/pack.scm b/tests/pack.scm
index a4c388d93e..2e3b9d0ca4 100644
--- a/tests/pack.scm
+++ b/tests/pack.scm
@@ -75,43 +75,43 @@  (define %ar-bootstrap %bootstrap-binutils)
                                         #:compressor %gzip-compressor
                                         #:archiver %tar-bootstrap))
        (check   (gexp->derivation
-                 "check-tarball"
-                 (with-imported-modules '((guix build utils))
-                   #~(begin
-                       (use-modules (guix build utils)
-                                    (srfi srfi-1))
-
-                       (define store
-                         ;; The unpacked store.
-                         (string-append "." (%store-directory) "/"))
-
-                       (define (canonical? file)
-                         ;; Return #t if FILE is read-only and its mtime is 1.
-                         (let ((st (lstat file)))
-                           (or (not (string-prefix? store file))
-                               (eq? 'symlink (stat:type st))
-                               (and (= 1 (stat:mtime st))
-                                    (zero? (logand #o222
-                                                   (stat:mode st)))))))
-
-                       (define bin
-                         (string-append "." #$profile "/bin"))
-
-                       (setenv "PATH"
-                               (string-append #$%tar-bootstrap "/bin"))
-                       (system* "tar" "xvf" #$tarball)
-                       (mkdir #$output)
-                       (exit
-                        (and (file-exists? (string-append bin "/guile"))
-                             (file-exists? store)
-                             (every canonical?
-                                    (find-files "." (const #t)
-                                                #:directories? #t))
-                             (string=? (string-append #$%bootstrap-guile "/bin")
-                                       (readlink bin))
-                             (string=? (string-append ".." #$profile
-                                                      "/bin/guile")
-                                       (readlink "bin/Guile")))))))))
+                    "check-tarball"
+                  (with-imported-modules '((guix build utils))
+                    #~(begin
+                        (use-modules (guix build utils)
+                                     (srfi srfi-1))
+
+                        (define store
+                          ;; The unpacked store.
+                          (string-append "." (%store-directory) "/"))
+
+                        (define (canonical? file)
+                          ;; Return #t if FILE is read-only and its mtime is 1.
+                          (let ((st (lstat file)))
+                            (or (not (string-prefix? store file))
+                                (eq? 'symlink (stat:type st))
+                                (and (= 1 (stat:mtime st))
+                                     (zero? (logand #o222
+                                                    (stat:mode st)))))))
+
+                        (define bin
+                          (string-append "." #$profile "/bin"))
+
+                        (setenv "PATH"
+                                (string-append #$%tar-bootstrap "/bin"))
+                        (system* "tar" "xvf" #$tarball)
+                        (mkdir #$output)
+                        (exit
+                         (and (file-exists? (string-append bin "/guile"))
+                              (file-exists? store)
+                              (every canonical?
+                                     (find-files "." (const #t)
+                                                 #:directories? #t))
+                              (string=? (string-append #$%bootstrap-guile "/bin")
+                                        (readlink bin))
+                              (string=? (string-append ".." #$profile
+                                                       "/bin/guile")
+                                        (readlink "bin/Guile")))))))))
     (built-derivations (list check))))
 
 ;; The following test needs guile-sqlite3, libgcrypt, etc. as a consequence of
@@ -132,16 +132,16 @@  (define bin
          (tarball (self-contained-tarball "tar-pack" profile
                                           #:localstatedir? #t))
          (check   (gexp->derivation
-                   "check-tarball"
-                   #~(let ((bin (string-append "." #$profile "/bin")))
-                       (setenv "PATH"
-                               (string-append #$%tar-bootstrap "/bin"))
-                       (system* "tar" "xvf" #$tarball)
-                       (mkdir #$output)
-                       (exit
-                        (and (file-exists? "var/guix/db/db.sqlite")
-                             (string=? (string-append #$%bootstrap-guile "/bin")
-                                       (readlink bin))))))))
+                      "check-tarball"
+                    #~(let ((bin (string-append "." #$profile "/bin")))
+                        (setenv "PATH"
+                                (string-append #$%tar-bootstrap "/bin"))
+                        (system* "tar" "xvf" #$tarball)
+                        (mkdir #$output)
+                        (exit
+                         (and (file-exists? "var/guix/db/db.sqlite")
+                              (string=? (string-append #$%bootstrap-guile "/bin")
+                                        (readlink bin))))))))
       (built-derivations (list check))))
 
   (unless store (test-skip 1))
@@ -155,44 +155,44 @@  (define bin
          (tarball (self-contained-tarball "tar-pack" tree
                                           #:localstatedir? #t))
          (check   (gexp->derivation
-                   "check-tarball"
-                   (with-extensions (list guile-sqlite3 guile-gcrypt)
-                     (with-imported-modules (source-module-closure
-                                             '((guix store database)))
-                       #~(begin
-                           (use-modules (guix store database)
-                                        (rnrs io ports)
-                                        (srfi srfi-1))
-
-                           (define (valid-file? basename data)
-                             (define file
-                               (string-append "./" #$tree "/" basename))
-
-                             (string=? (call-with-input-file (pk 'file file)
-                                         get-string-all)
-                                       data))
-
-                           (setenv "PATH"
-                                   (string-append #$%tar-bootstrap "/bin"))
-                           (system* "tar" "xvf" #$tarball)
-
-                           (sql-schema
-                            #$(local-file (search-path %load-path
-                                                       "guix/store/schema.sql")))
-                           (with-database "var/guix/db/db.sqlite" db
-                             ;; Make sure non-ASCII file names are properly
-                             ;; handled.
-                             (setenv "GUIX_LOCPATH"
-                                     #+(file-append glibc-utf8-locales
-                                                    "/lib/locale"))
-                             (setlocale LC_ALL "en_US.utf8")
-
-                             (mkdir #$output)
-                             (exit
-                              (and (every valid-file?
-                                          '("α" "λ")
-                                          '("alpha" "lambda"))
-                                   (integer? (path-id db #$tree)))))))))))
+                      "check-tarball"
+                    (with-extensions (list guile-sqlite3 guile-gcrypt)
+                      (with-imported-modules (source-module-closure
+                                              '((guix store database)))
+                        #~(begin
+                            (use-modules (guix store database)
+                                         (rnrs io ports)
+                                         (srfi srfi-1))
+
+                            (define (valid-file? basename data)
+                              (define file
+                                (string-append "./" #$tree "/" basename))
+
+                              (string=? (call-with-input-file (pk 'file file)
+                                          get-string-all)
+                                        data))
+
+                            (setenv "PATH"
+                                    (string-append #$%tar-bootstrap "/bin"))
+                            (system* "tar" "xvf" #$tarball)
+
+                            (sql-schema
+                             #$(local-file (search-path %load-path
+                                                        "guix/store/schema.sql")))
+                            (with-database "var/guix/db/db.sqlite" db
+                              ;; Make sure non-ASCII file names are properly
+                              ;; handled.
+                              (setenv "GUIX_LOCPATH"
+                                      #+(file-append glibc-utf8-locales
+                                                     "/lib/locale"))
+                              (setlocale LC_ALL "en_US.utf8")
+
+                              (mkdir #$output)
+                              (exit
+                               (and (every valid-file?
+                                           '("α" "λ")
+                                           '("alpha" "lambda"))
+                                    (integer? (path-id db #$tree)))))))))))
       (built-derivations (list check))))
 
   (unless store (test-skip 1))
@@ -207,33 +207,33 @@  (define file
                                 #:symlinks '(("/bin/Guile" -> "bin/guile"))
                                 #:localstatedir? #t))
          (check   (gexp->derivation
-                   "check-tarball"
-                   (with-imported-modules '((guix build utils))
-                     #~(begin
-                         (use-modules (guix build utils)
-                                      (ice-9 match))
-
-                         (define bin
-                           (string-append "." #$profile "/bin"))
-
-                         (setenv "PATH" (string-append #$%tar-bootstrap "/bin"))
-                         (mkdir "base")
-                         (with-directory-excursion "base"
-                           (invoke "tar" "xvf" #$tarball))
-
-                         (match (find-files "base" "layer.tar")
-                           ((layer)
-                            (invoke "tar" "xvf" layer)))
-
-                         (when
-                          (and (file-exists? (string-append bin "/guile"))
-                               (file-exists? "var/guix/db/db.sqlite")
-                               (file-is-directory? "tmp")
-                               (string=? (string-append #$%bootstrap-guile "/bin")
-                                         (pk 'binlink (readlink bin)))
-                               (string=? (string-append #$profile "/bin/guile")
-                                         (pk 'guilelink (readlink "bin/Guile"))))
-                          (mkdir #$output)))))))
+                      "check-tarball"
+                    (with-imported-modules '((guix build utils))
+                      #~(begin
+                          (use-modules (guix build utils)
+                                       (ice-9 match))
+
+                          (define bin
+                            (string-append "." #$profile "/bin"))
+
+                          (setenv "PATH" (string-append #$%tar-bootstrap "/bin"))
+                          (mkdir "base")
+                          (with-directory-excursion "base"
+                            (invoke "tar" "xvf" #$tarball))
+
+                          (match (find-files "base" "layer.tar")
+                            ((layer)
+                             (invoke "tar" "xvf" layer)))
+
+                          (when
+                              (and (file-exists? (string-append bin "/guile"))
+                                   (file-exists? "var/guix/db/db.sqlite")
+                                   (file-is-directory? "tmp")
+                                   (string=? (string-append #$%bootstrap-guile "/bin")
+                                             (pk 'binlink (readlink bin)))
+                                   (string=? (string-append #$profile "/bin/guile")
+                                             (pk 'guilelink (readlink "bin/Guile"))))
+                            (mkdir #$output)))))))
       (built-derivations (list check))))
 
   (unless store (test-skip 1))
@@ -248,31 +248,31 @@  (define bin
                                   #:symlinks '(("/bin" -> "bin"))
                                   #:localstatedir? #t))
          (check   (gexp->derivation
-                   "check-tarball"
-                   (with-imported-modules '((guix build utils))
-                     #~(begin
-                         (use-modules (guix build utils)
-                                      (ice-9 match))
-
-                         (define bin
-                           (string-append "." #$profile "/bin"))
-
-                         (setenv "PATH"
-                                 (string-append #$squashfs-tools "/bin"))
-                         (invoke "unsquashfs" #$image)
-                         (with-directory-excursion "squashfs-root"
-                           (when (and (file-exists? (string-append bin
-                                                                   "/guile"))
-                                      (file-exists? "var/guix/db/db.sqlite")
-                                      (string=? (string-append #$%bootstrap-guile "/bin")
-                                                (pk 'binlink (readlink bin)))
-
-                                      ;; This is a relative symlink target.
-                                      (string=? (string-drop
-                                                 (string-append #$profile "/bin")
-                                                 1)
-                                                (pk 'guilelink (readlink "bin"))))
-                             (mkdir #$output))))))))
+                      "check-tarball"
+                    (with-imported-modules '((guix build utils))
+                      #~(begin
+                          (use-modules (guix build utils)
+                                       (ice-9 match))
+
+                          (define bin
+                            (string-append "." #$profile "/bin"))
+
+                          (setenv "PATH"
+                                  (string-append #$squashfs-tools "/bin"))
+                          (invoke "unsquashfs" #$image)
+                          (with-directory-excursion "squashfs-root"
+                            (when (and (file-exists? (string-append bin
+                                                                    "/guile"))
+                                       (file-exists? "var/guix/db/db.sqlite")
+                                       (string=? (string-append #$%bootstrap-guile "/bin")
+                                                 (pk 'binlink (readlink bin)))
+
+                                       ;; This is a relative symlink target.
+                                       (string=? (string-drop
+                                                  (string-append #$profile "/bin")
+                                                  1)
+                                                 (pk 'guilelink (readlink "bin"))))
+                              (mkdir #$output))))))))
       (built-derivations (list check))))
 
   (unless store (test-skip 1))