[dpdk-dev,2/3] version: adjust printing for new version scheme

Message ID 1450704384-9986-3-git-send-email-bruce.richardson@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: Thomas Monjalon
Headers

Commit Message

Bruce Richardson Dec. 21, 2015, 1:26 p.m. UTC
  Since we are now using a year-month numbering scheme, adjust
the printing of the version to always use 2-digits for YY.MM
format.
Also omit the patch version unless there is a patch version present,
since patches for releases are rare on DPDK. This means that the
final release of 16.04 will report as 16.04, rather than 16.04.0.
Release candidates for it will similarly report as 16.04-rcX.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_eal/common/include/rte_version.h | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)
  

Comments

Thomas Monjalon Dec. 28, 2015, 10:25 p.m. UTC | #1
2015-12-21 13:26, Bruce Richardson:
> Since we are now using a year-month numbering scheme, adjust
> the printing of the version to always use 2-digits for YY.MM
> format.

Yes
It must be done for "make showversion" also.

> Also omit the patch version unless there is a patch version present,
> since patches for releases are rare on DPDK. This means that the
> final release of 16.04 will report as 16.04, rather than 16.04.0.

So the numbering of master and maintenance releases will not be consistent:
	16.04 and then 16.04.1
It's true that maintenance releases are rare but it has been discussed at
Dublin to have ones in future.
So are we OK to omit the .0 even if not consistent?

> Release candidates for it will similarly report as 16.04-rcX.

Yes, 16.04-rcX looks nicer than 16.04.0-rcX.

Shouldn't we take the opportunity to update RTE_VER_PREFIX from
"RTE" to "DPDK"?

>  /**
>   * Major version number i.e. the x in x.y.z
>   */
> -#define RTE_VER_MAJOR 16
> +#define RTE_REL_YEAR 16
>  
>  /**
>   * Minor version number i.e. the y in x.y.z
>   */
> -#define RTE_VER_MINOR 4
> +#define RTE_REL_MONTH 4

Why renaming from _VER_ to _REL_?
mk/rte.sdkconfig.mk is not updated accordingly
(make showversion is broken)

[...]
>  #define RTE_VERSION RTE_VERSION_NUM( \
> -			RTE_VER_MAJOR, \
> -			RTE_VER_MINOR, \
> +			RTE_REL_YEAR, \
> +			RTE_REL_MONTH, \
>  			RTE_VER_PATCH_LEVEL, \
>  			RTE_VER_PATCH_RELEASE)

Is there a better name for RTE_VER_PATCH_LEVEL and RTE_VER_PATCH_RELEASE?
I think PATCH_LEVEL should be MINOR, i.e. the number increased when
doing some maintenance releases.
The last digit is useful for release candidates and non-official packaging
(downstream consumers like Linux distros or vendors). It should be updated
when delivering a patched DPDK version. RTE_VER_SUFFIX should also be updated
accordingly. So is RTE_VER_PATCH_RELEASE the right name? I guess yes but not
sure.

[...]
> +	if (RTE_VER_PATCH_LEVEL > 0)
> +		pos += snprintf(version + pos, sizeof(version) - pos, ".%d",
>  			RTE_VER_PATCH_LEVEL);

I disagree.
It's important to know that it is the first of the major release (.0).
I think we can remove it elsewhere. Example: PROJECT_NUMBER in doxygen.
  
John McNamara Feb. 10, 2016, 2:28 p.m. UTC | #2
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Monday, December 28, 2015 10:25 PM
> To: Richardson, Bruce <bruce.richardson@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 2/3] version: adjust printing for new
> version scheme
> 
> 2015-12-21 13:26, Bruce Richardson:
> > Since we are now using a year-month numbering scheme, adjust the
> > printing of the version to always use 2-digits for YY.MM format.

Hi,

In the interest of getting the version patch in and the release notes renamed
I'm going to drop this part of the patchset and resubmit 1/3 and 3/3. If we 
want to rework the version output we can do it in a separate patch.

I am going to maintain the zero padding of the month so that it appear as 16.04
and not 16.4 in "make showversion" and rte_version().

> Shouldn't we take the opportunity to update RTE_VER_PREFIX from "RTE" to
> "DPDK"?

I'm also going to make this change "en passant" since it makes sense and is
in the same file. The version outputs now look like this:

    $ make showversion
    16.04.0-rc0

    RTE>> version_autotest
    Version string: 'DPDK 16.04.0-rc0'

Note, if the s/RTE/DPDK/ change breaks anyone's parsing of output for tests
or otherwise, let me know.

John.
--
  

Patch

diff --git a/lib/librte_eal/common/include/rte_version.h b/lib/librte_eal/common/include/rte_version.h
index f1c7b98..7feea73 100644
--- a/lib/librte_eal/common/include/rte_version.h
+++ b/lib/librte_eal/common/include/rte_version.h
@@ -55,12 +55,12 @@  extern "C" {
 /**
  * Major version number i.e. the x in x.y.z
  */
-#define RTE_VER_MAJOR 16
+#define RTE_REL_YEAR 16
 
 /**
  * Minor version number i.e. the y in x.y.z
  */
-#define RTE_VER_MINOR 4
+#define RTE_REL_MONTH 4
 
 /**
  * Patch level number i.e. the z in x.y.z
@@ -88,8 +88,8 @@  extern "C" {
  * All version numbers in one to compare with RTE_VERSION_NUM()
  */
 #define RTE_VERSION RTE_VERSION_NUM( \
-			RTE_VER_MAJOR, \
-			RTE_VER_MINOR, \
+			RTE_REL_YEAR, \
+			RTE_REL_MONTH, \
 			RTE_VER_PATCH_LEVEL, \
 			RTE_VER_PATCH_RELEASE)
 
@@ -102,20 +102,19 @@  static inline const char *
 rte_version(void)
 {
 	static char version[32];
+	int pos;
 	if (version[0] != 0)
 		return version;
-	if (strlen(RTE_VER_SUFFIX) == 0)
-		snprintf(version, sizeof(version), "%s %d.%d.%d",
+
+	pos = snprintf(version, sizeof(version), "%s %02d.%02d",
 			RTE_VER_PREFIX,
-			RTE_VER_MAJOR,
-			RTE_VER_MINOR,
+			RTE_REL_YEAR,
+			RTE_REL_MONTH);
+	if (RTE_VER_PATCH_LEVEL > 0)
+		pos += snprintf(version + pos, sizeof(version) - pos, ".%d",
 			RTE_VER_PATCH_LEVEL);
-	else
-		snprintf(version, sizeof(version), "%s %d.%d.%d%s%d",
-			RTE_VER_PREFIX,
-			RTE_VER_MAJOR,
-			RTE_VER_MINOR,
-			RTE_VER_PATCH_LEVEL,
+	if (strlen(RTE_VER_SUFFIX) > 0)
+		pos += snprintf(version + pos, sizeof(version) - pos, "%s%d",
 			RTE_VER_SUFFIX,
 			RTE_VER_PATCH_RELEASE < 16 ?
 				RTE_VER_PATCH_RELEASE :