Skip to content

fix relative blocking lock 20261011 - #2284

Merged
Byron merged 2 commits into
mainfrom
fix-relative-blocking-lock-20261011
Oct 14, 2026
Merged

Byron merged 2 commits into
mainfrom
fix-relative-blocking-lock-20261011

Conversation

@Byron

@Byron Byron commented Oct 14, 2026

Copy link
Copy Markdown
Member

Supersedes #2282 which I can't push into for some reason.

Copilot AI balanced review requested due to automatic review settings October 14, 2026 08:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

3 open findings
What changed in this PR

This PR adjusts BlockingLockFile to only retry on true lock contention (an existing lock) and adds tests to cover relative-path locking and error propagation scenarios.

Changes:

  • Update BlockingLockFile._obtain_lock() to immediately raise non-contention lock creation errors (instead of retrying).
  • Fix parent-directory existence check for relative lock paths (treat empty dirname as .).
  • Add new tests for waiting, timeout, missing directory, and non-contention error propagation (including config locking).
File Description
git/​util.py Changes retry logic to only loop on existing-lock contention and adjusts relative-path parent-dir checking.
test/​test_util.py Adds a focused test matrix for relative/absolute paths and validates non-contention errors aren’t retried.
test/​test_config.py Adds a config-level regression test ensuring non-contention lock errors propagate without retries.

🧠 Review effort: Lite


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread git/util.py
Comment thread test/test_util.py
Comment thread test/test_util.py
`BlockingLockFile('resource')` incorrectly treats the empty `dirname` of
`resource.lock` as a missing directory when another holder owns the lock.
Use `os.curdir` for that case so the retry and timeout paths work normally.

Add seven tests covering relative and absolute `str`/`Path` inputs, holder
release, timeouts and real missing parents. The complete Linux/Python 3.12
suite passes: 1696 tests, 89 existing skips, one existing xfail, 64 subtests.
Pinned hooks, mypy, basedpyright, docs, builds and strict Twine also pass.

Prepared by OpenAI Codex, an AI agent operating through FanWu-ai's account.
@Byron
Byron force-pushed the fix-relative-blocking-lock-20261011 branch from 05af848 to e0f75ac Compare October 14, 2026 10:00
Copilot AI balanced review requested due to automatic review settings October 14, 2026 10:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread git/util.py
- [P2] Propagate non-contention errors before retrying relative locks —
  git/util.py:1313-1313 For a basename-only path in an
  unwritable current directory, or on POSIX after that directory is removed,
  `isdir('.')` still returns true despite lock creation failing with `EACCES`
  or `ENOENT`. This fallback turns previously immediate failures into
  effectively indefinite retries with the default timeout, also affecting
  callers using `GitConfigParser.t_lock = BlockingLockFile`. Propagate
  non-contention errors rather than retrying them, matching the [native Git
  baseline](git/lockfile.c#L234-L235), which
  retries only `EEXIST`.

Assisted-by: GPT 6.1 Sol
Co-authored-by: GPT 6.1 Sol <codex@openai.com>
@Byron
Byron force-pushed the fix-relative-blocking-lock-20261011 branch from e0f75ac to 3b9bb6d Compare October 14, 2026 10:08
Copilot AI balanced review requested due to automatic review settings October 14, 2026 10:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused implementation matches the intended behavior and has comprehensive regression coverage.

0 open findings

1 resolved since last review

🧠 Review effort: Balanced

@Byron
Byron merged commit 109017e into main Oct 14, 2026
51 checks passed
@Byron
Byron deleted the fix-relative-blocking-lock-20261011 branch October 14, 2026 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants