-
Notifications
You must be signed in to change notification settings - Fork 315
Improve resiliency of concurrent use the downloads directory. #1600
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
Improve resiliency of concurrent use the downloads directory. #1600
Conversation
…, avoid getting exe path 3 times.
…rectory in configure-environment. Allows outright deletion of set_directory_to_archive_contents.
include/vcpkg/base/files.h
Outdated
// | ||
// If `old_path` and `new_path` resolve to the same file, the behavior is undefined. | ||
bool rename_cas_like(const Path& old_path, const Path& new_path, std::error_code& ec) const; | ||
bool rename_cas_like(const Path& old_path, const Path& new_path, LineInfo li) const; |
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.
cas
? Maybe choose a different name 😅
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 considered something like xchg
but that would imply the target is overwritten if it exists. I'm happy to use a different name but I don't have ideas for anything better.
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.
rename_to_or_delete_if_exists
😄
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 feel like that name is a bit too close to just 'putting the code entirely in the name' but when I don't feel strongly about things and someone felt strongly enough about it to make a comment, I'm going to apply the suggestion.
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.
How about rename_or_delete
? The 'to' part is already implied by the rename, and avoids 'delete if exists' sounding like 'delete the target if it exists'. This way, one of those two verbs are clearly happening to the same subject (the source); we either rename the thing, or if we can't, we delete it.
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.
Sounds good!
This might workaround some failures reported by internal customers.