Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Too easy to reach MAX_PATH on windows #11303

Open
hhugo opened this issue Jan 15, 2025 · 10 comments
Open

Too easy to reach MAX_PATH on windows #11303

hhugo opened this issue Jan 15, 2025 · 10 comments
Labels

Comments

@hhugo
Copy link
Collaborator

hhugo commented Jan 15, 2025

Context

Windows has big constraint on file-path length. See https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation?tabs=registry

Some combinaison of dune features can result in file path that are quite long, increasing the probability of reaching the limit.

Here is an example involving sandboxing, inline-test and wasm_of_ocaml (236 chars)

_build/.sandbox/f61526bdca960dd7983254616ff32625/default/lib/base_bigstring/test/.base_bigstring_test.inline-tests/inline_test_runner_base_bigstring_test.bc.wasm.assets/dune__exe__Inline_test_runner_base_bigstring_test-2e87e8ef.wasm.map

Changing name and layout of the test to make the path as small as possible (inline test named t at the root), you get 158 chars.

_build/.sandbox/f61526bdca960dd7983254616ff32625/default/.t.inline-tests/inline_test_runner_t.bc.wasm.assets/dune__exe__Inline_test_runner_t-2e87e8ef.wasm.map

Question

Can we change some naming to make paths smaller ?
Can we do something to hint users about enabling long paths ? I don't know how much this can be a solution..

@hhugo
Copy link
Collaborator Author

hhugo commented Jan 15, 2025

@dra27, do you have anything to say about this ?

@rgrinberg
Copy link
Member

I suppose we can disable sandboxing if limit of the path goes above 255? Not really sure what else we could do.

@hhugo
Copy link
Collaborator Author

hhugo commented Jan 16, 2025

Here are few ideas:

@dra27
Copy link
Member

dra27 commented Jan 16, 2025

cf. a recent foray in ocaml/ocaml#13444. The TL;DR from my perspective was "Windows long path support doesn't work, or certainly not well enough to be turning it on automatically"!

I think anything which requires the user to reconfigure their machine is a mistake in terms of general OCaml/Dune usability. Given that all these paths are internal to Dune, Dune could manually transform paths so that it can access long paths (using the \\? et al notations) - but this is still a usability problem, because it means that _build becomes tricky to access from shells (I think the directory becomes hard to delete from the Windows shell, for example).

FWIW - and I realise it adds a layer of complexity - I'd index/hash these names on all systems, and keep the lengths down - it's not like very long paths don't run into occasional problems on Unix too (cf. shebang issues, etc.).

@hhugo
Copy link
Collaborator Author

hhugo commented Jan 16, 2025

Quoting a comment from ocaml/ocaml#13444 (comment)

Honestly, there needs to be a separate issue for Dune. dune.exe is always the reason I see MAX_PATH problems since dune generates obscenely long files like _build/default/DkHelloScript_Std/Y33ArticleX/.DkHelloScript_Std__Y33ArticleX__S044Lst3Parties.objs/byte\dkHelloScript_Std__Y33ArticleX__S044Lst3Parties__S044Lst3Parties.cmt48559e.tmp, even after trying to shorten module names by embedding numbers.

cc @jonahbeckford, to make sure you see the current issue

@hhugo
Copy link
Collaborator Author

hhugo commented Jan 16, 2025

I've a PoC with some renaming: #11307
One can remove some redundant information from path. Here the name of the lib for the inline test doesn't need to appear under .<LIBNAME>.inline-tests directory.

length = 155

_build/default/compiler/tests-jsoo/.j__comp.inline-tests/inline_test_runner_j__comp.bc.wasm.assets/dune__exe__Inline_test_runner_j__comp-aa3bf309.wasm.map

becomes

length = 109

_build/default/compiler/tests-jsoo/.j__comp.inline-tests/run.bc.wasm.assets/dune__exe__Run-65ca7159.wasm.map

@hhugo
Copy link
Collaborator Author

hhugo commented Jan 16, 2025

and 134

_build/default/compiler/tests-jsoo/.j__comp.inline-tests/.j__comp.inline-tests.eobjs/jsoo/dune__exe__Inline_test_runner_j__comp.wasmo

becomes 94

_build/default/compiler/tests-jsoo/.j__comp.inline-tests/.run.eobjs/jsoo/dune__exe__Run.wasmo

@hhugo
Copy link
Collaborator Author

hhugo commented Jan 16, 2025

Note that the example above use a short name for inline-tests because of MAX_PATH.
With more realistic names, I get the following reduction in size 200 -> 124

@rgrinberg
Copy link
Member

Here the name of the lib for the inline test doesn't need to appear under ..inline-tests directory

But in general, we could have more than one library per directory defining inline tests.

@hhugo
Copy link
Collaborator Author

hhugo commented Jan 16, 2025

many libs with inline tests can live together, one segment of the path still contains the libname (.<LIBNAME1>.inline-tests and .<LIBNAME2>.inline-tests are fine next to each-other)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants