diff mbox series

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

Message ID 17a16e85b50e98d45fdc98a292745c4cd3cc4a4d.1720608766.git.tredaelli@redhat.com
State Accepted
Commit 9e6d43ef32152527f7887d7f316a191adb5f338c
Delegated to: Ilya Maximets
Headers show
Series [ovs-dev,v9] rhel: Make the version, displayed to the user, customizable. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation fail test: fail

Commit Message

Timothy Redaelli July 10, 2024, 11:06 a.m. UTC
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 <tredaelli@redhat.com>
---
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

v8 -> v9: Fix make distcheck when --with-version-suffix is specified (as
          repoted by Ilya). I know AM_DISTCHECK_CONFIGURE_FLAGS is
          discouraged upstream, but it's also used by GNU m4 for a similar
          scenario so I guess it's ok to use it (see
          https://www.gnu.org/software/automake/manual/html_node/Checking-the-Distribution.html)
          Restore the loop in setup.py.template (as reported by Ilya)
          Use version suffixalso for libraries (aka test-lib) (as reported by Ilya)
          Fix a typo in configure (as reported by Ilya)
---
 Makefile.am                            |  3 +++
 acinclude.m4                           | 13 ++++++++++++
 configure.ac                           |  1 +
 include/openvswitch/version.h.in       |  2 +-
 lib/ovsdb-error.c                      |  2 +-
 lib/util.c                             |  8 +++++---
 ovsdb/ovsdb-server.c                   |  3 ++-
 python/.gitignore                      |  1 +
 python/automake.mk                     | 22 +++++++++++++-------
 python/{setup.py => setup.py.template} | 28 +++++++++-----------------
 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, 64 insertions(+), 41 deletions(-)
 rename python/{setup.py => setup.py.template} (87%)

Comments

