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

Fail conanfile with compile fail #242

Merged
merged 2 commits into from
May 15, 2024

Conversation

patkenneally
Copy link
Collaborator

@patkenneally patkenneally commented May 10, 2024

  • Tickets addressed: MAX-917
  • Review: By commit
  • Merge strategy: Merge (no squash)

Description

Previously, if the sub process that called conan.build() failed (e.g. compilation failed) the sub process return code was ignored and the conanfile execution would return as successful. Of course, this is incorrect behavior. This PR modifies the sub process call and checks the return code. If the return code is not 0 (success) then an exception is thrown and the conanfile.py execution fails. As a nice side effect, because the conanfile.py execution fails, then the encapsulating github action step fails when the build fails (which didn't happen previously). Additionally, os.system is not the recommend method to run a sub process and this PR changes to use subprocess package.

Verification

CI runs successfully. Also a purposeful compilation error was added to spacecraft.cpp and the CI run to demonstrate the failing conanfile.py execution. See CI run https://github.com/lasp/basilisk/actions/runs/9037970400

Documentation

None new, none invalidated.

Future work

None

@patkenneally patkenneally self-assigned this May 10, 2024
@patkenneally patkenneally force-pushed the ci/fail-conanfile-with-compile-fail branch from 79d093f to 0a90098 Compare May 10, 2024 22:51
Checking the return code will ensure that if
the subprocess fails then the conanfile fails.
This is correct and as a nice side effect it
ensures that the github action step fails when
the build fails. Additionally, os.system is
not the recommend method to run a subprocess.
@patkenneally patkenneally force-pushed the ci/fail-conanfile-with-compile-fail branch from 0a90098 to 666e677 Compare May 10, 2024 22:52
@patkenneally patkenneally merged commit fc30032 into develop May 15, 2024
3 checks passed
@patkenneally patkenneally deleted the ci/fail-conanfile-with-compile-fail branch May 15, 2024 00:30
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