On 6/27/23 16:13, 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).
> ---
>  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                     | 11 ++++++++++-
>  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, 49 insertions(+), 21 deletions(-)
>  rename python/{setup.py => setup.py.template} (95%)
> 
> diff --git a/Makefile.am b/Makefile.am
> index db341504d..793cd10dc 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -160,6 +160,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 ac1eab790..a02f5bb00 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -470,6 +470,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 d05e544b5..3df7156da 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -198,6 +198,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 3fb3a4b40..c1bfda0e6 100644
> --- a/lib/util.c
> +++ b/lib/util.c
> @@ -617,8 +617,9 @@ 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"\n",
> +    if (!strcmp(version, VERSION VERSION_SUFFIX)) {
> +        program_version = xasprintf("%s (Open vSwitch) "VERSION
> +                                    VERSION_SUFFIX"\n",
>                                      program_name);
>      } else {
>          program_version = xasprintf("%s %s\n"
> diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
> index 9bad0c8dd..796906f26 100644
> --- a/ovsdb/ovsdb-server.c
> +++ b/ovsdb/ovsdb-server.c
> @@ -424,7 +424,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 d00911828..f1baa0520 100644
> --- a/python/automake.mk
> +++ b/python/automake.mk
> @@ -124,7 +124,7 @@ 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 && \
> +     echo 'VERSION = "$(VERSION)$(VERSION_SUFFIX)"' >> $(@F).tmp && \
>       if cmp -s $(@F).tmp $@; then touch $@; rm $(@F).tmp; else mv $(@F).tmp 
> $@; fi
> 
>  ALL_LOCAL += $(srcdir)/python/ovs/dirs.py
> @@ -142,6 +142,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] $@
> +EXTRA_DIST += python/setup.py.template
> +CLEANFILES += python/setup.py

A bit higher in this file we're adding setup.py into EXTRA_DIST.
Should we not do that?

Also, FLAKE8_PYFILES includes setup.py, should it be a template instead?

And setup.py is used by python-sdist and pypi-upload.  It should become
a dependency for these targets.

> +
>  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 95%
> rename from python/setup.py
> rename to python/setup.py.template
> index 27684c404..50c68ea75 100644
> --- a/python/setup.py
> +++ b/python/setup.py.template
> @@ -23,19 +23,18 @@ except ImportError:  # Needed for setuptools < 59.0
> 
>  import setuptools
> 
> -VERSION = "unknown"
> +VERSION = "@VERSION@"
> 
>  try:
> -    # Try to set the version from the generated ovs/version.py
> -    exec(open("ovs/version.py").read())
> +    # Try to open generated ovs/version.py
> +    open("ovs/version.py")
>  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()
> +    # Try to open generated ovs/dirs.py
>      open("ovs/dirs.py")
>  except IOError:
>      print("Ensure dirs.py is created by running make python/ovs/dirs.py",

Since checks above are the same now, can we create a loop instead
of duplicating a check?

> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
> index 44899c1ca..952dbe79b 100644
> --- a/rhel/openvswitch-fedora.spec.in
> +++ b/rhel/openvswitch-fedora.spec.in
> @@ -178,6 +178,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 e0ba910f9..c34e5888a 100755
> --- a/utilities/ovs-pki.in
> +++ b/utilities/ovs-pki.in
> @@ -118,7 +118,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 420c11eb8..ea4a07212 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)
> @@ -449,7 +449,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 f5dc59ad0..2e0ef5930 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -3344,7 +3344,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);
>          }
>      }
> 
> --
> 2.41.0
> 
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to