unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#59661] [PATCH 0/3] Add e2fsprogs to %base-packages-utils.
@ 2022-11-28 19:18 Maxim Cournoyer
  2022-11-28 20:01 ` [bug#59661] [PATCH 1/3] system: Rename and move %base-packages-disk-utilities Maxim Cournoyer
  2022-12-04  4:43 ` [bug#59661] [PATCH v2 1/3] system: Rename and move %base-packages-disk-utilities Maxim Cournoyer
  0 siblings, 2 replies; 17+ messages in thread
From: Maxim Cournoyer @ 2022-11-28 19:18 UTC (permalink / raw)
  To: 59661; +Cc: Maxim Cournoyer

Hi Guix,

Here's a simple series adjusting some annoyance noticed by someone on
guix-help (no e2fsprogs in installer) and by myself (no lsattr/chattr on my
system).

Thank you,

Maxim Cournoyer (3):
  system: Rename and move %base-packages-disk-utilities.
  install: Add missing e2fsprogs utility.
  system: Add e2fsprogs to %base-packages-utils.

 gnu/system.scm         | 18 ++----------------
 gnu/system/install.scm | 17 ++++++++++++++++-
 2 files changed, 18 insertions(+), 17 deletions(-)


base-commit: 059d38dc3f8b087f4a42df586daeb05761ee18d7
-- 
2.38.1





^ permalink raw reply	[flat|nested] 17+ messages in thread

* [bug#59661] [PATCH 1/3] system: Rename and move %base-packages-disk-utilities.
  2022-11-28 19:18 [bug#59661] [PATCH 0/3] Add e2fsprogs to %base-packages-utils Maxim Cournoyer
@ 2022-11-28 20:01 ` Maxim Cournoyer
  2022-11-28 20:01   ` [bug#59661] [PATCH 2/3] install: Add missing e2fsprogs utility Maxim Cournoyer
  2022-11-28 20:01   ` [bug#59661] [PATCH 3/3] system: Add e2fsprogs to %base-packages-utils Maxim Cournoyer
  2022-12-04  4:43 ` [bug#59661] [PATCH v2 1/3] system: Rename and move %base-packages-disk-utilities Maxim Cournoyer
  1 sibling, 2 replies; 17+ messages in thread
From: Maxim Cournoyer @ 2022-11-28 20:01 UTC (permalink / raw)
  To: 59661; +Cc: Maxim Cournoyer

Rationale: It is only used in INSTALLATION-OS and doesn't make sense to be
used in another context, given that file systems now automatically pull their
dependencies since commit 45eac6cdf5c8d9d7b0c564b105c790d2d2007799 (services:
Add file system utilities to profile).

* gnu/system.scm (%base-packages-disk-utilities): Rename to...
* gnu/system/install.scm (%installer-disk-utilities): ... this.
(installation-os) [packages]: Adjust accordingly.
---
 gnu/system.scm         | 19 -------------------
 gnu/system/install.scm | 19 ++++++++++++++++++-
 2 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index 3478afcec4..3241a18288 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -49,9 +49,6 @@ (define-module (gnu system)
   #:use-module (gnu packages bash)
   #:use-module (gnu packages compression)
   #:use-module (gnu packages cross-base)
-  #:use-module (gnu packages cryptsetup)
-  #:use-module (gnu packages disk)
-  #:use-module (gnu packages file-systems)
   #:use-module (gnu packages firmware)
   #:use-module (gnu packages gawk)
   #:use-module (gnu packages guile)
@@ -180,7 +177,6 @@ (define-module (gnu system)
             %base-packages-interactive
             %base-packages-linux
             %base-packages-networking
-            %base-packages-disk-utilities
             %base-packages-utils
             %base-firmware
             %default-kernel-arguments))
@@ -896,21 +892,6 @@ (define %base-packages-networking
         ;; many people are familiar with, so keep it around.
         iw wireless-tools))
 
-(define %base-packages-disk-utilities
-  ;; A well-rounded set of packages for interacting with disks,
-  ;; partitions and filesystems, included with the Guix installation
-  ;; image.
-  (list parted gptfdisk ddrescue
-        ;; We used to provide fdisk from GNU fdisk, but as of version 2.0.0a
-        ;; it pulls Guile 1.8, which takes unreasonable space; furthermore
-        ;; util-linux's fdisk is already available, in %base-packages-linux.
-        cryptsetup mdadm
-        dosfstools
-        btrfs-progs
-        f2fs-tools
-        jfsutils
-        xfsprogs))
-
 (define %base-packages
   ;; Default set of packages globally visible.  It should include anything
   ;; required for basic administrator tasks.
diff --git a/gnu/system/install.scm b/gnu/system/install.scm
index 003c49a3e7..d34d974338 100644
--- a/gnu/system/install.scm
+++ b/gnu/system/install.scm
@@ -48,6 +48,9 @@ (define-module (gnu system install)
   #:use-module (gnu packages bootloaders)
   #:use-module (gnu packages certs)
   #:use-module (gnu packages compression)
+  #:use-module (gnu packages cryptsetup)
+  #:use-module (gnu packages disk)
+  #:use-module (gnu packages file-systems)
   #:use-module (gnu packages fonts)
   #:use-module (gnu packages fontutils)
   #:use-module (gnu packages guile)
@@ -458,6 +461,20 @@ (define %issue
 \x1b[1;33mUse Alt-F2 for documentation.\x1b[0m
 ")
 
+(define %installer-disk-utilities
+  ;; A well-rounded set of packages for interacting with disks, partitions and
+  ;; file systems, included with the Guix installation image.
+  (list parted gptfdisk ddrescue
+        ;; We used to provide fdisk from GNU fdisk, but as of version 2.0.0a
+        ;; it pulls Guile 1.8, which takes unreasonable space; furthermore
+        ;; util-linux's fdisk is already available, in %base-packages-linux.
+        cryptsetup mdadm
+        dosfstools
+        btrfs-progs
+        f2fs-tools
+        jfsutils
+        xfsprogs))
+
 (define installation-os
   ;; The operating system used on installation images for USB sticks etc.
   (operating-system
@@ -530,7 +547,7 @@ (define installation-os
                       font-dejavu font-gnu-unifont
                       grub          ; mostly so xrefs to its manual work
                       nss-certs)    ; To access HTTPS, use git, etc.
-                %base-packages-disk-utilities
+                %installer-disk-utilities
                 %base-packages))))
 
 (define* (os-with-u-boot os board #:key (bootloader-target "/dev/mmcblk0")

base-commit: 059d38dc3f8b087f4a42df586daeb05761ee18d7
-- 
2.38.1





^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [bug#59661] [PATCH 2/3] install: Add missing e2fsprogs utility.
  2022-11-28 20:01 ` [bug#59661] [PATCH 1/3] system: Rename and move %base-packages-disk-utilities Maxim Cournoyer
@ 2022-11-28 20:01   ` Maxim Cournoyer
  2022-11-28 20:01   ` [bug#59661] [PATCH 3/3] system: Add e2fsprogs to %base-packages-utils Maxim Cournoyer
  1 sibling, 0 replies; 17+ messages in thread
From: Maxim Cournoyer @ 2022-11-28 20:01 UTC (permalink / raw)
  To: 59661; +Cc: Maxim Cournoyer

* gnu/system/install.scm (%installer-disk-utilities): Add e2fsprogs.
---
 gnu/system/install.scm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gnu/system/install.scm b/gnu/system/install.scm
index d34d974338..f6f1923121 100644
--- a/gnu/system/install.scm
+++ b/gnu/system/install.scm
@@ -471,6 +471,7 @@ (define %installer-disk-utilities
         cryptsetup mdadm
         dosfstools
         btrfs-progs
+        e2fsprogs
         f2fs-tools
         jfsutils
         xfsprogs))
-- 
2.38.1





^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [bug#59661] [PATCH 3/3] system: Add e2fsprogs to %base-packages-utils.
  2022-11-28 20:01 ` [bug#59661] [PATCH 1/3] system: Rename and move %base-packages-disk-utilities Maxim Cournoyer
  2022-11-28 20:01   ` [bug#59661] [PATCH 2/3] install: Add missing e2fsprogs utility Maxim Cournoyer
@ 2022-11-28 20:01   ` Maxim Cournoyer
  1 sibling, 0 replies; 17+ messages in thread
From: Maxim Cournoyer @ 2022-11-28 20:01 UTC (permalink / raw)
  To: 59661; +Cc: Maxim Cournoyer

Rationale: Even when not using an ext file system, the utilities provided by
e2fsprogs are useful, for example to set the copy-on-write attribute of a
Btrfs file system.

* gnu/system.scm (%base-packages-utils): Add e2fsprogs.
---
 gnu/system.scm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gnu/system.scm b/gnu/system.scm
index 3241a18288..84daf036f3 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -854,6 +854,8 @@ (define %base-packages-utils
  (cons* procps psmisc which
         (@ (gnu packages admin) shadow-with-man-pages) ;for 'passwd'
 
+        e2fsprogs                 ;for lsattr, chattr, etc.
+
         guile-3.0-latest
 
         ;; The packages below are also in %FINAL-INPUTS, so take them from
-- 
2.38.1





^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [bug#59661] [PATCH v2 1/3] system: Rename and move %base-packages-disk-utilities.
  2022-11-28 19:18 [bug#59661] [PATCH 0/3] Add e2fsprogs to %base-packages-utils Maxim Cournoyer
  2022-11-28 20:01 ` [bug#59661] [PATCH 1/3] system: Rename and move %base-packages-disk-utilities Maxim Cournoyer
@ 2022-12-04  4:43 ` Maxim Cournoyer
  2022-12-04  4:43   ` [bug#59661] [PATCH v2 2/3] install: Add missing e2fsprogs utility Maxim Cournoyer
                     ` (2 more replies)
  1 sibling, 3 replies; 17+ messages in thread
From: Maxim Cournoyer @ 2022-12-04  4:43 UTC (permalink / raw)
  To: 59661; +Cc: ludo, Maxim Cournoyer

Rationale: It is only used in INSTALLATION-OS and doesn't make sense to be
used in another context, given that file systems now automatically pull their
dependencies since commit 45eac6cdf5c8d9d7b0c564b105c790d2d2007799 (services:
Add file system utilities to profile).

* gnu/system.scm (%base-packages-disk-utilities): Deprecate and rename to...
* gnu/system/install.scm (%installer-disk-utilities): ... this.
(installation-os) [packages]: Adjust accordingly.
---
 gnu/system.scm         | 19 ++-----------------
 gnu/system/install.scm | 19 ++++++++++++++++++-
 2 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index 3478afcec4..1c119c31b6 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -38,6 +38,7 @@ (define-module (gnu system)
   #:use-module (guix gexp)
   #:use-module (guix records)
   #:use-module (guix packages)
+  #:use-module (guix deprecation)
   #:use-module (guix derivations)
   #:use-module (guix profiles)
   #:use-module ((guix utils) #:select (substitute-keyword-arguments))
@@ -49,9 +50,6 @@ (define-module (gnu system)
   #:use-module (gnu packages bash)
   #:use-module (gnu packages compression)
   #:use-module (gnu packages cross-base)
-  #:use-module (gnu packages cryptsetup)
-  #:use-module (gnu packages disk)
-  #:use-module (gnu packages file-systems)
   #:use-module (gnu packages firmware)
   #:use-module (gnu packages gawk)
   #:use-module (gnu packages guile)
@@ -896,20 +894,7 @@ (define %base-packages-networking
         ;; many people are familiar with, so keep it around.
         iw wireless-tools))
 
-(define %base-packages-disk-utilities
-  ;; A well-rounded set of packages for interacting with disks,
-  ;; partitions and filesystems, included with the Guix installation
-  ;; image.
-  (list parted gptfdisk ddrescue
-        ;; We used to provide fdisk from GNU fdisk, but as of version 2.0.0a
-        ;; it pulls Guile 1.8, which takes unreasonable space; furthermore
-        ;; util-linux's fdisk is already available, in %base-packages-linux.
-        cryptsetup mdadm
-        dosfstools
-        btrfs-progs
-        f2fs-tools
-        jfsutils
-        xfsprogs))
+(define-deprecated %base-packages-disk-utilities #f '())
 
 (define %base-packages
   ;; Default set of packages globally visible.  It should include anything
diff --git a/gnu/system/install.scm b/gnu/system/install.scm
index 003c49a3e7..d34d974338 100644
--- a/gnu/system/install.scm
+++ b/gnu/system/install.scm
@@ -48,6 +48,9 @@ (define-module (gnu system install)
   #:use-module (gnu packages bootloaders)
   #:use-module (gnu packages certs)
   #:use-module (gnu packages compression)
+  #:use-module (gnu packages cryptsetup)
+  #:use-module (gnu packages disk)
+  #:use-module (gnu packages file-systems)
   #:use-module (gnu packages fonts)
   #:use-module (gnu packages fontutils)
   #:use-module (gnu packages guile)
@@ -458,6 +461,20 @@ (define %issue
 \x1b[1;33mUse Alt-F2 for documentation.\x1b[0m
 ")
 
+(define %installer-disk-utilities
+  ;; A well-rounded set of packages for interacting with disks, partitions and
+  ;; file systems, included with the Guix installation image.
+  (list parted gptfdisk ddrescue
+        ;; We used to provide fdisk from GNU fdisk, but as of version 2.0.0a
+        ;; it pulls Guile 1.8, which takes unreasonable space; furthermore
+        ;; util-linux's fdisk is already available, in %base-packages-linux.
+        cryptsetup mdadm
+        dosfstools
+        btrfs-progs
+        f2fs-tools
+        jfsutils
+        xfsprogs))
+
 (define installation-os
   ;; The operating system used on installation images for USB sticks etc.
   (operating-system
@@ -530,7 +547,7 @@ (define installation-os
                       font-dejavu font-gnu-unifont
                       grub          ; mostly so xrefs to its manual work
                       nss-certs)    ; To access HTTPS, use git, etc.
-                %base-packages-disk-utilities
+                %installer-disk-utilities
                 %base-packages))))
 
 (define* (os-with-u-boot os board #:key (bootloader-target "/dev/mmcblk0")

base-commit: bf46192d4c7c4cd8d71edb8ace2cdf86322aafe7
-- 
2.38.1





^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [bug#59661] [PATCH v2 2/3] install: Add missing e2fsprogs utility.
  2022-12-04  4:43 ` [bug#59661] [PATCH v2 1/3] system: Rename and move %base-packages-disk-utilities Maxim Cournoyer
@ 2022-12-04  4:43   ` Maxim Cournoyer
  2022-12-04 16:34     ` Ludovic Courtès
  2022-12-04  4:43   ` [bug#59661] [PATCH v2 3/3] system: " Maxim Cournoyer
  2022-12-04 16:20   ` [bug#59661] [PATCH v2 1/3] system: Rename and move %base-packages-disk-utilities Ludovic Courtès
  2 siblings, 1 reply; 17+ messages in thread
From: Maxim Cournoyer @ 2022-12-04  4:43 UTC (permalink / raw)
  To: 59661; +Cc: ludo, Maxim Cournoyer

* gnu/system/install.scm (%installer-disk-utilities): Add e2fsprogs.
---
 gnu/system/install.scm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gnu/system/install.scm b/gnu/system/install.scm
index d34d974338..f6f1923121 100644
--- a/gnu/system/install.scm
+++ b/gnu/system/install.scm
@@ -471,6 +471,7 @@ (define %installer-disk-utilities
         cryptsetup mdadm
         dosfstools
         btrfs-progs
+        e2fsprogs
         f2fs-tools
         jfsutils
         xfsprogs))
-- 
2.38.1





^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [bug#59661] [PATCH v2 3/3] system: Add e2fsprogs to %base-packages-utils.
  2022-12-04  4:43 ` [bug#59661] [PATCH v2 1/3] system: Rename and move %base-packages-disk-utilities Maxim Cournoyer
  2022-12-04  4:43   ` [bug#59661] [PATCH v2 2/3] install: Add missing e2fsprogs utility Maxim Cournoyer
@ 2022-12-04  4:43   ` Maxim Cournoyer
  2022-12-04 16:36     ` Ludovic Courtès
  2022-12-04 16:20   ` [bug#59661] [PATCH v2 1/3] system: Rename and move %base-packages-disk-utilities Ludovic Courtès
  2 siblings, 1 reply; 17+ messages in thread
From: Maxim Cournoyer @ 2022-12-04  4:43 UTC (permalink / raw)
  To: 59661; +Cc: ludo, Maxim Cournoyer

Rationale: Even when not using an ext file system, the utilities provided by
e2fsprogs are useful, for example to set the copy-on-write attribute of a
Btrfs file system.

* gnu/system.scm (%base-packages-utils): Add e2fsprogs.
---
 gnu/system.scm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gnu/system.scm b/gnu/system.scm
index 1c119c31b6..62c8e0c2b6 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -856,6 +856,8 @@ (define %base-packages-utils
  (cons* procps psmisc which
         (@ (gnu packages admin) shadow-with-man-pages) ;for 'passwd'
 
+        e2fsprogs                 ;for lsattr, chattr, etc.
+
         guile-3.0-latest
 
         ;; The packages below are also in %FINAL-INPUTS, so take them from
-- 
2.38.1





^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [bug#59661] [PATCH v2 1/3] system: Rename and move %base-packages-disk-utilities.
  2022-12-04  4:43 ` [bug#59661] [PATCH v2 1/3] system: Rename and move %base-packages-disk-utilities Maxim Cournoyer
  2022-12-04  4:43   ` [bug#59661] [PATCH v2 2/3] install: Add missing e2fsprogs utility Maxim Cournoyer
  2022-12-04  4:43   ` [bug#59661] [PATCH v2 3/3] system: " Maxim Cournoyer
@ 2022-12-04 16:20   ` Ludovic Courtès
  2022-12-04 21:38     ` Maxim Cournoyer
  2022-12-05 10:36     ` [bug#59661] [PATCH v2 1/3] system: Rename and move %base-packages-disk-utilities Efraim Flashner
  2 siblings, 2 replies; 17+ messages in thread
From: Ludovic Courtès @ 2022-12-04 16:20 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: 59661, Efraim Flashner

Hi,

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

> Rationale: It is only used in INSTALLATION-OS and doesn't make sense to be
> used in another context, given that file systems now automatically pull their
> dependencies since commit 45eac6cdf5c8d9d7b0c564b105c790d2d2007799 (services:
> Add file system utilities to profile).
>
> * gnu/system.scm (%base-packages-disk-utilities): Deprecate and rename to...
> * gnu/system/install.scm (%installer-disk-utilities): ... this.
> (installation-os) [packages]: Adjust accordingly.

Efraim, this variable was added in
e6e076281e62518056987e9ddd3d96fccab20475 and used in
4170af491c8bc3b0a5308116a26e758d8ff245c5; do you think it’s okay to
remove now?  (It LGTM, but I’d like to make sure we don’t create churn.)

> +(define-deprecated %base-packages-disk-utilities #f '())

‘#f’ here would lead to weird deprecation messages.  I’d make it:

  (define-deprecated %base-packages-disk-utilities %base-packages '())

This is not quite accurate but it should convey the idea.

Thanks,
Ludo’.




^ permalink raw reply	[flat|nested] 17+ messages in thread

* [bug#59661] [PATCH v2 2/3] install: Add missing e2fsprogs utility.
  2022-12-04  4:43   ` [bug#59661] [PATCH v2 2/3] install: Add missing e2fsprogs utility Maxim Cournoyer
@ 2022-12-04 16:34     ` Ludovic Courtès
  2022-12-04 17:28       ` [bug#59661] [PATCH 0/3] Add e2fsprogs to %base-packages-utils Ludovic Courtès
  0 siblings, 1 reply; 17+ messages in thread
From: Ludovic Courtès @ 2022-12-04 16:34 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: 59661

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

> * gnu/system/install.scm (%installer-disk-utilities): Add e2fsprogs.

LGTM!

e2fsprogs binaries are indeed missing from $PATH in
guix-system-install-1.4.0rc1.*.iso; I’ll cherry-pick it on
‘version-1.4.0’.

What I don’t get is that our manual installation tests should have
caught this issue because they use ‘mkfs.ext4’ and expect to have it in
$PATH.  I’ll take a look…

Ludo’.




^ permalink raw reply	[flat|nested] 17+ messages in thread

* [bug#59661] [PATCH v2 3/3] system: Add e2fsprogs to %base-packages-utils.
  2022-12-04  4:43   ` [bug#59661] [PATCH v2 3/3] system: " Maxim Cournoyer
@ 2022-12-04 16:36     ` Ludovic Courtès
  0 siblings, 0 replies; 17+ messages in thread
From: Ludovic Courtès @ 2022-12-04 16:36 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: 59661

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

> Rationale: Even when not using an ext file system, the utilities provided by
> e2fsprogs are useful, for example to set the copy-on-write attribute of a
> Btrfs file system.
>
> * gnu/system.scm (%base-packages-utils): Add e2fsprogs.

LGTM!




^ permalink raw reply	[flat|nested] 17+ messages in thread

* [bug#59661] [PATCH 0/3] Add e2fsprogs to %base-packages-utils.
  2022-12-04 16:34     ` Ludovic Courtès
@ 2022-12-04 17:28       ` Ludovic Courtès
  2022-12-05 16:44         ` bug#59661: " Maxim Cournoyer
  0 siblings, 1 reply; 17+ messages in thread
From: Ludovic Courtès @ 2022-12-04 17:28 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: 59661

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

> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> * gnu/system/install.scm (%installer-disk-utilities): Add e2fsprogs.
>
> LGTM!
>
> e2fsprogs binaries are indeed missing from $PATH in
> guix-system-install-1.4.0rc1.*.iso; I’ll cherry-pick it on
> ‘version-1.4.0’.
>
> What I don’t get is that our manual installation tests should have
> caught this issue because they use ‘mkfs.ext4’ and expect to have it in
> $PATH.  I’ll take a look…

Got it: Mathieu worked around it in
0f66ef9aa99d2043abccbc80d858bdeca57534ac by explicitly adding e2fsprogs
and the installation system used by the tests:

  commit 0f66ef9aa99d2043abccbc80d858bdeca57534ac
  Author: Mathieu Othacehe <othacehe@gnu.org>
  Date:   Fri Sep 30 15:19:36 2022 +0200

      tests: install: Fix iso-image-installer test.

      This is a follow-up of: 45eac6cdf5c8d9d7b0c564b105c790d2d2007799.
      It fixes the following error:

      + mkfs.ext4 -L my-root /dev/vda2
      sh: line 12: mkfs.ext4: command not found

      * gnu/tests/install.scm (%test-iso-image-installer): Add e2fsprogs to the
      appended packages.

We should be able to revert this commit once the installer provides
e2fsprogs by default.

Thanks,
Ludo’.




^ permalink raw reply	[flat|nested] 17+ messages in thread

* [bug#59661] [PATCH v2 1/3] system: Rename and move %base-packages-disk-utilities.
  2022-12-04 16:20   ` [bug#59661] [PATCH v2 1/3] system: Rename and move %base-packages-disk-utilities Ludovic Courtès
@ 2022-12-04 21:38     ` Maxim Cournoyer
  2022-12-05 15:32       ` [bug#59661] [PATCH 0/3] Add e2fsprogs to %base-packages-utils Ludovic Courtès
  2022-12-05 10:36     ` [bug#59661] [PATCH v2 1/3] system: Rename and move %base-packages-disk-utilities Efraim Flashner
  1 sibling, 1 reply; 17+ messages in thread
From: Maxim Cournoyer @ 2022-12-04 21:38 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 59661, Efraim Flashner

Hi Ludovic,

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

> Hi,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> Rationale: It is only used in INSTALLATION-OS and doesn't make sense to be
>> used in another context, given that file systems now automatically pull their
>> dependencies since commit 45eac6cdf5c8d9d7b0c564b105c790d2d2007799 (services:
>> Add file system utilities to profile).
>>
>> * gnu/system.scm (%base-packages-disk-utilities): Deprecate and rename to...
>> * gnu/system/install.scm (%installer-disk-utilities): ... this.
>> (installation-os) [packages]: Adjust accordingly.
>
> Efraim, this variable was added in
> e6e076281e62518056987e9ddd3d96fccab20475 and used in
> 4170af491c8bc3b0a5308116a26e758d8ff245c5; do you think it’s okay to
> remove now?  (It LGTM, but I’d like to make sure we don’t create churn.)
>
>> +(define-deprecated %base-packages-disk-utilities #f '())
>
> ‘#f’ here would lead to weird deprecation messages.  I’d make it:
>
>   (define-deprecated %base-packages-disk-utilities %base-packages '())
>
> This is not quite accurate but it should convey the idea.

I had shown an actual message example produced when using #f.  It is
what I want (it just mentions the variable is deprecated, and doesn't
mention a replacement -- there are none in this case).  For a quick
reference, this is how 'warn-about-deprecation' is defined:

--8<---------------cut here---------------start------------->8---
(define* (warn-about-deprecation variable properties
                                 #:key replacement)
  (let ((location (and properties (source-properties->location properties))))
    (if replacement
        (warning location (G_ "'~a' is deprecated, use '~a' instead~%")
                 variable replacement)
        (warning location (G_ "'~a' is deprecated~%")
                 variable))))
--8<---------------cut here---------------end--------------->8---

So I prefer my version.  If you still think the produced message is
weird, I'll need a bit more explanation to understand why you think so
:-).

-- 
Thanks,
Maxim




^ permalink raw reply	[flat|nested] 17+ messages in thread

* [bug#59661] [PATCH v2 1/3] system: Rename and move %base-packages-disk-utilities.
  2022-12-04 16:20   ` [bug#59661] [PATCH v2 1/3] system: Rename and move %base-packages-disk-utilities Ludovic Courtès
  2022-12-04 21:38     ` Maxim Cournoyer
@ 2022-12-05 10:36     ` Efraim Flashner
  2022-12-05 15:30       ` Maxim Cournoyer
  2022-12-05 15:34       ` [bug#59661] [PATCH 0/3] Add e2fsprogs to %base-packages-utils Ludovic Courtès
  1 sibling, 2 replies; 17+ messages in thread
From: Efraim Flashner @ 2022-12-05 10:36 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 59661, Maxim Cournoyer

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

On Sun, Dec 04, 2022 at 05:20:42PM +0100, Ludovic Courtès wrote:
> Hi,
> 
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
> 
> > Rationale: It is only used in INSTALLATION-OS and doesn't make sense to be
> > used in another context, given that file systems now automatically pull their
> > dependencies since commit 45eac6cdf5c8d9d7b0c564b105c790d2d2007799 (services:
> > Add file system utilities to profile).
> >
> > * gnu/system.scm (%base-packages-disk-utilities): Deprecate and rename to...
> > * gnu/system/install.scm (%installer-disk-utilities): ... this.
> > (installation-os) [packages]: Adjust accordingly.
> 
> Efraim, this variable was added in
> e6e076281e62518056987e9ddd3d96fccab20475 and used in
> 4170af491c8bc3b0a5308116a26e758d8ff245c5; do you think it’s okay to
> remove now?  (It LGTM, but I’d like to make sure we don’t create churn.)

I looked back through the commits around there. The whole point was the
following commit, to divide the longish haphazard list of packages into
sets of a sort. If it's only used in (gnu system install) then I don't
see a problem with moving it there. The closest I know of to another
user of %base-packages-disk-utilities is my gparted guix image¹.

> > +(define-deprecated %base-packages-disk-utilities #f '())
> 
> ‘#f’ here would lead to weird deprecation messages.  I’d make it:
> 
>   (define-deprecated %base-packages-disk-utilities %base-packages '())
> 
> This is not quite accurate but it should convey the idea.

Would it be better to do

(define-deprecated/alias-public %base-packages-disk-utilities
  %installer-disk-utilities)

And remove the export from the list at the top of (gnu system)?

¹ https://git.sr.ht/~efraim/guix-config/tree/master/item/gparted.scm

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [bug#59661] [PATCH v2 1/3] system: Rename and move %base-packages-disk-utilities.
  2022-12-05 10:36     ` [bug#59661] [PATCH v2 1/3] system: Rename and move %base-packages-disk-utilities Efraim Flashner
@ 2022-12-05 15:30       ` Maxim Cournoyer
  2022-12-05 15:34       ` [bug#59661] [PATCH 0/3] Add e2fsprogs to %base-packages-utils Ludovic Courtès
  1 sibling, 0 replies; 17+ messages in thread
From: Maxim Cournoyer @ 2022-12-05 15:30 UTC (permalink / raw)
  To: Efraim Flashner; +Cc: 59661, Ludovic Courtès

Hi Efraim,

Efraim Flashner <efraim@flashner.co.il> writes:

[...]

> Would it be better to do
>
> (define-deprecated/alias-public %base-packages-disk-utilities
>   %installer-disk-utilities)
>
> And remove the export from the list at the top of (gnu system)?

Currently %installer-disk-utilities is not exported from (gnu system
install).  And even if it was, we'd still have to import it in (gnu
system), which would perhaps introduce a module cycle.

So I think the deprecation warning saying it's gone (i.e., using #f as
replacement) is the most accurate and least problematic option.

-- 
Thanks,
Maxim




^ permalink raw reply	[flat|nested] 17+ messages in thread

* [bug#59661] [PATCH 0/3] Add e2fsprogs to %base-packages-utils.
  2022-12-04 21:38     ` Maxim Cournoyer
@ 2022-12-05 15:32       ` Ludovic Courtès
  0 siblings, 0 replies; 17+ messages in thread
From: Ludovic Courtès @ 2022-12-05 15:32 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: 59661, Efraim Flashner

Hi,

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

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

[...]

>>> +(define-deprecated %base-packages-disk-utilities #f '())
>>
>> ‘#f’ here would lead to weird deprecation messages.  I’d make it:
>>
>>   (define-deprecated %base-packages-disk-utilities %base-packages '())
>>
>> This is not quite accurate but it should convey the idea.
>
> I had shown an actual message example produced when using #f.  It is
> what I want (it just mentions the variable is deprecated, and doesn't
> mention a replacement -- there are none in this case).  For a quick
> reference, this is how 'warn-about-deprecation' is defined:
>
> (define* (warn-about-deprecation variable properties
>                                  #:key replacement)
>   (let ((location (and properties (source-properties->location properties))))
>     (if replacement
>         (warning location (G_ "'~a' is deprecated, use '~a' instead~%")
>                  variable replacement)
>         (warning location (G_ "'~a' is deprecated~%")
>                  variable))))
>
> So I prefer my version.

Oh I had overlooked it, sorry for the confusion.  I prefer your version
as well; LGTM!

Ludo’.




^ permalink raw reply	[flat|nested] 17+ messages in thread

* [bug#59661] [PATCH 0/3] Add e2fsprogs to %base-packages-utils.
  2022-12-05 10:36     ` [bug#59661] [PATCH v2 1/3] system: Rename and move %base-packages-disk-utilities Efraim Flashner
  2022-12-05 15:30       ` Maxim Cournoyer
@ 2022-12-05 15:34       ` Ludovic Courtès
  1 sibling, 0 replies; 17+ messages in thread
From: Ludovic Courtès @ 2022-12-05 15:34 UTC (permalink / raw)
  To: Efraim Flashner; +Cc: 59661, Maxim Cournoyer

Hi,

Efraim Flashner <efraim@flashner.co.il> skribis:

> On Sun, Dec 04, 2022 at 05:20:42PM +0100, Ludovic Courtès wrote:
>> Hi,
>> 
>> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>> 
>> > Rationale: It is only used in INSTALLATION-OS and doesn't make sense to be
>> > used in another context, given that file systems now automatically pull their
>> > dependencies since commit 45eac6cdf5c8d9d7b0c564b105c790d2d2007799 (services:
>> > Add file system utilities to profile).
>> >
>> > * gnu/system.scm (%base-packages-disk-utilities): Deprecate and rename to...
>> > * gnu/system/install.scm (%installer-disk-utilities): ... this.
>> > (installation-os) [packages]: Adjust accordingly.
>> 
>> Efraim, this variable was added in
>> e6e076281e62518056987e9ddd3d96fccab20475 and used in
>> 4170af491c8bc3b0a5308116a26e758d8ff245c5; do you think it’s okay to
>> remove now?  (It LGTM, but I’d like to make sure we don’t create churn.)
>
> I looked back through the commits around there. The whole point was the
> following commit, to divide the longish haphazard list of packages into
> sets of a sort. If it's only used in (gnu system install) then I don't
> see a problem with moving it there.

OK.

>>   (define-deprecated %base-packages-disk-utilities %base-packages '())
>> 
>> This is not quite accurate but it should convey the idea.
>
> Would it be better to do
>
> (define-deprecated/alias-public %base-packages-disk-utilities
>   %installer-disk-utilities)
>
> And remove the export from the list at the top of (gnu system)?

I don’t think we want users to rely on ‘%installer-disk-utilities’ in
their OS config, so I’d go for Maxim’s version.

Thanks,
Ludo’.




^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#59661: [PATCH 0/3] Add e2fsprogs to %base-packages-utils.
  2022-12-04 17:28       ` [bug#59661] [PATCH 0/3] Add e2fsprogs to %base-packages-utils Ludovic Courtès
@ 2022-12-05 16:44         ` Maxim Cournoyer
  0 siblings, 0 replies; 17+ messages in thread
From: Maxim Cournoyer @ 2022-12-05 16:44 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 59661-done

Hi,

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

> Ludovic Courtès <ludo@gnu.org> skribis:
>
>> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>>
>>> * gnu/system/install.scm (%installer-disk-utilities): Add e2fsprogs.
>>
>> LGTM!
>>
>> e2fsprogs binaries are indeed missing from $PATH in
>> guix-system-install-1.4.0rc1.*.iso; I’ll cherry-pick it on
>> ‘version-1.4.0’.
>>
>> What I don’t get is that our manual installation tests should have
>> caught this issue because they use ‘mkfs.ext4’ and expect to have it in
>> $PATH.  I’ll take a look…
>
> Got it: Mathieu worked around it in
> 0f66ef9aa99d2043abccbc80d858bdeca57534ac by explicitly adding e2fsprogs
> and the installation system used by the tests:
>
>   commit 0f66ef9aa99d2043abccbc80d858bdeca57534ac
>   Author: Mathieu Othacehe <othacehe@gnu.org>
>   Date:   Fri Sep 30 15:19:36 2022 +0200
>
>       tests: install: Fix iso-image-installer test.
>
>       This is a follow-up of: 45eac6cdf5c8d9d7b0c564b105c790d2d2007799.
>       It fixes the following error:
>
>       + mkfs.ext4 -L my-root /dev/vda2
>       sh: line 12: mkfs.ext4: command not found
>
>       * gnu/tests/install.scm (%test-iso-image-installer): Add e2fsprogs to the
>       appended packages.
>
> We should be able to revert this commit once the installer provides
> e2fsprogs by default.

I applied this series, reverted 0f66ef9aa99d2043abccbc80d858bdeca57534ac
as suggested above, ran:

--8<---------------cut here---------------start------------->8---
make check-system TESTS=iso-image-installer
--8<---------------cut here---------------end--------------->8---

And pushed!

Closing.

-- 
Thanks,
Maxim




^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2022-12-05 16:45 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-28 19:18 [bug#59661] [PATCH 0/3] Add e2fsprogs to %base-packages-utils Maxim Cournoyer
2022-11-28 20:01 ` [bug#59661] [PATCH 1/3] system: Rename and move %base-packages-disk-utilities Maxim Cournoyer
2022-11-28 20:01   ` [bug#59661] [PATCH 2/3] install: Add missing e2fsprogs utility Maxim Cournoyer
2022-11-28 20:01   ` [bug#59661] [PATCH 3/3] system: Add e2fsprogs to %base-packages-utils Maxim Cournoyer
2022-12-04  4:43 ` [bug#59661] [PATCH v2 1/3] system: Rename and move %base-packages-disk-utilities Maxim Cournoyer
2022-12-04  4:43   ` [bug#59661] [PATCH v2 2/3] install: Add missing e2fsprogs utility Maxim Cournoyer
2022-12-04 16:34     ` Ludovic Courtès
2022-12-04 17:28       ` [bug#59661] [PATCH 0/3] Add e2fsprogs to %base-packages-utils Ludovic Courtès
2022-12-05 16:44         ` bug#59661: " Maxim Cournoyer
2022-12-04  4:43   ` [bug#59661] [PATCH v2 3/3] system: " Maxim Cournoyer
2022-12-04 16:36     ` Ludovic Courtès
2022-12-04 16:20   ` [bug#59661] [PATCH v2 1/3] system: Rename and move %base-packages-disk-utilities Ludovic Courtès
2022-12-04 21:38     ` Maxim Cournoyer
2022-12-05 15:32       ` [bug#59661] [PATCH 0/3] Add e2fsprogs to %base-packages-utils Ludovic Courtès
2022-12-05 10:36     ` [bug#59661] [PATCH v2 1/3] system: Rename and move %base-packages-disk-utilities Efraim Flashner
2022-12-05 15:30       ` Maxim Cournoyer
2022-12-05 15:34       ` [bug#59661] [PATCH 0/3] Add e2fsprogs to %base-packages-utils Ludovic Courtès

Code repositories for project(s) associated with this public inbox

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).