Ilya Maximets July 12, 2024, 4:14 p.m. UTC | #1
On 7/10/24 13:06, 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 <tredaelli@redhat.com>
> ---
> 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
> 
> v8 -> v9: Fix make distcheck when --with-version-suffix is specified (as
>           repoted by Ilya). I know AM_DISTCHECK_CONFIGURE_FLAGS is
>           discouraged upstream, but it's also used by GNU m4 for a similar
>           scenario so I guess it's ok to use it (see
>           https://www.gnu.org/software/automake/manual/html_node/Checking-the-Distribution.html)
>           Restore the loop in setup.py.template (as reported by Ilya)
>           Use version suffixalso for libraries (aka test-lib) (as reported by Ilya)
>           Fix a typo in configure (as reported by Ilya)
> ---
>  Makefile.am                            |  3 +++
>  acinclude.m4                           | 13 ++++++++++++
>  configure.ac                           |  1 +
>  include/openvswitch/version.h.in       |  2 +-
>  lib/ovsdb-error.c                      |  2 +-
>  lib/util.c                             |  8 +++++---
>  ovsdb/ovsdb-server.c                   |  3 ++-
>  python/.gitignore                      |  1 +
>  python/automake.mk                     | 22 +++++++++++++-------
>  python/{setup.py => setup.py.template} | 28 +++++++++-----------------
>  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, 64 insertions(+), 41 deletions(-)
>  rename python/{setup.py => setup.py.template} (87%)
> 
> diff --git a/Makefile.am b/Makefile.am
> index e6c90a911..e8f13d76b 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -8,6 +8,8 @@
>  AUTOMAKE_OPTIONS = foreign subdir-objects
>  ACLOCAL_AMFLAGS = -I m4
>  
> +AM_DISTCHECK_CONFIGURE_FLAGS = --with-version-suffix=$(VERSION_SUFFIX)
> +
>  AM_CPPFLAGS = $(SSL_CFLAGS)
>  AM_LDFLAGS = $(SSL_LDFLAGS)
>  AM_LDFLAGS += $(OVS_LDFLAGS)
> @@ -163,6 +165,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..1ace70c92 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 string 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 9ad42b232..56512fc28 100644
> --- a/lib/ovsdb-error.c
> +++ b/lib/ovsdb-error.c
> @@ -146,7 +146,7 @@ ovsdb_internal_error(struct ovsdb_error *inner_error,
>          ds_put_char(&ds, ')');
>      }
>  
> -    ds_put_format(&ds, " (%s %s)", program_name, VERSION);
> +    ds_put_format(&ds, " (%s %s)", program_name, VERSION VERSION_SUFFIX);
>  
>      if (inner_error) {
>          char *s = ovsdb_error_to_string_free(inner_error);
> diff --git a/lib/util.c b/lib/util.c
> index 84e8c4966..5253921b2 100644
> --- a/lib/util.c
> +++ b/lib/util.c
> @@ -618,12 +618,14 @@ ovs_set_program_name(const char *argv0, const char *version)
>      program_name = basename;
>  
>      free(program_version);
> -    if (!strcmp(version, VERSION)) {
> -        program_version = xasprintf("%s (Open vSwitch) "VERSION,
> +    if (!strcmp(version, VERSION VERSION_SUFFIX)) {
> +        program_version = xasprintf("%s (Open vSwitch) "VERSION
> +                                    VERSION_SUFFIX,
>                                      program_name);
>      } else {
>          program_version = xasprintf("%s %s\n"
> -                                    "Open vSwitch Library "VERSION,
> +                                    "Open vSwitch Library "VERSION
> +                                    VERSION_SUFFIX,
>                                      program_name, version);
>      }
>  }
> diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
> index b51fd42fe..a876f8bcf 100644
> --- a/ovsdb/ovsdb-server.c
> +++ b/ovsdb/ovsdb-server.c
> @@ -816,7 +816,8 @@ main(int argc, char *argv[])
>          /* ovsdb-server is usually a long-running process, in which case it
>           * makes plenty of sense to log the version, but --run makes
>           * ovsdb-server more like a command-line tool, so skip it.  */
> -        VLOG_INFO("%s (Open vSwitch) %s", program_name, VERSION);
> +        VLOG_INFO("%s (Open vSwitch) %s", program_name,
> +                  VERSION VERSION_SUFFIX);
>      }
>  
>      unixctl_command_register("exit", "", 0, 0, ovsdb_server_exit, &exiting);
> diff --git a/python/.gitignore b/python/.gitignore
> index 60ace6f05..ad5486af8 100644
> --- a/python/.gitignore
> +++ b/python/.gitignore
> @@ -1,2 +1,3 @@
>  dist/
>  *.egg-info
> +setup.py
> diff --git a/python/automake.mk b/python/automake.mk
> index 84cf2eab5..1e7563156 100644
> --- a/python/automake.mk
> +++ b/python/automake.mk
> @@ -75,25 +75,24 @@ EXTRA_DIST += \
>  EXTRA_DIST += \
>  	python/ovs/compat/sortedcontainers/LICENSE \
>  	python/README.rst \
> -	python/setup.py \
>  	python/test_requirements.txt
>  
>  # C extension support.
>  EXTRA_DIST += python/ovs/_json.c
>  
> -PYFILES = $(ovs_pyfiles) python/ovs/dirs.py $(ovstest_pyfiles) $(ovs_pytests)
> +PYFILES = $(ovs_pyfiles) python/ovs/dirs.py python/setup.py $(ovstest_pyfiles) $(ovs_pytests)
>  
>  EXTRA_DIST += $(PYFILES)
>  PYCOV_CLEAN_FILES += $(PYFILES:.py=.py,cover)
>  
>  FLAKE8_PYFILES += \
> -	$(filter-out python/ovs/compat/% python/ovs/dirs.py,$(PYFILES)) \
> +	$(filter-out python/ovs/compat/% python/ovs/dirs.py python/setup.py,$(PYFILES)) \
>  	python/ovs_build_helpers/__init__.py \
>  	python/ovs_build_helpers/extract_ofp_fields.py \
>  	python/ovs_build_helpers/nroff.py \
>  	python/ovs_build_helpers/soutil.py \
>  	python/ovs/dirs.py.template \
> -	python/setup.py
> +	python/setup.py.template
>  
>  nobase_pkgdata_DATA = $(ovs_pyfiles) $(ovstest_pyfiles)
>  ovs-install-data-local:
> @@ -113,7 +112,7 @@ ovs-install-data-local:
>  	rm python/ovs/dirs.py.tmp
>  
>  .PHONY: python-sdist
> -python-sdist: $(srcdir)/python/ovs/version.py $(ovs_pyfiles) python/ovs/dirs.py
> +python-sdist: $(srcdir)/python/ovs/version.py $(ovs_pyfiles) python/ovs/dirs.py python/setup.py
>  	cd python/ && $(PYTHON3) -m build --sdist
>  
>  .PHONY: pypi-upload
> @@ -129,8 +128,8 @@ ovs-uninstall-local:
>  ALL_LOCAL += $(srcdir)/python/ovs/version.py
>  $(srcdir)/python/ovs/version.py: config.status
>  	$(AM_V_GEN)$(ro_shell) > $(@F).tmp && \
> -	echo 'VERSION = "$(VERSION)"' >> $(@F).tmp && \
> -	if cmp -s $(@F).tmp $@; then touch $@; rm $(@F).tmp; else mv $(@F).tmp $@; fi
> +	echo 'VERSION = "$(VERSION)$(VERSION_SUFFIX)"' >> $(@F).tmp && \
> +	if cmp -s $(@F).tmp $@; then touch $@; else cp $(@F).tmp $@; fi; rm $(@F).tmp
>  
>  ALL_LOCAL += $(srcdir)/python/ovs/dirs.py
>  $(srcdir)/python/ovs/dirs.py: python/ovs/dirs.py.template
> @@ -147,6 +146,15 @@ $(srcdir)/python/ovs/dirs.py: python/ovs/dirs.py.template
>  EXTRA_DIST += python/ovs/dirs.py.template
>  CLEANFILES += python/ovs/dirs.py
>  
> +ALL_LOCAL += $(srcdir)/python/setup.py
> +$(srcdir)/python/setup.py: python/setup.py.template
> +	$(AM_V_GEN)sed \
> +		-e 's,[@]VERSION[@],$(VERSION),g' \
> +		< $? > $@.tmp && \
> +	mv $@.tmp $@

This target needs a dependency on config.status the same as version.py.
Otherwise, if we change the version number in configure.ac, the setup.py
is not getting re-generated.

I think, we need something like this:

diff --git a/python/automake.mk b/python/automake.mk
index 1e7563156..d0523870d 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -147,11 +147,11 @@ EXTRA_DIST += python/ovs/dirs.py.template
 CLEANFILES += python/ovs/dirs.py
 
 ALL_LOCAL += $(srcdir)/python/setup.py
-$(srcdir)/python/setup.py: python/setup.py.template
+$(srcdir)/python/setup.py: python/setup.py.template config.status
        $(AM_V_GEN)sed \
                -e 's,[@]VERSION[@],$(VERSION),g' \
-               < $? > $@.tmp && \
-       mv $@.tmp $@
+               < $(srcdir)/python/setup.py.template > $(@F).tmp && \
+       if cmp -s $(@F).tmp $@; then touch $@; else cp $(@F).tmp $@; fi; rm $(@F).tmp
 EXTRA_DIST += python/setup.py.template
 CLEANFILES += python/setup.py
 
---

If that looks good to you, I can fold it in while applying the patch,
unless some other issue will appear.

What do you think?

Best regards, Ilya Maximets.
Timothy Redaelli July 15, 2024, 8:48 a.m. UTC | #2
On Fri, 12 Jul 2024 18:14:28 +0200
Ilya Maximets <i.maximets@ovn.org> wrote:

> On 7/10/24 13:06, 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 <tredaelli@redhat.com>
> > ---
> > 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
> > 
> > v8 -> v9: Fix make distcheck when --with-version-suffix is specified (as
> >           repoted by Ilya). I know AM_DISTCHECK_CONFIGURE_FLAGS is
> >           discouraged upstream, but it's also used by GNU m4 for a similar
> >           scenario so I guess it's ok to use it (see
> >           https://www.gnu.org/software/automake/manual/html_node/Checking-the-Distribution.html)
> >           Restore the loop in setup.py.template (as reported by Ilya)
> >           Use version suffixalso for libraries (aka test-lib) (as reported by Ilya)
> >           Fix a typo in configure (as reported by Ilya)
> > ---
> >  Makefile.am                            |  3 +++
> >  acinclude.m4                           | 13 ++++++++++++
> >  configure.ac                           |  1 +
> >  include/openvswitch/version.h.in       |  2 +-
> >  lib/ovsdb-error.c                      |  2 +-
> >  lib/util.c                             |  8 +++++---
> >  ovsdb/ovsdb-server.c                   |  3 ++-
> >  python/.gitignore                      |  1 +
> >  python/automake.mk                     | 22 +++++++++++++-------
> >  python/{setup.py => setup.py.template} | 28 +++++++++-----------------
> >  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, 64 insertions(+), 41 deletions(-)
> >  rename python/{setup.py => setup.py.template} (87%)
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index e6c90a911..e8f13d76b 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -8,6 +8,8 @@
> >  AUTOMAKE_OPTIONS = foreign subdir-objects
> >  ACLOCAL_AMFLAGS = -I m4
> >  
> > +AM_DISTCHECK_CONFIGURE_FLAGS = --with-version-suffix=$(VERSION_SUFFIX)
> > +
> >  AM_CPPFLAGS = $(SSL_CFLAGS)
> >  AM_LDFLAGS = $(SSL_LDFLAGS)
> >  AM_LDFLAGS += $(OVS_LDFLAGS)
> > @@ -163,6 +165,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..1ace70c92 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 string 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 9ad42b232..56512fc28 100644
> > --- a/lib/ovsdb-error.c
> > +++ b/lib/ovsdb-error.c
> > @@ -146,7 +146,7 @@ ovsdb_internal_error(struct ovsdb_error *inner_error,
> >          ds_put_char(&ds, ')');
> >      }
> >  
> > -    ds_put_format(&ds, " (%s %s)", program_name, VERSION);
> > +    ds_put_format(&ds, " (%s %s)", program_name, VERSION VERSION_SUFFIX);
> >  
> >      if (inner_error) {
> >          char *s = ovsdb_error_to_string_free(inner_error);
> > diff --git a/lib/util.c b/lib/util.c
> > index 84e8c4966..5253921b2 100644
> > --- a/lib/util.c
> > +++ b/lib/util.c
> > @@ -618,12 +618,14 @@ ovs_set_program_name(const char *argv0, const char *version)
> >      program_name = basename;
> >  
> >      free(program_version);
> > -    if (!strcmp(version, VERSION)) {
> > -        program_version = xasprintf("%s (Open vSwitch) "VERSION,
> > +    if (!strcmp(version, VERSION VERSION_SUFFIX)) {
> > +        program_version = xasprintf("%s (Open vSwitch) "VERSION
> > +                                    VERSION_SUFFIX,
> >                                      program_name);
> >      } else {
> >          program_version = xasprintf("%s %s\n"
> > -                                    "Open vSwitch Library "VERSION,
> > +                                    "Open vSwitch Library "VERSION
> > +                                    VERSION_SUFFIX,
> >                                      program_name, version);
> >      }
> >  }
> > diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
> > index b51fd42fe..a876f8bcf 100644
> > --- a/ovsdb/ovsdb-server.c
> > +++ b/ovsdb/ovsdb-server.c
> > @@ -816,7 +816,8 @@ main(int argc, char *argv[])
> >          /* ovsdb-server is usually a long-running process, in which case it
> >           * makes plenty of sense to log the version, but --run makes
> >           * ovsdb-server more like a command-line tool, so skip it.  */
> > -        VLOG_INFO("%s (Open vSwitch) %s", program_name, VERSION);
> > +        VLOG_INFO("%s (Open vSwitch) %s", program_name,
> > +                  VERSION VERSION_SUFFIX);
> >      }
> >  
> >      unixctl_command_register("exit", "", 0, 0, ovsdb_server_exit, &exiting);
> > diff --git a/python/.gitignore b/python/.gitignore
> > index 60ace6f05..ad5486af8 100644
> > --- a/python/.gitignore
> > +++ b/python/.gitignore
> > @@ -1,2 +1,3 @@
> >  dist/
> >  *.egg-info
> > +setup.py
> > diff --git a/python/automake.mk b/python/automake.mk
> > index 84cf2eab5..1e7563156 100644
> > --- a/python/automake.mk
> > +++ b/python/automake.mk
> > @@ -75,25 +75,24 @@ EXTRA_DIST += \
> >  EXTRA_DIST += \
> >  	python/ovs/compat/sortedcontainers/LICENSE \
> >  	python/README.rst \
> > -	python/setup.py \
> >  	python/test_requirements.txt
> >  
> >  # C extension support.
> >  EXTRA_DIST += python/ovs/_json.c
> >  
> > -PYFILES = $(ovs_pyfiles) python/ovs/dirs.py $(ovstest_pyfiles) $(ovs_pytests)
> > +PYFILES = $(ovs_pyfiles) python/ovs/dirs.py python/setup.py $(ovstest_pyfiles) $(ovs_pytests)
> >  
> >  EXTRA_DIST += $(PYFILES)
> >  PYCOV_CLEAN_FILES += $(PYFILES:.py=.py,cover)
> >  
> >  FLAKE8_PYFILES += \
> > -	$(filter-out python/ovs/compat/% python/ovs/dirs.py,$(PYFILES)) \
> > +	$(filter-out python/ovs/compat/% python/ovs/dirs.py python/setup.py,$(PYFILES)) \
> >  	python/ovs_build_helpers/__init__.py \
> >  	python/ovs_build_helpers/extract_ofp_fields.py \
> >  	python/ovs_build_helpers/nroff.py \
> >  	python/ovs_build_helpers/soutil.py \
> >  	python/ovs/dirs.py.template \
> > -	python/setup.py
> > +	python/setup.py.template
> >  
> >  nobase_pkgdata_DATA = $(ovs_pyfiles) $(ovstest_pyfiles)
> >  ovs-install-data-local:
> > @@ -113,7 +112,7 @@ ovs-install-data-local:
> >  	rm python/ovs/dirs.py.tmp
> >  
> >  .PHONY: python-sdist
> > -python-sdist: $(srcdir)/python/ovs/version.py $(ovs_pyfiles) python/ovs/dirs.py
> > +python-sdist: $(srcdir)/python/ovs/version.py $(ovs_pyfiles) python/ovs/dirs.py python/setup.py
> >  	cd python/ && $(PYTHON3) -m build --sdist
> >  
> >  .PHONY: pypi-upload
> > @@ -129,8 +128,8 @@ ovs-uninstall-local:
> >  ALL_LOCAL += $(srcdir)/python/ovs/version.py
> >  $(srcdir)/python/ovs/version.py: config.status
> >  	$(AM_V_GEN)$(ro_shell) > $(@F).tmp && \
> > -	echo 'VERSION = "$(VERSION)"' >> $(@F).tmp && \
> > -	if cmp -s $(@F).tmp $@; then touch $@; rm $(@F).tmp; else mv $(@F).tmp $@; fi
> > +	echo 'VERSION = "$(VERSION)$(VERSION_SUFFIX)"' >> $(@F).tmp && \
> > +	if cmp -s $(@F).tmp $@; then touch $@; else cp $(@F).tmp $@; fi; rm $(@F).tmp
> >  
> >  ALL_LOCAL += $(srcdir)/python/ovs/dirs.py
> >  $(srcdir)/python/ovs/dirs.py: python/ovs/dirs.py.template
> > @@ -147,6 +146,15 @@ $(srcdir)/python/ovs/dirs.py: python/ovs/dirs.py.template
> >  EXTRA_DIST += python/ovs/dirs.py.template
> >  CLEANFILES += python/ovs/dirs.py
> >  
> > +ALL_LOCAL += $(srcdir)/python/setup.py
> > +$(srcdir)/python/setup.py: python/setup.py.template
> > +	$(AM_V_GEN)sed \
> > +		-e 's,[@]VERSION[@],$(VERSION),g' \
> > +		< $? > $@.tmp && \
> > +	mv $@.tmp $@
> 
> This target needs a dependency on config.status the same as version.py.
> Otherwise, if we change the version number in configure.ac, the setup.py
> is not getting re-generated.
> 
> I think, we need something like this:
> 
> diff --git a/python/automake.mk b/python/automake.mk
> index 1e7563156..d0523870d 100644
> --- a/python/automake.mk
> +++ b/python/automake.mk
> @@ -147,11 +147,11 @@ EXTRA_DIST += python/ovs/dirs.py.template
>  CLEANFILES += python/ovs/dirs.py
>  
>  ALL_LOCAL += $(srcdir)/python/setup.py
> -$(srcdir)/python/setup.py: python/setup.py.template
> +$(srcdir)/python/setup.py: python/setup.py.template config.status
>         $(AM_V_GEN)sed \
>                 -e 's,[@]VERSION[@],$(VERSION),g' \
> -               < $? > $@.tmp && \
> -       mv $@.tmp $@
> +               < $(srcdir)/python/setup.py.template > $(@F).tmp && \
> +       if cmp -s $(@F).tmp $@; then touch $@; else cp $(@F).tmp $@; fi; rm $(@F).tmp
>  EXTRA_DIST += python/setup.py.template
>  CLEANFILES += python/setup.py
>  
> ---
> 
> If that looks good to you, I can fold it in while applying the patch,
> unless some other issue will appear.
> 
> What do you think?

LGTM, thank you

> Best regards, Ilya Maximets.
>
Ilya Maximets July 15, 2024, 3:18 p.m. UTC | #3
On 7/15/24 10:48, Timothy Redaelli wrote:
> On Fri, 12 Jul 2024 18:14:28 +0200
> Ilya Maximets <i.maximets@ovn.org> wrote:
> 
>> On 7/10/24 13:06, 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 <tredaelli@redhat.com>
>>> ---
>>> 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
>>>
>>> v8 -> v9: Fix make distcheck when --with-version-suffix is specified (as
>>>           repoted by Ilya). I know AM_DISTCHECK_CONFIGURE_FLAGS is
>>>           discouraged upstream, but it's also used by GNU m4 for a similar
>>>           scenario so I guess it's ok to use it (see
>>>           https://www.gnu.org/software/automake/manual/html_node/Checking-the-Distribution.html)
>>>           Restore the loop in setup.py.template (as reported by Ilya)
>>>           Use version suffixalso for libraries (aka test-lib) (as reported by Ilya)
>>>           Fix a typo in configure (as reported by Ilya)
>>> ---
>>>  Makefile.am                            |  3 +++
>>>  acinclude.m4                           | 13 ++++++++++++
>>>  configure.ac                           |  1 +
>>>  include/openvswitch/version.h.in       |  2 +-
>>>  lib/ovsdb-error.c                      |  2 +-
>>>  lib/util.c                             |  8 +++++---
>>>  ovsdb/ovsdb-server.c                   |  3 ++-
>>>  python/.gitignore                      |  1 +
>>>  python/automake.mk                     | 22 +++++++++++++-------
>>>  python/{setup.py => setup.py.template} | 28 +++++++++-----------------
>>>  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, 64 insertions(+), 41 deletions(-)
>>>  rename python/{setup.py => setup.py.template} (87%)
>>>

<snip>

>>> @@ -147,6 +146,15 @@ $(srcdir)/python/ovs/dirs.py: python/ovs/dirs.py.template
>>>  EXTRA_DIST += python/ovs/dirs.py.template
>>>  CLEANFILES += python/ovs/dirs.py
>>>  
>>> +ALL_LOCAL += $(srcdir)/python/setup.py
>>> +$(srcdir)/python/setup.py: python/setup.py.template
>>> +	$(AM_V_GEN)sed \
>>> +		-e 's,[@]VERSION[@],$(VERSION),g' \
>>> +		< $? > $@.tmp && \
>>> +	mv $@.tmp $@
>>
>> This target needs a dependency on config.status the same as version.py.
>> Otherwise, if we change the version number in configure.ac, the setup.py
>> is not getting re-generated.
>>
>> I think, we need something like this:
>>
>> diff --git a/python/automake.mk b/python/automake.mk
>> index 1e7563156..d0523870d 100644
>> --- a/python/automake.mk
>> +++ b/python/automake.mk
>> @@ -147,11 +147,11 @@ EXTRA_DIST += python/ovs/dirs.py.template
>>  CLEANFILES += python/ovs/dirs.py
>>  
>>  ALL_LOCAL += $(srcdir)/python/setup.py
>> -$(srcdir)/python/setup.py: python/setup.py.template
>> +$(srcdir)/python/setup.py: python/setup.py.template config.status
>>         $(AM_V_GEN)sed \
>>                 -e 's,[@]VERSION[@],$(VERSION),g' \
>> -               < $? > $@.tmp && \
>> -       mv $@.tmp $@
>> +               < $(srcdir)/python/setup.py.template > $(@F).tmp && \
>> +       if cmp -s $(@F).tmp $@; then touch $@; else cp $(@F).tmp $@; fi; rm $(@F).tmp
>>  EXTRA_DIST += python/setup.py.template
>>  CLEANFILES += python/setup.py
>>  
>> ---
>>
>> If that looks good to you, I can fold it in while applying the patch,
>> unless some other issue will appear.
>>
>> What do you think?
> 
> LGTM, thank you
> 

OK, I folded that in.

>>> diff --git a/Makefile.am b/Makefile.am
>>> index e6c90a911..e8f13d76b 100644
>>> --- a/Makefile.am
>>> +++ b/Makefile.am
>>> @@ -8,6 +8,8 @@
>>>  AUTOMAKE_OPTIONS = foreign subdir-objects
>>>  ACLOCAL_AMFLAGS = -I m4
>>>  
>>> +AM_DISTCHECK_CONFIGURE_FLAGS = --with-version-suffix=$(VERSION_SUFFIX)

I also added quotes around $(VERSION_SUFFIX) above so it doesn't break
distcheck if the suffix contains spaces.  With that applied to main.

Thanks!

Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/Makefile.am b/Makefile.am
index e6c90a911..e8f13d76b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -8,6 +8,8 @@ 
 AUTOMAKE_OPTIONS = foreign subdir-objects
 ACLOCAL_AMFLAGS = -I m4
 
+AM_DISTCHECK_CONFIGURE_FLAGS = --with-version-suffix=$(VERSION_SUFFIX)
+
 AM_CPPFLAGS = $(SSL_CFLAGS)
 AM_LDFLAGS = $(SSL_LDFLAGS)
 AM_LDFLAGS += $(OVS_LDFLAGS)
@@ -163,6 +165,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..1ace70c92 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 string 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 9ad42b232..56512fc28 100644
--- a/lib/ovsdb-error.c
+++ b/lib/ovsdb-error.c
@@ -146,7 +146,7 @@  ovsdb_internal_error(struct ovsdb_error *inner_error,
         ds_put_char(&ds, ')');
     }
 
-    ds_put_format(&ds, " (%s %s)", program_name, VERSION);
+    ds_put_format(&ds, " (%s %s)", program_name, VERSION VERSION_SUFFIX);
 
     if (inner_error) {
         char *s = ovsdb_error_to_string_free(inner_error);
diff --git a/lib/util.c b/lib/util.c
index 84e8c4966..5253921b2 100644
--- a/lib/util.c
+++ b/lib/util.c
@@ -618,12 +618,14 @@  ovs_set_program_name(const char *argv0, const char *version)
     program_name = basename;
 
     free(program_version);
-    if (!strcmp(version, VERSION)) {
-        program_version = xasprintf("%s (Open vSwitch) "VERSION,
+    if (!strcmp(version, VERSION VERSION_SUFFIX)) {
+        program_version = xasprintf("%s (Open vSwitch) "VERSION
+                                    VERSION_SUFFIX,
                                     program_name);
     } else {
         program_version = xasprintf("%s %s\n"
-                                    "Open vSwitch Library "VERSION,
+                                    "Open vSwitch Library "VERSION
+                                    VERSION_SUFFIX,
                                     program_name, version);
     }
 }
diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
index b51fd42fe..a876f8bcf 100644
--- a/ovsdb/ovsdb-server.c
+++ b/ovsdb/ovsdb-server.c
@@ -816,7 +816,8 @@  main(int argc, char *argv[])
         /* ovsdb-server is usually a long-running process, in which case it
          * makes plenty of sense to log the version, but --run makes
          * ovsdb-server more like a command-line tool, so skip it.  */
-        VLOG_INFO("%s (Open vSwitch) %s", program_name, VERSION);
+        VLOG_INFO("%s (Open vSwitch) %s", program_name,
+                  VERSION VERSION_SUFFIX);
     }
 
     unixctl_command_register("exit", "", 0, 0, ovsdb_server_exit, &exiting);
diff --git a/python/.gitignore b/python/.gitignore
index 60ace6f05..ad5486af8 100644
--- a/python/.gitignore
+++ b/python/.gitignore
@@ -1,2 +1,3 @@ 
 dist/
 *.egg-info
+setup.py
diff --git a/python/automake.mk b/python/automake.mk
index 84cf2eab5..1e7563156 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -75,25 +75,24 @@  EXTRA_DIST += \
 EXTRA_DIST += \
 	python/ovs/compat/sortedcontainers/LICENSE \
 	python/README.rst \
-	python/setup.py \
 	python/test_requirements.txt
 
 # C extension support.
 EXTRA_DIST += python/ovs/_json.c
 
-PYFILES = $(ovs_pyfiles) python/ovs/dirs.py $(ovstest_pyfiles) $(ovs_pytests)
+PYFILES = $(ovs_pyfiles) python/ovs/dirs.py python/setup.py $(ovstest_pyfiles) $(ovs_pytests)
 
 EXTRA_DIST += $(PYFILES)
 PYCOV_CLEAN_FILES += $(PYFILES:.py=.py,cover)
 
 FLAKE8_PYFILES += \
-	$(filter-out python/ovs/compat/% python/ovs/dirs.py,$(PYFILES)) \
+	$(filter-out python/ovs/compat/% python/ovs/dirs.py python/setup.py,$(PYFILES)) \
 	python/ovs_build_helpers/__init__.py \
 	python/ovs_build_helpers/extract_ofp_fields.py \
 	python/ovs_build_helpers/nroff.py \
 	python/ovs_build_helpers/soutil.py \
 	python/ovs/dirs.py.template \
-	python/setup.py
+	python/setup.py.template
 
 nobase_pkgdata_DATA = $(ovs_pyfiles) $(ovstest_pyfiles)
 ovs-install-data-local:
@@ -113,7 +112,7 @@  ovs-install-data-local:
 	rm python/ovs/dirs.py.tmp
 
 .PHONY: python-sdist
-python-sdist: $(srcdir)/python/ovs/version.py $(ovs_pyfiles) python/ovs/dirs.py
+python-sdist: $(srcdir)/python/ovs/version.py $(ovs_pyfiles) python/ovs/dirs.py python/setup.py
 	cd python/ && $(PYTHON3) -m build --sdist
 
 .PHONY: pypi-upload
@@ -129,8 +128,8 @@  ovs-uninstall-local:
 ALL_LOCAL += $(srcdir)/python/ovs/version.py
 $(srcdir)/python/ovs/version.py: config.status
 	$(AM_V_GEN)$(ro_shell) > $(@F).tmp && \
-	echo 'VERSION = "$(VERSION)"' >> $(@F).tmp && \
-	if cmp -s $(@F).tmp $@; then touch $@; rm $(@F).tmp; else mv $(@F).tmp $@; fi
+	echo 'VERSION = "$(VERSION)$(VERSION_SUFFIX)"' >> $(@F).tmp && \
+	if cmp -s $(@F).tmp $@; then touch $@; else cp $(@F).tmp $@; fi; rm $(@F).tmp
 
 ALL_LOCAL += $(srcdir)/python/ovs/dirs.py
 $(srcdir)/python/ovs/dirs.py: python/ovs/dirs.py.template
@@ -147,6 +146,15 @@  $(srcdir)/python/ovs/dirs.py: python/ovs/dirs.py.template
 EXTRA_DIST += python/ovs/dirs.py.template
 CLEANFILES += python/ovs/dirs.py
 
+ALL_LOCAL += $(srcdir)/python/setup.py
+$(srcdir)/python/setup.py: python/setup.py.template
+	$(AM_V_GEN)sed \
+		-e 's,[@]VERSION[@],$(VERSION),g' \
+		< $? > $@.tmp && \
+	mv $@.tmp $@
+EXTRA_DIST += python/setup.py.template
+CLEANFILES += python/setup.py
+
 EXTRA_DIST += python/TODO.rst
 
 $(srcdir)/python/ovs/flow/ofp_fields.py: $(srcdir)/build-aux/gen_ofp_field_decoders include/openvswitch/meta-flow.h
diff --git a/python/setup.py b/python/setup.py.template
similarity index 87%
rename from python/setup.py
rename to python/setup.py.template
index bcf832ce9..e7d59f2ca 100644
--- a/python/setup.py
+++ b/python/setup.py.template
@@ -23,24 +23,16 @@  except ImportError:  # Needed for setuptools < 59.0
 
 import setuptools
 
-VERSION = "unknown"
-
-try:
-    # Try to set the version from the generated ovs/version.py
-    exec(open("ovs/version.py").read())
-except IOError:
-    print("Ensure version.py is created by running make python/ovs/version.py",
-          file=sys.stderr)
-    sys.exit(-1)
-
-try:
-    # Try to open generated ovs/dirs.py. However, in this case we
-    # don't need to exec()
-    open("ovs/dirs.py")
-except IOError:
-    print("Ensure dirs.py is created by running make python/ovs/dirs.py",
-          file=sys.stderr)
-    sys.exit(-1)
+VERSION = "@VERSION@"
+
+for x in ("version.py", "dirs.py"):
+    try:
+        # Try to open generated ovs/{version,dirs}.py
+        open(f"ovs/{x}")
+    except IOError:
+        print(f"Ensure {x} is created by running make python/ovs/{x}",
+              file=sys.stderr)
+        sys.exit(-1)
 
 ext_errors = (CCompilerError, ExecError, PlatformError)
 if sys.platform == 'win32':
diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index 94b6d7431..f129bc646 100644
--- a/rhel/openvswitch-fedora.spec.in
+++ b/rhel/openvswitch-fedora.spec.in
@@ -186,6 +186,7 @@  This package provides IPsec tunneling support for OVS tunnels.
         --disable-static \
         --enable-shared \
         --with-pkidir=%{_sharedstatedir}/openvswitch/pki \
+        --with-version-suffix=-%{release} \
         PYTHON3=%{__python3}
 
 build-aux/dpdkstrip.py \
diff --git a/utilities/ovs-dpctl-top.in b/utilities/ovs-dpctl-top.in
index 2c1766eff..ec57eccd6 100755
--- a/utilities/ovs-dpctl-top.in
+++ b/utilities/ovs-dpctl-top.in
@@ -351,7 +351,7 @@  def args_get():
     # None is a special value indicating to read flows from stdin.
     # This handles the case
     #   ovs-dpctl dump-flows | ovs-dpctl-flows.py
-    parser.add_argument("-v", "--version", version="@VERSION@",
+    parser.add_argument("-v", "--version", version="@VERSION@@VERSION_SUFFIX@",
                         action="version", help="show version")
     parser.add_argument("-f", "--flow-file", dest="flowFiles", default=None,
                         action="append",
diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
index 7812a94ee..d162227dc 100644
--- a/utilities/ovs-lib.in
+++ b/utilities/ovs-lib.in
@@ -70,7 +70,7 @@  ovs_ctl () {
     esac
 }
 
-VERSION='@VERSION@'
+VERSION='@VERSION@@VERSION_SUFFIX@'
 
 DAEMON_CWD=/
 
diff --git a/utilities/ovs-parse-backtrace.in b/utilities/ovs-parse-backtrace.in
index f44f05cd1..42f831eed 100755
--- a/utilities/ovs-parse-backtrace.in
+++ b/utilities/ovs-parse-backtrace.in
@@ -51,7 +51,7 @@  def addr2line(binary, addr):
 
 
 def main():
-    parser = optparse.OptionParser(version='@VERSION@',
+    parser = optparse.OptionParser(version='@VERSION@@VERSION_SUFFIX@',
                                    usage="usage: %prog [binary]",
                                    description="""\
 Parses the output of ovs-appctl backtrace producing a more human readable
diff --git a/utilities/ovs-pcap.in b/utilities/ovs-pcap.in
index 6b5f63399..d0ca94788 100755
--- a/utilities/ovs-pcap.in
+++ b/utilities/ovs-pcap.in
@@ -85,7 +85,7 @@  if __name__ == "__main__":
             if key in ['-h', '--help']:
                 usage()
             elif key in ['-V', '--version']:
-                print("ovs-pcap (Open vSwitch) @VERSION@")
+                print("ovs-pcap (Open vSwitch) @VERSION@@VERSION_SUFFIX@")
             else:
                 sys.exit(0)
 
diff --git a/utilities/ovs-pki.in b/utilities/ovs-pki.in
index 3d2ef911c..69060b4ac 100755
--- a/utilities/ovs-pki.in
+++ b/utilities/ovs-pki.in
@@ -189,7 +189,7 @@  EOF
             exit 0
             ;;
         -V|--version)
-            echo "ovs-pki (Open vSwitch) @VERSION@"
+            echo "ovs-pki (Open vSwitch) @VERSION@@VERSION_SUFFIX@"
             exit 0
             ;;
         --di*=*)
diff --git a/utilities/ovs-tcpdump.in b/utilities/ovs-tcpdump.in
index eada803bb..cb46e43ba 100755
--- a/utilities/ovs-tcpdump.in
+++ b/utilities/ovs-tcpdump.in
@@ -47,7 +47,7 @@  try:
     from ovs.fatal_signal import add_hook
 except Exception:
     print("ERROR: Please install the correct Open vSwitch python support")
-    print("       libraries (version @VERSION@).")
+    print("       libraries (version @VERSION@@VERSION_SUFFIX@).")
     print("       Alternatively, check that your PYTHONPATH is pointing to")
     print("       the correct location.")
     sys.exit(1)
@@ -453,7 +453,7 @@  def main():
         if cur in ['-h', '--help']:
             usage()
         elif cur in ['-V', '--version']:
-            print("ovs-tcpdump (Open vSwitch) @VERSION@")
+            print("ovs-tcpdump (Open vSwitch) @VERSION@@VERSION_SUFFIX@")
             sys.exit(0)
         elif cur in ['--db-sock']:
             db_sock = nxt
diff --git a/utilities/ovs-tcpundump.in b/utilities/ovs-tcpundump.in
index ede5448b4..2a1b08332 100755
--- a/utilities/ovs-tcpundump.in
+++ b/utilities/ovs-tcpundump.in
@@ -46,7 +46,7 @@  if __name__ == "__main__":
         if key in ['-h', '--help']:
             usage()
         elif key in ['-V', '--version']:
-            print("ovs-tcpundump (Open vSwitch) @VERSION@")
+            print("ovs-tcpundump (Open vSwitch) @VERSION@@VERSION_SUFFIX@")
             sys.exit(0)
         else:
             sys.exit(0)
diff --git a/utilities/ovs-vlan-test.in b/utilities/ovs-vlan-test.in
index de3ae1686..3c15e2b13 100755
--- a/utilities/ovs-vlan-test.in
+++ b/utilities/ovs-vlan-test.in
@@ -393,7 +393,7 @@  def main():
             usage()
             return 0
         elif key in ['-V', '--version']:
-            print_safe('ovs-vlan-test (Open vSwitch) @VERSION@')
+            print_safe('ovs-vlan-test (Open vSwitch) @VERSION@@VERSION_SUFFIX@')
             return 0
         elif key in ['-s', '--server']:
             server = True
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 95a65fcdc..0352030fe 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -3398,7 +3398,8 @@  bridge_run(void)
 
             vlog_enable_async();
 
-            VLOG_INFO_ONCE("%s (Open vSwitch) %s", program_name, VERSION);
+            VLOG_INFO_ONCE("%s (Open vSwitch) %s", program_name,
+                           VERSION VERSION_SUFFIX);
         }
     }