Ensure POSIX path normalizing behavior under NODERAWFS

[kleisauke]

Avoids mangling C:/tmp to C:\tmp. Should be safe, the underlying Windows API can accept either the backslash or slash to separate directory and file components of a path.

See: https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file

Ensures NODERAWFS compatibility on Windows that was regressed since #18163.

[sbc100]

I'm not sure how this solves the issues since surely /dev/stdin simply doesn't exist on windows (no matter which slashes you use right?)

Can you add @crossplatform to test_noderawfs too?

[sbc100]

Can you mention in the title or description that this is fix for NODERAWFS on windows which was broken by #18163

[kleisauke]

I'm not sure how this solves the issues since surely /dev/stdin simply doesn't exist on windows (no matter which slashes you use right?)

/dev/stdin is a virtualized path on the filesystem which is reused under NODERAWFS after PR #18163.

emscripten/src/library_noderawfs.js

Lines 19 to 21 in cd097fb

	// Call the original FS.init, this will setup the
	// stdin, stdout and stderr devices
	VFS.init(input, output, error);

emscripten/src/library_fs.js

Lines 1415 to 1419 in cd097fb

	if (Module['stdin']) {
	FS.createDevice('/dev', 'stdin', Module['stdin']);
	} else {
	FS.symlink('/dev/tty', '/dev/stdin');
	}

emscripten/src/library_fs.js

Lines 1357 to 1363 in cd097fb

	// setup /dev/tty and /dev/tty1
	// stderr needs to print output using err() rather than out()
	// so we register a second tty just for it.
	TTY.register(FS.makedev(5, 0), TTY.default_tty_ops);
	TTY.register(FS.makedev(6, 0), TTY.default_tty1_ops);
	FS.mkdev('/dev/tty', FS.makedev(5, 0));
	FS.mkdev('/dev/tty1', FS.makedev(6, 0));

Can you add @crossplatform to test_noderawfs too?
Can you mention in the title or description that this is fix for NODERAWFS on windows which was broken by #18163

Done.

[kleisauke]

BTW, I'm not sure why the Windows CI actually did pass on #18163. I added @crossplatform to test_module_stdin and test_module_stdout_stderr in that PR, so I expected that test_module_stdin_rawfs and test_module_stdout_stderr_rawfs were also executed under Windows.

[sbc100]

FS.symlink('/dev/tty', '/dev/stdin');

When this line runs in linux will it actually create symlink in the real /dev/ directory when running under NODERAWFS?

[sbc100]

BTW, I'm not sure why the Windows CI actually did pass on #18163. I added @crossplatform to test_module_stdin and test_module_stdout_stderr in that PR, so I expected that test_module_stdin_rawfs and test_module_stdout_stderr_rawfs were also executed under Windows.

Looks like we were missing @wraps in also_with_noderawfs. I'm got a PR coming to fix that.

[kleisauke]

When this line runs in linux will it actually create symlink in the real /dev/ directory when running under NODERAWFS?

No, since the overwriting occurs later, a virtualized symlink is used.

emscripten/src/library_noderawfs.js

Lines 36 to 41 in cd097fb

	// Wrap the whole in-memory filesystem API with
	// our Node.js based functions
	for (var _key in NODERAWFS) {
	/** @suppress {partialAlias} */
	FS[_key] = _wrapNodeError(NODERAWFS[_key]);
	}

[kleisauke]

Looks like we were missing @wraps in also_with_noderawfs.

If you want, I can add that in this PR to verify its correctness.

[sbc100]

Looks like we were missing @wraps in also_with_noderawfs.

If you want, I can add that in this PR to verify its correctness.

Yup, that makes sense. We can verify the test actually runs. I will still send a PR to make all decorators use @wraps

[sbc100]

BTW, if windows accepts both slashes, why is this change needed?

[kleisauke]

Yup, that makes sense. We can verify the test actually runs. I will still send a PR to make all decorators use @wraps

Great! I just pushed commit eb5869f.

BTW, if windows accepts both slashes, why is this change needed?

Since the virtualized/in-memory file system relies on POSIX/UNIX-style separators.

Details

emscripten/src/library_fs.js

Line 55 in cd097fb

currentPath: '/',

emscripten/src/library_fs.js

Line 195 in cd097fb

var parts = path.split('/').filter((p) => !!p);

emscripten/src/library_fs.js

Line 199 in cd097fb

var current_path = '/';

emscripten/src/library_fs.js

Line 244 in cd097fb

return mount[mount.length-1] !== '/' ? `${mount}/${path}` : mount + path;

emscripten/src/library_fs.js

Line 572 in cd097fb

var root = mountpoint === '/';

emscripten/src/library_fs.js

Line 695 in cd097fb

var dirs = path.split('/');

emscripten/src/library_fs.js

Line 699 in cd097fb

d += '/' + dirs[i];

emscripten/src/library_fs.js

Line 1450 in cd097fb

FS.mount(MEMFS, {}, '/');

emscripten/src/library_fs.js

Line 1533 in cd097fb

ret.isRoot = lookup.path === '/';

emscripten/src/library_fs.js

Line 1541 in cd097fb

var parts = path.split('/').reverse();

(i.e. that's grep-ing for '/')

[sbc100]

I guess we can/should revert this change too?

[kleisauke]

Indeed, we should probably revert this too after PR #22393 lands.

Sorry for the trouble caused, the good thing is that this found that decorator bug. :)