On Fri, 12 Jul 2024 18:14:28 +0200 Ilya Maximets <[email protected]> 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 <[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? LGTM, thank you > Best regards, Ilya Maximets. > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
