[dpdk-dev] [PATCH] config: sort Meson options by categories

[dpdk-dev] [PATCH] config: sort Meson options by categories

[Thomas Monjalon]

Options used to be sorted alphabetically.
It looks easier to read when major options are first,
then path tuning, libs options, and drivers options.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 meson_options.txt | 75 ++++++++++++++++++++++++-----------------------
 1 file changed, 39 insertions(+), 36 deletions(-)

diff --git a/meson_options.txt b/meson_options.txt
index 7c220ad68d..281719c794 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -1,50 +1,53 @@
-# Please keep these options sorted alphabetically.
-
-option('check_includes', type: 'boolean', value: false, description:
-       'build "chkincs" to verify each header file can compile alone')
-option('cpu_instruction_set', type: 'string', value: 'auto',
-	description: 'Set the target machine ISA (instruction set architecture). Will be set according to the platform option by default.')
-option('developer_mode', type: 'feature', description:
-       'turn on additional build checks relevant for DPDK developers')
-option('disable_drivers', type: 'string', value: '', description:
-       'Comma-separated list of drivers to explicitly disable.')
-option('disable_libs', type: 'string', value: '', description:
-       'Comma-separated list of libraries to explicitly disable. [NOTE: not all libs can be disabled]')
+# general compilation tuning
+option('platform', type: 'string', value: 'native', description:
+       'Platform to build, either "native", "generic" or a SoC. Please refer to the Linux build guide for more information.')
+option('cpu_instruction_set', type: 'string', value: 'auto', description:
+       'Set the target machine ISA (instruction set architecture). Will be set according to the platform option by default.')
+option('machine', type: 'string', value: 'auto', description:
+       'Alias of cpu_instruction_set.')
+option('include_subdir_arch', type: 'string', value: '', description:
+       'Subdirectory where to install arch-dependent headers.')
 option('drivers_install_subdir', type: 'string', value: 'dpdk/pmds-<VERSION>', description:
        'Subdirectory of libdir where to install PMDs. Defaults to using a versioned subdirectory.')
-option('enable_docs', type: 'boolean', value: false, description:
-       'build documentation')
+option('disable_libs', type: 'string', value: '', description:
+       'Comma-separated list of libraries to explicitly disable. [NOTE: not all libs can be disabled]')
+option('disable_drivers', type: 'string', value: '', description:
+       'Comma-separated list of drivers to explicitly disable.')
 option('enable_drivers', type: 'string', value: '', description:
        'Comma-separated list of drivers to build. If unspecified, build all drivers.')
 option('enable_driver_sdk', type: 'boolean', value: false, description:
        'Install headers to build drivers.')
 option('enable_kmods', type: 'boolean', value: false, description:
-       'build kernel modules')
-option('examples', type: 'string', value: '', description:
-       'Comma-separated list of examples to build by default')
-option('flexran_sdk', type: 'string', value: '', description:
-       'Path to FlexRAN SDK optional Libraries for BBDEV device')
-option('ibverbs_link', type: 'combo', choices : ['static', 'shared', 'dlopen'], value: 'shared', description:
-       'Linkage method (static/shared/dlopen) for Mellanox PMDs with ibverbs dependencies.')
-option('include_subdir_arch', type: 'string', value: '', description:
-       'subdirectory where to install arch-dependent headers')
+       'Build kernel modules.')
 option('kernel_dir', type: 'string', value: '', description:
        'Path to the kernel for building kernel modules. Headers must be in $kernel_dir or $kernel_dir/build. Modules will be installed in /lib/modules.')
-option('machine', type: 'string', value: 'auto', description:
-       'Alias of cpu_instruction_set.')
-option('max_ethports', type: 'integer', value: 32, description:
-       'maximum number of Ethernet devices')
-option('max_lcores', type: 'string', value: 'default', description:
-       'Set maximum number of cores/threads supported by EAL; "default" is different per-arch, "detect" detects the number of cores on the build machine.')
+option('enable_docs', type: 'boolean', value: false, description:
+       'Build documentation.')
+option('examples', type: 'string', value: '', description:
+       'Comma-separated list of examples to build by default.')
+option('tests', type: 'boolean', value: true, description:
+       'Build unit tests.')
+option('developer_mode', type: 'feature', description:
+       'Turn on additional build checks relevant for DPDK developers.')
+option('check_includes', type: 'boolean', value: false, description:
+       'Build chkincs to verify each header file can compile alone.')
+
+# library-specific options
 option('max_numa_nodes', type: 'string', value: 'default', description:
        'Set the highest NUMA node supported by EAL; "default" is different per-arch, "detect" detects the highest NUMA node on the build machine.')
