Skip to content

signal errors in ci_fetch_deps subprocesses #6588

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

Merged
merged 2 commits into from
Jul 15, 2022

Conversation

jepler
Copy link

@jepler jepler commented Jul 13, 2022

A recent build failed. The original error seemed to be during ci_fetch_deps where a build message said

  fatal: reference is not a tree: 346c936e14c6ea3a8d3d65cb1fa46202dc92999d
  fatal: Unable to checkout '346c936e14c6ea3a8d3d65cb1fa46202dc92999d' in submodule path 'extmod/ulab'

(along with other problems), but this step didn't signal failure to github actions.

By adding the check= parameter, a failure of the subprocess will cause a CalledProcessError to be raised, which will make ci_fetch_deps exit with nonzero status. In turn, this should let Actions understand that something went wrong with this step, instead of waiting for some subsequent step(s) to go wrong.

A recent build failed. The original error seemed to be during ci_fetch_deps
where a build message said
```
  fatal: reference is not a tree: 346c936e14c6ea3a8d3d65cb1fa46202dc92999d
  fatal: Unable to checkout '346c936e14c6ea3a8d3d65cb1fa46202dc92999d' in submodule path 'extmod/ulab'
```
(along with other problems), but this step didn't signal failure to
github actions.

By adding the check= parameter, a failure of the subprocess will cause
a CalledProcessError to be raised, which will make ci_fetch_deps exit with
nonzero status. In turn, this should let Actions understand that something
went wrong with this step, instead of waiting for some subsequent step(s)
to go wrong.
@tannewt
Copy link
Member

tannewt commented Jul 14, 2022

Looks like there are already errors that this catches. :-/

@jepler jepler requested a review from tannewt July 15, 2022 00:38
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Thank you!

@tannewt tannewt merged commit bad080a into adafruit:main Jul 15, 2022
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