Re: [dpdk-dev] [PATCH 2/4] build: use version number from config file

[David Marchand <david.marchand@redhat.com>]

On Thu, Mar 7, 2019 at 1:37 PM David Marchand <david.marchand@redhat.com>
wrote:

>
>
> On Thu, Mar 7, 2019 at 12:55 PM Bruce Richardson <
> bruce.richardson@intel.com> wrote:
>
>> Since we have the version number in a separate file at the root level,
>> we should not need to duplicate this in rte_version.h too. Best
>> approach here is to move the macros for specifying the year/month/etc.
>> parts from the version header file to the build config file - leaving
>> the other utility macros for e.g. printing the version string, where they
>> are.
>>
>> For make this is done by having a little bit of awk parse the version
>> file and pass the results through to the preprocessor for the config
>> generation stage.
>>
>> For meson, this is done by parsing the version and adding it to the
>> standard dpdk_conf object.
>>
>> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
>> ---
>>  DPDK_VERSION                                |  2 +-
>>  config/common_base                          | 14 +++++++++
>>  config/rte_config.h                         |  5 ++++
>>  lib/librte_eal/common/include/rte_version.h | 32 ---------------------
>>  meson.build                                 | 14 +++++++++
>>  mk/rte.sdkconfig.mk                         |  2 ++
>>  6 files changed, 36 insertions(+), 33 deletions(-)
>>
>> diff --git a/DPDK_VERSION b/DPDK_VERSION
>> index c4a4cd00b..e013b54dd 100644
>> --- a/DPDK_VERSION
>> +++ b/DPDK_VERSION
>> @@ -1 +1 @@
>> -19.05.0-rc0
>> +19.05.1
>>
>
> Might be a bit early for the stable while the 19.05 is still being cooked
> :-)
>
>
> diff --git a/config/common_base b/config/common_base
>> index 0b09a9348..6292bc4af 100644
>> --- a/config/common_base
>> +++ b/config/common_base
>> @@ -1,6 +1,20 @@
>>  # SPDX-License-Identifier: BSD-3-Clause
>>  # Copyright(c) 2010-2017 Intel Corporation
>>
>> +#
>> +# String that appears before the version number
>> +#
>> +CONFIG_RTE_VER_PREFIX="DPDK"
>> +
>> +#
>> +# Version information completed when this file is processed for a build
>> +#
>> +CONFIG_RTE_VER_YEAR=__YEAR
>> +CONFIG_RTE_VER_MONTH=__MONTH
>> +CONFIG_RTE_VER_MINOR=__MINOR
>> +CONFIG_RTE_VER_SUFFIX=__SUFFIX
>> +CONFIG_RTE_VER_RELEASE=__RELEASE
>> +
>>
>
> ?
> I think you broke make support.
>
> == Build lib/librte_eal/linuxapp/eal
>   CC eal.o
> In file included from
> /home/dmarchan/git/pub/dpdk/lib/librte_eal/linuxapp/eal/eal.c:49:0:
> /home/dmarchan/git/pub/dpdk/master/include/rte_version.h: In function
> ‘rte_version’:
> /home/dmarchan/git/pub/dpdk/master/include/rte_version.h:47:13: error:
> ‘RTE_VER_SUFFIX’ undeclared (first use in this function)
>   if (strlen(RTE_VER_SUFFIX) == 0)
>              ^
> /home/dmarchan/git/pub/dpdk/master/include/rte_version.h:47:13: note: each
> undeclared identifier is reported only once for each function it appears in
> /home/dmarchan/git/pub/dpdk/master/include/rte_version.h:49:4: error:
> ‘RTE_VER_PREFIX’ undeclared (first use in this function)
>     RTE_VER_PREFIX,
>     ^
> /home/dmarchan/git/pub/dpdk/master/include/rte_version.h:50:4: error:
> ‘RTE_VER_YEAR’ undeclared (first use in this function)
>     RTE_VER_YEAR,
>     ^
> /home/dmarchan/git/pub/dpdk/master/include/rte_version.h:51:4: error:
> ‘RTE_VER_MONTH’ undeclared (first use in this function)
>     RTE_VER_MONTH,
>     ^
> /home/dmarchan/git/pub/dpdk/master/include/rte_version.h:52:4: error:
> ‘RTE_VER_MINOR’ undeclared (first use in this function)
>     RTE_VER_MINOR);
>     ^
> /home/dmarchan/git/pub/dpdk/master/include/rte_version.h:60:4: error:
> ‘RTE_VER_RELEASE’ undeclared (first use in this function)
>     RTE_VER_RELEASE);
>     ^
> make[5]: *** [eal.o] Error 1
> make[4]: *** [eal] Error 2
> make[3]: *** [linuxapp] Error 2
> make[2]: *** [librte_eal] Error 2
> make[1]: *** [lib] Error 2
> make: *** [all] Error 2
>
>
>
>  #
>>  # define executive environment
>>  # RTE_EXEC_ENV values are the directories in mk/exec-env/
>> diff --git a/config/rte_config.h b/config/rte_config.h
>> index 7606f5d7b..5f1749dbb 100644
>> --- a/config/rte_config.h
>> +++ b/config/rte_config.h
>> @@ -18,6 +18,11 @@
>>
>>  #include <rte_build_config.h>
>>
>> +/**
>> + * String that appears before the version number
>> + */
>> +#define RTE_VER_PREFIX "DPDK"
>> +
>>  /****** library defines ********/
>>
>>  /* compat defines */
>> diff --git a/lib/librte_eal/common/include/rte_version.h
>> b/lib/librte_eal/common/include/rte_version.h
>> index a7eed7ab7..50867ea81 100644
>> --- a/lib/librte_eal/common/include/rte_version.h
>> +++ b/lib/librte_eal/common/include/rte_version.h
>> @@ -19,38 +19,6 @@ extern "C" {
>>  #include <stdio.h>
>>  #include <rte_common.h>
>>
>> -/**
>> - * String that appears before the version number
>> - */
>> -#define RTE_VER_PREFIX "DPDK"
>> -
>> -/**
>> - * Major version/year number i.e. the yy in yy.mm.z
>> - */
>> -#define RTE_VER_YEAR 19
>> -
>> -/**
>> - * Minor version/month number i.e. the mm in yy.mm.z
>> - */
>> -#define RTE_VER_MONTH 5
>> -
>> -/**
>> - * Patch level number i.e. the z in yy.mm.z
>> - */
>> -#define RTE_VER_MINOR 0
>> -
>> -/**
>> - * Extra string to be appended to version number
>> - */
>> -#define RTE_VER_SUFFIX "-rc"
>> -
>> -/**
>> - * Patch release number
>> - *   0-15 = release candidates
>> - *   16   = release
>> - */
>> -#define RTE_VER_RELEASE 0
>> -
>>  /**
>>   * Macro to compute a version number usable for comparisons
>>   */
>> diff --git a/meson.build b/meson.build
>> index 757618144..9d5320395 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -22,6 +22,20 @@ dpdk_app_link_libraries = []
>>  pver = meson.project_version().split('.')
>>  major_version = '@0@.@1@'.format(pver.get(0), pver.get(1))
>>
>> +# extract all version information into the build configuration
>> +dpdk_conf.set('RTE_VER_YEAR', pver.get(0))
>> +dpdk_conf.set('RTE_VER_MONTH', pver.get(1))
>> +if pver.get(2).contains('-rc')
>> +       rc_ver = pver.get(2).split('-rc')
>> +       dpdk_conf.set('RTE_VER_MINOR', rc_ver.get(0))
>> +       dpdk_conf.set_quoted('RTE_VER_SUFFIX', '-rc')
>> +       dpdk_conf.set('RTE_VER_RELEASE', rc_ver.get(1))
>> +else
>> +       dpdk_conf.set('RTE_VER_MINOR', pver.get(2))
>> +       dpdk_conf.set_quoted('RTE_VER_SUFFIX', '')
>> +       dpdk_conf.set('RTE_VER_RELEASE', 0)
>> +endif
>> +
>>  pmd_subdir_opt = get_option('drivers_install_subdir')
>>  if pmd_subdir_opt.contains('<VERSION>')
>>         pmd_subdir_opt =
>> major_version.join(pmd_subdir_opt.split('<VERSION>'))
>> diff --git a/mk/rte.sdkconfig.mk b/mk/rte.sdkconfig.mk
>> index 8dc31d7cf..bbfa24dee 100644
>> --- a/mk/rte.sdkconfig.mk
>> +++ b/mk/rte.sdkconfig.mk
>> @@ -76,6 +76,8 @@ else
>>  $(RTE_OUTPUT)/.config: $(RTE_CONFIG_TEMPLATE) FORCE | $(RTE_OUTPUT)
>>         $(Q)if [ "$(RTE_CONFIG_TEMPLATE)" != "" -a -f
>> "$(RTE_CONFIG_TEMPLATE)" ]; then \
>>                 $(CPP) -undef -P -x assembler-with-cpp \
>> +               `cat $(RTE_SRCDIR)/DPDK_VERSION | sed -e 's/-rc/.-rc./' |
>> \
>> +               awk -F '.' '{print "-D__YEAR="$$1, "-D__MONTH="$$2,
>> "-D__MINOR="$$3, "-D__SUFFIX=\""$$4"\"", "-D__RELEASE="int($$5)}'` \
>>
>
> Can't we just pass the RTE_VER_XXX as CFLAGS here ?
>

Argh no, I mean passing RTE_VER_XXX as global CFLAGS.
I am not really a fan of having the version in the config file, but I have
no real argument against it.


-- 
David Marchand