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

Use a single find command to delete pycache files #3562

Merged
merged 1 commit into from
May 20, 2024

Conversation

echoix
Copy link
Collaborator

@echoix echoix commented May 17, 2024

I saw some failures that happened when deleting files in the pycache directories. It shouldn’t be happening, and what I suspect is the fact that both individual files (pyc and pyo files) were deleted with rm -rf, but also the directory containing them (__pycache__). Probably that it caused a race condition, where the folder was deleted (recursively), and afterwards a file that was inside was called to be deleted.

According to https://www.gnu.org/software/findutils/manual/html_mono/find.html#Deleting-Files, this way is not only safer, but also more efficient. (It should be even better since we had a pipeline in the way here).

If you’d like, some commands to delete npm unneeded files could be merged together in a single find call (and traversal).

Proposed Changes

Readiness Checklist

Author/Contributor

  • Add entry to the CHANGELOG listing the change and linking to the corresponding issue (if appropriate)
  • If documentation is needed for this change, has that been included in this pull request

Reviewing Maintainer

  • Label as breaking if this is a large fundamental change
  • Label as either automation, bug, documentation, enhancement, infrastructure, or performance

@nvuillam
Copy link
Member

nvuillam commented May 19, 2024

Great @echoix :)
I solved the CI build issue, so it should pass if you update your branch :)

@echoix
Copy link
Collaborator Author

echoix commented May 19, 2024

Im doing that

@echoix
Copy link
Collaborator Author

echoix commented May 19, 2024

I want to rebase first to clean up unrelated diff

Update build.py

* [build-command] Update generated files
@echoix
Copy link
Collaborator Author

echoix commented May 19, 2024

Done

Copy link
Member

@nvuillam nvuillam left a comment

Choose a reason for hiding this comment

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

Very good catch, i never understood why these random failures happened ^^

@nvuillam nvuillam merged commit d584e0c into oxsecurity:main May 20, 2024
6 checks passed
@echoix
Copy link
Collaborator Author

echoix commented May 26, 2024

@nvuillam I think I understand some extra failures that I think I understand some more failures that happened after this #3562.

I think it is the order of the change directory and finding of files. The cd happens before finding files to delete, and the cd returns to the root of the filesystem (/). That means that the search looks everywhere, and isn't able to go into all files, like inside /proc or (or /run), or other special folders.

I saw logs with those errors.

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