GNU bug report logs - #59661
[PATCH 0/3] Add e2fsprogs to %base-packages-utils.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 59661 in the body.
You can then email your comments to 59661 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: guix-patches <at> gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH 0/3] Add e2fsprogs to %base-packages-utils.
Date: Mon, 28 Nov 2022 14:18:31 -0500

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

Message #8 received at 59661 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 59661 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH 1/3] system: Rename and move %base-packages-disk-utilities.
Date: Mon, 28 Nov 2022 15:01:43 -0500

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

Message #11 received at 59661 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 59661 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH 3/3] system: Add e2fsprogs to %base-packages-utils.
Date: Mon, 28 Nov 2022 15:01:45 -0500

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

Message #14 received at 59661 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 59661 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH 2/3] install: Add missing e2fsprogs utility.
Date: Mon, 28 Nov 2022 15:01:44 -0500

* 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

Message #17 received at 59661 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 59661 <at> debbugs.gnu.org
Cc: ludo <at> gnu.org, Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH v2 1/3] system: Rename and move %base-packages-disk-utilities.
Date: Sat,  3 Dec 2022 23:43:45 -0500

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

Message #20 received at 59661 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 59661 <at> debbugs.gnu.org
Cc: ludo <at> gnu.org, Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH v2 2/3] install: Add missing e2fsprogs utility.
Date: Sat,  3 Dec 2022 23:43:46 -0500

* 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

Message #23 received at 59661 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 59661 <at> debbugs.gnu.org
Cc: ludo <at> gnu.org, Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH v2 3/3] system: Add e2fsprogs to %base-packages-utils.
Date: Sat,  3 Dec 2022 23:43:47 -0500

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

Message #26 received at 59661 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 59661 <at> debbugs.gnu.org, Efraim Flashner <efraim <at> flashner.co.il>
Subject: Re: [PATCH v2 1/3] system: Rename and move
 %base-packages-disk-utilities.
Date: Sun, 04 Dec 2022 17:20:42 +0100

Hi,

Maxim Cournoyer <maxim.cournoyer <at> 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’.

Message #29 received at 59661 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 59661 <at> debbugs.gnu.org
Subject: Re: [PATCH v2 2/3] install: Add missing e2fsprogs utility.
Date: Sun, 04 Dec 2022 17:34:23 +0100

Maxim Cournoyer <maxim.cournoyer <at> 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’.

Message #32 received at 59661 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 59661 <at> debbugs.gnu.org
Subject: Re: [PATCH v2 3/3] system: Add e2fsprogs to %base-packages-utils.
Date: Sun, 04 Dec 2022 17:36:54 +0100

Maxim Cournoyer <maxim.cournoyer <at> 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!

Message #35 received at 59661 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 59661 <at> debbugs.gnu.org
Subject: Re: bug#59661: [PATCH 0/3] Add e2fsprogs to %base-packages-utils.
Date: Sun, 04 Dec 2022 18:28:44 +0100

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

> Maxim Cournoyer <maxim.cournoyer <at> 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 <at> 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’.

Message #38 received at 59661 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 59661 <at> debbugs.gnu.org, Efraim Flashner <efraim <at> flashner.co.il>
Subject: Re: [PATCH v2 1/3] system: Rename and move
 %base-packages-disk-utilities.
Date: Sun, 04 Dec 2022 16:38:13 -0500

Hi Ludovic,

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

> Hi,
>
> Maxim Cournoyer <maxim.cournoyer <at> 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

Message #41 received at 59661 <at> debbugs.gnu.org (full text, mbox):

From: Efraim Flashner <efraim <at> flashner.co.il>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 59661 <at> debbugs.gnu.org, Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: Re: [PATCH v2 1/3] system: Rename and move
 %base-packages-disk-utilities.
Date: Mon, 5 Dec 2022 12:36:21 +0200

[Message part 1 (text/plain, inline)]

On Sun, Dec 04, 2022 at 05:20:42PM +0100, Ludovic Courtès wrote:
> Hi,
> 
> Maxim Cournoyer <maxim.cournoyer <at> 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 <at> 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

[signature.asc (application/pgp-signature, inline)]

Message #44 received at 59661 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Efraim Flashner <efraim <at> flashner.co.il>
Cc: 59661 <at> debbugs.gnu.org, Ludovic Courtès <ludo <at> gnu.org>
Subject: Re: [PATCH v2 1/3] system: Rename and move
 %base-packages-disk-utilities.
Date: Mon, 05 Dec 2022 10:30:36 -0500

Hi Efraim,

Efraim Flashner <efraim <at> 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

Message #47 received at 59661 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 59661 <at> debbugs.gnu.org, Efraim Flashner <efraim <at> flashner.co.il>
Subject: Re: bug#59661: [PATCH 0/3] Add e2fsprogs to %base-packages-utils.
Date: Mon, 05 Dec 2022 16:32:35 +0100

Hi,

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

> Ludovic Courtès <ludo <at> 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’.

Message #50 received at 59661 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Efraim Flashner <efraim <at> flashner.co.il>
Cc: 59661 <at> debbugs.gnu.org, Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: Re: bug#59661: [PATCH 0/3] Add e2fsprogs to %base-packages-utils.
Date: Mon, 05 Dec 2022 16:34:50 +0100

Hi,

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

> On Sun, Dec 04, 2022 at 05:20:42PM +0100, Ludovic Courtès wrote:
>> Hi,
>> 
>> Maxim Cournoyer <maxim.cournoyer <at> 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’.

Message #57 received at 59661-done <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 59661-done <at> debbugs.gnu.org
Subject: Re: bug#59661: [PATCH 0/3] Add e2fsprogs to %base-packages-utils.
Date: Mon, 05 Dec 2022 11:44:50 -0500

Hi,

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

> Ludovic Courtès <ludo <at> gnu.org> skribis:
>
>> Maxim Cournoyer <maxim.cournoyer <at> 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 <at> 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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.