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

Add miri to CI pipeline #401

Closed
wants to merge 10 commits into from
Closed

Add miri to CI pipeline #401

wants to merge 10 commits into from

Conversation

Robbepop
Copy link
Collaborator

@Robbepop Robbepop commented May 20, 2020

We only test ink_core/storage2 so far.

The miri tool requires rust-src and xargo tools to be installed.
This has been done in a separate PR already.

So far miri runs kind of successfully but it doesn't play well with RUST_WRAPPER and TARGET_DIR so we have to implement some miri specific work arounds here and there to avoid crashing other jobs as can be seen often with the clippy and test jobs.

Links: #401 (comment)

Copy link
Contributor

@TriplEight TriplEight left a comment

Choose a reason for hiding this comment

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

LGTM if it works

@Robbepop
Copy link
Collaborator Author

@TriplEight https://gitlab.parity.io/parity/ink/-/jobs/519777

any ideas what is causing this?

@codecov-commenter
Copy link

codecov-commenter commented May 20, 2020

Codecov Report

Merging #401 into master will increase coverage by 0.06%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #401      +/-   ##
==========================================
+ Coverage   86.27%   86.34%   +0.06%     
==========================================
  Files         131      131              
  Lines        5727     5727              
==========================================
+ Hits         4941     4945       +4     
+ Misses        786      782       -4     
Impacted Files Coverage Δ
core/src/hash/accumulator.rs 92.59% <0.00%> (+14.81%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e1dcdc8...335a6f7. Read the comment docs.

@TriplEight
Copy link
Contributor

there are some issues with miri which are suggesting me it's not yet stable enough and should be used isolated from the other CI environment:
rust-lang/miri#1311
rust-lang/miri#1421
As a matter of fact, they are proposing unsetting RUSTC_WRAPPER and CARGO_TARGET_DIR .

@TriplEight TriplEight self-requested a review May 20, 2020 17:40
@Robbepop
Copy link
Collaborator Author

Robbepop commented Oct 7, 2020

On hold until we fixed miri on ink_storage.

@Robbepop Robbepop added the E-on-ice The issue or PR is currently on ice and not further discussed or developed. label Nov 10, 2020
@Robbepop
Copy link
Collaborator Author

Robbepop commented Dec 2, 2020

Closing this PR since it is heavily outdated. We maybe still want miri integration but I'd prefer miri to stabilize further to make it less of a pain to use for us.

@Robbepop Robbepop closed this Dec 2, 2020
@RalfJung
Copy link

RalfJung commented Dec 2, 2020

Miri is used on CI by quite a few projects, as far as I know. So if you have something that you think needs to happen to make Miri more usable on CI, please let us know by reporting an issue! Right now I am not aware of any major issues.

@HCastano HCastano deleted the robin-add-miri-to-ci branch July 29, 2022 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-on-ice The issue or PR is currently on ice and not further discussed or developed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants