-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
make package cache interit permission from source #34573
Conversation
@test filemode(sourcefile) == filemode(cachefile) | ||
finally | ||
rm(load_path, recursive=true) | ||
rm(load_cache_path, recursive=true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even though CI is passing, should the updates to LOAD_PATH
and DEPOT_PATH
be reversed again?
rm(load_cache_path, recursive=true) | |
rm(load_cache_path, recursive=true) | |
filter!(s -> s != load_path, LOAD_PATH) | |
filter!(s -> s != load_cache_path, DEPOT_PATH) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should suggest using popfirst!(LOAD_PATH)
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've thought about that, too, but using filter!
would seems to be more robust in case of a failure in between or even before the pushfirst!
s, or should the tests be run concurrently some time in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another way is to copy
before running the code and then copy!
it back when done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed those paths as @jonas-schulze suggested. Thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, mktempdir
now automatically deletes the folder when the process exits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still a best practice to delete when done using.
* make package cache interit permission from source * remove temporary directries (cherry picked from commit 5075472)
* make package cache interit permission from source * remove temporary directries
This fixes #25971.