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

[configuration] Make cmake configuration fail early upon boost or openBLAS installation failure #488

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lambdaxdotx
Copy link

TLDR: This PR introduces a couple of checks for the cmake configuration process to make it fail upon the installation failure of either boost or openblas.

Problem

At the moment, whenever any tools/download_*.sh script fails, the configuration process continues w/o any explicit message, for failing only at the end. This may surprise and mislead the (non-dev) user: issue #481 seems to be an example of that.

Catching the erratic state of the configuration and fail early seems a better strategy to me.

Proposed solution

  • Make tools/download_*.sh exit on any failure by passing the -e to /bin/bash
  • Check the exit code of those scripts in the CMakeLists.txt and fail accordingly.

Let me know what you think.

…nBLAS installation failure with a clear message.
@lambdaxdotx
Copy link
Author

The CI fails because the download_boost.sh fails at some point. In particular, this means that the script was already failing.

The next step, I guess, is to look at the precise command that fails in the script and let maintainers decide how to proceed.

@AleksandarZeljic
Copy link
Collaborator

Thanks for submitting the PR. I am pretty sure that the failure has nothing to do with the PR. A recent PR was pushed into master after passing CI checks, but once integrated failed because boost wasn't downloaded correctly only to suddenly work again without intervention. I will look into this.

@lambdaxdotx
Copy link
Author

Hi @AleksandarZeljic

Thanks for giving this PR some attention. Let me know if you need help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants