Skip to content
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

Ensure tox-created directories contain CACHEDIR.TAG #3342

Closed
wants to merge 6 commits into from

Conversation

akx
Copy link

@akx akx commented Sep 10, 2024

This PR ensures that Tox-created work directories contain a CACHEDIR.TAG file, which can be used to exclude the directories from e.g. backups.

Fixes #3334

  • ran the linter to address style issues (tox -e fix)
  • wrote descriptive pull request text
  • ensured there are test(s) validating the fix
  • added news fragment in docs/changelog folder
  • updated/extended the documentation

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.

Why create these in sub folders? Why not just created it within the .tox root directory and be done with it?

With this implementation, the .tox directory itself won't contain the tag file, since there is no explicit mkdir call for the workdir. Only the subdirectories explicitly created by calls around Tox will be populated with the file.

Just because there isn't does not mean we can't add it.

@akx
Copy link
Author

akx commented Sep 10, 2024

Why not just created it within the .tox root directory and be done with it?

I felt there was an idea behind the decision to not have an explicit call to create that directory – maybe that the workdir wouldn't be eagerly created before it's needed (just to hold a subdirectory).

Just because there isn't does not mean we can't add it.

Sure, I'll do that. Would you prefer eagerly creating the workdir with the cache tag, or having the places that create those workdir subdirs ensure the workdir has the cache tag?

@gaborbernat
Copy link
Member

having the places that create those workdir subdirs ensure the workdir has the cache tag

Probably this 🤔

@akx
Copy link
Author

akx commented Sep 11, 2024

having the places that create those workdir subdirs ensure the workdir has the cache tag

Probably this 🤔

It turned out a little trickier like that, since an user could point Tox to a pre-created workdir, which we then certainly do not want to litter with a CACHEDIR.TAG file.

I still didn't write tests for this incarnation, just in case you'd like to take a review look first.

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.

The CI is fialing

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.

Needs tests too 👍

@gaborbernat gaborbernat marked this pull request as draft September 11, 2024 18:06
@gaborbernat
Copy link
Member

@akx you still plan to finish this or should I close it?

@akx
Copy link
Author

akx commented Oct 1, 2024

@akx you still plan to finish this or should I close it?

Yeah, it's still under construction – just been busy with work and figuring out how to best test-cover this. Rebased & added a commit that has some WIP tests :)

@gaborbernat
Copy link
Member

No worries, but note the CI is still failing 😊

@gaborbernat
Copy link
Member

@akx friendly bump here 👍

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

Successfully merging this pull request may close these issues.

.tox folder should maybe contain a CACHEDIR.TAG
2 participants