+option('max_lcores', type: 'string', value: 'default', description:
+       'Set maximum number of cores/threads supported by EAL; "default" is different per-arch, "detect" detects the number of cores on the build machine.')
 option('mbuf_refcnt_atomic', type: 'boolean', value: true, description:
        'Atomically access the mbuf refcnt.')
-option('platform', type: 'string', value: 'native', description:
-       'Platform to build, either "native", "generic" or a SoC. Please refer to the Linux build guide for more information.')
-option('enable_trace_fp', type: 'boolean', value: false, description:
-       'enable fast path trace points.')
-option('tests', type: 'boolean', value: true, description:
-       'build unit tests')
+option('max_ethports', type: 'integer', value: 32, description:
+       'Maximum number of Ethernet devices.')
 option('use_hpet', type: 'boolean', value: false, description:
-       'use HPET timer in EAL')
+       'Use HPET timer in EAL.')
+option('enable_trace_fp', type: 'boolean', value: false, description:
+       'Enable fast path trace points.')
+
+# driver-specific options
+option('flexran_sdk', type: 'string', value: '', description:
+       'Path to FlexRAN SDK optional libraries for bbdev driver.')
+option('ibverbs_link', type: 'combo', choices : ['static', 'shared', 'dlopen'], value: 'shared', description:
+       'Linkage method (static/shared/dlopen) for Mellanox PMDs with ibverbs dependencies.')
-- 
2.33.0

Re: [dpdk-dev] [PATCH] config: sort Meson options by categories

[Thomas Monjalon]

25/10/2021 18:17, Thomas Monjalon:
> Options used to be sorted alphabetically.
> It looks easier to read when major options are first,
> then path tuning, libs options, and drivers options.

Even better, we could insert a blank line between each option.

[PATCH v2] config: sort Meson options by categories

[Thomas Monjalon]

Options used to be sorted alphabetically.
It looks easier to read when major options are first,
then path tuning, libs options, and drivers options.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
v2: add blank lines for readability
---
 meson_options.txt | 99 ++++++++++++++++++++++++++++++-----------------
 1 file changed, 63 insertions(+), 36 deletions(-)

diff --git a/meson_options.txt b/meson_options.txt
index 7c220ad68d..b43273a27d 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -1,50 +1,77 @@
-# Please keep these options sorted alphabetically.
-
-option('check_includes', type: 'boolean', value: false, description:
-       'build "chkincs" to verify each header file can compile alone')
-option('cpu_instruction_set', type: 'string', value: 'auto',
-	description: 'Set the target machine ISA (instruction set architecture). Will be set according to the platform option by default.')
-option('developer_mode', type: 'feature', description:
-       'turn on additional build checks relevant for DPDK developers')
-option('disable_drivers', type: 'string', value: '', description:
-       'Comma-separated list of drivers to explicitly disable.')
-option('disable_libs', type: 'string', value: '', description:
-       'Comma-separated list of libraries to explicitly disable. [NOTE: not all libs can be disabled]')
+# general compilation tuning
+
+option('platform', type: 'string', value: 'native', description:
+       'Platform to build, either "native", "generic" or a SoC. Please refer to the Linux build guide for more information.')
+
+option('cpu_instruction_set', type: 'string', value: 'auto', description:
+       'Set the target machine ISA (instruction set architecture). Will be set according to the platform option by default.')
+
+option('machine', type: 'string', value: 'auto', description:
+       'Alias of cpu_instruction_set.')
+
+option('include_subdir_arch', type: 'string', value: '', description:
+       'Subdirectory where to install arch-dependent headers.')
+
 option('drivers_install_subdir', type: 'string', value: 'dpdk/pmds-<VERSION>', description:
        'Subdirectory of libdir where to install PMDs. Defaults to using a versioned subdirectory.')
-option('enable_docs', type: 'boolean', value: false, description:
-       'build documentation')
+
+option('disable_libs', type: 'string', value: '', description:
+       'Comma-separated list of libraries to explicitly disable. [NOTE: not all libs can be disabled]')
+
+option('disable_drivers', type: 'string', value: '', description:
+       'Comma-separated list of drivers to explicitly disable.')
+
 option('enable_drivers', type: 'string', value: '', description:
        'Comma-separated list of drivers to build. If unspecified, build all drivers.')
+
 option('enable_driver_sdk', type: 'boolean', value: false, description:
        'Install headers to build drivers.')
