[dpdk-dev,v2] usertools: script to output version string

Message ID 20170706145649.65075-1-keith.wiles@intel.com (mailing list archive)
State Rejected, archived
Delegated to: Thomas Monjalon
Headers

Checks

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

Commit Message

Wiles, Keith July 6, 2017, 2:56 p.m. UTC
  Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 usertools/dpdk-version.sh | 124 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 124 insertions(+)
 create mode 100755 usertools/dpdk-version.sh
  

Comments

Thomas Monjalon Aug. 7, 2017, 11:35 a.m. UTC | #1
Hi,

06/07/2017 16:56, Keith Wiles:
> +# Locate the rte_version.h file and parse out the version strings.

I think this script is not needed because we have already
something in mk/rte.sdkconfig.mk.
Example:
	% make showversion
	17.08.0-rc4

Do you need more?
  
Wiles, Keith Aug. 7, 2017, 1:17 p.m. UTC | #2
> On Aug 7, 2017, at 6:35 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> Hi,
> 
> 06/07/2017 16:56, Keith Wiles:
>> +# Locate the rte_version.h file and parse out the version strings.
> 
> I think this script is not needed because we have already
> something in mk/rte.sdkconfig.mk.
> Example:
> 	% make showversion
> 	17.08.0-rc4

Executing make to find out a version seems to bit of over kill to me and a simple script would be much easier IMO. I do not really see harm in having this script that can output the version into different formats is helpful when running external scripts. Also in the future we are most like going to replace the build system and then we would need to add this support to that build system and having it in a standalone script is easier manage in the long run.

> 
> Do you need more?

Regards,
Keith
  
Thomas Monjalon Aug. 7, 2017, 4:49 p.m. UTC | #3
07/08/2017 15:17, Wiles, Keith:
> 
> > On Aug 7, 2017, at 6:35 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> > 
> > Hi,
> > 
> > 06/07/2017 16:56, Keith Wiles:
> >> +# Locate the rte_version.h file and parse out the version strings.
> > 
> > I think this script is not needed because we have already
> > something in mk/rte.sdkconfig.mk.
> > Example:
> > 	% make showversion
> > 	17.08.0-rc4
> 
> Executing make to find out a version seems to bit of over kill to me and a simple script would be much easier IMO. I do not really see harm in having this script that can output the version into different formats is helpful when running external scripts. Also in the future we are most like going to replace the build system and then we would need to add this support to that build system and having it in a standalone script is easier manage in the long run.

"make showversion" is here for several years.
Usually we do not implement the same thing in several places
without a good reason.
And some users can use the old command.
Especially when talking about versions, it is very convenient
to use the same command in every versions.

That's why I insist: why implementing it differently?
If it is only about the new build system, it should be part
of the build system migration plan (long term).
  
Wiles, Keith Aug. 7, 2017, 5:31 p.m. UTC | #4
> On Aug 7, 2017, at 11:49 AM, Thomas Monjalon <thomas@monjalon.net> wrote:

> 

> 07/08/2017 15:17, Wiles, Keith:

>> 

>>> On Aug 7, 2017, at 6:35 AM, Thomas Monjalon <thomas@monjalon.net> wrote:

>>> 

>>> Hi,

>>> 

>>> 06/07/2017 16:56, Keith Wiles:

>>>> +# Locate the rte_version.h file and parse out the version strings.

>>> 

>>> I think this script is not needed because we have already

>>> something in mk/rte.sdkconfig.mk.

>>> Example:

>>> 	% make showversion

>>> 	17.08.0-rc4

>> 

>> Executing make to find out a version seems to bit of over kill to me and a simple script would be much easier IMO. I do not really see harm in having this script that can output the version into different formats is helpful when running external scripts. Also in the future we are most like going to replace the build system and then we would need to add this support to that build system and having it in a standalone script is easier manage in the long run.

> 

> "make showversion" is here for several years.

> Usually we do not implement the same thing in several places

> without a good reason.

> And some users can use the old command.

> Especially when talking about versions, it is very convenient

> to use the same command in every versions.


I grep’d the repo and found couple references to showversion.

