Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated TriBITS is disabling downstream TPLs #557

Closed
bartlettroscoe opened this issue Dec 24, 2022 · 1 comment
Closed

Updated TriBITS is disabling downstream TPLs #557

bartlettroscoe opened this issue Dec 24, 2022 · 1 comment

Comments

@bartlettroscoe
Copy link
Member

bartlettroscoe commented Dec 24, 2022

Description

The updated TriBITS snapshotted to Trilinos in PR trilinos/Trilinos#11380 has broken backward compatibility in that disabling an upstream TPL like HDF5 will disable a downstream TPL like NetCDF. Well, that breaks some configure scripts for Trilinos like the one for Albany as reported in trilinos/Trilinos#11426.

We, it turns out that I knew that was going to happen as I documented it in the tribits/CHANGLOG.md 2022-10-20 entry:

Changed: Disabling an external package/TPL will now disable any downstream external packages/TPLs that list a dependency on that external package/TPL through its FindTPL<tplName>Dependencies.cmake file. Prior to this, disabling an external package/TPL would not disable dependent downstream external packages/TPLs (it would only disable downstream dependent required internal packages). To avoid this, simply leave the enable status of the upstream external package/TPL empty "" and no downstream propagation of disables will take place.

which you can see got posted in the PR in the file tribits/CHANGLOG.md.

Proposed solution

I think the proposed solution, for now, is to make TPL dependencies optional instead of required. Such dependencies already behave like they are optional in that enabling a TPL like NetCDF will not automatically enable a dependent upstream TPL like HDF5. To provide maximum flexibility in how people define TPLs, we should likely make all TPL dependencies optional.

NOTE: The reason that TPL dependencies had to be added was to correctly propagate and order TPL libraries to downstream targets. TPL dependencies were not added to change enable/disable behavior. (Not sure why I decided to make TPL dependencies behave this way.)

bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Jan 6, 2023
Disabling the upstream TPL HDF5 was disabling the TPL Netcdf and that broke
several customer's Trilinos configure scripts.  The issue is, that we can't
treat TPL-to-TPL dependencies as required for cases like this.

I also updated some documentation to make this more clear.

In the future, we may need to expand TPL intra-dependencies to support
optional and required dependencies because there are cases where you would
like the behavior of required dependencies for some TPL relationships.
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Jan 6, 2023
Disabling the upstream TPL HDF5 was disabling the TPL Netcdf and that broke
several customer's Trilinos configure scripts.  The issue is, that we can't
treat TPL-to-TPL dependencies as required for cases like this.

I also updated some documentation to make this more clear.

In the future, we may need to expand TPL intra-dependencies to support
optional and required dependencies because there are cases where you would
like the behavior of required dependencies for some TPL relationships.
@bartlettroscoe
Copy link
Member Author

This was resolved with the merge of the PR:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

1 participant