Skip to content

Conversation

@martin-belanger
Copy link

@martin-belanger martin-belanger commented Nov 10, 2025

Change the build flow so that libnvme is no longer defined as a Meson subproject of nvme-cli. Instead, include libnvme's meson.build directly from the top-level meson.build.

This allows sharing a common ccan directory between nvme-cli and libnvme, and generates a single configuration header, project-config.h (formerly config.h), used by all components. The old file name conflicted with plugins/solidigm/solidigm-telemetry/config.h, hence the rename.

Also reformat all meson.build files to fix indentation inconsistencies and other cosmetic discrepancies.

@igaw - I tested different build options (enabling all possible options including generating the docs). Everything is working although I couldn't test cross-compilation and other exotic options (e.g. muon).

One thing that I wasn't sure about is the licensing which is slightly different between nvme-cli and libnvme. I have kept both licenses as follows. I made sure these would get used in the nvme.spec.in and libnvme.spec.in.

project(
    'nvme-cli', ['c'],
    meson_version: '>= 0.62.0',
    license: [
        'GPL-2.0-only',         # nvme-cli
        'LGPL-2.1-or-later',    # libnvme
    ],
    version: '3.0-a.1',
    default_options: [
        'c_std=gnu99',
        'buildtype=debugoptimized',
        'prefix=/usr/local',
        'warning_level=1',
        'sysconfdir=etc',
        'wrap_mode=nofallback',
    ],
)

@martin-belanger martin-belanger force-pushed the do-not-subproject-libnvme branch 5 times, most recently from 595f21a to 363f4d9 Compare November 11, 2025 18:01
@igaw
Copy link
Collaborator

igaw commented Dec 1, 2025

Thanks for this work! There is some nice cleanups and improvements 👍

A few things should be addressed before the merge though. Nothing too serious IMO but the last point:

  • the python code doesn't build for me:
FAILED: [code=1] libnvme/libnvme/_nvme.cpython-314-x86_64-linux-gnu.so.p/meson-generated_.._nvme_wrap.c.o
cc -Ilibnvme/libnvme/_nvme.cpython-314-x86_64-linux-gnu.so.p -Ilibnvme/libnvme -I../libnvme/libnvme -I. -I.. -Iccan -I../ccan -Ilibnvme -I../libnvme -Ilibnvme/src -I../libnvme/src -I/usr/include/json-c -I/usr/include/python3.14 -fvisibility=hidden -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -std=gnu99 -O2 -g -fomit-frame-pointer -D_GNU_SOURCE -include project-config.h -fPIC -MD -MQ libnvme/libnvme/_nvme.cpython-314-x86_64-linux-gnu.so.p/meson-generated_.._nvme_wrap.c.o -MF libnvme/libnvme/_nvme.cpython-314-x86_64-linux-gnu.so.p/meson-generated_.._nvme_wrap.c.o.d -o libnvme/libnvme/_nvme.cpython-314-x86_64-linux-gnu.so.p/meson-generated_.._nvme_wrap.c.o -c libnvme/libnvme/nvme_wrap.c
In file included from /usr/include/python3.14/Python.h:71,
                 from libnvme/libnvme/nvme_wrap.c:203:
/usr/include/python3.14/pyport.h:660:35: error: missing ‘)’ after ‘__has_attribute’
  660 | #if _Py__has_attribute(fallthrough) && (!defined(__clang__) || \
      |                                   ^
