On 11 January 2022 19:17:56 CET Jonas Smedegaard wrote: > Quoting Diederik de Haas (2022-01-11 16:39:47) > > One of the upstream maintainers ran a build and reported that 22 tests > > ran and they all succeeded. On buildd 1 test out of *20* failed. > > The Debian package currently disables 2 tests, due to them (succeeding > locally using cowbuild, but) failing on Debian build daemons using > sbuild: > https://salsa.debian.org/debian/iwd/-/blob/debian/master/debian/rules#L5 > Mentioning both as that explains the 20 versus 22 tests, but also
Good to know where the difference in the number of tests comes from. But I consider disabling tests a very last resource method. Or valid when the test really does not make sense (in this context). Tests are meant to reveal (potential) problems (due to code changes) and disabling test makes you ignore the problems, instead of fixing them. <snip> > > a kernel module can be builtin, like CONFIG_CRYPTO_SHA256 is. I looked up why it was builtin the Debian kernel and found this: #. Must be built-in for IMA_DEFAULT_HASH_SHA256 CONFIG_CRYPTO_SHA256=y That (almost) sound like an apology to set it to =y instead of =m (More on this later...) The snippet above is from debian/config/config#L121 > In itself your report helps point to a likely cause of test failures > that had me puzzled, as mentioned above. So thanks for that! Cool :) > Simplest is to just skip failing tests - as I did already. As described above, not a big fan of such 'solutions'. > Next would be to share that upstream, and (as they deem sensible) allow > them to implement gracefully skipping tests generally, for the benefit > of other users as well - as you did already for one of the 3 tests. I'm all for degrading gracefully, but I did not suggest that; one of the upstream maintainers came up with that. The reason I said to do it in Debian and not upstream is that tests whether certain kernel modules are available (builtin or as module) is specific per distribution. In Debian you can check /boot/config-$(uname -r), while on other systems you need to grep /proc/config or (zgrep) /proc/config.gz (or (z)cat-ing and grep-ing), optionally after modprobe-ing a kernel module which loads it there. Other distributions may also store it on disk like Debian does, but uses a different filename convention. > Maybe the kernel modules needed for (some of) these 3 failing tests > would be generally beneficial for Debian users to have compiled built-in > for the default Debian kernel. If that seems likely, it would be > helpful to open a bug against the package "linux" suggesting that to the > linux kernel maintainers. Generally beneficial is a reason to enable a module ... as loadable module. Only if it's absolutely required, will it be builtin. I know that because in the last year I've learned how to build a kernel myself and also contributed some improvements to the Debian kernel. In https://salsa.debian.org/kernel-team/linux/-/merge_requests/381 I proposed several changes to the *rpi* kernel variant and the following change was NOT merged: `CONFIG_CPU_FREQ_GOV_CONSERVATIVE=y` because it's not strictly needed. I also learned that modprobe-ing (or sth similar) is considered perfectly normal. (This work is the reason I was able to come up with my analyses.) > I guess (but don't know for sure) the official build daemons cannot be > told to load modules - i.e. that builds are executes inside some > container not permitting that. As you also elaborate on, that might > open a can of worm related to reproducibility. I think the DSA team can (and should be able to) answer that? If it is indeed not permitted, then disabling that test is the only solution. But I think it's worth asking/having that discussion as explicitly loading kernel modules is (normally) considered perfectly fine. And as iwd explicitly and deliberately uses kernel functionality, which may be available (on disk), just not loaded into RAM (yet), it seems like a very valid use case to me. > So I kinda think it is > better to *not* check tests requiring non-eneric hardware features. > Somewhat related to that is this recent discussion about a package > depending on certain GPU features for its testsuite: > https://lists.debian.org/[email protected] > m I think that case is significantly different as it actually wants to use 'external' hardware. We/iwd want to use kernel features/modules (which its *auto*-loading may be triggered by certain hardware), which are waiting to be used and do not depend on external hardware (f.e. because it's baked into many CPUs already). The kernel can do various crypto operations, whether it is *accelerated* by hardware or not. > Seems to me that this case is similar, and we should simply enable only > the parts of the testsuite that can _generally_ be expected to succeed > (and work with upstream to improve tests to gracefully handle kernel > features missing, so that we can enable the full testsuite). I think this is extremely hard if not impossible as anyone can create their own kernel configuration and other distros likely have (significantly) different kernel module policies. Some make kernels specific per board, while Debian uses 1 kernel config for (f.e.) all arm64 boards. Some make generally useful modules (for a board) builtin, while Debian does not. Etc. > > PS: Based on the discussion I had on IRC, the following patch was > > proposed (and applied) upstream: > > https://lists.01.org/hyperkitty/list/[email protected]/thread/USXHQD27OI7RJ > > EC52MPJLNGQCKEBBJKO/ > Great. If I read that patch correctly, that means we can re-enable > unit/test-dpp when that patch lands (i.e. from next upstream release). I actually think it won't make a difference as I get the *feeling* that it is testing for availability for SHA256 kernel module, which is the one which is actually loaded in all systems (see above). But I actually lack the knowledge to properly understand the l_checksum_is_supported function, so I may be totally wrong. > Would be helpful if you could suggest upstream to try identify and > similarly patch tests unit/test-wsc unit/test-eap-sim - assuming the > cause of failure is similar (since the pattern - failure not locally but > on build daemons - is same). As I think this should be resolved in Debian, even disabling tests if there is no other way, and I'm entirely clueless about pretty much the whole codebase, so I won't be able to give any (intelligent) answer to questions they'd undoubtedly have, I have no plans to do so. (sorry) Cheers, Diederik
signature.asc
Description: This is a digitally signed message part.
