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

Fix C++ dependency warning #924

Merged
merged 2 commits into from
Mar 7, 2023
Merged

Fix C++ dependency warning #924

merged 2 commits into from
Mar 7, 2023

Conversation

jtcooper10
Copy link
Collaborator

Updating the C++ dependency list to look for SCons instead of GNU Make. The process for doing this is a little different from Make, since it's usually a Python package.

Changes Made

  • Removed make from the list of C++ dependencies to check for, added scons
  • Updated scons dependency check and call to look for the executable first, defaulting to the Python package if not found
  • Updated warning in CSolver constructor to reference scons instead of make

Testing

Run test_check_cpp_support unit test in test_check_cpp_support.py or any explicit C++ solver test with:

  • SCons installed as a Python package, e.g. python3 -m pip install scons (should build and run successfully)
  • SCons installed externally, e.g. via a package manager or from the SCons download page (should build and run successfully)
  • SCons not installed at all (should fail with the correct warning message)

Fixes #923

@jtcooper10 jtcooper10 marked this pull request as ready for review February 26, 2023 23:19
@briandrawert
Copy link
Member

When testing, be sure that model.run() will select the 'C' solver. Also verify that current develop does not do this.

@briandrawert briandrawert merged commit b88ac00 into develop Mar 7, 2023
@briandrawert briandrawert deleted the fix/cpp-deps-warn-scons branch March 7, 2023 19:12
@seanebum seanebum added this to the 1.8.1 Release milestone Mar 7, 2023
@BryanRumsey BryanRumsey mentioned this pull request Mar 7, 2023
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.

4 participants