./project-config.h:86:35: error: missing binary operator before token ‘(’
   86 | #define fallthrough __attribute__((__fallthrough__))
      |                                   ^
/usr/include/python3.14/pyport.h:25:49: note: in definition of macro ‘_Py__has_attribute’
   25 | #  define _Py__has_attribute(x) __has_attribute(x)
      |                                                 ^
/usr/include/python3.14/pyport.h:660:24: note: in expansion of macro ‘fallthrough’
  660 | #if _Py__has_attribute(fallthrough) && (!defined(__clang__) || \
      |                        ^~~~~~~~~~~
  • installing with user rights fails
› meson install -C .build-incl-lib
ninja: Entering directory `/home/wagi/work/nvme-cli/.build-incl-lib'
ninja: no work to do.
Installing libnvme/src/libnvme.so.3.0.0 to /tmp/test/lib64
Installing nvme to /tmp/test/sbin
Installing /home/wagi/work/nvme-cli/libnvme/src/libnvme.h to /tmp/test/include
'/tmp/test/include/libnvme.h': Unable to set owner 0 and group 0: Operation not permitted, ignoring...
Installing /home/wagi/work/nvme-cli/libnvme/src/libnvme-mi.h to /tmp/test/include
  • nvme seems not to be installed
  • the script/release.sh script needs also be updated, though this is not necessary for this stuff. Just one thing to remember.
  • we need a way to configure/build the library independent of nvme-cli. This means also we should be able to build nvme-cli without library. In this case the dependency for the library (e.g. OpenSSL) should not be required. This is an important feature the distros ask for.

To your questions:

  • project-config.h: if we rename it, I'd say we should go with 'nvme-config.h'. Though we could just rename the plugin config.h. In the end this plugin needs to intergrate against the core code. :)
  • in order to build with muon, you can use the script/build.sh script. This is also script which the CI build is using. So it the different configuration are supposed to work (debug/release, gcc/clang,...)
› ./scripts/build.sh -m muon
~/work/nvme-cli/.build-tools/muon ~/work/nvme-cli

@martin-belanger
Copy link
Author

Good feedback. I'll start working on it. Thanks.

@martin-belanger martin-belanger force-pushed the do-not-subproject-libnvme branch from 363f4d9 to af2cbd6 Compare December 1, 2025 19:28
@martin-belanger
Copy link
Author

martin-belanger commented Dec 1, 2025

I fixed the following:

  • the python code doesn't build for me: The Python code should build now. The problem was that we define the macro fallthrough, which is clashing with the same macro definition in /usr/include/.../Python.h. This only happens with Python 3.14 since previous versions did not define fallthrough. This explains why this was building for me with Python 3.12 and not for you.
  • build with muon: Fixed.
  • project-config.h: I renamed project-config.h to nvme-config.h. I prefer using a nvme-specific file name (i.e. nvme-config.h) rather than a generic name (i.e. config.h) to avoid possible conflicts with 3rd party code in the future (better safe than sorry...)
  • installing with user rights fails. I fixed it by removing the UID=0 and GID=0 from the install_mode. In other words, install_mode: 'rw-r--r--' instead of install_mode: '[rw-r--r--', 0, 0]. This way, files get installed with the current user/group instead of forcing user/group to root.

I have a few questions regarding the following items:

  • the script/release.sh script needs also be updated: I'm not familiar with what this script does. If you don't mind, I will leave this one to you. 😉
  • nvme seems not to be installed. Is this related to installing with user rights under /tmp/test/, or do you mean that nvme just does not get installed at all regardless of whether we install under /usr/sbin or elsewhere.
  • we need a way to configure/build the library independent of nvme-cli. That's a good point. I'll need some time for this one...

@martin-belanger martin-belanger force-pushed the do-not-subproject-libnvme branch 2 times, most recently from 6568ff0 to 8b3fd95 Compare December 2, 2025 11:14
@martin-belanger
Copy link
Author

@igaw - Added 2 new meson options: -Dnvme=[auto,enabled,disabled] and -Dlibnvme=[auto,enabled,disabled]. Both default to enabled. These can be used to control whether to build the nvme executable and/or the libnvme.so library.

Regarding nvme not being installed, I was not able to reproduce the problem you observed. Please retest and let me know if you still see the issue.

@martin-belanger martin-belanger force-pushed the do-not-subproject-libnvme branch from 8b3fd95 to a651347 Compare December 2, 2025 11:21
Change the build flow so that libnvme is no longer defined as a Meson
subproject of nvme-cli. Instead, include libnvme's meson.build directly
from the top-level meson.build.

This allows sharing a common ccan directory between nvme-cli and libnvme,
and generates a single configuration header, project-config.h (formerly
config.h), used by all components. The old file name conflicted with
plugins/solidigm/solidigm-telemetry/config.h, hence the rename.

Also reformat all meson.build files to fix indentation inconsistencies and
other cosmetic discrepancies.

Signed-off-by: Martin Belanger <[email protected]>
@martin-belanger martin-belanger force-pushed the do-not-subproject-libnvme branch from a651347 to de94af4 Compare December 3, 2025 11:55
@martin-belanger
Copy link
Author

@igaw - Regarding your earlier comment:

we need a way to configure/build the library independent of nvme-cli. This means also we should be able to build nvme-cli without library. In this case the dependency for the library (e.g. OpenSSL) should not be required. This is an important feature the distros ask for.

Building the library (libnvme.so) without nvme-cli (nvme) is not a problem. However, building nvme-cli w/o the library present will result in a whole bunch of missing symbols (see example below). Maybe there's a gcc option to allow for that, but I'm not aware of it.

/usr/bin/ld: fabrics.c:(.text+0x3239): undefined reference to `nvme_ctrl_find'
/usr/bin/ld: fabrics.c:(.text+0x3281): undefined reference to `nvme_subsystem_next_ctrl'
/usr/bin/ld: fabrics.c:(.text+0x3319): undefined reference to `nvme_ctrl_get_subsysnqn'
/usr/bin/ld: fabrics.c:(.text+0x3326): undefined reference to `nvme_ctrl_is_persistent'
/usr/bin/ld: fabrics.c:(.text+0x3454): undefined reference to `nvme_free_ctrl'
/usr/bin/ld: fabrics.c:(.text+0x34e2): undefined reference to `nvme_host_is_pdc_enabled'
/usr/bin/ld: fabrics.c:(.text+0x34f2): undefined reference to `nvme_disconnect_ctrl'
etc...

Please let me know how you want to proceed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants