On 7/15/24 10:48, Timothy Redaelli wrote: > 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%) >>>
<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' \ >>> + < $? > [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 > 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. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
