-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Implement file_lock feature #130999
Implement file_lock feature #130999
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @joboet (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
Thank you for the PR! This PR contains a public library API change, which should follow the proper process of creating an ACP first: https://std-dev-guide.rust-lang.org/development/feature-lifecycle.html |
@Noratrieb If I understand correctly, this already has an accepted ACP in rust-lang/libs-team#412 |
@ChrisDenton ah thanks! I changed it @Noratrieb Yep, it has already been accepted in rust-lang/libs-team#412 |
@rustbot ready |
Should this document which exact mechanism is used on each platform? I believe some platforms have multiple advisory lock mechanisms that don't affect each other. On such platforms switching mechanism in a future rust version would prevent the locks from being effective if you ever mix binaries compiled using different rustc versions. |
Ah yes, good idea. I added that |
/// | ||
/// This function currently corresponds to the `flock` function on Unix with the `LOCK_EX` flag, | ||
/// and the `LockFileEx` function on Windows with the `LOCKFILE_EXCLUSIVE_LOCK` flag. Note that, | ||
/// this [may change in the future][changes]. |
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.
It must not change for programs compiled with different rustc versions to interoperate wrt locking, right?
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 included may change in the future
because that language is used in all the other places that Platform-specific behavior
is documented, that I could find. Is there guidance on when to include or not include it?
It seems appropriate to include it to me, because the implementation can be changed and still interoperate. For example, LockFile
can be called instead of LockFileEx
in the case of lock()
and lock_shared()
. Similarly, you can imagine that a new syscall might be added to Unix and/or Windows which can be used as an alternative.
Just wanted to check in on this PR. @bjorn3 , @ChrisDenton anything I can do to help move it forward? |
2744f79
to
84982e5
Compare
These commits modify the If this was unintentional then you should revert the changes before this PR is merged. |
This adds lock(), lock_shared(), try_lock(), try_lock_shared(), and unlock() to File gated behind the file_lock feature flag
Hey @joboet just wanted to checkin on this PR. Any additional changes/tests you'd like to see? |
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.
Sorry for the delay. This looks mostly fine!
Co-authored-by: Jonas Böttiger <[email protected]>
@joboet no worries, thanks for the comments! I've addressed them |
Great, thank you! |
…iaskrgr Rollup of 4 pull requests Successful merges: - rust-lang#129627 (Ensure that tail expr receive lifetime extension) - rust-lang#130999 (Implement file_lock feature) - rust-lang#132873 (handle separate prefixes in clippy rules) - rust-lang#132891 (Remove `rustc_session::config::rustc_short_optgroups`) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 4 pull requests Successful merges: - rust-lang#129627 (Ensure that tail expr receive lifetime extension) - rust-lang#130999 (Implement file_lock feature) - rust-lang#132873 (handle separate prefixes in clippy rules) - rust-lang#132891 (Remove `rustc_session::config::rustc_short_optgroups`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#130999 - cberner:flock_pr, r=joboet Implement file_lock feature This adds lock(), lock_shared(), try_lock(), try_lock_shared(), and unlock() to File gated behind the file_lock feature flag This is the initial implementation of rust-lang#130994 for Unix and Windows platforms. I will follow it up with an implementation for WASI preview 2
Implement file_lock feature This adds lock(), lock_shared(), try_lock(), try_lock_shared(), and unlock() to File gated behind the file_lock feature flag This is the initial implementation of rust-lang#130994 for Unix and Windows platforms. I will follow it up with an implementation for WASI preview 2
…iaskrgr Rollup of 4 pull requests Successful merges: - rust-lang#129627 (Ensure that tail expr receive lifetime extension) - rust-lang#130999 (Implement file_lock feature) - rust-lang#132873 (handle separate prefixes in clippy rules) - rust-lang#132891 (Remove `rustc_session::config::rustc_short_optgroups`) r? `@ghost` `@rustbot` modify labels: rollup
This adds lock(), lock_shared(), try_lock(), try_lock_shared(), and unlock() to File gated behind the file_lock feature flag
This is the initial implementation of #130994 for Unix and Windows platforms. I will follow it up with an implementation for WASI preview 2