./devtools/git-log-fixes.sh:72:		make showversion | cut -d'.' -f-2
./doc/guides/conf.py:67:version = subprocess.check_output(['make', '-sRrC', '../../', 'showversion'])
./mk/rte.sdkconfig.mk:32:.PHONY: showversion
./mk/rte.sdkconfig.mk:33:showversion:
./mk/rte.sdkconfig.mk:45:.PHONY: showversionum
./mk/rte.sdkconfig.mk:46:showversionum:
./mk/rte.sdkdoc.mk:76:	                     $(MAKE) -rRs showversion && \
./mk/rte.sdkroot.mk:91:.PHONY: config defconfig showconfigs showversion showversionum
./mk/rte.sdkroot.mk:92:config defconfig showconfigs showversion showversionum:


Which means to me this was a completely undocumented feature, which is why I created this shell script. This also means the feature was not used widely. Then I suggest we remove the 'make showversion' and use the script in the usertools dir, which is easier to find and does not require the use of Make or Makefile at all. The script allows you to print the version from any directory as long as RTE_SDK is defined or if you are located in a SDK directory. Also allows the developer/user to execute dpdk-version.sh from the installed location or the RTE_SDK/usertools directory. I have the changes already to install dpdk-version.h tool during build like the other tools, but have not submitted it yet. I can also update the documents to help users find the tool.

> 

> That's why I insist: why implementing it differently?

> If it is only about the new build system, it should be part

> of the build system migration plan (long term).


Regards,
Keith
  
Ferruh Yigit Jan. 23, 2019, 7:16 p.m. UTC | #5
On 8/7/2017 6:31 PM, keith.wiles at intel.com (Wiles, Keith) wrote:
>> On Aug 7, 2017, at 11:49 AM, Thomas Monjalon <thomas at monjalon.net> wrote:
>>
>> 07/08/2017 15:17, Wiles, Keith:
>>>
>>>> On Aug 7, 2017, at 6:35 AM, Thomas Monjalon <thomas at monjalon.net> wrote:
>>>>
>>>> Hi,
>>>>
>>>> 06/07/2017 16:56, Keith Wiles:
>>>>> +# Locate the rte_version.h file and parse out the version strings.
>>>>
>>>> I think this script is not needed because we have already
>>>> something in mk/rte.sdkconfig.mk.
>>>> Example:
>>>> 	% make showversion
>>>> 	17.08.0-rc4
>>>
>>> Executing make to find out a version seems to bit of over kill to me and a simple script would be much easier IMO. I do not really see harm in having this script that can output the version into different formats is helpful when running external scripts. Also in the future we are most like going to replace the build system and then we would need to add this support to that build system and having it in a standalone script is easier manage in the long run.
>>
>> "make showversion" is here for several years.
>> Usually we do not implement the same thing in several places
>> without a good reason.
>> And some users can use the old command.
>> Especially when talking about versions, it is very convenient
>> to use the same command in every versions.
> 
> I grep?d the repo and found couple references to showversion.
> 
> ./devtools/git-log-fixes.sh:72:		make showversion | cut -d'.' -f-2
> ./doc/guides/conf.py:67:version = subprocess.check_output(['make', '-sRrC', '../../', 'showversion'])
> ./mk/rte.sdkconfig.mk:32:.PHONY: showversion
> ./mk/rte.sdkconfig.mk:33:showversion:
> ./mk/rte.sdkconfig.mk:45:.PHONY: showversionum
> ./mk/rte.sdkconfig.mk:46:showversionum:
> ./mk/rte.sdkdoc.mk:76:	                     $(MAKE) -rRs showversion && \
> ./mk/rte.sdkroot.mk:91:.PHONY: config defconfig showconfigs showversion showversionum
> ./mk/rte.sdkroot.mk:92:config defconfig showconfigs showversion showversionum:
> 
> 
> Which means to me this was a completely undocumented feature, which is why I created this shell script. This also means the feature was not used widely. Then I suggest we remove the 'make showversion' and use the script in the usertools dir, which is easier to find and does not require the use of Make or Makefile at all. The script allows you to print the version from any directory as long as RTE_SDK is defined or if you are located in a SDK directory. Also allows the developer/user to execute dpdk-version.sh from the installed location or the RTE_SDK/usertools directory. I have the changes already to install dpdk-version.h tool during build like the other tools, but have not submitted it yet. I can also update the documents to help users find the tool.

Hi Keith,

This patch is also sitting in the patchwork for a long time without a decision.

I am not against this script explicitly, but we already has a solution what this
script does and I don't see the motivation to replace it.
Perhaps we may need it when switched to meson, I don't know if we have a
solution in meson, cc'ed Luca for comment.

