Activations interfere with each other modules

Open

Details

2 participants

Hilton Chain
Tomas Volf

Owner: unassigned

Submitted by: Tomas Volf

Severity: normal

Debbugs page

Tomas Volf wrote 1 weeks ago

Recipients:(address . bug-guix@gnu.org)

Message-ID:87jz97gc4h.fsf@wolfsden.cz

Hello,

when using `activation-service-type' combined with `with-extension', the

various modules seem to interfere with each other.

In my case the samba-service-type interferes with my custom service:

Toggle snippet (16 lines)(define (watcher-activation* config)
  (match-record config <watcher-configuration>
                (user group cache-dir state-dir)
    (with-extension/guile-wolfsden
     #~(begin
         (use-modules (wolfsden fs)
                      (wolfsden sh))
         (for-each (lambda (dir)
                     (mkdir-p dir)
                     (invoke* #$(file-append coreutils "/bin/chown")
                              "-Rv" (string-append #$user ":" #$group) dir)
                     (chmod dir #o700))
                   '(#$cache-dir
                     #$state-dir))))))

samba-service-type's activation starts with:

Toggle snippet (6 lines)

(define (samba-activation config)

(let ((package (samba-configuration-package config))

(config-file (samba-configuration-config-file config)))

(with-imported-modules '((guix build utils))

Both (guix build utils) and (wolfsden sh) provide (invoke) procedure,

however with incompatible parameters, leading to somewhat annoying to

debug issues. In my case the error was:

Toggle snippet (4 lines)

guix deploy: error: failed to deploy x: failed to switch systems while deploying '127.0.0.1':

keyword-argument-error #f "Invalid keyword" () ("/etc/samba/smb.conf")

Which makes sense now when I understand what the problem is, but it took
while to get to it.

I do not think this should be the case.  Either each activation service
should be isolated into a fresh user module, or the services provided by
Guix should not introduce any additional modules into the scope.
Alternatively, each activation service could be wrapped into
program-file, that would ensure isolation as well.

Tomas
-- 
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.

Hilton Chain wrote 6 days ago

Recipients:(name . Tomas Volf)(address . ~@wolfsden.cz)

Message-ID:87cyeuua0l.wl-hako@ultrarare.space

Hi Tomas,

On Mon, 03 Mar 2025 09:02:22 +0800,

Tomas Volf wrote:

Toggle quote (52 lines)>
> Hello,
>
> when using `activation-service-type' combined with `with-extension', the
> various modules seem to interfere with each other.
>
> In my case the samba-service-type interferes with my custom service:
>
> --8<---------------cut here---------------start------------->8---
> (define (watcher-activation* config)
>   (match-record config <watcher-configuration>
>                 (user group cache-dir state-dir)
>     (with-extension/guile-wolfsden
>      #~(begin
>          (use-modules (wolfsden fs)
>                       (wolfsden sh))
>          (for-each (lambda (dir)
>                      (mkdir-p dir)
>                      (invoke* #$(file-append coreutils "/bin/chown")
>                               "-Rv" (string-append #$user ":" #$group) dir)
>                      (chmod dir #o700))
>                    '(#$cache-dir
>                      #$state-dir))))))
> --8<---------------cut here---------------end--------------->8---
>
> samba-service-type's activation starts with:
>
> --8<---------------cut here---------------start------------->8---
> (define (samba-activation config)
>   (let ((package (samba-configuration-package config))
>         (config-file (samba-configuration-config-file config)))
>     (with-imported-modules '((guix build utils))
> --8<---------------cut here---------------end--------------->8---
>
> Both (guix build utils) and (wolfsden sh) provide (invoke) procedure,
> however with incompatible parameters, leading to somewhat annoying to
> debug issues.  In my case the error was:
>
> --8<---------------cut here---------------start------------->8---
> guix deploy: error: failed to deploy x: failed to switch systems while deploying '127.0.0.1':
> keyword-argument-error #f "Invalid keyword" () ("/etc/samba/smb.conf")
> --8<---------------cut here---------------end--------------->8---
>
> Which makes sense now when I understand what the problem is, but it took
> while to get to it.
>
> I do not think this should be the case.  Either each activation service
> should be isolated into a fresh user module, or the services provided by
> Guix should not introduce any additional modules into the scope.
> Alternatively, each activation service could be wrapped into
> program-file, that would ensure isolation as well.

Activation scripts are already ‘program-file’s, but unfortunately

(gnu build activation) and (guix build utils) are implicit dependencies. This

is hard to change at the moment.

I have sent a patch which might partially address your issue:

[PATCH v2 3/3] services: activation: Continue on exceptions.

https://issues.guix.gnu.org/73494#26

It executes activation scripts by ‘invoke’, so they won't change the

environment.

What I'm trying to achieve is to avoid blocking the activation process when one

script fails.

With that patch, when one script fails, its backtrace is printed out by

‘invoke’, additionally a warning is emitted to indicate the failed script:

Toggle snippet (26 lines)

activating system...

The following derivation will be built:

/gnu/store/78x8n7b58nqcllfqfpr4p73q6n2wazak-switch-to-system.scm.drv

building /gnu/store/78x8n7b58nqcllfqfpr4p73q6n2wazak-switch-to-system.scm.drv...

making '/var/guix/profiles/system-110-link' the current system...

populating /etc from /gnu/store/v206hfm2idagh2nczc6ii0m628xyg5dk-etc...

setting up privileged programs in '/run/privileged/bin'...

Backtrace:

3 (primitive-load "/gnu/store/59651880cmil350zf5hagyhdi7y…")

In ice-9/eval.scm:

619:8 2 (_ #f)

619:8 1 (_ #(#(#(#(#<directory (guile-user) 7fdc0c317…>) …) …) …))

In unknown file:

0 (rmdir "/var/run/dbus")

ERROR: In procedure rmdir:

In procedure rmdir: Directory not empty

guix system: warning: failed to activate '/gnu/store/59651880cmil350zf5hagyhdi7y66dgz-activate-service.scm'.

The following derivation will be built:

/gnu/store/m1c65sxmqvvpfdsazcka51miixcpsy9g-install-bootloader.scm.drv

building /gnu/store/m1c65sxmqvvpfdsazcka51miixcpsy9g-install-bootloader.scm.drv...

guix system: bootloader successfully installed on '(/efi)'

(Cc'd Ludo) Any concern on this change? Does the "TODO: Use 'load-compiled'."

still apply?

Tomas Volf wrote 4 days ago

Recipients:(name . Hilton Chain)(address . hako@ultrarare.space)

Message-ID:87y0xgzfzk.fsf@wolfsden.cz

Hilton Chain <hako@ultrarare.space> writes:

Toggle quote (58 lines)> Hi Tomas,
>
> On Mon, 03 Mar 2025 09:02:22 +0800,
> Tomas Volf wrote:
>>
>> Hello,
>>
>> when using `activation-service-type' combined with `with-extension', the
>> various modules seem to interfere with each other.
>>
>> In my case the samba-service-type interferes with my custom service:
>>
>> --8<---------------cut here---------------start------------->8---
>> (define (watcher-activation* config)
>>   (match-record config <watcher-configuration>
>>                 (user group cache-dir state-dir)
>>     (with-extension/guile-wolfsden
>>      #~(begin
>>          (use-modules (wolfsden fs)
>>                       (wolfsden sh))
>>          (for-each (lambda (dir)
>>                      (mkdir-p dir)
>>                      (invoke* #$(file-append coreutils "/bin/chown")
>>                               "-Rv" (string-append #$user ":" #$group) dir)
>>                      (chmod dir #o700))
>>                    '(#$cache-dir
>>                      #$state-dir))))))
>> --8<---------------cut here---------------end--------------->8---
>>
>> samba-service-type's activation starts with:
>>
>> --8<---------------cut here---------------start------------->8---
>> (define (samba-activation config)
>>   (let ((package (samba-configuration-package config))
>>         (config-file (samba-configuration-config-file config)))
>>     (with-imported-modules '((guix build utils))
>> --8<---------------cut here---------------end--------------->8---
>>
>> Both (guix build utils) and (wolfsden sh) provide (invoke) procedure,
>> however with incompatible parameters, leading to somewhat annoying to
>> debug issues.  In my case the error was:
>>
>> --8<---------------cut here---------------start------------->8---
>> guix deploy: error: failed to deploy x: failed to switch systems while deploying '127.0.0.1':
>> keyword-argument-error #f "Invalid keyword" () ("/etc/samba/smb.conf")
>> --8<---------------cut here---------------end--------------->8---
>>
>> Which makes sense now when I understand what the problem is, but it took
>> while to get to it.
>>
>> I do not think this should be the case.  Either each activation service
>> should be isolated into a fresh user module, or the services provided by
>> Guix should not introduce any additional modules into the scope.
>> Alternatively, each activation service could be wrapped into
>> program-file, that would ensure isolation as well.
>
> Activation scripts are already ‘program-file’s,

Ah, yes, on second look you are right. I did not realized at first,

since they are sourced using primitive-load instead of executed.

Toggle quote (3 lines)

> but unfortunately (gnu build activation) and (guix build utils) are

> implicit dependencies. This is hard to change at the moment.

I do not think this is true. When I look at the activation script for

my service, I do not see any additional code then my service has. So

assuming it would be executed, instead of loaded, (guix build utils)

should not be in the environment. Am I missing something?

Toggle quote (12 lines)>
> I have sent a patch which might partially address your issue:
>
> [PATCH v2 3/3] services: activation: Continue on exceptions.
> https://issues.guix.gnu.org/73494#26
>
> It executes activation scripts by ‘invoke’, so they won't change the
> environment.
>
> What I'm trying to achieve is to avoid blocking the activation process when one
> script fails.

Well, that patch would probably resolve my issues, correct.  What I am
unsure about is whether ignoring errors is a good idea and what (if any)
impact it will have on the rest of the deploy process.  Like, if the
activation fails, would it not be better to rollback instead of push
forward?  Dunno, I probably just do not know enough about this. :)

Tomas

-- 
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.

Hilton Chain wrote 3 days ago

Recipients:(name . Tomas Volf)(address . ~@wolfsden.cz)

Message-ID:87pliqaig3.wl-hako@ultrarare.space

On Sat, 08 Mar 2025 05:29:51 +0800,

Tomas Volf wrote:

Toggle quote (9 lines)

> > but unfortunately (gnu build activation) and (guix build utils) are

> > implicit dependencies. This is hard to change at the moment.

> I do not think this is true. When I look at the activation script for

> my service, I do not see any additional code then my service has. So

> assuming it would be executed, instead of loaded, (guix build utils)

> should not be in the environment. Am I missing something?

(gnu build activation) and (guix build utils) are added to the environment

before loading activation scripts, existing activation services may depend on

this. For example firmware-service-type uses ‘activate-firmware’ directly

without importing (gnu build activation).

Toggle quote (17 lines)> > I have sent a patch which might partially address your issue:
> >
> > [PATCH v2 3/3] services: activation: Continue on exceptions.
> > https://issues.guix.gnu.org/73494#26
> >
> > It executes activation scripts by ‘invoke’, so they won't change the
> > environment.
> >
> > What I'm trying to achieve is to avoid blocking the activation process when one
> > script fails.
>
> Well, that patch would probably resolve my issues, correct.  What I am
> unsure about is whether ignoring errors is a good idea and what (if any)
> impact it will have on the rest of the deploy process.  Like, if the
> activation fails, would it not be better to rollback instead of push
> forward?  Dunno, I probably just do not know enough about this. :)

Makes sense, but too late at this stage since it's already switched to the new
generation before activation.

The current behavior is, one failed activation breaks the whole activation
process, without a clear indication.  As a result, all services depending on
activation may fail.  And I want to limit failed services to relevant ones.

Tomas Volf wrote 2 days ago

Recipients:(name . Hilton Chain)(address . hako@ultrarare.space)

Message-ID:87bjuazb5g.fsf@wolfsden.cz

Hilton Chain <hako@ultrarare.space> writes:

Toggle quote (16 lines)> On Sat, 08 Mar 2025 05:29:51 +0800,
> Tomas Volf wrote:
>>
>> > but unfortunately (gnu build activation) and (guix build utils) are
>> > implicit dependencies.  This is hard to change at the moment.
>>
>> I do not think this is true.  When I look at the activation script for
>> my service, I do not see any additional code then my service has.  So
>> assuming it would be executed, instead of loaded, (guix build utils)
>> should not be in the environment.  Am I missing something?
>
> (gnu build activation) and (guix build utils) are added to the environment
> before loading activation scripts, existing activation services may depend on
> this.  For example firmware-service-type uses ‘activate-firmware’ directly
> without importing (gnu build activation).

I am not sure I follow. When I have an activation service looking like:

Toggle snippet (3 lines)

#~(mkdir "/x")

The generated file looks like the following:

Toggle snippet (5 lines)

#!/gnu/store/ylwk2vn18dkzkj0nxq2h4vjzhz17bm7c-guile-3.0.9/bin/guile --no-auto-compile

(mkdir "/x")

Wait... When I looked again I have now noticed that your patch actually
modifies the activation-script procedure to explicitly add the modules.
It did not do that before.  That is a shame.  Would it not be better to
just add the appropriate use-modules into the services that are missing
them?

I am not sure these two modules being available is documented anywhere,
so in-tree users can be fixed, and out-of-tree users are relying on
something they should not assume (so they should be fixed as well,
possibly with some deprecation period).

Toggle quote (26 lines)>
>> > I have sent a patch which might partially address your issue:
>> >
>> > [PATCH v2 3/3] services: activation: Continue on exceptions.
>> > https://issues.guix.gnu.org/73494#26
>> >
>> > It executes activation scripts by ‘invoke’, so they won't change the
>> > environment.
>> >
>> > What I'm trying to achieve is to avoid blocking the activation process when one
>> > script fails.
>>
>> Well, that patch would probably resolve my issues, correct.  What I am
>> unsure about is whether ignoring errors is a good idea and what (if any)
>> impact it will have on the rest of the deploy process.  Like, if the
>> activation fails, would it not be better to rollback instead of push
>> forward?  Dunno, I probably just do not know enough about this. :)
>
> Makes sense, but too late at this stage since it's already switched to the new
> generation before activation.
>
> The current behavior is, one failed activation breaks the whole activation
> process, without a clear indication.  As a result, all services depending on
> activation may fail.  And I want to limit failed services to relevant
> ones.

I see. Makes sense, thanks for explanation.

Tomas

There are only two hard things in Computer Science:

cache invalidation, naming things and off-by-one errors.

Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 76698@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it

mumi current 76698

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

You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags

mumi command -t +confirmed -t +easy

Or, remove the moreinfo tag and set the help tag

mumi command -t -moreinfo -t +help

is:open	open issues
is:done	closed issues
submitter:<who>	search issue submitter
author:<who>	search by message author
date:yesterday..now	search by issue date
mdate:3m..2d	search by message date