| Message ID | 3f84325cb812fc5e28f652cd38af7ea71c812327.1659379739.git.tredaelli@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev] rhel: make the version, displayed to the user, customizable | expand |
| 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 | success | test: success |
Timothy Redaelli <tredaelli@redhat.com> writes: > 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 DISPLAY_VERSION as ./configure environment variable in > order to set an OVS version that should be shown to the user via > ovs-vsctl -V and, so, also on database and on ovs-vsctl show. > > DISPLAY_VERSION 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> > --- Thanks for doing this - this will be useful for distributions that want to have their patch-level suffixes added to the version. Acked-by: Aaron Conole <aconole@redhat.com>
On 8/1/22 20:48, 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. That makes sense to me. See some comments inline. > > This commit adds DISPLAY_VERSION as ./configure environment variable in > order to set an OVS version that should be shown to the user via > ovs-vsctl -V and, so, also on database and on ovs-vsctl show. What about versions printed in a log file? This patch also doesn't cover versions printed by some utilities, especially python-based. It will be confusing to have different versions printed in different places. > > DISPLAY_VERSION is used in Fedora/CentOS/RHEL spec file in order to have > the version be aligned with the downstream one. Allowing users to fully replace the version string doesn't seem like a good approach, and it also doesn't seem to be needed. The patch changes the version for fedora builds to %{version}-%{release}, so it looks like that we only need a version suffix, not the whole version string. This way we can also use the ARG_WITH instead of ARG_VAR. It just looks better, IMO. E.g. --with-version-suffix=%{release}. What do you think? Best regards, Ilya Maximets.
diff --git a/configure.ac b/configure.ac index ef2d75bcb..845953665 100644 --- a/configure.ac +++ b/configure.ac @@ -189,6 +189,7 @@ OVS_CHECK_DPCLS_AUTOVALIDATOR OVS_CHECK_DPIF_AVX512_DEFAULT OVS_CHECK_MFEX_AUTOVALIDATOR OVS_CHECK_AVX512 +OVS_DISPLAY_VERSION AC_ARG_VAR(KARCH, [Kernel Architecture String]) AC_SUBST(KARCH) diff --git a/lib/util.c b/lib/util.c index 1195c7982..9d8873f36 100644 --- a/lib/util.c +++ b/lib/util.c @@ -41,6 +41,9 @@ #ifdef _WIN32 #include <shlwapi.h> #endif +#ifndef DISPLAY_VERSION +#define DISPLAY_VERSION VERSION +#endif VLOG_DEFINE_THIS_MODULE(util); @@ -615,7 +618,7 @@ ovs_set_program_name(const char *argv0, const char *version) free(program_version); if (!strcmp(version, VERSION)) { - program_version = xasprintf("%s (Open vSwitch) "VERSION"\n", + program_version = xasprintf("%s (Open vSwitch) "DISPLAY_VERSION"\n", program_name); } else { program_version = xasprintf("%s %s\n" diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4 index 6eef1fb35..4eb68f2bd 100644 --- a/m4/openvswitch.m4 +++ b/m4/openvswitch.m4 @@ -720,3 +720,11 @@ AC_DEFUN([OVS_CHECK_UNWIND], fi AM_CONDITIONAL([HAVE_UNWIND], [test "$HAVE_UNWIND" = yes]) AC_SUBST([HAVE_UNWIND])]) + +dnl Set a specific OVS version. +AC_DEFUN([OVS_DISPLAY_VERSION], + [AC_ARG_VAR(DISPLAY_VERSION, [Set a specific OVS version]) + if test "$DISPLAY_VERSION"; then + AC_DEFINE_UNQUOTED([DISPLAY_VERSION], ["$DISPLAY_VERSION"], + [Set a specific OVS version]) + fi]) diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in index 67268cb78..da4ca12b1 100644 --- a/rhel/openvswitch-fedora.spec.in +++ b/rhel/openvswitch-fedora.spec.in @@ -176,7 +176,8 @@ This package provides IPsec tunneling support for OVS tunnels. --disable-static \ --enable-shared \ --with-pkidir=%{_sharedstatedir}/openvswitch/pki \ - PYTHON3=%{__python3} + PYTHON3=%{__python3} \ + DISPLAY_VERSION=%{version}-%{release} build-aux/dpdkstrip.py \ %if %{with dpdk}
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 DISPLAY_VERSION as ./configure environment variable in order to set an OVS version that should be shown to the user via ovs-vsctl -V and, so, also on database and on ovs-vsctl show. DISPLAY_VERSION 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> --- configure.ac | 1 + lib/util.c | 5 ++++- m4/openvswitch.m4 | 8 ++++++++ rhel/openvswitch-fedora.spec.in | 3 ++- 4 files changed, 15 insertions(+), 2 deletions(-)