fix std.fs.Dir.makePath silent failure #16878
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Would be good to add a test case to |
squeek502
reviewed
Aug 19, 2023
squeek502
reviewed
Aug 19, 2023
|
I updated the solution per your above comments and added a test to std/fs/test.zig. Tests on linux & windows OK - I don't have the capability to test on MacOS but don't anticipate that this should be significantly different. |
squeek502
reviewed
Aug 19, 2023
|
Made a PR that will fix the WASI compilation error you're hitting: #17029 (it's unrelated to your changes, |
squeek502
reviewed
Sep 1, 2023
f94ef1a to
d4a8bd0
Compare
std.fs.dir.makePath silently fails if one of the items in the path already exists. For example:
cwd.makePath("foo/bar/baz")
Silently failing is OK if "bar" is already a directory - this is the intended use of makePath (like mkdir -p). But if bar is a file then the subdirectory baz cannot be created - the end result is that makePath doesn't do anything which should be a detectable error because baz is never created.
The existing code had a TODO comment that did not specifically cover this error, but the solution for this silent failure also accomplishes the TODO task - the code now stats "foo" and returns an appropriate error. The new code also handles potential race condition if "bar" is deleted/permissions changed/etc in between the initial makeDir and statFile calls.
d4a8bd0 to
3f6c199
Compare
|
Thank you @amiralawi, and @squeek502 for helping get this PR to completion. |
bilaliscarioth
pushed a commit
to bilaliscarioth/zig
that referenced
this pull request
Jan 27, 2024
std.fs.dir.makePath silently failed if one of the items in the path already exists. For example:
cwd.makePath("foo/bar/baz")
Silently failing is OK if "bar" is already a directory - this is the intended use of makePath (like mkdir -p). But if bar is a file then the subdirectory baz cannot be created - the end result is that makePath doesn't do anything which should be a detectable error because baz is never created.
The existing code had a TODO comment that did not specifically cover this error, but the solution for this silent failure also accomplishes the TODO task - the code now stats "foo" and returns an appropriate error. The new code also handles potential race condition if "bar" is deleted/permissions changed/etc in between the initial makeDir and statFile calls.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
std.fs.dir.makePath silently fails if one of the items in the path already exists. For example:
cwd.makePath("foo/bar/baz")
Silently failing is OK if "bar" is already a directory or a symlink that resolves to a directory - this is the intended use of makePath (like mkdir -p). But if bar is a file then the subdirectory baz cannot be created - the end result is that makePath doesn't do anything and never creates the directory baz which should be a detectable error.
The existing code had a TODO comment that did not specifically cover this error, but the solution for this silent failure also accomplishes the TODO task - the code now stats "foo" and returns an appropriate error if necessary. The new code also handles potential race condition if "bar" is deleted/permissions changed/etc in between the initial makeDir and statFile calls.