On 7/15/22 05:04, Gregory Rose wrote: > > > On 7/14/2022 6:45 PM, Ilya Maximets 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? >> > > Hi Ilya, this patch may introduce a conflict with my series of > patches. Probably best you apply this first and then I'll > rebase and fix up any issues.
Yeah, I knew there will be conflicts. I'll merge these patches manually, one-by-one resolving conflicts along the way. Should not be a huge problem, I have time allocated for this activity. :) But thanks for taking a look! I'm planning to spend a next few hours on merging and testing both patch sets, then I'll push them to master, if things will go well. Will keep you updated. Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
