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

With all the changes discussed, applied.  Thanks, everyone!

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to