mountpoint detection: wrong marker path, and requestedCount stops one short #6

New Issue

2026-08-30T13:34:22+02:00

clawbot commented

2026-08-30 13:34:22 +02:00

Two defects in findDCFMountPoints (pkg/dcf/helpers.go), found while bringing the repo to standards. Both predate that work and its PR leaves the behaviour exactly as it was, because deciding either is a behaviour change rather than a lint fix.

1. The marker directory checked is M4ROOT, not PRIVATE/M4ROOT.

The original code computed filepath.Join(mountpoint, "PRIVATE") and then immediately overwrote it with filepath.Join(mountpoint, "M4ROOT"), so only the second was ever stat'd. That dead store was a lint finding and had to go; the surviving check is the one that was live, so a Sony card is detected only if it happens to have M4ROOT at the filesystem root. README and the spec both say the directory is PRIVATE/M4ROOT.

2. requestedCount returns one fewer store than asked for.

The loop breaks on len(filteredMountpoints)+1 >= requestedCount, so requestedCount == 2 returns 1 store and requestedCount == 3 returns 2. Only requestedCount == 1 and 0 (all) behave as documented.

Both want a decision about intended behaviour, plus a test each.

Two defects in `findDCFMountPoints` (`pkg/dcf/helpers.go`), found while bringing the repo to standards. Both predate that work and its PR leaves the behaviour exactly as it was, because deciding either is a behaviour change rather than a lint fix. **1. The marker directory checked is `M4ROOT`, not `PRIVATE/M4ROOT`.** The original code computed `filepath.Join(mountpoint, "PRIVATE")` and then immediately overwrote it with `filepath.Join(mountpoint, "M4ROOT")`, so only the second was ever stat'd. That dead store was a lint finding and had to go; the surviving check is the one that was live, so a Sony card is detected only if it happens to have `M4ROOT` at the filesystem root. README and the spec both say the directory is `PRIVATE/M4ROOT`. **2. `requestedCount` returns one fewer store than asked for.** The loop breaks on `len(filteredMountpoints)+1 >= requestedCount`, so `requestedCount == 2` returns 1 store and `requestedCount == 3` returns 2. Only `requestedCount == 1` and `0` (all) behave as documented. Both want a decision about intended behaviour, plus a test each.

clawbot referenced this issue

2026-08-30 13:37:52 +02:00

bring the repo up to org standards (closes #1) #7

clawbot referenced this issue

2026-10-03 14:28:47 +02:00

bring the repo up to repo standards in one PR #1

clawbot commented

2026-10-03 14:28:52 +02:00

Plan. The repo's own documents settle both points, so this needs no decision from sneak.

Marker path. The README says cameras put files in DCIM and PRIVATE/M4ROOT. Keep a mount point when it has DCIM or PRIVATE/M4ROOT at its root; drop the check for a bare M4ROOT at the root. Fix the function comment to match.
Count. The function comment says the argument is how many mount points to return, 0 meaning all. Return exactly that many when at least that many match.
Return the error from listing partitions instead of discarding it, if the standards PR (#1) has not already.

Tests: split the per-mount-point check and the counting from the partition listing, so a test can hand it temporary directories. Cases: a directory with only PRIVATE/M4ROOT is kept; one with only a root M4ROOT is not; one with DCIM is kept; with three matching directories, asking for 0, 1, 2 and 3 returns 3, 1, 2 and 3.

Depends on #1 (no make check before it). Done: those tests pass under make check; README, comment and code agree.

Model: opus-5-5

Plan. The repo's own documents settle both points, so this needs no decision from sneak. 1. **Marker path.** The README says cameras put files in `DCIM` and `PRIVATE/M4ROOT`. Keep a mount point when it has `DCIM` or `PRIVATE/M4ROOT` at its root; drop the check for a bare `M4ROOT` at the root. Fix the function comment to match. 2. **Count.** The function comment says the argument is how many mount points to return, 0 meaning all. Return exactly that many when at least that many match. 3. Return the error from listing partitions instead of discarding it, if the standards PR (https://git.eeqj.de/sneak/dcf/issues/1) has not already. Tests: split the per-mount-point check and the counting from the partition listing, so a test can hand it temporary directories. Cases: a directory with only `PRIVATE/M4ROOT` is kept; one with only a root `M4ROOT` is not; one with `DCIM` is kept; with three matching directories, asking for 0, 1, 2 and 3 returns 3, 1, 2 and 3. Depends on https://git.eeqj.de/sneak/dcf/issues/1 (no `make check` before it). Done: those tests pass under `make check`; README, comment and code agree. Model: opus-5-5

clawbot referenced this issue

2026-10-03 14:29:48 +02:00

API cleanup: fs.FS-based store access, no *[]DCFStore, contexts on walks #4

clawbot referenced this issue

2026-10-03 14:38:11 +02:00

bring the repo up to repo standards in one PR #1

clawbot commented

2026-10-03 14:38:15 +02:00