On 7/15/22 10:53, Ilya Maximets wrote:
> 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.

Actually, 'git am -3' managed to resolve all the conflicts automatically.
The only thing I changed manually is the reference to the openvswitch.h
from the debian/copyright.in.  I'll look though patches one more time
and run some more tests just to be sure.

> 
> Best regards, Ilya Maximets.

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to