Closed iawia002 closed 6 days ago
Thanks for the PR! Personally though I find path manipulation very tricky/subtle. For example in the presence of symlinks I'm not sure that this implementation is correct. In lieu of that perhaps std::fs::canonicalize
could be used instead? If that returns Ok
then the resulting path can be used and if it returns Err
then the original path can be used as-is to probably return an error somewhere else.
Thanks for the review, I agree that path manipulation is tricky, std::fs::canonicalize
is more of a compromise solution. It can handle some scenarios (such as when the path itself exists but the wit
file does not). However, if the path itself does not exist, the error message will still be the same as it is now. But I believe this is acceptable because we would rather not incorrectly normalize the path in some corner cases.
I encountered a file path issue while using the
wasmtime::component::bindgen!
macro, I used a relative file path (e.g.,../wit
) in thepath
parameter, and then I received an error message:The file path in the error message was quite confusing, in reality, the path that was not found should be
/Users/code/test-component/wit
. The relative file path in the error message made it hard for me to immediately understand which directorywasmtime::component::bindgen!
was using to look for WIT files. Therefore, this patch normalizes thepath
before resolving WIT files.Error message before:
after:
The
wit_bindgen::generate!
macro has the same issue. If this patch is reasonable, I will make the same changes towit_bindgen::generate!
afterward.