Conversation
kkysen
left a comment
kkysen
left a comment
There was a problem hiding this comment.
How'd you determine these new invocation commands? The most up-to-date commands are in pdg/src/main.rs in mod tests, as those are the ones tested in cargo test and CI.
Could we just say to look there in the README.md?
|
This is basically what's in |
| ${CARGO_TARGET_DIR:-../target/debug/c2rust-instrument} --metadata instrument.target/debug/metadata.bc -- build --manifest-path analysis/tests/misc/Cargo.toml | ||
| (cd analysis/tests/misc/instrument.target/debug; INSTRUMENT_BACKEND=debug INSTRUMENT_RUNTIME=bg METADATA_FILE=metadata.bc ./c2rust-analysis-tests-misc) |
There was a problem hiding this comment.
I think this still might be broken, which is why I hadn't approved it earlier. The ${CARGO_TARGET_DIR:-../target/debug/c2rust-instrument} assumes the cwd is dynamic_instrumentation/ (I think, or at least some other directory), but analysis/tests/misc/Cargo.toml is relative to the repo root. Also, I don't remember if --metadata is supposed to be relative to the cwd or to the manifest dir.
There was a problem hiding this comment.
I pushed a fix that works correctly with the repo root as cwd. Even when building the entire repo with cargo build in the repo root, c2rust-instrument ends up in the debug subdir of the cargo target directory; the other commands already expected to be there.
--metadata is relative to the cwd, which does mean that we need it to include the base path of the Cargo.toml.
2 participants
The invocation here was out-of-date as of #554; the command now works as intended.