[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

Debbugs page

M
M
Maxim Cournoyer wrote on 28 Nov 2022 11: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 12: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 12: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 12: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 3 Dec 2022 20: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 3 Dec 2022 20: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 3 Dec 2022 20: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 08: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 08: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 08: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 09: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 13: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 02: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 07: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 07: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 07: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 08: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 08: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
?
Your comment

This issue is archived.

To comment on this conversation send an email to 59661@patchwise.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 59661
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch