[bug#55653] guix: Add syntactic sugar for profile generation.

Message ID 10354f31e0be9bcb88b78da2fb8a2a3c3acbde10.camel@gmail.com
State New
Headers
Series [bug#55653] guix: Add syntactic sugar for profile generation. |

Commit Message

Liliana Marie Prikler May 26, 2022, 9:01 a.m. UTC
  * guix/profiles.scm (%profile, package-compatibility-helper): New variables.
(profile): Implement in terms of package-compatibility-helper.
---
Hi Guix,

this is a first step towards managing multiple profiles via Guix Home.
It makes it so that regular Guix profiles can more easily be specified, though
I'm not quite sure whether the mlet of packages->profile-entry should not also
be used here.

In any case, with this it should now be relatively easy for users to specify
profiles such as
  (profile (name "emacs") (packages emacs emacs-magit emacs-org ...))
  (profile (name "r") (packages r r-plyr emacs emacs-ess ...))
  (profile (name "python") (packages python python-beautifulsoup4 ...))
  ...
What's still missing is a way to link them up with /var/guix/profiles/per-user
and $HOME – for the latter, there would be a home-*-service-type.

WDYT?

 guix/profiles.scm  | 23 ++++++++++++++++++++++-
 tests/profiles.scm | 16 ++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletion(-)
  

Comments

Andrew Tropin May 27, 2022, 12:39 p.m. UTC | #1
On 2022-05-26 11:01, Liliana Marie Prikler wrote:

> * guix/profiles.scm (%profile, package-compatibility-helper): New variables.
> (profile): Implement in terms of package-compatibility-helper.
> ---
> Hi Guix,
>
> this is a first step towards managing multiple profiles via Guix Home.
> It makes it so that regular Guix profiles can more easily be specified, though
> I'm not quite sure whether the mlet of packages->profile-entry should not also
> be used here.

The sugar looks reasonable to me.

>
> In any case, with this it should now be relatively easy for users to specify
> profiles such as
>   (profile (name "emacs") (packages emacs emacs-magit emacs-org ...))
>   (profile (name "r") (packages r r-plyr emacs emacs-ess ...))
>   (profile (name "python") (packages python python-beautifulsoup4 ...))
>   ...
> What's still missing is a way to link them up with /var/guix/profiles/per-user

Don't think that they have to be linked to /var/guix/profiles/per-user,
as mentioned earlier profiles built for the home environment should be a
part of home environment, link to the profile will be inside
home-environment directory:
/var/guix/profiles/per-user/bob/guix-home/profiles/PROFILE_NAME

links in /var/guix/profiles/per-user needed for switching between
profile versions, it's not possible in case profile is a part of home
environment, otherwise home environment will stop being reproducible.

> and $HOME – for the latter, there would be a home-*-service-type.

Yep, can be done by home-files-* and symlink-manager I guess.

>
> WDYT?
>
>  guix/profiles.scm  | 23 ++++++++++++++++++++++-
>  tests/profiles.scm | 16 ++++++++++++++++
>  2 files changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/guix/profiles.scm b/guix/profiles.scm
> index bf50c00a1e..fbc343c456 100644
> --- a/guix/profiles.scm
> +++ b/guix/profiles.scm
> @@ -1974,7 +1974,7 @@ (define builder
>                                             (manifest-entries manifest))))))))
>  
>  ;; Declarative profile.
> -(define-record-type* <profile> profile make-profile
> +(define-record-type* <profile> %profile make-profile
>    profile?
>    (name               profile-name (default "profile")) ;string
>    (content            profile-content)                  ;<manifest>
> @@ -1987,6 +1987,27 @@ (define-record-type* <profile> profile make-profile
>    (relative-symlinks? profile-relative-symlinks?  ;Boolean
>                        (default #f)))
>  
> +(define-syntax package-compatibility-helper
> +  (syntax-rules (packages manifest)
> +    ((_ () (fields ...))
> +     (%profile fields ...))
> +    ((_ ((packages exp) rest ...) (others ...))
> +     (package-compatibility-helper
> +      (rest ...)
> +      (others ... (content (packages->manifest
> +                            (delete-duplicates exp eq?))))))
> +    ((_ ((manifest exp) rest ...) (others ...))
> +     (package-compatibility-helper
> +      (rest ...)
> +      (others ... (content exp))))
> +    ((_ (field rest ...) (others ...))
> +     (package-compatibility-helper (rest ...) (others ... field)))))
> +
> +(define-syntax-rule (profile fields ...)
> +  "Build a <profile> record, automatically converting 'packages' or 'manifest '
> +field specifications to 'content'."
> +  (package-compatibility-helper (fields ...) ()))
> +
>  (define-gexp-compiler (profile-compiler (profile <profile>) system target)
>    "Compile PROFILE to a derivation."
>    (match profile
> diff --git a/tests/profiles.scm b/tests/profiles.scm
> index d59d75985f..970a34b6cc 100644
> --- a/tests/profiles.scm
> +++ b/tests/profiles.scm
> @@ -272,6 +272,22 @@ (define transform1
>                                    (manifest-pattern (name name))))
>             '("gcc" "binutils" "glibc" "coreutils" "grep" "sed"))))
>  
> +(test-assert "profile syntax sugar"
> +  (let ((p1 (dummy-package "p1"))
> +        (p2 (dummy-package "p2")))
> +    (define (profile=? . profiles)
> +      (define (manifest=? . manifests)
> +        ;; Since we're using the same packages, we could also compare via eq?
> +        (apply list= manifest-entry=? (map manifest-entries manifests)))
> +      (apply manifest=? (map profile-content profiles)))
> +
> +    (profile=?
> +     (profile (content (manifest
> +                        (map package->manifest-entry (list p1 p2)))))
> +     (profile (content (packages->manifest (list p1 p2))))
> +     (profile (manifest (packages->manifest (list p1 p2))))
> +     (profile (packages (list p1 p2))))))
> +
>  (test-assertm "profile-derivation"
>    (mlet* %store-monad
>        ((entry ->   (package->manifest-entry %bootstrap-guile))
  
Ludovic Courtès May 31, 2022, 1:47 p.m. UTC | #2
Hello!

Liliana Marie Prikler <liliana.prikler@gmail.com> skribis:

> * guix/profiles.scm (%profile, package-compatibility-helper): New variables.
> (profile): Implement in terms of package-compatibility-helper.
> ---
> Hi Guix,
>
> this is a first step towards managing multiple profiles via Guix Home.
> It makes it so that regular Guix profiles can more easily be specified, though
> I'm not quite sure whether the mlet of packages->profile-entry should not also
> be used here.
>
> In any case, with this it should now be relatively easy for users to specify
> profiles such as
>   (profile (name "emacs") (packages emacs emacs-magit emacs-org ...))
>   (profile (name "r") (packages r r-plyr emacs emacs-ess ...))
>   (profile (name "python") (packages python python-beautifulsoup4 ...))
>   ...

So the goal is to make things slightly more concise than:

  (profile (content (packages->manifest (list …))))

right?

We don’t have syntactic sugar like this elsewhere, and I would prefer to
remain consistent here.  However, if that helps, we could have a
procedure, like:

  (define (packages->profile name packages)
    (profile (name name) …))

Thoughts?

Ludo’.
  
Liliana Marie Prikler May 31, 2022, 6:07 p.m. UTC | #3
Hi,

Am Dienstag, dem 31.05.2022 um 15:47 +0200 schrieb Ludovic Courtès:
> So the goal is to make things slightly more concise than:
> 
>   (profile (content (packages->manifest (list …))))
> 
> right?
More or less.  However, I also feel that "content" is somewhat
confusing if it were to be user-facing so I added a syntactic manifest
noop as well.

> We don’t have syntactic sugar like this elsewhere, and I would prefer
> to remain consistent here.  
We do have origin sha256, which sets both hash-algo and hash-content,
so it's not unprecedented in my opinion.

> However, if that helps, we could have a procedure, like:
> 
>   (define (packages->profile name packages)
>     (profile (name name) …))
> 
> Thoughts?
I do think syntactic constructors feel better here, because the end
goal would be embedding things in (thunked) configuration fields. 
Having a procedure might be acceptable, but feels more clunky in the
context of Guix.

Chees
  
Ludovic Courtès June 1, 2022, 7:43 p.m. UTC | #4
Hi,

Liliana Marie Prikler <liliana.prikler@gmail.com> skribis:

> Am Dienstag, dem 31.05.2022 um 15:47 +0200 schrieb Ludovic Courtès:
>> So the goal is to make things slightly more concise than:
>> 
>>   (profile (content (packages->manifest (list …))))
>> 
>> right?
> More or less.  However, I also feel that "content" is somewhat
> confusing if it were to be user-facing so I added a syntactic manifest
> noop as well.

OK.

>> We don’t have syntactic sugar like this elsewhere, and I would prefer
>> to remain consistent here.  
> We do have origin sha256, which sets both hash-algo and hash-content,
> so it's not unprecedented in my opinion.

‘sha256’ is a backward-compatibility hack for ‘content-hash’, so it’s a
bit different in my view.

To be clear, what I meant is that record construction always look like
this:

  (constructor
    (field1 value1)
    (field2 value2))

whereas here the proposal (IIUC) is:

  (constructor
    (field1 several values that get bundled together magically))

>> However, if that helps, we could have a procedure, like:
>> 
>>   (define (packages->profile name packages)
>>     (profile (name name) …))
>> 
>> Thoughts?
> I do think syntactic constructors feel better here, because the end
> goal would be embedding things in (thunked) configuration fields. 
> Having a procedure might be acceptable, but feels more clunky in the
> context of Guix.

To me, ‘packages->profile’ doesn’t look any more clunky than
‘packages->manifest’ or similar procedures.

Do you think a procedure like this would address the verbosity problem
that prompted you to propose this patch?

Thanks,
Ludo’.
  
Liliana Marie Prikler June 1, 2022, 8:15 p.m. UTC | #5
Hi,

Am Mittwoch, dem 01.06.2022 um 21:43 +0200 schrieb Ludovic Courtès:
> [...]
> 
> > 
> > > We don’t have syntactic sugar like this elsewhere, and I would
> > > prefer to remain consistent here.  
> > We do have origin sha256, which sets both hash-algo and hash-
> > content, so it's not unprecedented in my opinion.
> 
> ‘sha256’ is a backward-compatibility hack for ‘content-hash’, so it’s
> a bit different in my view.
> 
> To be clear, what I meant is that record construction always look
> like this:
> 
>   (constructor
>     (field1 value1)
>     (field2 value2))
> 
> whereas here the proposal (IIUC) is:
> 
>   (constructor
>     (field1 several values that get bundled together magically))
If it reads like that, then that's probably a mistake somewhere.  My
actual proposal to allow both of the following:

(package
  other-fields ...
  (manifest some-manifest))
(package 
  other-fields ...
  (packages (list bash coreutils emacs ...)))

> > > However, if that helps, we could have a procedure, like:
> > > 
> > >   (define (packages->profile name packages)
> > >     (profile (name name) …))
> > > 
> > > Thoughts?
> > I do think syntactic constructors feel better here, because the end
> > goal would be embedding things in (thunked) configuration fields. 
> > Having a procedure might be acceptable, but feels more clunky in
> > the context of Guix.
> 
> To me, ‘packages->profile’ doesn’t look any more clunky than
> ‘packages->manifest’ or similar procedures.
> 
> Do you think a procedure like this would address the verbosity
> problem that prompted you to propose this patch?
I don't think it does tbh.  We currently have two implementations of
packages->profile-entry, one for Guix System, one for Guix Home, which
at the time of writing are exactly the same.  My use case of naming
profiles would be served by such a procedure, but using a syntactic
constructor has other benefits in that all of the fields of the profile
become accessible.  That means that users could (once profile
management via Guix Home is implemented) for instance run less hooks or
additional hooks for certain profiles, allow collisions, use relative
symlinks, etc. for basically free, not to mention that changes which
break record ABI (such as added fields) get promoted directly through
syntax but not through a plain procedure.

Cheers
  
Maxime Devos June 1, 2022, 8:34 p.m. UTC | #6
Liliana Marie Prikler schreef op wo 01-06-2022 om 22:15 [+0200]:
> not to mention that changes which
> break record ABI (such as added fields) get promoted directly through
> syntax but not through a plain procedure.

Using a procedure instead of the record construction macro would make
the ABI stable (*).  ABI-wise, wouldn't a stable ABI (regular
procudure) be better than an unstable ABI?  There might be some
benefits for a macro concerning readability or such, but I don't see
any benefits ABI-wise.

(*) assuming no inlining.

Greetings,
Maxime.
  
Ludovic Courtès June 2, 2022, 1:32 p.m. UTC | #7
Hallo!

Liliana Marie Prikler <liliana.prikler@gmail.com> skribis:

> If it reads like that, then that's probably a mistake somewhere.  My
> actual proposal to allow both of the following:
>
> (package
>   other-fields ...
>   (manifest some-manifest))
> (package 
>   other-fields ...
>   (packages (list bash coreutils emacs ...)))

Oh OK, I got it wrong, sorry.

Still I’m not a fan of having syntax that looks like a field but is not
an actual field, if we can avoid it.  I prefer to expose the data
structure as it exists and, if needed, to build abstractions on top of
it.  (The ABI issue that Maxime mention is real but I don’t think it’s a
big problem in practice.)

>> > > However, if that helps, we could have a procedure, like:
>> > > 
>> > >   (define (packages->profile name packages)
>> > >     (profile (name name) …))
>> > > 
>> > > Thoughts?
>> > I do think syntactic constructors feel better here, because the end
>> > goal would be embedding things in (thunked) configuration fields. 
>> > Having a procedure might be acceptable, but feels more clunky in
>> > the context of Guix.
>> 
>> To me, ‘packages->profile’ doesn’t look any more clunky than
>> ‘packages->manifest’ or similar procedures.
>> 
>> Do you think a procedure like this would address the verbosity
>> problem that prompted you to propose this patch?
> I don't think it does tbh.  We currently have two implementations of
> packages->profile-entry, one for Guix System, one for Guix Home, which
> at the time of writing are exactly the same.

Looks like we could start by factorizing it.  :-)

> My use case of naming profiles would be served by such a procedure,
> but using a syntactic constructor has other benefits in that all of
> the fields of the profile become accessible.  That means that users
> could (once profile management via Guix Home is implemented) for
> instance run less hooks or additional hooks for certain profiles,
> allow collisions, use relative symlinks, etc. for basically free, not
> to mention that changes which break record ABI (such as added fields)
> get promoted directly through syntax but not through a plain
> procedure.

This is an argument (and probably a good one) in favor of using
<profile> records.  I don’t read it as an argument in favor of the
‘packages’ pseudo field though?

Thanks,
Ludo’.
  
Maxime Devos June 2, 2022, 3:07 p.m. UTC | #8
Ludovic Courtès schreef op do 02-06-2022 om 15:32 [+0200]:
> Still I’m not a fan of having syntax that looks like a field but is not
> an actual field, if we can avoid it.  I prefer to expose the data
> structure as it exists and, if needed, to build abstractions on top of
> it.  (The ABI issue that Maxime mention is real but I don’t think it’s a
> big problem in practice.)

FWIW I've some ideas for stable ABIs for record constructors even when
fields are added/removed or change from direct/thunked/delayed, a
stable ABI can be defined later.

Greetings,
Maxime.
  
Liliana Marie Prikler June 2, 2022, 4:51 p.m. UTC | #9
Am Donnerstag, dem 02.06.2022 um 15:32 +0200 schrieb Ludovic Courtès:
> Hallo!
> 
> Liliana Marie Prikler <liliana.prikler@gmail.com> skribis:
> 
> > If it reads like that, then that's probably a mistake somewhere. 
> > My actual proposal to allow both of the following:
> > 
> > (package
> >   other-fields ...
> >   (manifest some-manifest))
> > (package 
> >   other-fields ...
> >   (packages (list bash coreutils emacs ...)))
> 
> Oh OK, I got it wrong, sorry.
Actually, I too got it wrong in the patch description.  The
implementation and test should be fine though.

> Still I’m not a fan of having syntax that looks like a field but is
> not an actual field, if we can avoid it.  I prefer to expose the data
> structure as it exists and, if needed, to build abstractions on top
> of it.
I can see where you're coming from, but IMHO the content field does not
itself provide a useful abstraction, and changing the profile record +
constructor would itself break ABI.  Thus the syntactic sugar.


> [...]
> > My use case of naming profiles would be served by such a procedure,
> > but using a syntactic constructor has other benefits in that all of
> > the fields of the profile become accessible.  That means that users
> > could (once profile management via Guix Home is implemented) for
> > instance run less hooks or additional hooks for certain profiles,
> > allow collisions, use relative symlinks, etc. for basically free,
> > not to mention that changes which break record ABI (such as added
> > fields) get promoted directly through syntax but not through a
> > plain procedure.
> 
> This is an argument (and probably a good one) in favor of using
> <profile> records.  I don’t read it as an argument in favor of the
> ‘packages’ pseudo field though?
Indeed, my argument for the pseudo-field is mere readability.  Compare
the four semantically equivalent ways of writing a profile in the test
case I added.

Cheers
  

Patch

diff --git a/guix/profiles.scm b/guix/profiles.scm
index bf50c00a1e..fbc343c456 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -1974,7 +1974,7 @@  (define builder
                                            (manifest-entries manifest))))))))
 
 ;; Declarative profile.
-(define-record-type* <profile> profile make-profile
+(define-record-type* <profile> %profile make-profile
   profile?
   (name               profile-name (default "profile")) ;string
   (content            profile-content)                  ;<manifest>
@@ -1987,6 +1987,27 @@  (define-record-type* <profile> profile make-profile
   (relative-symlinks? profile-relative-symlinks?  ;Boolean
                       (default #f)))
 
+(define-syntax package-compatibility-helper
+  (syntax-rules (packages manifest)
+    ((_ () (fields ...))
+     (%profile fields ...))
+    ((_ ((packages exp) rest ...) (others ...))
+     (package-compatibility-helper
+      (rest ...)
+      (others ... (content (packages->manifest
+                            (delete-duplicates exp eq?))))))
+    ((_ ((manifest exp) rest ...) (others ...))
+     (package-compatibility-helper
+      (rest ...)
+      (others ... (content exp))))
+    ((_ (field rest ...) (others ...))
+     (package-compatibility-helper (rest ...) (others ... field)))))
+
+(define-syntax-rule (profile fields ...)
+  "Build a <profile> record, automatically converting 'packages' or 'manifest '
+field specifications to 'content'."
+  (package-compatibility-helper (fields ...) ()))
+
 (define-gexp-compiler (profile-compiler (profile <profile>) system target)
   "Compile PROFILE to a derivation."
   (match profile
diff --git a/tests/profiles.scm b/tests/profiles.scm
index d59d75985f..970a34b6cc 100644
--- a/tests/profiles.scm
+++ b/tests/profiles.scm
@@ -272,6 +272,22 @@  (define transform1
                                   (manifest-pattern (name name))))
            '("gcc" "binutils" "glibc" "coreutils" "grep" "sed"))))
 
+(test-assert "profile syntax sugar"
+  (let ((p1 (dummy-package "p1"))
+        (p2 (dummy-package "p2")))
+    (define (profile=? . profiles)
+      (define (manifest=? . manifests)
+        ;; Since we're using the same packages, we could also compare via eq?
+        (apply list= manifest-entry=? (map manifest-entries manifests)))
+      (apply manifest=? (map profile-content profiles)))
+
+    (profile=?
+     (profile (content (manifest
+                        (map package->manifest-entry (list p1 p2)))))
+     (profile (content (packages->manifest (list p1 p2))))
+     (profile (manifest (packages->manifest (list p1 p2))))
+     (profile (packages (list p1 p2))))))
+
 (test-assertm "profile-derivation"
   (mlet* %store-monad
       ((entry ->   (package->manifest-entry %bootstrap-guile))