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.
Your solution makes sense to me and it all builds fine here too, feel
free to fold it into the patch.
> ---
> .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]>
--
Frode Nordahl
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
