Re: abi-compliance-checker

From: Peter Eisentraut <peter(at)eisentraut(dot)org>
To: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: abi-compliance-checker
Date: 2023-11-01 11:09:58
Message-ID: f486f888-0f52-596f-e5ea-eedcecc8de66@eisentraut.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Here is an updated version of this patch. It doesn't have any new
functionality, just a rebase and some minor adjustments.

I have split up the one patch into several ones, which could be
considered incrementally, namely:

v3-0001-abidw-option.patch

This adds the abidw meson option, which produces the xml files with the
ABI description. With that, you can then implement a variety of
workflows, such as the abidiff proposed in the later patches, or
something rigged up via CI, or you can just build various versions
locally and compare them. With this patch, you get the files to compare
built automatically and don't have to remember to cover all the
libraries or which options to use.

I think this patch is mostly pretty straightforward and agreeable,
subject to technical review in detail.

TODO: documentation
TODO: Do we want a configure/make variant of this?

v3-0002-Enable-abidw-option-on-Cirrus-CI.patch

This adds the abidw option to some CI tasks. This was mostly used by me
during development to get feedback from other machines and to produce
base files for the subsequent abidiff patch. I'm not sure whether we
need it in isolation (other than for general testing that the option
works at all).

v3-0003-abidiff-option.patch

This adds the abidiff test suite that compares base files previously
produced with the abidw option to the currently built libraries. There
is clearly some uncertainty here whether the produced files are stable
enough, whether we want this particular workflow, what additional
burdens this would create, etc., so I'm not hung up on this right now,
it's mostly a demonstration.

v3-0004-abidiff-support-files.patch

This contains the support files for patch 0003, just split out because
they are bulky and boring.

On 10.06.23 16:17, Peter Eisentraut wrote:
> 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
v3-0001-abidw-option.patch text/plain 5.6 KB
v3-0002-Enable-abidw-option-on-Cirrus-CI.patch text/plain 2.6 KB
v3-0003-abidiff-option.patch text/plain 5.9 KB
v3-0004-abidiff-support-files.patch text/plain 1.1 MB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Aleksander Alekseev 2023-11-01 11:12:30 Re: pg_resetwal tests, logging, and docs update
Previous Message Matthias van de Meent 2023-11-01 10:32:46 Re: btree: implement dynamic prefix truncation (was: Improving btree performance through specializing by key shape, take 2)