DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] config: sort Meson options by categories
@ 2021-10-25 16:17 Thomas Monjalon
  2021-10-25 16:23 ` Thomas Monjalon
  2021-11-24 12:56 ` [PATCH v2] " Thomas Monjalon
  0 siblings, 2 replies; 4+ messages in thread
From: Thomas Monjalon @ 2021-10-25 16:17 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH] config: sort Meson options by categories
  2021-10-25 16:17 [dpdk-dev] [PATCH] config: sort Meson options by categories Thomas Monjalon
@ 2021-10-25 16:23 ` Thomas Monjalon
  2021-11-24 12:56 ` [PATCH v2] " Thomas Monjalon
  1 sibling, 0 replies; 4+ messages in thread
From: Thomas Monjalon @ 2021-10-25 16:23 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev

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.




^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2] config: sort Meson options by categories
  2021-10-25 16:17 [dpdk-dev] [PATCH] config: sort Meson options by categories Thomas Monjalon
  2021-10-25 16:23 ` Thomas Monjalon
@ 2021-11-24 12:56 ` Thomas Monjalon
  2021-11-24 14:29   ` Bruce Richardson
  1 sibling, 1 reply; 4+ messages in thread
From: Thomas Monjalon @ 2021-11-24 12:56 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] config: sort Meson options by categories
  2021-11-24 12:56 ` [PATCH v2] " Thomas Monjalon
@ 2021-11-24 14:29   ` Bruce Richardson
  0 siblings, 0 replies; 4+ messages in thread
From: Bruce Richardson @ 2021-11-24 14:29 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-11-24 14:29 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-25 16:17 [dpdk-dev] [PATCH] config: sort Meson options by categories Thomas Monjalon
2021-10-25 16:23 ` Thomas Monjalon
2021-11-24 12:56 ` [PATCH v2] " Thomas Monjalon
2021-11-24 14:29   ` Bruce Richardson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).