Unless there is a demand/need for it, I am for updating it as rejected.

Thanks,
ferruh

> 
>>
>> That's why I insist: why implementing it differently?
>> If it is only about the new build system, it should be part
>> of the build system migration plan (long term).
> 
> Regards,
> Keith
> 
>
  
Ferruh Yigit March 1, 2019, 4:46 p.m. UTC | #6
On 1/23/2019 7:16 PM, Ferruh Yigit wrote:
> On 8/7/2017 6:31 PM, keith.wiles at intel.com (Wiles, Keith) wrote:
>>> On Aug 7, 2017, at 11:49 AM, Thomas Monjalon <thomas at monjalon.net> wrote:
>>>
>>> 07/08/2017 15:17, Wiles, Keith:
>>>>
>>>>> On Aug 7, 2017, at 6:35 AM, Thomas Monjalon <thomas at monjalon.net> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> 06/07/2017 16:56, Keith Wiles:
>>>>>> +# Locate the rte_version.h file and parse out the version strings.
>>>>>
>>>>> I think this script is not needed because we have already
>>>>> something in mk/rte.sdkconfig.mk.
>>>>> Example:
>>>>> 	% make showversion
>>>>> 	17.08.0-rc4
>>>>
>>>> Executing make to find out a version seems to bit of over kill to me and a simple script would be much easier IMO. I do not really see harm in having this script that can output the version into different formats is helpful when running external scripts. Also in the future we are most like going to replace the build system and then we would need to add this support to that build system and having it in a standalone script is easier manage in the long run.
>>>
>>> "make showversion" is here for several years.
>>> Usually we do not implement the same thing in several places
>>> without a good reason.
>>> And some users can use the old command.
>>> Especially when talking about versions, it is very convenient
>>> to use the same command in every versions.
>>
>> I grep?d the repo and found couple references to showversion.
>>
>> ./devtools/git-log-fixes.sh:72:		make showversion | cut -d'.' -f-2
>> ./doc/guides/conf.py:67:version = subprocess.check_output(['make', '-sRrC', '../../', 'showversion'])
>> ./mk/rte.sdkconfig.mk:32:.PHONY: showversion
>> ./mk/rte.sdkconfig.mk:33:showversion:
>> ./mk/rte.sdkconfig.mk:45:.PHONY: showversionum
>> ./mk/rte.sdkconfig.mk:46:showversionum:
>> ./mk/rte.sdkdoc.mk:76:	                     $(MAKE) -rRs showversion && \
>> ./mk/rte.sdkroot.mk:91:.PHONY: config defconfig showconfigs showversion showversionum
>> ./mk/rte.sdkroot.mk:92:config defconfig showconfigs showversion showversionum:
>>
>>
>> Which means to me this was a completely undocumented feature, which is why I created this shell script. This also means the feature was not used widely. Then I suggest we remove the 'make showversion' and use the script in the usertools dir, which is easier to find and does not require the use of Make or Makefile at all. The script allows you to print the version from any directory as long as RTE_SDK is defined or if you are located in a SDK directory. Also allows the developer/user to execute dpdk-version.sh from the installed location or the RTE_SDK/usertools directory. I have the changes already to install dpdk-version.h tool during build like the other tools, but have not submitted it yet. I can also update the documents to help users find the tool.
> 
> Hi Keith,
> 
> This patch is also sitting in the patchwork for a long time without a decision.
> 
> I am not against this script explicitly, but we already has a solution what this
> script does and I don't see the motivation to replace it.
> Perhaps we may need it when switched to meson, I don't know if we have a
> solution in meson, cc'ed Luca for comment.
> 
> Unless there is a demand/need for it, I am for updating it as rejected.

It seems meson does not have this feature but also it needs to have the version
information in root folder meson file, which is easier to implement to grep it
and get this information.

This patch can be referenced when it is needed, but for now removed from backlog.

> 
> Thanks,
> ferruh
> 
>>
>>>
>>> That's why I insist: why implementing it differently?
>>> If it is only about the new build system, it should be part
>>> of the build system migration plan (long term).
>>
>> Regards,
>> Keith
>>
>>
>
  

Patch

