183 lines
13 KiB
Markdown
183 lines
13 KiB
Markdown
# Learnings — tofu-core
|
|
|
|
Conventions, patterns, and successful approaches discovered during work on this plan.
|
|
|
|
_Auto-scaffolded by /start-work. Append new entries below - never overwrite._
|
|
|
|
---
|
|
|
|
## Task 5 — tofu.vercmp (RPM-style version comparison)
|
|
|
|
### Module naming
|
|
- `version` is a D reserved keyword (conditional compilation). Module is named
|
|
`tofu.vercmp`, file `src/tofu/vercmp.d`. The `DepConstraint` struct in
|
|
`types.d` uses field `ver` (not `version`) for the same reason.
|
|
|
|
### Algorithm (exact port from ZETA/lib/vercmp.lua)
|
|
- `nextSegment`: skips any non-digit/non-letter chars (separators); then reads
|
|
a contiguous run of same-type chars. Returns `(segment, nextIndex)`.
|
|
- `cmpNumeric`: strips leading zeros from both segments (`"007"` → `"7"`,
|
|
`"000"` → `"0"`). Longer stripped string wins; then lexicographic.
|
|
- `compare`: strips all whitespace, then loops over alternating segments.
|
|
If both segments are digit-typed → `cmpNumeric`, else lexical. When one
|
|
version exhausts segments: the exhausted one is *older* (shorter = older).
|
|
This matches RPM semantics: `compare("1.0.0", "1.0") > 0`.
|
|
- `satisfies`: uses `final switch` on `DepOp` enum. Maps to `compare` result
|
|
as expected (ge→c>=0, le→c<=0, eq→c==0, ne→c!=0, gt→c>0, lt→c<0).
|
|
- `parseDep`: delegates to `DepConstraint.parse` (already ported in types.d by
|
|
task 4). Both implementations are kept consistent.
|
|
|
|
### whitespace stripping
|
|
- Could not use `filter`+`array` because Phobos auto-decodes `string` to
|
|
`dchar` range elements, returning `dchar[]` not `string`. Implemented a
|
|
manual `removeWhitespace` helper using raw code-unit indexing, with a
|
|
`@trusted` cast since the freshly-allocated `char[]` has no aliasing.
|
|
|
|
### Test porting
|
|
- All 10 ZETA test categories ported as separate `@safe unittest` blocks.
|
|
- 4 modules pass unittests (types, vercmp, plus pre-existing ones).
|
|
- `dub build` passes with warnings-as-errors enabled.
|
|
|
|
### Future considerations
|
|
- When task 12 (constraint resolution) imports `tofu.vercmp`, the `compare`
|
|
and `satisfies` functions are ready. The `parseDep` integration with
|
|
`DepConstraint.parse` ensures consistency between ZETA and tofu's dep
|
|
constraint semantics.
|
|
|
|
---
|
|
|
|
## Task 2: Config Loading (src/tofu/config.d)
|
|
|
|
### toml package API (v1.0.0, Kripth/toml)
|
|
- Import: `import toml;` → provides `TOMLDocument`, `TOMLValue`, `parseTOML`, `TOMLException`, `TOMLParserException`
|
|
- `TOMLDocument` has `alias table this` — acts like `TOMLValue[string]` AA
|
|
- Access sections: `doc["core"]` returns `TOMLValue` with `type == TOML_TYPE.TABLE`
|
|
- Access string values: `doc["core"]["key"].str` (throws if type mismatch)
|
|
- Check existence: `"key" in doc` returns `TOMLValue*` (null if absent)
|
|
- `parseTOML()` is `@system` — must wrap in `@trusted` block when calling from `@safe` code
|
|
- Keys without `.str` / `.integer` access will throw `TOMLException`
|
|
|
|
### D language gotchas
|
|
- **Nested function declarations**: D does NOT allow `void foo() { ... }` inside a function body. Use lambdas: `auto foo = () { ... }; foo();` or anonymous `() @trusted { ... }();`
|
|
- **`version` is a keyword**: Cannot use `string version;` as field name — use `string version_;` or initialize with `= ""`
|
|
- **`parseTOML` is `@system`**: The toml package's parse function isn't marked `@safe` — must wrap in `@trusted` lambda
|
|
- **`std.file.readText` is `@system`**: Same — wrap in `@trusted`
|
|
- **`std.process.environment.get()`**: Returns empty string when env var is unset; `"VAR" in environment` checks existence but `environment.get("VAR", "default")` is cleaner
|
|
- **`stderr.writefln` is `@safe`** in recent DMD (2.106+) — usable directly in `@safe` code
|
|
- **AA `in` operator**: Returns pointer (`V*`) or `null` — must check for null before dereferencing
|
|
|
|
### Pattern: env-overrideable config loading
|
|
- Priority: env vars > TOML file > hardcoded defaults
|
|
- TOML read happens BEFORE env override — so TOML values serve as base, env vars overwrite
|
|
- Missing config file → no error, just skip to defaults
|
|
- Malformed TOML → `TOMLParserException` caught, warning to stderr, fall back to defaults
|
|
- Testability: `load()` accepts optional `envOverrides` map and explicit `configFile` path to avoid real env/filesystem
|
|
|
|
### Config design decisions
|
|
- `TOFU_CONFIG` env var overrides default config file path (`~/.config/tofu/config.toml`)
|
|
- `~` in default paths expanded with `std.path.expandTilde`
|
|
- All path fields default to empty string when "find on PATH" — no hardcoded binary paths
|
|
- `defaultJobs` typed as `int` (not `size_t`/`ulong`) since it's a user-facing count
|
|
|
|
### buildOptions warningsAsErrors deprecation
|
|
- DUB warns about `warningsAsErrors` in `buildOptions` — recommends `buildRequirements` instead
|
|
- Not blocking, informational only
|
|
|
|
## Task 3 — `tofu.log` (colored, NO_COLOR-aware logging)
|
|
|
|
- **dmd 2.112 gotcha**: the `std.stdio` globals `stdout`/`stderr` are `@system` to *access* — `makeGlobal` uses `__gshared File result`, which fails `@safe` inference. An `@safe` function cannot call `stdout.writeln(...)` directly. Fix: route every write through a tiny `@trusted` helper (`writeStdout(string)`/`writeStderr(string)`), keep the public log functions `@safe`. Compiler hint: *"using `__gshared` instead of `shared` makes it fail to infer `@safe`"*.
|
|
- **`std.process.environment` (dmd 2.112)**: it is an `abstract final class` of *static* methods — `environment.get(name)` is `@safe` and returns `null` when the var is unset (so "NO_COLOR set" == `get !is null`, even for an empty string). `environment["K"] = v` and `environment.remove(name)` are `@trusted`. NOT deprecated in 2.112.
|
|
- **Testable color decision**: make `colorEnabled()` re-read the environment on every call instead of caching in `static this()`. Enables NO_COLOR/TERM tests without restarting the process. Costs one `getenv` per log line — negligible.
|
|
- **Env save/restore in unittests**: a `private struct ColorEnv` (in `version(unittest)`) that snapshots NO_COLOR+TERM in its ctor and restores them in `~this()` (destructor) — `auto env = ColorEnv("1", "xterm")` scopes the restore. `null` value = remove the var. Double-destruction from a copied temporary is harmless (restore is idempotent).
|
|
- **Capturing stdout/stderr in unittests**: swap the global `stdout`/`stderr` File to a `File(name, "w+")`, write, `flush()`, swap back, then `rewind()` + `readln()`. Restore the global before the temp File's destructor runs.
|
|
- **zeta log format details**: `warn ` paints the *whole prefix* yellow (`paint("yellow", "warn ") ~ msg`), but `error` paints *only the word* then `" " ~ msg` (`paint("red","error") ~ " " ~ msg`). Assertion must expect `\x1b[31merror\x1b[0m failed deploy` — the reset code sits between "error" and the message.
|
|
- **`version` is a D keyword** — cannot be used as a struct field / identifier (trap for the parallel `types.d` work).
|
|
- **`std.stdio.writeln` flushes per call** — already satisfies "no buffering"; no explicit `flush()` needed in production paths.
|
|
- **Verifying a single module without the full build**: `dmd -main -unittest -i src/tofu/log.d` runs just that module's unittests when sibling modules (`config.d`, `types.d`) are mid-edit by parallel agents. `-i` pulls imports in automatically.
|
|
|
|
## Task 4 — `tofu.types` (core data structures)
|
|
|
|
- **`version` is a D keyword** — all struct fields named `version` must be renamed to `ver`. The D compiler treats `version` as a conditional-compilation directive and rejects it as a field name (see task 3 learnings above).
|
|
- **Struct field defaults in D**: `string` fields initialise to `null` by default. To enforce empty-string semantics (`""`), every string field must be explicitly `= ""`.
|
|
- **`final switch` for exhaustive enum handling**: using `final switch` on enum types forces the compiler to verify all cases are covered — catches missing branches at compile time. Used for `poolToString()`.
|
|
- **Manual character scanning preferred over `std.regex`**: the dep-spec parser uses simple char-by-char scanning with `isNameChar()` / `isWhite()` helpers instead of regex. This avoids the `std.regex` dependency, keeps the parser `@safe`, and matches the Lua reference semantics exactly.
|
|
- **Dep spec parsing order matters**: 2-character operators (`>=`, `<=`, `==`, `~=`) MUST be checked before 1-character operators (`>`, `<`, `=`) to avoid false matches. The Lua reference uses an ordered table `OPS = { ">=", "<=", "==", "~=", ">", "<", "=" }` — we replicate this with if/else chains.
|
|
- **`=` normalises to `==`**: the Lua code has `if op == "=" then op = "==" end`; we map both to `DepOp.eq`.
|
|
- **Trailing garbage detection**: after extracting version chars, any non-whitespace remaining in the spec string causes a `TypesException`. The Lua pattern uses `$` anchor for this.
|
|
- **`dub test` includes all `.d` files in source paths** for executable targets, not just imported modules. Pre-existing compile errors in sibling modules (config.d) block test runs. Fixed config.d by removing `private` from a nested function (access specifiers not allowed on local functions in D).
|
|
- **`BuildResult.failed` as `BuildFailure[]`**: the plan describes it as `struct {string name; string reason}[]` — in D this is a named struct `BuildFailure` with array field `BuildFailure[] failed`. The `failedNames()` helper extracts just the names for caller convenience.
|
|
- **`DepConstraint.parse` is a static factory method**: returns a new `DepConstraint` by value, never allocates on the heap. `@safe` throughout.
|
|
- **All structs are `@safe`**: no methods do I/O, no `@system` calls. The module stands alone — no imports from other tofu modules.
|
|
|
|
---
|
|
|
|
## Task 6 — `tofu.http` (sync HTTP client via std.net.curl)
|
|
|
|
### std.net.curl API (key members for HTTP struct)
|
|
- **Construction**: `HTTP(url)` via `opCall`, or `HTTP()` + `.url = url` setter.
|
|
The `.url` setter auto-prepends `http://` if no scheme is present.
|
|
- **Timeouts** (all are `@property` setters taking `core.time.Duration`):
|
|
- `connectTimeout` — connect phase only (`CurlOption.connecttimeout_ms`).
|
|
- `operationTimeout` — DNS + connect + transfer total (`CurlOption.timeout_ms`).
|
|
- `dataTimeout` — low-speed activity timeout (NOT total read timeout;
|
|
sets `low_speed_limit` + `low_speed_time`).
|
|
- For a "read 120s" requirement use `operationTimeout = 120.seconds` —
|
|
it covers the entire operation including data transfer.
|
|
- **Redirects**: `maxRedirects` (property setter, takes `uint`).
|
|
Defaults to 10. Set to 0 to disable, `uint.max` for infinite.
|
|
Internally sets `CurlOption.followlocation` + `CurlOption.maxredirs`.
|
|
- **User-Agent**: `setUserAgent(string)` — instance method (not property).
|
|
- **onReceive**: `void delegate(ubyte[])` returning `size_t`. Must accept
|
|
all bytes or the request aborts. Use `return data.length;`.
|
|
- **onReceiveStatusLine**: Not a direct property on HTTP struct — must
|
|
be set via the `onReceiveHeader` property setter which wraps the
|
|
delegate and internally fires `onReceiveStatusLine` when it detects
|
|
an HTTP status line. After `perform()` the final status is available
|
|
via `http.statusLine`.
|
|
- **statusLine**: `HTTP.StatusLine` struct with `.code` (`ushort`),
|
|
`.majorVersion`, `.minorVersion`, `.reason`. Reset to zero before
|
|
each `perform()`.
|
|
- **perform()**: `CurlCode perform(ThrowOnError = Yes.throwOnError)`.
|
|
Default throws on curl errors, but the thrown exception classes are:
|
|
- `CurlTimeoutException : CurlException` — operation timed out (code 28).
|
|
- `CurlException` — all other curl errors.
|
|
- `HTTPStatusException : CurlException` — only thrown by high-level
|
|
wrappers (_basicHTTP); low-level `perform()` does NOT throw on
|
|
HTTP status codes — you must check `statusLine.code` yourself.
|
|
- **Exception isolation pattern**: catch `CurlTimeoutException` and
|
|
`CurlException` in `@trusted` helpers, re-throw as `HttpException`
|
|
(which extends `Exception`, not `CurlException`). Callers never see
|
|
curl types.
|
|
- **verifyPeer / verifyHost**: On by default in HTTP struct (no change needed).
|
|
|
|
### @safe / @trusted architecture
|
|
- All public functions (`get`, `downloadFile`) are `@safe`.
|
|
- The `configure(HTTP)`, `getImpl`, and `downloadFileImpl` helpers are
|
|
`@trusted` — they are the ONLY places `std.net.curl` (which is
|
|
`@system`-heavy) is used.
|
|
|
|
### Download pattern (.part temp file + rename)
|
|
- Ported from ZETA `lib/fetch.lua`: write to `dest.part`, rename on
|
|
success, remove partial on error. `scope(failure)` at function
|
|
level handles all error paths automatically.
|
|
- `File(tmpPath, "wb")` — binary write mode. File destructor auto-closes;
|
|
explicit `f.close()` needed before rename.
|
|
|
|
### Unittest local server
|
|
- Used `std.concurrency.spawn` + `std.socket.TcpSocket` for a
|
|
self-contained one-shot HTTP server (no external dependency on Python3).
|
|
- Pattern: bind ephemeral port (`InternetAddress.PORT_ANY`), spawn thread
|
|
that `accept()`s one connection, sends canned HTTP response, exits.
|
|
`cast(shared)` the listener to pass to `spawn`. Mark test helpers
|
|
`@trusted`.
|
|
- HTTP response must include `Content-Length` header or libcurl hangs.
|
|
|
|
### Connection refused test
|
|
- Bind socket, record port, close it, then `get()` to that port.
|
|
curl returns error 7 (`CURLE_COULDNT_CONNECT`) → `CurlException`
|
|
→ caught and re-thrown as `HttpException`.
|
|
|
|
### Build verified
|
|
- `dub build` passes with warnings-as-errors.
|
|
- `dub test` passes — all 5 modules (config, log, types, vercmp, http).
|