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 <[email protected]>
> ---
> 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' \
> +             < $? > [email protected] && \
> +     mv [email protected] $@

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' \
-               < $? > [email protected] && \
-       mv [email protected] $@
+               < $(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.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to