[v2,2/4] build: use version number from config file

Message ID 20190307133502.55321-3-bruce.richardson@intel.com
State Superseded, archived
Delegated to: Thomas Monjalon
Headers show
Series
  • One versionfile to rule them all...
Related show

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/checkpatch success coding style OK

Commit Message

Bruce Richardson March 7, 2019, 1:35 p.m.
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>
Acked-by: Luca Boccassi <bluca@debian.org>
---
V2: removed extra change to DPDK_VERSION that crept into patch. Added ack
---
 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 ++
 5 files changed, 35 insertions(+), 32 deletions(-)

Comments

Thomas Monjalon March 13, 2019, 11:13 a.m. | #1
07/03/2019 14:35, Bruce Richardson:
> +#
> +# 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
[...]
> -/**
> - * Patch release number
> - *   0-15 = release candidates
> - *   16   = release
> - */
> -#define RTE_VER_RELEASE 0

So you are removing the special value 16.
Is it replaced by an empty value?

I think it is an issue for version comparison with RTE_VERSION_NUM() macro.
The -rc3 must be lower than the final release (which had number 16
for this reason).
Bruce Richardson March 13, 2019, 11:20 a.m. | #2
On Wed, Mar 13, 2019 at 12:13:52PM +0100, Thomas Monjalon wrote:
> 07/03/2019 14:35, Bruce Richardson:
> > +#
> > +# 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
> [...]
> > -/**
> > - * Patch release number
> > - *   0-15 = release candidates
> > - *   16   = release
> > - */
> > -#define RTE_VER_RELEASE 0
> 
> So you are removing the special value 16.
> Is it replaced by an empty value?
> 
> I think it is an issue for version comparison with RTE_VERSION_NUM() macro.
> The -rc3 must be lower than the final release (which had number 16
> for this reason).
> 
Aha, that explains the reason for the 16 value. I did wonder why we
bothered with it when it wasn't actually used in printing etc. Let me go
back and look into this set again, armed with this new info. [We also need
to document this reason in the code comments for future information]
Thomas Monjalon March 13, 2019, 11:24 a.m. | #3
13/03/2019 12:20, Bruce Richardson:
> On Wed, Mar 13, 2019 at 12:13:52PM +0100, Thomas Monjalon wrote:
> > 07/03/2019 14:35, Bruce Richardson:
> > > +#
> > > +# 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
> > [...]
> > > -/**
> > > - * Patch release number
> > > - *   0-15 = release candidates
> > > - *   16   = release
> > > - */
> > > -#define RTE_VER_RELEASE 0
> > 
> > So you are removing the special value 16.
> > Is it replaced by an empty value?
> > 
> > I think it is an issue for version comparison with RTE_VERSION_NUM() macro.
> > The -rc3 must be lower than the final release (which had number 16
> > for this reason).
> > 
> Aha, that explains the reason for the 16 value. I did wonder why we
> bothered with it when it wasn't actually used in printing etc. Let me go
> back and look into this set again, armed with this new info. [We also need
> to document this reason in the code comments for future information]

Yes it was not documented, sorry.
Thanks for working on it.

Patch

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
+
 #
 # 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 aaa6002ae..9fefa1b75 100644
--- a/meson.build
+++ b/meson.build
@@ -24,6 +24,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 d9c8d53d2..007022adf 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)}'` \
 		-ffreestanding \
 		-o $(RTE_OUTPUT)/.config_tmp $(RTE_CONFIG_TEMPLATE) ; \
 		config=$$(cat $(RTE_OUTPUT)/.config_tmp) ; \