diff --git a/usertools/dpdk-version.sh b/usertools/dpdk-version.sh
new file mode 100755
index 000000000..7b794bcd6
--- /dev/null
+++ b/usertools/dpdk-version.sh
@@ -0,0 +1,124 @@ 
+#!/bin/bash
+#
+#   BSD LICENSE
+#
+#   Copyright(c) 2017 Intel Corporation. All rights reserved.
+#   All rights reserved.
+#
+#   Redistribution and use in source and binary forms, with or without
+#   modification, are permitted provided that the following conditions
+#   are met:
+#
+#     * Redistributions of source code must retain the above copyright
+#       notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above copyright
+#       notice, this list of conditions and the following disclaimer in
+#       the documentation and/or other materials provided with the
+#       distribution.
+#     * Neither the name of Intel Corporation nor the names of its
+#       contributors may be used to endorse or promote products derived
+#       from this software without specific prior written permission.
+#
+#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+#
+# Locate the rte_version.h file and parse out the version strings.
+#
+# If RTE_SDK is set, then it is used to locate the file. If the
+# RTE_SDK variable is not set then the current working directory
+# is used.
+#
+# See usage below for more details.
+#
+
+fname=rte_version.h
+
+function usage() {
+	cat <<EOM
+  Usage: dpdk-version.sh
+
+  Locate the rte_version.h file and parse out the version information.
+
+  If RTE_SDK is set, then it is used to locate the file. If the
+  RTE_SDK variable is not set then the current working directory
+  is used.
+
+  The default output of the version is 'DPDK Version: YY.MM.MINOR'
+	e.g. 'dpdk-version.sh' gives 'DPDK Version: 17.08.0'
+
+  Options: Only one option can be used at a time.
+
+  '-h' prints usage messages
+
+  '-l' prints out the full version string
+	e.g. 'dpdk-version.sh -l' gives 'DPDK-17.08.0-rc0'
+
+  '-s' prints a shorter version string
+	e.g. 'dpdk-version.sh -s' gives '17.08.0'
+
+  '-m' prints out information is comma delimited format.
+	e.g. 'dpdk-version.sh -m' give 'DPDK,17,08,0,-rc0'
+
+  '-p' prints out a Python readable format of the version.
+	e.g. 'dpdk-version.sh -m' gives
+
+	version = {
+		'prefix': 'DPDK',
+		'year': '17',
+		'month': '08',
+		'minor': '0',
+		'suffix': '-rc'
+	}
+EOM
+	exit 1
+}
+
+if [ -z ${RTE_SDK} ] ; then
+	echo "*** RTE_SDK is not set, using "${PWD}
+	sdk=${PWD}
+else
+	sdk=${RTE_SDK}
+fi
+
+fpath=${sdk}/lib/librte_eal/common/include/${fname}
+
+if [ ! -e ${fpath} ]; then
+	echo "File not found @ "${fpath}
+	usage
+fi
+
+PREFIX=`grep "define RTE_VER_PREFIX" ${fpath} | cut -d ' ' -f 3 | sed -e 's/\"//g'`
+SUFFIX=`grep "define RTE_VER_SUFFIX" ${fpath} | cut -d ' ' -f 3 | sed -e 's/\"//g'`
+
+YY=`grep "define RTE_VER_YEAR" ${fpath} | cut -d ' ' -f 3`
+MM=`grep "define RTE_VER_MONTH" ${fpath} | cut -d ' ' -f 3`
+MINOR=`grep "define RTE_VER_MINOR" ${fpath} | cut -d ' ' -f 3`
+
+if [ "$1" = "-h" ]; then
+	usage
+elif [ "$1" = "-l" ]; then
+	echo ${PREFIX}-${YY}.${MM}.${MINOR}${SUFFIX}
+elif [ "$1" = '-s' ]; then
+	echo ${YY}.${MM}.${MINOR}
+elif [ "$1" = "-p" ]; then
+	echo "dpdk_version = {"
+	echo "    'prefix': '"${PREFIX}"',"
+	echo "    'year': '"${YY}"',"
+	echo "    'month': '"${MM}"',"
+	echo "    'minor': '"${MINOR}"',"
+	echo "    'suffix': '"${SUFFIX}"'"
+	echo "}"
+elif [ "$1" = "-m" ]; then
+	echo ""${PREFIX}","${YY}","${MM}","${MINOR}","${SUFFIX}""
+else
+	echo "DPDK Version: "${YY}.${MM}.${MINOR}
+fi