On 7/14/22 17:55, Frode Nordahl wrote: > From: Ilya Maximets <[email protected]> > > Signed-off-by: Ilya Maximets <[email protected]> > Co-authored-by: Frode Nordahl <[email protected]> > Signed-off-by: Frode Nordahl <[email protected]> > --- > .ci/linux-build.sh | 6 ++- > .github/workflows/build-and-test.yml | 12 +++++- > .gitignore | 1 + > Documentation/intro/install/debian.rst | 27 ++++++------ > debian/.gitignore | 21 +++++++++ > debian/automake.mk | 22 +++++++++- > debian/clean | 4 +- > debian/{control => control.in} | 60 +++++++++++++------------- > debian/rules | 6 +++ > 9 files changed, 110 insertions(+), 49 deletions(-) > create mode 100644 debian/.gitignore > rename debian/{control => control.in} (85%) >
<snip> > diff --git a/debian/automake.mk b/debian/automake.mk > index a3c2d7289..2f94c2999 100644 > --- a/debian/automake.mk > +++ b/debian/automake.mk > @@ -3,6 +3,7 @@ EXTRA_DIST += \ > debian/changelog \ > debian/clean \ > debian/control \ > + debian/control.in \ > debian/copyright \ > debian/copyright.in \ > debian/dirs \ > @@ -88,4 +89,23 @@ $(srcdir)/debian/copyright: AUTHORS.rst debian/copyright.in > sed -e '1,/%AUTHORS%/d' $(srcdir)/debian/copyright.in; \ > } > $@ > > -DISTCLEANFILES += debian/copyright > +$(srcdir)/debian/control: debian/control.in config.h > +if DPDK_NETDEV > + sed -e 's/^# DPDK_NETDEV //' \ > + < $(srcdir)/debian/control.in > $(srcdir)/debian/control > +else > + grep -v "^# DPDK_NETDEV" \ > + < $(srcdir)/debian/control.in > $(srcdir)/debian/control > +endif > + > +debian: $(srcdir)/debian/copyright $(srcdir)/debian/control > +.PHONY: debian > + > + > +debian-deb: debian > + make distclean > +if DPDK_NETDEV > + DEB_BUILD_OPTIONS="nocheck parallel=`nproc`" fakeroot debian/rules > binary > +else > + DEB_BUILD_OPTIONS="nocheck parallel=`nproc` nodpdk" fakeroot > debian/rules binary > +endif There are several chicken-and-egg problems with this one patch: - Since targets for control and copyright files are trying to re-write the file in $(srcdir), the distcheck is failing, because during distcheck, the source directory is read-only. - Since control file has a dependency on config.h, it is getting re-built during distcheck. copyright is not, that's why distcheck is not failing without this patch. But we do need a dependency for control, otherwise it will not be re-built after re-configuration. - Since control and copyright are auto-generated files, they technically should not be part of the distribution. If we will remove them from EXTRA_DIST and add to CLEANFILES, they will be cleaned up before packing the source archive and that fixes the distcheck, IIRC (It's been a few hours of experiments, so I don't really remember if that fixed the distcheck). - However, 'debian/rules binary' requires the source directory to not be configured, so the distclean. But distclean removes generated control and copyright. We could copy them, distclean and copy back, but... - 'make distcean' is really not enough. Most of the information about the previous build of debian packages is preserved, so some old directories are getting re-used all the time and packages are not really what they are supposed to be. We have to call 'debian/rules clean', but that will execute distclean and fail on removed control file... - If we'll add control and copyright back to the distribution, but will allow their generation in the build directory instead of a source directory, that helps with distcheck a bit, but fails at distcleancheck, because files are not cleaned. - And you need dpdk-dev already installed for ./configure --with-dpdk=shared to succeed. So, after lots of experiments the only solution that appears to work seems to be following: - Remove control and copyright from the distribution. - Add them to CLEANFILES - Create files in build directory, not source. - In the debian-deb target, call distclean first, then re-generate both control and copyright, then call 'debian/rules clean'. It won't call distclean again, because the directory is not configured after the first distclean. - Install libdpdk-dev beforehand in GHA. - Add the builddir sanity check as 'make debian-deb' doesn't make a lot of sense if not building from the source directory. With that schema, by the time we're calling 'debian/rules binary' we have all files in place. And if they are getting generated during distcheck, it will be possible to create them and they will be correctly cleaned up in the process. Also, we're always performing a clean build, so no need for the documentation note about 'debian/rules clean'. And I got a green build in GHA with it: https://github.com/igsilya/ovs/actions/runs/2674107164 See the diff below. If that make sense (and if that will still make sense for me in the morning :) ), I can fold the diff in before applying the patch. What do you think? --- .ci/linux-build.sh | 3 -- .github/workflows/build-and-test.yml | 3 ++ Documentation/intro/install/debian.rst | 11 ------ debian/automake.mk | 47 +++++++++++++++++++------- 4 files changed, 37 insertions(+), 27 deletions(-) diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh index 8dcfbdcea..b7d93f36b 100755 --- a/.ci/linux-build.sh +++ b/.ci/linux-build.sh @@ -204,9 +204,6 @@ function build_ovs() } if [ "$DEB_PACKAGE" ]; then - if [ -n "$DPDK" ]; then - DPDK=no - fi ./boot.sh && ./configure --with-dpdk=$DPDK && make debian mk-build-deps --install --root-cmd sudo --remove debian/control dpkg-checkbuilddeps diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 419ef0def..4c84b3a96 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -242,6 +242,9 @@ jobs: run: sudo apt update || true - name: install dependencies for debian packages run: sudo apt install -y ${{ env.deb_dependencies }} + - name: install dpdk-dev + if: matrix.dpdk != 'no' + run: sudo apt install -y libdpdk-dev - name: prepare run: ./.ci/linux-prepare.sh diff --git a/Documentation/intro/install/debian.rst b/Documentation/intro/install/debian.rst index 888b17723..6d2687830 100644 --- a/Documentation/intro/install/debian.rst +++ b/Documentation/intro/install/debian.rst @@ -81,17 +81,6 @@ install some dependencies, it will tell you which ones. $ make debian-deb -.. note:: - - There are a few pitfalls in the Debian packaging building system so that, - occasionally, you may find that in a tree that you have using for a while, - the build command above exits immediately without actually building anything. - To fix the problem, run:: - - $ fakeroot debian/rules clean - - or start over from a fresh copy of the source tree. - 5. The generated .deb files will be in the parent directory of the Open vSwitch source distribution. diff --git a/debian/automake.mk b/debian/automake.mk index 2f94c2999..7b2afafae 100644 --- a/debian/automake.mk +++ b/debian/automake.mk @@ -2,9 +2,7 @@ EXTRA_DIST += \ debian/README.Debian \ debian/changelog \ debian/clean \ - debian/control \ debian/control.in \ - debian/copyright \ debian/copyright.in \ debian/dirs \ debian/gbp.conf \ @@ -81,31 +79,54 @@ check-debian-changelog-version: ALL_LOCAL += check-debian-changelog-version DIST_HOOKS += check-debian-changelog-version -$(srcdir)/debian/copyright: AUTHORS.rst debian/copyright.in + +update_deb_copyright = \ $(AM_V_GEN) \ { sed -n -e '/%AUTHORS%/q' -e p < $(srcdir)/debian/copyright.in; \ tail -n +28 $(srcdir)/AUTHORS.rst | sed '1,/^$$/d' | \ sed -n -e '/^$$/q' -e 's/^/ /p'; \ sed -e '1,/%AUTHORS%/d' $(srcdir)/debian/copyright.in; \ - } > $@ + } > debian/copyright + +debian/copyright: AUTHORS.rst debian/copyright.in + $(update_deb_copyright) + +CLEANFILES += debian/copyright + -$(srcdir)/debian/control: debian/control.in config.h if DPDK_NETDEV - sed -e 's/^# DPDK_NETDEV //' \ - < $(srcdir)/debian/control.in > $(srcdir)/debian/control +update_deb_control = \ + $(AM_V_GEN) sed -e 's/^\# DPDK_NETDEV //' \ + < $(srcdir)/debian/control.in > debian/control else - grep -v "^# DPDK_NETDEV" \ - < $(srcdir)/debian/control.in > $(srcdir)/debian/control +update_deb_control = \ + $(AM_V_GEN) grep -v '^\# DPDK_NETDEV' \ + < $(srcdir)/debian/control.in > debian/control endif -debian: $(srcdir)/debian/copyright $(srcdir)/debian/control +debian/control: $(srcdir)/debian/control.in Makefile + $(update_deb_control) + +CLEANFILES += debian/control + + +debian: debian/copyright debian/control .PHONY: debian debian-deb: debian - make distclean + @if test X"$(srcdir)" != X"$(top_builddir)"; then \ + echo "Debian packages should be built from $(abs_srcdir)/"; \ + exit 1; \ + fi + $(MAKE) distclean + $(update_deb_copyright) + $(update_deb_control) + $(AM_V_GEN) fakeroot debian/rules clean if DPDK_NETDEV - DEB_BUILD_OPTIONS="nocheck parallel=`nproc`" fakeroot debian/rules binary + $(AM_V_GEN) DEB_BUILD_OPTIONS="nocheck parallel=`nproc`" \ + fakeroot debian/rules binary else - DEB_BUILD_OPTIONS="nocheck parallel=`nproc` nodpdk" fakeroot debian/rules binary + $(AM_V_GEN) DEB_BUILD_OPTIONS="nocheck parallel=`nproc` nodpdk" \ + fakeroot debian/rules binary endif -- _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
