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

Make TPL-to-TPL dependencies optional (#557) #558

Merged

Conversation

bartlettroscoe
Copy link
Member

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 (see trilinos/Trilinos#11426).

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.

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.
Copy link
Collaborator

@rabartlett1972 rabartlett1972 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Letting this merge so I can snapshot to Trilinos. Then I will set up a post-merge review.

@bartlettroscoe
Copy link
Member Author

Hello @KyleFromKitware, can you please do a post-merge review of this PR when you get a chance?

Copy link
Collaborator

@KyleFromKitware KyleFromKitware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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

Successfully merging this pull request may close these issues.

3 participants