locks.py: composition over inheritance for dummy lock#52499
Open
haampie wants to merge 5 commits into
Open
Conversation
a684fed to
3ca9255
Compare
3ca9255 to
079c59c
Compare
c88946d to
343c81d
Compare
343c81d to
91752e4
Compare
1105ac1 to
b25f01e
Compare
Prep for backend extraction: makes _poll_lock body byte-identical to the
future PosixBackend.poll so the move is reviewable. _log_debug is just
tty.debug(*args, level=kwargs.get("level", 2)).
Signed-off-by: Harmen Stoppels <me@harmenstoppels.nl>
Pure rename + small wrapper: - Add Lock.prepare(op) bundling _ensure_valid_handle() + the LOCK_EX/rb LockROFileError check that was inlined in _lock and try_acquire_write. - Rename Lock._poll_lock -> Lock.poll. - Rename Lock._unlock -> Lock.release. The subclass in spack.util.lock and the _poll_lock test are updated to match. No behavior change. Signed-off-by: Harmen Stoppels <me@harmenstoppels.nl>
Move the fcntl-based primitives (_ensure_valid_handle, prepare, poll, _read_log_debug_data, _write_log_debug_data, release, cleanup) and the pid/host/_file_ref/_cached_key state off Lock and onto a new PosixBackend. Add a DummyBackend with no-op equivalents. Lock now delegates to self.backend (PosixBackend on POSIX, DummyBackend on Windows). Method bodies are unchanged; review with --color-moved=zebra --color-moved-ws=allow-indentation-change. Signed-off-by: Harmen Stoppels <me@harmenstoppels.nl>
Lock.__init__ now takes enable: bool = True; when False (or on Windows) it uses DummyBackend. spack.util.lock no longer needs its own subclass to disable locking, so it just re-exports Lock from spack.llnl.util.lock. Signed-off-by: Harmen Stoppels <me@harmenstoppels.nl>
b25f01e to
bc4af55
Compare
becker33
previously approved these changes
Jun 7, 2026
Signed-off-by: Harmen Stoppels <me@harmenstoppels.nl>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces #52488 and #52479.
Condense the
if self._enabledbranches into a single one in the constructor, instead of sprinkling them in every method. It works by extracting the syscalls to a backend class in a composition over inheritance style. On Windows the backend is a no-op version.This is best reviewed commit by commit, with
git show --color-moved=dimmed-zebra c94402845c6eda4fb9a36108a6037a68880a9507for the commit that moves code.In a follow-up PR we could use a mock backend in tests to drop
fcntlmonkeypatching.