From 623bfe9a43fe8d7542b5b4387dc4744e3c0f8be1 Mon Sep 17 00:00:00 2001 From: clawbot Date: Mon, 2 Nov 2026 20:30:17 +0100 Subject: [PATCH 1/5] bring the repo up to repo standards (closes #1) Adopt the standard tooling from sneak/prompts: the Makefile and script/ entrypoints, lint and test stages in one Dockerfile, the lint config, the CI workflow and the formatter settings. make fmt also formats Go, with a pinned Go from script/bootstrap. The module path becomes sneak.berlin/go/dcf. Fix the lint findings in pkg/dcf. DCFStore becomes Store and DCFObject becomes Object. Three behaviours change. An error listing mount points is now returned, where a typo used to drop it. Store.String() prints Store{ instead of DCFStore{, and a negative total as 0 B instead of a huge size. Replace the placeholder test with real tests. Move the README into the required sections. make build compiles the library; it pointed at a command that does not exist. Model: opus-5-5 Co-authored-by: clawbot --- .dockerignore | 63 ++++ .editorconfig | 12 + .gitea/workflows/check.yml | 9 + .gitignore | 33 +- .golangci.yml | 98 ++++++ .prettierignore | 2 + .prettierrc | 4 + Dockerfile | 64 ++++ Makefile | 37 ++- README.md | 143 ++++++++- REPO_POLICIES.md | 603 +++++++++++++++++++++++++++++++++++ go.mod | 5 +- go.sum | 2 - package.json | 5 + pkg/dcf/dcf.go | 128 +++++--- pkg/dcf/dcf_internal_test.go | 39 +++ pkg/dcf/dcf_test.go | 159 ++++++++- pkg/dcf/helpers.go | 51 ++- script/bootstrap | 209 ++++++++++++ script/build | 18 ++ script/check | 16 + script/cibuild | 29 ++ script/docker | 25 ++ script/fmt | 37 +++ script/fmt-check | 43 +++ script/install-precommit | 16 + script/lint | 23 ++ script/precommit | 12 + script/projectname | 12 + script/setup | 13 + script/test | 19 ++ yarn.lock | 8 + 32 files changed, 1828 insertions(+), 109 deletions(-) create mode 100644 .dockerignore create mode 100644 .editorconfig create mode 100644 .gitea/workflows/check.yml create mode 100644 .golangci.yml create mode 100644 .prettierignore create mode 100644 .prettierrc create mode 100644 Dockerfile create mode 100644 REPO_POLICIES.md create mode 100644 package.json create mode 100644 pkg/dcf/dcf_internal_test.go create mode 100755 script/bootstrap create mode 100755 script/build create mode 100755 script/check create mode 100755 script/cibuild create mode 100755 script/docker create mode 100755 script/fmt create mode 100755 script/fmt-check create mode 100755 script/install-precommit create mode 100755 script/lint create mode 100755 script/precommit create mode 100755 script/projectname create mode 100755 script/setup create mode 100755 script/test create mode 100644 yarn.lock diff --git a/.dockerignore b/.dockerignore new file mode 100644 index 0000000..7b7e7fd --- /dev/null +++ b/.dockerignore @@ -0,0 +1,63 @@ +# .dockerignore does NOT use .gitignore semantics. Docker matches with +# moby/patternmatcher: filepath.Match plus `**`, so `*` does not cross +# `/` and an unprefixed pattern is anchored at the context root. Every +# depth-independent pattern therefore needs `**/`, or `config/.env` and +# `certs/server.key` still ship while this file reads as solved. Only +# genuinely root-anchored entries go unprefixed. Never transplant these +# into .gitignore, where `**/` is wrong. +# +# Matching is case-sensitive, so secrets use character ranges rather +# than an ALL-CAPS twin, which would still miss `Server.Key`. +# +# Extend with this repo's own host-built artifacts, written anchored: +# `/myapp`, never `**/myapp`, which also matches `cmd/myapp/` and +# deletes the package directory from the context. + +# Excluding .git means `git describe` cannot run in any build stage and +# fails quietly there; pass the version in with --build-arg VERSION. +.git + +# Agent scratch: one full checkout of the repo per in-flight agent. +# Anchored because it occurs once where agents run at the repo root. +# KNOWN GAP: a repo running agents in subdirectories still ships +# `services/api/.claude/` and must add its own anchored entry. +.claude + +# Environment files. `*.env` covers bare `.env` and the `prod.env` +# convention. Re-include a committed template with a negation if the +# build needs one: `!docs/example.env`. +**/*.[eE][nN][vV] +**/.[eE][nN][vV].* +**/.[eE][nN][vV][rR][cC] + +# Private keys and the bundles carrying them. Public certificates +# (*.crt, *.cer) are deliberately absent: they are legitimate inputs. +**/*.[pP][eE][mM] +**/*.[kK][eE][yY] +**/*.[pP]12 +**/*.[pP][fF][xX] +**/[iI][dD]_[rR][sS][aA] +**/[iI][dD]_[dD][sS][aA] +**/[iI][dD]_[eE][cC][dD][sS][aA] +**/[iI][dD]_[eE][dD]25519 + +# Dependencies: restored inside the image, never copied in. +**/node_modules + +# OS metadata. +**/.DS_Store +**/Thumbs.db + +# Editor state: never a build input, and it churns COPY. +**/*.swp +**/*.swo +**/*~ +**/*.bak +**/.idea +**/.vscode +**/*.sublime-* + +# Go test binaries, coverage output and logs built on the host. +**/*.test +**/*.out +**/*.log diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 0000000..2fe0ce0 --- /dev/null +++ b/.editorconfig @@ -0,0 +1,12 @@ +root = true + +[*] +indent_style = space +indent_size = 4 +end_of_line = lf +charset = utf-8 +trim_trailing_whitespace = true +insert_final_newline = true + +[Makefile] +indent_style = tab diff --git a/.gitea/workflows/check.yml b/.gitea/workflows/check.yml new file mode 100644 index 0000000..ee73864 --- /dev/null +++ b/.gitea/workflows/check.yml @@ -0,0 +1,9 @@ +name: check +on: [push] +jobs: + check: + runs-on: ubuntu-latest + steps: + # actions/checkout v4.2.2, 2026-02-22 + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 + - run: script/cibuild diff --git a/.gitignore b/.gitignore index 73f3499..5afdf94 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,32 @@ -dcfinfo +# OS +.DS_Store +Thumbs.db + +# Editors +*.swp +*.swo +*~ +*.bak +.idea/ +.vscode/ +*.sublime-* + +# Agent scratch (worktrees of this repo, created and destroyed by +# in-flight tooling). Unanchored: .gitignore patterns already match at +# every depth, so no prefix is wanted here. This is not a .dockerignore +# entry and must not be given a `**/` prefix on the way into one. +.claude/ + +# Node +node_modules/ + +# Go +*.log +*.out +*.test + +# Environment / secrets +.env +.env.* +*.pem +*.key diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..a7a74c2 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,98 @@ +version: "2" + +# Config schema uses the golangci-lint v2 layout (settings live under +# linters.settings, not top-level linters-settings) so that the +# thresholds below are actually applied by golangci-lint >= v2. + +run: + timeout: 5m + modules-download-mode: readonly + +linters: + default: all + enable: + # Successor to the deprecated gomodguard. Named explicitly, rather than + # left to `default: all`, because it carries the module policy below. + - gomodguard_v2 + disable: + # Genuinely incompatible with project patterns + - exhaustruct # Requires all struct fields + - godot # Requires comments to end with periods + - wrapcheck # Too verbose for internal packages + - varnamelen # Short names like db, id are idiomatic Go + # Deprecated: the warning is attached to the old name, so it is + # silenced by disabling that name, not by enabling the successor. + - wsl # Deprecated, replaced by wsl_v5 + - gomodguard # Deprecated, replaced by gomodguard_v2 + settings: + lll: + line-length: 88 + funlen: + lines: 80 + statements: 50 + cyclop: + max-complexity: 15 + dupl: + threshold: 100 + depguard: + # Test-support code must not be compiled into the shipped binary. A + # test-support package exists to hand a test privileges the program + # itself must never have, so a file that is not a test must not import + # one. Test files, and the files inside a package whose directory name + # ends in `test`, are where that code belongs, and are exempt. + # + # The deny list below is the one part of this file a repository is + # expected to extend, and the only part it may. depguard matches an + # import path against a list of prefixes, so it cannot be told "any path + # whose last segment ends in test"; a repository's own test-support + # packages have to be named here one at a time, by full import path, + # under a module path that differs from repository to repository. Add + # them; change nothing else. + rules: + test-support: + list-mode: lax + files: + - "$all" + - "!$test" + - "!**/*test/**" + deny: + - pkg: net/http/httptest + desc: >- + Test-support code belongs in test files and in packages whose + directory name ends in test, not in the shipped binary. + # Only decisions already recorded in the Go package defaults are + # listed here. Every entry matches the module path exactly. + gomodguard_v2: + blocked: + - module: github.com/rs/zerolog + recommendations: + - log/slog + reason: "Structured logging is stdlib log/slog." + # One entry per pre-fork module path, because the later releases + # are separate paths. A prefix match would be shorter but would + # also reach github.com/go-redis/redismock, the test double for + # the successor these entries recommend. + - module: github.com/go-redis/redis + recommendations: + - github.com/redis/go-redis/v9 + reason: "Pre-fork module; use the maintained go-redis v9." + - module: github.com/go-redis/redis/v7 + recommendations: + - github.com/redis/go-redis/v9 + reason: "Pre-fork module; use the maintained go-redis v9." + - module: github.com/go-redis/redis/v8 + recommendations: + - github.com/redis/go-redis/v9 + reason: "Pre-fork module; use the maintained go-redis v9." + - module: github.com/sergi/go-diff + recommendations: + - github.com/aymanbagabas/go-udiff + reason: "No unified diff output; use go-udiff." + - module: github.com/hexops/gotextdiff + recommendations: + - github.com/aymanbagabas/go-udiff + reason: "Unmaintained fork; use go-udiff." + +issues: + max-issues-per-linter: 0 + max-same-issues: 0 diff --git a/.prettierignore b/.prettierignore new file mode 100644 index 0000000..23d67fc --- /dev/null +++ b/.prettierignore @@ -0,0 +1,2 @@ +node_modules/ +yarn.lock diff --git a/.prettierrc b/.prettierrc new file mode 100644 index 0000000..8af31cd --- /dev/null +++ b/.prettierrc @@ -0,0 +1,4 @@ +{ + "tabWidth": 4, + "proseWrap": "always" +} diff --git a/Dockerfile b/Dockerfile new file mode 100644 index 0000000..9efc66a --- /dev/null +++ b/Dockerfile @@ -0,0 +1,64 @@ +# Lint phase. The linter is invoked directly rather than through `make +# lint` or `script/lint`, which are themselves a docker build and would +# recurse into a daemon that does not exist in a build step. +# +# golangci/golangci-lint:v2.12.2, 2026-11-02 +FROM golangci/golangci-lint@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240 AS lint + +WORKDIR /src + +COPY go.mod go.sum ./ +RUN go mod download + +COPY . . + +RUN golangci-lint run --config .golangci.yml ./... + +# Test phase, invoked directly for the same reason. The Debian image has +# the C compiler the race detector needs. +# +# golang:1.27.3-trixie, 2026-11-02. It carries the Go that +# script/bootstrap installs on the host; change both together, and the +# same image in the last stage. +FROM golang@sha256:c65ecfe613b539ac74c62a4c3b2c27e8f8a6076384c8f27e8a3c0f3be17cc385 AS test + +WORKDIR /src + +COPY go.mod go.sum ./ +RUN go mod download + +COPY . . + +RUN go test -count=1 -timeout 90s -race -cover ./... || \ + { echo "--- Rerunning with -v for details ---"; \ + go test -count=1 -timeout 90s -race -v ./...; exit 1; } + +# Development environment, and the last stage: a plain `docker build .` +# names no target and so builds this one. Nothing is wanted from the two +# phases above; the copies are what make BuildKit build them first, so +# this image cannot be produced unless lint and test passed. A stage +# appended after this one would drop all three out of a plain build. +# +# golang:1.27.3-trixie, 2026-11-02 +FROM golang@sha256:c65ecfe613b539ac74c62a4c3b2c27e8f8a6076384c8f27e8a3c0f3be17cc385 + +COPY --from=lint /src/go.sum /dev/null +COPY --from=test /src/go.sum /dev/null + +WORKDIR /src + +# script/bootstrap finds its pinned Go already in this image. It installs +# the Go modules and the tools for formatting Markdown. Debian is used +# here because the node it installs needs glibc. +COPY script/ script/ +COPY package.json yarn.lock go.mod go.sum ./ +RUN script/bootstrap + +COPY . . + +RUN script/build + +# The version is computed on the host and passed in, because +# .dockerignore excludes .git. +ARG VERSION=dev +LABEL org.opencontainers.image.version="${VERSION}" diff --git a/Makefile b/Makefile index 1eb937b..721bc76 100644 --- a/Makefile +++ b/Makefile @@ -1,14 +1,35 @@ -default: test +.PHONY: bootstrap setup test lint fmt fmt-check check build docker hooks + +# Makefile targets are thin shims; the implementations live in script/ +# per the scripts-to-rule-them-all pattern (see the Entrypoints section +# of README.md). + +bootstrap: + @script/bootstrap + +setup: + @script/setup test: - go test -v ./... + @script/test -run: build - DEBUG=1 ./dcfinfo +lint: + @script/lint + +fmt: + @script/fmt + +fmt-check: + @script/fmt-check + +check: + @script/check build: - go env -w CGO_ENABLED="0" - cd cmd/dcfinfo && go build -o ../../dcfinfo + @script/build -clean: - rm -f dcfinfo +docker: + @script/docker + +hooks: + @script/install-precommit diff --git a/README.md b/README.md index fdba280..5ef3834 100644 --- a/README.md +++ b/README.md @@ -1,33 +1,144 @@ # dcf -Golang reader implementation of the so-called "Design rule for Camera File -system" or DCF, aka JEITA (Japan Electronics and Information Technology -Industries Association) specification number CP-3461. +## Description -[wikipedia.org/wiki/Design_rule_for_Camera_File_system](https://en.wikipedia.org/wiki/Design_rule_for_Camera_File_system) +`dcf` is a WTFPL-licensed Go library by [@sneak](https://sneak.berlin) that +reads DCF memory cards from digital cameras. + +DCF is the "Design rule for Camera File system". It is also JEITA specification +number CP-3461. JEITA is the Japan Electronics and Information Technology +Industries Association. Wikipedia describes it at +[Design rule for Camera File system](https://en.wikipedia.org/wiki/Design_rule_for_Camera_File_system). The DCF specification is why your digital camera puts images and videos in `DCIM` and `PRIVATE/M4ROOT` directories on the memory card. -# status +`dcf` is incomplete and under development. It does not work yet. -incomplete, under development, does not work yet +## Getting Started -# why +Add the library to your Go module: -I wanted to copy images off my memory cards for processing and like +```sh +go get sneak.berlin/go/dcf +``` + +Then list the DCF stores on the mounted filesystems: + +```go +package main + +import ( + "fmt" + "log" + + "sneak.berlin/go/dcf/pkg/dcf" +) + +func main() { + stores, err := dcf.GetDCFStores(0) + if err != nil { + log.Fatal(err) + } + + for _, store := range *stores { + fmt.Println(store.String()) + } +} +``` + +To work on the library itself, you need docker and git: + +```sh +git clone git@git.eeqj.de:sneak/dcf.git +cd dcf +script/setup +make check +``` + +## Entrypoints + +This repo adheres to the +[Scripts to Rule Them All](https://github.com/github/scripts-to-rule-them-all) +standard. Each entrypoint is a POSIX sh script in `script/`. Use these instead +of running the underlying tools directly. + +- `script/bootstrap` (`make bootstrap`) installs the tools the host needs. It + does not install docker or the linter. It installs: + - `make` and git + - Go into `~/.local/go`, at the version in the `Dockerfile`, unless the `go` + on the `PATH` is already that version + - the Go modules + - node and yarn + - `prettier` +- `script/setup` (`make setup`) runs `script/bootstrap` and then installs the + git pre-commit hook. +- `script/test` (`make test`) runs the tests by building the `test` stage of the + `Dockerfile`. +- `script/lint` (`make lint`) checks the Go code by building the `lint` stage of + the `Dockerfile`. +- `script/fmt` (`make fmt`) rewrites the Go code with `go fmt` and the Markdown + files with `prettier`. +- `script/fmt-check` (`make fmt-check`) checks the same formatting and changes + nothing. +- `script/check` (`make check`) runs the tests, then the linter, then the + formatting check. +- `script/build` (`make build`) compiles every package. It needs the Go that + `script/bootstrap` installs. +- `script/docker` (`make docker`) builds the development image, tagged `dcf`. +- `script/cibuild` runs the whole CI build. The Gitea workflow runs it on every + push. +- `script/precommit` is what the git pre-commit hook runs. It runs + `script/check`. +- `script/install-precommit` (`make hooks`) installs the git pre-commit hook. +- `script/projectname` prints the project name for the other scripts. + +The development image cannot be built unless the `lint` and `test` stages pass. +Run `docker run --rm -it dcf` for a shell inside it, with Go and `make`. + +## Rationale + +I wanted to copy images off my memory cards for processing. I also like overengineering and reusable code. -# bugs +## Design -does not yet handle "dcf file groups" where multiple objects share the same -index number. if you need this, send me a link to a zip or tar of a memory -card that uses it and i'll see what i can do. +All the code is in the `pkg/dcf` package. The repo has no commands. -# author +- `dcf.go` holds the public API. `GetDCFStores` finds the DCF stores and lists + the files on them. +- `helpers.go` finds the mount points that hold a DCF store. -sneak <[sneak@sneak.berlin](mailto:sneak@sneak.berlin)> +The main types are: -# license +- `Store` is one DCF store on one mounted filesystem. +- `Object` is one file in a store. +- `Image` and `Video` embed `Object`. The file extension decides which one a + file becomes. -WTFPL +A mounted filesystem is a DCF store when its root holds a `DCIM` or `M4ROOT` +directory. `dcf` never mounts a filesystem. + +## TODO + +- Handle DCF file groups, where multiple objects share the same index number. + `dcf` does not handle these yet. If you need this, send me a link to a zip or + tar of a memory card that uses them, and I'll see what I can do. +- Parse the DCF directory and file names, rather than listing every media file + on the filesystem. +- Look for `PRIVATE/M4ROOT` rather than `M4ROOT` when detecting a DCF store. +- Return exactly the number of stores asked for. `GetDCFStores` returns one too + few when asked for two or more. +- Read stores through `fs.FS`, with a `context.Context` to cancel a walk. +- Return a plain slice from `GetDCFStores`, not a pointer to one. +- Test the walk of a store and the detection of mount points. They only run + against real mounted media today. +- Copy files off a card, and verify each copy against its SHA256 hash. + +## License + +WTFPL. See [LICENSE](LICENSE). + +## Author + +[@sneak](https://sneak.berlin) <[sneak@sneak.berlin](mailto:sneak@sneak.berlin)> diff --git a/REPO_POLICIES.md b/REPO_POLICIES.md new file mode 100644 index 0000000..2256291 --- /dev/null +++ b/REPO_POLICIES.md @@ -0,0 +1,603 @@ +--- +title: Repository Policies +last_modified: 2026-09-08 +--- + +This document covers repository structure, tooling, and workflow standards. Code +style conventions are in separate documents: + +- [Code Styleguide](https://git.eeqj.de/sneak/prompts/raw/branch/main/prompts/CODE_STYLEGUIDE.md) + (general, bash, Docker) +- [Go](https://git.eeqj.de/sneak/prompts/raw/branch/main/prompts/CODE_STYLEGUIDE_GO.md) +- [JavaScript](https://git.eeqj.de/sneak/prompts/raw/branch/main/prompts/CODE_STYLEGUIDE_JS.md) +- [Python](https://git.eeqj.de/sneak/prompts/raw/branch/main/prompts/CODE_STYLEGUIDE_PYTHON.md) +- [Go HTTP Server Conventions](https://git.eeqj.de/sneak/prompts/raw/branch/main/prompts/GO_HTTP_SERVER_CONVENTIONS.md) + +--- + +- Cross-project documentation (such as this file) must include + `last_modified: YYYY-MM-DD` in the YAML front matter so it can be kept in sync + with the authoritative source as policies evolve. + +- **ALL external references must be pinned by cryptographic hash.** This + includes Docker base images, Go modules, npm packages, GitHub Actions, and + anything else fetched from a remote source. Version tags (`@v4`, `@latest`, + `:3.21`, etc.) are server-mutable and therefore remote code execution + vulnerabilities. The ONLY acceptable way to reference an external dependency + is by its content hash (Docker `@sha256:...`, Go module hash in `go.sum`, npm + integrity hash in lockfile, GitHub Actions `@`). No exceptions. + This also means never `curl | bash` to install tools like pyenv, nvm, rustup, + etc. Instead, download a specific release archive from GitHub, verify its hash + (hardcoded in the Dockerfile or script), and only then install. Unverified + install scripts are arbitrary remote code execution. This is the single most + important rule in this document. Double-check every external reference in + every file before committing. There are zero exceptions to this rule. + +- Every repo with software must have a root `Makefile` with these targets: + `make bootstrap`, `make setup`, `make test`, `make lint`, `make fmt` (writes), + `make fmt-check` (read-only), `make check` (runs `test`, `lint`, `fmt-check`), + `make docker`, and `make hooks` (installs pre-commit hook). A model Makefile + is at `https://git.eeqj.de/sneak/prompts/raw/branch/main/Makefile`. + +- Repos follow the + [Scripts to Rule Them All](https://github.com/github/scripts-to-rule-them-all) + pattern: the implementation of each Makefile target lives in an executable + script in `script/` (`script/bootstrap`, `script/setup`, `script/test`, + `script/lint`, `script/fmt`, `script/fmt-check`, `script/check`, + `script/docker`), and the Makefile targets are thin shims that call them. The + scripts must be POSIX sh (`#!/bin/sh`, `set -eu`, no bashisms) so they run in + minimal containers (e.g. alpine images have no bash); locate the repo root + with `$(cd "$(dirname "$0")/.." && pwd -P)` and `cd` there before acting. From + the standard's canonical set we use `bootstrap`, `setup` (make the repo ready + for development after a fresh clone: runs `bootstrap`, then + `install-precommit`, plus any repo-specific initialization), `test`, and + `cibuild`. `script/bootstrap` installs all dependencies idempotently and + assumes nothing is present: base tools come from nix, apt, brew, or apk + (detected in that order; apt runs noninteractive). For node it uses the + installed node if present; otherwise it installs a PINNED node version via + nvm, first installing nvm itself if missing — from a hash-verified GitHub + release archive (never `curl | sh`), with bash installed as an explicit + prerequisite since nvm requires bash. yarn is then pinned via + `corepack prepare yarn@ --activate`. Never install "latest" or "lts"; + always exact versions. `script/cibuild` runs the CI build: it changes to the + repo root, runs `script/bootstrap`, runs `script/check`, and builds the image + with the version; the Gitea workflow calls it. **`script/cibuild` runs + `script/bootstrap` first**, because the workflow checks out the repo and runs + nothing else, while `script/fmt-check` runs the formatter on the host: on a + pristine checkout with nothing installed the run dies there, after the + containerised gates have passed. **The bootstrap alone is not enough**: + `script/bootstrap` installs node and yarn under nvm and leaves neither on the + `PATH` of the shell that called it, so a bare `yarn` still exits 127. The host + entrypoints that need yarn — `script/fmt` and `script/fmt-check` — therefore + source nvm for the pinned node version before invoking it, exactly as + `script/bootstrap`'s own install step does. A runner carrying nothing but + docker and git then gets through `script/check`. Four further scripts are our + own extensions to the standard: `script/check` runs `script/test`, + `script/lint` and `script/fmt-check`; `script/precommit` is what the git + pre-commit hook runs, and it calls `script/check`; `script/install-precommit` + installs the git pre-commit hook (the `make hooks` target shims to it); and + `script/projectname` (literally that filename) simply outputs the project's + name. Scripts that need the name call `script/projectname` — e.g. + `script/docker` assembles its image tag from it — so those scripts stay + byte-identical across all repos. Repo-type-specific pre-commit extras (e.g. + `go mod tidy` verification in Go repos) belong in `script/precommit`, not in + the hook itself. Model scripts are at + `https://git.eeqj.de/sneak/prompts/raw/branch/main/script/`. The README + must document the provided scripts in an **Entrypoints** section (see the + README requirements below). + +- Always use Makefile targets (`make fmt`, `make test`, `make lint`, etc.) + instead of invoking the underlying tools directly. The Makefile is the single + source of truth for how these operations are run. + +- The Makefile is authoritative documentation for how the repo is used. Beyond + the required targets above, it should have targets for every common operation: + running a local development server (`make run`, `make dev`), re-initializing + or migrating the database (`make db-reset`, `make migrate`), building + artifacts (`make build`), generating code, seeding data, or anything else a + developer would do regularly. If someone checks out the repo and types + `make`, they should see every meaningful operation available. A new + contributor should be able to understand the entire development workflow by + reading the Makefile. + +- Every repo should have a `Dockerfile`, and it carries the repo's gates: a + `lint` phase and a `test` phase, with the final stage depending on both so the + image cannot be built unless they pass. For non-server repos the final stage + brings up a development environment; for server repos it is the runtime image. + Dockerfiles install development prerequisites by running `script/bootstrap` + rather than duplicating installs inline; COPY `script/` and the dependency + manifests (`package.json` + `yarn.lock`, `go.mod` + `go.sum`, etc.) before + running it. + +- **Linting and testing run in Docker, as phases of the `Dockerfile`.** There is + no separate lint file. `script/lint` and `script/test` each build one phase + and nothing else: + + ```sh + docker build --no-cache --target lint -t "$(script/projectname)-lint" . + docker build --no-cache --target test -t "$(script/projectname)-test" . + ``` + + **A stage that is not the last one in the file is built only when the final + stage's chain depends on it, or when `--target` names it.** That is why the + two gates are always invoked by name here, and why the final stage carries a + `COPY --from=` of a harmless file from each of them: without that edge a + plain `docker build .` builds the last stage alone and exits 0 having linted + and tested nothing. + + **Every `docker build` in `script/` is tagged**, here and in + `script/cibuild` and `script/docker`. An untagged build leaves a dangling + image behind on every invocation, on every developer host and every CI + runner; a tagged one replaces the previous image. + + Inside a phase the tool is invoked directly — `golangci-lint`, `go test`, + `eslint`, `prettier` — never through `make lint` or `script/test`, which are + themselves a `docker build` and would recurse into a daemon that does not + exist in a build step. Formatting is the exception and stays on the host: + `script/fmt` writes the working tree, and `script/fmt-check` is its + read-only twin. + + **No lint verdict may come from a host invocation of the linter.** On a + shared host golangci-lint reads a result cache keyed on file content rather + than location, so a second checkout of the same content is served the first + one's findings, and a host-global lock in `$TMPDIR` makes concurrent runs + exit non-zero with `parallel golangci-lint is running` — a status a caller + cannot tell from real findings. Both have produced wrong verdicts in this + org, in both directions. A container has its own cache, its own `TMPDIR` and + a digest-pinned binary, so neither is reachable. + +- **Any build that runs checks is built with `--no-cache`.** Docker invalidates + a `COPY` layer only when the copied content changes, so on an unchanged tree + the check `RUN` is served from cache, nothing executes, and the build still + exits 0. Every `docker build` in `script/` therefore passes `--no-cache`: + `script/lint`, `script/test`, `script/cibuild` and `script/docker` are the + four, and there is no fifth — `script/check` runs the two gate phases and + `script/fmt-check`, and builds no image of its own. A bare `docker build .` is + not evidence that anything ran: a sub-second build reporting success is a + cache hit, not a result. Never invalidate by pruning — `docker builder prune` + and friends destroy a build cache shared with every other build on the host. + +- **The gate phases are separate stages, and the build stage depends on both.** + The lint phase is based on the `golangci/golangci-lint` image (pinned by + hash), so lint failures surface in seconds rather than after a full compile, + and the test phase is based on the Go image. The canonical Go repo + `Dockerfile`: + + ```dockerfile + # Lint phase + # golangci/golangci-lint:v2.x.x, YYYY-MM-DD + FROM golangci/golangci-lint@sha256:... AS lint + WORKDIR /src + COPY go.mod go.sum ./ + RUN go mod download + COPY . . + RUN golangci-lint run --config .golangci.yml ./... + + # Test phase + # golang:1.x-alpine, YYYY-MM-DD + FROM golang@sha256:... AS test + WORKDIR /src + COPY go.mod go.sum ./ + RUN go mod download + COPY . . + RUN go test -timeout 90s -race -cover ./... || \ + { echo "--- Rerunning with -v for details ---"; \ + go test -timeout 90s -race -v ./...; exit 1; } + + # Build stage. Nothing is wanted from either phase above; the copies + # are what make BuildKit build them first, so this stage cannot run + # unless lint and test passed. + # golang:1.x-alpine, YYYY-MM-DD + FROM golang@sha256:... AS builder + COPY --from=lint /src/go.sum /dev/null + COPY --from=test /src/go.sum /dev/null + WORKDIR /src + COPY go.mod go.sum ./ + RUN go mod download + COPY . . + + ARG VERSION=dev + RUN CGO_ENABLED=0 go build -trimpath \ + -ldflags="-s -w -X main.Version=${VERSION}" \ + -o /app ./cmd/app/ + + # Runtime stage, and the last one + FROM alpine@sha256:... + COPY --from=builder /app /usr/local/bin/app + ENTRYPOINT ["app"] + ``` + + Key points: + - The lint phase uses the `golangci/golangci-lint` image directly (it has + both Go and the linter), so nothing needs installing. + - `COPY --from= /src/go.sum /dev/null` is a no-op copy whose only + purpose is the ordering edge. BuildKit runs stages in parallel by default, + and a stage nothing depends on is not built at all, so without these two + lines a red gate would not fail the build. + - Keep the runtime stage last, and if you add a stage after it, give it the + same two copies. A plain `docker build .` builds the last stage's chain + and nothing else. + - If the project uses `//go:embed` directives that reference build artifacts + (e.g. a web frontend compiled in a separate stage), the lint phase must + create placeholder files so the embed directives resolve. Example: + `RUN mkdir -p web/dist && touch web/dist/index.html web/dist/style.css`. + - If the project requires CGO or system libraries for linting (e.g. + `vips-dev`), install them in the lint phase with `apk add`. + - `ARG VERSION=dev` is declared in the stage that compiles and supplied by + `script/docker` and `script/cibuild`; no stage may call `git describe`. + +- Every repo should have a Gitea Actions workflow (`.gitea/workflows/`) that + runs `script/cibuild` on push, and checks out the repo as its only other step. + That script bootstraps, runs the gate phases, and then builds the image, so a + successful run means every check passed; a bare `docker build .` does not + carry the same guarantee, because its gate phases may come from the cache. The + image build is uncached and so runs the gate phases a second time. That is the + price of the rule above, and it is worth paying: the image that ships is built + from a run of its own gates rather than from a cache entry. + +- Use platform-standard formatters: `black` for Python, `prettier` for + JS/CSS/Markdown/HTML, `go fmt` for Go. Always use default configuration with + two exceptions: four-space indents (except Go), and `proseWrap: always` for + Markdown (hard-wrap at 80 columns). Documentation and writing repos (Markdown, + HTML, CSS) should also have `.prettierrc` and `.prettierignore`. + +- Pre-commit hook: runs `script/precommit`, which calls `script/check`. If local + testing is not possible in the repo, `script/precommit` may skip `script/test` + and run only `script/lint` and `script/fmt-check`. The hook is installed by + `script/install-precommit`; the Makefile must provide a `make hooks` target + that shims to it. + +- All repos with software must have tests that run via the platform-standard + test framework (`go test`, `pytest`, `jest`/`vitest`, etc.). If no meaningful + tests exist yet, add the most minimal test possible — e.g. importing the + module under test to verify it compiles/parses. There is no excuse for + `make test` to be a no-op. + +- `make test` must complete in under 60 seconds. That is the hard cap, and a + suite that exceeds it fails. Under 20 seconds is the target. A suite between + 20 and 60 seconds is still green, but the overage must be filed as an + improvement bug against that repo. Add a 90-second timeout to the test + invocation (`go test -timeout 90s`). The backstop deliberately sits above the + hard cap so that it catches a genuinely hung test rather than a merely slow + one. + +- **The test command should use the conditional verbose rerun pattern.** Run + tests without `-v` (verbose) first. If tests fail, automatically rerun with + `-v` to show full output. This keeps CI logs and `docker build` output clean + on success (just package/suite summaries) while providing full diagnostic + detail on failure (every test case, every assertion). The command lives in the + `test` phase of the `Dockerfile`, since `script/test` builds that phase; the + Makefile form below is the same pattern for any repo-local invocation: + + ```makefile + test: + @ || \ + { echo "--- Rerunning with -v for details ---"; \ + ; exit 1; } + ``` + + Go example: + + ```makefile + test: + @go test -count=1 -timeout 90s -race -cover ./... || \ + { echo "--- Rerunning with -v for details ---"; \ + go test -count=1 -timeout 90s -race -v ./...; exit 1; } + ``` + + `-count=1` is required on both invocations: it defeats Go's test _result_ + cache, so the target cannot report a pass it did not earn, and the rerun + reproduces a failure instead of replaying it. It leaves the build cache + alone, so it costs the runtime of the suite and no recompilation. + + Note that this is a second, independent cache, stacked below the Docker + layer cache that [issue #26](https://git.eeqj.de/sneak/prompts/issues/26) + addresses. `CHECK_EPOCH` guarantees the `RUN make test` _step_ re-executes; + it does not guarantee `go test` inside that step does any work, because the + `GOCACHE` baked into earlier image layers survives into the re-executed + step. They are two separate defects requiring two separate fixes, and a fix + for one must not be recorded as covering the other. + + Python example: + + ```makefile + test: + @python -m pytest || \ + { echo "--- Rerunning with -v for details ---"; \ + python -m pytest -v; exit 1; } + ``` + + The `exit 1` ensures the target always fails after a rerun — the first run + already proved the tests are broken, so the build must not pass even if a + flaky test happens to succeed on the second attempt. The rerun exists solely + for diagnostic output. + +- Docker builds must complete in under 5 minutes. + +- `make check` must not modify any files in the repo. Tests may use temporary + directories. + +- `main` must always pass `make check`, no exceptions. + +- Never commit secrets. `.env` files, credentials, API keys, and private keys + must be in `.gitignore`. No exceptions. + +- `.gitignore` should be comprehensive from the start: OS files (`.DS_Store`), + editor files (`.swp`, `*~`), in-repo agent scratch directories (`.claude/`), + language build artifacts, and `node_modules/`. Fetch the standard `.gitignore` + from `https://git.eeqj.de/sneak/prompts/raw/branch/main/.gitignore` when + setting up a new repo. These patterns are written to `.gitignore`'s own + semantics, in which an unanchored pattern already matches at every depth; they + are not a `.dockerignore` and must not be transplanted into one unmodified. + +- **`.dockerignore` does not use `.gitignore` semantics, and copying patterns + across unmodified leaves secrets in the build context.** Docker matches with + `moby/patternmatcher`: `filepath.Match` semantics plus a `**` extension, so + `*` does not cross `/` and a pattern without a leading `**/` is anchored at + the build-context root. A `.dockerignore` listing `.env`, `*.pem` and `*.key` + therefore excludes only the copies at the repository root, while `config/.env` + and `certs/server.key` still reach the context and can land in an image layer + — which is more dangerous than a short file with no secret patterns at all, + because it reads as solved and stops anyone looking. Give every + depth-independent pattern the `**/` prefix and leave only genuinely + root-anchored entries unprefixed: `.git`, and the repo's own host-built + binary, written `/myapp` and never `**/myapp`, which would also match + `cmd/myapp/` and delete the package directory from the context. Matching is + case-sensitive, and an ALL-CAPS twin per pattern still misses `Server.Key`, so + secret names use character ranges — `**/*.[kK][eE][yY]`, `**/*.[pP][eE][mM]`, + and likewise for `.envrc` and the extensionless SSH keys. Where such a pattern + also catches something the build needs, re-include it with a negation + (`!docs/example.env`); deleting the pattern reopens the exposure for every + other file it covers. Fetch the standard `.dockerignore` from + `https://git.eeqj.de/sneak/prompts/raw/branch/main/.dockerignore` and extend + it with the repo's own artifacts. + +- **In-repo agent scratch belongs in both files, written to each file's own + semantics.** `.claude/` holds one worktree per in-flight agent — an entire + additional checkout of the repo — so under `COPY . .` the build context + inflates by a multiple of the repo and another session's unreviewed work can + be copied into an image layer. In `.gitignore` the entry is `.claude/`, + unanchored. In `.dockerignore` it is `.claude`, anchored and with **no** `**/` + prefix, because the prefixed form would also delete any nested directory of + that name from the build. Anchoring carries a known gap that the canonical + `.dockerignore` states in its own comment, since consuming repos receive the + file and not the tracker: the directory is created in the agent's working + directory, so a repo running agents in subdirectories still ships + `services/api/.claude/` and must add its own anchored entry there. + +- **Excluding `.git` means `git describe` cannot run inside any build stage, and + it fails quietly there.** In a build stage there is no repository, so + `git describe` writes nothing to stdout, `-X main.Version=` comes out empty, + the binary reports no version at all, and the build still exits 0. Compute the + version on the host and thread it in as a build arg. `script/docker` and + `script/cibuild` do this, byte-identically across repos: + + ```sh + # Own line: a failing command substitution inside an argument does not + # trip `set -e`, so the inline form degrades to an empty constant. + version="$(git describe --tags --always --dirty 2>/dev/null || true)" + [ -n "$version" ] || version="unknown" + docker build --no-cache \ + --build-arg VERSION="$version" \ + -t "$(script/projectname)" . + ``` + + `--always` makes an untagged repo yield an abbreviated commit hash rather + than failing, and the `[ -n "$version" ]` line is the single place the + fallback is applied — a live check that fires on a build from an export with + no `.git` and on a repository with no commits yet. Do not fold it into the + substitution as `|| echo unknown`, which makes the guard unreachable. The + Dockerfile's side is `ARG VERSION=dev` in the stage that compiles, declared + there because `ARG` is stage-scoped; passing `VERSION` to a repo whose + Dockerfile declares no such `ARG` is ignored and costs nothing, which is why + the scripts stay byte-identical. One consequence for CI: the standard + checkout action clones shallow and fetches no tags, so a repo that embeds a + tag-derived version must set `fetch-depth: 0` on its checkout step. + +- **Verify `.dockerignore` by enumerating the image, not by reading the + patterns.** Plant files at the root _and_ at least two directories deep, build + a probe image that does `COPY . .`, and list what actually landed + (`docker run --rm --entrypoint find IMAGE /app`). The `transferring context` + size is not a substitute: a nested secret is a few bytes, and BuildKit + transfers only the delta from the previous build. + +- **No build artifacts in version control.** Code-derived data (compiled + bundles, minified output, generated assets) must never be committed to the + repository if it can be avoided. The build process (e.g. Dockerfile, Makefile) + should generate these at build time. Notable exception: Go protobuf generated + files (`.pb.go`) ARE committed because repos need to work with `go get`, which + downloads code but does not execute code generation. + +- Never use `git add -A` or `git add .`. Always stage files explicitly by name. + +- Never force-push to `main`. + +- Make all changes on a feature branch. You can do whatever you want on a + feature branch. + +- `.golangci.yml` is standardized. The vendored copy in a consuming repo must + _NEVER_ be modified by an agent: fetch it from + `https://git.eeqj.de/sneak/prompts/raw/branch/main/.golangci.yml` and keep it + byte-identical, so that no repo can quietly loosen its own linting. Linter + configuration changes are made to the canonical copy in the `prompts` repo and + reach consuming repos by re-vendoring; an agent may open a PR against + canonical, which only the user merges. One list is exempt from byte-identity, + because it cannot be written once for every repo: the `deny` list of the + `test-support` depguard rule, where a repo names its own test-support packages + by full import path. A repo adds entries there and changes nothing else, and a + re-vendor carries its entries forward. The canonical golangci-lint version is + v2.12.2 (released 2026-05-06), pinned as the digest of the lint phase's base + image + (`golangci/golangci-lint@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240`, + which reports `2.12.2 built with go1.26.2 from c0d3ddc9`). That digest is the + only pin, since no repo installs golangci-lint on the host: bumping the + version means changing it and nothing else. + +- **`script/bootstrap` installs a pinned tool by comparing versions, never by + testing presence.** An `if ! command -v ; then install; fi` guard tests + `PATH` only, so on an already-provisioned machine the pin is inert and a + version bump is a silent no-op — while the Dockerfile, installing into a clean + image, gets the pinned version, so a local `make check` and `make docker` can + disagree about what the tool even is. The canonical form: + - compares the installed version against the pin over the **whole** version + token; a parser that stops at the first `-` reports `2.12.2` for a host + running `2.12.2-rc1` and skips the install; + - treats absent, non-zero, empty or unrecognised `--version` output as a + mismatch, so the failure direction is a redundant install and never a + skipped one; + - after installing, re-resolves the binary the way callers do — `hash -r`, + then through `PATH`, not through the directory the installer wrote to — + and fails naming the resolved path, since an install that a shadowing + binary hides succeeds while changing nothing any caller sees; + - is actually called, and prints the version on both success paths: a + function defined and never invoked has the same exit status and the same + empty output as one that worked. + + Keep it POSIX sh: no arrays, no `[[`, no `grep -P`. + +- When pinning images or packages by hash, add a comment above the reference + with the version and date (YYYY-MM-DD). + +- Use `yarn`, not `npm`. + +- Write all dates as YYYY-MM-DD (ISO 8601). + +- Simple projects should be configured with environment variables. + +- Dockerized web services listen on port 8080 by default, overridable with + `PORT`. + +- **HTTP/web services must be hardened for production internet exposure before + tagging 1.0.** This means full compliance with security best practices + including, without limitation, all of the following: + - **Security headers** on every response: + - `Strict-Transport-Security` (HSTS) with `max-age` of at least one year + and `includeSubDomains`. + - `Content-Security-Policy` (CSP) with a restrictive default policy + (`default-src 'self'` as a baseline, tightened per-resource as + needed). Never use `unsafe-inline` or `unsafe-eval` unless + unavoidable, and document the reason. + - `X-Frame-Options: DENY` (or `SAMEORIGIN` if framing is required). + Prefer the `frame-ancestors` CSP directive as the primary control. + - `X-Content-Type-Options: nosniff`. + - `Referrer-Policy: strict-origin-when-cross-origin` (or stricter). + - `Permissions-Policy` restricting access to browser features the + application does not use (camera, microphone, geolocation, etc.). + - **Request and response limits:** + - Maximum request body size enforced on all endpoints (e.g. Go + `http.MaxBytesReader`). Choose a sane default per-route; never accept + unbounded input. + - Maximum response body size where applicable (e.g. paginated APIs). + - `ReadTimeout` and `ReadHeaderTimeout` on the `http.Server` to defend + against slowloris attacks. + - `WriteTimeout` on the `http.Server`. + - `IdleTimeout` on the `http.Server`. + - Per-handler execution time limits via `context.WithTimeout` or + chi/stdlib `middleware.Timeout`. + - **Authentication and session security:** + - Rate limiting on password-based authentication endpoints. API keys are + high-entropy and not susceptible to brute force, so they are exempt. + - CSRF tokens on all state-mutating HTML forms. API endpoints + authenticated via `Authorization` header (Bearer token, API key) are + exempt because the browser does not attach these automatically. + - Passwords stored using bcrypt, scrypt, or argon2 — never plain-text, + MD5, or SHA. + - Session cookies set with `HttpOnly`, `Secure`, and `SameSite=Lax` (or + `Strict`) attributes. + - **Reverse proxy awareness:** + - True client IP detection when behind a reverse proxy + (`X-Forwarded-For`, `X-Real-IP`). The application must accept + forwarded headers only from a configured set of trusted proxy + addresses — never trust `X-Forwarded-For` unconditionally. + - **CORS:** + - Authenticated endpoints must restrict `Access-Control-Allow-Origin` to + an explicit allowlist of known origins. Wildcard (`*`) is acceptable + only for public, unauthenticated read-only APIs. + - **Error handling:** + - Internal errors must never leak stack traces, SQL queries, file paths, + or other implementation details to the client. Return generic error + messages in production; detailed errors only when `DEBUG` is enabled. + - **TLS:** + - Services never terminate TLS directly. They are always deployed behind + a TLS-terminating reverse proxy. The service itself listens on plain + HTTP. However, HSTS headers and `Secure` cookie flags must still be + set by the application so that the browser enforces HTTPS end-to-end. + + This list is non-exhaustive. Apply defense-in-depth: if a standard security + hardening measure exists for HTTP services and is not listed here, it is + still expected. When in doubt, harden. + +- `README.md` is the primary documentation. Required sections: + - **Description**: First line must include the project name, purpose, + category (web server, SPA, CLI tool, etc.), license, and author. Example: + "µPaaS is an MIT-licensed Go web application by @sneak that receives + git-frontend webhooks and deploys applications via Docker in realtime." + - **Getting Started**: Copy-pasteable install/usage code block. + - **Entrypoints**: Opens by stating that the repo adheres to the + [Scripts to Rule Them All](https://github.com/github/scripts-to-rule-them-all) + standard (with that link), then documents each provided `script/` + entrypoint and its purpose. + - **Rationale**: Why does this exist? + - **Design**: How is the program structured? + - **TODO**: Update meticulously, even between commits. When planning, put + the todo list in the README so a new agent can pick up where the last one + left off. + - **License**: MIT, GPL, or WTFPL. Ask the user for new projects. Include a + `LICENSE` file in the repo root and a License section in the README. + - **Author**: [@sneak](https://sneak.berlin). + +- First commit of a new repo should contain only `README.md`. + +- Go module root: `sneak.berlin/go/`. Always run `go mod tidy` before + committing. + +- Use SemVer. + +- Database migrations live in `internal/db/migrations/` and must be embedded in + the binary. + - `000_migration.sql` — contains ONLY the creation of the migrations + tracking table itself. Nothing else. + - `001_schema.sql` — the full application schema. + - **Pre-1.0.0:** never add additional migration files (002, 003, etc.). + There is no installed base to migrate. Edit `001_schema.sql` directly. + - **Post-1.0.0:** add new numbered migration files for each schema change. + Never edit existing migrations after release. + +- All repos should have an `.editorconfig` enforcing the project's indentation + settings. + +- Avoid putting files in the repo root unless necessary. Root should contain + only project-level config files (`README.md`, `Makefile`, `Dockerfile`, + `LICENSE`, `.gitignore`, `.editorconfig`, `REPO_POLICIES.md`, and + language-specific config). Everything else goes in a subdirectory. Canonical + subdirectory names: + - `bin/` — executable scripts and tools + - `cmd/` — Go command entrypoints; thin only: one `main.go` per binary whose + body is a single call into `internal/` or `pkg/`, no project logic in + `cmd/` + - `configs/` — configuration templates and examples + - `deploy/` — deployment manifests (k8s, compose, terraform) + - `docs/` — documentation and markdown (README.md stays in root) + - `internal/` — Go internal packages + - `internal/db/migrations/` — database migrations + - `pkg/` — Go library packages + - `share/` — systemd units, data files + - `static/` — static assets (images, fonts, etc.) + - `web/` — web frontend source + +- When setting up a new repo, files from the `prompts` repo may be used as + templates. Fetch them from + `https://git.eeqj.de/sneak/prompts/raw/branch/main/`. + +- New repos must contain at minimum: + - `README.md`, `.git`, `.gitignore`, `.editorconfig` + - `LICENSE`, `REPO_POLICIES.md` (copy from the `prompts` repo) + - `Makefile` + - `script/` entrypoints (`bootstrap`, `setup`, `projectname`, `test`, + `lint`, `fmt`, `fmt-check`, `check`, `docker`, `cibuild`, `precommit`, + `install-precommit`) + - `Dockerfile`, `.dockerignore` + - `.gitea/workflows/check.yml` + - Go: `go.mod`, `go.sum`, `.golangci.yml` + - JS: `package.json`, `yarn.lock`, `.prettierrc`, `.prettierignore` + - Python: `pyproject.toml` diff --git a/go.mod b/go.mod index 706aef8..2359215 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ -module git.eeqj.de/sneak/dcf +module sneak.berlin/go/dcf -go 1.22.1 +go 1.26.2 require ( github.com/dustin/go-humanize v1.0.1 @@ -9,7 +9,6 @@ require ( require ( github.com/go-ole/go-ole v1.3.0 // indirect - github.com/lmittmann/tint v1.0.4 // indirect github.com/yusufpapurcu/wmi v1.2.4 // indirect golang.org/x/sys v0.20.0 // indirect ) diff --git a/go.sum b/go.sum index 1724945..5312366 100644 --- a/go.sum +++ b/go.sum @@ -3,8 +3,6 @@ github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+m github.com/go-ole/go-ole v1.2.6/go.mod h1:pprOEPIfldk/42T2oK7lQ4v4JSDwmV0As9GaiUsvbm0= github.com/go-ole/go-ole v1.3.0 h1:Dt6ye7+vXGIKZ7Xtk4s6/xVdGDQynvom7xCFEdWr6uE= github.com/go-ole/go-ole v1.3.0/go.mod h1:5LS6F96DhAwUc7C+1HLexzMXY1xGRSryjyPPKW6zv78= -github.com/lmittmann/tint v1.0.4 h1:LeYihpJ9hyGvE0w+K2okPTGUdVLfng1+nDNVR4vWISc= -github.com/lmittmann/tint v1.0.4/go.mod h1:HIS3gSy7qNwGCj+5oRjAutErFBl4BzdQP6cJZ0NfMwE= github.com/shirou/gopsutil v3.21.11+incompatible h1:+1+c1VGhc88SSonWP6foOcLhvnKlUeu/erjjvaPEYiI= github.com/shirou/gopsutil v3.21.11+incompatible/go.mod h1:5b4v6he4MtMOwMlS0TUMTu2PcXUg8+E1lC7eC3UO/RA= github.com/yusufpapurcu/wmi v1.2.4 h1:zFUKzehAFReQwLys1b/iSMl+JQGSCSjtVqQn9bBrPo0= diff --git a/package.json b/package.json new file mode 100644 index 0000000..dc05cde --- /dev/null +++ b/package.json @@ -0,0 +1,5 @@ +{ + "devDependencies": { + "prettier": "3.8.1" + } +} diff --git a/pkg/dcf/dcf.go b/pkg/dcf/dcf.go index cac3167..94484ee 100644 --- a/pkg/dcf/dcf.go +++ b/pkg/dcf/dcf.go @@ -1,29 +1,32 @@ +// Package dcf reads DCF cards: memory cards laid out to the "Design rule for +// Camera File system" specification (JEITA CP-3461). It finds the cards among +// the mounted filesystems and lists the images and videos on each one. package dcf import ( "crypto/sha256" + "encoding/hex" "fmt" "io" "os" "path/filepath" + "slices" "strings" "github.com/dustin/go-humanize" ) -var imageExtensions = []string{".jpg", ".jpeg", ".png", ".gif", ".bmp", ".tiff", ".raw", ".arw"} -var videoExtensions = []string{".mp4", ".mov", ".avi", ".mkv", ".wmv"} - -// DCFStore represents a DCF store, which is a directory sturcture that contains +// Store represents a DCF store, which is a directory structure that contains // images and videos in the root of a mounted filesystem. -type DCFStore struct { +type Store struct { RootDirectory string Images []*Image Videos []*Video } -// DCFObject represents a file in a DCF store. It is embedded in Image and Video types. -type DCFObject struct { +// Object represents a file in a DCF store. It is embedded in Image and Video +// types. +type Object struct { DCFStoreRoot string Path string Size int64 @@ -32,104 +35,133 @@ type DCFObject struct { // Image represents an image file on a DCF store. type Image struct { - DCFObject + Object } // Video represents a video file on a DCF store. type Video struct { - DCFObject + Object } -func (d *DCFObject) FullFilePath() string { +// FullFilePath returns the path of the file on the local filesystem. +func (d *Object) FullFilePath() string { return filepath.Join(d.DCFStoreRoot, d.Path) } // VideosCount returns the number of videos in the DCF store. -func (d *DCFStore) VideosCount() int { +func (d *Store) VideosCount() int { return len(d.Videos) } // ImagesCount returns the number of images in the DCF store. -func (d *DCFStore) ImagesCount() int { +func (d *Store) ImagesCount() int { return len(d.Images) } -func (d *DCFStore) TotalImageSize() int64 { +// TotalImageSize returns the total size in bytes of the images in the DCF +// store. +func (d *Store) TotalImageSize() int64 { totalSize := int64(0) for _, image := range d.Images { totalSize += image.Size } + return totalSize } -func (d *DCFStore) TotalVideoSize() int64 { +// TotalVideoSize returns the total size in bytes of the videos in the DCF +// store. +func (d *Store) TotalVideoSize() int64 { totalSize := int64(0) for _, video := range d.Videos { totalSize += video.Size } + return totalSize } -func (d *DCFStore) TotalSize() int64 { +// TotalSize returns the total size in bytes of the images and videos in the +// DCF store. +func (d *Store) TotalSize() int64 { return d.TotalImageSize() + d.TotalVideoSize() } -func (d *DCFStore) String() string { - return fmt.Sprintf("DCFStore{RootDirectory: %s, Images: %d, Videos: %d, TotalSize: %s}", +func (d *Store) String() string { + // A caller can set a negative Size, and uint64 would make that huge. + totalSize := uint64(max(d.TotalSize(), 0)) + + return fmt.Sprintf( + "Store{RootDirectory: %s, Images: %d, Videos: %d, TotalSize: %s}", d.RootDirectory, d.ImagesCount(), d.VideosCount(), - humanize.Bytes(uint64(d.TotalSize())), + humanize.Bytes(totalSize), ) } func (v *Video) String() string { - return fmt.Sprintf("Video{Path: %s, Size: %d, Extension: %s}", v.Path, v.Size, v.Extension) + return fmt.Sprintf( + "Video{Path: %s, Size: %d, Extension: %s}", + v.Path, v.Size, v.Extension, + ) } func (i *Image) String() string { - return fmt.Sprintf("Image{Path: %s, Size: %d, Extension: %s}", i.Path, i.Size, i.Extension) + return fmt.Sprintf( + "Image{Path: %s, Size: %d, Extension: %s}", + i.Path, i.Size, i.Extension, + ) } -// Hash() returns the SHA256 hash of the file as a hex string. -func (d *DCFObject) Hash() (string, error) { +// Hash returns the SHA256 hash of the file as a hex string. +func (d *Object) Hash() (string, error) { return pathToSHA256(d.FullFilePath()) } func pathToSHA256(path string) (string, error) { - file, err := os.Open(path) + file, err := os.Open(filepath.Clean(path)) if err != nil { return "", err } - defer file.Close() + + defer func() { _ = file.Close() }() + hash := sha256.New() - if _, err := io.Copy(hash, file); err != nil { + + _, err = io.Copy(hash, file) + if err != nil { return "", err } - return fmt.Sprintf("%x", hash.Sum(nil)), nil + + return hex.EncodeToString(hash.Sum(nil)), nil } -// GetDCFStores returns a list of DCF stores found on the system in the root of any mounted filesystems. -// the integer argument is the number of DCF stores to return. If the argument is 0, all DCF stores are returned. -// It does not mount filesystems or search outside of filesystem root directories. It does, however, -// walk the filesystems to find images and videos and populate the returned DCFStores. -func GetDCFStores(requestedCount int) (*[]DCFStore, error) { +// GetDCFStores returns a list of DCF stores found on the system in the root of +// any mounted filesystems. The integer argument is the number of DCF stores to +// return. If the argument is 0, all DCF stores are returned. It does not mount +// filesystems or search outside of filesystem root directories. It does, +// however, walk the filesystems to find images and videos and populate the +// returned Stores. +func GetDCFStores(requestedCount int) (*[]Store, error) { dcfStorePaths, err := findDCFMountPoints(requestedCount) if err != nil { return nil, err } - dcfStores := []DCFStore{} + + dcfStores := []Store{} + for _, dcfStorePath := range dcfStorePaths { - dcfStore := DCFStore{ + dcfStore := Store{ RootDirectory: dcfStorePath, Images: make([]*Image, 0), Videos: make([]*Video, 0), } - err := filepath.Walk(dcfStorePath, func(path string, info os.FileInfo, err error) error { + addFile := func(path string, info os.FileInfo, err error) error { if err != nil { return err } + if !info.IsDir() && isImageFile(path) { image := Image{} image.Path = strings.TrimPrefix(path, dcfStorePath+"/") @@ -138,6 +170,7 @@ func GetDCFStores(requestedCount int) (*[]DCFStore, error) { image.DCFStoreRoot = dcfStorePath dcfStore.Images = append(dcfStore.Images, &image) } + if !info.IsDir() && isVideoFile(path) { video := Video{} video.Path = strings.TrimPrefix(path, dcfStorePath+"/") @@ -146,32 +179,31 @@ func GetDCFStores(requestedCount int) (*[]DCFStore, error) { video.DCFStoreRoot = dcfStorePath dcfStore.Videos = append(dcfStore.Videos, &video) } + return nil - }) + } + + err := filepath.Walk(dcfStorePath, addFile) if err != nil { return nil, err } + dcfStores = append(dcfStores, dcfStore) } + return &dcfStores, nil } func isImageFile(path string) bool { - ext := strings.ToLower(filepath.Ext(path)) - for _, imageExt := range imageExtensions { - if ext == imageExt { - return true - } + imageExtensions := []string{ + ".jpg", ".jpeg", ".png", ".gif", ".bmp", ".tiff", ".raw", ".arw", } - return false + + return slices.Contains(imageExtensions, strings.ToLower(filepath.Ext(path))) } func isVideoFile(path string) bool { - ext := strings.ToLower(filepath.Ext(path)) - for _, videoExt := range videoExtensions { - if ext == videoExt { - return true - } - } - return false + videoExtensions := []string{".mp4", ".mov", ".avi", ".mkv", ".wmv"} + + return slices.Contains(videoExtensions, strings.ToLower(filepath.Ext(path))) } diff --git a/pkg/dcf/dcf_internal_test.go b/pkg/dcf/dcf_internal_test.go new file mode 100644 index 0000000..46cead7 --- /dev/null +++ b/pkg/dcf/dcf_internal_test.go @@ -0,0 +1,39 @@ +package dcf + +import "testing" + +func TestIsImageFile(t *testing.T) { + t.Parallel() + + cases := map[string]bool{ + "DCIM/100MSDCF/DSC00001.jpg": true, + "DCIM/100MSDCF/DSC00001.ARW": true, + "PRIVATE/M4ROOT/CLIP/C0001.MP4": false, + "DCIM/100MSDCF/notes.txt": false, + "DCIM/100MSDCF/noextension": false, + } + + for path, want := range cases { + if got := isImageFile(path); got != want { + t.Errorf("isImageFile(%q) = %v, want %v", path, got, want) + } + } +} + +func TestIsVideoFile(t *testing.T) { + t.Parallel() + + cases := map[string]bool{ + "PRIVATE/M4ROOT/CLIP/C0001.mp4": true, + "PRIVATE/M4ROOT/CLIP/C0001.MOV": true, + "DCIM/100MSDCF/DSC00001.JPG": false, + "DCIM/100MSDCF/notes.txt": false, + "DCIM/100MSDCF/noextension": false, + } + + for path, want := range cases { + if got := isVideoFile(path); got != want { + t.Errorf("isVideoFile(%q) = %v, want %v", path, got, want) + } + } +} diff --git a/pkg/dcf/dcf_test.go b/pkg/dcf/dcf_test.go index 1cbca2a..9ba82db 100644 --- a/pkg/dcf/dcf_test.go +++ b/pkg/dcf/dcf_test.go @@ -1,10 +1,161 @@ -package dcf +package dcf_test import ( + "os" + "path/filepath" "testing" + + "sneak.berlin/go/dcf/pkg/dcf" ) -func TestCompile(t *testing.T) { - // This is a placeholder test to ensure that the module compiles successfully. - // You can add more meaningful tests here once you start implementing your code. +const cardRoot = "/mnt/card" + +// newStore returns a store holding two images and one video of known sizes. +func newStore() *dcf.Store { + return &dcf.Store{ + RootDirectory: cardRoot, + Images: []*dcf.Image{ + {Object: dcf.Object{Size: 2000}}, + {Object: dcf.Object{Size: 500}}, + }, + Videos: []*dcf.Video{ + {Object: dcf.Object{Size: 9000}}, + }, + } +} + +func TestObjectFullFilePath(t *testing.T) { + t.Parallel() + + object := dcf.Object{ + DCFStoreRoot: cardRoot, + Path: "DCIM/100MSDCF/DSC00001.ARW", + } + + want := cardRoot + "/DCIM/100MSDCF/DSC00001.ARW" + if got := object.FullFilePath(); got != want { + t.Fatalf("FullFilePath() = %q, want %q", got, want) + } +} + +func TestStoreCounts(t *testing.T) { + t.Parallel() + + store := newStore() + + if got := store.ImagesCount(); got != 2 { + t.Errorf("ImagesCount() = %d, want 2", got) + } + + if got := store.VideosCount(); got != 1 { + t.Errorf("VideosCount() = %d, want 1", got) + } +} + +func TestStoreSizes(t *testing.T) { + t.Parallel() + + store := newStore() + + if got := store.TotalImageSize(); got != 2500 { + t.Errorf("TotalImageSize() = %d, want 2500", got) + } + + if got := store.TotalVideoSize(); got != 9000 { + t.Errorf("TotalVideoSize() = %d, want 9000", got) + } + + if got := store.TotalSize(); got != 11500 { + t.Errorf("TotalSize() = %d, want 11500", got) + } +} + +func TestStoreString(t *testing.T) { + t.Parallel() + + want := "Store{RootDirectory: /mnt/card, Images: 2, Videos: 1, " + + "TotalSize: 12 kB}" + if got := newStore().String(); got != want { + t.Fatalf("String() = %q, want %q", got, want) + } +} + +func TestStoreStringShowsNegativeTotalAsZero(t *testing.T) { + t.Parallel() + + store := dcf.Store{ + RootDirectory: cardRoot, + Images: []*dcf.Image{{Object: dcf.Object{Size: -1}}}, + } + + want := "Store{RootDirectory: /mnt/card, Images: 1, Videos: 0, " + + "TotalSize: 0 B}" + if got := store.String(); got != want { + t.Fatalf("String() = %q, want %q", got, want) + } +} + +func TestImageString(t *testing.T) { + t.Parallel() + + image := dcf.Image{Object: dcf.Object{ + Path: "DCIM/100MSDCF/DSC00001.ARW", + Size: 2000, + Extension: ".ARW", + }} + + want := "Image{Path: DCIM/100MSDCF/DSC00001.ARW, Size: 2000, Extension: .ARW}" + if got := image.String(); got != want { + t.Fatalf("String() = %q, want %q", got, want) + } +} + +func TestVideoString(t *testing.T) { + t.Parallel() + + video := dcf.Video{Object: dcf.Object{ + Path: "PRIVATE/M4ROOT/CLIP/C0001.MP4", + Size: 9000, + Extension: ".MP4", + }} + + want := "Video{Path: PRIVATE/M4ROOT/CLIP/C0001.MP4, Size: 9000, Extension: .MP4}" + if got := video.String(); got != want { + t.Fatalf("String() = %q, want %q", got, want) + } +} + +func TestObjectHashIsSHA256OfFile(t *testing.T) { + t.Parallel() + + root := t.TempDir() + + err := os.WriteFile(filepath.Join(root, "a.jpg"), []byte("abc"), 0o600) + if err != nil { + t.Fatalf("WriteFile: %v", err) + } + + object := dcf.Object{DCFStoreRoot: root, Path: "a.jpg"} + + got, err := object.Hash() + if err != nil { + t.Fatalf("Hash() returned error: %v", err) + } + + // The published SHA-256 test vector for "abc". + want := "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad" + if got != want { + t.Fatalf("Hash() = %q, want %q", got, want) + } +} + +func TestObjectHashOfMissingFileReturnsError(t *testing.T) { + t.Parallel() + + object := dcf.Object{DCFStoreRoot: t.TempDir(), Path: "absent.jpg"} + + _, err := object.Hash() + if err == nil { + t.Fatal("Hash() of a missing file returned no error") + } } diff --git a/pkg/dcf/helpers.go b/pkg/dcf/helpers.go index 1672e3a..db80710 100644 --- a/pkg/dcf/helpers.go +++ b/pkg/dcf/helpers.go @@ -5,71 +5,68 @@ import ( "log/slog" "os" "path/filepath" - "runtime" + "slices" "github.com/shirou/gopsutil/disk" ) func findAllMountPoints() ([]string, error) { mountpoints := []string{} + partitions, err := disk.Partitions(false) // physical devices only, so false if err != nil { return nil, err } + for _, partition := range partitions { - if !contains(mountpoints, partition.Mountpoint) { + if !slices.Contains(mountpoints, partition.Mountpoint) { mountpoints = append(mountpoints, partition.Mountpoint) } } + return mountpoints, nil } -func dump(x interface{}) { - _, file, line, _ := runtime.Caller(1) - slog.Debug(fmt.Sprintf("%s:%d %#v\n", file, line, x)) -} - -// findDCFMountPoints returns a list of strings of the paths of mountpoints that -// contain a DCIM or PRIVATE directory, which is how we detect DCF stores. Its -// only argument is an integer of how many mountpoints to return. If the +// findDCFMountPoints returns a list of strings of the paths of mountpoints +// that contain a DCIM or M4ROOT directory, which is how we detect DCF stores. +// Its only argument is an integer of how many mountpoints to return. If the // argument is 0, all mountpoints are returned. func findDCFMountPoints(requestedCount int) ([]string, error) { filteredMountpoints := []string{} - var erro error - mountpoints, err := findAllMountPoints() - if erro != nil { + mountpoints, err := findAllMountPoints() + if err != nil { return nil, err } + for _, mountpoint := range mountpoints { dcimPath := filepath.Join(mountpoint, "DCIM") - privatePath := filepath.Join(mountpoint, "PRIVATE") - privatePath = filepath.Join(mountpoint, "M4ROOT") + m4rootPath := filepath.Join(mountpoint, "M4ROOT") shouldKeep := false - if _, err := os.Stat(dcimPath); err == nil { + + _, err = os.Stat(dcimPath) + if err == nil { shouldKeep = true + slog.Debug(fmt.Sprintf("Found DCIM directory at %s\n", dcimPath)) } - if _, err := os.Stat(privatePath); err == nil { + + _, err = os.Stat(m4rootPath) + if err == nil { shouldKeep = true - slog.Debug(fmt.Sprintf("Found M4ROOT directory at %s\n", privatePath)) + + slog.Debug(fmt.Sprintf("Found M4ROOT directory at %s\n", m4rootPath)) } + if shouldKeep { slog.Debug(fmt.Sprintf("Keeping mountpoint %s\n", mountpoint)) + filteredMountpoints = append(filteredMountpoints, mountpoint) if (requestedCount > 0) && (len(filteredMountpoints)+1 >= requestedCount) { break } } } + return filteredMountpoints, nil } - -func contains(slice []string, str string) bool { - for _, s := range slice { - if s == str { - return true - } - } - return false -} diff --git a/script/bootstrap b/script/bootstrap new file mode 100755 index 0000000..d1a8b48 --- /dev/null +++ b/script/bootstrap @@ -0,0 +1,209 @@ +#!/bin/sh +# script/bootstrap: install all dependencies needed to build and develop +# this repo. Idempotent: every install is guarded by a check so already +# installed tools are skipped. Base tooling comes from nix, apt, brew, +# or apk (detected in that order); assumes nothing is present. Go is +# installed at the version the Dockerfile's Go image carries, from the +# official release archive, into ~/.local/go, and then the Go modules. +# Node is used directly if installed; otherwise it is installed at a +# pinned version via nvm (installing nvm itself first, from a +# hash-verified release archive, never curl | sh). +set -eu + +ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" + +# Must match the Go in the Dockerfile's golang image; the sha256 of each +# archive is in install_go. +GO_VERSION="1.27.3" + +# This script cannot change its caller's PATH, so script/fmt, +# script/fmt-check and script/build put this directory first on their +# own PATH, as is done here. +GO_DIR="$HOME/.local/go" +PATH="$GO_DIR/bin:$PATH" + +# Pinned versions, 2026-07-06 +NODE_VERSION="22.17.0" +NVM_VERSION="0.40.3" +# sha256 of https://github.com/nvm-sh/nvm/archive/refs/tags/v0.40.3.tar.gz +NVM_SHA256="5f4d6aaa04a177dc93c985e31dbc411ab6b8c6e1e21d8015dbc1372625fcd1d0" +YARN_VERSION="1.22.22" + +PKGMGR="" +SUDO="" + +detect_pkgmgr() { + [ -n "$PKGMGR" ] && return 0 + if command -v nix-env >/dev/null 2>&1; then + PKGMGR="nix" + elif command -v apt-get >/dev/null 2>&1; then + PKGMGR="apt" + elif command -v brew >/dev/null 2>&1; then + PKGMGR="brew" + elif command -v apk >/dev/null 2>&1; then + PKGMGR="apk" + else + echo "bootstrap: no supported package manager (nix, apt, brew, apk)" >&2 + exit 1 + fi + if [ "$PKGMGR" = "apt" ]; then + export DEBIAN_FRONTEND=noninteractive + if [ "$(id -u)" != "0" ]; then + SUDO="sudo" + fi + # This runs once, before the first install: a fresh host or + # runner image has no package lists yet. + $SUDO apt-get update + fi +} + +# pkg_install +pkg_install() { + detect_pkgmgr + case "$PKGMGR" in + nix) nix-env -iA "nixpkgs.$1" ;; + apt) $SUDO env DEBIAN_FRONTEND=noninteractive apt-get install -y "$2" ;; + brew) brew install "$3" ;; + apk) apk add --no-cache "$4" ;; + esac +} + +missing() { + ! command -v "$1" >/dev/null 2>&1 +} + +# verify_sha256 +verify_sha256() { + if command -v sha256sum >/dev/null 2>&1; then + actual="$(sha256sum "$1" | cut -d' ' -f1)" + else + actual="$(shasum -a 256 "$1" | cut -d' ' -f1)" + fi + if [ "$actual" != "$2" ]; then + echo "bootstrap: sha256 mismatch for $1" >&2 + echo " expected: $2" >&2 + echo " actual: $actual" >&2 + exit 1 + fi +} + +# True when the go first on PATH reports exactly GO_VERSION. No go, a go +# that fails, or any other output is a mismatch. +go_version_matches() { + out="$(go version 2>/dev/null)" || return 1 + case "$out" in + "go version go$GO_VERSION "*) return 0 ;; + *) return 1 ;; + esac +} + +install_go() { + # sha256 of the go1.27.3 archives at https://go.dev/dl/, 2026-11-02 + case "$(uname -s)-$(uname -m)" in + Linux-x86_64) + platform="linux-amd64" + sha256="65efbc80fdc1fa2cd2631cad5b481777a68e8725f34e89f50fc6ac714e379b40" + ;; + Linux-aarch64) + platform="linux-arm64" + sha256="a06e99309dbcbdca1a8ccd2282c3ee146e55930886e26b4f1193269587165f49" + ;; + Darwin-x86_64) + platform="darwin-amd64" + sha256="b6f8e931e4ab497fcde6da8ae4499a58f9d258e55c8a80824a516eae5950a750" + ;; + Darwin-arm64) + platform="darwin-arm64" + sha256="dd3908cf20959d798c1e06ebe5b72213f0505ce01373b40ce3110975a6cd99c2" + ;; + *) + echo "bootstrap: no Go archive pinned for $(uname -s) $(uname -m)" >&2 + exit 1 + ;; + esac + if missing curl; then pkg_install curl curl curl curl; fi + tmp="$(mktemp -d)" + curl -fsSL -o "$tmp/go.tar.gz" \ + "https://go.dev/dl/go${GO_VERSION}.${platform}.tar.gz" + verify_sha256 "$tmp/go.tar.gz" "$sha256" + # An archive unpacked over an older Go leaves a broken tree. + rm -rf "$GO_DIR" + mkdir -p "$GO_DIR" + tar -xzf "$tmp/go.tar.gz" -C "$GO_DIR" --strip-components=1 + rm -rf "$tmp" +} + +# nvm is a bash script; run a command in a bash with nvm loaded +nvm_sh() { + bash -c ". \"\$HOME/.nvm/nvm.sh\" && $*" +} + +ensure_nvm() { + [ -s "$HOME/.nvm/nvm.sh" ] && return 0 + # nvm prerequisites; nvm itself requires bash + if missing bash; then pkg_install bash bash bash bash; fi + if missing curl; then pkg_install curl curl curl curl; fi + if missing git; then pkg_install git git git git; fi + tmp="$(mktemp -d)" + curl -fsSL -o "$tmp/nvm.tar.gz" \ + "https://github.com/nvm-sh/nvm/archive/refs/tags/v${NVM_VERSION}.tar.gz" + verify_sha256 "$tmp/nvm.tar.gz" "$NVM_SHA256" + mkdir -p "$HOME/.nvm" + tar -xzf "$tmp/nvm.tar.gz" -C "$HOME/.nvm" --strip-components=1 + rm -rf "$tmp" +} + +ensure_node() { + if ! missing node; then return 0; fi + ensure_nvm + nvm_sh "nvm install $NODE_VERSION" +} + +ensure_yarn() { + if ! missing yarn; then return 0; fi + if ! missing corepack; then + corepack enable + corepack prepare "yarn@$YARN_VERSION" --activate + elif [ -s "$HOME/.nvm/nvm.sh" ]; then + nvm_sh "nvm use $NODE_VERSION >/dev/null && corepack enable && \ + corepack prepare yarn@$YARN_VERSION --activate" + else + npm install -g "yarn@$YARN_VERSION" + fi +} + +install_js_deps() { + if missing yarn && [ -s "$HOME/.nvm/nvm.sh" ]; then + nvm_sh "nvm use $NODE_VERSION >/dev/null && cd \"$ROOT\" && \ + yarn install --frozen-lockfile" + else + yarn install --frozen-lockfile + fi +} + +main() { + cd "$ROOT" + + if missing make; then pkg_install gnumake make make make; fi + if missing git; then pkg_install git git git git; fi + + if ! go_version_matches; then + install_go + hash -r + if ! go_version_matches; then + echo "bootstrap: $(command -v go) is not go$GO_VERSION after installing it" >&2 + exit 1 + fi + fi + go version + + go mod download + + ensure_node + ensure_yarn + install_js_deps + + echo "bootstrap complete" +} + +main "$@" diff --git a/script/build b/script/build new file mode 100755 index 0000000..20b2ba7 --- /dev/null +++ b/script/build @@ -0,0 +1,18 @@ +#!/bin/sh +# script/build: compile every package. dcf is a library, so this checks +# that it compiles and writes no binary. It needs the Go that +# script/bootstrap installs, and the final stage of the Dockerfile runs +# it too. +set -eu + +ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" + +# Where script/bootstrap installs Go; it cannot put it on our PATH. +PATH="$HOME/.local/go/bin:$PATH" + +main() { + cd "$ROOT" + CGO_ENABLED=0 go build ./... +} + +main "$@" diff --git a/script/check b/script/check new file mode 100755 index 0000000..92875f7 --- /dev/null +++ b/script/check @@ -0,0 +1,16 @@ +#!/bin/sh +# script/check: run all checks (test, lint, fmt-check). Our own +# extension to scripts-to-rule-them-all. test and lint are Docker +# phases; fmt-check is native, because a formatter writes the working +# tree. Must not modify any files. +set -eu + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" + +main() { + "$SCRIPT_DIR/test" + "$SCRIPT_DIR/lint" + "$SCRIPT_DIR/fmt-check" +} + +main "$@" diff --git a/script/cibuild b/script/cibuild new file mode 100755 index 0000000..688299f --- /dev/null +++ b/script/cibuild @@ -0,0 +1,29 @@ +#!/bin/sh +# script/cibuild: run the CI build. It bootstraps first: a CI runner +# checks out and runs this and nothing else, and script/fmt-check runs +# the formatter on the host, which a pristine checkout cannot do. +# --no-cache for the same reason as script/docker: the gate phases the +# final stage depends on are RUN steps, and a cached one is a check that +# did not run. +set -eu + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" +ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)" + +main() { + cd "$ROOT" + "$SCRIPT_DIR/bootstrap" + "$SCRIPT_DIR/check" + # Own line: a failing command substitution inside an argument does + # not trip `set -e`, so the inline form degrades silently to an + # empty constant. VERSION is computed here because .dockerignore + # excludes .git, so `git describe` in a build stage yields an empty + # version without failing. + version="$(git describe --tags --always --dirty 2>/dev/null || true)" + [ -n "$version" ] || version="unknown" + docker build --no-cache \ + --build-arg VERSION="$version" \ + -t "$("$SCRIPT_DIR/projectname")" . +} + +main "$@" diff --git a/script/docker b/script/docker new file mode 100755 index 0000000..c4688e8 --- /dev/null +++ b/script/docker @@ -0,0 +1,25 @@ +#!/bin/sh +# script/docker: build the Docker image tagged with the project name. +# Identical in all repos; the tag comes from script/projectname. +# --no-cache because the gate phases the final stage depends on are RUN +# steps, and a cached one is a check that did not run. +set -eu + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" +ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)" + +main() { + cd "$ROOT" + # Own line: a failing command substitution inside an argument does + # not trip `set -e`, so the inline form degrades silently to an + # empty constant. VERSION is computed here because .dockerignore + # excludes .git, so `git describe` in a build stage yields an empty + # version without failing. + version="$(git describe --tags --always --dirty 2>/dev/null || true)" + [ -n "$version" ] || version="unknown" + docker build --no-cache \ + --build-arg VERSION="$version" \ + -t "$("$SCRIPT_DIR/projectname")" . +} + +main "$@" diff --git a/script/fmt b/script/fmt new file mode 100755 index 0000000..aa5a8cc --- /dev/null +++ b/script/fmt @@ -0,0 +1,37 @@ +#!/bin/sh +# script/fmt: format all files (writes): the Go code with go fmt, then +# the Markdown files with prettier. +set -eu + +ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" + +# Where script/bootstrap installs Go; it cannot put it on our PATH. +PATH="$HOME/.local/go/bin:$PATH" + +# Must match the pin in script/bootstrap. +NODE_VERSION="22.17.0" + +# script/bootstrap installs node and yarn under nvm and leaves neither +# on the PATH of the shell that called it, so resolve the pinned +# toolchain here the way bootstrap's own install step does. nvm is a +# bash script, hence the subshell. +run_yarn() { + if command -v yarn >/dev/null 2>&1; then + exec yarn "$@" + fi + if [ ! -s "$HOME/.nvm/nvm.sh" ]; then + echo "fmt: no yarn; run script/bootstrap first" >&2 + exit 1 + fi + exec bash -c '. "$HOME/.nvm/nvm.sh" && nvm use "$1" >/dev/null && + shift && exec yarn "$@"' bash "$NODE_VERSION" "$@" +} + +main() { + cd "$ROOT" + # Before prettier, because run_yarn replaces this shell with yarn. + go fmt ./... + run_yarn run prettier --write '**/*.md' --tab-width 4 --prose-wrap always +} + +main "$@" diff --git a/script/fmt-check b/script/fmt-check new file mode 100755 index 0000000..8d8399d --- /dev/null +++ b/script/fmt-check @@ -0,0 +1,43 @@ +#!/bin/sh +# script/fmt-check: check formatting (read-only). Same scope as +# script/fmt, but fails instead of writing. +set -eu + +ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" + +# Where script/bootstrap installs Go; it cannot put it on our PATH. +PATH="$HOME/.local/go/bin:$PATH" + +# Must match the pin in script/bootstrap. +NODE_VERSION="22.17.0" + +# script/bootstrap installs node and yarn under nvm and leaves neither +# on the PATH of the shell that called it, so resolve the pinned +# toolchain here the way bootstrap's own install step does. nvm is a +# bash script, hence the subshell. +run_yarn() { + if command -v yarn >/dev/null 2>&1; then + exec yarn "$@" + fi + if [ ! -s "$HOME/.nvm/nvm.sh" ]; then + echo "fmt-check: no yarn; run script/bootstrap first" >&2 + exit 1 + fi + exec bash -c '. "$HOME/.nvm/nvm.sh" && nvm use "$1" >/dev/null && + shift && exec yarn "$@"' bash "$NODE_VERSION" "$@" +} + +main() { + cd "$ROOT" + # Before prettier, because run_yarn replaces this shell with yarn. On + # its own line so that set -e stops here if gofmt fails or is missing. + unformatted="$(gofmt -l .)" + if [ -n "$unformatted" ]; then + echo "fmt-check: run make fmt; gofmt would change:" >&2 + echo "$unformatted" >&2 + exit 1 + fi + run_yarn run prettier --check '**/*.md' --tab-width 4 --prose-wrap always +} + +main "$@" diff --git a/script/install-precommit b/script/install-precommit new file mode 100755 index 0000000..bef6406 --- /dev/null +++ b/script/install-precommit @@ -0,0 +1,16 @@ +#!/bin/sh +# script/install-precommit: install the git pre-commit hook that runs +# script/precommit. Our own extension to scripts-to-rule-them-all. +set -eu + +ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" + +main() { + cd "$ROOT" + hook=".git/hooks/pre-commit" + printf '#!/bin/sh\nset -e\nscript/precommit\n' > .git/hooks/pre-commit + chmod +x .git/hooks/pre-commit + echo "pre-commit hook installed: runs script/precommit" +} + +main "$@" diff --git a/script/lint b/script/lint new file mode 100755 index 0000000..2d8b075 --- /dev/null +++ b/script/lint @@ -0,0 +1,23 @@ +#!/bin/sh +# script/lint: run the linter. Linting is a phase of the Dockerfile and +# this builds that phase alone; the linter is never installed or run on +# a developer host, where a shared result cache and a host-global lock +# make its answer untrustworthy. +# +# The phase is not the last stage in the file, so it is built only when +# --target names it. --no-cache because a cached lint layer is a lint +# that did not run. The tag makes each build replace the previous image +# instead of leaving a dangling one behind. +set -eu + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" +ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)" + +main() { + cd "$ROOT" + docker build --no-cache \ + --target lint \ + -t "$("$SCRIPT_DIR/projectname")-lint" . +} + +main "$@" diff --git a/script/precommit b/script/precommit new file mode 100755 index 0000000..c0a7867 --- /dev/null +++ b/script/precommit @@ -0,0 +1,12 @@ +#!/bin/sh +# script/precommit: run by the git pre-commit hook; fails the commit if +# checks fail. Our own extension to scripts-to-rule-them-all. +set -eu + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" + +main() { + "$SCRIPT_DIR/check" +} + +main "$@" diff --git a/script/projectname b/script/projectname new file mode 100755 index 0000000..e77c19e --- /dev/null +++ b/script/projectname @@ -0,0 +1,12 @@ +#!/bin/sh +# script/projectname: output the name of this project. Our own +# extension to scripts-to-rule-them-all. Other scripts that need the +# name (e.g. script/docker) call this, so they can stay identical +# across all repos. +set -eu + +main() { + echo "dcf" +} + +main "$@" diff --git a/script/setup b/script/setup new file mode 100755 index 0000000..4cc5b6b --- /dev/null +++ b/script/setup @@ -0,0 +1,13 @@ +#!/bin/sh +# script/setup: set up the repo for development after a fresh clone: +# installs dependencies and the git pre-commit hook. +set -eu + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" + +main() { + "$SCRIPT_DIR/bootstrap" + "$SCRIPT_DIR/install-precommit" +} + +main "$@" diff --git a/script/test b/script/test new file mode 100755 index 0000000..cd239f2 --- /dev/null +++ b/script/test @@ -0,0 +1,19 @@ +#!/bin/sh +# script/test: run the test suite. Testing is a phase of the Dockerfile +# and this builds that phase alone, on the same terms as script/lint: +# --target because a phase that is not the last stage is built only when +# named, --no-cache because a cached test layer is a test that did not +# run, and a tag so each build replaces the previous image. +set -eu + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" +ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)" + +main() { + cd "$ROOT" + docker build --no-cache \ + --target test \ + -t "$("$SCRIPT_DIR/projectname")-test" . +} + +main "$@" diff --git a/yarn.lock b/yarn.lock new file mode 100644 index 0000000..d846639 --- /dev/null +++ b/yarn.lock @@ -0,0 +1,8 @@ +# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. +# yarn lockfile v1 + + +prettier@3.8.1: + version "3.8.1" + resolved "https://registry.yarnpkg.com/prettier/-/prettier-3.8.1.tgz#edf48977cf991558f4fcbd8a3ba6015ba2a3a173" + integrity sha512-UOnG6LftzbdaHZcKoPFtOcCKztrQ57WkHDeRD9t/PTQtmT0NHSeWWepj6pS0z/N7+08BHFDQVUrfmfMRcZwbMg== -- 2.54.0 From 445424ba3ea4ddf2b8801d82376048edd2bddaa1 Mon Sep 17 00:00:00 2001 From: clawbot Date: Mon, 2 Nov 2026 21:01:29 +0100 Subject: [PATCH 2/5] detect PRIVATE/M4ROOT and return the requested store count (closes #6) A card was detected only with M4ROOT at its root, and asking for two or more stores returned one too few. A mount point now counts when its root holds a DCIM or PRIVATE/M4ROOT directory, and the count is exact. The check and the counting are split from the partition listing so tests can hand them temporary directories. Model: opus-5-5 Co-authored-by: clawbot --- README.md | 10 ++-- pkg/dcf/helpers.go | 62 ++++++++++++----------- pkg/dcf/helpers_internal_test.go | 85 ++++++++++++++++++++++++++++++++ 3 files changed, 122 insertions(+), 35 deletions(-) create mode 100644 pkg/dcf/helpers_internal_test.go diff --git a/README.md b/README.md index 5ef3834..39b33b0 100644 --- a/README.md +++ b/README.md @@ -116,8 +116,8 @@ The main types are: - `Image` and `Video` embed `Object`. The file extension decides which one a file becomes. -A mounted filesystem is a DCF store when its root holds a `DCIM` or `M4ROOT` -directory. `dcf` never mounts a filesystem. +A mounted filesystem is a DCF store when its root holds a `DCIM` or +`PRIVATE/M4ROOT` directory. `dcf` never mounts a filesystem. ## TODO @@ -126,13 +126,9 @@ directory. `dcf` never mounts a filesystem. tar of a memory card that uses them, and I'll see what I can do. - Parse the DCF directory and file names, rather than listing every media file on the filesystem. -- Look for `PRIVATE/M4ROOT` rather than `M4ROOT` when detecting a DCF store. -- Return exactly the number of stores asked for. `GetDCFStores` returns one too - few when asked for two or more. - Read stores through `fs.FS`, with a `context.Context` to cancel a walk. - Return a plain slice from `GetDCFStores`, not a pointer to one. -- Test the walk of a store and the detection of mount points. They only run - against real mounted media today. +- Test the walk of a store. It only runs against real mounted media today. - Copy files off a card, and verify each copy against its SHA256 hash. ## License diff --git a/pkg/dcf/helpers.go b/pkg/dcf/helpers.go index db80710..5725697 100644 --- a/pkg/dcf/helpers.go +++ b/pkg/dcf/helpers.go @@ -28,45 +28,51 @@ func findAllMountPoints() ([]string, error) { } // findDCFMountPoints returns a list of strings of the paths of mountpoints -// that contain a DCIM or M4ROOT directory, which is how we detect DCF stores. -// Its only argument is an integer of how many mountpoints to return. If the -// argument is 0, all mountpoints are returned. +// that hold a DCF store. Its only argument is an integer of how many mountpoints +// to return. If the argument is 0, all mountpoints are returned. func findDCFMountPoints(requestedCount int) ([]string, error) { - filteredMountpoints := []string{} - mountpoints, err := findAllMountPoints() if err != nil { return nil, err } + return filterDCFMountPoints(mountpoints, requestedCount), nil +} + +// filterDCFMountPoints returns the first requestedCount of the mountpoints that +// hold a DCF store. If requestedCount is 0, it returns all of them. +func filterDCFMountPoints(mountpoints []string, requestedCount int) []string { + filteredMountpoints := []string{} + for _, mountpoint := range mountpoints { - dcimPath := filepath.Join(mountpoint, "DCIM") - m4rootPath := filepath.Join(mountpoint, "M4ROOT") - shouldKeep := false - - _, err = os.Stat(dcimPath) - if err == nil { - shouldKeep = true - - slog.Debug(fmt.Sprintf("Found DCIM directory at %s\n", dcimPath)) + if !isDCFMountPoint(mountpoint) { + continue } - _, err = os.Stat(m4rootPath) - if err == nil { - shouldKeep = true + slog.Debug(fmt.Sprintf("Keeping mountpoint %s\n", mountpoint)) - slog.Debug(fmt.Sprintf("Found M4ROOT directory at %s\n", m4rootPath)) - } - - if shouldKeep { - slog.Debug(fmt.Sprintf("Keeping mountpoint %s\n", mountpoint)) - - filteredMountpoints = append(filteredMountpoints, mountpoint) - if (requestedCount > 0) && (len(filteredMountpoints)+1 >= requestedCount) { - break - } + filteredMountpoints = append(filteredMountpoints, mountpoint) + if (requestedCount > 0) && (len(filteredMountpoints) >= requestedCount) { + break } } - return filteredMountpoints, nil + return filteredMountpoints +} + +// isDCFMountPoint reports whether the root of mountpoint holds a DCIM or +// PRIVATE/M4ROOT directory, which is how we detect DCF stores. +func isDCFMountPoint(mountpoint string) bool { + for _, directory := range []string{"DCIM", "PRIVATE/M4ROOT"} { + path := filepath.Join(mountpoint, directory) + + info, err := os.Stat(path) + if err == nil && info.IsDir() { + slog.Debug(fmt.Sprintf("Found %s directory at %s\n", directory, path)) + + return true + } + } + + return false } diff --git a/pkg/dcf/helpers_internal_test.go b/pkg/dcf/helpers_internal_test.go new file mode 100644 index 0000000..1d6bef9 --- /dev/null +++ b/pkg/dcf/helpers_internal_test.go @@ -0,0 +1,85 @@ +package dcf + +import ( + "os" + "path/filepath" + "slices" + "testing" +) + +// newMountPoint returns a temporary directory holding the given directories. +func newMountPoint(t *testing.T, directories ...string) string { + t.Helper() + + root := t.TempDir() + + for _, directory := range directories { + err := os.MkdirAll(filepath.Join(root, directory), 0o750) + if err != nil { + t.Fatalf("MkdirAll: %v", err) + } + } + + return root +} + +func TestIsDCFMountPoint(t *testing.T) { + t.Parallel() + + cases := map[string]bool{ + "DCIM": true, + "PRIVATE/M4ROOT": true, + "M4ROOT": false, + "PRIVATE": false, + } + + for directory, want := range cases { + got := isDCFMountPoint(newMountPoint(t, directory)) + if got != want { + t.Errorf("isDCFMountPoint with only %q = %v, want %v", + directory, got, want) + } + } +} + +func TestIsDCFMountPointIgnoresPlainFile(t *testing.T) { + t.Parallel() + + root := t.TempDir() + + err := os.WriteFile(filepath.Join(root, "DCIM"), nil, 0o600) + if err != nil { + t.Fatalf("WriteFile: %v", err) + } + + if isDCFMountPoint(root) { + t.Fatal("isDCFMountPoint counted a plain file named DCIM as a directory") + } +} + +func TestFilterDCFMountPointsReturnsRequestedCount(t *testing.T) { + t.Parallel() + + first := newMountPoint(t, "DCIM") + second := newMountPoint(t, "PRIVATE/M4ROOT") + third := newMountPoint(t, "DCIM") + + // This one holds no DCF store, so it must not use up the requested count. + mountpoints := []string{newMountPoint(t), first, second, third} + + cases := map[int][]string{ + 0: {first, second, third}, + 1: {first}, + 2: {first, second}, + 3: {first, second, third}, + 4: {first, second, third}, + } + + for requestedCount, want := range cases { + got := filterDCFMountPoints(mountpoints, requestedCount) + if !slices.Equal(got, want) { + t.Errorf("filterDCFMountPoints(%d) = %v, want %v", + requestedCount, got, want) + } + } +} -- 2.54.0 From 2c61b85a53af398b76985db66bf9574f53e65af1 Mon Sep 17 00:00:00 2001 From: clawbot Date: Tue, 3 Nov 2026 19:25:32 +0100 Subject: [PATCH 3/5] parse the DCF structure under DCIM (closes #2) A card is now read by the naming rules in CIPA DC-009-2010, not by looking for media extensions anywhere on the volume. Store.Directories holds the DCF directories and their objects, with directory and file numbers. Store.NonConforming lists everything else under DCIM, including symlinks and names fs.FS refuses. Media under PRIVATE/M4ROOT stays, with no DCF numbers. Media anywhere else is no longer part of the store. A symlink is never followed, and a read error names the card's mount point. Model: opus-5-5 Co-authored-by: clawbot --- README.md | 45 +- pkg/dcf/dcf.go | 89 ++-- pkg/dcf/dcf_internal_test.go | 56 ++- pkg/dcf/helpers.go | 11 +- pkg/dcf/helpers_internal_test.go | 15 + pkg/dcf/parse.go | 275 +++++++++++ pkg/dcf/parse_internal_test.go | 809 +++++++++++++++++++++++++++++++ 7 files changed, 1243 insertions(+), 57 deletions(-) create mode 100644 pkg/dcf/parse.go create mode 100644 pkg/dcf/parse_internal_test.go diff --git a/README.md b/README.md index 39b33b0..a0d4cc4 100644 --- a/README.md +++ b/README.md @@ -105,30 +105,57 @@ overengineering and reusable code. All the code is in the `pkg/dcf` package. The repo has no commands. -- `dcf.go` holds the public API. `GetDCFStores` finds the DCF stores and lists - the files on them. +- `dcf.go` holds the public API. `GetDCFStores` finds the DCF stores and reads + each one. - `helpers.go` finds the mount points that hold a DCF store. +- `parse.go` reads one DCF store through an `fs.FS` rooted at its mount point. The main types are: - `Store` is one DCF store on one mounted filesystem. -- `Object` is one file in a store. +- `Directory` is one DCF directory. It has a directory number, a name and the + objects inside it. +- `Object` is one file in a store. A file in a DCF directory also has a + directory number and a file number. - `Image` and `Video` embed `Object`. The file extension decides which one a - file becomes. + file becomes. A file with any other extension, such as a `THM` thumbnail, is + neither. A mounted filesystem is a DCF store when its root holds a `DCIM` or -`PRIVATE/M4ROOT` directory. `dcf` never mounts a filesystem. +`PRIVATE/M4ROOT` directory. It needs only one of them. `dcf` never mounts a +filesystem. Media anywhere else on the filesystem is not part of the store. + +`dcf` never follows a symlink. + +`dcf` applies the naming rules of the DCF specification, CIPA DC-009-2010, under +`DCIM`: + +- A DCF directory sits directly under `DCIM`. Its name is a directory number + from 100 to 999, then five characters, such as `100MSDCF`. +- A DCF file sits directly in a DCF directory. Its name is four characters, then + a file number from 0001 to 9999, then a dot and a three-character extension, + such as `DSC00001.ARW`. +- The characters allowed are `A` to `Z`, `0` to `9` and `_`. Lower-case letters + count as upper-case. +- Directories that share a directory number are not DCF directories. +- `JPG` files that share a file number in one directory are not DCF files. + +`Store.NonConforming` lists the path of everything else under `DCIM`. It +includes everything inside a directory that breaks the rules. It also includes +every symlink, whatever its name. These entries have no DCF numbers, and they +are not counted as images or videos. + +The naming rules do not apply under `PRIVATE/M4ROOT`. The images and videos +there are in `Store.Images` and `Store.Videos`, and their DCF numbers are 0. +`dcf` skips any symlink there. ## TODO - Handle DCF file groups, where multiple objects share the same index number. `dcf` does not handle these yet. If you need this, send me a link to a zip or tar of a memory card that uses them, and I'll see what I can do. -- Parse the DCF directory and file names, rather than listing every media file - on the filesystem. -- Read stores through `fs.FS`, with a `context.Context` to cancel a walk. +- Take a `context.Context` to cancel the walk of a store. - Return a plain slice from `GetDCFStores`, not a pointer to one. -- Test the walk of a store. It only runs against real mounted media today. - Copy files off a card, and verify each copy against its SHA256 hash. ## License diff --git a/pkg/dcf/dcf.go b/pkg/dcf/dcf.go index 94484ee..69d5917 100644 --- a/pkg/dcf/dcf.go +++ b/pkg/dcf/dcf.go @@ -1,6 +1,8 @@ // Package dcf reads DCF cards: memory cards laid out to the "Design rule for // Camera File system" specification (JEITA CP-3461). It finds the cards among -// the mounted filesystems and lists the images and videos on each one. +// the mounted filesystems and parses the DCF directories and files on each one +// into their directory numbers and file numbers. It lists the images and videos +// on each card, and everything under DCIM that is not a DCF directory or file. package dcf import ( @@ -20,8 +22,30 @@ import ( // images and videos in the root of a mounted filesystem. type Store struct { RootDirectory string - Images []*Image - Videos []*Video + // Directories holds the DCF directories under DCIM. + Directories []*Directory + // Images and Videos hold the images and videos among the objects in + // Directories, and those under PRIVATE/M4ROOT. + Images []*Image + Videos []*Video + // NonConforming holds the path, relative to RootDirectory, of everything + // under DCIM that is not a DCF directory or a DCF file. It includes every + // symlink, and everything inside a directory that breaks the DCF naming + // rules. A directory whose name is not valid UTF-8 is listed without what is + // inside it, because fs.FS refuses to open one. + NonConforming []string +} + +// Directory represents a DCF directory: a directory directly under DCIM whose +// name follows the DCF naming rules. +type Directory struct { + // Number is the directory number, from 100 to 999. + Number int + // Name is the name of the directory as it appears on the card. + Name string + // Objects holds every DCF file in the directory, including those that are + // neither images nor videos. + Objects []*Object } // Object represents a file in a DCF store. It is embedded in Image and Video @@ -31,6 +55,10 @@ type Object struct { Path string Size int64 Extension string + // DirectoryNumber and FileNumber are the DCF numbers of a file in a DCF + // directory. Both are 0 for a file under PRIVATE/M4ROOT. + DirectoryNumber int + FileNumber int } // Image represents an image file on a DCF store. @@ -140,58 +168,37 @@ func pathToSHA256(path string) (string, error) { // any mounted filesystems. The integer argument is the number of DCF stores to // return. If the argument is 0, all DCF stores are returned. It does not mount // filesystems or search outside of filesystem root directories. It does, -// however, walk the filesystems to find images and videos and populate the -// returned Stores. +// however, read the DCIM and PRIVATE/M4ROOT directories of each filesystem to +// populate the returned Stores. It never follows a symlink. If a card cannot be +// read, it returns an error that names the card's mount point. func GetDCFStores(requestedCount int) (*[]Store, error) { dcfStorePaths, err := findDCFMountPoints(requestedCount) if err != nil { return nil, err } + dcfStores, err := parseStores(dcfStorePaths) + if err != nil { + return nil, err + } + + return &dcfStores, nil +} + +// parseStores reads the card at each of the given mount points. +func parseStores(mountpoints []string) ([]Store, error) { dcfStores := []Store{} - for _, dcfStorePath := range dcfStorePaths { - dcfStore := Store{ - RootDirectory: dcfStorePath, - Images: make([]*Image, 0), - Videos: make([]*Video, 0), - } - - addFile := func(path string, info os.FileInfo, err error) error { - if err != nil { - return err - } - - if !info.IsDir() && isImageFile(path) { - image := Image{} - image.Path = strings.TrimPrefix(path, dcfStorePath+"/") - image.Size = info.Size() - image.Extension = filepath.Ext(path) - image.DCFStoreRoot = dcfStorePath - dcfStore.Images = append(dcfStore.Images, &image) - } - - if !info.IsDir() && isVideoFile(path) { - video := Video{} - video.Path = strings.TrimPrefix(path, dcfStorePath+"/") - video.Size = info.Size() - video.Extension = filepath.Ext(path) - video.DCFStoreRoot = dcfStorePath - dcfStore.Videos = append(dcfStore.Videos, &video) - } - - return nil - } - - err := filepath.Walk(dcfStorePath, addFile) + for _, mountpoint := range mountpoints { + dcfStore, err := parseStore(os.DirFS(mountpoint), mountpoint) if err != nil { return nil, err } - dcfStores = append(dcfStores, dcfStore) + dcfStores = append(dcfStores, *dcfStore) } - return &dcfStores, nil + return dcfStores, nil } func isImageFile(path string) bool { diff --git a/pkg/dcf/dcf_internal_test.go b/pkg/dcf/dcf_internal_test.go index 46cead7..8d91c6e 100644 --- a/pkg/dcf/dcf_internal_test.go +++ b/pkg/dcf/dcf_internal_test.go @@ -1,6 +1,60 @@ package dcf -import "testing" +import ( + "path/filepath" + "strings" + "testing" +) + +func TestParseStoresReadsEachMountPoint(t *testing.T) { + t.Parallel() + + first := newCardOnDisk(t, "DCIM/100ABCDE/ABCD0001.JPG") + second := newCardOnDisk(t, "PRIVATE/M4ROOT/CLIP/C0001.MP4") + + stores, err := parseStores([]string{first, second}) + if err != nil { + t.Fatalf("parseStores: %v", err) + } + + if len(stores) != 2 { + t.Fatalf("parseStores returned %d stores, want 2", len(stores)) + } + + cases := map[string]Object{ + filepath.Join(first, "DCIM/100ABCDE/ABCD0001.JPG"): stores[0].Images[0].Object, + filepath.Join(second, "PRIVATE/M4ROOT/CLIP/C0001.MP4"): stores[1].Videos[0].Object, + } + + for want, object := range cases { + if got := object.FullFilePath(); got != want { + t.Errorf("FullFilePath() = %q, want %q", got, want) + } + + got, err := object.Hash() + if err != nil { + t.Fatalf("Hash() returned error: %v", err) + } + + // The published SHA-256 test vector for "abc". + wantHash := "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad" + if got != wantHash { + t.Errorf("Hash() = %q, want %q", got, wantHash) + } + } +} + +func TestParseStoresReturnsErrorNamingTheCard(t *testing.T) { + t.Parallel() + + // Every lookup under a plain file fails. + mountpoint := filepath.Join(newCardOnDisk(t, "FILE"), "FILE") + + _, err := parseStores([]string{mountpoint}) + if err == nil || !strings.Contains(err.Error(), mountpoint) { + t.Fatalf("parseStores error = %v, want one naming %s", err, mountpoint) + } +} func TestIsImageFile(t *testing.T) { t.Parallel() diff --git a/pkg/dcf/helpers.go b/pkg/dcf/helpers.go index 5725697..35558de 100644 --- a/pkg/dcf/helpers.go +++ b/pkg/dcf/helpers.go @@ -4,7 +4,6 @@ import ( "fmt" "log/slog" "os" - "path/filepath" "slices" "github.com/shirou/gopsutil/disk" @@ -63,12 +62,12 @@ func filterDCFMountPoints(mountpoints []string, requestedCount int) []string { // isDCFMountPoint reports whether the root of mountpoint holds a DCIM or // PRIVATE/M4ROOT directory, which is how we detect DCF stores. func isDCFMountPoint(mountpoint string) bool { - for _, directory := range []string{"DCIM", "PRIVATE/M4ROOT"} { - path := filepath.Join(mountpoint, directory) + fsys := os.DirFS(mountpoint) - info, err := os.Stat(path) - if err == nil && info.IsDir() { - slog.Debug(fmt.Sprintf("Found %s directory at %s\n", directory, path)) + for _, directory := range []string{dcimDirectory, m4RootDirectory} { + found, err := hasDirectory(fsys, directory) + if err == nil && found { + slog.Debug(fmt.Sprintf("Found %s directory at %s\n", directory, mountpoint)) return true } diff --git a/pkg/dcf/helpers_internal_test.go b/pkg/dcf/helpers_internal_test.go index 1d6bef9..7670e3d 100644 --- a/pkg/dcf/helpers_internal_test.go +++ b/pkg/dcf/helpers_internal_test.go @@ -57,6 +57,21 @@ func TestIsDCFMountPointIgnoresPlainFile(t *testing.T) { } } +func TestIsDCFMountPointIgnoresSymlink(t *testing.T) { + t.Parallel() + + outside := newMountPoint(t, "M4ROOT") + + for _, link := range []string{dcimDirectory, privateDirectory, m4RootDirectory} { + root := newMountPoint(t, filepath.Dir(link)) + addSymlink(t, root, link, outside) + + if isDCFMountPoint(root) { + t.Errorf("isDCFMountPoint counted a symlink at %s as a directory", link) + } + } +} + func TestFilterDCFMountPointsReturnsRequestedCount(t *testing.T) { t.Parallel() diff --git a/pkg/dcf/parse.go b/pkg/dcf/parse.go new file mode 100644 index 0000000..08c6b38 --- /dev/null +++ b/pkg/dcf/parse.go @@ -0,0 +1,275 @@ +package dcf + +import ( + "errors" + "fmt" + "io/fs" + "path" + "regexp" + "strconv" + "strings" +) + +// The two directories at the root of a card that hold its files. +const ( + dcimDirectory = "DCIM" + m4RootDirectory = "PRIVATE/M4ROOT" +) + +// The naming rules of the DCF specification, CIPA DC-009-2010, sections 2.3, +// 4.2.2 and 4.3.1. Names use A-Z, 0-9 and "_", and lower-case letters count as +// upper-case. Each pattern captures the number in the name. +var ( + dcfDirectoryName = regexp.MustCompile(`^([1-9][0-9]{2})[0-9A-Za-z_]{5}$`) + dcfFileName = regexp.MustCompile(`^[0-9A-Za-z_]{4}([0-9]{4})\.[0-9A-Za-z_]{3}$`) +) + +// parseStore reads one card from fsys, which is rooted at the card's mount +// point. The root argument is the path of that mount point. Only DCIM and +// PRIVATE/M4ROOT are read. A card can lack either one. A symlink is never +// followed. An error names the mount point, because fsys reports only paths +// inside it. +func parseStore(fsys fs.FS, root string) (*Store, error) { + store := &Store{RootDirectory: root} + + err := parseDCIM(fsys, store) + if err == nil { + err = parseM4Root(fsys, store) + } + + if err != nil { + return nil, fmt.Errorf("reading the card at %s: %w", root, err) + } + + return store, nil +} + +// hasDirectory reports whether the card holds the named directory. A plain file +// or a symlink at any step of the path counts as missing. Any other failure to +// look is an error. +func hasDirectory(fsys fs.FS, name string) (bool, error) { + parent := path.Dir(name) + if parent != "." { + found, err := hasDirectory(fsys, parent) + if err != nil || !found { + return false, err + } + } + + info, err := fs.Lstat(fsys, name) + if errors.Is(err, fs.ErrNotExist) { + return false, nil + } + + if err != nil { + return false, err + } + + return info.IsDir(), nil +} + +// parseDCIM adds the DCF directories under DCIM to store. Everything else there +// is not conforming. That includes directories that share a directory number, +// which section 7.1.2 of the specification treats as non-DCF directories. +func parseDCIM(fsys fs.FS, store *Store) error { + found, err := hasDirectory(fsys, dcimDirectory) + if err != nil || !found { + return err + } + + entries, err := fs.ReadDir(fsys, dcimDirectory) + if err != nil { + return err + } + + directoriesWithNumber := map[int]int{} + + for _, entry := range entries { + if entry.IsDir() { + directoriesWithNumber[parseNumber(dcfDirectoryName, entry.Name())]++ + } + } + + for _, entry := range entries { + number := parseNumber(dcfDirectoryName, entry.Name()) + + if entry.IsDir() && number != 0 && directoriesWithNumber[number] == 1 { + err = parseDirectory(fsys, store, entry.Name(), number) + } else { + err = addNonConforming(fsys, store, dcimDirectory, entry) + } + + if err != nil { + return err + } + } + + return nil +} + +// parseDirectory adds one DCF directory and the DCF files directly inside it to +// store. A DCF file is a regular file, so a symlink with a DCF name is not one. +// Everything else there is not conforming. That includes JPG files that share a +// file number, which section 7.2.2 of the specification says must not show DCF +// numbers. Other files that share a file number do conform. +func parseDirectory(fsys fs.FS, store *Store, name string, number int) error { + directoryPath := path.Join(dcimDirectory, name) + + entries, err := fs.ReadDir(fsys, directoryPath) + if err != nil { + return err + } + + jpgsWithNumber := map[int]int{} + + for _, entry := range entries { + if entry.Type().IsRegular() && isJPG(entry.Name()) { + jpgsWithNumber[parseNumber(dcfFileName, entry.Name())]++ + } + } + + directory := &Directory{Number: number, Name: name} + + for _, entry := range entries { + fileNumber := parseNumber(dcfFileName, entry.Name()) + sharedJPG := isJPG(entry.Name()) && jpgsWithNumber[fileNumber] > 1 + + if !entry.Type().IsRegular() || fileNumber == 0 || sharedJPG { + err = addNonConforming(fsys, store, directoryPath, entry) + if err != nil { + return err + } + + continue + } + + entryPath := path.Join(directoryPath, entry.Name()) + + object, err := newObject(store, entryPath, entry) + if err != nil { + return err + } + + object.DirectoryNumber = number + object.FileNumber = fileNumber + directory.Objects = append(directory.Objects, &object) + + addMedia(store, object) + } + + store.Directories = append(store.Directories, directory) + + return nil +} + +// parseM4Root adds the images and videos under PRIVATE/M4ROOT to store. The DCF +// naming rules do not apply there, so these files have no DCF numbers. It skips +// a symlink. It also skips a name that is not valid UTF-8, because fs.FS refuses +// to open one. +func parseM4Root(fsys fs.FS, store *Store) error { + found, err := hasDirectory(fsys, m4RootDirectory) + if err != nil || !found { + return err + } + + return fs.WalkDir(fsys, m4RootDirectory, + func(filePath string, entry fs.DirEntry, err error) error { + if err != nil { + return err + } + + validName := fs.ValidPath(filePath) + + if entry.IsDir() && !validName { + return fs.SkipDir + } + + if !entry.Type().IsRegular() || !validName { + return nil + } + + object, err := newObject(store, filePath, entry) + if err != nil { + return err + } + + addMedia(store, object) + + return nil + }) +} + +// addNonConforming lists one entry of the directory dir on store as not +// conforming. If the entry is a directory, it lists everything inside it too. It +// does not follow a symlink. It does not read a directory whose name is not +// valid UTF-8, because fs.FS refuses to open one. +func addNonConforming(fsys fs.FS, store *Store, dir string, entry fs.DirEntry) error { + entryPath := path.Join(dir, entry.Name()) + store.NonConforming = append(store.NonConforming, entryPath) + + if !entry.IsDir() || !fs.ValidPath(entryPath) { + return nil + } + + entries, err := fs.ReadDir(fsys, entryPath) + if err != nil { + return err + } + + for _, inner := range entries { + err = addNonConforming(fsys, store, entryPath, inner) + if err != nil { + return err + } + } + + return nil +} + +// newObject returns the Object for the file at filePath, with no DCF numbers. +func newObject(store *Store, filePath string, entry fs.DirEntry) (Object, error) { + info, err := entry.Info() + if err != nil { + return Object{}, err + } + + return Object{ + DCFStoreRoot: store.RootDirectory, + Path: filePath, + Size: info.Size(), + Extension: path.Ext(filePath), + }, nil +} + +// addMedia adds object to the images or videos of store. A file with any other +// extension is neither, and is not added. +func addMedia(store *Store, object Object) { + if isImageFile(object.Path) { + store.Images = append(store.Images, &Image{Object: object}) + } + + if isVideoFile(object.Path) { + store.Videos = append(store.Videos, &Video{Object: object}) + } +} + +// parseNumber returns the number that pattern captures in name. It returns 0 +// when name does not match. No DCF number is 0, so this also rejects the file +// number 0000, which the specification forbids. +func parseNumber(pattern *regexp.Regexp, name string) int { + match := pattern.FindStringSubmatch(name) + if match == nil { + return 0 + } + + number, err := strconv.Atoi(match[1]) + if err != nil { + return 0 + } + + return number +} + +func isJPG(name string) bool { + return strings.EqualFold(path.Ext(name), ".JPG") +} diff --git a/pkg/dcf/parse_internal_test.go b/pkg/dcf/parse_internal_test.go new file mode 100644 index 0000000..633dd79 --- /dev/null +++ b/pkg/dcf/parse_internal_test.go @@ -0,0 +1,809 @@ +package dcf + +import ( + "errors" + "io/fs" + "os" + "path" + "path/filepath" + "reflect" + "slices" + "testing" + "testing/fstest" +) + +const ( + testRoot = "/mnt/card" + privateDirectory = "PRIVATE" +) + +// newCard returns a card holding an empty file at each of the given paths. +func newCard(paths ...string) fstest.MapFS { + card := fstest.MapFS{} + + for _, filePath := range paths { + card[filePath] = &fstest.MapFile{} + } + + return card +} + +// newCardOnDisk returns a temporary directory holding the three bytes "abc" in a +// file at each of the given paths. It is for tests that fstest.MapFS cannot do. +func newCardOnDisk(t *testing.T, paths ...string) string { + t.Helper() + + root := t.TempDir() + + for _, filePath := range paths { + fullPath := filepath.Join(root, filePath) + + err := os.MkdirAll(filepath.Dir(fullPath), 0o750) + if err != nil { + t.Fatalf("MkdirAll: %v", err) + } + + err = os.WriteFile(fullPath, []byte("abc"), 0o600) + if err != nil { + t.Fatalf("WriteFile: %v", err) + } + } + + return root +} + +// addSymlink creates a symlink to target at the named path under root. +func addSymlink(t *testing.T, root, name, target string) { + t.Helper() + + err := os.Symlink(target, filepath.Join(root, name)) + if err != nil { + t.Fatalf("Symlink: %v", err) + } +} + +func mustParseStore(t *testing.T, fsys fs.FS) *Store { + t.Helper() + + store, err := parseStore(fsys, testRoot) + if err != nil { + t.Fatalf("parseStore: %v", err) + } + + return store +} + +func imagePaths(store *Store) []string { + paths := make([]string, 0, len(store.Images)) + + for _, image := range store.Images { + paths = append(paths, image.Path) + } + + return paths +} + +func videoPaths(store *Store) []string { + paths := make([]string, 0, len(store.Videos)) + + for _, video := range store.Videos { + paths = append(paths, video.Path) + } + + return paths +} + +func objectPaths(directory *Directory) []string { + paths := make([]string, 0, len(directory.Objects)) + + for _, object := range directory.Objects { + paths = append(paths, object.Path) + } + + return paths +} + +func directoryNumbers(store *Store) []int { + numbers := make([]int, 0, len(store.Directories)) + + for _, directory := range store.Directories { + numbers = append(numbers, directory.Number) + } + + return numbers +} + +func assertPaths(t *testing.T, what string, got []string, want ...string) { + t.Helper() + + if !slices.Equal(got, want) { + t.Errorf("%s = %q, want %q", what, got, want) + } +} + +func TestParseStorePutsEachFileWhereItBelongs(t *testing.T) { + t.Parallel() + + card := fstest.MapFS{ + "DCIM/100MSDCF/ABCD0001.ARW": {Data: make([]byte, 2000)}, + "DCIM/100MSDCF/readme.txt": {Data: make([]byte, 7)}, + "DCIM/101CANON/MVI_0002.MP4": {Data: make([]byte, 9000)}, + "DCIM/101CANON/MVI_0002.THM": {Data: make([]byte, 100)}, + "DCIM/MISC/DSC00003.JPG": {Data: make([]byte, 7)}, + "DCIM/DSC00004.JPG": {Data: make([]byte, 7)}, + "PRIVATE/M4ROOT/CLIP/C0002.MP4": {Data: make([]byte, 400)}, + "PRIVATE/M4ROOT/CLIP/C0002M01.XML": {Data: make([]byte, 7)}, + "PRIVATE/OTHER/C0003.MP4": {Data: make([]byte, 7)}, + "BACKUP/DSC00005.JPG": {Data: make([]byte, 7)}, + "DSC00006.JPG": {Data: make([]byte, 7)}, + } + + store := mustParseStore(t, card) + + arw := Object{ + DCFStoreRoot: testRoot, + Path: "DCIM/100MSDCF/ABCD0001.ARW", + Size: 2000, + Extension: ".ARW", + DirectoryNumber: 100, + FileNumber: 1, + } + mp4 := Object{ + DCFStoreRoot: testRoot, + Path: "DCIM/101CANON/MVI_0002.MP4", + Size: 9000, + Extension: ".MP4", + DirectoryNumber: 101, + FileNumber: 2, + } + thm := Object{ + DCFStoreRoot: testRoot, + Path: "DCIM/101CANON/MVI_0002.THM", + Size: 100, + Extension: ".THM", + DirectoryNumber: 101, + FileNumber: 2, + } + clip := Object{ + DCFStoreRoot: testRoot, + Path: "PRIVATE/M4ROOT/CLIP/C0002.MP4", + Size: 400, + Extension: ".MP4", + } + + wantDirectories := []*Directory{ + {Number: 100, Name: "100MSDCF", Objects: []*Object{&arw}}, + {Number: 101, Name: "101CANON", Objects: []*Object{&mp4, &thm}}, + } + if !reflect.DeepEqual(store.Directories, wantDirectories) { + t.Errorf("Directories = %+v, want %+v", store.Directories, wantDirectories) + } + + wantImages := []*Image{{Object: arw}} + if !reflect.DeepEqual(store.Images, wantImages) { + t.Errorf("Images = %+v, want %+v", store.Images, wantImages) + } + + wantVideos := []*Video{{Object: mp4}, {Object: clip}} + if !reflect.DeepEqual(store.Videos, wantVideos) { + t.Errorf("Videos = %+v, want %+v", store.Videos, wantVideos) + } + + assertPaths(t, "NonConforming", store.NonConforming, + "DCIM/100MSDCF/readme.txt", + "DCIM/DSC00004.JPG", + "DCIM/MISC", + "DCIM/MISC/DSC00003.JPG", + ) +} + +func TestParseStoreCountsOnlyImagesAndVideos(t *testing.T) { + t.Parallel() + + card := fstest.MapFS{ + "DCIM/100ABCDE/ABCD0001.ARW": {Data: make([]byte, 2000)}, + "DCIM/100ABCDE/ABCD0002.THM": {Data: make([]byte, 100)}, + "DCIM/100ABCDE/ABCD0003.MP4": {Data: make([]byte, 9000)}, + "DCIM/MISC/ABCD0004.JPG": {Data: make([]byte, 50)}, + "PRIVATE/M4ROOT/CLIP/C0004.MP4": {Data: make([]byte, 400)}, + "ABCD0005.JPG": {Data: make([]byte, 30)}, + } + + store := mustParseStore(t, card) + + if got := store.ImagesCount(); got != 1 { + t.Errorf("ImagesCount() = %d, want 1", got) + } + + if got := store.VideosCount(); got != 2 { + t.Errorf("VideosCount() = %d, want 2", got) + } + + if got := store.TotalSize(); got != 11400 { + t.Errorf("TotalSize() = %d, want 11400", got) + } +} + +func TestParseStoreSetsFullFilePath(t *testing.T) { + t.Parallel() + + store := mustParseStore(t, newCard("DCIM/100MSDCF/DSC00001.JPG")) + + want := testRoot + "/DCIM/100MSDCF/DSC00001.JPG" + if got := store.Images[0].FullFilePath(); got != want { + t.Fatalf("FullFilePath() = %q, want %q", got, want) + } +} + +func TestParseNumberOfDirectoryName(t *testing.T) { + t.Parallel() + + cases := map[string]int{ + "100MSDCF": 100, + "999VWXYZ": 999, + "100D3500": 100, + "101abcde": 101, + "102A_B_C": 102, + "099ABCDE": 0, + "000ABCDE": 0, + "103A": 0, + "IM104ABC": 0, + "105_abcdefg": 0, + "107@-%+]": 0, + "IM08ABCD": 0, + "100ABC-E": 0, + "100ABCD": 0, + "100ABCDEF": 0, + "1000ABCDE": 0, + "X100ABCDE": 0, + "TEMP": 0, + } + + for name, want := range cases { + if got := parseNumber(dcfDirectoryName, name); got != want { + t.Errorf("directory %q parsed to %d, want %d", name, got, want) + } + } +} + +func TestParseNumberOfFileName(t *testing.T) { + t.Parallel() + + cases := map[string]int{ + "DSC00001.ARW": 1, + "ABCD9999.JPG": 9999, + "abcd0002.jpg": 2, + "_ABC0001.JPG": 1, + "IMG_0001.JPG": 1, + "12340005.JPG": 5, + "ABCD0000.JPG": 0, + "A0004.JPG": 0, + "ABCDE0005.JPG": 0, + "ABCD10000.JPG": 0, + "abcdefg0006.JPG": 0, + "+-@]0007.JPG": 0, + "ABCD00~1.JPG": 0, + "ABCDEFGH.JPG": 0, + "ABCD0001": 0, + "ABCD0001.": 0, + "ABCD0001.JP": 0, + "ABCD0001.JPEG": 0, + "ABCD0001.J-G": 0, + "._DSC0001.JPG": 0, + ".DS_Store": 0, + } + + for name, want := range cases { + if got := parseNumber(dcfFileName, name); got != want { + t.Errorf("file %q parsed to %d, want %d", name, got, want) + } + } +} + +func TestParseStoreTreatsLowerCaseNamesAsDCFNames(t *testing.T) { + t.Parallel() + + store := mustParseStore(t, newCard("DCIM/101abcde/abcd0002.jpg")) + + want := []*Directory{{ + Number: 101, + Name: "101abcde", + Objects: []*Object{{ + DCFStoreRoot: testRoot, + Path: "DCIM/101abcde/abcd0002.jpg", + Extension: ".jpg", + DirectoryNumber: 101, + FileNumber: 2, + }}, + }} + if !reflect.DeepEqual(store.Directories, want) { + t.Errorf("Directories = %+v, want %+v", store.Directories, want) + } + + assertPaths(t, "Images", imagePaths(store), "DCIM/101abcde/abcd0002.jpg") + assertPaths(t, "NonConforming", store.NonConforming) +} + +func TestParseStoreAcceptsUnderscoreInNames(t *testing.T) { + t.Parallel() + + store := mustParseStore(t, newCard("DCIM/100A_B_C/_ABC0001.JPG")) + + assertPaths(t, "Images", imagePaths(store), "DCIM/100A_B_C/_ABC0001.JPG") + assertPaths(t, "NonConforming", store.NonConforming) +} + +func TestParseStoreRejectsDirectoriesSharingANumber(t *testing.T) { + t.Parallel() + + store := mustParseStore(t, newCard( + "DCIM/102ABCDE/ABCD0001.JPG", + "DCIM/102PQRST/ABCD0002.JPG", + "DCIM/103ABCDE/ABCD0003.JPG", + )) + + if got, want := directoryNumbers(store), []int{103}; !slices.Equal(got, want) { + t.Errorf("directory numbers = %v, want %v", got, want) + } + + assertPaths(t, "Images", imagePaths(store), "DCIM/103ABCDE/ABCD0003.JPG") + assertPaths(t, "NonConforming", store.NonConforming, + "DCIM/102ABCDE", + "DCIM/102ABCDE/ABCD0001.JPG", + "DCIM/102PQRST", + "DCIM/102PQRST/ABCD0002.JPG", + ) +} + +func TestParseStoreDoesNotCountPlainFileAsSharingADirectoryNumber(t *testing.T) { + t.Parallel() + + store := mustParseStore(t, newCard( + "DCIM/100ABCDE", + "DCIM/100PQRST/ABCD0001.JPG", + )) + + if got, want := directoryNumbers(store), []int{100}; !slices.Equal(got, want) { + t.Errorf("directory numbers = %v, want %v", got, want) + } + + assertPaths(t, "NonConforming", store.NonConforming, "DCIM/100ABCDE") +} + +func TestParseStoreRejectsJPGsSharingAFileNumber(t *testing.T) { + t.Parallel() + + store := mustParseStore(t, newCard( + "DCIM/100ABCDE/ABCD0003.JPG", + "DCIM/100ABCDE/PQRS0003.jpg", + "DCIM/101ABCDE/ABCD0003.JPG", + )) + + assertPaths(t, "objects", objectPaths(store.Directories[0])) + assertPaths(t, "objects", objectPaths(store.Directories[1]), + "DCIM/101ABCDE/ABCD0003.JPG", + ) + assertPaths(t, "Images", imagePaths(store), "DCIM/101ABCDE/ABCD0003.JPG") + assertPaths(t, "NonConforming", store.NonConforming, + "DCIM/100ABCDE/ABCD0003.JPG", + "DCIM/100ABCDE/PQRS0003.jpg", + ) +} + +func TestParseStoreKeepsOtherFilesSharingAFileNumber(t *testing.T) { + t.Parallel() + + store := mustParseStore(t, newCard( + "DCIM/100ABCDE/DSC00004.ARW", + "DCIM/100ABCDE/DSC00004.JPG", + "DCIM/100ABCDE/MVI_0005.MP4", + "DCIM/100ABCDE/MVI_0005.THM", + )) + + assertPaths(t, "objects", objectPaths(store.Directories[0]), + "DCIM/100ABCDE/DSC00004.ARW", + "DCIM/100ABCDE/DSC00004.JPG", + "DCIM/100ABCDE/MVI_0005.MP4", + "DCIM/100ABCDE/MVI_0005.THM", + ) + assertPaths(t, "Images", imagePaths(store), + "DCIM/100ABCDE/DSC00004.ARW", + "DCIM/100ABCDE/DSC00004.JPG", + ) + assertPaths(t, "Videos", videoPaths(store), "DCIM/100ABCDE/MVI_0005.MP4") + assertPaths(t, "NonConforming", store.NonConforming) +} + +func TestParseStoreKeepsARWSharingANumberWithTwoJPGs(t *testing.T) { + t.Parallel() + + store := mustParseStore(t, newCard( + "DCIM/100ABCDE/ABCD0004.ARW", + "DCIM/100ABCDE/ABCD0004.JPG", + "DCIM/100ABCDE/PQRS0004.JPG", + )) + + assertPaths(t, "objects", objectPaths(store.Directories[0]), + "DCIM/100ABCDE/ABCD0004.ARW", + ) + assertPaths(t, "NonConforming", store.NonConforming, + "DCIM/100ABCDE/ABCD0004.JPG", + "DCIM/100ABCDE/PQRS0004.JPG", + ) +} + +func TestParseStoreDoesNotCountDirectoryAsSharingAFileNumber(t *testing.T) { + t.Parallel() + + store := mustParseStore(t, newCard( + "DCIM/100ABCDE/ABCD0003.JPG", + "DCIM/100ABCDE/PQRS0003.JPG/README.TXT", + )) + + assertPaths(t, "objects", objectPaths(store.Directories[0]), + "DCIM/100ABCDE/ABCD0003.JPG", + ) + assertPaths(t, "NonConforming", store.NonConforming, + "DCIM/100ABCDE/PQRS0003.JPG", + "DCIM/100ABCDE/PQRS0003.JPG/README.TXT", + ) +} + +func TestParseStoreDoesNotCountDirectoryNamedLikeAVideo(t *testing.T) { + t.Parallel() + + store := mustParseStore(t, newCard("PRIVATE/M4ROOT/CLIP.MP4/C0001.XML")) + + assertPaths(t, "Videos", videoPaths(store)) +} + +func TestParseStoreRejectsSubdirectoryOfDCFDirectory(t *testing.T) { + t.Parallel() + + store := mustParseStore(t, newCard( + "DCIM/100ABCDE/SUB/ABCD0001.JPG", + "DCIM/100ABCDE/ABCD0002.JPG/ABCD0003.JPG", + )) + + assertPaths(t, "objects", objectPaths(store.Directories[0])) + assertPaths(t, "Images", imagePaths(store)) + assertPaths(t, "NonConforming", store.NonConforming, + "DCIM/100ABCDE/ABCD0002.JPG", + "DCIM/100ABCDE/ABCD0002.JPG/ABCD0003.JPG", + "DCIM/100ABCDE/SUB", + "DCIM/100ABCDE/SUB/ABCD0001.JPG", + ) +} + +func TestParseStoreListsEmptyDirectories(t *testing.T) { + t.Parallel() + + store := mustParseStore(t, fstest.MapFS{ + "DCIM/100EMPTY": {Mode: fs.ModeDir}, + "DCIM/MISC": {Mode: fs.ModeDir}, + }) + + if got, want := directoryNumbers(store), []int{100}; !slices.Equal(got, want) { + t.Errorf("directory numbers = %v, want %v", got, want) + } + + assertPaths(t, "NonConforming", store.NonConforming, "DCIM/MISC") +} + +func TestParseStoreWithoutDCIM(t *testing.T) { + t.Parallel() + + store := mustParseStore(t, newCard("PRIVATE/M4ROOT/CLIP/C0001.MP4")) + + if len(store.Directories) != 0 { + t.Errorf("Directories = %+v, want none", store.Directories) + } + + assertPaths(t, "Videos", videoPaths(store), "PRIVATE/M4ROOT/CLIP/C0001.MP4") +} + +func TestParseStoreWithoutM4Root(t *testing.T) { + t.Parallel() + + store := mustParseStore(t, newCard("DCIM/100ABCDE/ABCD0001.JPG")) + + assertPaths(t, "Images", imagePaths(store), "DCIM/100ABCDE/ABCD0001.JPG") + assertPaths(t, "Videos", videoPaths(store)) +} + +func TestParseStoreTreatsPlainFilesAsMissingDirectories(t *testing.T) { + t.Parallel() + + store := mustParseStore(t, newCard("DCIM", "PRIVATE/M4ROOT")) + + assertPaths(t, "Images", imagePaths(store)) + assertPaths(t, "Videos", videoPaths(store)) + assertPaths(t, "NonConforming", store.NonConforming) +} + +// A real filesystem reports a lookup under a plain file differently from a +// missing path, which fstest.MapFS does not reproduce. +func TestParseStoreReadsRealFilesystemWithPlainFileNamedPrivate(t *testing.T) { + t.Parallel() + + root := newCardOnDisk(t, "DCIM/100ABCDE/ABCD0001.JPG", "PRIVATE") + + store := mustParseStore(t, os.DirFS(root)) + + assertPaths(t, "Images", imagePaths(store), "DCIM/100ABCDE/ABCD0001.JPG") + + if got := store.TotalImageSize(); got != 3 { + t.Errorf("TotalImageSize() = %d, want 3", got) + } +} + +// A real filesystem can hold a name with the byte 0xff, which is not valid UTF-8. +// fs.FS refuses to open such a name. +func TestParseStoreListsNamesThatFSRefuses(t *testing.T) { + t.Parallel() + + root := newCardOnDisk(t, + "DCIM/100ABCDE/ABCD0001.JPG", + "DCIM/100ABCDE/A\xffDIR/INNER", + "DCIM/100ABCDE/A\xffFILE", + "DCIM/A\xffDIR/INNER", + "DCIM/A\xffFILE", + "DCIM/MISC/A\xffDIR/INNER", + "DCIM/MISC/A\xffFILE", + ) + + store := mustParseStore(t, os.DirFS(root)) + + assertPaths(t, "Images", imagePaths(store), "DCIM/100ABCDE/ABCD0001.JPG") + assertPaths(t, "NonConforming", store.NonConforming, + "DCIM/100ABCDE/A\xffDIR", + "DCIM/100ABCDE/A\xffFILE", + "DCIM/A\xffDIR", + "DCIM/A\xffFILE", + "DCIM/MISC", + "DCIM/MISC/A\xffDIR", + "DCIM/MISC/A\xffFILE", + ) +} + +func TestParseStoreListsDanglingSymlinks(t *testing.T) { + t.Parallel() + + root := newCardOnDisk(t, "DCIM/100ABCDE/ABCD0001.JPG", "DCIM/MISC/README.TXT") + + addSymlink(t, root, "DCIM/100ABCDE/ABCD0002.JPG", "absent") + addSymlink(t, root, "DCIM/100ABCDE/LINK", "absent") + addSymlink(t, root, "DCIM/101ABCDE", "absent") + addSymlink(t, root, "DCIM/LINK", "absent") + addSymlink(t, root, "DCIM/MISC/LINK", "absent") + + store := mustParseStore(t, os.DirFS(root)) + + if got, want := directoryNumbers(store), []int{100}; !slices.Equal(got, want) { + t.Errorf("directory numbers = %v, want %v", got, want) + } + + assertPaths(t, "Images", imagePaths(store), "DCIM/100ABCDE/ABCD0001.JPG") + assertPaths(t, "NonConforming", store.NonConforming, + "DCIM/100ABCDE/ABCD0002.JPG", + "DCIM/100ABCDE/LINK", + "DCIM/101ABCDE", + "DCIM/LINK", + "DCIM/MISC", + "DCIM/MISC/LINK", + "DCIM/MISC/README.TXT", + ) +} + +func TestParseStoreDoesNotFollowSymlinksUnderDCIM(t *testing.T) { + t.Parallel() + + outside := newCardOnDisk(t, "OUTSIDE.JPG", "DIRECTORY/ABCD0009.JPG") + outsideFile := filepath.Join(outside, "OUTSIDE.JPG") + outsideDirectory := filepath.Join(outside, "DIRECTORY") + + root := newCardOnDisk(t, + "DCIM/100ABCDE/ABCD0001.JPG", + "DCIM/102ABCDE/ABCD0002.JPG", + "DCIM/MISC/README.TXT", + ) + + // These two have DCF file names. The second has the number of a real JPG, + // which must not make that JPG shared. + addSymlink(t, root, "DCIM/100ABCDE/ABCD0003.JPG", outsideFile) + addSymlink(t, root, "DCIM/100ABCDE/PQRS0001.JPG", outsideFile) + + // These two have DCF directory names. The second has the number of a real + // directory, which must not make that directory shared. + addSymlink(t, root, "DCIM/101ABCDE", outsideDirectory) + addSymlink(t, root, "DCIM/102PQRST", outsideDirectory) + + addSymlink(t, root, "DCIM/LINK", outsideDirectory) + addSymlink(t, root, "DCIM/MISC/LINK", outsideDirectory) + + store := mustParseStore(t, os.DirFS(root)) + + if got, want := directoryNumbers(store), []int{100, 102}; !slices.Equal(got, want) { + t.Errorf("directory numbers = %v, want %v", got, want) + } + + assertPaths(t, "Images", imagePaths(store), + "DCIM/100ABCDE/ABCD0001.JPG", + "DCIM/102ABCDE/ABCD0002.JPG", + ) + + if got := store.TotalImageSize(); got != 6 { + t.Errorf("TotalImageSize() = %d, want 6", got) + } + + assertPaths(t, "NonConforming", store.NonConforming, + "DCIM/100ABCDE/ABCD0003.JPG", + "DCIM/100ABCDE/PQRS0001.JPG", + "DCIM/101ABCDE", + "DCIM/102PQRST", + "DCIM/LINK", + "DCIM/MISC", + "DCIM/MISC/LINK", + "DCIM/MISC/README.TXT", + ) +} + +func TestParseStoreSkipsSymlinksAndNamesThatFSRefusesUnderM4Root(t *testing.T) { + t.Parallel() + + outside := newCardOnDisk(t, "OUTSIDE.MP4") + + // The names with the byte 0xff sort before CLIP, so a walk that stops at one + // of them does not reach it. + root := newCardOnDisk(t, + "PRIVATE/M4ROOT/A\xffDIR/C0002.MP4", + "PRIVATE/M4ROOT/A\xffFILE.MP4", + "PRIVATE/M4ROOT/CLIP/C0001.MP4", + ) + + addSymlink(t, root, "PRIVATE/M4ROOT/CLIP/C0003.MP4", + filepath.Join(outside, "OUTSIDE.MP4")) + addSymlink(t, root, "PRIVATE/M4ROOT/CLIP/C0004.MP4", "absent") + addSymlink(t, root, "PRIVATE/M4ROOT/LINK", outside) + + store := mustParseStore(t, os.DirFS(root)) + + assertPaths(t, "Videos", videoPaths(store), "PRIVATE/M4ROOT/CLIP/C0001.MP4") + + if got := store.TotalVideoSize(); got != 3 { + t.Errorf("TotalVideoSize() = %d, want 3", got) + } +} + +func TestParseStoreTreatsSymlinksAsMissingDirectories(t *testing.T) { + t.Parallel() + + outside := newCardOnDisk(t, + "100ABCDE/ABCD0001.JPG", + "M4ROOT/C0001.MP4", + "C0002.MP4", + ) + + cases := []struct{ link, target string }{ + {dcimDirectory, outside}, + {dcimDirectory, dcimDirectory}, + {privateDirectory, outside}, + {privateDirectory, privateDirectory}, + {m4RootDirectory, outside}, + } + + for _, testCase := range cases { + root := newMountPoint(t, filepath.Dir(testCase.link)) + addSymlink(t, root, testCase.link, testCase.target) + + store := mustParseStore(t, os.DirFS(root)) + + if !reflect.DeepEqual(store, &Store{RootDirectory: testRoot}) { + t.Errorf("symlink at %s to %s read as %+v, want an empty store", + testCase.link, testCase.target, store) + } + } +} + +// failingFS is a card on which one call fails for one path, the way a failing +// card does. The call is Lstat, ReadDir or Info. +type failingFS struct { + fstest.MapFS + + call string + name string +} + +func (f failingFS) Lstat(name string) (fs.FileInfo, error) { + err := f.failure("Lstat", name) + if err != nil { + return nil, err + } + + return f.MapFS.Lstat(name) +} + +func (f failingFS) ReadDir(name string) ([]fs.DirEntry, error) { + err := f.failure("ReadDir", name) + if err != nil { + return nil, err + } + + entries, err := f.MapFS.ReadDir(name) + if err != nil { + return nil, err + } + + for i, entry := range entries { + err = f.failure("Info", path.Join(name, entry.Name())) + if err != nil { + entries[i] = failingEntry{DirEntry: entry, err: err} + } + } + + return entries, nil +} + +func (f failingFS) failure(call, name string) error { + if call == f.call && name == f.name { + return &fs.PathError{Op: call, Path: name, Err: fs.ErrPermission} + } + + return nil +} + +// failingEntry is a directory entry whose Info always returns err. +type failingEntry struct { + fs.DirEntry + + err error +} + +func (e failingEntry) Info() (fs.FileInfo, error) { + return nil, e.err +} + +func TestParseStoreReturnsReadErrorsNamingTheCard(t *testing.T) { + t.Parallel() + + card := newCard( + "DCIM/100ABCDE/ABCD0001.JPG", + "DCIM/100ABCDE/SUB/README.TXT", + "DCIM/MISC/SUB/README.TXT", + "PRIVATE/M4ROOT/CLIP/C0001.MP4", + ) + + cases := map[string][]string{ + "Lstat": {dcimDirectory, privateDirectory, m4RootDirectory}, + "ReadDir": { + dcimDirectory, + "DCIM/100ABCDE", + "DCIM/100ABCDE/SUB", + "DCIM/MISC", + "DCIM/MISC/SUB", + m4RootDirectory, + "PRIVATE/M4ROOT/CLIP", + }, + "Info": {"DCIM/100ABCDE/ABCD0001.JPG", "PRIVATE/M4ROOT/CLIP/C0001.MP4"}, + } + + for call, names := range cases { + for _, name := range names { + fsys := failingFS{MapFS: card, call: call, name: name} + + _, err := parseStore(fsys, testRoot) + if !errors.Is(err, fs.ErrPermission) { + t.Errorf("%s of %s: error = %v, want %v", + call, name, err, fs.ErrPermission) + + continue + } + + want := "reading the card at " + testRoot + ": " + + call + " " + name + ": permission denied" + if err.Error() != want { + t.Errorf("error = %q, want %q", err.Error(), want) + } + } + } +} -- 2.54.0 From cf7611ea1b3f10227cad821013d7197c6e2182d0 Mon Sep 17 00:00:00 2001 From: clawbot Date: Tue, 3 Nov 2026 20:05:44 +0100 Subject: [PATCH 4/5] group DCF files that share a file number (closes #3) Files in one DCF directory that share a file number are one file group, as section 4.3.2.2 of the specification says. `Directory.FileGroups` lists them, each with its numbers, its members and its primary member. A lone file is a group of one. A group reports its member count and total size. Section 6.2.1.3 gives a DCF basic file priority over a DCF extended image file. It says nothing about a DCF optional file, so ranking every `JPG` first is a choice. A `THM` comes last. `isImageFile` now accepts `.tif`, so a `TIF` counts as an image. `Store.Images`, `Store.Videos` and the totals still count each file on its own. Model: opus-5-5 Co-authored-by: clawbot --- README.md | 38 ++++- pkg/dcf/dcf.go | 36 ++++- pkg/dcf/dcf_internal_test.go | 2 + pkg/dcf/filegroup.go | 67 +++++++++ pkg/dcf/filegroup_internal_test.go | 232 +++++++++++++++++++++++++++++ pkg/dcf/parse.go | 1 + pkg/dcf/parse_internal_test.go | 32 ++-- 7 files changed, 388 insertions(+), 20 deletions(-) create mode 100644 pkg/dcf/filegroup.go create mode 100644 pkg/dcf/filegroup_internal_test.go diff --git a/README.md b/README.md index a0d4cc4..4ba4805 100644 --- a/README.md +++ b/README.md @@ -109,14 +109,16 @@ All the code is in the `pkg/dcf` package. The repo has no commands. each one. - `helpers.go` finds the mount points that hold a DCF store. - `parse.go` reads one DCF store through an `fs.FS` rooted at its mount point. +- `filegroup.go` sorts the files of one DCF directory into file groups. The main types are: - `Store` is one DCF store on one mounted filesystem. -- `Directory` is one DCF directory. It has a directory number, a name and the - objects inside it. +- `Directory` is one DCF directory. It has a directory number, a name, the + objects inside it and their file groups. - `Object` is one file in a store. A file in a DCF directory also has a directory number and a file number. +- `FileGroup` is one file group in a DCF directory. - `Image` and `Video` embed `Object`. The file extension decides which one a file becomes. A file with any other extension, such as a `THM` thumbnail, is neither. @@ -149,11 +151,37 @@ The naming rules do not apply under `PRIVATE/M4ROOT`. The images and videos there are in `Store.Images` and `Store.Videos`, and their DCF numbers are 0. `dcf` skips any symlink there. +A file group is made of the DCF files in one DCF directory that share a file +number. The specification calls it a DCF object. A RAW file and its JPEG form a +file group, and so do a video and its `THM` thumbnail. + +- The first four characters and the extension can differ within a group. +- A DCF file that shares its number with no other file is a group of one. +- Files in different DCF directories are never in the same group. +- Only DCF files are grouped. Files in `Store.NonConforming` and under + `PRIVATE/M4ROOT` are in no group. + +`Directory.FileGroups` lists the groups in file number order. Each `FileGroup` +has: + +- `DirectoryNumber` and `FileNumber` +- `Members`, which is every file in the group +- `Primary`, which is the member that stands for the whole group +- `MembersCount`, which returns the number of members +- `TotalSize`, which returns the total size of the members in bytes + +The primary member is the first of these that the group holds: + +1. a `JPG` file +2. any other file in `Store.Images` +3. a file in `Store.Videos` +4. any other file except a `THM` +5. a `THM` + +`Store.Images` and `Store.Videos` list each file on its own, whatever its group. + ## TODO -- Handle DCF file groups, where multiple objects share the same index number. - `dcf` does not handle these yet. If you need this, send me a link to a zip or - tar of a memory card that uses them, and I'll see what I can do. - Take a `context.Context` to cancel the walk of a store. - Return a plain slice from `GetDCFStores`, not a pointer to one. - Copy files off a card, and verify each copy against its SHA256 hash. diff --git a/pkg/dcf/dcf.go b/pkg/dcf/dcf.go index 69d5917..94ba701 100644 --- a/pkg/dcf/dcf.go +++ b/pkg/dcf/dcf.go @@ -2,7 +2,8 @@ // Camera File system" specification (JEITA CP-3461). It finds the cards among // the mounted filesystems and parses the DCF directories and files on each one // into their directory numbers and file numbers. It lists the images and videos -// on each card, and everything under DCIM that is not a DCF directory or file. +// on each card, and everything under DCIM that is not a DCF directory or file. It +// groups the files in each DCF directory that share a file number. package dcf import ( @@ -46,6 +47,22 @@ type Directory struct { // Objects holds every DCF file in the directory, including those that are // neither images nor videos. Objects []*Object + // FileGroups holds the same files, grouped by file number, in file number + // order. + FileGroups []*FileGroup +} + +// FileGroup represents one file group: the files in one DCF directory that share +// a file number. The DCF specification calls this a DCF object. A file that +// shares its number with no other file is a group of one. +type FileGroup struct { + DirectoryNumber int + FileNumber int + // Members holds every file in the group, including Primary, in name order. + // Two members can have the same extension. + Members []*Object + // Primary is the member that stands for the whole group. + Primary *Object } // Object represents a file in a DCF store. It is embedded in Image and Video @@ -114,6 +131,21 @@ func (d *Store) TotalSize() int64 { return d.TotalImageSize() + d.TotalVideoSize() } +// MembersCount returns the number of files in the file group. +func (g *FileGroup) MembersCount() int { + return len(g.Members) +} + +// TotalSize returns the total size in bytes of the files in the file group. +func (g *FileGroup) TotalSize() int64 { + totalSize := int64(0) + for _, member := range g.Members { + totalSize += member.Size + } + + return totalSize +} + func (d *Store) String() string { // A caller can set a negative Size, and uint64 would make that huge. totalSize := uint64(max(d.TotalSize(), 0)) @@ -203,7 +235,7 @@ func parseStores(mountpoints []string) ([]Store, error) { func isImageFile(path string) bool { imageExtensions := []string{ - ".jpg", ".jpeg", ".png", ".gif", ".bmp", ".tiff", ".raw", ".arw", + ".jpg", ".jpeg", ".png", ".gif", ".bmp", ".tif", ".tiff", ".raw", ".arw", } return slices.Contains(imageExtensions, strings.ToLower(filepath.Ext(path))) diff --git a/pkg/dcf/dcf_internal_test.go b/pkg/dcf/dcf_internal_test.go index 8d91c6e..57de998 100644 --- a/pkg/dcf/dcf_internal_test.go +++ b/pkg/dcf/dcf_internal_test.go @@ -62,6 +62,8 @@ func TestIsImageFile(t *testing.T) { cases := map[string]bool{ "DCIM/100MSDCF/DSC00001.jpg": true, "DCIM/100MSDCF/DSC00001.ARW": true, + "DCIM/100MSDCF/DSC00001.TIF": true, + "DCIM/100MSDCF/DSC00001.tif": true, "PRIVATE/M4ROOT/CLIP/C0001.MP4": false, "DCIM/100MSDCF/notes.txt": false, "DCIM/100MSDCF/noextension": false, diff --git a/pkg/dcf/filegroup.go b/pkg/dcf/filegroup.go new file mode 100644 index 0000000..4849938 --- /dev/null +++ b/pkg/dcf/filegroup.go @@ -0,0 +1,67 @@ +package dcf + +import ( + "cmp" + "path" + "slices" + "strings" +) + +// newFileGroups sorts the objects of one DCF directory into file groups, in file +// number order. Section 4.3.2.2 of the specification says the files in a DCF +// directory that share a file number are one group. Their first four characters +// and their extensions can differ. +func newFileGroups(objects []*Object) []*FileGroup { + var groups []*FileGroup + + groupWithNumber := map[int]*FileGroup{} + + for _, object := range objects { + group, found := groupWithNumber[object.FileNumber] + if !found { + group = &FileGroup{ + DirectoryNumber: object.DirectoryNumber, + FileNumber: object.FileNumber, + } + groupWithNumber[object.FileNumber] = group + groups = append(groups, group) + } + + group.Members = append(group.Members, object) + } + + for _, group := range groups { + group.Primary = primaryMember(group.Members) + } + + slices.SortFunc(groups, func(a, b *FileGroup) int { + return cmp.Compare(a.FileNumber, b.FileNumber) + }) + + return groups +} + +// primaryMember returns the member that stands for the whole group. Section +// 6.2.1.3 of the specification gives a DCF basic file priority over a DCF extended +// image file. It says nothing about a DCF optional file, the other kind of JPG, so +// ranking every JPG first is a choice. Section 4.6 makes a THM only the thumbnail +// of another file, so it comes last. The specification leaves the rest open, so an +// image comes before a video, and a video before any other file. Among equals, the +// first in name order wins. +func primaryMember(members []*Object) *Object { + choices := []func(string) bool{isJPG, isImageFile, isVideoFile, isNotTHM} + + for _, choice := range choices { + for _, member := range members { + if choice(member.Path) { + return member + } + } + } + + return members[0] +} + +func isNotTHM(name string) bool { + return !strings.EqualFold(path.Ext(name), ".THM") +} diff --git a/pkg/dcf/filegroup_internal_test.go b/pkg/dcf/filegroup_internal_test.go new file mode 100644 index 0000000..b3d7d81 --- /dev/null +++ b/pkg/dcf/filegroup_internal_test.go @@ -0,0 +1,232 @@ +package dcf + +import ( + "slices" + "testing" + "testing/fstest" +) + +// mustFileGroups returns the file groups of directory, and stops the test unless +// there are exactly count of them. +func mustFileGroups(t *testing.T, directory *Directory, count int) []*FileGroup { + t.Helper() + + if len(directory.FileGroups) != count { + t.Fatalf("%s has %d file groups, want %d", + directory.Name, len(directory.FileGroups), count) + } + + return directory.FileGroups +} + +// assertFileGroup checks the numbers, the primary member and the members of group. +func assertFileGroup( + t *testing.T, + group *FileGroup, + directoryNumber, fileNumber int, + primary string, + members ...string, +) { + t.Helper() + + if group.DirectoryNumber != directoryNumber || group.FileNumber != fileNumber { + t.Errorf("group numbers = %d and %d, want %d and %d", + group.DirectoryNumber, group.FileNumber, directoryNumber, fileNumber) + } + + if group.Primary.Path != primary { + t.Errorf("Primary = %q, want %q", group.Primary.Path, primary) + } + + paths := make([]string, 0, len(group.Members)) + + for _, member := range group.Members { + paths = append(paths, member.Path) + } + + assertPaths(t, "members", paths, members...) + + if got := group.MembersCount(); got != len(members) { + t.Errorf("MembersCount() = %d, want %d", got, len(members)) + } +} + +// assertSizes checks the total size of group and the size of each member. +func assertSizes(t *testing.T, group *FileGroup, total int64, sizes ...int64) { + t.Helper() + + if got := group.TotalSize(); got != total { + t.Errorf("TotalSize() = %d, want %d", got, total) + } + + got := make([]int64, 0, len(group.Members)) + + for _, member := range group.Members { + got = append(got, member.Size) + } + + if !slices.Equal(got, sizes) { + t.Errorf("member sizes = %v, want %v", got, sizes) + } +} + +// assertPrimaryMember checks which of the named members primaryMember picks. +func assertPrimaryMember(t *testing.T, want string, names ...string) { + t.Helper() + + members := make([]*Object, 0, len(names)) + + for _, name := range names { + members = append(members, &Object{Path: name}) + } + + if got := primaryMember(members).Path; got != want { + t.Errorf("primaryMember of %q = %q, want %q", names, got, want) + } +} + +func TestParseStoreGroupsFilesSharingAFileNumber(t *testing.T) { + t.Parallel() + + store := mustParseStore(t, fstest.MapFS{ + "DCIM/100ABCDE/DSC00001.ARW": {Data: make([]byte, 2000)}, + "DCIM/100ABCDE/DSC00001.JPG": {Data: make([]byte, 500)}, + "DCIM/100ABCDE/MVI_0002.MP4": {Data: make([]byte, 9000)}, + "DCIM/100ABCDE/MVI_0002.THM": {Data: make([]byte, 100)}, + "DCIM/100ABCDE/DSC00003.JPG": {Data: make([]byte, 700)}, + "DCIM/101ABCDE/DSC00001.JPG": {Data: make([]byte, 300)}, + }) + + first := mustFileGroups(t, store.Directories[0], 3) + second := mustFileGroups(t, store.Directories[1], 1) + + assertFileGroup(t, first[0], 100, 1, "DCIM/100ABCDE/DSC00001.JPG", + "DCIM/100ABCDE/DSC00001.ARW", + "DCIM/100ABCDE/DSC00001.JPG", + ) + assertSizes(t, first[0], 2500, 2000, 500) + + assertFileGroup(t, first[1], 100, 2, "DCIM/100ABCDE/MVI_0002.MP4", + "DCIM/100ABCDE/MVI_0002.MP4", + "DCIM/100ABCDE/MVI_0002.THM", + ) + assertSizes(t, first[1], 9100, 9000, 100) + + assertFileGroup(t, first[2], 100, 3, "DCIM/100ABCDE/DSC00003.JPG", + "DCIM/100ABCDE/DSC00003.JPG", + ) + assertSizes(t, first[2], 700, 700) + + assertFileGroup(t, second[0], 101, 1, "DCIM/101ABCDE/DSC00001.JPG", + "DCIM/101ABCDE/DSC00001.JPG", + ) + assertSizes(t, second[0], 300, 300) + + // The images, the videos and the totals still count each file on its own. + if got := store.ImagesCount(); got != 4 { + t.Errorf("ImagesCount() = %d, want 4", got) + } + + if got := store.VideosCount(); got != 1 { + t.Errorf("VideosCount() = %d, want 1", got) + } + + if got := store.TotalSize(); got != 12500 { + t.Errorf("TotalSize() = %d, want 12500", got) + } +} + +func TestParseStoreGroupsFilesWhoseFirstFourCharactersDiffer(t *testing.T) { + t.Parallel() + + store := mustParseStore(t, newCard( + "DCIM/100ABCDE/ABCD0004.TIF", + "DCIM/100ABCDE/THMB0004.THM", + )) + + groups := mustFileGroups(t, store.Directories[0], 1) + + assertFileGroup(t, groups[0], 100, 4, "DCIM/100ABCDE/ABCD0004.TIF", + "DCIM/100ABCDE/ABCD0004.TIF", + "DCIM/100ABCDE/THMB0004.THM", + ) +} + +func TestParseStoreOrdersFileGroupsByFileNumber(t *testing.T) { + t.Parallel() + + store := mustParseStore(t, newCard( + "DCIM/100ABCDE/ABCD0002.JPG", + "DCIM/100ABCDE/WXYZ0001.JPG", + )) + + groups := mustFileGroups(t, store.Directories[0], 2) + + assertFileGroup(t, groups[0], 100, 1, "DCIM/100ABCDE/WXYZ0001.JPG", + "DCIM/100ABCDE/WXYZ0001.JPG", + ) + assertFileGroup(t, groups[1], 100, 2, "DCIM/100ABCDE/ABCD0002.JPG", + "DCIM/100ABCDE/ABCD0002.JPG", + ) +} + +func TestParseStoreLeavesOtherFilesOutOfFileGroups(t *testing.T) { + t.Parallel() + + store := mustParseStore(t, newCard( + "DCIM/100ABCDE/ABCD0004.ARW", + "DCIM/100ABCDE/ABCD0004.JPG", + "DCIM/100ABCDE/PQRS0004.JPG", + "DCIM/100ABCDE/README.TXT", + "DCIM/MISC/ABCD0005.JPG", + "PRIVATE/M4ROOT/CLIP/C0006.MP4", + )) + + if len(store.Directories) != 1 { + t.Fatalf("got %d directories, want 1", len(store.Directories)) + } + + groups := mustFileGroups(t, store.Directories[0], 1) + + assertFileGroup(t, groups[0], 100, 4, "DCIM/100ABCDE/ABCD0004.ARW", + "DCIM/100ABCDE/ABCD0004.ARW", + ) +} + +func TestParseStoreGivesDirectoryWithoutDCFFilesNoFileGroups(t *testing.T) { + t.Parallel() + + store := mustParseStore(t, newCard("DCIM/100ABCDE/README.TXT")) + + mustFileGroups(t, store.Directories[0], 0) +} + +func TestPrimaryMember(t *testing.T) { + t.Parallel() + + // A JPG comes first, in either case. + assertPrimaryMember(t, "A0010001.JPG", "A0010001.ARW", "A0010001.JPG") + assertPrimaryMember(t, "A0010001.JPG", "A0010001.JPG", "A0010001.MOV") + assertPrimaryMember(t, "a0010001.jpg", "a0010001.arw", "a0010001.jpg") + + // Then any other image. + assertPrimaryMember(t, "Z0010001.ARW", "A0010001.MP4", "Z0010001.ARW") + assertPrimaryMember(t, "Z0010001.ARW", "A0010001.WAV", "Z0010001.ARW") + assertPrimaryMember(t, "A0010001.TIF", "A0010001.MOV", "A0010001.TIF") + assertPrimaryMember(t, "Z0010001.TIF", "A0010001.WAV", "Z0010001.TIF") + + // Then a video. + assertPrimaryMember(t, "A0010001.MP4", "A0010001.AAC", "A0010001.MP4") + assertPrimaryMember(t, "Z0010001.MP4", "A0010001.THM", "Z0010001.MP4") + + // Then any other file except a THM, in either case. + assertPrimaryMember(t, "Z0010001.WAV", "A0010001.THM", "Z0010001.WAV") + assertPrimaryMember(t, "a0010001.wav", "a0010001.thm", "a0010001.wav") + + // A THM is primary only when the group holds nothing else. + assertPrimaryMember(t, "A0010001.THM", "A0010001.THM") + + // Among equals, the first in name order wins. + assertPrimaryMember(t, "A0010001.THM", "A0010001.THM", "Z0010001.THM") + assertPrimaryMember(t, "A0010001.WAV", "A0010001.WAV", "Z0010001.XML") +} diff --git a/pkg/dcf/parse.go b/pkg/dcf/parse.go index 08c6b38..ffef610 100644 --- a/pkg/dcf/parse.go +++ b/pkg/dcf/parse.go @@ -157,6 +157,7 @@ func parseDirectory(fsys fs.FS, store *Store, name string, number int) error { addMedia(store, object) } + directory.FileGroups = newFileGroups(directory.Objects) store.Directories = append(store.Directories, directory) return nil diff --git a/pkg/dcf/parse_internal_test.go b/pkg/dcf/parse_internal_test.go index 633dd79..7925c9b 100644 --- a/pkg/dcf/parse_internal_test.go +++ b/pkg/dcf/parse_internal_test.go @@ -113,6 +113,16 @@ func directoryNumbers(store *Store) []int { return numbers } +// wantDirectory returns the Directory that parseStore should build for objects. +func wantDirectory(number int, name string, objects ...*Object) *Directory { + return &Directory{ + Number: number, + Name: name, + Objects: objects, + FileGroups: newFileGroups(objects), + } +} + func assertPaths(t *testing.T, what string, got []string, want ...string) { t.Helper() @@ -172,8 +182,8 @@ func TestParseStorePutsEachFileWhereItBelongs(t *testing.T) { } wantDirectories := []*Directory{ - {Number: 100, Name: "100MSDCF", Objects: []*Object{&arw}}, - {Number: 101, Name: "101CANON", Objects: []*Object{&mp4, &thm}}, + wantDirectory(100, "100MSDCF", &arw), + wantDirectory(101, "101CANON", &mp4, &thm), } if !reflect.DeepEqual(store.Directories, wantDirectories) { t.Errorf("Directories = %+v, want %+v", store.Directories, wantDirectories) @@ -305,17 +315,13 @@ func TestParseStoreTreatsLowerCaseNamesAsDCFNames(t *testing.T) { store := mustParseStore(t, newCard("DCIM/101abcde/abcd0002.jpg")) - want := []*Directory{{ - Number: 101, - Name: "101abcde", - Objects: []*Object{{ - DCFStoreRoot: testRoot, - Path: "DCIM/101abcde/abcd0002.jpg", - Extension: ".jpg", - DirectoryNumber: 101, - FileNumber: 2, - }}, - }} + want := []*Directory{wantDirectory(101, "101abcde", &Object{ + DCFStoreRoot: testRoot, + Path: "DCIM/101abcde/abcd0002.jpg", + Extension: ".jpg", + DirectoryNumber: 101, + FileNumber: 2, + })} if !reflect.DeepEqual(store.Directories, want) { t.Errorf("Directories = %+v, want %+v", store.Directories, want) } -- 2.54.0 From acb108a82e9c92e4082b2d5a9ba13050f12c01b2 Mon Sep 17 00:00:00 2001 From: clawbot Date: Tue, 3 Nov 2026 20:54:19 +0100 Subject: [PATCH 5/5] get cards with GetStores and GetStore, read them through fs.FS (closes #4) GetStores replaces GetDCFStores: it takes a context, has no count argument and returns a plain slice. GetStore returns the one mounted card, or ErrNoStore or ErrMultipleStores. Each Object keeps the fs.FS its card was read through, and Hash reads through it, so tests can use fstest.MapFS. A cancelled context stops the read before the next directory. Library errors wrap with %w. Model: opus-5-5 Co-authored-by: clawbot --- README.md | 23 +++++--- pkg/dcf/dcf.go | 80 ++++++++++++++++++-------- pkg/dcf/dcf_internal_test.go | 88 ++++++++++++++++++++++++++++- pkg/dcf/dcf_test.go | 54 ++++++++---------- pkg/dcf/helpers.go | 32 ++++++----- pkg/dcf/helpers_internal_test.go | 23 ++------ pkg/dcf/parse.go | 70 +++++++++++++++++------ pkg/dcf/parse_internal_test.go | 97 +++++++++++++++++++++++++++++++- 8 files changed, 352 insertions(+), 115 deletions(-) diff --git a/README.md b/README.md index 4ba4805..3f658a5 100644 --- a/README.md +++ b/README.md @@ -29,6 +29,7 @@ Then list the DCF stores on the mounted filesystems: package main import ( + "context" "fmt" "log" @@ -36,17 +37,21 @@ import ( ) func main() { - stores, err := dcf.GetDCFStores(0) + stores, err := dcf.GetStores(context.Background()) if err != nil { log.Fatal(err) } - for _, store := range *stores { + for _, store := range stores { fmt.Println(store.String()) } } ``` +Call `dcf.GetStore` instead when you expect exactly one DCF store. It returns +`dcf.ErrNoStore` when none is mounted, and `dcf.ErrMultipleStores` when more +than one is. + To work on the library itself, you need docker and git: ```sh @@ -105,8 +110,8 @@ overengineering and reusable code. All the code is in the `pkg/dcf` package. The repo has no commands. -- `dcf.go` holds the public API. `GetDCFStores` finds the DCF stores and reads - each one. +- `dcf.go` holds the public API. `GetStores` finds the DCF stores and reads each + one. `GetStore` returns the one DCF store when exactly one is mounted. - `helpers.go` finds the mount points that hold a DCF store. - `parse.go` reads one DCF store through an `fs.FS` rooted at its mount point. - `filegroup.go` sorts the files of one DCF directory into file groups. @@ -117,7 +122,9 @@ The main types are: - `Directory` is one DCF directory. It has a directory number, a name, the objects inside it and their file groups. - `Object` is one file in a store. A file in a DCF directory also has a - directory number and a file number. + directory number and a file number. `Object.Hash` reads the file through the + `fs.FS` of its store. An `Object` you build yourself has none, so its `Hash` + returns an error. - `FileGroup` is one file group in a DCF directory. - `Image` and `Video` embed `Object`. The file extension decides which one a file becomes. A file with any other extension, such as a `THM` thumbnail, is @@ -129,6 +136,10 @@ filesystem. Media anywhere else on the filesystem is not part of the store. `dcf` never follows a symlink. +`GetStores` and `GetStore` take a `context.Context`. When it is cancelled, the +read stops before the next directory. The returned error matches +`context.Canceled`. + `dcf` applies the naming rules of the DCF specification, CIPA DC-009-2010, under `DCIM`: @@ -182,8 +193,6 @@ The primary member is the first of these that the group holds: ## TODO -- Take a `context.Context` to cancel the walk of a store. -- Return a plain slice from `GetDCFStores`, not a pointer to one. - Copy files off a card, and verify each copy against its SHA256 hash. ## License diff --git a/pkg/dcf/dcf.go b/pkg/dcf/dcf.go index 94ba701..6df82f3 100644 --- a/pkg/dcf/dcf.go +++ b/pkg/dcf/dcf.go @@ -7,10 +7,13 @@ package dcf import ( + "context" "crypto/sha256" "encoding/hex" + "errors" "fmt" "io" + "io/fs" "os" "path/filepath" "slices" @@ -19,6 +22,14 @@ import ( "github.com/dustin/go-humanize" ) +var ( + // ErrNoStore is the error from GetStore when no DCF store is mounted. + ErrNoStore = errors.New("no DCF store is mounted") + // ErrMultipleStores is the error from GetStore when more than one DCF store + // is mounted. + ErrMultipleStores = errors.New("more than one DCF store is mounted") +) + // Store represents a DCF store, which is a directory structure that contains // images and videos in the root of a mounted filesystem. type Store struct { @@ -76,6 +87,9 @@ type Object struct { // directory. Both are 0 for a file under PRIVATE/M4ROOT. DirectoryNumber int FileNumber int + // fsys is the fs.FS the store was read through. Hash reads the file through + // it. + fsys fs.FS } // Image represents an image file on a DCF store. @@ -173,15 +187,17 @@ func (i *Image) String() string { ) } -// Hash returns the SHA256 hash of the file as a hex string. +// Hash returns the SHA256 hash of the file as a hex string. It reads the file +// through the fs.FS of its store. An Object built by hand has none, so Hash +// returns an error that matches fs.ErrInvalid. func (d *Object) Hash() (string, error) { - return pathToSHA256(d.FullFilePath()) -} + if d.fsys == nil { + return "", fmt.Errorf("hashing %s: %w", d.FullFilePath(), fs.ErrInvalid) + } -func pathToSHA256(path string) (string, error) { - file, err := os.Open(filepath.Clean(path)) + file, err := d.fsys.Open(d.Path) if err != nil { - return "", err + return "", fmt.Errorf("hashing %s: %w", d.FullFilePath(), err) } defer func() { _ = file.Close() }() @@ -190,44 +206,64 @@ func pathToSHA256(path string) (string, error) { _, err = io.Copy(hash, file) if err != nil { - return "", err + return "", fmt.Errorf("hashing %s: %w", d.FullFilePath(), err) } return hex.EncodeToString(hash.Sum(nil)), nil } -// GetDCFStores returns a list of DCF stores found on the system in the root of -// any mounted filesystems. The integer argument is the number of DCF stores to -// return. If the argument is 0, all DCF stores are returned. It does not mount -// filesystems or search outside of filesystem root directories. It does, -// however, read the DCIM and PRIVATE/M4ROOT directories of each filesystem to -// populate the returned Stores. It never follows a symlink. If a card cannot be -// read, it returns an error that names the card's mount point. -func GetDCFStores(requestedCount int) (*[]Store, error) { - dcfStorePaths, err := findDCFMountPoints(requestedCount) +// GetStores finds the DCF stores in the root of the mounted filesystems and reads +// each one. It does not mount filesystems or search outside of filesystem root +// directories. It reads the DCIM and PRIVATE/M4ROOT directories of each +// filesystem to populate the returned Stores. It never follows a symlink. If a +// card cannot be read, it returns an error that names the card's mount point. +// When ctx is cancelled, the read stops before the next directory, and the error +// matches ctx.Err(). +func GetStores(ctx context.Context) ([]*Store, error) { + dcfStorePaths, err := findDCFMountPoints(ctx) if err != nil { return nil, err } - dcfStores, err := parseStores(dcfStorePaths) + return parseStores(ctx, dcfStorePaths) +} + +// GetStore returns the one DCF store when exactly one is mounted. It returns +// ErrNoStore when none is mounted, and ErrMultipleStores when more than one is. +// It reads every card first, the way GetStores does, so a card that cannot be +// read returns that error instead. +func GetStore(ctx context.Context) (*Store, error) { + stores, err := GetStores(ctx) if err != nil { return nil, err } - return &dcfStores, nil + return onlyStore(stores) +} + +// onlyStore returns the store when stores holds exactly one. +func onlyStore(stores []*Store) (*Store, error) { + switch len(stores) { + case 0: + return nil, ErrNoStore + case 1: + return stores[0], nil + default: + return nil, ErrMultipleStores + } } // parseStores reads the card at each of the given mount points. -func parseStores(mountpoints []string) ([]Store, error) { - dcfStores := []Store{} +func parseStores(ctx context.Context, mountpoints []string) ([]*Store, error) { + dcfStores := []*Store{} for _, mountpoint := range mountpoints { - dcfStore, err := parseStore(os.DirFS(mountpoint), mountpoint) + dcfStore, err := parseStore(ctx, os.DirFS(mountpoint), mountpoint) if err != nil { return nil, err } - dcfStores = append(dcfStores, *dcfStore) + dcfStores = append(dcfStores, dcfStore) } return dcfStores, nil diff --git a/pkg/dcf/dcf_internal_test.go b/pkg/dcf/dcf_internal_test.go index 57de998..e3e10c4 100644 --- a/pkg/dcf/dcf_internal_test.go +++ b/pkg/dcf/dcf_internal_test.go @@ -1,9 +1,13 @@ package dcf import ( + "context" + "errors" + "io/fs" "path/filepath" "strings" "testing" + "testing/fstest" ) func TestParseStoresReadsEachMountPoint(t *testing.T) { @@ -12,7 +16,7 @@ func TestParseStoresReadsEachMountPoint(t *testing.T) { first := newCardOnDisk(t, "DCIM/100ABCDE/ABCD0001.JPG") second := newCardOnDisk(t, "PRIVATE/M4ROOT/CLIP/C0001.MP4") - stores, err := parseStores([]string{first, second}) + stores, err := parseStores(t.Context(), []string{first, second}) if err != nil { t.Fatalf("parseStores: %v", err) } @@ -50,12 +54,92 @@ func TestParseStoresReturnsErrorNamingTheCard(t *testing.T) { // Every lookup under a plain file fails. mountpoint := filepath.Join(newCardOnDisk(t, "FILE"), "FILE") - _, err := parseStores([]string{mountpoint}) + _, err := parseStores(t.Context(), []string{mountpoint}) if err == nil || !strings.Contains(err.Error(), mountpoint) { t.Fatalf("parseStores error = %v, want one naming %s", err, mountpoint) } } +func TestParseStoresReturnsErrorWhenCancelled(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithCancel(t.Context()) + cancel() + + mountpoint := newCardOnDisk(t, "DCIM/100ABCDE/ABCD0001.JPG") + + _, err := parseStores(ctx, []string{mountpoint}) + if !errors.Is(err, context.Canceled) { + t.Fatalf("parseStores error = %v, want %v", err, context.Canceled) + } +} + +func TestOnlyStore(t *testing.T) { + t.Parallel() + + first := newMountPoint(t, dcimDirectory) + second := newMountPoint(t, m4RootDirectory) + + stores, err := parseStores(t.Context(), []string{first, second}) + if err != nil { + t.Fatalf("parseStores: %v", err) + } + + _, err = onlyStore(stores[:0]) + if !errors.Is(err, ErrNoStore) { + t.Errorf("onlyStore of no cards: error = %v, want %v", err, ErrNoStore) + } + + store, err := onlyStore(stores[:1]) + if err != nil { + t.Fatalf("onlyStore of one card: %v", err) + } + + if store.RootDirectory != first { + t.Errorf("onlyStore returned the card at %s, want %s", + store.RootDirectory, first) + } + + _, err = onlyStore(stores) + if !errors.Is(err, ErrMultipleStores) { + t.Errorf("onlyStore of two cards: error = %v, want %v", + err, ErrMultipleStores) + } +} + +func TestObjectHashReadsThroughTheCard(t *testing.T) { + t.Parallel() + + card := fstest.MapFS{"DCIM/100ABCDE/ABCD0001.JPG": {Data: []byte("abc")}} + + got, err := mustParseStore(t, card).Images[0].Hash() + if err != nil { + t.Fatalf("Hash() returned error: %v", err) + } + + // The published SHA-256 test vector for "abc". + want := "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad" + if got != want { + t.Fatalf("Hash() = %q, want %q", got, want) + } +} + +func TestObjectHashOfMissingFileReturnsError(t *testing.T) { + t.Parallel() + + const filePath = "DCIM/100ABCDE/ABCD0001.JPG" + + card := newCard(filePath) + image := mustParseStore(t, card).Images[0] + + delete(card, filePath) + + _, err := image.Hash() + if !errors.Is(err, fs.ErrNotExist) { + t.Fatalf("Hash() error = %v, want %v", err, fs.ErrNotExist) + } +} + func TestIsImageFile(t *testing.T) { t.Parallel() diff --git a/pkg/dcf/dcf_test.go b/pkg/dcf/dcf_test.go index 9ba82db..3f514cd 100644 --- a/pkg/dcf/dcf_test.go +++ b/pkg/dcf/dcf_test.go @@ -1,8 +1,9 @@ package dcf_test import ( - "os" - "path/filepath" + "context" + "errors" + "io/fs" "testing" "sneak.berlin/go/dcf/pkg/dcf" @@ -125,37 +126,30 @@ func TestVideoString(t *testing.T) { } } -func TestObjectHashIsSHA256OfFile(t *testing.T) { +func TestObjectHashOfObjectBuiltByHandReturnsError(t *testing.T) { t.Parallel() - root := t.TempDir() - - err := os.WriteFile(filepath.Join(root, "a.jpg"), []byte("abc"), 0o600) - if err != nil { - t.Fatalf("WriteFile: %v", err) - } - - object := dcf.Object{DCFStoreRoot: root, Path: "a.jpg"} - - got, err := object.Hash() - if err != nil { - t.Fatalf("Hash() returned error: %v", err) - } - - // The published SHA-256 test vector for "abc". - want := "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad" - if got != want { - t.Fatalf("Hash() = %q, want %q", got, want) - } -} - -func TestObjectHashOfMissingFileReturnsError(t *testing.T) { - t.Parallel() - - object := dcf.Object{DCFStoreRoot: t.TempDir(), Path: "absent.jpg"} + object := dcf.Object{DCFStoreRoot: cardRoot, Path: "a.jpg"} _, err := object.Hash() - if err == nil { - t.Fatal("Hash() of a missing file returned no error") + if !errors.Is(err, fs.ErrInvalid) { + t.Fatalf("Hash() error = %v, want %v", err, fs.ErrInvalid) + } +} + +func TestGetStoresAndGetStoreReturnCancellation(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithCancel(t.Context()) + cancel() + + _, err := dcf.GetStores(ctx) + if !errors.Is(err, context.Canceled) { + t.Errorf("GetStores error = %v, want %v", err, context.Canceled) + } + + _, err = dcf.GetStore(ctx) + if !errors.Is(err, context.Canceled) { + t.Errorf("GetStore error = %v, want %v", err, context.Canceled) } } diff --git a/pkg/dcf/helpers.go b/pkg/dcf/helpers.go index 35558de..3dd2822 100644 --- a/pkg/dcf/helpers.go +++ b/pkg/dcf/helpers.go @@ -1,6 +1,7 @@ package dcf import ( + "context" "fmt" "log/slog" "os" @@ -9,12 +10,19 @@ import ( "github.com/shirou/gopsutil/disk" ) -func findAllMountPoints() ([]string, error) { +func findAllMountPoints(ctx context.Context) ([]string, error) { mountpoints := []string{} - partitions, err := disk.Partitions(false) // physical devices only, so false + // disk.PartitionsWithContext does not check ctx on Linux. + err := ctx.Err() if err != nil { - return nil, err + return nil, fmt.Errorf("listing the mounted filesystems: %w", err) + } + + // physical devices only, so false + partitions, err := disk.PartitionsWithContext(ctx, false) + if err != nil { + return nil, fmt.Errorf("listing the mounted filesystems: %w", err) } for _, partition := range partitions { @@ -26,21 +34,18 @@ func findAllMountPoints() ([]string, error) { return mountpoints, nil } -// findDCFMountPoints returns a list of strings of the paths of mountpoints -// that hold a DCF store. Its only argument is an integer of how many mountpoints -// to return. If the argument is 0, all mountpoints are returned. -func findDCFMountPoints(requestedCount int) ([]string, error) { - mountpoints, err := findAllMountPoints() +// findDCFMountPoints returns the paths of the mountpoints that hold a DCF store. +func findDCFMountPoints(ctx context.Context) ([]string, error) { + mountpoints, err := findAllMountPoints(ctx) if err != nil { return nil, err } - return filterDCFMountPoints(mountpoints, requestedCount), nil + return filterDCFMountPoints(mountpoints), nil } -// filterDCFMountPoints returns the first requestedCount of the mountpoints that -// hold a DCF store. If requestedCount is 0, it returns all of them. -func filterDCFMountPoints(mountpoints []string, requestedCount int) []string { +// filterDCFMountPoints returns the mountpoints that hold a DCF store. +func filterDCFMountPoints(mountpoints []string) []string { filteredMountpoints := []string{} for _, mountpoint := range mountpoints { @@ -51,9 +56,6 @@ func filterDCFMountPoints(mountpoints []string, requestedCount int) []string { slog.Debug(fmt.Sprintf("Keeping mountpoint %s\n", mountpoint)) filteredMountpoints = append(filteredMountpoints, mountpoint) - if (requestedCount > 0) && (len(filteredMountpoints) >= requestedCount) { - break - } } return filteredMountpoints diff --git a/pkg/dcf/helpers_internal_test.go b/pkg/dcf/helpers_internal_test.go index 7670e3d..17b8aca 100644 --- a/pkg/dcf/helpers_internal_test.go +++ b/pkg/dcf/helpers_internal_test.go @@ -72,29 +72,16 @@ func TestIsDCFMountPointIgnoresSymlink(t *testing.T) { } } -func TestFilterDCFMountPointsReturnsRequestedCount(t *testing.T) { +func TestFilterDCFMountPointsKeepsThoseHoldingAStore(t *testing.T) { t.Parallel() first := newMountPoint(t, "DCIM") second := newMountPoint(t, "PRIVATE/M4ROOT") - third := newMountPoint(t, "DCIM") - // This one holds no DCF store, so it must not use up the requested count. - mountpoints := []string{newMountPoint(t), first, second, third} + got := filterDCFMountPoints([]string{newMountPoint(t), first, second}) - cases := map[int][]string{ - 0: {first, second, third}, - 1: {first}, - 2: {first, second}, - 3: {first, second, third}, - 4: {first, second, third}, - } - - for requestedCount, want := range cases { - got := filterDCFMountPoints(mountpoints, requestedCount) - if !slices.Equal(got, want) { - t.Errorf("filterDCFMountPoints(%d) = %v, want %v", - requestedCount, got, want) - } + want := []string{first, second} + if !slices.Equal(got, want) { + t.Errorf("filterDCFMountPoints = %v, want %v", got, want) } } diff --git a/pkg/dcf/parse.go b/pkg/dcf/parse.go index ffef610..c818b2d 100644 --- a/pkg/dcf/parse.go +++ b/pkg/dcf/parse.go @@ -1,6 +1,7 @@ package dcf import ( + "context" "errors" "fmt" "io/fs" @@ -28,13 +29,13 @@ var ( // point. The root argument is the path of that mount point. Only DCIM and // PRIVATE/M4ROOT are read. A card can lack either one. A symlink is never // followed. An error names the mount point, because fsys reports only paths -// inside it. -func parseStore(fsys fs.FS, root string) (*Store, error) { +// inside it. When ctx is cancelled, the read stops before the next directory. +func parseStore(ctx context.Context, fsys fs.FS, root string) (*Store, error) { store := &Store{RootDirectory: root} - err := parseDCIM(fsys, store) + err := parseDCIM(ctx, fsys, store) if err == nil { - err = parseM4Root(fsys, store) + err = parseM4Root(ctx, fsys, store) } if err != nil { @@ -68,16 +69,26 @@ func hasDirectory(fsys fs.FS, name string) (bool, error) { return info.IsDir(), nil } +// readDir returns the entries of the named directory, unless ctx is cancelled. +func readDir(ctx context.Context, fsys fs.FS, name string) ([]fs.DirEntry, error) { + err := ctx.Err() + if err != nil { + return nil, err + } + + return fs.ReadDir(fsys, name) +} + // parseDCIM adds the DCF directories under DCIM to store. Everything else there // is not conforming. That includes directories that share a directory number, // which section 7.1.2 of the specification treats as non-DCF directories. -func parseDCIM(fsys fs.FS, store *Store) error { +func parseDCIM(ctx context.Context, fsys fs.FS, store *Store) error { found, err := hasDirectory(fsys, dcimDirectory) if err != nil || !found { return err } - entries, err := fs.ReadDir(fsys, dcimDirectory) + entries, err := readDir(ctx, fsys, dcimDirectory) if err != nil { return err } @@ -94,9 +105,9 @@ func parseDCIM(fsys fs.FS, store *Store) error { number := parseNumber(dcfDirectoryName, entry.Name()) if entry.IsDir() && number != 0 && directoriesWithNumber[number] == 1 { - err = parseDirectory(fsys, store, entry.Name(), number) + err = parseDirectory(ctx, fsys, store, entry.Name(), number) } else { - err = addNonConforming(fsys, store, dcimDirectory, entry) + err = addNonConforming(ctx, fsys, store, dcimDirectory, entry) } if err != nil { @@ -112,10 +123,16 @@ func parseDCIM(fsys fs.FS, store *Store) error { // Everything else there is not conforming. That includes JPG files that share a // file number, which section 7.2.2 of the specification says must not show DCF // numbers. Other files that share a file number do conform. -func parseDirectory(fsys fs.FS, store *Store, name string, number int) error { +func parseDirectory( + ctx context.Context, + fsys fs.FS, + store *Store, + name string, + number int, +) error { directoryPath := path.Join(dcimDirectory, name) - entries, err := fs.ReadDir(fsys, directoryPath) + entries, err := readDir(ctx, fsys, directoryPath) if err != nil { return err } @@ -135,7 +152,7 @@ func parseDirectory(fsys fs.FS, store *Store, name string, number int) error { sharedJPG := isJPG(entry.Name()) && jpgsWithNumber[fileNumber] > 1 if !entry.Type().IsRegular() || fileNumber == 0 || sharedJPG { - err = addNonConforming(fsys, store, directoryPath, entry) + err = addNonConforming(ctx, fsys, store, directoryPath, entry) if err != nil { return err } @@ -145,7 +162,7 @@ func parseDirectory(fsys fs.FS, store *Store, name string, number int) error { entryPath := path.Join(directoryPath, entry.Name()) - object, err := newObject(store, entryPath, entry) + object, err := newObject(fsys, store, entryPath, entry) if err != nil { return err } @@ -167,7 +184,7 @@ func parseDirectory(fsys fs.FS, store *Store, name string, number int) error { // naming rules do not apply there, so these files have no DCF numbers. It skips // a symlink. It also skips a name that is not valid UTF-8, because fs.FS refuses // to open one. -func parseM4Root(fsys fs.FS, store *Store) error { +func parseM4Root(ctx context.Context, fsys fs.FS, store *Store) error { found, err := hasDirectory(fsys, m4RootDirectory) if err != nil || !found { return err @@ -185,11 +202,16 @@ func parseM4Root(fsys fs.FS, store *Store) error { return fs.SkipDir } + // fs.WalkDir reads a directory right after this call for it. + if entry.IsDir() { + return ctx.Err() + } + if !entry.Type().IsRegular() || !validName { return nil } - object, err := newObject(store, filePath, entry) + object, err := newObject(fsys, store, filePath, entry) if err != nil { return err } @@ -204,7 +226,13 @@ func parseM4Root(fsys fs.FS, store *Store) error { // conforming. If the entry is a directory, it lists everything inside it too. It // does not follow a symlink. It does not read a directory whose name is not // valid UTF-8, because fs.FS refuses to open one. -func addNonConforming(fsys fs.FS, store *Store, dir string, entry fs.DirEntry) error { +func addNonConforming( + ctx context.Context, + fsys fs.FS, + store *Store, + dir string, + entry fs.DirEntry, +) error { entryPath := path.Join(dir, entry.Name()) store.NonConforming = append(store.NonConforming, entryPath) @@ -212,13 +240,13 @@ func addNonConforming(fsys fs.FS, store *Store, dir string, entry fs.DirEntry) e return nil } - entries, err := fs.ReadDir(fsys, entryPath) + entries, err := readDir(ctx, fsys, entryPath) if err != nil { return err } for _, inner := range entries { - err = addNonConforming(fsys, store, entryPath, inner) + err = addNonConforming(ctx, fsys, store, entryPath, inner) if err != nil { return err } @@ -228,7 +256,12 @@ func addNonConforming(fsys fs.FS, store *Store, dir string, entry fs.DirEntry) e } // newObject returns the Object for the file at filePath, with no DCF numbers. -func newObject(store *Store, filePath string, entry fs.DirEntry) (Object, error) { +func newObject( + fsys fs.FS, + store *Store, + filePath string, + entry fs.DirEntry, +) (Object, error) { info, err := entry.Info() if err != nil { return Object{}, err @@ -239,6 +272,7 @@ func newObject(store *Store, filePath string, entry fs.DirEntry) (Object, error) Path: filePath, Size: info.Size(), Extension: path.Ext(filePath), + fsys: fsys, }, nil } diff --git a/pkg/dcf/parse_internal_test.go b/pkg/dcf/parse_internal_test.go index 7925c9b..ce5a4af 100644 --- a/pkg/dcf/parse_internal_test.go +++ b/pkg/dcf/parse_internal_test.go @@ -1,6 +1,7 @@ package dcf import ( + "context" "errors" "io/fs" "os" @@ -65,7 +66,7 @@ func addSymlink(t *testing.T, root, name, target string) { func mustParseStore(t *testing.T, fsys fs.FS) *Store { t.Helper() - store, err := parseStore(fsys, testRoot) + store, err := parseStore(t.Context(), fsys, testRoot) if err != nil { t.Fatalf("parseStore: %v", err) } @@ -157,6 +158,7 @@ func TestParseStorePutsEachFileWhereItBelongs(t *testing.T) { Extension: ".ARW", DirectoryNumber: 100, FileNumber: 1, + fsys: card, } mp4 := Object{ DCFStoreRoot: testRoot, @@ -165,6 +167,7 @@ func TestParseStorePutsEachFileWhereItBelongs(t *testing.T) { Extension: ".MP4", DirectoryNumber: 101, FileNumber: 2, + fsys: card, } thm := Object{ DCFStoreRoot: testRoot, @@ -173,12 +176,14 @@ func TestParseStorePutsEachFileWhereItBelongs(t *testing.T) { Extension: ".THM", DirectoryNumber: 101, FileNumber: 2, + fsys: card, } clip := Object{ DCFStoreRoot: testRoot, Path: "PRIVATE/M4ROOT/CLIP/C0002.MP4", Size: 400, Extension: ".MP4", + fsys: card, } wantDirectories := []*Directory{ @@ -313,7 +318,9 @@ func TestParseNumberOfFileName(t *testing.T) { func TestParseStoreTreatsLowerCaseNamesAsDCFNames(t *testing.T) { t.Parallel() - store := mustParseStore(t, newCard("DCIM/101abcde/abcd0002.jpg")) + card := newCard("DCIM/101abcde/abcd0002.jpg") + + store := mustParseStore(t, card) want := []*Directory{wantDirectory(101, "101abcde", &Object{ DCFStoreRoot: testRoot, @@ -321,6 +328,7 @@ func TestParseStoreTreatsLowerCaseNamesAsDCFNames(t *testing.T) { Extension: ".jpg", DirectoryNumber: 101, FileNumber: 2, + fsys: card, })} if !reflect.DeepEqual(store.Directories, want) { t.Errorf("Directories = %+v, want %+v", store.Directories, want) @@ -797,7 +805,7 @@ func TestParseStoreReturnsReadErrorsNamingTheCard(t *testing.T) { for _, name := range names { fsys := failingFS{MapFS: card, call: call, name: name} - _, err := parseStore(fsys, testRoot) + _, err := parseStore(t.Context(), fsys, testRoot) if !errors.Is(err, fs.ErrPermission) { t.Errorf("%s of %s: error = %v, want %v", call, name, err, fs.ErrPermission) @@ -813,3 +821,86 @@ func TestParseStoreReturnsReadErrorsNamingTheCard(t *testing.T) { } } } + +// cancelingFS is a card that cancels the read when the read reaches the named +// directory. It records every directory read in read. +type cancelingFS struct { + fstest.MapFS + + name string + cancel context.CancelFunc + read *[]string +} + +func (f cancelingFS) ReadDir(name string) ([]fs.DirEntry, error) { + *f.read = append(*f.read, name) + + if name == f.name { + f.cancel() + } + + return f.MapFS.ReadDir(name) +} + +func TestParseStoreStopsBeforeTheNextDirectoryWhenCancelled(t *testing.T) { + t.Parallel() + + card := newCard( + "DCIM/100ABCDE/SUB/README.TXT", + "DCIM/OTHER/SUB/README.TXT", + "PRIVATE/M4ROOT/CLIP/SUB/C0001.MP4", + ) + + // Each of these directories holds another directory, which must not be read. + names := []string{ + dcimDirectory, + "DCIM/100ABCDE", + "DCIM/OTHER", + m4RootDirectory, + "PRIVATE/M4ROOT/CLIP", + } + + for _, name := range names { + ctx, cancel := context.WithCancel(t.Context()) + + var read []string + + fsys := cancelingFS{MapFS: card, name: name, cancel: cancel, read: &read} + + _, err := parseStore(ctx, fsys, testRoot) + if !errors.Is(err, context.Canceled) { + t.Errorf("cancelled at %s: error = %v, want %v", + name, err, context.Canceled) + } + + if read[len(read)-1] != name { + t.Errorf("cancelled at %s: read %q", name, read) + } + } +} + +func TestParseStoreReadsNoDirectoryWhenAlreadyCancelled(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithCancel(t.Context()) + cancel() + + cards := []fstest.MapFS{ + newCard("DCIM/README.TXT"), + newCard("PRIVATE/M4ROOT/C0001.MP4"), + } + + for _, card := range cards { + var read []string + + // The context is already cancelled, so this needs no name to cancel at. + fsys := cancelingFS{MapFS: card, read: &read} + + _, err := parseStore(ctx, fsys, testRoot) + if !errors.Is(err, context.Canceled) { + t.Errorf("error = %v, want %v", err, context.Canceled) + } + + assertPaths(t, "directories read", read) + } +} -- 2.54.0