Widen the accepted pimsync range to the 0.6 series; stream sync output and bound the Google HTTP calls

The version work and the sync-output work landed in one change; they can be
split apart if the history wants them separate.

README.md

@@ -25,7 +25,16 @@ Or straight from the repo, without cloning it first:
 nix profile install "git+https://code.randogoth.com/randogoth/CalCalist.git#calcalist"
 ```
 
-One runtime dependency is needed for CalDAV and iCal sync: **`pimsync` 0.5.11 or newer on `PATH`**. The Nix flake installation method already ensures this.
+One runtime dependency is needed for CalDAV and iCal sync: **`pimsync`, from 0.5.11 up to but not including 0.7.0, on `PATH`**. The Nix flake installation method already ensures this: it builds pimsync from source and wraps `calcalist` so that copy is the one found, whatever else is installed.
+
+To run a different pimsync — a fork, or a working tree — override the input at install time:
+
+```sh
+nix profile install "/path/to/calcalist#calcalist" \
+  --override-input pimsync-src "git+file:///path/to/pimsync"
+```
+
+The built pimsync reports the revision it came from (`pimsync 0.6.0-a08ee3c`), so the two are told apart. Note that `nix profile upgrade` does **not** remember an override: upgrading rebuilds against the default input, and the install command has to be repeated to get the fork back.
 
 Then check the environment, which is worth doing before writing any config:
 

flake.lock

@@ -16,9 +16,28 @@
         "type": "github"
       }
     },
+    "pimsync-src": {
+      "flake": false,
+      "locked": {
+        "lastModified": 1788568982,
+        "narHash": "sha256-vpmms553q1mzxfkMGEjvTddQVksEAzu50g8bVudZsXg=",
+        "ref": "refs/tags/v0.6.0",
+        "rev": "a111a44cd9e65c031ce9801fdced912057e4a4a0",
+        "revCount": 1616,
+        "type": "git",
+        "url": "https://git.sr.ht/~whynothugo/pimsync"
+      },
+      "original": {
+        "ref": "refs/tags/v0.6.0",
+        "rev": "a111a44cd9e65c031ce9801fdced912057e4a4a0",
+        "type": "git",
+        "url": "https://git.sr.ht/~whynothugo/pimsync"
+      }
+    },
     "root": {
       "inputs": {
-        "nixpkgs": "nixpkgs"
+        "nixpkgs": "nixpkgs",
+        "pimsync-src": "pimsync-src"
       }
     }
   },

flake.nix

@@ -3,8 +3,26 @@
 
   inputs.nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable";
 
+  # pimsync is built from source rather than taken from nixpkgs, which trails
+  # the releases calcalist has been read against. Being an input rather than a
+  # `fetchFromSourcehut` call makes it substitutable: a fork or a working tree
+  # can be swapped in at install time with
+  # `--override-input pimsync-src git+file:///path/to/pimsync`, without the
+  # committed flake naming anything that exists on one machine only.
+  #
+  # The rev is the commit v0.6.0 points at: nix cannot peel an annotated tag,
+  # and resolving the ref alone fails.
+  inputs.pimsync-src = {
+    url = "git+https://git.sr.ht/~whynothugo/pimsync?ref=refs/tags/v0.6.0&rev=a111a44cd9e65c031ce9801fdced912057e4a4a0";
+    flake = false;
+  };
+
   outputs =
