mirror of
https://github.com/Abdess/retroarch_system.git
synced 2026-11-06 10:03:49 -06:00
fix: a lock reader refuses a missing directory
artifact_lock created its directory even for a shared reader, so a mistyped path given to --verify-packs or release_record was left as a directory holding a .lock, and split_pack created the parent before printing a bare errno. A reader now raises ArtifactDirectoryMissing, which run_cli prints on one line, and split_pack checks its target before taking the lock.
This commit is contained in:
1 parent
19dddedff6
commit
a1ddee8e5e
4 files changed
+40
-1
No files matched your search
@@ -209,6 +209,10 @@ def _strip_timestamps(text: str) -> str:
|
||||
class ArtifactLockBusy(RuntimeError):
|
||||
"""Raised when another process already holds the artifact directory."""
|
||||
|
||||
|
||||
class ArtifactDirectoryMissing(FileNotFoundError):
|
||||
"""A reader named a directory that does not exist."""
|
||||
|
||||
# A run that holds artifact directories for its whole duration names them
|
||||
# here, os.pathsep apart, for the steps it spawns: they would otherwise be
|
||||
# refused by it. A pipeline holds the repository root and its --output-dir.
|
||||
@@ -228,6 +232,14 @@ def artifact_lock(directory: str, exclusive: bool = True) -> Iterator[None]:
|
||||
Writers take the lock exclusively, readers share it. On platforms
|
||||
without flock the lock is a no-op.
|
||||
"""
|
||||
# A reader creating the directory it was pointed at left a typo behind
|
||||
# as a directory holding a .lock, and the next run with the same typo
|
||||
# no longer said the path did not exist.
|
||||
if not exclusive and not os.path.isdir(directory):
|
||||
raise ArtifactDirectoryMissing(
|
||||
f"{directory}: no such directory, pass the --output-dir the packs "
|
||||
"were written to (default dist/)"
|
||||
)
|
||||
try:
|
||||
import fcntl
|
||||
except ImportError:
|
||||
|
||||
+2
-1
@@ -3587,6 +3587,7 @@ from artifacts import ( # noqa: E402,F401
|
||||
copy_file_atomic,
|
||||
plain_file_mode,
|
||||
set_plain_mode,
|
||||
ArtifactDirectoryMissing,
|
||||
ArtifactLockBusy,
|
||||
artifact_lock,
|
||||
hold_artifact_lock,
|
||||
@@ -3634,7 +3635,7 @@ def run_cli(main: Callable[[], int | None]) -> None:
|
||||
try:
|
||||
code = main()
|
||||
sys.stdout.flush()
|
||||
except (InputMissing, ArtifactLockBusy) as exc:
|
||||
except (InputMissing, ArtifactLockBusy, ArtifactDirectoryMissing) as exc:
|
||||
print(f"ERROR: {exc}", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
except BrokenPipeError:
|
||||
|
||||
@@ -338,6 +338,10 @@ def main() -> int:
|
||||
# The packs are rewritten in place by generate_pack under this lock: a
|
||||
# build running beside the split mixes its bytes into the pack being
|
||||
# read, or writes a pack that the split then deletes.
|
||||
if not args.target.exists():
|
||||
print(f"Error: {args.target}: no such pack or directory, pass a pack or the "
|
||||
"--output-dir generate_pack wrote (default dist/)", file=sys.stderr)
|
||||
return 1
|
||||
directory = args.target if args.target.is_dir() else args.target.parent
|
||||
try:
|
||||
with artifact_lock(str(directory)):
|
||||
|
||||
@@ -61,6 +61,28 @@ class ArtifactLockTest(unittest.TestCase):
|
||||
with artifact_lock(self.dir, exclusive=False):
|
||||
pass
|
||||
|
||||
def test_a_reader_never_creates_the_directory(self):
|
||||
"""A mistyped --output-dir given to --verify-packs or release_record
|
||||
was created holding a .lock, and the next run with the same typo no
|
||||
longer said the path did not exist."""
|
||||
from common import ArtifactDirectoryMissing # noqa: PLC0415
|
||||
|
||||
missing = os.path.join(self.dir, "sub", "dsit")
|
||||
with self.assertRaises(ArtifactDirectoryMissing):
|
||||
with artifact_lock(missing, exclusive=False):
|
||||
pass
|
||||
self.assertFalse(os.path.exists(os.path.join(self.dir, "sub")))
|
||||
|
||||
def test_split_refuses_a_missing_target_before_locking(self):
|
||||
missing = Path(self.dir) / "sub" / "dsit"
|
||||
proc = subprocess.run(
|
||||
[sys.executable, "scripts/split_pack.py", str(missing)],
|
||||
cwd=REPO_ROOT, capture_output=True, text=True, check=False, timeout=60,
|
||||
)
|
||||
self.assertEqual(proc.returncode, 1)
|
||||
self.assertIn("no such pack or directory", proc.stderr)
|
||||
self.assertFalse(missing.parent.exists())
|
||||
|
||||
def test_lock_is_released_on_exit(self):
|
||||
with artifact_lock(self.dir):
|
||||
pass
|
||||
|
||||
Reference in new issue
Block a user