diff --git a/DESIGN.md b/DESIGN.md index 104fe0c..ad87aca 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -49,65 +49,71 @@ invocation it documents **one package installed in the current switch**: package's dependencies already compiled** in `--odoc-dir`. Ordering is our job (see Ordering). -So the wrapper tool's responsibilities reduce to: recording what changed, -ordering the work, invoking the driver once per stale package, and -cleaning up removals. +So the wrapper tool's responsibilities reduce to: ordering the work and +invoking the driver once per stale package (recording what changed and +cleaning up removals are done directly by the coreutils hooks, see below). ## Components ### 1. opam hooks (configured in `~/.opam/config`, shipped via opamrc) -Three wrapper fields drive everything. The two per-package hooks are dumb -recorders that must be near-instant; all real work happens once per session. +Three wrapper fields drive everything. The two per-package hooks are plain +coreutils — crucially they invoke **no switchdocs binary** — so they keep +working even in a switch where switchdocs isn't installed (or is mid-upgrade, +or where a *global* hook fires for a switch that never had it). All real work +happens once per session, in the one hook that does need the binary. ``` post-install-commands: [ - [ "%{hooks}%/switchdocs" "record" "install" "%{name}%" "%{version}%" ] - { error-code = 0 } + [ "sh" "-c" "mkdir -p \"$1/odoc/$2\" && touch \"$1/odoc/$2/.switchdocs-stale\"" + "--" "%{prefix}%" "%{name}%" ] { error-code = 0 } ] post-remove-commands: [ - [ "%{hooks}%/switchdocs" "record" "remove" "%{name}%" "%{version}%" ] - { error-code = 0 } + [ "rm" "-rf" "%{prefix}%/odoc/%{name}%" ] { error-code = 0 } ] post-session-commands: [ - [ "%{hooks}%/switchdocs" "sync" ] { success } + [ "%{hooks}%/switchdocs" "sync" "--prefix" "%{prefix}%" ] { success } ] ``` -Why a per-package recorder instead of just reading `%{new}%`/`%{removed}%` -in the session hook: reinstallations at the same version (dependency -triggered a rebuild) are invisible to the session-level variables, but a -rebuild can change the `.cmti`s the docs are generated from. The -post-install hook fires for every install action including same-version -rebuilds. - -`record` appends one line (`install pkg version` / `remove pkg version`) -to the pending file with a single `O_APPEND` write, and exits 0 -unconditionally. Opam runs package actions in parallel, so appends from -independent packages interleave; single-write lines keep the file -uncorrupted. **Line order carries no meaning** — the file is a set (see -Ordering below). - -`sync` does everything else, once, and also always exits 0: doc failures -are logged, never propagated, because opam aborts the invocation with a -configuration error if a session hook fails (`opamSolution.ml`, post-session -handling). +- **post-install** marks the package stale by `touch`-ing + `odoc//.switchdocs-stale` (after `mkdir -p`-ing the dir). It fires for + every install action including same-version rebuilds — which matters, + because a dependency-triggered rebuild changes the `.cmti`s but is invisible + to session-level `%{new}%`/`%{removed}%`. A marker is a single empty file per + package, so opam's parallel package actions can't corrupt anything (distinct + files; `touch` is idempotent). **The set of markers is the work list** — no + ordering or content is implied (order is recomputed at sync time, see + Ordering below). +- **post-remove** deletes the package's doc subtree outright. Removal needs no + marker and no deferral: the deletion *is* the action, done immediately and + binary-free. (This also clears any stale marker that lived in that subtree.) +- **post-session** runs `switchdocs sync`, the one hook that needs the binary. + It always exits 0: doc failures are logged, never propagated, because opam + aborts the invocation with a configuration error if a session hook fails + (`opamSolution.ml`, post-session handling). If switchdocs is absent, this + hook simply doesn't run; the markers persist and are drained by the next + session that does have it — deferred, never lost. ### 2. State and layout (all under `$OPAM_SWITCH_PREFIX`) ``` var/cache/switchdocs/ - pending # set of stale/removed packages, appended by `record` log # sync output, since hooks must stay quiet + lock # guards a manual sync against a hook-invoked one odoc/ - /... # per-package odoc, odocl and HTML (driver defaults) + / + .switchdocs-stale # marker: post-install touched it, sync rebuilds + ... # per-package odoc, odocl and HTML (driver defaults) index.html # switch-wide landing page (ours) odoc-search/... # driver support files, search assets ``` The driver's defaults are taken as-is: one `odoc/` tree per switch holding -both intermediates and HTML, separate from opam's own `/doc`. Our own -state is just the pending file and a log. +both intermediates and HTML, separate from opam's own `/doc`. The +work list lives in that tree too — a `.switchdocs-stale` marker file inside +each stale package's dir — so the post-install hook can write it without +switchdocs. Our only other state is the log and lock. ### 3. The `sync` step @@ -119,27 +125,27 @@ is the switch environment, so the driver's `opam switch show` resolves to the right switch; being read-only, the nested opam calls don't contend with the lock the surrounding session holds. -1. **Read and dedupe the pending file** into `stale : (pkg, ver) set` and - `removed : (pkg, ver) set`. A package appearing in both (upgrade = - remove old + install new) counts as stale. -2. **Erase**: for removed-and-not-reinstalled packages, delete - `odoc//` outright. For stale packages, also delete `odoc//` - before rebuilding — the flat layout has no version in the path, so an - upgrade overwrites in place, and pre-deleting prevents files from - modules that no longer exist surviving from the old version. -3. **Compute dependency order** over `stale` (see Ordering). -4. **For each stale package, in order**, run - `odoc_driver_opam --actions all` - (all directory options left at their switch defaults). On success, - remove the package's lines from the pending file (rewrite-and-rename). - On failure, leave them: the next session retries automatically. -5. **Regenerate the top-level index**: a landing page listing all packages - with built docs (directory listing of `odoc/`, filtered to package - dirs). The per-package pages and redirects are the driver's job; only - this one page is ours. - -Empty pending file ⇒ `sync` exits immediately, so sessions that change -nothing cost nothing. +1. **Collect the stale set**: the package names whose `odoc//` holds a + `.switchdocs-stale` marker, intersected with the installed set. (A marker + for a no-longer-installed package can only survive an install+remove in the + same session — the remove hook's `rm -rf` normally clears it — so tidy it + away.) Removals need no handling here: the post-remove hook already deleted + the docs. +2. **Compute dependency order** over the stale set (see Ordering). +3. **For each stale package, in order**: delete `odoc//` (a clean build — + the flat layout has no version in the path, so this prevents files from + modules that no longer exist surviving from a previous version, and it + removes the marker), then run `odoc_driver_opam --actions all` (all + directory options left at their switch defaults). On success the marker + stays gone; on failure re-create it (`mkdir -p` + `touch`) so the next + session retries. +4. **Regenerate the top-level index** *every session*: a landing page listing + all installed packages with built docs (directory listing of `odoc/`, + filtered). Doing this unconditionally is what lets a session that only + *removed* packages drop them from the page — there's no removal marker to + trigger on. The write is skipped when the content is unchanged, so idle + sessions stay cheap. The per-package pages and redirects are the driver's + job; only this one page is ours. ### 4. Ordering @@ -149,14 +155,13 @@ facts make this tractable: - **The stale set is closed under reverse dependencies.** If A's rebuild could affect B's docs, opam rebuilt B too (that is what triggers - recompilation), so B was recorded by the post-install hook. We never + recompilation), so B was marked stale by the post-install hook. We never need to add packages to the work list ourselves. -- **Order is computed at sync time, never trusted from the pending file.** - Within one session append order happens to be topological (a dependency's - install action — including its post-install hook — completes before any - dependent's action starts), but the property does not survive - retry-across-sessions: a leftover entry for B can precede a freshly - appended entry for its dependency A. +- **Order is computed at sync time, never inferred from the markers.** The + markers are an unordered set (filesystem entries with no meaningful + timestamps to trust), so order is always recomputed from package metadata — + necessary anyway, since a marker left over from a failed earlier session can + coexist with a freshly touched one for its dependency. The graph is built from opam's installed-package metadata: `$OPAM_SWITCH_PREFIX/.opam-switch/packages/./opam` — the opam @@ -199,35 +204,36 @@ itself) behaves — possibly just a no-op build to skip. One OCaml executable, `switchdocs`, with subcommands: -- `switchdocs record {install|remove} ` — the hook recorder. -- `switchdocs sync` — the session worker described above. +- `switchdocs sync` — the session worker described above (the post-session hook). - `switchdocs rebuild [--all | ...]` — manual escape hatch: mark packages (or everything installed) stale and run sync. `--all` works on - pre-existing switches because opam 2 always writes `.changes` files, - whether or not hooks were configured at install time. + pre-existing switches because the installed-package metadata is always + present, whether or not hooks were configured at install time. - `switchdocs setup` — write the three wrapper fields into `~/.opam/config` - (or emit an opamrc fragment) and install the hook script into - `%{hooks}%`. + with `opam option --global` (idempotent). + +There is deliberately **no** `record` subcommand: recording a change (a +`touch`) and cleaning up a removal (an `rm -rf`) are plain coreutils in the +hooks, so they never depend on switchdocs being installed. Dependencies: `opam-format` (opam file parsing), `bos`, `fpath`, `cmdliner`; `odoc_driver_opam` is invoked as an external binary so its -(large) dependency cone stays out of the hook tool. The recorder path must -not load any of this — `record` is argv parsing plus one `write()`. -`odoc_driver_opam` itself should be installed per-switch (it links -against the switch's odoc), found via the switch `PATH`. +(large) dependency cone stays out of the tool. `odoc_driver_opam` itself +should be installed per-switch (it links against the switch's odoc), found +via the switch `PATH`. -Distribution: an opamrc using `init-scripts:` to place the `switchdocs` -shim in the hooks dir plus the three `*-commands` fields, for +Distribution: an opamrc adding the three `*-commands` fields for `opam init --config`; `switchdocs setup` for retrofitting existing roots. ## Failure handling summary | Failure | Behaviour | |---|---| -| package build fails | hooks filtered on `error-code = 0` / `{ success }`; nothing recorded for that package, no sync | -| doc build fails for one package | logged; its pending entries survive; later packages still attempted (their deps' docs may be stale — accepted, fixed on retry) | -| sync interrupted | pending file intact (entries removed only after per-package success); next session resumes | -| `odoc_driver_opam` missing from switch | sync logs and exits 0; entries survive until the driver is installed | +| package build fails | hooks filtered on `error-code = 0` / `{ success }`; the package isn't marked stale, no sync | +| switchdocs not installed in the switch | post-install/post-remove still work (coreutils); markers accumulate, drained by the next session that has switchdocs — deferred, never lost | +| doc build fails for one package | logged; its marker is re-created; later packages still attempted (their deps' docs may be stale — accepted, fixed on retry) | +| sync interrupted | markers intact (a marker is only cleared by a successful build); next session resumes | +| `odoc_driver_opam` missing from switch | sync logs and exits 0; markers survive until the driver is installed | | docs failure overall | `sync` exits 0 regardless; opam reports its own success untouched | ## Open questions diff --git a/README.md b/README.md index 51005f8..6652ccc 100644 --- a/README.md +++ b/README.md @@ -3,9 +3,13 @@ Keep an opam switch's HTML documentation continuously up to date. `switchdocs` wires opam's `post-install-commands`, `post-remove-commands` -and `post-session-commands` hooks to `odoc_driver_opam` so that after every -successful `opam install` / `opam upgrade` / `opam remove`, the docs for -exactly the packages that changed are regenerated, in dependency order. +and `post-session-commands` hooks so that after every successful +`opam install` / `opam upgrade` / `opam remove`, the docs for exactly the +packages that changed are regenerated, in dependency order, by +`odoc_driver_opam`. The per-package hooks are plain coreutils (a `touch` to +mark a package stale, an `rm -rf` to drop a removed one's docs), so they keep +working even when switchdocs itself isn't installed in the switch; only the +once-per-session worker needs the binary. Docs are written to `$OPAM_SWITCH_PREFIX/odoc//`, with a landing page at `$OPAM_SWITCH_PREFIX/odoc/index.html`. (This is switchdocs' own tree, kept separate from opam's `$OPAM_SWITCH_PREFIX/doc`, where packages install @@ -25,12 +29,9 @@ $ switchdocs setup --apply # add it to ~/.opam/config via `opam option` ## Commands -- `switchdocs record {install|remove} NAME VERSION` — hook recorder; appends - to the pending set. Always exits 0 (a recording failure must never fail an - opam action). -- `switchdocs sync` — session worker; processes the pending set: deletes - docs of removed packages, rebuilds stale ones in dependency order, updates - the landing page. Always exits 0; details go to +- `switchdocs sync` — session worker (the post-session hook); rebuilds every + package marked stale, in dependency order, and regenerates the landing page. + Always exits 0; details go to `$OPAM_SWITCH_PREFIX/var/cache/switchdocs/log`. - `switchdocs rebuild [--all | PKG...]` — mark packages stale and sync. Exits non-zero on failure (user-facing, unlike the hook commands). @@ -40,6 +41,9 @@ $ switchdocs setup --apply # add it to ~/.opam/config via `opam option` documentation. Queries every package's `sherlodoc_db.marshal` together (a single query covers the whole switch), printing each match with its owning package. `--package` restricts to named packages. +- `switchdocs show REFERENCE` — print an item's documentation as Markdown, + resolving an odoc reference (e.g. `Stdlib.List.map`, `Astring.String`, or a + package-qualified `/stdlib/Stdlib.List.map`) against the whole switch. ## Development diff --git a/bin/main.ml b/bin/main.ml index aeb529d..f2201c6 100644 --- a/bin/main.ml +++ b/bin/main.ml @@ -46,67 +46,6 @@ let driver_t = in Term.(const resolve $ arg) -(* record — hook-facing: must never fail an opam action, so always exits 0, - reporting problems on stderr only. *) -let record_cmd = - let verb_t = - let parse s = - match Switchdocs.Pending.verb_of_string s with - | Some v -> Ok v - | None -> - Error - (`Msg (Printf.sprintf "expected 'install' or 'remove', got %S" s)) - in - let print ppf v = - Format.pp_print_string ppf - (match v with - | Switchdocs.Pending.Install -> "install" - | Switchdocs.Pending.Remove -> "remove") - in - Arg.conv (parse, print) - in - let verb = - Arg.( - required - & pos 0 (some verb_t) None - & info [] ~docv:"VERB" ~doc:"$(b,install) or $(b,remove).") - in - let name_t = - Arg.( - required - & pos 1 (some string) None - & info [] ~docv:"NAME" ~doc:"Package name.") - in - let version_t = - Arg.( - required - & pos 2 (some string) None - & info [] ~docv:"VERSION" ~doc:"Package version.") - in - let run switch verb name version = - (match switch with - | Error (`Msg m) -> Printf.eprintf "switchdocs record: %s\n" m - | Ok sw -> ( - match Switchdocs.Pending.record sw verb ~name ~version with - | Ok () -> () - | Error (`Msg m) -> Printf.eprintf "switchdocs record: %s\n" m)); - 0 - in - let doc = "Record a package action in the pending set (opam hook recorder)" in - let man = - [ - `S Manpage.s_description; - `P - "Appends one entry to the switch's pending file. Intended to be run \ - from opam's $(b,post-install-commands) and $(b,post-remove-commands) \ - hooks; see $(b,switchdocs setup). Always exits 0: a recording failure \ - must never fail the surrounding opam action."; - ] - in - Cmd.v - (Cmd.info "record" ~doc ~man) - Term.(const run $ switch_t $ verb $ name_t $ version_t) - let print_outcome sw outcome = Printf.printf "switchdocs: %s\n" (Switchdocs.Sync.summary outcome); if outcome.Switchdocs.Sync.failed <> [] then @@ -120,7 +59,7 @@ let sync_cmd = | Error (`Msg m) -> Printf.eprintf "switchdocs sync: %s\n" m | Ok sw -> ( match Switchdocs.Sync.sync ~driver sw with - | Ok { built = []; failed = []; removed = [] } -> () + | Ok { built = []; failed = [] } -> () | Ok outcome -> print_outcome sw outcome | Error (`Msg m) -> Switchdocs.Sync.log sw m; @@ -132,12 +71,12 @@ let sync_cmd = [ `S Manpage.s_description; `P - "Processes the pending set recorded by $(b,switchdocs record): deletes \ - the docs of removed packages, rebuilds stale packages in dependency \ - order by running the driver once per package, and regenerates the \ - landing page. Intended to be run from opam's \ + "Rebuilds every package marked stale by the opam post-install hook, in \ + dependency order, running the driver once per package, and \ + regenerates the landing page (so packages deleted by the post-remove \ + hook drop off it). Intended to be run from opam's \ $(b,post-session-commands) hook. Always exits 0; failed packages stay \ - pending and are retried by the next session, with details in the log."; + marked and are retried by the next session, with details in the log."; ] in Cmd.v (Cmd.info "sync" ~doc ~man) Term.(const run $ switch_t $ driver_t) @@ -176,17 +115,14 @@ let rebuild_cmd = match targets with | Error m -> `Error (false, m) | Ok targets -> ( - let record_all = + let mark_all = List.fold_left - (fun acc (name, version) -> - Result.bind acc @@ fun () -> - Switchdocs.Pending.record sw Switchdocs.Pending.Install ~name - ~version) + (fun acc (name, _version) -> + Result.bind acc @@ fun () -> Switchdocs.Pending.mark sw name) (Ok ()) targets in match - Result.bind record_all @@ fun () -> - Switchdocs.Sync.sync ~driver sw + Result.bind mark_all @@ fun () -> Switchdocs.Sync.sync ~driver sw with | Error (`Msg m) -> `Error (false, m) | Ok outcome -> @@ -381,12 +317,14 @@ let setup_cmd = [ `S Manpage.s_description; `P - "Adds $(b,switchdocs record) to opam's $(b,post-install-commands) and \ - $(b,post-remove-commands), and $(b,switchdocs sync) to \ - $(b,post-session-commands), in the global opam configuration. Without \ - $(b,--apply), prints the $(b,opam option) invocations instead of \ - running them. Fields already mentioning this binary are skipped, so \ - the command is idempotent."; + "Adds to the global opam configuration: a $(b,post-install-commands) \ + hook that marks a package's docs stale (a plain $(b,mkdir)+$(b,touch), \ + so it works even when switchdocs isn't installed), a \ + $(b,post-remove-commands) hook that deletes a package's docs \ + ($(b,rm -rf)), and a $(b,post-session-commands) hook that runs \ + $(b,switchdocs sync). Without $(b,--apply), prints the $(b,opam \ + option) invocations instead of running them. Fields already carrying \ + our command are skipped, so the command is idempotent."; ] in Cmd.v (Cmd.info "setup" ~doc ~man) Term.(ret (const run $ apply)) @@ -398,16 +336,18 @@ let main_cmd = `S Manpage.s_description; `P "$(mname) keeps the HTML documentation of an opam switch in sync with \ - its installed packages. opam hooks record which packages each opam \ - invocation touched; at the end of the session the worker rebuilds \ - exactly those packages' docs, in dependency order, using an odoc \ - driver. Output goes to $(b,\\$OPAM_SWITCH_PREFIX/odoc), with a landing \ - page at $(b,odoc/index.html)."; + its installed packages. An opam post-install hook marks each changed \ + package stale (a marker file it touches, needing no switchdocs binary) \ + and a post-remove hook deletes a package's docs; at the end of the \ + session the worker rebuilds exactly the stale packages, in dependency \ + order, using an odoc driver. Output goes to \ + $(b,\\$OPAM_SWITCH_PREFIX/odoc), with a landing page at \ + $(b,odoc/index.html)."; `P "Run $(b,switchdocs setup) to configure the hooks."; ] in Cmd.group (Cmd.info "switchdocs" ~version:"%%VERSION%%" ~doc ~man) - [ record_cmd; sync_cmd; rebuild_cmd; order_cmd; search_cmd; show_cmd; setup_cmd ] + [ sync_cmd; rebuild_cmd; order_cmd; search_cmd; show_cmd; setup_cmd ] let () = exit (Cmd.eval' main_cmd) diff --git a/lib/index_page.ml b/lib/index_page.ml index 0b8ac18..459848a 100644 --- a/lib/index_page.ml +++ b/lib/index_page.ml @@ -35,6 +35,12 @@ let write sw = let entries = List.filter (fun (name, _) -> has_docs sw name) (Switch.installed sw) in - Result.bind (Bos.OS.Dir.create ~path:true (Switch.odoc_dir sw)) - @@ fun (_ : bool) -> - Bos.OS.File.write Fpath.(Switch.odoc_dir sw / "index.html") (page entries) + let content = page entries in + let file = Fpath.(Switch.odoc_dir sw / "index.html") in + (* [sync] regenerates the page every session (so removals are reflected even + when nothing was rebuilt), so skip the write when nothing changed. *) + match Bos.OS.File.read file with + | Ok current when current = content -> Ok () + | _ -> + Result.bind (Bos.OS.Dir.create ~path:true (Switch.odoc_dir sw)) + @@ fun (_ : bool) -> Bos.OS.File.write file content diff --git a/lib/pending.ml b/lib/pending.ml index 10b1dcd..3b4beb6 100644 --- a/lib/pending.ml +++ b/lib/pending.ml @@ -1,62 +1,18 @@ -type verb = Install | Remove -type entry = { verb : verb; name : string; version : string } - -let verb_of_string = function - | "install" -> Some Install - | "remove" -> Some Remove - | _ -> None - -let verb_to_string = function Install -> "install" | Remove -> "remove" - -let record sw verb ~name ~version = - Result.bind (Bos.OS.Dir.create ~path:true (Switch.state_dir sw)) - @@ fun (_ : bool) -> - let line = Printf.sprintf "%s %s %s\n" (verb_to_string verb) name version in - match - Unix.openfile - (Fpath.to_string (Switch.pending_file sw)) - [ O_WRONLY; O_APPEND; O_CREAT ] - 0o644 - with - | exception Unix.Unix_error (e, _, _) -> Error (`Msg (Unix.error_message e)) - | fd -> - Fun.protect - ~finally:(fun () -> Unix.close fd) - (fun () -> - let len = String.length line in - if Unix.write_substring fd line 0 len = len then Ok () - else Error (`Msg "short write to pending file")) - -let parse_line l = - match String.split_on_char ' ' (String.trim l) with - | [ verb; name; version ] -> ( - match verb_of_string verb with - | Some verb when name <> "" && version <> "" -> - Some { verb; name; version } - | _ -> None) - | _ -> None +let marker_name = ".switchdocs-stale" +let marker sw name = Fpath.(Switch.odoc_dir sw / name / marker_name) let read sw = - let file = Switch.pending_file sw in - match Bos.OS.File.read_lines file with + match Bos.OS.Dir.contents (Switch.odoc_dir sw) with | Error _ -> [] - | Ok lines -> List.filter_map parse_line lines + | Ok entries -> + List.filter_map + (fun d -> + match Bos.OS.File.exists Fpath.(d / marker_name) with + | Ok true -> Some (Fpath.basename d) + | _ -> None) + entries + |> List.sort compare -let remove_packages sw names = - let kept = List.filter (fun e -> not (List.mem e.name names)) (read sw) in - let file = Switch.pending_file sw in - if kept = [] then Bos.OS.File.delete file - else - let tmp = Fpath.(Switch.state_dir sw / "pending.tmp") in - let content = - String.concat "" - (List.map - (fun e -> - Printf.sprintf "%s %s %s\n" (verb_to_string e.verb) e.name - e.version) - kept) - in - Result.bind (Bos.OS.File.write tmp content) @@ fun () -> - match Unix.rename (Fpath.to_string tmp) (Fpath.to_string file) with - | () -> Ok () - | exception Unix.Unix_error (e, _, _) -> Error (`Msg (Unix.error_message e)) +let mark sw name = + Result.bind (Bos.OS.Dir.create ~path:true Fpath.(Switch.odoc_dir sw / name)) + @@ fun (_ : bool) -> Bos.OS.File.write (marker sw name) "" diff --git a/lib/pending.mli b/lib/pending.mli index 5a18ad3..f8e7ddd 100644 --- a/lib/pending.mli +++ b/lib/pending.mli @@ -1,29 +1,20 @@ -(** The pending file: the set of packages whose documentation is out of date. - Appended to by the hook recorder, consumed by sync. +(** Stale markers: the set of packages whose documentation is out of date. - Line order carries no meaning — ordering is recomputed from package metadata - at sync time (see {!Deps}). *) + A package is marked by the empty file [/odoc//.switchdocs-stale]. + The opam post-install hook creates it with a plain [mkdir -p] + [touch], so + recording a change needs no switchdocs binary in the switch — only [sync] + (and [rebuild]) do. Removals are handled directly by the post-remove hook + ([rm -rf] of the package's doc directory), which also clears any marker, so + there is no remove marker. *) -type verb = Install | Remove -type entry = { verb : verb; name : string; version : string } +val marker : Switch.t -> string -> Fpath.t +(** [/odoc//.switchdocs-stale]. *) -val verb_of_string : string -> verb option +val read : Switch.t -> string list +(** Names of packages with a stale marker — the subdirectories of + [/odoc] containing a {!marker} — sorted. *) -val record : - Switch.t -> - verb -> - name:string -> - version:string -> - (unit, [ `Msg of string ]) result -(** Append one entry with a single [O_APPEND] write, so concurrent recorders - (opam runs package actions in parallel) cannot interleave within a line. - Creates the state directory if needed. *) - -val read : Switch.t -> entry list -(** All well-formed entries; malformed lines are ignored. *) - -val remove_packages : - Switch.t -> string list -> (unit, [ `Msg of string ]) result -(** Drop every entry for the given package names, atomically - (rewrite-and-rename). Safe against concurrent recorders only because sync - runs under opam's switch lock. *) +val mark : Switch.t -> string -> (unit, [ `Msg of string ]) result +(** Create the stale marker for a package, [mkdir -p]-ing its doc directory + first. Used by [rebuild] and by [sync] to keep a package pending after a + failed build. *) diff --git a/lib/setup.ml b/lib/setup.ml index ce2d389..ee11e20 100644 --- a/lib/setup.ml +++ b/lib/setup.ml @@ -1,43 +1,56 @@ let quote s = Printf.sprintf "%S" s +(* Each entry is an opam field, the command to append to it, and a substring + that identifies our command if it is already present (for idempotency). The + per-package hooks deliberately don't mention the switchdocs binary — they are + plain coreutils so recording works even when switchdocs isn't installed — so + the marker can't just be [exe]. *) let fields ~exe = - let record verb = - Printf.sprintf - {|[%s "record" "%s" "--prefix" "%%{prefix}%%" "%%{name}%%" "%%{version}%%"] {error-code = 0}|} - (quote exe) verb - in [ - ("post-install-commands", record "install"); - ("post-remove-commands", record "remove"); + (* post-install (incl. same-version rebuilds): mark the package stale. + [mkdir -p] then [touch], passing prefix/name as $1/$2 to avoid quoting + opam variables into the script. *) + ( "post-install-commands", + {|["sh" "-c" "mkdir -p \"$1/odoc/$2\" && touch \"$1/odoc/$2/.switchdocs-stale\"" "--" "%{prefix}%" "%{name}%"] {error-code = 0}|}, + ".switchdocs-stale" ); + (* post-remove: delete the package's docs outright. Run as an argv list (no + shell), so a prefix with spaces needs no quoting. *) + ( "post-remove-commands", + {|["rm" "-rf" "%{prefix}%/odoc/%{name}%"] {error-code = 0}|}, + "/odoc/%{name}%" ); + (* post-session: the one hook that needs switchdocs — rebuild stale packages + and refresh the landing page. *) ( "post-session-commands", Printf.sprintf {|[%s "sync" "--prefix" "%%{prefix}%%"] {success}|} - (quote exe) ); + (quote exe), + exe ); ] -let opam_option_arg (field, value) = Printf.sprintf "%s+=%s" field value +let opam_option_arg field value = Printf.sprintf "%s+=%s" field value let print ~exe = List.iter - (fun fv -> Printf.printf "opam option --global '%s'\n" (opam_option_arg fv)) + (fun (field, value, _marker) -> + Printf.printf "opam option --global '%s'\n" (opam_option_arg field value)) (fields ~exe) -let already_set field exe = +let already_set field marker = let cmd = Bos.Cmd.(v "opam" % "option" % "--global" % field) in match Bos.OS.Cmd.(run_out ~err:err_null cmd |> to_string ~trim:true) with - | Ok current -> Astring.String.is_infix ~affix:exe current + | Ok current -> Astring.String.is_infix ~affix:marker current | Error _ -> false let apply ~exe = List.fold_left - (fun acc (field, value) -> + (fun acc (field, value, marker) -> Result.bind acc @@ fun () -> - if already_set field exe then ( + if already_set field marker then ( Printf.printf "%s already configured; skipping\n" field; Ok ()) else let cmd = Bos.Cmd.( - v "opam" % "option" % "--global" % opam_option_arg (field, value)) + v "opam" % "option" % "--global" % opam_option_arg field value) in match Bos.OS.Cmd.run cmd with | Ok () -> diff --git a/lib/setup.mli b/lib/setup.mli index d81ec87..372d299 100644 --- a/lib/setup.mli +++ b/lib/setup.mli @@ -3,9 +3,11 @@ never by writing the config file directly, so the file always matches the opam version that owns it. *) -val fields : exe:string -> (string * string) list -(** The wrapper fields and the command elements to append to them, for the - recorder/worker binary at [exe]. *) +val fields : exe:string -> (string * string * string) list +(** The wrapper fields, the command to append to each, and a substring + identifying our command if already present. The per-package hooks are plain + coreutils ([mkdir]/[touch]/[rm]) so they work without switchdocs installed; + only the post-session worker invokes the binary at [exe]. *) val print : exe:string -> unit (** Print the [opam option] invocations that {!apply} would run. *) diff --git a/lib/switch.ml b/lib/switch.ml index e94094c..c07a1a3 100644 --- a/lib/switch.ml +++ b/lib/switch.ml @@ -29,7 +29,6 @@ let detect () = m))) let state_dir t = Fpath.(t.prefix / "var" / "cache" / "switchdocs") -let pending_file t = Fpath.(state_dir t / "pending") let log_file t = Fpath.(state_dir t / "log") let lock_file t = Fpath.(state_dir t / "lock") let work_dir t = Fpath.(state_dir t / "work") diff --git a/lib/switch.mli b/lib/switch.mli index 4ec46bf..215504c 100644 --- a/lib/switch.mli +++ b/lib/switch.mli @@ -12,9 +12,8 @@ val detect : unit -> (t, [ `Msg of string ]) result val prefix : t -> Fpath.t val state_dir : t -> Fpath.t -(** [/var/cache/switchdocs], holding all switchdocs state. *) +(** [/var/cache/switchdocs], holding the log and lock. *) -val pending_file : t -> Fpath.t val log_file : t -> Fpath.t val lock_file : t -> Fpath.t diff --git a/lib/sync.ml b/lib/sync.ml index e394304..a8ec3c6 100644 --- a/lib/sync.ml +++ b/lib/sync.ml @@ -1,14 +1,10 @@ module SS = Set.Make (String) -type outcome = { - built : string list; - failed : string list; - removed : string list; -} +type outcome = { built : string list; failed : string list } let summary o = - Printf.sprintf "%d built, %d failed, %d removed" (List.length o.built) - (List.length o.failed) (List.length o.removed) + Printf.sprintf "%d built, %d failed" (List.length o.built) + (List.length o.failed) let timestamp () = let tm = Unix.localtime (Unix.gettimeofday ()) in @@ -109,34 +105,32 @@ let run_driver sw ~driver ~env name = let sync ?(driver = Bos.Cmd.v "odoc_driver_opam") sw = with_lock sw @@ fun () -> - match Pending.read sw with - | [] -> Ok { built = []; failed = []; removed = [] } - | entries -> - let installed_names = SS.of_list (List.map fst (Switch.installed sw)) in - let touched = SS.of_list (List.map (fun e -> e.Pending.name) entries) in - let removed = SS.diff touched installed_names in - let stale = SS.inter touched installed_names in - SS.iter (erase_doc sw) removed; - Result.bind (Pending.remove_packages sw (SS.elements removed)) - @@ fun () -> - let finish outcome = - Result.bind (Index_page.write sw) @@ fun () -> - log sw (summary outcome); - Ok outcome - in + let installed = SS.of_list (List.map fst (Switch.installed sw)) in + let marked = Pending.read sw in + let stale = List.filter (fun n -> SS.mem n installed) marked in + (* A marker for a package that is no longer installed can only be left over + from an install+remove in the same session (the remove hook's [rm -rf] + normally clears it). Tidy it away. *) + List.iter (fun n -> if not (SS.mem n installed) then erase_doc sw n) marked; + (* Always regenerate the landing page (skipping the write when unchanged), so + a session that only removed packages still drops them from the list. *) + let finish outcome = + Result.bind (Index_page.write sw) @@ fun () -> + if outcome.built <> [] || outcome.failed <> [] then log sw (summary outcome); + Ok outcome + in + match stale with + | [] -> finish { built = []; failed = [] } + | _ -> let driver_exists = match Bos.OS.Cmd.resolve driver with Ok _ -> true | Error _ -> false in if not driver_exists then ( log sw (Printf.sprintf "driver %s not found; leaving %d package(s) pending" - (Bos.Cmd.to_string driver) (SS.cardinal stale)); - finish - { - built = []; - failed = SS.elements stale; - removed = SS.elements removed; - }) + (Bos.Cmd.to_string driver) (List.length stale)); + (* Markers left intact, so the next session retries. *) + finish { built = []; failed = stale }) else Result.bind (Result.map @@ -144,21 +138,18 @@ let sync ?(driver = Bos.Cmd.v "odoc_driver_opam") sw = (Bos.OS.Dir.create ~path:true (Switch.work_dir sw))) @@ fun () -> let env = driver_env sw in - let ordered = Deps.order ~warn:(log sw) sw (SS.elements stale) in + let ordered = Deps.order ~warn:(log sw) sw stale in let built, failed = List.fold_left (fun (built, failed) name -> + (* Erase first (removing the marker) for a clean build; on failure + re-mark so the package is retried next session. *) erase_doc sw name; match run_driver sw ~driver ~env name with - | Ok () -> - ignore (Pending.remove_packages sw [ name ]); - (name :: built, failed) - | Error () -> (built, name :: failed)) + | Ok () -> (name :: built, failed) + | Error () -> + ignore (Pending.mark sw name); + (built, name :: failed)) ([], []) ordered in - finish - { - built = List.rev built; - failed = List.rev failed; - removed = SS.elements removed; - } + finish { built = List.rev built; failed = List.rev failed } diff --git a/lib/sync.mli b/lib/sync.mli index 585b3d8..0031998 100644 --- a/lib/sync.mli +++ b/lib/sync.mli @@ -1,21 +1,20 @@ -(** The session worker: process the pending set once. *) +(** The session worker: process the stale markers once. *) -type outcome = { - built : string list; - failed : string list; - removed : string list; -} +type outcome = { built : string list; failed : string list } val log : Switch.t -> string -> unit (** Append a timestamped line to the switch's switchdocs log. Best-effort; never raises. *) val sync : ?driver:Bos.Cmd.t -> Switch.t -> (outcome, [ `Msg of string ]) result -(** Process the pending set: delete the docs of removed packages, rebuild stale - ones in dependency order (each erased then rebuilt via the driver, default - [odoc_driver_opam]), update the landing page. Entries are dropped from the - pending file per package on success, so failures are retried by the next - session. Runs under a lock file; driver output goes to the log. *) +(** Rebuild every package with a stale marker ({!Pending}), in dependency order, + each erased then rebuilt via the driver (default [odoc_driver_opam]), and + regenerate the landing page. A successful build clears the marker (the erase + removes it); a failed one re-marks the package so the next session retries. + Removals are not handled here — the post-remove hook deletes the docs + directly — but the landing page is regenerated every session so removed + packages drop off it. Runs under a lock file; driver output goes to the + log. *) val summary : outcome -> string -(** One line, e.g. ["2 built, 0 failed, 1 removed"]. *) +(** One line, e.g. ["2 built, 0 failed"]. *) diff --git a/test/record.t b/test/record.t deleted file mode 100644 index ddd9281..0000000 --- a/test/record.t +++ /dev/null @@ -1,19 +0,0 @@ -The recorder appends one line per package action to the pending file, -creating the state directory as needed. - - $ switchdocs record install --prefix prefix foo 1.0 - $ switchdocs record install --prefix prefix bar 2.0+dev - $ switchdocs record remove --prefix prefix baz 0.3 - $ cat prefix/var/cache/switchdocs/pending - install foo 1.0 - install bar 2.0+dev - remove baz 0.3 - -It is hook-facing, so it reports errors on stderr but still exits 0 — a -recording failure must never fail the surrounding opam action. Here the -state directory path is occupied by a file: - - $ mkdir -p broken/var/cache && touch broken/var/cache/switchdocs - $ switchdocs record install --prefix broken foo 1.0 2>&1 | head -1 | sed 's/:.*//' - switchdocs record - $ switchdocs record install --prefix broken foo 1.0 2>/dev/null diff --git a/test/sync.t b/test/sync.t index 866b1a2..b541075 100644 --- a/test/sync.t +++ b/test/sync.t @@ -20,76 +20,81 @@ package's doc directory. > EOF $ chmod +x driver.sh -An empty pending set is a no-op with no output: +The post-install hook marks a package stale by touching a marker file; the +post-remove hook just deletes the package's doc directory. Stand in for them: + + $ stale() { mkdir -p "prefix/odoc/$1"; touch "prefix/odoc/$1/.switchdocs-stale"; } + $ removed() { rm -rf "prefix/odoc/$1"; } + +No stale markers is a no-op with no output: $ switchdocs sync --prefix prefix --driver ./driver.sh -Recorded packages are rebuilt in dependency order, regardless of the order -they were recorded in (b was recorded first but depends on a): +Marked packages are rebuilt in dependency order, regardless of the order they +were marked in (b was marked first but depends on a): - $ switchdocs record install --prefix prefix b 1 - $ switchdocs record install --prefix prefix a 1 + $ stale b + $ stale a $ switchdocs sync --prefix prefix --driver ./driver.sh - switchdocs: 2 built, 0 failed, 0 removed + switchdocs: 2 built, 0 failed $ cat prefix/build-order a b -The pending file is consumed, and a landing page lists the documented +A successful build clears the marker, and a landing page lists the documented packages: - $ test -e prefix/var/cache/switchdocs/pending || echo consumed + $ test -e prefix/odoc/a/.switchdocs-stale || echo consumed consumed $ grep -o '
  • .*
  • ' prefix/odoc/index.html | sed 's/<[^>]*>//g' a 1 b 1 -A failing package stays pending for the next session; others still build: +A failing package stays marked for the next session; others still build: $ mkpkg bad 1 < opam-version: "2.0" > EOF - $ switchdocs record install --prefix prefix bad 1 - $ switchdocs record install --prefix prefix a 1 + $ stale bad + $ stale a $ switchdocs sync --prefix prefix --driver ./driver.sh - switchdocs: 1 built, 1 failed, 0 removed + switchdocs: 1 built, 1 failed switchdocs: see $TESTCASE_ROOT/prefix/var/cache/switchdocs/log - $ cat prefix/var/cache/switchdocs/pending - install bad 1 + $ test -e prefix/odoc/bad/.switchdocs-stale && echo still-marked + still-marked -A recorded removal (package no longer installed) deletes its docs and its -landing-page entry. Note that bad, still pending from the previous run, is -retried (and fails again) — that is the designed retry-on-next-session -behaviour: +A removal deletes the package's docs (post-remove hook); the landing page +drops it the next time sync runs. Note that bad, still marked from the previous +run, is retried (and fails again) — the designed retry behaviour: $ test -d prefix/odoc/b && echo present present $ rm -r prefix/.opam-switch/packages/b.1 - $ switchdocs record remove --prefix prefix b 1 + $ removed b $ switchdocs sync --prefix prefix --driver ./driver.sh - switchdocs: 0 built, 1 failed, 1 removed + switchdocs: 0 built, 1 failed switchdocs: see $TESTCASE_ROOT/prefix/var/cache/switchdocs/log $ test -d prefix/odoc/b || echo gone gone $ grep -o '
  • .*
  • ' prefix/odoc/index.html | sed 's/<[^>]*>//g' a 1 -A missing driver leaves everything pending and is reported via the summary +A missing driver leaves marked packages stale and is reported via the summary (sync still exits 0 — it is hook-facing): - $ switchdocs record install --prefix prefix a 1 + $ stale a $ switchdocs sync --prefix prefix --driver ./no-such-driver - switchdocs: 0 built, 2 failed, 0 removed + switchdocs: 0 built, 2 failed switchdocs: see $TESTCASE_ROOT/prefix/var/cache/switchdocs/log - $ cat prefix/var/cache/switchdocs/pending - install bad 1 - install a 1 + $ ls prefix/odoc/a/.switchdocs-stale prefix/odoc/bad/.switchdocs-stale + prefix/odoc/a/.switchdocs-stale + prefix/odoc/bad/.switchdocs-stale rebuild is the user-facing equivalent: it marks packages stale itself and fails loudly: $ switchdocs rebuild --prefix prefix --driver ./driver.sh a bad - switchdocs: 1 built, 1 failed, 0 removed + switchdocs: 1 built, 1 failed switchdocs: see $TESTCASE_ROOT/prefix/var/cache/switchdocs/log [1] $ switchdocs rebuild --prefix prefix --driver ./driver.sh nosuchpkg