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

Ensure cargo-kani setup is idempotent #1193

Merged
merged 4 commits into from
May 12, 2022

Conversation

tedinski
Copy link
Contributor

Description of changes:

Quick fixes for lack of idempotency in cargo-kani setup

Resolved issues:

Resolves #1156

Call-outs:

  1. First fix is symlink creation. It would fail if it already existed. I also switched to using the native rust instead of shelling out to ln -s
  2. Second fix was the rename in the --use-local-bundle code. I changed this to directly extract the bundle to the correct location in the first place, making its effective behavior more like the download case.
  3. Minor fix: canonicalize can fail if the file doesn't exist. Print the file before canonicalizing.

Testing:

  • How is this change tested? A re-install test added to the release bundle tests

  • 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
  • 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.

@tedinski tedinski requested a review from a team as a code owner May 11, 2022 23:27
Copy link
Contributor

@zhassan-aws zhassan-aws left a comment

Choose a reason for hiding this comment

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

Needs a formatting fix, but otherwise looks good.

Copy link
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.

The first docker command in this group also contains a double whitespace. Can you please correct that too?

@@ -107,6 +107,7 @@ jobs:
docker run -w /tmp/kani/tests/cargo-kani/simple-lib kani-latest cargo kani
docker run -w /tmp/kani/tests/cargo-kani/simple-visualize kani-latest cargo kani
docker run -w /tmp/kani/tests/cargo-kani/build-rs-works kani-latest cargo kani
docker run kani-latest cargo-kani setup --use-local-bundle ./kani-latest-x86_64-unknown-linux-gnu.tar.gz
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
docker run kani-latest cargo-kani setup --use-local-bundle ./kani-latest-x86_64-unknown-linux-gnu.tar.gz
docker run kani-latest cargo-kani setup --use-local-bundle ./kani-latest-x86_64-unknown-linux-gnu.tar.gz

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They all do, I was trying to keep the "command run in the container" more separate.

@tedinski tedinski merged commit fa42232 into model-checking:main May 12, 2022
@tedinski tedinski deleted the idempotent-setup branch May 12, 2022 20:37
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.

cargo-kani setup should be idempotent
3 participants