Skip to content

Conversation

HamzaHassanain
Copy link

The createDirectory function will succeed without error if the target directory already exists.

Motivation

This change addresses issue #3404. Currently, fileSystem.createDirectory fails if the target directory already exists, forcing users to write boilerplate try/catch blocks to handle this common and expected case.

The goal is to make this function's behavior idempotent.

Modifications

To achieve this, I've made the following changes:

(Implementation) A new private helper function, _handleCreateDirectoryFileExists, was introduced. This function is responsible for:

  1. Performing a stat call on the path that failed.
  2. Checking if the existing item is a directory (S_IFDIR).
  3. Returning a success result if it's a directory, or re-throwing the original .fileExists error if it's a file or another type of entity.

(Logic) The core _createDirectory function was updated to call this new helper function whenever Syscall.mkdir fails with an EEXIST (.fileExists) error. This check is applied in both internal loops to correctly handle cases where either an intermediate directory or the final target directory already exists.

Result

With this change, users can now call the function fileSystem.createDirectory and the operation will succeed even if the directory is already present, leading to cleaner and more predictable code.

Copy link
Contributor

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

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

Thanks! I've left some feedback inline. We'll also need some tests for this though.

Comment on lines +807 to +825
/// Handle _createDirectory failure if the directory already exists.
private func _handleCreateDirectoryFileExists(
at path: FilePath,
) -> Result<Void, FileSystemError> {
// This function is called when _createDirectory fails with (errno = .fileExists),
// - If the path exists and is a directory, if so return success.
// - If the path exists and is not a directory, return failure.
switch Syscall.stat(path: path) {
case .success(let stat):
// Check if it's a directory
if (stat.st_mode & S_IFMT) == S_IFDIR {
return .success(())
} else {
return .failure(.mkdir(errno: .fileExists, path: path, location: .here()))
}
case let .failure(errno):
return .failure(.mkdir(errno: errno, path: path, location: .here()))
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need a whole new function for this, we already have the _info function which does most of this for you.

Comment on lines +886 to +888
if errno == .fileExists {
return self._handleCreateDirectoryFileExists(at: path)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't necessary: we're creating child directories here which we'll only do if it didn't already exist.

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.

2 participants