Reloading fails on Windows with Unix search path

[gabdube]

On windows, if a unix path is used in "search_paths" (ex: DynamicReload::new(Some(vec!["./routes/target/debug/"]), Some("target/debug"), Search::Default); ), the changes to the libraries are not reported. This is most likely because dynamic_reload cannot match the path sent by the watcher to the one saved in the Lib.

Here's an example of events that fails to be matched ex: RawEvent { path: Some("C:\\Users\\Gab\\Documents\\projects\\test\\./routes/target/debug\\routes.dll"), op: Ok(WRITE), cookie: None }

I've managed to fix this by mapping the path in search_paths with .canonicalize. Therefore replacing search_paths: Vec<&'a str> by search_paths: Vec<PathBuf>

With this the event becomes

RawEvent { path: Some("\\\\?\\C:\\Users\\Gab\\Documents\\projects\\test\\routes\\target\\debug\\routes.dll"), op: Ok(WRITE), cookie: None }

And everything works fine. Not exactly sure how this would impact non Windows OS though...

[emoon]

Hi. Thanks for the report. Could you try to make a PR with this change? As I have some CI running for non-Windows platforms it should report if something goes bad.

Cheers!

[gabdube]

So the tests didn't pass because canonicalize returns an Err if it's called on a path that do not exist (ex: "../test"). To fix the issue, I've added a fallback that use the non-canon path in that case.

I assume it's OK to pass a path that do not exist just in case the application creates the folder after DynamicReload was is instanced.

[emoon]

Yeah. It may be a directory that gets populated later and things like that.

[Boscop]

@gabdube Does that mean if the folder is only created later (so the path only becomes valid after DynamicReload::new/get_search_paths is called), it doesn't get canonicalized, so the issue still happens in that case?

[gabdube]

Just tested it and it panics. Do you think we could just create the path if it doesn't already exists?

[Boscop]

@gabdube I think it would make more sense to canonicalize the path as soon as it exists and the dll is loaded.

[gabdube]

@Boscop Not sure if that's possible because we are sending the path to the watcher and at that point its out of our control.

[Boscop]

@gabdube Ah right. I think the best solution would be to compare paths in a way that treats / and \ the same. This crate could be useful: https://crates.io/crates/relative-path Or this one (probably even more useful): https://crates.io/crates/path-slash (But it could also be done in-place to avoid allocations.)

[Boscop]

There's a new crate that could be very useful for this issue: https://www.reddit.com/r/rust/comments/jqx8hf/normpath_more_reliable_path_manipulation/

normpath - a crate meant to make correct path manipulation easier. It lets you to avoid doing canonicalization that is often unnecessary on Windows and unreliable: #45067, #48249, #52440, #55812, #58613, #59107, #74327. Canonicalization can cause errors for some valid paths (e.g., shared partitions) and always returns a verbatim path, which is usually unexpected. Normalization is probably what you want instead, which this crate provides with PathExt::normalize.

It also defines BasePath and BasePathBuf, which improve the API of Path and PathBuf and handle more edge cases when joining paths. For example, you can use them to join verbatim paths without custom workarounds.

[Boscop]

On nightly, std now has a function to make paths absolute without requiring the file to exist!

https://doc.rust-lang.org/nightly/std/path/fn.absolute.html

This would solve this issue but it's only on nightly for now.