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.

Sign up for GitHub

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

Jump to bottom

Wasmtime: remaps for coverage #5392

Merged
merged 3 commits into from
Mar 21, 2021
Merged

Wasmtime: remaps for coverage #5392

merged 3 commits into from
Mar 21, 2021

Conversation

catenacyber
Copy link
Contributor

cc @alexcrichton cf #5366 (comment)

This should fix wasmtime coverage build

alexcrichton
alexcrichton approved these changes Mar 15, 2021
View reviewed changes
Copy link
Contributor

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

I always find bash a bit, uh, esoteric to read...

In any case though looks good to me, thanks!

projects/wasmtime/build.sh
@@ -26,6 +26,12 @@ build() {
shift
PROJECT_DIR=$SRC/$project

cd $PROJECT_DIR
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this have a comment for the purpose of what's happening? (basically just ensuring that absolute paths are used everywhere in debuginfo and such)

alexcrichton
alexcrichton approved these changes Mar 15, 2021
View reviewed changes
Copy link
Contributor

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Thanks for your help here!

@inferno-chromium inferno-chromium enabled auto-merge (squash) March 15, 2021 16:43
@catenacyber catenacyber mentioned this pull request Mar 15, 2021
Closed
@catenacyber
Copy link
Contributor Author

@inferno-chromium could we merge this ?

@inferno-chromium
Copy link
Collaborator

@inferno-chromium could we merge this ?

Can you check CI failures, are they related to this change, can you maybe push again to see if they still happen. if not, leave a comment and we can merge.

@catenacyber
Copy link
Contributor Author

CI is happier except for coverage failure which seems unrelated
LLVM ERROR: IO failure on output stream: No space left on device

@inferno-chromium inferno-chromium merged commit 2e64b7a into google:master Mar 21, 2021
@alexcrichton
Copy link
Contributor

Thanks again for this @catenacyber, we got our first successful build today as a result.

@inferno-chromium if I go to https://oss-fuzz.com/ and click "total coverage" it links me to https://oss-fuzz.com/coverage-report/job/libfuzzer_asan_wasmtime/latest which gives me " Failed to get coverage report. (alex@alexcrichton.com) ". Is that something that needs to be configured on y'all's end? Or should I just wait a day or so for things to propagate?

@catenacyber
Copy link
Contributor Author

There is one available here
https://storage.googleapis.com/oss-fuzz-coverage/wasmtime/reports/20210322/linux/report.html

@alexcrichton
Copy link
Contributor

Oh wow, thanks! How did you end up finding the link to that?

@catenacyber
Copy link
Contributor Author

Just clicked a link to one of my projects and replaced the name ;-)
I guess you can wait a day or two for thing to propagate...

@catenacyber
Copy link
Contributor Author

And now your link https://oss-fuzz.com/coverage-report/job/libfuzzer_asan_wasmtime/latest works (and redirects to mine)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexcrichton alexcrichton alexcrichton approved these changes

@inferno-chromium inferno-chromium inferno-chromium approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@catenacyber @inferno-chromium @alexcrichton