Skip to content

Fix make_temp: revert fallback to working_directory #720

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

simant01
Copy link

This pull request fixes an issue introduced by a recent commit that changed the temporary file creation behavior in make_temp

Previously, make_temp used self.working_directory and a default prefix of 'devlib-test', which provided the necessary permissions for temporary file cleanup. The change to using self.tmp_directory caused permission errors during cleanup and consequently breakages.

This PR reverts the change by:

  • Setting the default prefix to 'devlib-test'
  • Reverting the fallback directory from self.tmp_directory back to self.working_directory

@douglas-raillard-arm
Copy link
Collaborator

Instead of reverting that change, as tmp_directory's only purpose is to be the root of temporary locations, could you give a go at fixing the logic that computes a sane default for it ?
In particular:

def _resolve_paths(self):

@douglas-raillard-arm
Copy link
Collaborator

Also if you can share details on your setup, the specific paths involved and the specific error you encounter that would be useful. AFAICT tmp_directory can only be derived from the following sources:

  1. User-given. I assume it's not what is happening
  2. From mktemp -d call. If that returns a folder that is not writeable (which I assume is the problem ?) your platform is pretty broken and you may need to set TMPDIR manually in some shell configuration file.
  3. /data/local/tmp, which AFAIR is an ok location on android (or used to be)
  4. From working_directory, which I assume is not the case considering your report.

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