On Fri, Jan 05, 2024 at 02:18:08 -0800, Andrea Bolognani wrote:
On Fri, Jan 05, 2024 at 10:53:10AM +0100, Peter Krempa wrote:
- mingw builds for some unkonwn-to-me reason use '--auto-features=enabled' which is weird - the logic handling errors in use of the 'nbdkit_config_default' meson option is a bit questionable:
--auto-features=enabled -Dnbdkit=disabled -Dnbdkit_config_default=auto
should NOT result in a failure even when the basic logic says so
My patch is strictly fixing the build failure by explicitly disabling nbdkit_config_default on mingw and enabling 'nbdkit' only on rhel>=9.
You can investigate how to improve the questionable logic in auto-selection of nbdkit_config_default.
In the long run, the logic should probably just be "if nbdkit_config_default is auto, then use the same value as nbdkit". Right now of course that can't work because we want to keep the former disabled by default.
The question is, when auto-features=enabled is used, can you really implement something reasonable? In other words, can you tell
--auto-features=enabled -Dnbdkit_config_default=auto
and
-Dnbdkit_config_default=enabled
apart? If that's not the case, auto-features=enabled seems pretty much completely broken.
Explicitly setting -Dnbdkit_config_default=disabled in the mingw part of the spec file was the right thing to do anyway, it was just unfortunately missed. Thankfully, now that we do mingw RPM builds as part of the CI pipeline, it was caught reasonably early :)
A brief look into meson docs and a few tries seem to confirm your suspicion that auto-features=enabled is an anti-feature. If you declare an option as a 'feature' in meson_options.txt (a config file with .txt siuffix?!?!?) then internally it's a UserFeatureOption object and you have the .enabled(), .disabled(). and .auto() methods at your disposal, but they operate on the final value after 'auto-features' was applied to any 'auto' setting. Even newer methods such as '.disable_auto_if()' seem to apply only when the final setting is auto, so auto-features=enabled breaks it. IMO auto-features=enabled shouldn't be used and definitely not in our spec file. I can see value in auto-features=disabled though, which IMO should be preferable also in spec files.