Re: [dpdk-dev] [PATCHv2] Remove validate-abi.sh from tree

[Thomas Monjalon <thomas@monjalon.net>]

10/04/2020 08:26, Ray Kinsella:
> On 09/04/2020 17:51, Thomas Monjalon wrote:
> > 09/04/2020 18:29, Ray Kinsella:
> >> On 09/04/2020 16:18, Thomas Monjalon wrote:
> >>> 09/04/2020 16:52, Ray Kinsella:
> >>>> On 09/04/2020 11:59, Thomas Monjalon wrote:
> >>>>> 09/04/2020 12:45, Ray Kinsella:
> >>>>>> On 09/04/2020 11:43, Bruce Richardson wrote:
> >>>>>>> On Thu, Apr 09, 2020 at 06:39:54AM -0400, Neil Horman wrote:
> >>>>>>>> On Thu, Apr 09, 2020 at 08:57:34AM +0100, Ray Kinsella wrote:
> >>>>>>>>> On 08/04/2020 20:56, Neil Horman wrote:
> >>>>>>>>>> +The syntax of the ``check-abi.sh`` utility is::
> >>>>>>>>>> +
> >>>>>>>>>> +   ./devtools/check-abi.sh <refdir> <newdir>
> >>>>>>>>>
> >>>>>>>>> (from v1 feedback)
> >>>>>>>>> Could we simplify this all greatly, by telling people to use the meson/ninja build,
> >>>>>>>>> so they get this checking out of the box, without all the headache below?
> >>>>>>>>>
> >>>>>>>> I think bruce noted that was never merged, correct?
> >>>>>>>>
> >>>>>>> Yep, correct. :-(
> >>>>>>
> >>>>>> apologies, was there a reason?
> >>>>>
> >>>>> Because build tool job is building, not checking.
> >>>>> It would be wrong to make (slow) checks mandatory in all builds.
> >>>>>
> >>>>> The need is to enforce checking ABI.
> >>>>> The result is already published by Travis in patchwork and in an
> >>>>> email to the author I believe.
> >>>>> Not checking email and patchwork is not a good excuse.
> >>>>>
> >>>>> Patchwork must be a mandatory read for everybody for all checks
> >>>>> in general. Let's not give up on general CI workflow.
> >>>>>
> >>>>
> >>>> Thomas 
> >>>>
> >>>> You are trying to solve two problems at once; CI tooling and ABI.
> >>>> Let's try to solve one at a time.
> >>>
> >>> No, you want to mix two problems in a single tool :-)
> >>>
> >>>
> >>>> 1. The ABI check, will make the build _marginally_ slower.
> >>>> You _should_ only need to rebuild the changes between A and B.
> >>>
> >>> Not so marginal.
> >>> A re-build takes less than a second. A mandatory check takes 10 secs
> >>> on my machine.
> >>>
> >>>
> >>>> 2. The meson/ninja are an order of magnitude faster than GNU Make. 
> >>>> We can afford this check.
> >>>
> >>> I am doing such build 10 times (each target) per patch.
> >>> But that's not the main issue.
> >>>
> >>>
> >>>> 3. If we want to lessen the ABI burden and send the correct message.
> >>>> It should be a build blocker, contributors need to hear the message loud and clear.
> >>>
> >>> The developer needs to get or build/save the ABI reference.
> >>> Making such ABI reference for each target is not so obvious:
> >>>   - all symbols must be enabled (dependencies)
> >>>   - some fixes may be needed for some compilers
> >>>
> >>>
> >>>> Most important people _consuming_ DPDK will never see this message.
> >>>> Only people _changing_ the ABI will see it - the people we want to hear the message loud and clear.
> >>>
> >>> No, they will have issue in DPDK compilation if something in the check
> >>> goes wrong. We should not bother end users with internal checks.
> >>>
> >>>
> >>> The message is 
> >>> a) run the check by
> >>>   1) setting DPDK_ABI_REF_VERSION on command line or in devel.config file
> >>>   2) running devtools/test-build.sh or devtools/test-meson-builds.sh
> >>> b) check what Travis is reporting in
> >>>   - email to you
> >>>   - patchwork reports
> >>>
> >>> I think Travis report is convenient to use.
> >>> The local check is integrated in build scripts
> >>> but cannot be run by default because of the reasons above.
> >>
> >> Thomas the reality on this is that people have a tendency to filter
> >> this messages into an email folder and don't always see them. 
> >>
> >> My 2c is that this will always be a struggle unless we find a way
> >> to make it un-ignore-able.
> >> Hence my build-wiring suggestion. 
> > 
> > My other concern is that we will have the same issue with all checks
> > done in a CI.
> > I think the right approach is to enforce people checking CI results.
> 
> Beyond asking maintainers to check, how would we enforce?

Because of the reason below...

> > They will be used to check CI in patchwork because the patches will
> > be blocked.