Fix "local crate" detection #3644
Conversation
`PackageId` is an opaque identifier whose internal format is subject to change, so looking up the names of local crates by ID is more robust than parsing the ID. Resolves rust-lang#3643.
narpfel
force-pushed
the
local-crates-metadata-format-update
branch
2 times, most recently
from
3be956e to
8bb14dc
Compare
|
Doesn't |
|
I think this cargo change was not considered a breaking change since there was never any guarantee about the format of that field. Some clients (Miri included) just used the field in ways that was never intended or supported. |
| @@ -0,0 +1,26 @@ | |||
| subcrate,issue_1567,exported_symbol_dep,cargo_miri_test,cdylib,executable,library_with_dashes,exported_symbol,issue_1691,issue_1705,issue_rust_86261,proc_macro_crate | |||
| error: Undefined Behavior: entering unreachable code | |||
| --> /build/local-crate/library-with-dashes/src/lib.rs:6:14 | |||
There was a problem hiding this comment.
Where does this path come from? It looks like an absolute path, which would be a problem -- the test is meant to work everywhere, not just on CI.
Also, this fails on Windows because there, some of the / become \.
Our test-cargo-miri isn't really set up to deal with error output I am afraid. Maybe it'd be easier to just test the contents of MIRI_LOCAL_CRATES rather than an actual error message? E.g. if we check that it contains issue_rust_86261, that should be enough, no?
There was a problem hiding this comment.
Where does this path come from? It looks like an absolute path, which would be a problem -- the test is meant to work everywhere, not just on CI.
The test runner uses --remap-path-prefix to replace the working directory with /build: 8bb14dc#diff-4fcb5764801672d83a446351eb4d6f44577f99d386d2632964e7a239bfbbb813R137
Maybe it'd be easier to just test the contents of
MIRI_LOCAL_CRATESrather than an actual error message?
Yeah, that’s probably better.
test-cargo-miri/Cargo.toml
Outdated
| @@ -17,6 +17,7 @@ issue_1567 = { path = "issue-1567" } | |||
| issue_1691 = { path = "issue-1691" } | |||
| issue_1705 = { path = "issue-1705" } | |||
| issue_rust_86261 = { path = "issue-rust-86261" } | |||
| executable = { path = "local-crate/executable" } | |||
There was a problem hiding this comment.
This needs comments to explain what the test is about, and ideally a better name -- "executable" doesn't say much. The point is that this one has UB and we're checking what that looks like.
There was a problem hiding this comment.
I renamed the test test-local-crate-detection. Is that better?
narpfel
force-pushed
the
local-crates-metadata-format-update
branch
from
8bb14dc to
449925b
Compare
|
Looks good, thanks! |
|
☀️ Test successful - checks-actions |
PackageIdis an opaque identifier whose internal format is subject to change, so looking up the names of local crates by ID is more robust than parsing the ID.Resolves #3643.