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
fix: Global flock file accessible by every user / Introducing FlockContext manager #1697
base: dev
Are you sure you want to change the base?
Conversation
@@ -0,0 +1,64 @@ | |||
# SPDX-FileCopyrightText: © 2024 Christian BUHTZ <c.buhtz@posteo.jp> |
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.
Not the only possible method but one of the recommended methods how to add copyright and license info to each file in a repo.
Further reading: REUSE Software and SPDX (ISO/IEC 5962:2021) specifications
self) | ||
|
||
|
||
class GlobalFlock(FlockContext): |
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.
Other naming suggestions? GlobalFileLock
, ...?
|
||
return True | ||
return 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.
This might have caused bugs in the past. The flock was never released when this "return" took affect. Now with the context manager the flock is always released.
|
||
raise | ||
raise |
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.
Here the call to releaseFlock() was also missing.
SDPX looks quite promising to me to support machine-readable licenses too |
Hope it is OK to now treat is as reviewed? I will also close the two related issues with merging this. |
Snapshots.backup()
is the only one using the global flock. The method smells like hell. There are ~170 lines between the lock and its release. And I decided not to touch this in this PR. So I just indented that lines to use them inside the new context manager.It is an idea.
Might...
Close #1676
Close #1122
EDIT: One of the two reporters tested and confirmed the PR.