Skip to content

Commit 752cd96

Browse files
authored
fix: keep staging files writable; record modes in .rescue-modes.json; fix live-only conflict message
Thread 64: Remove os.fchmod/chmod from staged files to avoid Windows read-only attribute that prevents shutil.rmtree from cleaning up. Original permission bits are now written to a .rescue-modes.json sidecar in the staging dir and reloaded during retry, with a fall-back to the staged file's own mode for backwards-compat with pre-sidecar staging dirs. Thread 65: Split the ValidationError message for staging-vs-live conflicts into two accurate cases: files that diverged between both locations ("Both copies have been preserved") and live-only files that have no backup counterpart, which previously incorrectly claimed "Both copies have been preserved" and offered a restore instruction that was impossible. Assisted-by: GitHub Copilot (model: claude-sonnet-4.5, autonomous)
1 parent 09e35ea commit 752cd96

1 file changed

Lines changed: 79 additions & 22 deletions

File tree

src/specify_cli/extensions/__init__.py

Lines changed: 79 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1549,11 +1549,30 @@ def _recognized_config_names(
15491549
# enumerated by staging, so without this it would be silently
15501550
# deleted by the rmtree below and its bytes lost. Treat it as a
15511551
# conflict so both locations are preserved and the user resolves it.
1552-
conflicting.update(live_names - staged_names)
1552+
live_only = live_names - staged_names
1553+
conflicting.update(live_only)
1554+
# Load original permission bits from the sidecar JSON written by
1555+
# the staging step. Staged files are kept at mode 0o600 so that
1556+
# rmtree always succeeds on Windows, so staged_stat.st_mode would
1557+
# always be 0o600 and must not be used for mode comparisons or
1558+
# restoration; the sidecar records the true original mode.
1559+
rescue_modes_file = rescue_staging_dir / ".rescue-modes.json"
1560+
_staged_modes: dict[str, int] = {}
1561+
if rescue_modes_file.is_file() and not rescue_modes_file.is_symlink():
1562+
try:
1563+
_staged_modes = json.loads(rescue_modes_file.read_bytes())
1564+
except (OSError, ValueError):
1565+
pass
15531566
for staged_name in sorted(staged_names):
15541567
staged_file = rescue_staging_dir / staged_name
15551568
staged_stat = staged_file.stat()
15561569
staged_bytes = staged_file.read_bytes()
1570+
# Prefer the sidecar-recorded mode; fall back to the staged
1571+
# file's own mode for backwards-compat with staging dirs
1572+
# written before the sidecar was introduced.
1573+
staged_mode = _staged_modes.get(
1574+
staged_name, stat.S_IMODE(staged_stat.st_mode)
1575+
)
15571576
live_file = dest_dir / staged_name
15581577
if live_file.is_symlink():
15591578
# A user may have replaced the live config with a symlink
@@ -1579,23 +1598,38 @@ def _recognized_config_names(
15791598
else:
15801599
if live_bytes != staged_bytes or stat.S_IMODE(
15811600
live_stat.st_mode
1582-
) != stat.S_IMODE(staged_stat.st_mode):
1601+
) != staged_mode:
15831602
conflicting.add(staged_name)
1584-
stranded_configs[staged_name] = (
1585-
staged_bytes,
1586-
staged_stat.st_mode,
1587-
)
1603+
stranded_configs[staged_name] = (staged_bytes, staged_mode)
15881604
if conflicting:
1589-
names = ", ".join(sorted(conflicting))
1590-
raise ValidationError(
1591-
"Preserved extension config conflict for "
1592-
f"'{manifest.id}': the current config file(s) ({names}) in "
1593-
f"{dest_dir} differ from the rescued backup left by an "
1594-
f"interrupted install in {rescue_staging_dir}. Both copies "
1595-
"have been preserved. Resolve manually — keep the current "
1596-
f"file and delete {rescue_staging_dir}, or restore the "
1597-
"backup over the current file — then reinstall."
1605+
# Split into two cases for accurate user guidance: files that
1606+
# exist in both locations but have diverged, and files that
1607+
# exist only in the live directory with no rescue-backup copy.
1608+
both_diverged = conflicting - live_only
1609+
live_only_conflict = conflicting & live_only
1610+
msg_parts: list[str] = [
1611+
f"Preserved extension config conflict for '{manifest.id}':"
1612+
]
1613+
if both_diverged:
1614+
names = ", ".join(sorted(both_diverged))
1615+
msg_parts.append(
1616+
f"The current config(s) ({names}) in {dest_dir} differ"
1617+
f" from their rescued backup in {rescue_staging_dir}."
1618+
" Both copies have been preserved."
1619+
)
1620+
if live_only_conflict:
1621+
names = ", ".join(sorted(live_only_conflict))
1622+
msg_parts.append(
1623+
f"The config(s) ({names}) exist only in {dest_dir}"
1624+
f" with no counterpart in the rescued backup at"
1625+
f" {rescue_staging_dir}."
1626+
)
1627+
msg_parts.append(
1628+
f"Reconcile {dest_dir} and {rescue_staging_dir} to the"
1629+
f" desired final state, delete {rescue_staging_dir},"
1630+
" then reinstall."
15981631
)
1632+
raise ValidationError(" ".join(msg_parts))
15991633
elif dest_dir.exists() and not self.registry.is_installed(manifest.id):
16001634
for cfg_file in (
16011635
list(dest_dir.glob("*-config.yml"))
@@ -1661,16 +1695,39 @@ def _recognized_config_names(
16611695
written = 0
16621696
while written < len(view):
16631697
written += os.write(fd, view[written:])
1664-
try:
1665-
os.fchmod(fd, stat.S_IMODE(mode))
1666-
except (AttributeError, NotImplementedError, OSError):
1667-
try:
1668-
staged.chmod(stat.S_IMODE(mode))
1669-
except (NotImplementedError, OSError):
1670-
pass # Best-effort; chmod may not be supported on all platforms.
1698+
# Do NOT chmod the staged file: setting a read-only
1699+
# mode (e.g. 0o444) makes the file undeletable on
1700+
# Windows and causes shutil.rmtree to fail during
1701+
# cleanup. Original modes are recorded separately in
1702+
# .rescue-modes.json so they can be reapplied when the
1703+
# config is actually restored.
16711704
_fsync_fd(fd)
16721705
finally:
16731706
os.close(fd)
1707+
# Persist the original permission bits in a sidecar JSON file
1708+
# so a retry can correctly reapply them even though the staged
1709+
# files themselves are kept at their creation mode (0o600).
1710+
rescue_modes_file = rescue_staging_dir / ".rescue-modes.json"
1711+
modes_payload = json.dumps(
1712+
{
1713+
filename: stat.S_IMODE(mode)
1714+
for filename, (_, mode) in stranded_configs.items()
1715+
},
1716+
sort_keys=True,
1717+
).encode()
1718+
modes_fd = os.open(
1719+
str(rescue_modes_file),
1720+
os.O_WRONLY | os.O_CREAT | os.O_EXCL | getattr(os, "O_BINARY", 0),
1721+
0o600,
1722+
)
1723+
try:
1724+
view = memoryview(modes_payload)
1725+
written = 0
1726+
while written < len(view):
1727+
written += os.write(modes_fd, view[written:])
1728+
_fsync_fd(modes_fd)
1729+
finally:
1730+
os.close(modes_fd)
16741731
# Flush the staging directory metadata before publishing the
16751732
# completion marker so a crash cannot leave a visible marker with
16761733
# only a subset of staged files.

0 commit comments

Comments
 (0)