Re: [ovs-dev] [PATCH v8] rhel: Make the version, displayed to the user, customizable.

2024-05-29 Thread Ilya Maximets
On 4/15/24 14:58, Timothy Redaelli wrote:
> Since on CentOS/RHEL the builds are based on stable branches and not on
> tags for debugging purpose it's better to have the downstream version as
> version so it's easier to know which commits are included in a build.
> 
> This commit adds --with-version-suffix as ./configure option in
> order to set an OVS version suffix that should be shown to the user via
> ovs-vsctl -V and, so, also on database, on ovs-vsctl show and the other
> utilities.
> 
> --with-version-suffix is used in Fedora/CentOS/RHEL spec file in order to have
> the version be aligned with the downstream one.
> 
> Signed-off-by: Timothy Redaelli 
> ---

Hi, Timothy.  Thanks for v8 and sorry for a long delay.

Unfortunately there are still a few issues with the patch.

If I ran the following:

$ ./boot.sh && ./configure --with-version-suffix=qwe && make -j8 distcheck

The build fails with:

printf '\043 Generated automatically -- do not modify!-*- buffer-read-only: 
t -*-\n' > version.py.tmp && \
echo 'VERSION = "3.3.90"' >> version.py.tmp && \
if cmp -s version.py.tmp ../../python/ovs/version.py; then touch 
../../python/ovs/version.py; rm version.py.tmp; else mv version.py.tmp 
../../python/ovs/version.py; fi
mv: cannot move 'version.py.tmp' to '../../python/ovs/version.py': Permission 
denied
make[2]: *** [Makefile:7345: ../../python/ovs/version.py] Error 1

Interestingly enough this doesn't happen if the version suffix
is not set.

A few more comments below.

> v1 -> v2: Use --with-version-suffix= and add version to other utilies
>   (as requested by Ilya).
> 
> v2 -> v3: Add versioning to python utilities and python library itself
>   (as suggested by Aaron).
> 
> v3 -> v4: Remove versioning to python library itself to avoid breaking
>   PEP440 (as requested by Ilya). Versioning is still used in
>   python utilities.
> 
> v4 -> v5: Re-add versioning to python library itself, but don't use it on
>   setup.py (to avoid breaking PEP440). This will permit to have the
>   custom version as ovs.version.VERSION (in case somebody uses it) 
> and,
>   so, also in python/ovs/unixctl/server.py (as suggested by Ilya).
> 
> v5 -> v6: Fix some setup.py leftovers and change the test as a loop
>   (as suggested by Ilya).

This change disappeared in v7+ for some reason.

> 
> v6 -> v7: Rebase with last master (it should pass CI now)
> 
> v7 -> v8: Be sure python-sdist depends on python/setup.py and run flake8 on
>   setup.py.template instead of setup.py (as suggested by Ilya).
>   Fix commit summary to make checkpatch.py happy
> ---
>  Makefile.am|  1 +
>  acinclude.m4   | 13 +
>  configure.ac   |  1 +
>  include/openvswitch/version.h.in   |  2 +-
>  lib/ovsdb-error.c  |  2 +-
>  lib/util.c |  5 +++--
>  ovsdb/ovsdb-server.c   |  3 ++-
>  python/.gitignore  |  1 +
>  python/automake.mk | 20 ++--
>  python/{setup.py => setup.py.template} |  9 -
>  rhel/openvswitch-fedora.spec.in|  1 +
>  utilities/ovs-dpctl-top.in |  2 +-
>  utilities/ovs-lib.in   |  2 +-
>  utilities/ovs-parse-backtrace.in   |  2 +-
>  utilities/ovs-pcap.in  |  2 +-
>  utilities/ovs-pki.in   |  2 +-
>  utilities/ovs-tcpdump.in   |  4 ++--
>  utilities/ovs-tcpundump.in |  2 +-
>  utilities/ovs-vlan-test.in |  2 +-
>  vswitchd/bridge.c  |  3 ++-
>  20 files changed, 53 insertions(+), 26 deletions(-)
>  rename python/{setup.py => setup.py.template} (95%)
> 
> diff --git a/Makefile.am b/Makefile.am
> index e6c90a911..fa89f591b 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -163,6 +163,7 @@ SUFFIXES += .in
>   -e 's,[@]PYTHON3[@],$(PYTHON3),g' \
>   -e 's,[@]RUNDIR[@],$(RUNDIR),g' \
>   -e 's,[@]VERSION[@],$(VERSION),g' \
> + -e 's,[@]VERSION_SUFFIX[@],$(VERSION_SUFFIX),g' \
>   -e 's,[@]localstatedir[@],$(localstatedir),g' \
>   -e 's,[@]pkgdatadir[@],$(pkgdatadir),g' \
>   -e 's,[@]sysconfdir[@],$(sysconfdir),g' \
> diff --git a/acinclude.m4 b/acinclude.m4
> index f1ba046c2..f7a81a734 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -497,6 +497,19 @@ AC_DEFUN([OVS_CHECK_DPDK], [
>AM_CONDITIONAL([DPDK_NETDEV], test "$DPDKLIB_FOUND" = true)
>  ])
>  
> +dnl Append a version suffix

Period at the end of the sentence.

