Skip to content

mock: add Eio_mock.Dir with POSIX and Windows path syntax - #908

Open
avsm wants to merge 2 commits into
ocaml-multicore:mainfrom
avsm:win-path-pi
Open

mock: add Eio_mock.Dir with POSIX and Windows path syntax#908
avsm wants to merge 2 commits into
ocaml-multicore:mainfrom
avsm:win-path-pi

Conversation

@avsm

@avsm avsm commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

( Update: most of this got merged in other PRs. Now this PR is just adding the mocks. }

Opening as a draft PR to check the approach and interface. This Incorporates the diffs from #738 as well

When implementing of_native (which is important for cmdliner arg parsing and config files), there's sufficient spec divergence even for the POSIX layer that we can't use the stdlib modules. Those vary behaviour based on the host, whereas our POSIX and Windows code is dispatched via the Eio Pi layer.

I'm still testing out some of the Windows parsing logic (between Cygwin and WSL) and doing some fuzzing, as that parsing logic is also quite complex! See the test cases for various permutations.

Also one question is whether the path operations should be a module type PATH that is then included into DIR.

@talex5 talex5 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a good approach.

I don't understand the of_native thing, though:

It seems to be the same as / (join), except that it also cleans the path up a bit? The name seems confusing, then.

Having a function to normalise a path is fine, but I don't think it has anything to do with being native or not (elsewhere, a "native" path is a plain string).

The path current/.. is not the same as . (current may be a symlink, and might change), so normalising isn't something you want to do by default.

Python just leaves in components that can't be normalised, which is probably better than rejecting them. e.g.

>>> import os
>>> os.path.normpath('foo/../bar')
'bar'
>>> os.path.normpath('../bar')
'../bar'

Note that the confinement doesn't depend on the string part of Path.t being sanitised.

Comment thread lib_eio/unix/eio_unix.mli Outdated
@avsm

avsm commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Right, the intention of of_native is to have a reasonable behaviour for a cmdliner parser which is taking external (native) paths and resolving them against some directory base. I do agree that symlink behaviour seems important here; I tried a purely lexical approach but this cleanup does require fs access. I'll read the Python approach and see if I can simplify this.

@talex5

talex5 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Right, the intention of of_native is to have a reasonable behaviour for a cmdliner parser which is taking external (native) paths and resolving them against some directory base.

But why do they need to be cleaned up at all? e.g. eio-trace takes strings from cmdliner and just combines them with fs:

https://github.com/ocaml-multicore/eio-trace/blob/ce5fb3cfd516783c13cc6ae1962459a0c57a2514/src/main.ml#L96-L99

I don't see what about command-line parsing requires normalisation in general.

@avsm

avsm commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

It's nice to be able to validate an external path before actually using it; for example turning "foo/../../bar" into something meaninful with respect to a root directory. It's possible to lexically clamp that when supplied with a root without ever touching the filesystem.

However, it is complex on Windows to do this, and we'll eventually need to worry about symlinks no matter what. So just treating it as an opaque string and just splitting on the volume/basepath could work as well. I'll give that a try and see what it looks like.

@avsm

avsm commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Pushed a version that removes the of_native and simplifies the diff considerably! I'm still differentially testing the Windows parser against Python's https://github.com/python/cpython/blob/main/Lib/ntpath.py and it mostly agrees (except for NTPATHs, which it doesnt treat as verbatim)

@avsm
avsm marked this pull request as ready for review July 28, 2026 16:56
@talex5

talex5 commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Also one question is whether the path operations should be a module type PATH that is then included into DIR.

Makes sense - then the backend can just use a module alias.

Since these path modules don't depend on the OS, we could even put them in eio.utils (e.g. as Eio_utils.Posix_path and Eio_utils.Nt_path). Then the mocks could provide mock POSIX and mock Windows directories using them, which might be useful for testing.

@avsm

avsm commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

This is getting cleaner! 68f2a9e moves them into eio.utils, and 98d678a adds a mock layer (I think functorising over posix/win32 is cleanest here, but can use FCMs if you think its better)

One behavioural change on Windows: after looking at Go and Rust, I changed the serialisation to always favour \ instead of / (even though the latter is allowed). It feels more natural to get c:\foo\bar rather than c:\foo/bar out when using Windows path syntax.

Comment thread lib_eio/fs.ml
val chown : follow:bool -> ?uid:int64 -> ?gid:int64 -> t -> path -> unit
val pp : t Fmt.t

val native : t -> string -> string option

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left this inside DIR for backwards compatibility

avsm added 2 commits August 5, 2026 11:09
This also makes a behavioural change I noticed while differentially
checking Go/Rust/Python. We prefer a \ when serialising Windows paths,
even though / is technically allowed. This seems less surprising
Trim out the mock_dir test; it was useful to visualise some of
the path operations, but is mostly duplicated with the real tests
now.
@talex5 talex5 changed the title implement Eio.Path.native as a Pi backend and add Windows support mock: add Eio_mock.Dir with POSIX and Windows path syntax Aug 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants