eclean-kernel icon indicating copy to clipboard operation
eclean-kernel copied to clipboard

[WIP] Add unified efistub handling, add actions CI

Open ajakk opened this issue 2 years ago • 3 comments

This is derived from Jannik's work last year: https://github.com/Jannik2099/eclean-kernel/commit/32b1a2d48813fec60826fa5f23a54c9ff4774588

There's one remaining test failure, and I'm opening this now because I'm not really sure how to handle it yet:

FAILED test/test_file.py::KernelImageTests::test_overflow_ver_string_bz - AssertionError: UnrecognizedKernelError not raised

The semantics of how the file is read have changed, and I'm not sure how best to make it error in this way.

ajakk avatar Nov 22 '22 01:11 ajakk

Hm, I've just found that eclean-kernel doesn't seem to handle dropping the configuration file at loader/entries/${machineid}-${version}.conf. How should it be taught that this file should be dropped too?

ajakk avatar Nov 22 '22 01:11 ajakk

Most recent commit adds support to prune these config entries as mentioned in my previous comment.

ajakk avatar Nov 22 '22 01:11 ajakk

Could you rebase now, please?

mgorny avatar Dec 02 '22 03:12 mgorny

Are you asking me about something specific? I see test failures.

mgorny avatar Jan 15 '23 07:01 mgorny

Yeah, from my first comment:

There's one remaining test failure, and I'm opening this now because I'm not really sure how to handle it yet:

FAILED test/test_file.py::KernelImageTests::test_overflow_ver_string_bz - AssertionError: UnrecognizedKernelError not raised

The semantics of how the file is read have changed, and I'm not sure how best to make it error in this way.

ajakk avatar Jan 15 '23 16:01 ajakk

I'm afraid this "complete rewrite of everything in one commit" approach is not going to work for me. Please split it into smaller changes, adding tests as you add support for new file variations.

mgorny avatar Feb 28 '23 16:02 mgorny

Ping.

mgorny avatar Mar 09 '23 11:03 mgorny

2023-02-28 16:23:34     @ajak   i'm not really sure how to split it up further, it's really one thing being added, but it comes with a bit of code moving around (and much of the additions are comments)
2023-02-28 16:24:09     @ajak   there's not really atomic changes beyond what's there i don't think
2023-02-28 16:24:29     @mgorny "things being moved around" also justifies a separate commit
2023-02-28 16:24:39     @mgorny if it doesn't change the behavior, it should work atomically
2023-02-28 16:28:19     @ajak   oh, there's a bunch more test failures than i remember there being, wtf
2023-02-28 16:29:13     @mgorny yes, pretty much everything fails for me
2023-02-28 16:29:59     @ajak   well, if you come up with a better way to handle it, please do

ajakk avatar Mar 10 '23 15:03 ajakk