Re: abi-compliance-checker

On 06.06.23 18:52, Tristan Partin wrote:
> It would make sense to me to mark abidiff and abidw as disabler: true.

ok

> +if abidiff.found()
> +  test('libpq.abidiff',
> +       abidiff,
> +       args: [files('libpq.base.abi.xml'), libpq_abi],
> +       suite: 'abidiff',
> +       verbose: true)
> +endif
> 
> With disabler: true, you can drop the conditionals. Disablers tell Meson
> to disable parts of the build[0].

ok

> I also don't think it makes sense to mark the custom_targets as
> build_by_default: true, unless you see value in that. I would just have
> them built when the test is ran.
> 
> However, it might make sense to create an alias_target of all the ABI
> XML files for people that want to interact with the files outside of the
> tests for whatever reason.

Thanks for the feedback.  Attached is a more complete patch.

I have rearranged this a bit.  There are now two build options, called 
abidw and abidiff.  The abidw option produces the xml file, that you 
would then at appropriate times commit into the tree as the base.  The 
abidiff option enables the abidiff tests.  This doesn't actually require 
abidw, since abidiff can compare the binary directly against the 
recorded XML file.  So these options are distinct and nonoverlapping.

Note that in this setup, you still need a conditional around the abidiff 
test() call, because otherwise meson setup will fail if the base file 
doesn't exist (yet), so it would be impossible to bootstrap this system.

The updated patch also includes the base files for all the ecpg 
libraries and the files all have OS and architecture specific names. 
The keep the patch small, I just added a dummy base file for the 
postgres binary and a suppression file that suppresses everything.

There is something weird going on where the cirrus linux/meson job 
doesn't upload the produced abidw artifacts, even though they are 
apparently built, and the equivalent works for the freebsd job.  Maybe 
someone can see something that I'm not seeing there.