Skip to content

Implement GH action script.#1589

Merged
YoshikiTakashima merged 26 commits intomodel-checking:mainfrom
YoshikiTakashima:1312-gh-action
Sep 6, 2022
Merged

Implement GH action script.#1589
YoshikiTakashima merged 26 commits intomodel-checking:mainfrom
YoshikiTakashima:1312-gh-action

Conversation

@YoshikiTakashima
Copy link
Copy Markdown
Contributor

@YoshikiTakashima YoshikiTakashima commented Aug 26, 2022

Description of changes:

Adds a github actions script that runs Kani on user code

Resolved issues:

Resolves #1312

Call-outs:

  • Blocked on Release Kani with GH packages. #1587 This action script relies on docker to work. Kani needs to publish Docker images as packages for this action to work. PR Added packages to release for ubuntu-20.04 only. #1595!

  • Testing is a bit odd on this one. We expect our customers to integrate via marketplace, but we can't test that integration until this script is merged. See testing sections for details

  • Integration method is through docker, which will require a package release. I think this is cleanest since it requires no build on customer end. Risk of breakage is less, we don't slow their CI pipeline with our install and build.

  • Kani options is intended to be passed in via Cargo.toml section [package.metadata.kani.flags]. Also configuring this action is only possible through Cargo.toml I think maintaining 2 APIs will be a mess and YAML is not very good for wrangling certain parameters (e.g. cargo kani -p packageA -p packageb). It would be much easier to maintain if we only allow configuration trhough Cargol.toml.

Testing:

  • How is this change tested? You can test this by pasting the action.yml into a cargo project that already uses Kani, then running the kani harnesses with it. Add a workflow referencing this action, and trigger it. Results should match that of running cargo kani on that project.

  • Is this a refactor change? NO

Checklist

  • Each commit message has a non-empty body, explaining why the change was made
  • Methods or procedures are documented. TBD Adding docs in the Kani book. 3d5b2af
  • Regression or unit tests are included, or existing tests cover the modified code
  • My PR is restricted to a single feature or bugfix

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.

@YoshikiTakashima YoshikiTakashima requested a review from a team as a code owner August 26, 2022 02:01
@YoshikiTakashima
Copy link
Copy Markdown
Contributor Author

@tedinski Would you mind reviewing this. It's still blocked, but demand (as far as I can tell from Rust users around me) is very strong on this one.

Comment thread docs/src/install-guide.md
Comment thread docs/src/SUMMARY.md
@YoshikiTakashima
Copy link
Copy Markdown
Contributor Author

Once this is merged, somebody with sufficient permissions will need to enable actions in the GH marketplace.

@YoshikiTakashima YoshikiTakashima changed the title [BLOCKED] Implement GH action script. Implement GH action script. Aug 29, 2022
@YoshikiTakashima
Copy link
Copy Markdown
Contributor Author

This issue is no longer blocked. Please do a second pass.

Also, to pin the image to the respective version, I had to make a hard-coded value in action.yml with a "update version" check every release. If anyone knows an auto-upgrading solution, please let me know.

Copy link
Copy Markdown
Contributor

@adpaco-aws adpaco-aws left a comment

Choose a reason for hiding this comment

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

Thanks @YoshikiTakashima !

One question: How hard would it be to extend this action with an include/exclude list of proofs to run?

In my experience, users may want to exclude a proof from being run in CI, either because it's too time-consuming or it no longer passes after changing the function under verification. That said, I think the right way to do that would be to add another attribute like "ignore" to the proofs themselves, so cargo kani can handle that instead.

Comment thread action.yml Outdated
Comment thread docs/src/install-github-ci.md Outdated
Comment thread docs/src/install-github-ci.md Outdated
Comment thread docs/src/install-github-ci.md Outdated
Comment thread docs/src/install-github-ci.md Outdated
Comment thread docs/src/install-guide.md Outdated
Yoshiki Takashima and others added 5 commits September 1, 2022 14:23
Co-authored-by: Adrian Palacios <73246657+adpaco-aws@users.noreply.github.com>
Co-authored-by: Adrian Palacios <73246657+adpaco-aws@users.noreply.github.com>
Co-authored-by: Adrian Palacios <73246657+adpaco-aws@users.noreply.github.com>
@YoshikiTakashima
Copy link
Copy Markdown
Contributor Author

@adpaco-aws As of now, any flag that works with rust can be used with [package.metadata.kani.flags]. So --harness will work, but I think it does not allow multiple harnesses to be specified at this point.

Copy link
Copy Markdown
Contributor

@danielsn danielsn left a comment

Choose a reason for hiding this comment

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

LGTM, a few comments

Comment thread .github/workflows/release.yml Outdated
Comment thread README.md Outdated
@YoshikiTakashima YoshikiTakashima merged commit f12097a into model-checking:main Sep 6, 2022
@YoshikiTakashima YoshikiTakashima deleted the 1312-gh-action branch September 6, 2022 20:28
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.

Create Kani Github Action

4 participants