Skip to content

Add check to Lockbud CI job #6898

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 4 commits into from
Feb 4, 2025
Merged

Add check to Lockbud CI job #6898

merged 4 commits into from
Feb 4, 2025

Conversation

eserilev
Copy link
Member

@eserilev eserilev commented Feb 1, 2025

Closes #6637

Adds a script to check if Lockbud failed. Thanks @magick93 for working on this!

@eserilev eserilev changed the base branch from stable to unstable February 1, 2025 18:01
@eserilev
Copy link
Member Author

eserilev commented Feb 3, 2025

heres a failure example:
image

@eserilev eserilev added bug Something isn't working infra-ci ready-for-review The code is ready for review labels Feb 3, 2025
output=$(cargo lockbud -k deadlock -b -l tokio_util 2>&1)

# Check if lockbud returned any issues
if echo "$output" | grep -q '"bug_kind"'; then
Copy link
Member

Choose a reason for hiding this comment

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

Note: this may produce false negatives if cargo lockbud failed for other reasons e.g.

  1. compilation error
  2. lockbud internal issues

Although i think this is already better than before! Given that (1) compilation errors should fail other jobs as well, and we haven't seen (2) happened before (it may be very unlikely), I think we can go ahead and merge this.

Copy link

Choose a reason for hiding this comment

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

Agree.
This script was only intended to be a quick fix.

I do suggest we

  • centralize this script as currently its copy pasta between anchor and LH.
  • ultimately improve the lockbud cli so it can return and error code with the result.

@jimmygchen jimmygchen added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Feb 4, 2025
mergify bot added a commit that referenced this pull request Feb 4, 2025
@mergify mergify bot merged commit 56f201a into sigp:unstable Feb 4, 2025
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working infra-ci ready-for-merge This PR is ready to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lockbud CI job doesn't ever fail
4 participants