Re: abi-compliance-checker

From: Peter Eisentraut <peter(at)eisentraut(dot)org>
To: Tristan Partin <tristan(at)neon(dot)tech>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Cc: Peter Geoghegan <pg(at)bowt(dot)ie>
Subject: Re: abi-compliance-checker
Date: 2023-06-10 14:17:27
Message-ID: 50e4a403-f1cf-f290-14dd-f4e3785aa719@eisentraut.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

Attachment Content-Type Size
v2-0001-abidiff-tests.patch text/plain 260.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Joe Conway 2023-06-10 15:19:46 Re: Cleaning up threading code
Previous Message Tom Lane 2023-06-10 13:27:38 Re: Views no longer in rangeTabls?