Show the plaform triplet in each warning and error from the auditor #1309
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
giordano
reviewed
Dec 29, 2023
src/Auditor.jl
Outdated
| @@ -84,7 +84,7 @@ function audit(prefix::Prefix, src_name::AbstractString = ""; | |||
| readmeta(f) do oh | |||
| if !is_for_platform(oh, platform) | |||
| if verbose | |||
| @warn("Skipping binary analysis of $(relpath(f, prefix.path)) (incorrect platform)") | |||
| @warn("Skipping binary analysis of $(relpath(f, prefix.path)) (incorrect platform)", platform=triplet(platform)) | |||
There was a problem hiding this comment.
I don't like this solution, it would make the auditor overly verbose in verbose mode (which is what I use most, and what's used in CI in Yggdrasil) where the platform is already printed.
There was a problem hiding this comment.
Ok, I'll remove it then. Personally I prefer it because it's easy to miss warnings in verbose mode, and even then you have to look for the platform in some other message.
The creation of `libpath_list` assumed that there would only be one directory in `LD_LIBRARY_PATH` (or the equivalents on other platforms), if there were more then creating the list would fail because Julia would try to `hcat()` all the elements together and throw an error about them having different lengths. Somewhat scarily, this would've silently done the wrong thing if `libpath_list` was a column vector instead of a row vector O_o (i.e. if it originally had a comma between the elements)
JamesWrigley
force-pushed
the
platform-warnings
branch
from
dd77a37 to
7d1fdbb
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This makes it a bit easier to nail down which platforms are producing the warnings.Also fixed a wee bug I found when running the tests in dd77a37. I wasn't able to run all the tests locally on my machine, I got aCross-device link (os error 18)when the tests were building a rust package, and I think the wizard tests were hanging.Old before/after (removed commit from PR)
Before:
After: