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.
