Skip to content

Commit 621362a

Browse files
committed
add file lock around nixpkg-review
multiple concurrent git fetches could override refs each other. In my experience also not all git operations cope well with concurrency, so I think it's better to just add a big lock around it.
1 parent e4eb84d commit 621362a

File tree

1 file changed

+59
-15
lines changed

1 file changed

+59
-15
lines changed

nixpkgs_review/review.py

Lines changed: 59 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
import argparse
22
import concurrent.futures
3+
import fcntl
34
import os
45
import subprocess
56
import sys
67
import tempfile
78
import time
9+
from collections.abc import Iterator
10+
from contextlib import contextmanager
811
from dataclasses import dataclass, field
912
from enum import Enum
1013
from pathlib import Path
@@ -685,8 +688,35 @@ def filter_packages(
685688
return packages
686689

687690

691+
@contextmanager
692+
def locked_open(filename: Path, mode: str = "r") -> Iterator[IO[str]]:
693+
"""
694+
This is a context manager that provides an advisory write lock on the file specified by `filename` when entering the context, and releases the lock when leaving the context.
695+
The lock is acquired using the `fcntl` module's `LOCK_EX` flag, which applies an exclusive write lock to the file.
696+
"""
697+
with filename.open(mode) as fd:
698+
fcntl.flock(fd, fcntl.LOCK_EX)
699+
yield fd
700+
fcntl.flock(fd, fcntl.LOCK_UN)
701+
702+
703+
def resolve_git_dir() -> Path:
704+
dotgit = Path(".git")
705+
if dotgit.is_file():
706+
actual_git_dir = dotgit.read_text().strip()
707+
if not actual_git_dir.startswith("gitdir: "):
708+
msg = f"Invalid .git file: {actual_git_dir} found in current directory"
709+
raise NixpkgsReviewError(msg)
710+
return Path() / actual_git_dir[len("gitdir: ") :]
711+
712+
if dotgit.is_dir():
713+
return dotgit
714+
715+
msg = "Cannot find .git file or directory in current directory"
716+
raise NixpkgsReviewError(msg)
717+
718+
688719
def fetch_refs(repo: str, *refs: str) -> list[str]:
689-
cmd = ["git", "-c", "fetch.prune=false", "fetch", "--no-tags", "--force", repo]
690720
shallow = subprocess.run(
691721
["git", "rev-parse", "--is-shallow-repository"],
692722
text=True,
@@ -696,23 +726,37 @@ def fetch_refs(repo: str, *refs: str) -> list[str]:
696726
if shallow.returncode != 0:
697727
msg = f"Failed to detect if {repo} is shallow repository"
698728
raise NixpkgsReviewError(msg)
729+
730+
fetch_cmd = [
731+
"git",
732+
"-c",
733+
"fetch.prune=false",
734+
"fetch",
735+
"--no-tags",
736+
"--force",
737+
repo,
738+
]
699739
if shallow.stdout.strip() == "true":
700-
cmd.append("--depth=1")
701-
for i, ref in enumerate(refs):
702-
cmd.append(f"{ref}:refs/nixpkgs-review/{i}")
703-
res = sh(cmd)
704-
if res.returncode != 0:
705-
msg = f"Failed to fetch {refs} from {repo}. git fetch failed with exit code {res.returncode}"
706-
raise NixpkgsReviewError(msg)
707-
shas = []
740+
fetch_cmd.append("--depth=1")
708741
for i, ref in enumerate(refs):
709-
cmd = ["git", "rev-parse", "--verify", f"refs/nixpkgs-review/{i}"]
710-
out = subprocess.run(cmd, text=True, stdout=subprocess.PIPE, check=False)
711-
if out.returncode != 0:
712-
msg = f"Failed to fetch {ref} from {repo} with command: {''.join(cmd)}"
742+
fetch_cmd.append(f"{ref}:refs/nixpkgs-review/{i}")
743+
dotgit = resolve_git_dir()
744+
with locked_open(dotgit / "nixpkgs-review", "w"):
745+
res = sh(fetch_cmd)
746+
if res.returncode != 0:
747+
msg = f"Failed to fetch {refs} from {repo}. git fetch failed with exit code {res.returncode}"
713748
raise NixpkgsReviewError(msg)
714-
shas.append(out.stdout.strip())
715-
return shas
749+
shas = []
750+
for i, ref in enumerate(refs):
751+
rev_parse_cmd = ["git", "rev-parse", "--verify", f"refs/nixpkgs-review/{i}"]
752+
out = subprocess.run(
753+
rev_parse_cmd, text=True, stdout=subprocess.PIPE, check=False
754+
)
755+
if out.returncode != 0:
756+
msg = f"Failed to fetch {ref} from {repo} with command: {''.join(rev_parse_cmd)}"
757+
raise NixpkgsReviewError(msg)
758+
shas.append(out.stdout.strip())
759+
return shas
716760

717761

718762
def differences(

0 commit comments

Comments
 (0)