Skip to content

Comments

Add missing _optionally_create_directory in Android user_log_dir and user_runtime_dir#452

Open
bysiber wants to merge 3 commits intotox-dev:mainfrom
bysiber:fix/android-ensure-exists-opinion
Open

Add missing _optionally_create_directory in Android user_log_dir and user_runtime_dir#452
bysiber wants to merge 3 commits intotox-dev:mainfrom
bysiber:fix/android-ensure-exists-opinion

Conversation

@bysiber
Copy link

@bysiber bysiber commented Feb 20, 2026

Summary

When ensure_exists=True is set, the Unix implementation correctly calls _optionally_create_directory on the opinionated subdirectory for user_log_dir (the log subdir). However, the Android implementation is missing these calls for both user_log_dir and user_runtime_dir.

Problem

On Android, calling user_log_dir or user_runtime_dir with ensure_exists=True and opinion=True does not create the opinionated subdirectories (log and tmp respectively), even though the parent cache directory is created. This is inconsistent with the Unix implementation which does call _optionally_create_directory after appending the log subdirectory.

For example, comparing user_log_dir in both platforms:

Unix (correct):

if self.opinion:
    path = os.path.join(path, "log")
    self._optionally_create_directory(path)

Android (before this fix):

if self.opinion:
    path = os.path.join(path, "log")
# missing _optionally_create_directory call

Fix

Added self._optionally_create_directory(path) calls in user_log_dir and user_runtime_dir within the if self.opinion: blocks, matching the existing Unix behavior.

…ntime_dir

When ensure_exists=True, the Unix implementation correctly calls
_optionally_create_directory on the opinionated subdirectory (e.g.
the 'log' subdir within user_log_dir). The Android implementation
was missing these calls, so the 'log' and 'tmp' subdirectories
would not be created even when ensure_exists was set.

This adds the missing _optionally_create_directory calls in
user_log_dir and user_runtime_dir on Android to match the Unix
behavior.
Copy link
Member

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

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

Test?

Verify that user_log_dir and user_runtime_dir create the opinion
subdirectories (log/ and tmp/) when ensure_exists=True.
@bysiber
Copy link
Author

bysiber commented Feb 21, 2026

Added a parametrized test that verifies user_log_dir and user_runtime_dir actually create the opinion subdirectories (log/ and tmp/) when ensure_exists=True.

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