Re: [Powertop] [PATCH v1 2/3] configure.ac: version strings from git describe

Re: [Powertop] [PATCH v1 2/3] configure.ac: version strings from git describe

[@ 2016-08-06 12:14 UTC (permalink / raw)]

[-- Attachment #1: Type: text/plain, Size: 2378 bytes --]



On Sat, Aug 6, 2016 at 4:53 AM, Magnus Fromreide <magfr(a)lysator.liu.se> wrote:
> On Fri, Aug 05, 2016 at 12:27:20PM -0700, Joe Konno wrote:
>> From: Joe Konno <joe.konno(a)intel.com>
>>
>> There are two version strings we need during runtime: a long version
>> (for reports and --version), and a short version for the ncurses UI. Use
>> a utility script to generate those strings (version-long and
>> version-short) which will be cat'ed by configure and bundled when 'make
>> dist' is run.
>>
>> Signed-off-by: Joe Konno <joe.konno(a)intel.com>
>
> What happens if I build this from a distributed package, i.e.
>
> ./configure
> make dist
> ta xzf powertop-version.tar.gz
> cd powertop-version
> ./configure
> make
>
> This would normally create a package with no git version info so the version
> scripts couldn't work.
>
> This could obviously be regarded as an unsupported case but if so then I think
> it should be said so explicitly.
Version script is called from autogen, configure just uses already generated files (which are in dist tarball), so there is no problem (since it is expected that autogen is called within git repo). If one wants to support case of autogen-out-of-git-repo, he should add version script to dist and modify it like this:

---8<---
#!/bin/sh

: "${LONG_VERSION_PATH=version-long}"
: "${SHORT_VERSION_PATH=version-short}"
: "${DIST_SUFFIX=-dist}"

LONG=$(cat "${LONG_VERSION_PATH}" 2>/dev/null)
SHORT=$(cat "${SHORT_VERSION_PATH}" 2>/dev/null)

git branch > /dev/null 2>&1
if [ "$?" = "0" ]; then
	LONG=$(git describe --abbrev=7 --tags --always --dirty 2> /dev/null)
	SHORT=\"$(git describe --tags --abbrev=0 2> /dev/null)\"
else
	[ -z "$LONG" -a -n "$SHORT" ] && LONG="${SHORT}"
	[ -z "$SHORT" -a -n "$LONG" ] && SHORT="${LONG}"

	if [ -z "$SHORT" -a -z "$LONG" ]
	then
		SHORT="${SHORT_VERSION-unknown}"
		LONG="${LONG_VERSION-unknown}"
	fi

	LONG="${LONG%${DIST_SUFFIX}}${DIST_SUFFIX}"
	SHORT="${SHORT%${DIST_SUFFIX}}${DIST_SUFFIX}"
fi

echo $LONG  > "${LONG_VERSION_PATH}"
echo $SHORT > "${SHORT_VERSION_PATH}"
--->8---

Two main points here — reading cached values at the start of the script and adding a dist prefix in order to signalize that this is out-of-repo configuration.

-- 
Eugene "eSyr" Syromyatnikov
mailto:evgSyr(a)gmail.com
xmpp:eSyr(a)jabber.{ru|org}

Re: [Powertop] [PATCH v1 2/3] configure.ac: version strings from git describe

[Joe Konno]

[-- Attachment #1: Type: text/plain, Size: 3581 bytes --]

Thank you both for the review and feedback. Some inlined comments below.

On Sat, 6 Aug 2016 15:14:18 +0300
Евгений Сыромятников <evgsyr(a)gmail.com> wrote:

> On Sat, Aug 6, 2016 at 4:53 AM, Magnus Fromreide <magfr(a)lysator.liu.se> wrote:
> > On Fri, Aug 05, 2016 at 12:27:20PM -0700, Joe Konno wrote:  
> >> From: Joe Konno <joe.konno(a)intel.com>
> >>
> >> There are two version strings we need during runtime: a long version
> >> (for reports and --version), and a short version for the ncurses UI. Use
> >> a utility script to generate those strings (version-long and
> >> version-short) which will be cat'ed by configure and bundled when 'make
> >> dist' is run.
> >>
> >> Signed-off-by: Joe Konno <joe.konno(a)intel.com>  
> >
> > What happens if I build this from a distributed package, i.e.
> >
> > ./configure
> > make dist
> > ta xzf powertop-version.tar.gz
> > cd powertop-version
> > ./configure
> > make
> >
> > This would normally create a package with no git version info so the version
> > scripts couldn't work.
> >
> > This could obviously be regarded as an unsupported case but if so then I
> > think it should be said so explicitly.  

My assumption is that autogen.sh is run from within a git repo, for any project
using git for version control. I'll add that verbiage to the commit message for
clarity, though. Thanks for the feedback!

> Version script is called from autogen, configure just uses already generated
> files (which are in dist tarball), so there is no problem (since it is
> expected that autogen is called within git repo). If one wants to support
> case of autogen-out-of-git-repo, he should add version script to dist and
> modify it like this:

My concern with the version script supporting the 'autogen out of git repo'
scenario-- using eSyr's sample code below as a reference point-- is loss of
connection to the git repository and the state of code when the version strings
are assembled.

My preference is to only support generation of version strings within a git
repository. This is to maintain something like version integrity, mapping code
state to 'git describe' strings. Nivedita and I will only run 'make dist'
inside a git repo when we cut release tarballs for that reason.

If folks want to support the 'autogen out of git repo' scenario, please speak
up, since I don't plan to support it. I hope my justification is clear-- if it
isn't, we can discuss this further. ^_^

> 
> ---8<---
> #!/bin/sh
> 
> : "${LONG_VERSION_PATH=version-long}"
> : "${SHORT_VERSION_PATH=version-short}"
> : "${DIST_SUFFIX=-dist}"
> 
> LONG=$(cat "${LONG_VERSION_PATH}" 2>/dev/null)
> SHORT=$(cat "${SHORT_VERSION_PATH}" 2>/dev/null)
> 
> git branch > /dev/null 2>&1
> if [ "$?" = "0" ]; then
> 	LONG=$(git describe --abbrev=7 --tags --always --dirty 2> /dev/null)
> 	SHORT=\"$(git describe --tags --abbrev=0 2> /dev/null)\"
> else
> 	[ -z "$LONG" -a -n "$SHORT" ] && LONG="${SHORT}"
> 	[ -z "$SHORT" -a -n "$LONG" ] && SHORT="${LONG}"
> 
> 	if [ -z "$SHORT" -a -z "$LONG" ]
> 	then
> 		SHORT="${SHORT_VERSION-unknown}"
> 		LONG="${LONG_VERSION-unknown}"
> 	fi
> 
> 	LONG="${LONG%${DIST_SUFFIX}}${DIST_SUFFIX}"
> 	SHORT="${SHORT%${DIST_SUFFIX}}${DIST_SUFFIX}"
> fi
> 
> echo $LONG  > "${LONG_VERSION_PATH}"
> echo $SHORT > "${SHORT_VERSION_PATH}"
> --->8---  
> 
> Two main points here — reading cached values at the start of the script and
> adding a dist prefix in order to signalize that this is out-of-repo
> configuration.
> 

[-- Attachment #2: attachment.sig --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Powertop] [PATCH v1 2/3] configure.ac: version strings from git describe

[Magnus Fromreide]

[-- Attachment #1: Type: text/plain, Size: 3318 bytes --]

On Fri, Aug 05, 2016 at 12:27:20PM -0700, Joe Konno wrote:
> From: Joe Konno <joe.konno(a)intel.com>
> 
> There are two version strings we need during runtime: a long version
> (for reports and --version), and a short version for the ncurses UI. Use
> a utility script to generate those strings (version-long and
> version-short) which will be cat'ed by configure and bundled when 'make
> dist' is run.
> 
> Signed-off-by: Joe Konno <joe.konno(a)intel.com>

What happens if I build this from a distributed package, i.e.

./configure
make dist
ta xzf powertop-version.tar.gz
cd powertop-version
./configure
make

This would normally create a package with no git version info so the version
scripts couldn't work.

This could obviously be regarded as an unsupported case but if so then I think
it should be said so explicitly.

/MF

> ---
>  .gitignore      |  1 +
>  Makefile.am     |  2 ++
>  autogen.sh      |  1 +
>  configure.ac    |  5 ++++-
>  scripts/version | 13 +++++++++++++
>  5 files changed, 21 insertions(+), 1 deletion(-)
>  create mode 100644 scripts/version
> 
> diff --git a/.gitignore b/.gitignore
> index 7eb138a094ce..ef817dc076bd 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -87,4 +87,5 @@ Makefile.in
>  /src/powertop
>  /stamp-h1
>  /stamp-po
> +/version*
>  tags
> diff --git a/Makefile.am b/Makefile.am
> index b519d110a118..3aabb22eca6e 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -14,4 +14,6 @@ EXTRA_DIST = \
>  	TODO \
>  	Android.mk \
>  	COPYING \
> +	version-long \
> +	version-short \
>  	autogen.sh
> diff --git a/autogen.sh b/autogen.sh
> index e210037d70e0..444f9998f806 100755
> --- a/autogen.sh
> +++ b/autogen.sh
> @@ -1,3 +1,4 @@
>  #!/bin/sh	
>  
> +sh scripts/version
>  autoreconf --install --verbose
> diff --git a/configure.ac b/configure.ac
> index 646b402101bb..ccfbf7ee0bb8 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -2,7 +2,8 @@
>  # Process this file with autoconf to produce a configure script.
>  
>  AC_PREREQ([2.68])
> -AC_INIT([powertop], [2.9-pre], [powertop(a)lists.01.org], [], [https://01.org/powertop])
> +AC_INIT([powertop], m4_esyscmd_s([cat version-long]),
> +	[powertop(a)lists.01.org], [], [https://01.org/powertop])
>  AM_INIT_AUTOMAKE([
>  	-Wall
>  	1.12.2
> @@ -149,5 +150,7 @@ AC_SEARCH_LIBS([inet_aton], [resolv], [], [
>  	AC_MSG_ERROR([libresolv is required but was not found])
>  ], [])
>  
> +AC_DEFINE([PACKAGE_SHORT_VERSION], m4_esyscmd_s([cat version-short]),
> +	[Short package version])
>  
>  AC_OUTPUT
> diff --git a/scripts/version b/scripts/version
> new file mode 100644
> index 000000000000..348b13d9f785
> --- /dev/null
> +++ b/scripts/version
> @@ -0,0 +1,13 @@
> +#!/bin/sh
> +
> +git branch > /dev/null 2>&1
> +if [ "$?" = "0" ]; then
> +	LONG=$(git describe --abbrev=7 --tags --always --dirty 2> /dev/null)
> +	SHORT=\"$(git describe --tags --abbrev=0 2> /dev/null)\"
> +else
> +	LONG="RUN-VERSION-SCRIPT-IN-GIT-REPOSITORY-ONLY"
> +	SHORT=\"$LONG\"
> +fi
> +
> +echo $LONG  > version-long
> +echo $SHORT > version-short
> -- 
> 1.8.3.1
> 
> _______________________________________________
> PowerTop mailing list
> PowerTop(a)lists.01.org
> https://lists.01.org/mailman/listinfo/powertop

[Powertop] [PATCH v1 2/3] configure.ac: version strings from git describe

[Joe Konno]

[-- Attachment #1: Type: text/plain, Size: 2529 bytes --]

From: Joe Konno <joe.konno(a)intel.com>

There are two version strings we need during runtime: a long version
(for reports and --version), and a short version for the ncurses UI. Use
a utility script to generate those strings (version-long and
version-short) which will be cat'ed by configure and bundled when 'make
dist' is run.

Signed-off-by: Joe Konno <joe.konno(a)intel.com>
---
 .gitignore      |  1 +
 Makefile.am     |  2 ++
 autogen.sh      |  1 +
 configure.ac    |  5 ++++-
 scripts/version | 13 +++++++++++++
 5 files changed, 21 insertions(+), 1 deletion(-)
 create mode 100644 scripts/version

diff --git a/.gitignore b/.gitignore
index 7eb138a094ce..ef817dc076bd 100644
--- a/.gitignore
+++ b/.gitignore
@@ -87,4 +87,5 @@ Makefile.in
 /src/powertop
 /stamp-h1
 /stamp-po
+/version*
 tags
diff --git a/Makefile.am b/Makefile.am
index b519d110a118..3aabb22eca6e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -14,4 +14,6 @@ EXTRA_DIST = \
 	TODO \
 	Android.mk \
 	COPYING \
+	version-long \
+	version-short \
 	autogen.sh
diff --git a/autogen.sh b/autogen.sh
index e210037d70e0..444f9998f806 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -1,3 +1,4 @@
 #!/bin/sh	
 
+sh scripts/version
 autoreconf --install --verbose
diff --git a/configure.ac b/configure.ac
index 646b402101bb..ccfbf7ee0bb8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2,7 +2,8 @@
 # Process this file with autoconf to produce a configure script.
 
 AC_PREREQ([2.68])
-AC_INIT([powertop], [2.9-pre], [powertop(a)lists.01.org], [], [https://01.org/powertop])
+AC_INIT([powertop], m4_esyscmd_s([cat version-long]),
+	[powertop(a)lists.01.org], [], [https://01.org/powertop])
 AM_INIT_AUTOMAKE([
 	-Wall
 	1.12.2
@@ -149,5 +150,7 @@ AC_SEARCH_LIBS([inet_aton], [resolv], [], [
 	AC_MSG_ERROR([libresolv is required but was not found])
 ], [])
 
+AC_DEFINE([PACKAGE_SHORT_VERSION], m4_esyscmd_s([cat version-short]),
+	[Short package version])
 
 AC_OUTPUT
diff --git a/scripts/version b/scripts/version
new file mode 100644
index 000000000000..348b13d9f785
--- /dev/null
+++ b/scripts/version
@@ -0,0 +1,13 @@
+#!/bin/sh
+
+git branch > /dev/null 2>&1
+if [ "$?" = "0" ]; then
+	LONG=$(git describe --abbrev=7 --tags --always --dirty 2> /dev/null)
+	SHORT=\"$(git describe --tags --abbrev=0 2> /dev/null)\"
+else
+	LONG="RUN-VERSION-SCRIPT-IN-GIT-REPOSITORY-ONLY"
+	SHORT=\"$LONG\"
+fi
+
+echo $LONG  > version-long
+echo $SHORT > version-short
-- 
1.8.3.1