-    { self, nixpkgs }:
+    {
+      self,
+      nixpkgs,
+      pimsync-src,
+    }:
     let
       systems = [
         "x86_64-linux"
@@ -19,30 +37,48 @@
         let
           pkgs = import nixpkgs { inherit system; };
 
-          # nixpkgs currently ships pimsync 0.5.9, but `discover`'s output
+          # pimsync's Cargo.toml carries no version — the binary learns its own
-          # format is undocumented and changed between 0.5.9 and 0.5.11 —
+          # from `git describe`, which has nothing to describe inside the
-          # calcalist's parser only understands the newer shape (see
+          # store. So it is assembled: the newest changelog heading for the
-          # SUPPORTED_PATCH_MIN in src/pimsync.rs). Build the exact version
+          # release, and the input's revision for the tree it came from. That
-          # rather than trust whatever nixpkgs happens to carry.
+          # second half is what distinguishes an overridden fork from the
-          pimsync0_5_11 = pkgs.rustPlatform.buildRustPackage (finalAttrs: {
+          # upstream release it descends from, and a suffixed version is
-            pname = "pimsync";
+          # pimsync's own convention for a build off a tag.
-            version = "0.5.11";
+          pimsyncVersion =
-            src = pkgs.fetchFromSourcehut {
+            let
-              owner = "~whynothugo";
+              lines = pkgs.lib.splitString "\n" (builtins.readFile "${pimsync-src}/CHANGELOG.rst");
-              repo = "pimsync";
+              isRelease = line: builtins.match "v[0-9]+\\.[0-9]+\\.[0-9]+" line != null;
-              rev = "v${finalAttrs.version}";
+              newest =
-              hash = "sha256-iMdBqSSguViF+54e47IGV8hH3qvTxcNkWkmND1QAAxw=";
+                pkgs.lib.findFirst isRelease (throw "no release heading found in pimsync's CHANGELOG.rst")
-            };
+                  lines;
-            cargoHash = "sha256-dvkZ047eJnvYvyH1iW1NJo3Uv0L2T7waPYKN12bi+dA=";
+              release = pkgs.lib.removePrefix "v" newest;
+            in
+            "${release}-${pimsync-src.dirtyShortRev or pimsync-src.shortRev}";
 
-            # Skips build.rs's `git describe` call, which has nothing to
+          pimsync = pkgs.rustPlatform.buildRustPackage {
-            # describe once fetched as a plain source tarball.
+            pname = "pimsync";
-            env.PIMSYNC_VERSION = finalAttrs.version;
+            version = pimsyncVersion;
+            src = pimsync-src;
+
+            cargoLock = {
+              lockFile = "${pimsync-src}/Cargo.lock";
+
+              # pimsync pins a few dependencies to git revisions, and vstorage
+              # — which the fork swaps in — keeps libdav and libjmap as
+              # submodules of its own. Naming those revisions in
+              # `outputHashes` fetches them without submodules and vendors
+              # two empty directories; the builtin fetcher recurses properly.
+              # The revisions come from the lock file, so evaluation stays
+              # pure, and there is no hash here to go stale when a fork moves.
+              allowBuiltinFetchGit = true;
+            };
+
+            env.PIMSYNC_VERSION = pimsyncVersion;
 
             nativeBuildInputs = [ pkgs.pkg-config ];
             buildInputs = [ pkgs.sqlite ];
             doCheck = false;
-          });
+          };
         in
         rec {
           calcalist = pkgs.rustPlatform.buildRustPackage {
@@ -62,7 +98,7 @@
             # against, regardless of what else is on the user's PATH.
             postInstall = ''
               wrapProgram $out/bin/calcalist \
-                --prefix PATH : ${pkgs.lib.makeBinPath [ pimsync0_5_11 ]}
+                --prefix PATH : ${pkgs.lib.makeBinPath [ pimsync ]}
             '';
 
             meta = {

src/doctor.rs

@@ -100,11 +100,10 @@ fn check_pimsync() -> Check {
     let outcome = match pimsync::probe() {
         Ok(version) if version.is_supported() => Outcome::Ok(version.to_string()),
         Ok(version) => Outcome::Warn(format!(
-            "{version} is outside the supported {}.{}.{}+ series; \
+            "{version} is outside the supported range, {} up to but not including {}; \
              config syntax may differ, and older patches misreport `discover`",
-            pimsync::SUPPORTED_MAJOR,
+            pimsync::MIN_VERSION,
-            pimsync::SUPPORTED_MINOR,
+            pimsync::NEXT_UNKNOWN_VERSION,
-            pimsync::SUPPORTED_PATCH_MIN,
         )),
         Err(error) => Outcome::Fail(error.to_string()),
     };

src/google/api.rs

@@ -62,10 +62,7 @@ pub struct Changes {
 
 impl Client {
     pub fn new(access_token: String, calendar_id: String) -> Self {
-        let agent: ureq::Agent = ureq::Agent::config_builder()
+        let agent = super::agent();
-            .http_status_as_error(false)
-            .build()
-            .into();
         Client {
             agent,
             access_token,

src/google/auth.rs

@@ -401,10 +401,7 @@ pub fn login(
 /// Returned as an option rather than an error: failing to name the account only
 /// costs the sharing, and the authorisation itself is perfectly good without it.
 fn primary_calendar_id(access_token: &str) -> Option<String> {
-    let agent: ureq::Agent = ureq::Agent::config_builder()
+    let agent = super::agent();
-        .http_status_as_error(false)
-        .build()
-        .into();
     let mut response = agent
         .get("https://www.googleapis.com/calendar/v3/calendars/primary")
         .header("Authorization", format!("Bearer {access_token}"))
@@ -487,10 +484,7 @@ fn post_token(form: &[(&str, &str)]) -> Result<IssuedToken, AuthError> {
         .collect::<Vec<_>>()
         .join("&");
 
-    let agent: ureq::Agent = ureq::Agent::config_builder()
+    let agent = super::agent();
-        .http_status_as_error(false)
-        .build()
-        .into();
     let mut response = agent
         .post(TOKEN_ENDPOINT)
         .content_type("application/x-www-form-urlencoded")

src/google/mod.rs

@@ -8,3 +8,28 @@
 pub mod api;
 pub mod auth;
 pub mod convert;
+
+use std::time::Duration;
+
+/// How long one request may take, end to end. ureq defaults every timeout to
+/// none, so a server that accepts a connection and then says nothing used to
+/// stall a cycle for as long as the machine stayed up — indistinguishable, from
+/// the terminal, from a sync that was merely slow.
+const REQUEST_TIMEOUT: Duration = Duration::from_secs(60);
+
+/// Long enough for a handshake over a bad link, short enough that an
+/// unreachable host is reported rather than waited on.
+const CONNECT_TIMEOUT: Duration = Duration::from_secs(15);
+
+/// The HTTP client every Google call goes through.
+///
+/// `http_status_as_error` is off because the callers read the status themselves:
+/// a 410 drives a full resync, and a 404 is an ordinary answer here.
+pub fn agent() -> ureq::Agent {
+    ureq::Agent::config_builder()
+        .http_status_as_error(false)
+        .timeout_global(Some(REQUEST_TIMEOUT))
+        .timeout_connect(Some(CONNECT_TIMEOUT))
+        .build()
+        .into()
+}

src/pimsync.rs

@@ -7,8 +7,9 @@
 use std::fmt;
 use std::fs;
 use std::io;
+use std::os::fd::AsFd;
 use std::path::{Path, PathBuf};
-use std::process::Command;
+use std::process::{Command, Output, Stdio};
 
 use thiserror::Error;
 
@@ -16,16 +17,27 @@ use crate::config::{Config, Endpoint, EndpointKind};
 
 pub const BINARY: &str = "pimsync";
 
-/// The `0.5.x` series this build generates configuration for. pimsync is
+/// The oldest pimsync this build generates configuration for. `discover`'s
-/// pre-1.0 and its config syntax may change between minor releases, so the
-/// range is deliberately narrow. The patch floor matters too: `discover`'s
 /// output format is undocumented and changed between 0.5.9 and 0.5.11, and
 /// `parse_discovery` only understands the newer shape.
-pub const SUPPORTED_MAJOR: u64 = 0;
+pub const MIN_VERSION: Version = Version {
-pub const SUPPORTED_MINOR: u64 = 5;
+    major: 0,
-pub const SUPPORTED_PATCH_MIN: u64 = 11;
+    minor: 5,
+    patch: 11,
+};
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+/// The first release this build makes no claim about. pimsync is pre-1.0 and
+/// its config syntax may change between minor releases, so a series is
+/// admitted only once read: 0.6.0 renamed `auth` to `auth_method`, which costs
+/// nothing here — generated storages carry `username` and `password`, never
+/// `auth` — and left `discover`'s output word for word as it was.
+pub const NEXT_UNKNOWN_VERSION: Version = Version {
+    major: 0,
+    minor: 7,
+    patch: 0,
+};
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
 pub struct Version {
     pub major: u64,
     pub minor: u64,
@@ -40,9 +52,7 @@ impl fmt::Display for Version {
 
 impl Version {
     pub fn is_supported(&self) -> bool {
-        self.major == SUPPORTED_MAJOR
+        *self >= MIN_VERSION && *self < NEXT_UNKNOWN_VERSION
-            && self.minor == SUPPORTED_MINOR
-            && self.patch >= SUPPORTED_PATCH_MIN
     }
 
     /// Parses the `pimsync 0.5.11` line printed by `pimsync version`.
@@ -50,18 +60,26 @@ impl Version {
         let field = output.split_whitespace().nth(1)?;
         let mut parts = field.split('.');
         Some(Version {
-            major: parts.next()?.parse().ok()?,
+            major: number(parts.next()?)?,
-            minor: parts.next()?.parse().ok()?,
+            minor: number(parts.next()?)?,
-            patch: parts.next().unwrap_or("0").parse().ok()?,
+            patch: number(parts.next().unwrap_or("0"))?,
         })
     }
 }
 
+/// The leading digits of one version component. pimsync takes its version from
+/// `git describe`, so a build off a tag carries a suffix — `0.6.0-1-ga08ee3c`
+/// — which belongs to no number.
+fn number(part: &str) -> Option<u64> {
+    part.split(|c: char| !c.is_ascii_digit())
+        .next()?
+        .parse()
+        .ok()
+}
+
 #[derive(Debug, Error)]
 pub enum PimsyncError {
-    #[error(
+    #[error("`{BINARY}` was not found on PATH; install pimsync {MIN_VERSION} or newer")]
-        "`{BINARY}` was not found on PATH; install pimsync {SUPPORTED_MAJOR}.{SUPPORTED_MINOR}.{SUPPORTED_PATCH_MIN} or newer"
-    )]
     NotFound,
     #[error("could not run `{BINARY}`: {0}")]
     Spawn(#[source] io::Error),
@@ -88,13 +106,7 @@ pub fn probe() -> Result<Version, PimsyncError> {
     let output = Command::new(BINARY)
         .arg("version")
         .output()
-        .map_err(|error| {
+        .map_err(spawn_error)?;
-            if error.kind() == io::ErrorKind::NotFound {
-                PimsyncError::NotFound
-            } else {
-                PimsyncError::Spawn(error)
-            }
-        })?;
     if !output.status.success() {
         return Err(PimsyncError::Failed(
             String::from_utf8_lossy(&output.stderr).trim().to_string(),
@@ -332,13 +344,7 @@ pub fn discover(
         .arg(&config_path)
         .arg("discover")
         .output()
-        .map_err(|error| {
+        .map_err(spawn_error)?;
-            if error.kind() == io::ErrorKind::NotFound {
-                PimsyncError::NotFound
-            } else {
-                PimsyncError::Spawn(error)
-            }
-        })?;
 
     // Everything, including the failures, comes back on stdout.
     let text = String::from_utf8_lossy(&output.stdout).into_owned();
@@ -453,34 +459,40 @@ fn strip_ansi(text: &str) -> String {
 ///
 /// Deliberately one-shot: `pimsync daemon` would write the same vdirs the
 /// reconciler is reading, so calcalist owns the scheduling instead.
+/// A cycle is minutes of network work, and pimsync's per-item progress is the
+/// only sign it is alive. Capturing that output left `sync` looking wedged for
+/// the whole run and then printing nothing, so it goes straight to the terminal.
 pub fn sync(config_path: &Path) -> Result<(), PimsyncError> {
-    run(config_path, "sync")
+    let status = invocation(config_path, "sync")
+        .stdout(progress_stream()?)
+        .stderr(Stdio::inherit())
+        .status()
+        .map_err(spawn_error)?;
+    if status.success() {
+        return Ok(());
+    }
+    // The detail has already been printed as it happened; repeating it here
+    // would only put a second copy under the report.
+    Err(PimsyncError::Command {
+        command: "sync".to_string(),
+        message: format!("exited with {status}"),
+    })
 }
 
 /// Validates the generated configuration and the storages it names.
+///
+/// Captured rather than streamed: `doctor` reads the message to tell a config
+/// it must fix from a server it cannot reach, and prints its own summary.
 pub fn check(config_path: &Path) -> Result<(), PimsyncError> {
-    run(config_path, "check")
+    let output = invocation(config_path, "check")
-}
-
-fn run(config_path: &Path, command: &str) -> Result<(), PimsyncError> {
-    let output = Command::new(BINARY)
-        .arg("-c")
-        .arg(config_path)
-        .arg(command)
         .output()
-        .map_err(|error| {
+        .map_err(spawn_error)?;
-            if error.kind() == io::ErrorKind::NotFound {
-                PimsyncError::NotFound
-            } else {
-                PimsyncError::Spawn(error)
-            }
-        })?;
     if output.status.success() {
         return Ok(());
     }
-    let message = String::from_utf8_lossy(&output.stderr).trim().to_string();
+    let message = diagnostics(&output);
     Err(PimsyncError::Command {
-        command: command.to_string(),
+        command: "check".to_string(),
         message: if message.is_empty() {
             format!("exited with {}", output.status)
         } else {
@@ -489,6 +501,51 @@ fn run(config_path: &Path, command: &str) -> Result<(), PimsyncError> {
     })
 }
 
+/// `pimsync -c <config> <command>`, with stdin closed so that a secret helper
+/// reaching for a terminal fails outright rather than waiting on one forever.
+fn invocation(config_path: &Path, command: &str) -> Command {
+    let mut process = Command::new(BINARY);
+    process
+        .arg("-c")
+        .arg(config_path)
+        .arg(command)
+        .stdin(Stdio::null());
+    process
+}
+
+/// pimsync's progress, routed to our stderr so that stdout carries only the
+/// report and stays worth piping.
+fn progress_stream() -> Result<Stdio, PimsyncError> {
+    io::stderr()
+        .as_fd()
+        .try_clone_to_owned()
+        .map(Stdio::from)
+        .map_err(PimsyncError::Spawn)
+}
+
+fn spawn_error(error: io::Error) -> PimsyncError {
+    if error.kind() == io::ErrorKind::NotFound {
+        PimsyncError::NotFound
+    } else {
+        PimsyncError::Spawn(error)
+    }
+}
+
+/// What a failed run had to say for itself.
+///
+/// Both streams are read: pimsync reports execution errors on stderr but puts
+/// parse failures on stdout, and which one carries the reason is not knowable
+/// from here.
+fn diagnostics(output: &Output) -> String {
+    let streams = [&output.stderr, &output.stdout];
+    let said: Vec<String> = streams
+        .iter()
+        .map(|stream| String::from_utf8_lossy(stream).trim().to_string())
+        .filter(|text| !text.is_empty())
+        .collect();
+    said.join("\n")
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -670,6 +727,20 @@ url_command = "secret-tool lookup service calcalist account gcal-ics"
         );
     }
 
+    /// pimsync built from a checkout rather than a tag reports `git describe`'s
+    /// suffix, which must not swallow the patch number.
+    #[test]
+    fn tolerates_a_describe_suffix() {
+        assert_eq!(
+            Version::parse("pimsync 0.6.0-1-ga08ee3c"),
+            Some(Version {
+                major: 0,
+                minor: 6,
+                patch: 0
+            })
+        );
+    }
+
     #[test]
     fn rejects_unparseable_output() {
         assert_eq!(Version::parse("pimsync"), None);
@@ -677,19 +748,25 @@ url_command = "secret-tool lookup service calcalist account gcal-ics"
     }
 
     #[test]
-    fn only_the_supported_series_is_accepted() {
+    fn the_read_series_are_accepted() {
-        let supported = Version {
+        let floor = Version {
             major: 0,
             minor: 5,
             patch: 11,
         };
-        let next_series = Version {
+        let read = Version {
             major: 0,
             minor: 6,
-            patch: 11,
+            patch: 0,
         };
-        assert!(supported.is_supported());
+        let unread = Version {
-        assert!(!next_series.is_supported());
+            major: 0,
+            minor: 7,
+            patch: 0,
+        };
+        assert!(floor.is_supported());
+        assert!(read.is_supported());
+        assert!(!unread.is_supported());
     }
 
     #[test]