From bb9442a07cebe70c3492cca31f31a8270bb83134 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 16 Oct 2026 16:52:45 +0200 Subject: fix(assistant): review of the Do row and fileops The canvas: a To .md ended any other way than its own Stop (the footer Stop, a question, a drop, a daemon restart, an older daemon refusing the command) left the card at Working forever. The footer Stop goes through stopOp(), asking and dropping end the operation first, a disconnect ends it, and an error while nothing else runs is taken as its outcome. Each fileops run gets its own Process, so a stopped run's late answer is never read as the next one's. To .md takes one PDF, as one file is written. Compress... no longer flips back when file(1) refines a kind, long errors are elided, and a desktop with no documents folder gets Documents/Assistant rather than ~/Assistant. fileops: an animated GIF to PNG or JPEG left one file per frame under names the rename never found; the first frame is taken. JPEG flattens transparency onto white. Zipping a folder holding the output folder read its own growing archive without end; only regular files outside it are added, which also skips FIFOs and dangling links. qpdf warnings no longer fail a merge. A folder's free name keeps its dots, a lone .gz or .xz is decompressed rather than refused, and a dotfile's archive is not hidden. Tools die with fileops even when it is killed outright, a killed run's leftovers are swept on the next run, a stop between fork and bookkeeping cannot miss the child, and a late stop cannot add a second answer. Error lines lose bsdtar's doubled prefix and its delayed-exit noise. Co-Authored-By: Claude Opus 5.5 --- desktop/assistant/fileops | 94 ++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 80 insertions(+), 14 deletions(-) (limited to 'desktop/assistant/fileops') diff --git a/desktop/assistant/fileops b/desktop/assistant/fileops index 4bfaee0..78a624b 100755 --- a/desktop/assistant/fileops +++ b/desktop/assistant/fileops @@ -18,6 +18,7 @@ the first free name; SIGTERM kills the running tool and removes what was half written. """ +import ctypes import json import os import shutil @@ -39,16 +40,19 @@ class Fail(Exception): def finish(result, code): + signal.signal(signal.SIGTERM, signal.SIG_IGN) # one answer: a late stop cannot add another print(json.dumps(result), flush=True) sys.exit(code) def stop(*_): + signal.signal(signal.SIGTERM, signal.SIG_IGN) if current is not None: try: os.killpg(current.pid, signal.SIGKILL) except ProcessLookupError: pass + current.wait() # gone before its folder is removed for t in temps: shutil.rmtree(t, ignore_errors=True) if os.path.isdir(t) else _unlink(t) finish({"ok": False, "error": "stopped"}, 1) @@ -61,22 +65,46 @@ def _unlink(path): pass +def die_with_us(): + # In the child: SIGKILL when fileops dies, however it dies. A stop is a + # SIGTERM that fileops handles; a shell reload can be a SIGKILL it + # cannot, and the tool, in its own session, would run on. + ctypes.CDLL("libc.so.6").prctl(1, signal.SIGKILL) # PR_SET_PDEATHSIG + + def run(argv): # Own process group, so a stop takes ffmpeg and its children too. global current if shutil.which(argv[0]) is None: hint = ": link it into ~/bin" if argv[0] == "trackcrop" else "" raise Fail(f"{argv[0]} not found{hint}") - current = subprocess.Popen(argv, stdin=subprocess.DEVNULL, stdout=subprocess.PIPE, - stderr=subprocess.PIPE, text=True, start_new_session=True) + # A stop between the fork and current being set would miss the child. + signal.pthread_sigmask(signal.SIG_BLOCK, {signal.SIGTERM}) + try: + current = subprocess.Popen(argv, stdin=subprocess.DEVNULL, stdout=subprocess.PIPE, + stderr=subprocess.PIPE, text=True, start_new_session=True, + preexec_fn=die_with_us) + finally: + signal.pthread_sigmask(signal.SIG_UNBLOCK, {signal.SIGTERM}) out, err = current.communicate() rc, current = current.returncode, None if rc: - last = err.strip().splitlines()[-1:] or [f"exit status {rc}"] - raise Fail(f"{argv[0]}: {last[0]}"[:300]) + raise Fail(f"{argv[0]}: {reason(argv[0], err, rc)}"[:300]) return out +def reason(tool, err, rc): + # The line that says what went wrong, without the tool's own prefix: + # bsdtar ends on "Error exit delayed from previous errors", the cause + # being earlier. + lines = [l.strip() for l in err.splitlines() if l.strip()] + lines = [l for l in lines if "Error exit delayed" not in l] or lines + line = lines[-1] if lines else f"exit status {rc}" + while line.startswith(tool + ":"): + line = line[len(tool) + 1:].strip() + return line + + def split(name): # ("report", ".pdf"), keeping a compound suffix whole: "a.tar.gz". low = name.lower() @@ -95,9 +123,10 @@ def stem(path): return os.path.splitext(name)[0] -def free(outdir, name): +def free(outdir, name, folder=False): + # A folder's name is whole: "v1.2 notes (pages)" has no suffix to keep. path = os.path.join(outdir, name) - base, ext = split(name) + base, ext = (name, "") if folder else split(name) n = 1 while os.path.lexists(path): path = os.path.join(outdir, f"{base} ({n}){ext}") @@ -108,12 +137,12 @@ def free(outdir, name): def make(outdir, name, build, folder=False): # build(tmp) writes the result at tmp; it keeps the suffix, which is how # ffmpeg and magick choose the format. - tmp = os.path.join(outdir, f".fileops-{os.getpid()}-{len(temps)}{split(name)[1]}") + tmp = os.path.join(outdir, f".fileops-{os.getpid()}-{len(temps)}{'' if folder else split(name)[1]}") temps.append(tmp) if folder: os.mkdir(tmp) build(tmp) - final = free(outdir, name) + final = free(outdir, name, folder) os.rename(tmp, final) temps.remove(tmp) return final @@ -160,17 +189,39 @@ def each(op, outdir, f): if op == "trackcrop": return make(outdir, f"{s} (tracked).mp4", lambda t: run(["trackcrop", f, t])) if op in ("png", "jpeg", "webp"): - return make(outdir, f"{s}.{'jpg' if op == 'jpeg' else op}", lambda t: run(["magick", f, t])) + # [0]: an animated GIF or WebP would come out as one file per frame, + # under names the rename never finds. JPEG has no transparency, so + # it is flattened onto white rather than left to turn black. + flat = ["-background", "white", "-flatten"] if op == "jpeg" else [] + src = f + "[0]" if op in ("png", "jpeg") else f + return make(outdir, f"{s}.{'jpg' if op == 'jpeg' else op}", + lambda t: run(["magick", src, *flat, t])) if op == "half": return make(outdir, f"{s} (50%){ext}", lambda t: run(["magick", f, "-resize", "50%", t])) if op == "extract": + # A lone compressed file (note.txt.gz) is no archive to bsdtar -x. + low = f.lower() + if low.endswith((".gz", ".xz", ".bz2", ".zst")) and not low.endswith( + (".tar.gz", ".tar.xz", ".tar.bz2", ".tar.zst")): + # Straight to the file: the content need not be text. + return make(outdir, s, lambda t: run(["sh", "-c", 'exec bsdcat -- "$1" > "$2"', + "sh", f, t])) return make(outdir, s, lambda t: run(["bsdtar", "-xf", f, "-C", t]), folder=True) raise Fail(f"unknown operation {op}") def compress(op, outdir, files): ext = {"zip": ".zip", "tgz": ".tar.gz", "txz": ".tar.xz", "tbz2": ".tar.bz2"}[op] - name = (stem(files[0]) if len(files) == 1 else "archive") + ext + # A dotfile's archive is not hidden: .bashrc gives bashrc.zip. + name = ((stem(files[0]).lstrip(".") if len(files) == 1 else "") or "archive") + ext + out = os.path.realpath(outdir) + + def wanted(p): + # Regular files only, and none inside the output folder: zipping a + # folder that holds it would read the archive being written, which + # never ends. A FIFO would block, a dangling link would fail. + real = os.path.realpath(p) + return os.path.isfile(p) and not real.startswith(out + os.sep) def build(t): if op == "zip": @@ -180,16 +231,28 @@ def compress(op, outdir, files): for root, _, names in os.walk(f): for n in names: p = os.path.join(root, n) - z.write(p, os.path.relpath(p, os.path.dirname(f))) + if wanted(p): + z.write(p, os.path.relpath(p, os.path.dirname(f))) else: z.write(f, os.path.basename(f)) else: with tarfile.open(t, "w:" + {"tgz": "gz", "txz": "xz", "tbz2": "bz2"}[op]) as tar: for f in files: - tar.add(f, arcname=os.path.basename(f)) + tar.add(f, arcname=os.path.basename(f), + filter=lambda i: None if i.isfifo() or i.isdev() else i) return make(outdir, name, build) +def sweep(outdir): + # Leftovers of a run that was killed outright (its pid is gone). + for n in os.listdir(outdir): + if n.startswith(".fileops-"): + pid = n.split("-")[1] + if pid.isdigit() and not os.path.exists(f"/proc/{pid}"): + p = os.path.join(outdir, n) + shutil.rmtree(p, ignore_errors=True) if os.path.isdir(p) else _unlink(p) + + def main(argv): if len(argv) < 3: raise Fail("usage: fileops ") @@ -197,14 +260,17 @@ def main(argv): if not os.path.isabs(outdir): raise Fail("the output folder must be an absolute path") for f in files: - if not os.path.exists(f): + if not (os.path.isabs(f) and os.path.exists(f)): raise Fail(f"cannot read {os.path.basename(f)}") os.makedirs(outdir, exist_ok=True) + sweep(outdir) if op == "pdf-merge": if len(files) < 2: raise Fail("merging needs two or more PDFs") + # --warning-exit-0: a PDF with a repaired xref merges fine, and qpdf + # would otherwise exit 3 and the result be thrown away. return [make(outdir, "merged.pdf", - lambda t: run(["qpdf", "--empty", "--pages", *files, "--", t]))] + lambda t: run(["qpdf", "--warning-exit-0", "--empty", "--pages", *files, "--", t]))] if op in ("zip", "tgz", "txz", "tbz2"): return [compress(op, outdir, files)] return [each(op, outdir, f) for f in files] -- cgit v1.2.3