[bug#59661,v2,1/3] system: Rename and move %base-packages-disk-utilities.
Commit Message
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(-)
base-commit: bf46192d4c7c4cd8d71edb8ace2cdf86322aafe7
Comments
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’.
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
:-).
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
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.
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’.
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’.
@@ -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
@@ -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")