[PATCH v1] channels: add transformer field

  • Open
  • quality assurance status badge
Details
2 participants
  • Jakob Kirsch
  • Rutherther
Owner
unassigned
Submitted by
Jakob Kirsch
Severity
normal

Debbugs page

J
J
Jakob Kirsch wrote on 17 Mar 08:19 -0700
(address . guix-patches@gnu.org)(address . guix-devel@gnu.org)
Z9g9hxAAk41ABXxZ@kernelpanicroom
Hey everyone,
I've recently tried to declaratively apply my own patches to the upstream guix channel or channels in general but I found it very hard (aka impossible) to do that cleanly so I made this simple patch.

This patch adds a new "transformer" field to the channel record. That "transformer" is a lambda that is called right before applying the quirk patches with the checkout path being its only argument.

What do you think about my proposal?
From 36462095039632348ed5fbd5763426f74f48049f Mon Sep 17 00:00:00 2001
Message-ID: <36462095039632348ed5fbd5763426f74f48049f.1742224546.git.jakob.kirsch@web.de>
From: Jakob Kirsch <jakob.kirsch@web.de>
Date: Mon, 17 Mar 2025 14:53:07 +0100
Subject: [PATCH v1] channels: add transformer field

Change-Id: I46c065eb096d9fccefde7a791e4373a614deac33
---
guix/channels.scm | 3 +++
1 file changed, 3 insertions(+)

Toggle diff (23 lines)
diff --git a/guix/channels.scm b/guix/channels.scm
index 4700f7a45d..091a5c2f16 100644
--- a/guix/channels.scm
+++ b/guix/channels.scm
@@ -135,6 +135,7 @@ (define-record-type* <channel> channel make-channel
(branch channel-branch (default "master"))
(commit channel-commit (default #f))
(introduction channel-introduction (default #f))
+ (transformer channel-transformer (default (lambda checkout #t)))
(location channel-location
(default (current-source-location)) (innate)))

@@ -456,6 +457,8 @@ (define* (latest-channel-instance store channel
thus potentially malicious code.")))))))))
(warning (G_ "channel authentication disabled~%")))

+ (apply (channel-transformer channel) (list checkout))
+
(when (guix-channel? channel)
;; Apply the relevant subset of PATCHES directly in CHECKOUT. This is
;; safe to do because 'switch-to-ref' eventually does a hard reset.

base-commit: 98be320183579b3d09cf4059e86a9781485628b4
--
2.48.1
R
R
Rutherther wrote on 22 Mar 03:13 -0700
(address . 77072@debbugs.gnu.org)(name . Jakob Kirsch)(address . jakob.kirsch@web.de)
87ecypxtjd.fsf@ditigal.xyz
Hello Jakob,

I also encountered this issue and would like a way to transform
channels. However, I have a few notes.

While looking through the code I noticed latest-channel-instance
has an argument called patches. This argument is already capable of
applying patches. Each patch has a predicate and application function,
First the predicate is ran to know if the patch is applicable,
and if so, then it's applied with the application function.

Wouldn't it make sense to rather than making a new mechanism,
extend the current one, by adding to channels a new field
that will hold list of `patch` records, and then in
latest-channels-instance we would just do

```
(apply-patches checkout commit (channel-patches patches))
```

?

Apart from that I am wondering if this is going to be user friendly,
because I would imagine most people will just want to patch with already
existing commits. And for that it would be much easier to just have a
list of files to apply, similar to what origin has.
While this will be possible with this transformer field, it will mean
calling the patch (or I could've missed a function that does that
already).
So maybe at least exposing a new procedure to apply a list of patches
would be good, so users can then just do something like:

```
(channel
(patches ;; already expecting list of patch records
(make-channel-patches
(list (local-file "fix-1.patch")))))
```
Where make-channel-patches takes a list of files
and returns a patch record returning #t for predicate,
and applying by calling the patch executable for each file, similarly
to what patch-and-repack in guix/packages.scm does.
Note that I am not sure if local-file is appropriate here
or if gexps are unusable.

What do you think?

Regards,
Rutherther
R
R
Rutherther wrote on 23 May 13:08 -0700
(address . 77072@debbugs.gnu.org)
87tt5bnl7z.fsf@ditigal.xyz
I was very sad I couldn't use this feature now to patch the channels.
And then it hit me! I can use it! I will pull guix with this patch,
once, and then keep this patch in the transformers fields until it's
merged! So the patch will be 'bootstraping' itself on each pull :D
J
J
Jakob Kirsch wrote on 24 May 06:10 -0700
(name . Rutherther)(address . rutherther@ditigal.xyz)(address . 77072@debbugs.gnu.org)
aDHFMDTPoKePUIQI@kernelpanicroom
On Fri, May 23, 2025 at 10:08:48PM +0200, Rutherther wrote:
Toggle quote (6 lines)
>
> I was very sad I couldn't use this feature now to patch the channels.
> And then it hit me! I can use it! I will pull guix with this patch,
> once, and then keep this patch in the transformers fields until it's
> merged! So the patch will be 'bootstraping' itself on each pull :D

That's what I've been doing for a while now. I've even added a function to my own channel that makes applying patches from issues easier.

R
R
Rutherther wrote on 19 Sep 07:11 -0700
(address . 77072@debbugs.gnu.org)
87o6r6fs6v.fsf@ditigal.xyz
Hi,

I think this currently breaks inferiors cache.
Specifically, the cache relies on the `commit` fields, merging them together
and obtaining a hash. Since the transformer field is not taken into
account, there can be an incorrect cache hit, specifically:

1. You make an inferior with some for given commits
2. You make an inferior with different transformers for same commits
This way you get cache hit, omitting the new transformers.

Same way if you change transformers from one to another.

This breaks both inferior in code and time-machine.

Moreover, I don't think this has an easy resolution. The thing is
that the transformers can do whatever, point to a file, download a file
from the internet... this means the result might not be the same for
first, second, third time you run it, because the underlying files can
change. For this reason, we cannot just hash contents of the transformer
field as text. I think that something similar to fixed output derivation
would have to be made. Specifically to tell Guix the patches haven't
changed. I think that what I described in my initial answer would help
this goal - instead of a transformer to use patches, those patches
would have a hash. Those hashes can then be used in the inferior cache
as well as for not obtaining the files again if not necessary. (they
could be fetched to the store as FODs) Alternatively the patches
could first be all obtained and hash made from the contents. This
would achieve the goal, while allowing for the files to change,
ie. if you point it to a PR and the PR gets a new version.

Rutherther
?
Your comment

Commenting via the web interface is currently disabled.

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

To respond to this issue using the mumi CLI, first switch to it
mumi current 77072
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