rivet icon indicating copy to clipboard operation
rivet copied to clipboard

[Bug]: Read Directory: missing path normalization for portability

Open mindplay-dk opened this issue 1 year ago • 3 comments

What happened?

The Read Directory node doesn't apply any path normalization to it's output - as a result, you can't easily filter with a regex:

image

I suspect this may be why the glob and filter features don't seem to work on Windows either.

the reason I reached for the regex node in this case, was because glob and exclude don't seem to work on Windows - so this issue might well be the root cause of that issue.

note that Windows at the file system API level will canonicalize slashes - hence, for better consistency with Linux and Mac OS paths, using the forward slash on Windows should be no problem.

What was the expected functionality?

Slashes in paths should be normalized to /

Describe your environment

Win 11 Pro

Relevant log output

No response

Relevant screenshots

No response

Code of Conduct

  • [X] I agree to follow this project's Code of Conduct

mindplay-dk avatar Jan 11 '24 11:01 mindplay-dk

Question: is wsl.localhost a mounted volume or a folder name?

codemile avatar Jan 16 '24 14:01 codemile

@abrenneke fixing this so Mac/Linux/Windows all use forward slashes requires adding something like this:

str = path.resolve(str).split(path.sep).join("/");

To the NodeNativeApi file where it calls readdir().

https://github.com/Ironclad/rivet/blob/7fb414acc2680bc98b00783b653e01254bf08469/packages/node/src/native/NodeNativeApi.ts#L31

Do you foresee any risks with such a change? As I don't know how extensively this API is used.

It would probably be best to only do the fix when path.sep !== '/'

codemile avatar Jan 16 '24 14:01 codemile

not seeing any problems with that @codemile

abrenneke avatar Jan 16 '24 16:01 abrenneke