[matio] Merge mat73 and hdf5 features with mat73 name

[traversaro]

This is just a proposal, but I thought it was easier to discuss with a concrete change proposal rather than a issue.

matio enables the hdf5 feature by default, while the mat73 feature is disabled by default. I think this may be misleading, as the hdf5 features does nothing at the code level unless mat73 is also enabled. As enabling mat73 does not add any additional dependency to the build, to fix the problem I think we can just add mat73 to the default features of the matio vcpkg port.

For reference:

• https://github.com/tbeu/matio/blob/v1.5.27/README#L78-L82
• https://github.com/tbeu/matio/blob/v1.5.27/cmake/options.cmake#L12
• https://github.com/tbeu/matio/blob/v1.5.27/src/matio_private.h#L75

See ami-iit/matio-cpp#84 for an example of confusion that this PR want to fix.

• Changes comply with the maintainer guide.
• SHA512s are updated for each updated download.
• The "supports" clause reflects platforms that may be fixed by this new version.
• Any fixed CI baseline entries are removed from that file.
• Any patches that are no longer applied are deleted from the port's directory.
• The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
• Only one version is added to each modified port's versions file.

[dg0yt]

I think this may be misleading, as the hdf5 features does nothing at the code level unless mat73 is also enabled. As enabling mat73 does not add any additional dependency to the build, to fix the problem I think we can just add mat73 to the default features of the matio vcpkg port.

Apart from the observation that maintainers have become reluctant to adding default-features, you seem to explain that feature mat73 is a dependency of feature hdf5. In the current form of the PR, matio[core,hdf5] would still be broken if doesn't provide hdf5 support.

[dg0yt]

• https://github.com/tbeu/matio/blob/v1.5.27/README#L78-L82
• https://github.com/tbeu/matio/blob/v1.5.27/src/matio_private.h#L75

This states that "MAT file version 7.3 requires the HDF5 library." This is the opposite. matio[core,mat73] would remain broken.

[dg0yt]

BTW does hdf5 add anything apart being required for mat73?
https://github.com/search?q=repo%3Atbeu%2Fmatio%20HAVE_HDF5&type=code

[traversaro]

Apart from the observation that maintainers have become reluctant to adding default-features

In that case, I think that either removing the hdf5 from default features or removing the hdf5 or mat73 feature and just enable both MATIO_WITH_HDF5 and MATIO_MAT73 are also perfectly fine iptions. What I think is really confusing is by default enabling hdf5 and not mat73, as just enabling hdf5 adds the dependency on the hdf5 library, without actually enabling any code path that uses the hdf5 library.

This is the opposite. matio[core,mat73] would remain broken.

I do not think this is the case, as the mat73 feature depends on hdf5 feature (see https://github.com/traversaro/vcpkg/blob/enablemat73defaultmatio/ports/matio/vcpkg.json#L39-L48), that is indeed correct. What would remain "broken" (in the sense that is confusing for users as it does not enable hdf5/mat73 support, see ami-iit/matio-cpp#84) would be matio[core,hdf5], but this seems an improvement over the current situation where matio itself is broken (as it seems that mat 7.3 files are supported as the hdf5 support is enabled, but they are actually not supported as the mat73).

[traversaro]

BTW does hdf5 add anything apart being required for mat73? https://github.com/search?q=repo%3Atbeu%2Fmatio%20HAVE_HDF5&type=code

That is exact, that is what I meant with "as the hdf5 features does nothing at the code level unless mat73 is also enabled." and why I find confusing that hdf5 is enabled by default and mat73 is not.

[dg0yt]

Well, then remove feature hdf5 completely and add mat73 to default features :-)
Sane behavior with no change in dependencies. mat73 is on by default in CMakeLists.txt.

[traversaro]

Well, then remove hdf5 completely and add mat73 to default features :-) Sane behavior with no change in dependencies. mat73 is on by default in CMakeLists.txt.

Ok for me, but are maintainers ok in just removing a feature that would break anyone having matio[hdf5] in its dependencies? If yes, that is perfectly fine for me.

[WangWeiLin-MV]

That is exact, that is what I meant with "as the hdf5 features does nothing at the code level unless mat73 is also enabled." and why I find confusing that hdf5 is enabled by default and mat73 is not.

So it seems that feature hdf5 should be merged to feature mat73.

Ok for me, but are maintainers ok in just removing a feature that would break anyone having matio[hdf5] in its dependencies? If yes, that is perfectly fine for me.

If it was broken, there should be no problem in removing it.

Or keep it as an alias for mat73? @dg0yt

[traversaro]

Ok for me, but are maintainers ok in just removing a feature that would break anyone having matio[hdf5] in its dependencies? If yes, that is perfectly fine for me.

If it was broken, there should be no problem in removing it.

The feature is not broken, simply it was not doing anything, unless mat73 was also enabled. Removing it, the installation itself would not work. Again, this is perfectly fine for me, but just to point out that this is not removing a non-default feature that is not working.

[dg0yt]

Since keeping features is no longer a requirement for vcpkg upgrade, vcpkg owners started to accept removing features without grace period.

Remove hdf5. Move dependency to mat73.
Remove pic. Triplet setting, generally enabled in all vcpkg toolchains.

[traversaro]

Remove hdf5. Move dependency to mat73.

Done, by renaming hdf5 feature to mat73 and ensuring that MATIO_MAT73 is enabled when mat73 feature is present.

Remove pic. Triplet setting, generally enabled in all vcpkg toolchains.

Just removing MATIO_PIC settings will results in MATIO_PIC options being enabled always (see https://github.com/tbeu/matio/blob/9e26c65fc31de240a051c8921119391f2c14f249/cmake/src.cmake#L91), while setting it to OFF will result in MATIO_PIC being OFF even if PIC is enabled in the triplet setting. I guess the best way to proceed is to patch out https://github.com/tbeu/matio/blob/9e26c65fc31de240a051c8921119391f2c14f249/cmake/src.cmake#L91, however this is a non-trivial changes that I do not think is strictly related to this PR, and we can track in #42106 .

[WangWeiLin-MV]

The port usage tests pass with the following triplets:

• x64-windows