Repository navigation
build: fail on impossible options, and modernize the meson files - #4148
Merged
Merged
Conversation
added 2 commits
October 9, 2026 13:25
An option set to "enabled" was dropped without an error when its condition was false, for example -Dnvme-discoverd=enabled with -Dfabrics=disabled, or -Dfabrics=enabled on FreeBSD. In the other direction, -Dnvmf-autoconnect=enabled with -Dfabrics=disabled installed units that run "nvme connect-all", which is not built without fabrics. Use require(): it fails the setup for an enabled option and turns an auto option off. fabrics, mi and top now default to auto, so the default build still works on FreeBSD and Windows. On Linux, auto builds them as before. A want_ variable now means a part of nvme-cli to build, and is set once, after all its checks. A library is tested through its dependency. The command-metadata test now checks that json-c was found, not only that it was allowed. Signed-off-by: Martin Belanger <martin.belanger@dell.com>
The install paths do not change. Signed-off-by: Martin Belanger <martin.belanger@dell.com>
Collaborator
|
Nice cleanup! Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes how the build handles options that cannot be built, and is also a cleanup of the meson files: it replaces older idioms with the ones meson provides today.
Fixes:
enabledwas dropped without an error when its condition was false. Examples:-Dnvme-discoverd=enabled -Dfabrics=disabled, or-Dfabrics=enabledon FreeBSD. The setup now fails with a message that names the missing condition.-Dnvmf-autoconnect=enabled -Dfabrics=disabledinstalled units that runnvme connect-all, which is not built without fabrics. This combination now fails too.Cleanup:
feature.require()andfeature.allowed()instead of hand-writtenif ... enabled() ... error()blocks.require()fails the setup for an enabled option and turns an auto option off.want_variable now means a part of nvme-cli to build, and is set once, after all its checks. A library is tested through its dependency (_dep.found()).join_paths()is replaced with the/operator. The install paths do not change.Behavior change:
fabrics,miandtopnow default toautoinstead ofenabled. On Linux they are built as before. On FreeBSD and Windows, the default build still works, and an explicitenablednow fails instead of being ignored.Tested with
meson setupin 53 option combinations, compared with master. Only the impossible combinations above now fail. All other combinations give the same configuration summary as master.