Re: [dpdk-dev] [PATCH v3] compat: merge compat library into EAL

[Neil Horman <nhorman@tuxdriver.com>]

On Wed, Feb 06, 2019 at 11:01:30AM +0000, Bruce Richardson wrote:
> Since compat library is only a single header, we can easily move it into
> the EAL common headers instead of tracking it separately. The downside of
> this is that it becomes a little more difficult to have any libs that are
> built before EAL depend on it. Thankfully, this is not a major problem as
> the only library which uses rte_compat.h and is built before EAL (kvargs)
> already has the path to the compat.h header file explicitly called out as
> an include path.
> 
> However, to ensure that we don't hit problems later with this, we can add
> EAL common headers folder to the global include list in the meson build
> which means that all common headers can be safely used by all libraries, no
> matter what their build order.
> 
This assumes that the compat lib will always just be a header though, no?  Will
this work in the event that someone wants to add some compatibility code that
requires its own C compilation unit?

> As a side-effect, this patch also fixes an issue with building on BSD using
> meson, due to compat lib no longer needing to be listed as a dependency.
> 
Can you elaborate here a bit please?  listing a lib as a dependency seems like a
fundamental function of a build system, was there a bug with meson in this
capacity?

Thanks
Neil

> Fixes: a8499f65a1d1 ("log: add missing experimental tag")
> 
> CC: stable@dpdk.org
> CC: Neil Horman <nhorman@tuxdriver.com>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> Reviewed-by: David Marchand <david.marchand@redhat.com>
> Tested-by: David Marchand <david.marchand@redhat.com>
> Tested-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
> V3: Add stable on CC, and remove extra word in commit log.
> V2: Clean up a few missed references to the compat library in our
>     documentation and MAINTAINERS file.
>     Added in fixes tag, as this patch should also fix build issues
>     with BSD.
> ---
>  MAINTAINERS                                         |  1 -
>  doc/api/doxy-api.conf.in                            |  1 -
>  doc/guides/contributing/documentation.rst           |  1 -
>  doc/guides/contributing/versioning.rst              |  2 +-
>  lib/Makefile                                        |  2 --
>  lib/librte_cmdline/meson.build                      |  1 -
>  lib/librte_compat/Makefile                          | 13 -------------
>  lib/librte_compat/meson.build                       |  8 --------
>  lib/librte_eal/common/Makefile                      |  2 +-
>  .../common/include}/rte_compat.h                    |  0
>  lib/librte_eal/common/meson.build                   |  1 +
>  lib/librte_eal/linuxapp/eal/meson.build             |  2 +-
>  lib/librte_eal/meson.build                          |  1 -
>  lib/librte_kvargs/meson.build                       |  3 ---
>  lib/meson.build                                     |  2 +-
>  meson.build                                         |  2 +-
>  16 files changed, 6 insertions(+), 36 deletions(-)
>  delete mode 100644 lib/librte_compat/Makefile
>  delete mode 100644 lib/librte_compat/meson.build
>  rename lib/{librte_compat => librte_eal/common/include}/rte_compat.h (100%)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 835d8a201..0707caea5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -121,7 +121,6 @@ F: buildtools/symlink-drivers-solibs.sh
>  
>  ABI versioning
>  M: Neil Horman <nhorman@tuxdriver.com>
> -F: lib/librte_compat/
>  F: doc/guides/rel_notes/deprecation.rst
>  F: devtools/validate-abi.sh
>  F: devtools/check-symbol-change.sh
> diff --git a/doc/api/doxy-api.conf.in b/doc/api/doxy-api.conf.in
> index bef9320c0..a365e669b 100644
> --- a/doc/api/doxy-api.conf.in
> +++ b/doc/api/doxy-api.conf.in
> @@ -23,7 +23,6 @@ INPUT                   = @TOPDIR@/doc/api/doxy-api-index.md \
>                            @TOPDIR@/lib/librte_bpf \
>                            @TOPDIR@/lib/librte_cfgfile \
>                            @TOPDIR@/lib/librte_cmdline \
> -                          @TOPDIR@/lib/librte_compat \
>                            @TOPDIR@/lib/librte_compressdev \
>                            @TOPDIR@/lib/librte_cryptodev \
>                            @TOPDIR@/lib/librte_distributor \
> diff --git a/doc/guides/contributing/documentation.rst b/doc/guides/contributing/documentation.rst
> index c72280a29..baf0921fb 100644
> --- a/doc/guides/contributing/documentation.rst
> +++ b/doc/guides/contributing/documentation.rst
> @@ -22,7 +22,6 @@ The main directories that contain files related to documentation are shown below
>     |-- librte_acl
>     |-- librte_cfgfile
>     |-- librte_cmdline
> -   |-- librte_compat
>     |-- librte_eal
>     |   |-- ...
>     ...
> diff --git a/doc/guides/contributing/versioning.rst b/doc/guides/contributing/versioning.rst
> index 01b36247e..18b031998 100644
> --- a/doc/guides/contributing/versioning.rst
> +++ b/doc/guides/contributing/versioning.rst
> @@ -167,7 +167,7 @@ functionality or behavior. When that occurs, it is desirable to allow for
>  backward compatibility for a time with older binaries that are dynamically
>  linked to the DPDK.
>  
> -To support backward compatibility the ``lib/librte_compat/rte_compat.h``
> +To support backward compatibility the ``rte_compat.h``
>  header file provides macros to use when updating exported functions. These
>  macros are used in conjunction with the ``rte_<library>_version.map`` file for
>  a given library to allow multiple versions of a symbol to exist in a shared
> diff --git a/lib/Makefile b/lib/Makefile
> index d6239d27c..ffbfd0d94 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -3,9 +3,7 @@
>  
>  include $(RTE_SDK)/mk/rte.vars.mk
>  
> -DIRS-y += librte_compat
>  DIRS-$(CONFIG_RTE_LIBRTE_KVARGS) += librte_kvargs
> -DEPDIRS-librte_kvargs := librte_compat
>  DIRS-$(CONFIG_RTE_LIBRTE_EAL) += librte_eal
>  DEPDIRS-librte_eal := librte_kvargs
>  DIRS-$(CONFIG_RTE_LIBRTE_PCI) += librte_pci
> diff --git a/lib/librte_cmdline/meson.build b/lib/librte_cmdline/meson.build
> index 30498906c..0fa61385f 100644
> --- a/lib/librte_cmdline/meson.build
> +++ b/lib/librte_cmdline/meson.build
> @@ -3,7 +3,6 @@
>  
>  # This library is processed before EAL
>  includes = [global_inc]
> -includes += include_directories('../librte_eal/common/include')
>  
>  version = 2
>  sources = files('cmdline.c',
> diff --git a/lib/librte_compat/Makefile b/lib/librte_compat/Makefile
> deleted file mode 100644
> index 61089fe77..000000000
> --- a/lib/librte_compat/Makefile
> +++ /dev/null
> @@ -1,13 +0,0 @@
> -# SPDX-License-Identifier: BSD-3-Clause
> -# Copyright(c) 2013 Neil Horman <nhorman@tuxdriver.com>
> -# All rights reserved.
> -
> -include $(RTE_SDK)/mk/rte.vars.mk
> -
> -
> -LIBABIVER := 1
> -
> -# install includes
> -SYMLINK-y-include := rte_compat.h
> -
> -include $(RTE_SDK)/mk/rte.install.mk
> diff --git a/lib/librte_compat/meson.build b/lib/librte_compat/meson.build
> deleted file mode 100644
> index 82c7eea55..000000000
> --- a/lib/librte_compat/meson.build
> +++ /dev/null
> @@ -1,8 +0,0 @@
> -# SPDX-License-Identifier: BSD-3-Clause
> -# Copyright(c) 2017 Intel Corporation
> -
> -
> -install_headers('rte_compat.h')
> -
> -set_variable('dep_rte_compat',
> -	declare_dependency(include_directories: include_directories('.')))
> diff --git a/lib/librte_eal/common/Makefile b/lib/librte_eal/common/Makefile
> index 87d8c455d..c487201b3 100644
> --- a/lib/librte_eal/common/Makefile
> +++ b/lib/librte_eal/common/Makefile
> @@ -3,7 +3,7 @@
>  
>  include $(RTE_SDK)/mk/rte.vars.mk
>  
> -INC := rte_branch_prediction.h rte_common.h
> +INC := rte_branch_prediction.h rte_common.h rte_compat.h
>  INC += rte_debug.h rte_eal.h rte_eal_interrupts.h
>  INC += rte_errno.h rte_launch.h rte_lcore.h
>  INC += rte_log.h rte_memory.h rte_memzone.h
> diff --git a/lib/librte_compat/rte_compat.h b/lib/librte_eal/common/include/rte_compat.h
> similarity index 100%
> rename from lib/librte_compat/rte_compat.h
> rename to lib/librte_eal/common/include/rte_compat.h
> diff --git a/lib/librte_eal/common/meson.build b/lib/librte_eal/common/meson.build
> index 2a10d57d8..5ecae0b1f 100644
> --- a/lib/librte_eal/common/meson.build
> +++ b/lib/librte_eal/common/meson.build
> @@ -53,6 +53,7 @@ common_headers = files(
>  	'include/rte_bitmap.h',
>  	'include/rte_class.h',
>  	'include/rte_common.h',
> +	'include/rte_compat.h',
>  	'include/rte_debug.h',
>  	'include/rte_devargs.h',
>  	'include/rte_dev.h',
> diff --git a/lib/librte_eal/linuxapp/eal/meson.build b/lib/librte_eal/linuxapp/eal/meson.build
> index 6e31c2aaa..7e68b2c0d 100644
> --- a/lib/librte_eal/linuxapp/eal/meson.build
> +++ b/lib/librte_eal/linuxapp/eal/meson.build
> @@ -1,7 +1,7 @@
>  # SPDX-License-Identifier: BSD-3-Clause
>  # Copyright(c) 2017 Intel Corporation
>  
> -eal_inc += include_directories('include', '../../../librte_compat')
> +eal_inc += include_directories('include')
>  install_subdir('include/exec-env', install_dir: get_option('includedir'))
>  
>  env_objs = []
> diff --git a/lib/librte_eal/meson.build b/lib/librte_eal/meson.build
> index a18f3a826..64d857a4a 100644
> --- a/lib/librte_eal/meson.build
> +++ b/lib/librte_eal/meson.build
> @@ -23,7 +23,6 @@ endif
>  
>  version = 9  # the version of the EAL API
>  allow_experimental_apis = true
> -deps += 'compat'
>  deps += 'kvargs'
>  sources = common_sources + env_sources
>  objs = common_objs + env_objs
> diff --git a/lib/librte_kvargs/meson.build b/lib/librte_kvargs/meson.build
> index acd3e5432..ecaedf5a5 100644
> --- a/lib/librte_kvargs/meson.build
> +++ b/lib/librte_kvargs/meson.build
> @@ -2,10 +2,7 @@
>  # Copyright(c) 2017 Intel Corporation
>  
>  includes = [global_inc]
> -includes += include_directories('../librte_eal/common/include')
>  
>  version = 1
>  sources = files('rte_kvargs.c')
>  headers = files('rte_kvargs.h')
> -
> -deps += 'compat'
> diff --git a/lib/meson.build b/lib/meson.build
> index e8b40f546..edcccdcb6 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -8,7 +8,7 @@
>  # sometimes skip deps that would be implied by others, e.g. if mempool is
>  # given as a dep, no need to mention ring. This is especially true for the
>  # core libs which are widely reused, so their deps are kept to a minimum.
> -libraries = [ 'compat', # just a header, used for versioning
> +libraries = [
>  	'cmdline', # ethdev depends on cmdline for parsing functions
>  	'kvargs', # eal depends on kvargs
>  	'eal', 'ring', 'mempool', 'mbuf', 'net', 'meter', 'ethdev', 'pci', # core
> diff --git a/meson.build b/meson.build
> index 21d428fa1..2c83a5452 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -32,7 +32,7 @@ eal_pmd_path = join_paths(get_option('prefix'), driver_install_path)
>  # configure the build, and make sure configs here and in config folder are
>  # able to be included in any file. We also store a global array of include dirs
>  # for passing to pmdinfogen scripts
> -global_inc = include_directories('.', 'config')
> +global_inc = include_directories('.', 'config', 'lib/librte_eal/common/include')
>  subdir('config')
>  
>  # build libs and drivers
> -- 
> 2.20.1
> 
>