> +
> +AC_DEFUN([OVS_CHECK_VERSION_SUFFIX], [
> +  AC_ARG_WITH([version-suffix],
> +  [AS_HELP_STRING([--with-version-suffix=ver_suffix],
> +  [Specify a version suffix that will be appended

s/a version suffix/a string/

> +   to OVS version])])
> +  

[ovs-dev] [PATCH v8] rhel: Make the version, displayed to the user, customizable.

2024-04-15 Thread Timothy Redaelli
Since on CentOS/RHEL the builds are based on stable branches and not on
tags for debugging purpose it's better to have the downstream version as
version so it's easier to know which commits are included in a build.

This commit adds --with-version-suffix as ./configure option in
order to set an OVS version suffix that should be shown to the user via
ovs-vsctl -V and, so, also on database, on ovs-vsctl show and the other
utilities.

--with-version-suffix is used in Fedora/CentOS/RHEL spec file in order to have
the version be aligned with the downstream one.

Signed-off-by: Timothy Redaelli 
---
v1 -> v2: Use --with-version-suffix= and add version to other utilies
  (as requested by Ilya).

v2 -> v3: Add versioning to python utilities and python library itself
  (as suggested by Aaron).

v3 -> v4: Remove versioning to python library itself to avoid breaking
  PEP440 (as requested by Ilya). Versioning is still used in
  python utilities.

v4 -> v5: Re-add versioning to python library itself, but don't use it on
  setup.py (to avoid breaking PEP440). This will permit to have the
  custom version as ovs.version.VERSION (in case somebody uses it) and,
  so, also in python/ovs/unixctl/server.py (as suggested by Ilya).

v5 -> v6: Fix some setup.py leftovers and change the test as a loop
  (as suggested by Ilya).

v6 -> v7: Rebase with last master (it should pass CI now)

v7 -> v8: Be sure python-sdist depends on python/setup.py and run flake8 on
  setup.py.template instead of setup.py (as suggested by Ilya).
  Fix commit summary to make checkpatch.py happy
---
 Makefile.am|  1 +
 acinclude.m4   | 13 +
 configure.ac   |  1 +
 include/openvswitch/version.h.in   |  2 +-
 lib/ovsdb-error.c  |  2 +-
 lib/util.c |  5 +++--
 ovsdb/ovsdb-server.c   |  3 ++-
 python/.gitignore  |  1 +
 python/automake.mk | 20 ++--
 python/{setup.py => setup.py.template} |  9 -
 rhel/openvswitch-fedora.spec.in|  1 +
 utilities/ovs-dpctl-top.in |  2 +-
 utilities/ovs-lib.in   |  2 +-
 utilities/ovs-parse-backtrace.in   |  2 +-
 utilities/ovs-pcap.in  |  2 +-
 utilities/ovs-pki.in   |  2 +-
 utilities/ovs-tcpdump.in   |  4 ++--
 utilities/ovs-tcpundump.in |  2 +-
 utilities/ovs-vlan-test.in |  2 +-
 vswitchd/bridge.c  |  3 ++-
 20 files changed, 53 insertions(+), 26 deletions(-)
 rename python/{setup.py => setup.py.template} (95%)

diff --git a/Makefile.am b/Makefile.am
index e6c90a911..fa89f591b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -163,6 +163,7 @@ SUFFIXES += .in
-e 's,[@]PYTHON3[@],$(PYTHON3),g' \
-e 's,[@]RUNDIR[@],$(RUNDIR),g' \
-e 's,[@]VERSION[@],$(VERSION),g' \
+   -e 's,[@]VERSION_SUFFIX[@],$(VERSION_SUFFIX),g' \
-e 's,[@]localstatedir[@],$(localstatedir),g' \
-e 's,[@]pkgdatadir[@],$(pkgdatadir),g' \
-e 's,[@]sysconfdir[@],$(sysconfdir),g' \
diff --git a/acinclude.m4 b/acinclude.m4
index f1ba046c2..f7a81a734 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -497,6 +497,19 @@ AC_DEFUN([OVS_CHECK_DPDK], [
   AM_CONDITIONAL([DPDK_NETDEV], test "$DPDKLIB_FOUND" = true)
 ])
 
+dnl Append a version suffix
+
+AC_DEFUN([OVS_CHECK_VERSION_SUFFIX], [
+  AC_ARG_WITH([version-suffix],
+  [AS_HELP_STRING([--with-version-suffix=ver_suffix],
+  [Specify a version suffix that will be appended
+   to OVS version])])
+  AC_DEFINE_UNQUOTED([VERSION_SUFFIX], ["$with_version_suffix"],
+ [Package version suffix])
+  AC_SUBST([VERSION_SUFFIX], [$with_version_suffix])
+  ])
+])
+
 dnl Checks for net/if_dl.h.
 dnl
 dnl (We use this as a proxy for checking whether we're building on FreeBSD
diff --git a/configure.ac b/configure.ac
index dd6553fea..8323e481d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -202,6 +202,7 @@ OVS_CHECK_LINUX_SCTP_CT
 OVS_CHECK_LINUX_VIRTIO_TYPES
 OVS_CHECK_DPDK
 OVS_CHECK_PRAGMA_MESSAGE
+OVS_CHECK_VERSION_SUFFIX
 AC_SUBST([CFLAGS])
 AC_SUBST([OVS_CFLAGS])
 AC_SUBST([OVS_LDFLAGS])
diff --git a/include/openvswitch/version.h.in b/include/openvswitch/version.h.in
index 23d8fde4f..231f61e30 100644
--- a/include/openvswitch/version.h.in
+++ b/include/openvswitch/version.h.in
@@ -19,7 +19,7 @@
 #define OPENVSWITCH_VERSION_H 1
 
 #define OVS_PACKAGE_STRING  "@PACKAGE_STRING@"
-#define OVS_PACKAGE_VERSION "@PACKAGE_VERSION@"
+#define OVS_PACKAGE_VERSION "@PACKAGE_VERSION@@VERSION_SUFFIX@"
 
 #define OVS_LIB_VERSION @LT_CURRENT@
 #define OVS_LIB_REVISION@LT_REVISION@
diff --git a/lib/ovsdb-error.c b/lib/ovsdb-error.c
index