Skip to content

Fix mkdir("a/b/..") should return EEXIST#23136

Merged
hoodmane merged 12 commits intoemscripten-core:mainfrom
hoodmane:mkdir-dotdot-eexist
Dec 18, 2024
Merged

Fix mkdir("a/b/..") should return EEXIST#23136
hoodmane merged 12 commits intoemscripten-core:mainfrom
hoodmane:mkdir-dotdot-eexist

Conversation

@hoodmane
Copy link
Collaborator

Before this change mkdir("a/b/..") surprisingly makes a directory called a/b/a. It should raise EEXIST.

Before this change `mkdir("a/b/..")` surprisingly makes a directory called `a/b/a`.
It should raise EEXIST.
@hoodmane
Copy link
Collaborator Author

Okay I updated this to reflect #23177 and #23180.

@hoodmane hoodmane merged commit c26d805 into emscripten-core:main Dec 18, 2024
@hoodmane hoodmane deleted the mkdir-dotdot-eexist branch December 18, 2024 19:16
hedwigz pushed a commit to hedwigz/emscripten that referenced this pull request Dec 18, 2024
Before this change `mkdir("a/b/..")` surprisingly makes a directory
called `a/b/a`. It should raise EEXIST.
@dschuff
Copy link
Member

dschuff commented Dec 18, 2024

Looks like this is another case where the errno values don't match on Windows:
https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket/8728179723714842673/+/u/Emscripten_testsuite__core0_/stdout

I don't know if we want to run all the tests on Windows all the time, but I wonder if it makes sense to have an optional way to run extra tests on Windows for changes like this that might affect it. I know that's possible with the Chromium CI, maybe we can do it on CircleCI too.

@sbc100
Copy link
Collaborator

sbc100 commented Dec 18, 2024

I think we just need to remember to mark all nodefs tests as @crossplatform

sbc100 added a commit to sbc100/emscripten that referenced this pull request Dec 19, 2024
This test was recently added in emscripten-core#23136 but the test fails on windows
due to errno mismatch.  See emscripten-core#8882.
sbc100 added a commit that referenced this pull request Dec 19, 2024
This test was recently added in #23136 but the test fails on windows due
to errno mismatch. See #8882.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants