[PATCH 0/3] Add e2fsprogs to %base-packages-utils.

  • Done
  • quality assurance status badge
Details
3 participants
  • Efraim Flashner
  • Ludovic Courtès
  • Maxim Cournoyer
Owner
unassigned
Submitted by
Maxim Cournoyer
Severity
normal
Merged with
M
M
Maxim Cournoyer wrote on 28 Nov 2022 20:18
(address . guix-patches@gnu.org)(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
20221128191831.1325-1-maxim.cournoyer@gmail.com
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
M
M
Maxim Cournoyer wrote on 28 Nov 2022 21:01
[PATCH 1/3] system: Rename and move %base-packages-disk-utilities.
(address . 59661@debbugs.gnu.org)(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
20221128200145.9078-1-maxim.cournoyer@gmail.com
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(-)

Toggle diff (92 lines)
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
M
M
Maxim Cournoyer wrote on 28 Nov 2022 21:01
[PATCH 3/3] system: Add e2fsprogs to %base-packages-utils.
(address . 59661@debbugs.gnu.org)(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
20221128200145.9078-3-maxim.cournoyer@gmail.com
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(+)

Toggle diff (15 lines)
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
M
M
Maxim Cournoyer wrote on 28 Nov 2022 21:01
[PATCH 2/3] install: Add missing e2fsprogs utility.
(address . 59661@debbugs.gnu.org)(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
20221128200145.9078-2-maxim.cournoyer@gmail.com
* gnu/system/install.scm (%installer-disk-utilities): Add e2fsprogs.
---
gnu/system/install.scm | 1 +
1 file changed, 1 insertion(+)

Toggle diff (14 lines)
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
M
M
Maxim Cournoyer wrote on 4 Dec 2022 05:43
[PATCH v2 1/3] system: Rename and move %base-packages-disk-utilities.
(address . 59661@debbugs.gnu.org)
20221204044347.23147-1-maxim.cournoyer@gmail.com
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(-)

Toggle diff (92 lines)
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
M
M
Maxim Cournoyer wrote on 4 Dec 2022 05:43
[PATCH v2 2/3] install: Add missing e2fsprogs utility.
(address . 59661@debbugs.gnu.org)
20221204044347.23147-2-maxim.cournoyer@gmail.com
* gnu/system/install.scm (%installer-disk-utilities): Add e2fsprogs.
---
gnu/system/install.scm | 1 +
1 file changed, 1 insertion(+)

Toggle diff (14 lines)
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
M
M
Maxim Cournoyer wrote on 4 Dec 2022 05:43
[PATCH v2 3/3] system: Add e2fsprogs to %base-packages-utils.
(address . 59661@debbugs.gnu.org)
20221204044347.23147-3-maxim.cournoyer@gmail.com
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(+)

Toggle diff (15 lines)
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
L
L
Ludovic Courtès wrote on 4 Dec 2022 17:20
Re: [PATCH v2 1/3] system: Rename and move %base-packages-disk-utilities.
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
875yernns5.fsf@gnu.org
Hi,

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

Toggle quote (9 lines)
> 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.)

Toggle quote (2 lines)
> +(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’.
L
L
Ludovic Courtès wrote on 4 Dec 2022 17:34
Re: [PATCH v2 2/3] install: Add missing e2fsprogs utility.
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)(address . 59661@debbugs.gnu.org)
87wn77m8kw.fsf@gnu.org
Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

Toggle quote (2 lines)
> * 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’.
L
L
Ludovic Courtès wrote on 4 Dec 2022 17:36
Re: [PATCH v2 3/3] system: Add e2fsprogs to %base-packages-utils.
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)(address . 59661@debbugs.gnu.org)
87sfhvm8gp.fsf@gnu.org
Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

Toggle quote (6 lines)
> 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!
L
L
Ludovic Courtès wrote on 4 Dec 2022 18:28
Re: bug#59661: [PATCH 0/3] Add e2fsprogs to %base-packages-utils.
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)(address . 59661@debbugs.gnu.org)
877cz7m62b.fsf_-_@gnu.org
Ludovic Courtès <ludo@gnu.org> skribis:

Toggle quote (14 lines)
> 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’.
M
M
Maxim Cournoyer wrote on 4 Dec 2022 22:38
Re: [PATCH v2 1/3] system: Rename and move %base-packages-disk-utilities.
(name . Ludovic Courtès)(address . ludo@gnu.org)
87y1rm6e9m.fsf@gmail.com
Hi Ludovic,

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

Toggle quote (26 lines)
> 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:

Toggle snippet (10 lines)
(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. If you still think the produced message is
weird, I'll need a bit more explanation to understand why you think so
:-).

--
Thanks,
Maxim
E
E
Efraim Flashner wrote on 5 Dec 2022 11:36
(name . Ludovic Courtès)(address . ludo@gnu.org)
Y43JpRDniUxrMF0r@3900XT
On Sun, Dec 04, 2022 at 05:20:42PM +0100, Ludovic Courtès wrote:
Toggle quote (18 lines)
> 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¹.

Toggle quote (8 lines)
> > +(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)?


--
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
-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEoov0DD5VE3JmLRT3Qarn3Mo9g1EFAmONyaEACgkQQarn3Mo9
g1GjVBAAn3pWezvi0wZEUD6KO0W2mBD4y/bXLeSulmtjYn0iUumXrpjqhKB6k2Vq
pMO1tgQlkVyVKbHJyOpeAKB7VNB6B9+g5ufLDjc4DjLFy8caP6m2ljSlHclqDH2C
q3Q5U4gO6mRrSMqjUS/mxYO+u6z/mGNZaxQ0Kl0YwhbKUoiHj8vzVGcQ/zDGpKRq
ElFztmRujy/unb2VKFTayIAZaAVahidaE8FCmmDJ5hZlrrhJ7i14J6kT3CVqlKiU
wNc7H6Pq8uuQzzCrgFGIQ9DQKiUoAvkykw6wTIhTzhOfB9VAhmmDgTYLgrVAYTJi
aes1v+EzSvx8w/oe2VRPVywNRTsZp97OEru+2Nb1ukTIXtG5aBoFPd/ZZttLQN8A
OFPFGSJKqtRUN543ro96dgf0n+y8/5vhz4TeOAcHw8qwov7A62dZ2uzI2LAsIBVQ
9eInOHTzBzE6dgZyoN4Qa2718/Tibl++lc8vFRdyMAKMrUfawZ2t0S9HI4ZAtQqt
WPciC8UACKpUPfOkhk7Ez0wrgp3xzltAbaFxLNAZg3xrqSHtlpwZhmnpsSbYCvDl
r4Rfg/znyEaLFCZvdkTh5xb/QS60yc+3HpRItJ6gHrTqYHqAVqQfE7n0lW1xmgdE
lxO2L6r/o9fN5iOYRr9naKnPdjQGlGM+koPGkLm35FvF7jLuA4k=
=TKgl
-----END PGP SIGNATURE-----


M
M
Maxim Cournoyer wrote on 5 Dec 2022 16:30
(name . Efraim Flashner)(address . efraim@flashner.co.il)
87pmcx6f6r.fsf@gmail.com
Hi Efraim,

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

[...]

Toggle quote (7 lines)
> 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
L
L
Ludovic Courtès wrote on 5 Dec 2022 16:32
Re: bug#59661: [PATCH 0/3] Add e2fsprogs to %base-packages-utils.
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87tu29q31o.fsf_-_@gnu.org
Hi,

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

Toggle quote (2 lines)
> Ludovic Courtès <ludo@gnu.org> writes:

[...]

Toggle quote (24 lines)
>>> +(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’.
L
L
Ludovic Courtès wrote on 5 Dec 2022 16:34
(name . Efraim Flashner)(address . efraim@flashner.co.il)
87pmcxq2xx.fsf_-_@gnu.org
Hi,

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

Toggle quote (24 lines)
> 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.

Toggle quote (11 lines)
>> (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’.
M
M
Maxim Cournoyer wrote on 5 Dec 2022 17:32
control message for bug #58238
(address . control@debbugs.gnu.org)
87h6y96ccb.fsf@gmail.com
merge 58238 59661
quit
M
M
Maxim Cournoyer wrote on 5 Dec 2022 17:44
Re: bug#59661: [PATCH 0/3] Add e2fsprogs to %base-packages-utils.
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 59661-done@debbugs.gnu.org)
878rjl6br1.fsf_-_@gmail.com
Hi,

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

Toggle quote (38 lines)
> 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:

Toggle snippet (3 lines)
make check-system TESTS=iso-image-installer

And pushed!

Closing.

--
Thanks,
Maxim
Closed
?