On 7/15/22 07:28, Frode Nordahl wrote: > On Fri, Jul 15, 2022 at 3:45 AM Ilya Maximets <[email protected]> wrote: >> >> 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? > > I knew I was entering dangerous territory when relying on configure to > prepare build artifacts only to run distclean as part of the build > right afterwards, and I apparently missed checking distcheck. > Apologies for leaving this riddle behind, and thank you so much for > taking the time to solve it.
Yeah, that wasn't an easy one, but I learned a lot, for sure. Maybe now I'll be able to fix the dirs.py packaging problem we have: https://patchwork.ozlabs.org/project/openvswitch/patch/98261e51f50ced45e2071e1fd47e419e59f5e8fb.1648230990.git.tredae...@redhat.com/ https://patchwork.ozlabs.org/project/openvswitch/patch/e11a3ef39ac91bbca0074e06d694539db467036d.1648236842.git.tredae...@redhat.com/ At least, now the error looks very familiar. :) > > Your solution makes sense to me and it all builds fine here too, feel > free to fold it into the patch. Will do. Thanks for checking! > >> --- >> .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 >> -- > > Acked-by: Frode Nordahl <[email protected]> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
