genodelabs / genode

Genode OS Framework
https://genode.org/
Other
1.07k stars 254 forks source link

Behavior of 'app/extract' as deployed by Genodians.org #5326

Closed cnuke closed 1 month ago

cnuke commented 2 months ago

The way the extract component is implemented and operated in the genodians.org appliance can lead to undesired behavior. A few days ago updating the website stopped as one of the extract operations failed:

[…]
[init -> genodians -> import -> fetchurl] fetch https://github.com/nfeske/genodian/archive/master.zip to /download/nfeske.zip
[init -> genodians -> import -> fetchurl] fetch https://github.com/mickenx/genodian/archive/main.zip to /download/mickenx.zip
[init -> genodians -> import -> fetchurl] fetch https://github.com/m-stein/genodian/archive/master.zip to /download/m-stein.zip
[…]
[init -> genodians -> import -> extract] extracted '/download/m-stein.zip' to '/content/m-stein/'
[init -> genodians -> import -> extract] Warning: reading from archive /download/mickenx.zip failed
[init -> genodians -> import -> extract] Error: Uncaught exception of type 'Genode::Exception'
[init -> genodians -> import -> extract] Warning: abort called - thread: ep
[…]

Since the extract component aborted the managing sequence component dwelt in the extract step as it only reacts upon a child exiting.

cnuke commented 2 months ago

Now, I am not overly happy with the commits implementation-wise but going by intend I favor the combination of the last ones. In particular for the gendians.org use-case where most of the time failures seem to be temporary processing the resources we can and trying the ones we could not in the next round worked well so far.

nfeske commented 2 months ago

Thanks @cnuke addressing this problem. Adding those new options looks sensible to me.

Implementation-wise, I think the bad smell comes from the use of exceptions. It would be very nice to follow up this work with the replacement of exceptions by the Attempt pattern. What do you think?

cnuke commented 2 months ago

@nfeske thanks for your suggestions, the following commits were adapted accordingly

nfeske commented 2 months ago

Beautiful work! Thank you very much. The README is the icing on the cake. :) Swiftly merged to staging.

nfeske commented 1 month ago

Merged in master.