Page MenuHomeFreeBSD

poc of atf_require_kld
Needs ReviewPublic

Authored by des on Nov 6 2024, 11:58 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 12, 6:41 PM
Unknown Object (File)
Sun, Dec 8, 11:54 PM
Unknown Object (File)
Sat, Dec 7, 10:52 AM
Unknown Object (File)
Fri, Dec 6, 3:34 PM
Unknown Object (File)
Fri, Dec 6, 8:32 AM
Unknown Object (File)
Fri, Dec 6, 4:55 AM
Unknown Object (File)
Thu, Dec 5, 10:41 AM
Unknown Object (File)
Tue, Dec 3, 9:24 PM
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 60553
Build 57437: arc lint + arc unit

Event Timeline

des requested review of this revision.Nov 6 2024, 11:58 PM

Thank you for working on this. If you plan to go ahead then please consider the following points.

  • The history behind ATF and Kyua is a bit messy, and the baseline is that ATF is left just as a test library to provide assertions etc. The high level things are covered by Kyua, and that's for good due to not all tests are based on ATF. That means that we need no changes on ATF at all. An ATF-based test case can simply declare require.modules metadata property and Kyua will read it and act on it.
  • I like the full word "modules", it follows the existing ATF/Kyua interface. And it would be much better to keep ATF and Kyua aligned and use the same, i.e. require.modules one ATF side and required_modules on Kyua side. Yes, the style differs (require vs. required, dots vs. underscores), but it's better for end users to follow the existing style.
  • The Kyua project is still kept as a cross-platform tool provided as github:freebsd/kyua public project, and it's relatively easy to make it built for Linux and macOS. We've been striving to keep it so. And I know about Linux and macOS users. For that we use conditional compilation with FreeBSD-specific code separated beneath os/freebsd src tree. It would be great to follow that and keep libutil usage extracted from the main code. I would propose to use something similar to the execution environment registry, i.e. we could introduce something like requirement checker registry which could be iterated by the engine in addition to the common checkers.
  • It would be great if Kyua reports all missing modules and not only the very first from the list. It must be easy to do, and this behavior was announced in the mailing lists.
  • To increase the chances that it goes as expected the following testing is encouraged for the patch:
    • on FreeBSD as part of FreeBSD base (usr.bin/kyua)
    • on FreeBSD as a stand-alone tarball-based third-party software from github:freebsd/kyua -- in order to run tests of Kyua itself and polish the rough edges
    • on Linux (e.g. Fedora) as a stand-alone tarball-based third-party software from github:freebsd/kyua -- in order to run tests of Kyua itself and polish the conditional compilation/linking
  • The respective man pages update. It looks kyuafile(5) must be enough.
  • On the final stage we will need to push it as a PR to the github:freebsd/kyua repo. Yeah, this is not the ideal process, but we are trying to keep the repos aligned to ease future work. ngie@ usually helps with that.
  • I'm probably forgetting some other topics, they will come up as needed.

Thanks for your time investment!

You seem to have me confused with a GSoC student. Do you want to try again?

The email (https://lists.freebsd.org/archives/freebsd-testing/2024-November/000399.html) was understood to mean that you wanted to offer help with the development and were asking about a review. This was interpreted as a request to take over the project entirely, which is why I shared the full plan I intended to follow—to avoid prolonging the process, reduce the number of iterations, and clearly outline all expectations upfront. Please, let me know if you did not intend to work on this project and the email was misunderstood.

No. I had done the work already and posted the review when I saw your email to avoid having someone else waste their time reproducing it.

You're also wrong about ATF vs. Kyua. We could replace Kyua as a test runner with relatively little effort (and almost _anything_ we could come up with would be better than Kyua) but would need years to convert all our existing ATF tests to some other test library. There may also be cases where we don't know if we need a module or not until we start the test. Therefore, implementing only the require.klds property but not atf_require_kld() would be a serious mistake.

  • #ifdef __FreeBSD__
  • s/kld/module/g
  • Avoid linking libatf with libutil
In D47470#1084294, @des wrote:

No. I had done the work already and posted the review when I saw your email to avoid having someone else waste their time reproducing it.

What is your intention with the patch? Igor has noted some missing pieces (documentation updates, upstreaming to ATF/kyua repos, having kyua print the full list of missing modules when one module is missing) and I think he would work on them. Should he take this patch and extend it and handle upstreaming, or do you plan to finish it?

contrib/atf/atf-c/tc.h
110

Sorry to provide another opinion after this has already changed, but IMO "module" is too generic - here it means "kernel module", so it should be spelled "kernel_module" or similar.

What is your intention with the patch?

I wrote it because I needed it. I'll be grateful if someone else steps up to add the missing bits (mostly documentation) but otherwise I'll try to find the time to finish it.

having kyua print the full list of missing modules when one module is missing

I never promised this, I'll let the person who did and who appears to think it's trivial take care of it.

Great, I can continue from any point where you decide to stop and land your commits.

By the way, regarding the question about the name that Mark brought up, how about required_kmods? The "kmod" abbr seems to be more common among major Unix-like systems (e.g. Linux community uses "kmod"), likely making its meaning easier to grasp, while keeping it short and OS agnostic. What do you think?