[WIP] Coverage Analysis #20
Conversation
Configure solaris to use solc as its compiler.
Removed unnecessary `-o .` flag for specifying output directory. Solc already outputs to the appropriate directory, and the flag incorrectly creates a new directory.
Initial writeup on how to get coverage manually, using cargo-cov and llvm-cov to get .gcov files with branching statistic and line coverage. Working on automating the process.
solc/src/lib.rs
Outdated
| @@ -3,7 +3,7 @@ mod platform { | |||
| use std::process::Command; | |||
|
|
|||
| pub fn solc() -> Command { | |||
| Command::new("solcjs") | |||
| Command::new("solc") | |||
There was a problem hiding this comment.
What's the reason behind this change? Currently we only require npm install -g solc, which installs solcjs. Do we need to use CPP version for some reason?
There was a problem hiding this comment.
While I agree with you that compiler change shouldn't be included in this PR, the general idea is to move to solc — I personally have much shorter compile times with it. (Eventually, we want to get rid of a separate compiler requirement at all, which is tracked in #8 )
There was a problem hiding this comment.
I agree that #8 is the best option, but I don't think it's going to happen soon. For me currently compile time of contracts is negligible compare to compilation time of all solaris dependencies. Also solcjs is way easier to install, which is pretty important for later adoption.
I would be in favour of detecting which of the two is available (with solc being prefered) and using that one, but indeed should be part of separate PR.
There was a problem hiding this comment.
I would be in favour of detecting which of the two is available (with solc being prefered) and using that one, but indeed should be part of separate PR.
This compiler change got rolled into the commits with this branch by mistake. I have another branch (lght-solc) which tracks these changes. I can make a PR for that branch, and attempt some detection of what is available on the system.
I personally have much shorter compile times with it.
^ same for me, and is my main motivation for moving away from solcjs. Because of the current CI setup (and proper separation of concerns), I will revert the compiler changes in this PR.
There was a problem hiding this comment.
Please fix the build and rollback the compiler change. I totally support @tomusdrw idea about autodetecting the available compiler -- as a separate PR we can easily do right now.
Other than that -- the wrteup is pretty solid and easy to understand, let's merge it soon!
As do I, this change was a mistake on my part. Reverted the changes :)
Thanks :) I'll keep making changes as I learn more about the available tooling. Apparently there is an issue with Rust profiling picking up on generic functions (see this issue in |
|
@lght do you want to wait for a second review, or we can merge this now? |
|
@lght glad to help! I've read your guide, are you mixing GCC/LLVM compiled code? mozilla/grcov#25 would make grcov work fine with a mix of GCC/LLVM gcda/gcno files. I haven't prioritized it yet because I don't need it for my purposes, but I can raise its priority if you need it. Another option would be to split the GCC and LLVM files into two different directories, then run grcov once with the GCC files, once with the LLVM files and once to merge the two. Note also that grcov doesn't need llvm-cov to be installed, as it is using the LLVM API directly to parse the gcda/gcno files. |
I like this idea a lot. Would make analysis of FFI/Native components easier. Something like "want analysis of wrapped C components, only run GCC grcov pass". Will look into helping with mozilla/grcov#25. Using the headers to determine GCC/LLVM, splitting them inside a tmp working folder, then running the two passes seems like it might work for below feature.
Did not realize this, makes sense now tho.
Yes. Parity has a number of components that are Rust wrappers around C/C++ code. Is there a way to instrument the build to use all clang? |
|
Just wanted to let you know that mozilla/grcov#25 has been fixed, so there's no longer any issue with mixing code compiled by GCC and LLVM. |
This is an attempt to get coverage reporting working for Solaris. Currently available tooling
cargo-covworks nicely for generating branch and line-coverage statistics, and getting an HTML-formatted report.For our purposes, however, we need a format we can upload to Coveralls (for that nice green badge :)
My current working(?) solution is to use
llvm-cov gcovto generate.gcovfiles from.gcda/.gcnofiles produced bycargo-cov. Ideally this will be one step in the near future.It is possible to achieve a one-step solution either patching
cargo-covto get intermediategcovoutput, or with a script that automates.gcovgeneration withllvm-cov gcov.Will update this PR as progress is made.