Include executable in JSON output. #6363
Conversation
dwijnand
force-pushed
the
add-executable-in-json-output
branch
from
bdae64e to
816ff27
Compare
dwijnand
force-pushed
the
add-executable-in-json-output
branch
from
816ff27 to
282f238
Compare
Apparently on Windows it creates an .exe & a .pdb.
tests/testsuite/bench.rs
Outdated
| } | ||
|
|
||
| let p = project() | ||
| .file("src/main.rs", "fn main() {}") |
There was a problem hiding this comment.
This will have the same non-determinism as the other test.
There was a problem hiding this comment.
I’d say we should add with_json_unordered , like we do for stderr:
https://github.com/rust-lang/cargo/blob/master/tests/testsuite/support/mod.rs#L660
There was a problem hiding this comment.
I would not be opposed, I have wanted it in the past. However, you have to be careful when writing unordered tests because the current code does not do "longest match", so it is easy to create non-deterministic tests (or tests that fail in a confusing way). Just be careful that one entry is not a prefix of another.
There was a problem hiding this comment.
Go crazy. Meanwhile I just trimmed the test.
|
@bors: r=matklad |
|
📌 Commit 020efe0 has been approved by |
Include executable in JSON output. Fixes #5426 Rebase of @patriksvensson's #5517 CC @matklad I didn't really get into the issue or the code, I just interatively rebased Patrik's branch and then massaged and cleaned up the code until the tests passed. So please double check it for code correctness, test case correctness and test case coverage. Particularly the branch changed an if condition according to [this suggestion](#5517 (comment)) by Aleksey. I rolled that back because at one point it helped fix a series of tests. But let me know if that should be included here.
| @@ -49,6 +49,16 @@ pub struct OutputFile { | |||
| pub flavor: FileFlavor, | |||
| } | |||
|
|
|||
| impl OutputFile { | |||
| /// Gets the hardlink if present. Otherwise returns the path. | |||
| pub fn bindst(&self) -> &PathBuf { | |||
There was a problem hiding this comment.
Perhaps a _ in the middle to separate "bin" and "dst"?
|
Oops, a few minutes too late! (no worries) |
|
☀️ Test successful - status-appveyor, status-travis |
Fixes #5426
Rebase of @patriksvensson's #5517
CC @matklad
I didn't really get into the issue or the code, I just interatively rebased Patrik's branch and then massaged and cleaned up the code until the tests passed. So please double check it for code correctness, test case correctness and test case coverage.
Particularly the branch changed an if condition according to this suggestion by Aleksey. I rolled that back because at one point it helped fix a series of tests. But let me know if that should be included here.