Skip to content

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

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

Merged
merged 12 commits into from
Dec 18, 2024

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
29 checks passed
@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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants