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

Remove some logic from the superbuild #2469

Merged
merged 2 commits into from
Sep 3, 2024

Conversation

benson31
Copy link
Collaborator

@benson31 benson31 commented Sep 3, 2024

The Superbuild cannot write to CMAKE_INSTALL_PREFIX during configure time. There's no guarantee that this variable is even set, or that it's a valid and writeable path if it is set (indeed, the default path is likely not writeable by the average superbuild user). Moreover, even if it is set, each package maintains its own install prefix, which may be wholly unrelated to whatever is in the CMAKE_INSTALL_PREFIX. Finally, the superbuild doesn't define an install target at all, so we cannot simply copy the file at "superbuild install time" since such a thing doesn't exist (and this certainly is not a compelling reason to change that).

In spite of this seemingly bleak status quo, clients of the superbuild are much more empowered than the superbuild can inherently be, and they have access to all of the relevant information. In the case of LBANN's CI infrastructure, we do choose to install all of the dependencies under a common prefix, so we can manually copy this file over to that prefix any time after configuration has completed.

As the last bit of cleanup, I removed a message about python something or another that's LBANN's problem and should go in LBANN's CMake, not the superbuild's (the message had nothing to do with interactions with or among any of the packages known to the superbuild, and therefore it is irrelevant to the goals of the superbuild system). Also, as far as I know, CNPY has no dependence on Python or the numpy python module, so I also culled that message, also believing it to be irrelevant and/or erroneous (LBANN's dependence (or not) on numpy is a concern for the LBANN project and not the superbuild as it is not a package manager).

@benson31 benson31 added review requested build CI Continuous Integration labels Sep 3, 2024
@bvanessen bvanessen merged commit c4cc48c into LLNL:develop Sep 3, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build CI Continuous Integration review requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants