diff mbox series

[v2] config: sort Meson options by categories

Message ID 20211124125642.1322551-1-thomas@monjalon.net (mailing list archive)
State Rejected, archived
Delegated to: Thomas Monjalon
Headers
Series [v2] config: sort Meson options by categories |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/github-robot: build success github build: passed
ci/intel-Testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS

Commit Message

Thomas Monjalon Nov. 24, 2021, 12:56 p.m. UTC 
  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(-)

Comments

Bruce Richardson Nov. 24, 2021, 2:29 p.m. UTC | #1 
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
diff mbox series

Patch

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.')