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 > >
