monorepo
monorepo copied to clipboard
replace hardcoded posix paths with platform agnostic solution
Problem
Hardcoded posix paths in the SDK lead to bugs on windows. Likely the reason for #2070
https://github.com/opral/monorepo/blob/b4d952e770d3838f43f693eae0b674b1b2946fac/inlang/source-code/sdk/src/migrations/maybeCreateFirstProjectId.ts#L19-L20
https://github.com/opral/monorepo/blob/b4d952e770d3838f43f693eae0b674b1b2946fac/inlang/source-code/sdk/src/migrations/maybeCreateFirstProjectId.ts#L27-L28
https://github.com/opral/monorepo/blob/b4d952e770d3838f43f693eae0b674b1b2946fac/inlang/source-code/sdk/src/loadProject.ts#L129-L130
https://github.com/opral/monorepo/blob/b4d952e770d3838f43f693eae0b674b1b2946fac/inlang/source-code/sdk/src/loadProject.ts#L144-L145
Proposal
Use normalizePath
from lix-fs
to ensure Windows compatibility.
@samuelstroschein i finally got access to our windows vm and i cannot confirm this causes any issues, project ids work as expected on windows even with those paths. even native powershell has no issues with this:
of course we do not support the old command shell but nearly all current software does not support that anymore.
cc @jldec @felixhaeberle
@janfjohannes Why the cc? Is there something to validate from my side? I constantly hit myself in the foot handling different kind of paths in VSC to an extend where I simply slap normalizePath
to every path I get.
This is surprisingly effective. 😄 But def not elegant.
for example: https://github.com/opral/monorepo/blob/870143a220b092162536f3e1d296703c9dbb2159/inlang/source-code/ide-extension/src/utilities/fs/createFileSystemMapper.ts#L10-L38
@janfjohannes - i expect that you're correct that the code mentioned in the issue where we concatenate paths like /project_id
are probably not the root cause of the windows bugs.
What @felixhaeberle mentioned looks like a good suggestion because it could help avoid scenarios where we mix normalized and non-normalized paths. From my observations earlier today, this is the reason for at least one of the windows failures:
loadProject() calls normalize() before calling maybeCreateFirstProjectId(). This does not work as expected in Windows because the nodeishFs.stat can't find the normalized Windows path and thinks the .inlang directory passed in does not exist.
(i just confirmed that no project id is auto-generated in this windows test)
Another possible vector for these bugs may be confusion (perhaps just on my part) around when to use "absolute" paths vs. relative, and where the paths are rooted (repo-root or fs-root or project-root or cwd)
I'd like to create a new test for windows similar to this one to test the auto-project-id generator.
@janfjohannes is there an easy way to create a (windows equivalent) snapshot like the one used in that test?
@jldec no all that tooling is posix only.
@felixhaeberle just pinged you as you have most windows experience, but also i dont 100% undestand what the windows issues are you experience with vscode extension if windows supprts unix file delimiters and the cli worked in my tests.
@janfjohannes
if windows supports unix file delimiters
Probably that's right for windows, but the VSC extension gives platform specific path delimiters. This causes, when not correctly resolved through path.resolve or simply used without preprocessing through normalizePath
, various issues down the road with the SDK. I can't name them all because most of the time I try to avoid them.
Am I correct that you say it should normally "just work" with not using normalizePath
at all?
The good news is that I can debug loadProject in windows
The ~bad~ other good news is that I can't repro a failure to create the project ID based on my theory from debugging the test above.
Using nodeishFs = node:fs/promises
on Windows, a normalilzed path can be passed to nodeishFs.stat
and it will find the path (@janfjohannes is right, both forward and backslash work).
I think the problem in the test from yesterday was that (unlike node) memoryFS doesn't handle windows paths.
@felixhaeberle i see, i think i need to get into the vscode extension more to fully understand our requirements there. not saying anything about normalizePath that does more than just touching delimiters