+
 option('enable_kmods', type: 'boolean', value: false, description:
-       'build kernel modules')
-option('examples', type: 'string', value: '', description:
-       'Comma-separated list of examples to build by default')
-option('flexran_sdk', type: 'string', value: '', description:
-       'Path to FlexRAN SDK optional Libraries for BBDEV device')
-option('ibverbs_link', type: 'combo', choices : ['static', 'shared', 'dlopen'], value: 'shared', description:
-       'Linkage method (static/shared/dlopen) for Mellanox PMDs with ibverbs dependencies.')
-option('include_subdir_arch', type: 'string', value: '', description:
-       'subdirectory where to install arch-dependent headers')
+       'Build kernel modules.')
+
 option('kernel_dir', type: 'string', value: '', description:
        'Path to the kernel for building kernel modules. Headers must be in $kernel_dir or $kernel_dir/build. Modules will be installed in /lib/modules.')
-option('machine', type: 'string', value: 'auto', description:
-       'Alias of cpu_instruction_set.')
-option('max_ethports', type: 'integer', value: 32, description:
-       'maximum number of Ethernet devices')
-option('max_lcores', type: 'string', value: 'default', description:
-       'Set maximum number of cores/threads supported by EAL; "default" is different per-arch, "detect" detects the number of cores on the build machine.')
+
+option('enable_docs', type: 'boolean', value: false, description:
+       'Build documentation.')
+
+option('examples', type: 'string', value: '', description:
+       'Comma-separated list of examples to build by default.')
+
+option('tests', type: 'boolean', value: true, description:
+       'Build unit tests.')
+
+option('developer_mode', type: 'feature', description:
+       'Turn on additional build checks relevant for DPDK developers.')
+
+option('check_includes', type: 'boolean', value: false, description:
+       'Build chkincs to verify each header file can compile alone.')
+
+# library-specific options
+
 option('max_numa_nodes', type: 'string', value: 'default', description:
        'Set the highest NUMA node supported by EAL; "default" is different per-arch, "detect" detects the highest NUMA node on the build machine.')
+
+option('max_lcores', type: 'string', value: 'default', description:
+       'Set maximum number of cores/threads supported by EAL; "default" is different per-arch, "detect" detects the number of cores on the build machine.')
+
 option('mbuf_refcnt_atomic', type: 'boolean', value: true, description:
        'Atomically access the mbuf refcnt.')
-option('platform', type: 'string', value: 'native', description:
-       'Platform to build, either "native", "generic" or a SoC. Please refer to the Linux build guide for more information.')
-option('enable_trace_fp', type: 'boolean', value: false, description:
-       'enable fast path trace points.')
-option('tests', type: 'boolean', value: true, description:
-       'build unit tests')
+
+option('max_ethports', type: 'integer', value: 32, description:
+       'Maximum number of Ethernet devices.')
+
 option('use_hpet', type: 'boolean', value: false, description:
-       'use HPET timer in EAL')
+       'Use HPET timer in EAL.')
+
+option('enable_trace_fp', type: 'boolean', value: false, description:
+       'Enable fast path trace points.')
+
+# driver-specific options
+
+option('flexran_sdk', type: 'string', value: '', description:
+       'Path to FlexRAN SDK optional libraries for bbdev driver.')
+
+option('ibverbs_link', type: 'combo', choices : ['static', 'shared', 'dlopen'], value: 'shared', description:
+       'Linkage method (static/shared/dlopen) for Mellanox PMDs with ibverbs dependencies.')
-- 
2.33.0

Re: [PATCH v2] config: sort Meson options by categories

[Bruce Richardson]

On Wed, Nov 24, 2021 at 01:56:42PM +0100, Thomas Monjalon wrote:
> Options used to be sorted alphabetically.
> It looks easier to read when major options are first,
> then path tuning, libs options, and drivers options.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
> v2: add blank lines for readability
> ---
>  meson_options.txt | 99 ++++++++++++++++++++++++++++++-----------------
>  1 file changed, 63 insertions(+), 36 deletions(-)

While I like the idea of being able to group items, in the absense of meson
support for displaying them as groups, I worry about it making it harder to
find and use options. It also has some ambiguity as to what should be in
what category: e.g. is max_lcores a global option or a library-specific one
for EAL. I would have considered it global, but you have it here as library
specific - which when I think about it is probably more correct, just not
what I thought of instinctively.

On the other hand, I agree that having them sorted alphabetically is not
brilliant either - I think I originally chose to order them alphabetically
so as to avoid arguments as to where exactly each option should go, since
it makes things unambiguous.

Overall, I'm fairly ambivilant about this patch, but would appreciate more
input from others, and I'm happy with whatever the majority decision is.

/Bruce