Skip to content

Conversation

@silverweed
Copy link
Contributor

@silverweed silverweed commented Nov 26, 2025

Currently this doesn't work as expected:

auto dir1 = file->mkdir("a/b");
auto dir2 = file->mkdir("a/b", "", /* returnExisting = */ true); // should return the already-existing dir
EXPECT_EQ(dir1, dir2); // FAILS: dir2 is nullptr

because TDirectoryFile::mkdir doesn't propagate the value of returnExistingDirectory to recursive mkdir calls. This PR fixes it.

In addition, title was currently only applied to the topmost directory in case of hierarchies, which is counterintuitive as the returned TDirectory is actually the innermost. This PR changes the (previously undocumented) title logic to the opposite: every non-leaf directory gets created with a default title and only the innermost gets the user-defined one. In our codebase we have 0 cases of mkdir being called with a hierarchy and an explicit title, so this change shouldn't impact anything.

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

@silverweed silverweed self-assigned this Nov 26, 2025
@silverweed silverweed requested a review from pcanal as a code owner November 26, 2025 10:38
@silverweed silverweed mentioned this pull request Nov 26, 2025
2 tasks
@github-actions
Copy link

github-actions bot commented Nov 26, 2025

Test Results

    21 files      21 suites   3d 22h 14m 6s ⏱️
 3 786 tests  3 786 ✅ 0 💤 0 ❌
77 561 runs  77 561 ✅ 0 💤 0 ❌

Results for commit c37b3bf.

♻️ This comment has been updated with latest results.

Copy link
Member

@pcanal pcanal left a comment

Choose a reason for hiding this comment

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

Thanks.

@silverweed silverweed merged commit 64922e4 into root-project:master Dec 8, 2025
28 of 29 checks passed
@silverweed silverweed deleted the tdirectory_mkdir branch December 8, 2025 07:06
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