abi-compliance-checker

Peter Geoghegan <pg@bowt.ie> writes:

Attached is a html report that was generated by a tool called
abi-compliance-checker/abi-dumper [1][2] (by using
"abi-compliance-checker -l libTest ... ") . I deliberately introduced
2 ABI compatibility issues affecting postgres, just to see what the
tool had to say about it.

This seems pretty cool. I agree that we're in dire need of some
tool of this sort for checking back-branch patches. I wonder
though if it'll have false-positive problems. Have you tried it
on live rather than mocked-up cases, for instance 13.0 vs 13.11?

regards, tom lane

On Sun, May 28, 2023 at 6:48 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

This seems pretty cool. I agree that we're in dire need of some
tool of this sort for checking back-branch patches. I wonder
though if it'll have false-positive problems. Have you tried it
on live rather than mocked-up cases, for instance 13.0 vs 13.11?

I tried comparing REL_11_0 to REL_11_20. Attached is the report for that.

I don't have time to study this in detail today, but the report seems
to have a plausible variety of issues. I noticed that it warns about
the breaking signature change to _bt_pagedel(). This is the
theoretical ABI break that I mentioned in the commit message of commit
b0229f26. This is arguably a false positive, since the tool doesn't
understand my reasoning about why it's okay in this particular
instance (namely "any extension that called that function was already
severely broken"). Obviously the tool couldn't possibly be expected to
know better in these kinds of situations, though, so whether or not it
counts as a false positive is just semantics.

Fortunately, there aren't very many issues in the report. Certainly
not enough for false positives (however you define them) to be of
great concern. This is nearly 5 years worth of ABI issues.

--
Peter Geoghegan

Peter Geoghegan <pg@bowt.ie> writes:

I tried comparing REL_11_0 to REL_11_20. Attached is the report for that.

Nice!

I don't have time to study this in detail today, but the report seems
to have a plausible variety of issues. I noticed that it warns about
the breaking signature change to _bt_pagedel(). This is the
theoretical ABI break that I mentioned in the commit message of commit
b0229f26. This is arguably a false positive, since the tool doesn't
understand my reasoning about why it's okay in this particular
instance (namely "any extension that called that function was already
severely broken"). Obviously the tool couldn't possibly be expected to
know better in these kinds of situations, though, so whether or not it
counts as a false positive is just semantics.

Agreed. The point of such a tool is to make sure that we notice
any ABI breaks; it can't be expected to make engineering judgments
about whether the alternatives are worse. For instance, I see that
it noticed commit 1f28ec6be (Rename rbtree.c functions to use "rbt"
prefix not "rb" prefix), which is not something we would have done
of our own choosing, but on balance it seemed the best solution.

I gather it'd catch things like NodeTag enum assignments changing,
which is something we really need to have a check for.

(Which reminds me that I forgot to turn on the ad-hoc check for
that in gen_node_support.pl. I'll go do that, but it'd be better
to have a more general-purpose solution.)

regards, tom lane

I wrote:

(Which reminds me that I forgot to turn on the ad-hoc check for
that in gen_node_support.pl. I'll go do that, but it'd be better
to have a more general-purpose solution.)

Oh, scratch that, it's not supposed to happen until we make the
v16 branch. It'd still be better to not need it.

regards, tom lane

On Sun, May 28, 2023 at 8:37 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

I gather it'd catch things like NodeTag enum assignments changing,
which is something we really need to have a check for.

Right. Any ABI break that involves machine-generated translation units
seems particularly prone to being overlooked. Just eyeballing code
(and perhaps double-checking struct layout using pahole) seems
inadequate.

I'll try to come up with a standard abi-compliance-checker Postgres
workflow once I'm back from pgCon. It already looks like
abi-compliance-checker is capable of taking most of the guesswork out
of ABI compatibility in stable releases, without any real downside,
which is encouraging. I have spent very little time on this, so it's
quite possible that some detail or other was overlooked.

--
Peter Geoghegan

On Sun, May 28, 2023 at 9:34 AM Peter Geoghegan <pg@bowt.ie> wrote:

I'll try to come up with a standard abi-compliance-checker Postgres
workflow once I'm back from pgCon.

Ideally, we'd be able to produce reports that cover an entire stable
release branch at once, including details about how things changed
over time. It turns out that there is a tool from the same family of
tools as abi-compliance-checker, that can do just that:

https://github.com/lvc/abi-tracker

There is an abi-tracker example report, here:

https://abi-laboratory.pro/?view=timeline&amp;l=glib

It's exactly the same presentation as the report I posted recently,
once you drill down. That seems ideal.

--
Peter Geoghegan

On 27.05.23 02:52, Peter Geoghegan wrote:

Attached is a html report that was generated by a tool called
abi-compliance-checker/abi-dumper [1][2] (by using
"abi-compliance-checker -l libTest ... ") .

I have been using the libabigail library/set of tools (abidiff, abidw)
for this. I was not familiar with the tool you used. The nice thing
about abidiff is that it gives you text output and a meaningful exit
status, so you can make it part of the build or test process. You can
also write suppression files to silence specific warnings.

I think the way to use this would be to compute the ABI for the .0
release (or rc1 or something like that) and commit it into the tree.
And then compute the current ABI and compare that against the recorded
base ABI.

Here is the workflow:

# build REL_11_0
abidw src/backend/postgres > src/tools/postgres-abi-REL_11_0.xml
# build REL_11_20
abidw src/backend/postgres > src/tools/postgres-abi.xml
abidiff --no-added-syms src/tools/postgres-abi-REL_11_0.xml
src/tools/postgres-abi.xml

This prints

Functions changes summary: 0 Removed, 0 Changed, 0 Added function
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
Function symbols changes summary: 14 Removed, 0 Added (85 filtered out)
function symbols not referenced by debug info
Variable symbols changes summary: 1 Removed, 0 Added (3 filtered out)
variable symbols not referenced by debug info

14 Removed function symbols not referenced by debug info:

[D] RelationHasUnloggedIndex
[D] assign_nestloop_param_placeholdervar
[D] assign_nestloop_param_var
[D] logicalrep_typmap_gettypname
[D] logicalrep_typmap_update
[D] pqsignal_no_restart
[D] rb_begin_iterate
[D] rb_create
[D] rb_delete
[D] rb_find
[D] rb_insert
[D] rb_iterate
[D] rb_leftmost
[D] transformCreateSchemaStmt

1 Removed variable symbol not referenced by debug info:

[D] wrconn

This appears to be similar to what your tool produced, but I haven't
checked it in detail.

On 30.05.23 06:32, Peter Eisentraut wrote:

I think the way to use this would be to compute the ABI for the .0
release (or rc1 or something like that) and commit it into the tree. And
then compute the current ABI and compare that against the recorded base
ABI.

Here is the workflow:

# build REL_11_0
abidw src/backend/postgres > src/tools/postgres-abi-REL_11_0.xml
# build REL_11_20
abidw src/backend/postgres > src/tools/postgres-abi.xml
abidiff --no-added-syms src/tools/postgres-abi-REL_11_0.xml
src/tools/postgres-abi.xml

Here is a demo patch that implements this.

Right now, I have only added support for libpq and postgres. For
completeness, the ecpg libraries should be covered as well.

(Unlike the above example, I did not use src/tools/ but each component's
own subdirectory.)

The patch as currently written will actually fail the tests because it
contains only one base ABI file to compare against, but the build_32
task on cirrus will of course produce a different ABI. But I left it
for now to able to see the results. Eventually, the base ABI file names
should include something from host_system.cpu_family().

Also, I commented out the abidiff test for postgres, because the base
file is 8 MB and I don't want to send that around.

Various findings while playing with these tools:

* Different Linux distributions produce slightly different ABI reports.
In some cases, symbols like 'optarg@GLIBC_2.17' leak out.

* PostgreSQL compilation options affect the exposed ABI. This is
perhaps expected to some degree, but there are some curious details.

* For example, --enable-cassert exposes additional symbols, and it's
maybe not impossible for those to leak into an extension.

* Also, --with-openssl actually *removes* symbols from the ABI (such as
pg_md5_init).

So it's probably not sensible to try to get some universal ABI
definition that works everywhere. Instead, I think it would be better
to get one specific case working, which would be the one tested on the
cirrus linux tasks and/or some equivalent buildfarm machine.

+abidiff = find_program('abidiff', native: false, required: false)
+abidw = find_program('abidw', native: false, required: false)
+
+abidw_flags = [
+  '--drop-undefined-syms',
+  '--no-architecture',
+  '--no-comp-dir-path',
+  '--no-elf-needed',
+  '--no-show-locs',
+  '--type-id-style', 'hash',
+]
+abidw_cmd = [abidw, abidw_flags, '--out-file', '@OUTPUT@', '@INPUT@']

It would make sense to me to mark abidiff and abidw as disabler: true.

+if abidw.found()
+  libpq_abi = custom_target('libpq.abi.xml',
+                            input: libpq_so,
+                            output: 'libpq.abi.xml',
+                            command: abidw_cmd,
+                            build_by_default: true)
+endif
+
+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]https://mesonbuild.com/Reference-manual_returned_disabler.html.

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.

[0]: https://mesonbuild.com/Reference-manual_returned_disabler.html

--
Tristan Partin
Neon (https://neon.tech)

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.

Hi,

On 2023-06-06 18:30:38 +0200, Peter Eisentraut wrote:

On 30.05.23 06:32, Peter Eisentraut wrote:

I think the way to use this would be to compute the ABI for the .0
release (or rc1 or something like that) and commit it into the tree. And
then compute the current ABI and compare that against the recorded base
ABI.

Here is the workflow:

# build REL_11_0
abidw src/backend/postgres > src/tools/postgres-abi-REL_11_0.xml
# build REL_11_20
abidw src/backend/postgres > src/tools/postgres-abi.xml
abidiff --no-added-syms src/tools/postgres-abi-REL_11_0.xml
src/tools/postgres-abi.xml

Here is a demo patch that implements this.

Right now, I have only added support for libpq and postgres. For
completeness, the ecpg libraries should be covered as well.

I think plpgsql would also be good to include, due to things like plpgsql
debuggers.

* Different Linux distributions produce slightly different ABI reports. In
some cases, symbols like 'optarg@GLIBC_2.17' leak out.

Hm, that's somewhat annoying.

* PostgreSQL compilation options affect the exposed ABI. This is perhaps
expected to some degree, but there are some curious details.

* For example, --enable-cassert exposes additional symbols, and it's maybe
not impossible for those to leak into an extension.

They *definitely* leak into extensions. A single Assert() in an extension or
use of an inline function or macro with an Assertion suffices to end up with a
reference to ExceptionalCondition.

diff --git a/src/interfaces/libpq/libpq.base.abi.xml b/src/interfaces/libpq/libpq.base.abi.xml
new file mode 100644
index 0000000000..691bf192af
--- /dev/null
+++ b/src/interfaces/libpq/libpq.base.abi.xml
@@ -0,0 +1,2634 @@
+<abi-corpus path='src/interfaces/libpq/libpq.so.5.16' soname='libpq.so.5'>
+  <elf-function-symbols>
+    <elf-symbol name='PQbackendPID' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+    <elf-symbol name='PQbinaryTuples' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+    <elf-symbol name='PQcancel' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
[...]
+    <elf-symbol name='termPQExpBuffer' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+  </elf-function-symbols>

This seems somewhat painful in its verbosity. We also effectively already have
it in the tree, in src/interfaces/libpq/exports.txt. But I guess that's
somewhat inevitable :/

It sounds we are planning to mostly rely on CI for this, perhaps we should
rely on an artifact from a prior build for a major version + specific task,
instead of committing this to source? That'd automatically take care of
differences in ABI across different platforms etc.

If we want to commit something to the tree, I think we need a fairly
complicated "fingerprint" to avoid false positives. OS, OS version, configure
options, compiler, compiler version at least?

+  <abi-instr version='1.0' address-size='64' path='../src/common/encnames.c' language='LANG_C99'>
+    <array-type-def dimensions='1' type-id='c8dedbef' size-in-bits='5376' id='752c85d9'>
+      <subrange length='42' type-id='7359adad' id='cb7c937f'/>
+    </array-type-def>
+    <array-type-def dimensions='1' type-id='c8dedbef' size-in-bits='infinite' id='ac835593'>
+      <subrange length='infinite' id='031f2035'/>
+    </array-type-def>
+    <array-type-def dimensions='1' type-id='56ef96d7' size-in-bits='5376' id='728d2ee1'>
+      <subrange length='42' type-id='7359adad' id='cb7c937f'/>
+    </array-type-def>
+    <array-type-def dimensions='1' type-id='56ef96d7' size-in-bits='infinite' id='a01b33bb'>
+      <subrange length='infinite' id='031f2035'/>
+    </array-type-def>
+    <typedef-decl name='pg_enc2name' type-id='79f06fd8' id='7a4268c7'/>
+    <class-decl name='pg_enc2name' size-in-bits='128' is-struct='yes' visibility='default' id='79f06fd8'>
+      <data-member access='public' layout-offset-in-bits='0'>
+        <var-decl name='name' type-id='80f4b756' visibility='default'/>
+      </data-member>
+      <data-member access='public' layout-offset-in-bits='64'>
+        <var-decl name='encoding' type-id='66325df6' visibility='default'/>
+      </data-member>
+    </class-decl>
+    <typedef-decl name='pg_enc' type-id='ea65169a' id='66325df6'/>
+    <enum-decl name='pg_enc' id='ea65169a'>
+      <underlying-type type-id='9cac1fee'/>

Hm - why is all of this stuff even ending up in the external ABI? It should
all be internal, unless I am missing something?

I might be looking the wrong way, but to me it sure looks like none of that
ends up being externally visible?

Greetings,

Andres Freund

Hi,

On 2023-06-10 12:48:46 -0700, Andres Freund wrote:

+    <typedef-decl name='pg_enc' type-id='ea65169a' id='66325df6'/>
+    <enum-decl name='pg_enc' id='ea65169a'>
+      <underlying-type type-id='9cac1fee'/>

Hm - why is all of this stuff even ending up in the external ABI? It should
all be internal, unless I am missing something?

I might be looking the wrong way, but to me it sure looks like none of that
ends up being externally visible?

Looks like we ought to add --exported-interfaces-only?

That still seems to include things that shouldn't be there, but much
less. E.g.:

<class-decl name='AddrInfo' size-in-bits='1152' is-struct='yes' naming-typedef-id='79c324ab' visibility='default' id='0b3a01e2'>
<data-member access='public' layout-offset-in-bits='0'>
<var-decl name='family' type-id='95e97e5e' visibility='default'/>
</data-member>
<data-member access='public' layout-offset-in-bits='64'>
<var-decl name='addr' type-id='8c37a12f' visibility='default'/>
</data-member>
</class-decl>

and things outside of our control:

<class-decl name='_IO_FILE' size-in-bits='1728' is-struct='yes' visibility='default' id='ec1ed955'>
<data-member access='public' layout-offset-in-bits='0'>
<var-decl name='_flags' type-id='95e97e5e' visibility='default'/>
</data-member>

I guess the latter would have to be suppressed via suppression file. But I
don't understand why things like AddrInfo ends up being included...

I tried using --header-file with --drop-private-types. But that ends up
dropping all enum definitions for some reason.

Independently, I'm a bit confused as to why we export pgresStatus in
exports.txt - I don't see any reason for that. Looks like it might be leftover
from before fa0f24165c0?

We're also a bit schizophrenic about where we install pqexpbuffer.h -
includedir_internal. But at the same time we export all the symbols?

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:

Independently, I'm a bit confused as to why we export pgresStatus in
exports.txt - I don't see any reason for that. Looks like it might be leftover
from before fa0f24165c0?

It looks like before fa0f24165, the *only* way to convert ExecStatusType
to text was to access that array directly. That commit invented the
wrapper function PQresStatus(), but at that point our docs were so poor
that there wasn't any good way to mark use of the array as deprecated.
A bit later, 9ceb5d8a7 moved the array declaration to libpq-int.h
(without any discussion in the commit message, but maybe there was
some on-list).

Maybe there's still application code out there using it, I dunno.
What I do know is that removing the exports.txt entry will provoke
squawks from distros' ABI checkers.

regards, tom lane

On Sat Jun 10, 2023 at 9:17 AM CDT, Peter Eisentraut wrote:

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.

Could you speak more to the workflow you see with managing the checked
in diff files?

At my previous job, I had tried to do something similar with regard to
making sure we didn't break ABI[0]https://github.com/hse-project/hse/blob/master/.github/workflows/abicheck.yaml, but I took a different approach
where instead of hooking into the Meson test infrastructure, I used a CI
job where I checked out the previous major version of the code and the
current version of the code, built both, and checked the built binaries.
The benefit of this workflow is that you don't check anything into the
source repo.

I think the same approach might be better here, but instead of writing
it all into the CI file like I did, use a perl script. Then once you
have the perl script, it could be possible to then hook into the Meson
test infrastructure.

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.

Nothing obvious is wrong to me. Was the failure maybe just a fluke?

[0]: https://github.com/hse-project/hse/blob/master/.github/workflows/abicheck.yaml

--
Tristan Partin
Neon (https://neon.tech)

On Mon Jun 12, 2023 at 10:10 AM CDT, Tristan Partin wrote:

On Sat Jun 10, 2023 at 9:17 AM CDT, Peter Eisentraut wrote:

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.

Could you speak more to the workflow you see with managing the checked
in diff files?

Just saw your other email which talks about the workflow.

--
Tristan Partin
Neon (https://neon.tech)

On 10.06.23 22:24, Andres Freund wrote:

Looks like we ought to add --exported-interfaces-only?

Btw., this option requires libabigail 2.1, which isn't available
everywhere yet. For example, Debian oldstable (used on Cirrus) doesn't
have it yet. So I'll leave this patch set as is for now. If it turns
out that this is the right option and we want to proceed with this patch
set, we might need to think about a version check or something.

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 Wed, 1 Nov 2023 at 16:43, Peter Eisentraut <peter@eisentraut.org> wrote:

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.

One of the test has failed in cfbot at [1]https://cirrus-ci.com/task/5961614579466240 with:
abi-compliance-checker
[12:04:10.537] The output from the failed tests:
[12:04:10.537]
[12:04:10.537] 129/282 postgresql:abidiff / plpgsql.abidiff FAIL 1.25s
(exit status 4)
[12:04:10.537]
[12:04:10.537] --- command ---
[12:04:10.537] 12:03:00 /usr/bin/abidiff
/tmp/cirrus-ci-build/build/../src/pl/plpgsql/src/plpgsql.x86_64-linux.abi.xml
src/pl/plpgsql/src/plpgsql.so
[12:04:10.537] --- Listing only the last 100 lines from a long log. ---
[12:04:10.537] 'NodeTag::T_RoleSpec' from value '66' to '67' at nodes.h:26:1
[12:04:10.537] 'NodeTag::T_FuncCall' from value '67' to '68' at nodes.h:26:1
[12:04:10.537] 'NodeTag::T_A_Star' from value '68' to '69' at nodes.h:26:1
[12:04:10.537] 'NodeTag::T_A_Indices' from value '69' to '70' at nodes.h:26:1
[12:04:10.537] 'NodeTag::T_A_Indirection' from value '70' to '71' at
nodes.h:26:1
[12:04:10.537] 'NodeTag::T_A_ArrayExpr' from value '71' to '72' at nodes.h:26:1
[12:04:10.537] 'NodeTag::T_ResTarget' from value '72' to '73' at nodes.h:26:1
[12:04:10.537] 'NodeTag::T_MultiAssignRef' from value '73' to '74' at
nodes.h:26:1
[12:04:10.537] 'NodeTag::T_SortBy' from value '74' to '75' at nodes.h:26:1
[12:04:10.537] 'NodeTag::T_WindowDef' from value '75' to '76' at nodes.h:26:1
....
[12:04:10.592] -------
[12:04:10.592]
[12:04:10.592]
[12:04:10.592] Summary of Failures:
[12:04:10.592]
[12:04:10.592] 129/282 postgresql:abidiff / plpgsql.abidiff FAIL 1.25s
(exit status 4)

[1]: https://cirrus-ci.com/task/5961614579466240

Regards,
Vignesh

On 06.01.24 18:25, vignesh C wrote:

One of the test has failed in cfbot at [1] with:
abi-compliance-checker
[12:04:10.537] The output from the failed tests:
[12:04:10.537]
[12:04:10.537] 129/282 postgresql:abidiff / plpgsql.abidiff FAIL 1.25s
(exit status 4)
[12:04:10.537]
[12:04:10.537] --- command ---
[12:04:10.537] 12:03:00 /usr/bin/abidiff
/tmp/cirrus-ci-build/build/../src/pl/plpgsql/src/plpgsql.x86_64-linux.abi.xml
src/pl/plpgsql/src/plpgsql.so
[12:04:10.537] --- Listing only the last 100 lines from a long log. ---
[12:04:10.537] 'NodeTag::T_RoleSpec' from value '66' to '67' at nodes.h:26:1
[12:04:10.537] 'NodeTag::T_FuncCall' from value '67' to '68' at nodes.h:26:1
[12:04:10.537] 'NodeTag::T_A_Star' from value '68' to '69' at nodes.h:26:1
[12:04:10.537] 'NodeTag::T_A_Indices' from value '69' to '70' at nodes.h:26:1
[12:04:10.537] 'NodeTag::T_A_Indirection' from value '70' to '71' at
nodes.h:26:1
[12:04:10.537] 'NodeTag::T_A_ArrayExpr' from value '71' to '72' at nodes.h:26:1
[12:04:10.537] 'NodeTag::T_ResTarget' from value '72' to '73' at nodes.h:26:1
[12:04:10.537] 'NodeTag::T_MultiAssignRef' from value '73' to '74' at
nodes.h:26:1
[12:04:10.537] 'NodeTag::T_SortBy' from value '74' to '75' at nodes.h:26:1
[12:04:10.537] 'NodeTag::T_WindowDef' from value '75' to '76' at nodes.h:26:1
....
[12:04:10.592] -------
[12:04:10.592]
[12:04:10.592]
[12:04:10.592] Summary of Failures:
[12:04:10.592]
[12:04:10.592] 129/282 postgresql:abidiff / plpgsql.abidiff FAIL 1.25s
(exit status 4)

[1] - https://cirrus-ci.com/task/5961614579466240

This is kind of intentional, as it shows the the test catches ABI changes.

If the patches were to be committed, then the base ABI file would be
updated.