BP5Deserialize: modify changes to keep abi compt

[vicentebolea]

I have found that we broke ABI compatibiltiy with the bugfix for BP5Deserializer, my bad. Now I am armed with a abi compatibility tool and I can see what went wrong :)

It appears that 2.9.1 is not fully ABI compatibly, that is something that we cannot change, the good this is that BP5Deserializer is an "internal" sort of type, thus is unlikely the user to hit a dynlink error. In order to make the future 2.9.2 as ABI compatible and to set a precedent I rework here the bugfix in order to revert to 2.9.0 ABI.

Here are the ABI compatibility report of 2.9.1 against 2.9.0:

Here are the ABI compatibility report of this MR against 2.9.0:

[eisenhauer]

I'm fine with the fix, but I am curious about this. Changing the way our serializers work isn't wildly unusual, but since they are only called by our own engines (which are themselves only called by our bindings, and occasionally directly by tools like remote_server and bpls (both of which use internal interfaces) we haven't worried about this a lot. I get having to be careful about changing the bindings, but I'd like to understand if there are circumstances where something like this might break compatibility with user code...

[vicentebolea]

@eisenhauer you are right it is very unlikely for an user to have issues with this since this header/type (while being installed) is (internal) and not intended for the user to directly use.

Unfortunately since we do not separate our internal headers into an internal path (or we do not install them), these automated tools will report it as part of the public ABI.

I intent to keep the door open for incoming release 2.10.X to have these API/ABI tests included as part of the CI.

This PR is an effort to start taking steps for ABI management and to set a precedent.

[eisenhauer]

So is the root of the problem that we're installing a ton of .h files that are neither used nor intended for any external access? Things like sst_data.h, all the reader and writer .h files, etc? Any way we can stop doing that?

[caitlinross]

So I think a lot of those .h files are installed because they may need to be used for external access, because they could possibly be used in plugin development. For instance, the ParaView in situ plugin ends up needing to include core/IO.h, and InlineWriter.h.

[vicentebolea]

@caitlinross good point, users developing plugins might need more "internal" headers and those headers might also include even more internal headers through nested included.

I think that in the future we could reduce the public stable ABI by using forward declarations and Pimpl.

Plugin dev also highlights the need of ABI stability across patch releases.

[eisenhauer]

I guess the root of my concern is that in the course of trying to solve the metadata scalability problem I had to make some far reaching changes in ADIOS internals and it was awkward enough doing that while trying to keep compatibility with published interfaces. If every internal interface is essentially public (albeit undocumented), then that's yet more difficult. But maybe the answer is that we just push very little into minor versions. Still leaves me concerned that plugins, which by their nature aren't tested in CI, might break with any update because they might depend upon anything. But I guess that's an issue with not having a specific plugin interface.

[vicentebolea]

plugins which by their nature aren't tested in CI

We do have some tests that build a plugin and load it, however, those tests do not test the API/ABI for the plugin interface though.

But maybe the answer is that we just push very little into minor versions.

If you refer patch releases, yes, that appears to be the only choice that we have for the remaining 2.9.X, for a future 2.10.X we could work reducing the headers files and moving as much as we can to our private interface.