Skip to content

Conversation

jjonescz
Copy link
Member

No description provided.

Copy link
Contributor

This PR is targeting main, which is now for .NET 11-facing work. If you intended to target .NET 10, either retarget this PR to release/10.0.1xx or make sure you backport the change to release/10.0.1xx after merging. See #50394 for more details.

@jjonescz jjonescz changed the base branch from main to release/10.0.1xx August 22, 2025 08:20
Copy link
Contributor

@KalleOlaviNiemitalo KalleOlaviNiemitalo left a comment

Choose a reason for hiding this comment

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

I hope this will address #20183.

Comment on lines 109 to 110
// NOTE: This modifies the permissions if needed, and throws if not possible.
Directory.CreateDirectory(path, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute);
Copy link
Contributor

@KalleOlaviNiemitalo KalleOlaviNiemitalo Aug 22, 2025

Choose a reason for hiding this comment

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

The comment there looks misleading.

If the directory exists already, then CreateDirectory ignores the UnixFileMode, except throws if the argument contains undefined bits.

If the directory does not exist yet, then CreateDirectory passes the UnixFileMode to the mkdir system call, which uses it when creating the directory. But the umask of the process may cause mkdir not to grant all the permissions listed in the UnixFileMode. That would be the user's misconfiguration though; it is not reasonable to expect programs to work if the umask prevents permissions from being granted to the user.

Copy link
Member Author

@jjonescz jjonescz Aug 22, 2025

Choose a reason for hiding this comment

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

Thanks, that's indeed not what I've expected. But I think the behavior is acceptable, I will just update the comment.

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

Successfully merging this pull request may close these issues.

3 participants