tell Cargo about the config.yml dependency for yarp
#1300
Conversation
There was a problem hiding this comment.
Great find,
I am not a rust expert, but found the docs here seem to explain what is happening: https://doc.rust-lang.org/cargo/reference/build-scripts.html#build-scripts
| let file = std::fs::File::open(config_path())?; | ||
| let path = config_path(); | ||
| let file = std::fs::File::open(&path)?; | ||
| println!("cargo:rerun-if-changed={}", path.display()); |
There was a problem hiding this comment.
path.display() escapes invalid UTF-8 bytes. This code will not behave correctly if a path contains a \xFF on Unix systems or an unpaired surrogate on Windows. I previously delivered feedback on both of these crate build scripts that this code should call path.to_str().unwrap() since using println! to emit a directive to cargo like this requires that the path actually be UTF-8.
There was a problem hiding this comment.
@lopopolo I didn't see that feedback; is it possible that you can point me to it?
There was a problem hiding this comment.
I left several comments of this sort on the initial PRs that introduced the yarp-sys and yarp crates.
Using Path::display to output a Path with println! is a lossy operation because println! requires UTF-8 and Paths on Unix and windows platforms are not required to be UTF-8.
Rather than print out a cargo directive with a lossy conversion which will cause the build to fail in odd ways further down the pipe, prefer to fail fast and require a UTF-8 path at the time it is used as such.
Otherwise we get peculiar build errors because Cargo doesn't think anything has changed.