feat: implement build backend and += env operator, add style guide
- Replace build stub with full phase execution (prepare/build/check/install) via /bin/sh with variable interpolation, config file generation, and env handling (hard-set, soft/?=, append/+=) - Add += append operator for environment variables across DSL, parser, system config parser, build backend, and formatter - Add STYLEGUIDE.md documenting all codebase conventions - Replace EnvEntry bool soft with EnvMode enum (Set/Soft/Append) - Add Plus token type to lexer for += parsing
This commit is contained in:
+484
@@ -0,0 +1,484 @@
|
||||
# Style Guide
|
||||
|
||||
If you're reading this because you thought kappa's C++ looked different from
|
||||
what you're used to — good. That's the point.
|
||||
|
||||
This isn't a suggestion box. It's what the codebase looks like, and it's what
|
||||
your code will look like after you've rewritten it three times because the PR
|
||||
reviewer sent it back. Save yourself the rewrite. Read this first.
|
||||
|
||||
---
|
||||
|
||||
## The Philosophy
|
||||
|
||||
We write C++ like it's the year 2026 and the committee finally shipped
|
||||
something usable. No polyfills. No third-party libraries. No Boost. The
|
||||
standard library is sufficient for a package manager. If you disagree, you
|
||||
haven't read `<format>` closely enough.
|
||||
|
||||
Every line of kappa assumes the reader is competent. We don't explain what
|
||||
`std::string_view` is. We don't annotate obvious control flow. Comments exist
|
||||
to explain *why*, never *what*. If your code needs a comment to be
|
||||
understood, the code is wrong.
|
||||
|
||||
Simplicity is a moral position. The scheduler is the hardest thing in this
|
||||
codebase, and it's 290 lines. If your feature adds more than that, you're
|
||||
building the wrong feature.
|
||||
|
||||
---
|
||||
|
||||
## Naming
|
||||
|
||||
### Structs, classes, enums
|
||||
|
||||
```cpp
|
||||
// PascalCase. Always.
|
||||
struct BuildResult { };
|
||||
enum class TokenType : std::uint8_t { };
|
||||
|
||||
// Enum values are PascalCase too. This isn't Java.
|
||||
enum class EnvMode : std::uint8_t { Set, Soft, Append };
|
||||
|
||||
// Acronyms stay capitalized. B-Tree is BTree, not Btree.
|
||||
// Two-letter acronyms stay capitalized. ID, not Id.
|
||||
```
|
||||
|
||||
Type names state what the thing *is*, not what it's *for*. `SchedResult`, not
|
||||
`ResultForScheduler`. `InitPaths`, not `PathsForInitSystems`.
|
||||
|
||||
### Variables and functions
|
||||
|
||||
```cpp
|
||||
// snake_case. No Hungarian notation. No m_ prefix. No s_ prefix.
|
||||
int pending_deps = 0;
|
||||
void compute_depths(Scheduler& s);
|
||||
std::string_view token_name(TokenType type);
|
||||
```
|
||||
|
||||
Member variables and locals look identical. If you can't tell them apart,
|
||||
your functions are too long. Fix the function.
|
||||
|
||||
### Files
|
||||
|
||||
```
|
||||
src/service/openrc.cpp # snake_case, lowercase
|
||||
include/kappa/service/types.hpp # .hpp for headers, .cpp for source
|
||||
```
|
||||
|
||||
One public class per header is a myth invented by Java developers. Group
|
||||
related declarations. `types.hpp` holds all enums and structs for a module.
|
||||
If a module has one public struct and one public function, they go in the
|
||||
same header.
|
||||
|
||||
---
|
||||
|
||||
## Formatting
|
||||
|
||||
### Indentation and braces
|
||||
|
||||
Four spaces. Attached braces (a K&R variant).
|
||||
|
||||
```cpp
|
||||
// ✓ yes — brace on the same line as the control structure
|
||||
if (pid < 0) {
|
||||
return -1;
|
||||
}
|
||||
|
||||
// ✗ no — Allman/BSD braces on their own line
|
||||
if (pid < 0)
|
||||
{
|
||||
return -1;
|
||||
}
|
||||
|
||||
// ✗ no — missing braces on single-statement bodies
|
||||
if (pid < 0) return -1;
|
||||
```
|
||||
|
||||
Always braces. Even for single statements. The compiler doesn't care.
|
||||
The human reviewing your diff at 2 AM does. clang-tidy will flag bare
|
||||
bodies — apply the fix every time.
|
||||
|
||||
### Line length
|
||||
|
||||
100 columns. Not 80 — we're not teletypes. Not 120 — if you need 120
|
||||
characters to express a thought, your thought is too complicated. Split it.
|
||||
|
||||
### Section separators
|
||||
|
||||
```cpp
|
||||
// --- Section description ---
|
||||
// or
|
||||
// ---------------------------------------------------------------------------
|
||||
// Longer section description spanning the full runway
|
||||
// ---------------------------------------------------------------------------
|
||||
```
|
||||
|
||||
Thin lines (`---`) for sub-sections within a file. Thick lines
|
||||
(`-----------`) for top-level section boundaries. The difference communicates
|
||||
hierarchy without nesting.
|
||||
|
||||
### Switch cases
|
||||
|
||||
```cpp
|
||||
switch (is) {
|
||||
case InitSystem::Systemd:
|
||||
return generate_systemd_service(spec);
|
||||
case InitSystem::S6:
|
||||
return generate_s6_service(spec);
|
||||
case InitSystem::Unknown:
|
||||
default:
|
||||
return {};
|
||||
}
|
||||
```
|
||||
|
||||
Cases at the same indentation as the switch. No blocks around single-return
|
||||
cases. Break or return in every non-fallthrough case. If you're falling
|
||||
through intentionally, wrap it with `[[fallthrough]];` on a line by itself.
|
||||
|
||||
---
|
||||
|
||||
## Types
|
||||
|
||||
### Use `auto` when the type is obvious, explicit when it isn't
|
||||
|
||||
```cpp
|
||||
// ✓ yes — type is obvious from initialization
|
||||
auto scope = eval::make_default_scope();
|
||||
auto& pkg = registry.at(name);
|
||||
|
||||
// ✓ yes — structured bindings, type is obvious
|
||||
for (auto& [key, val] : features) { }
|
||||
|
||||
// ✓ yes — explicit where the type carries meaning
|
||||
std::unique_lock lock(s.mtx); // not auto lock = ...
|
||||
std::uint64_t h = 14695981039346656037ULL; // not auto h = ...
|
||||
```
|
||||
|
||||
### `const` is the default
|
||||
|
||||
Everything is `const` until proven mutable.
|
||||
|
||||
```cpp
|
||||
// ✓ yes
|
||||
const auto& step = plan.steps[i];
|
||||
for (const auto& entry : entries) { }
|
||||
|
||||
// ✗ no — mutable when it shouldn't be
|
||||
auto& step = plan.steps[i];
|
||||
```
|
||||
|
||||
### View types over owning types in parameters
|
||||
|
||||
```cpp
|
||||
// ✓ yes
|
||||
void set_root(std::string_view path);
|
||||
bool is_supported(std::string_view name);
|
||||
|
||||
// ✗ no
|
||||
void set_root(const std::string& path);
|
||||
```
|
||||
|
||||
Return owning types. Accept views. The caller decides ownership. You decide
|
||||
what you need to read.
|
||||
|
||||
### Strong enums only
|
||||
|
||||
```cpp
|
||||
// ✓ yes
|
||||
enum class InitSystem : std::uint8_t { Systemd, OpenRC, S6 };
|
||||
|
||||
// ✗ no
|
||||
enum InitSystem { INIT_SYSTEMD, INIT_OPENRC, INIT_S6 };
|
||||
```
|
||||
|
||||
No unscoped enums. No ALL_CAPS enum values. No integer conversions without
|
||||
explicit intent. If you need to serialize an enum to an integer, write a
|
||||
`to_string` function. The enum's numeric value is an implementation detail,
|
||||
not an interface.
|
||||
|
||||
---
|
||||
|
||||
## Functions
|
||||
|
||||
### One responsibility per function
|
||||
|
||||
If your function name contains the word "and", it does at least two things
|
||||
and needs to be split. If the body doesn't fit on one screen, it does too
|
||||
much. "One screen" means approximately 30 lines. The scheduler's
|
||||
`compute_depths` is 30 lines. So is the resolver's `resolve`. They're at the
|
||||
upper bound. If yours is longer, you're doing something wrong.
|
||||
|
||||
### Error handling: return, don't throw
|
||||
|
||||
```cpp
|
||||
// ✓ yes
|
||||
struct FetchResult {
|
||||
std::filesystem::path work_dir;
|
||||
std::string error;
|
||||
bool ok() const { return error.empty(); }
|
||||
};
|
||||
|
||||
FetchResult fetch(const PackageDef& pkg);
|
||||
|
||||
// ✗ no
|
||||
void fetch(const PackageDef& pkg); // throws on error
|
||||
```
|
||||
|
||||
Exceptions are for unrecoverable programmer errors — out-of-memory, null
|
||||
dereference, violated invariants. They belong in constructors and in the
|
||||
parser (where `ParseError` is the only way to unwind back to diagnostics).
|
||||
|
||||
Every operational failure — network down, disk full, configure script failed,
|
||||
hash mismatch — is a return value. A struct with `bool ok` and
|
||||
`std::string error`. Check the `ok` field, read the `error` string, don't
|
||||
catch exceptions for normal operation.
|
||||
|
||||
### Return early, return often
|
||||
|
||||
```cpp
|
||||
// ✓ yes
|
||||
if (to.empty()) {
|
||||
return {false, "destination is empty"};
|
||||
}
|
||||
// ... main logic ...
|
||||
|
||||
// ✗ no
|
||||
if (!to.empty()) {
|
||||
// ... 40 lines of nesting ...
|
||||
} else {
|
||||
return {false, "destination is empty"};
|
||||
}
|
||||
```
|
||||
|
||||
Guard clauses at the top. Happy path straight down the left margin. If your
|
||||
code has three levels of nesting, you missed an early return opportunity.
|
||||
|
||||
### Static helpers over lambdas
|
||||
|
||||
If a helper is more than 5 lines, extract it to a file-static function above
|
||||
the public API. Named functions are greppable. Named functions show up in
|
||||
stack traces. Lambdas don't. The one exception is a `run_phase` lambda in
|
||||
`build()` — it captures local state that would require a 5-parameter helper
|
||||
and it's clearly a one-off control flow wrapper, not a reusable abstraction.
|
||||
|
||||
---
|
||||
|
||||
## Namespaces
|
||||
|
||||
```cpp
|
||||
namespace kappa::module {
|
||||
|
||||
// Everything goes here.
|
||||
|
||||
} // namespace kappa::module
|
||||
```
|
||||
|
||||
C++17 nested namespace syntax. Closing brace gets a comment with the
|
||||
namespace name. These comments survive diffs, refactors, and editors that
|
||||
collapse braces. They cost one line and save ten minutes of scrolling up to
|
||||
figure out which brace closes what.
|
||||
|
||||
### No `using namespace` at file scope
|
||||
|
||||
```cpp
|
||||
// ✓ yes — inside a function
|
||||
namespace fs = std::filesystem;
|
||||
|
||||
// ✗ no — at file scope
|
||||
using namespace std;
|
||||
```
|
||||
|
||||
Namespace aliases are acceptable inside functions — `namespace fs =
|
||||
std::filesystem;` is fine when the file does a lot of path manipulation. But
|
||||
at file scope? No. You're not writing `using namespace std;` at the top of a
|
||||
header and you're not doing the subtler version of the same sin.
|
||||
|
||||
---
|
||||
|
||||
## Headers
|
||||
|
||||
```cpp
|
||||
#pragma once
|
||||
|
||||
#include "kappa/resolve/plan.hpp"
|
||||
#include "kappa/dsl/ast.hpp"
|
||||
|
||||
#include <string>
|
||||
#include <vector>
|
||||
|
||||
namespace kappa::build {
|
||||
|
||||
struct BuildResult { };
|
||||
|
||||
BuildResult build(const resolve::BuildStep& step,
|
||||
const std::string& work_dir,
|
||||
int jobs);
|
||||
|
||||
} // namespace kappa::build
|
||||
```
|
||||
|
||||
`#pragma once` at the top. No include guards. This is 2026.
|
||||
|
||||
Project headers first, in quotes. System headers second, in angle brackets.
|
||||
Blank line between the two groups. Alphabetical within each group.
|
||||
|
||||
Headers include only what they need to compile. If `build.hpp` uses
|
||||
`resolve::BuildStep` by reference, it includes `resolve/plan.hpp`. It does
|
||||
not forward-declare `BuildStep` — we don't forward-declare across module
|
||||
boundaries. The include is the contract: "this module depends on that one."
|
||||
|
||||
Headers never contain implementation. No `inline` functions. No
|
||||
template definitions in headers (we don't use templates). The one exception
|
||||
is `parse_util.hpp`, which defines `ParseError` inline because it's a thin
|
||||
exception wrapper and splitting it would be ceremony for ceremony's sake.
|
||||
One exception per codebase is a pattern. Two is a problem.
|
||||
|
||||
---
|
||||
|
||||
## Modules
|
||||
|
||||
Every module follows this structure:
|
||||
|
||||
```
|
||||
include/kappa/{module}/
|
||||
├── types.hpp # enums, structs, parse/validate declarations
|
||||
├── {feature}.hpp # public function declarations
|
||||
|
||||
src/{module}/
|
||||
├── types.cpp # parse/validate/describe implementations
|
||||
├── backend_a.cpp # per-variant generation (if applicable)
|
||||
├── backend_b.cpp
|
||||
└── install.cpp # dispatch + orchestration (if applicable)
|
||||
```
|
||||
|
||||
If a module doesn't need `types.hpp` (single struct, single function), both
|
||||
go in `{feature}.hpp`. If a module has no backends, skip them. But don't
|
||||
invent a third pattern. `service/` and `boot/` are the templates. Copy them.
|
||||
|
||||
---
|
||||
|
||||
## Strings and formatting
|
||||
|
||||
```cpp
|
||||
// ✓ yes
|
||||
auto msg = std::format("building {} (depth={})", name, depth);
|
||||
result.error = std::format("command exited with code {}: {}", rc, cmd);
|
||||
|
||||
// ✗ no — ostringstream for trivial concatenation
|
||||
std::ostringstream oss;
|
||||
oss << "building " << name << " (depth=" << depth << ")";
|
||||
|
||||
// ✓ yes — ostringstream for incremental construction
|
||||
std::ostringstream out;
|
||||
out << "[Unit]\n";
|
||||
out << std::format("Description={}\n", desc);
|
||||
```
|
||||
|
||||
`std::format` for one-shot strings. `std::ostringstream` for building up
|
||||
output incrementally (service files, bootloader configs, formatter output).
|
||||
String concatenation with `+` is acceptable for two or three pieces.
|
||||
Anything more goes through `std::format`.
|
||||
|
||||
### String views for parameters
|
||||
|
||||
```cpp
|
||||
// ✓ yes
|
||||
InitSystem parse_init_system(std::string_view name);
|
||||
void print_error(std::ostream& os, std::string_view source,
|
||||
SourceLocation loc, std::string_view message);
|
||||
|
||||
// ✗ no
|
||||
InitSystem parse_init_system(const std::string& name);
|
||||
```
|
||||
|
||||
Views everywhere, except when you need to store the string.
|
||||
|
||||
---
|
||||
|
||||
## The DSL
|
||||
|
||||
The `.kap` DSL grammar is the contract. You can extend it. You cannot break
|
||||
existing configs. Every new token type requires:
|
||||
1. An entry in `TokenType`
|
||||
2. A case in `token_name()`
|
||||
3. Parsing logic in the appropriate parser
|
||||
4. A formatting case in `format.cpp`
|
||||
5. At least one test in `test.sh` that exercises the new syntax
|
||||
|
||||
If you're adding a keyword, think twice. The lexer already has 29 token
|
||||
types. Every new one increases parse time and mental overhead. Can this be
|
||||
expressed with the existing grammar? If yes, don't add a keyword.
|
||||
|
||||
---
|
||||
|
||||
## Thread safety
|
||||
|
||||
The scheduler is multithreaded. If you touch shared state, you own the lock.
|
||||
|
||||
```cpp
|
||||
{
|
||||
std::unique_lock lock(s.mtx);
|
||||
s.waiting.erase(idx);
|
||||
}
|
||||
// lock released here — no shared state access beyond this point
|
||||
```
|
||||
|
||||
Use scoped locks. Never lock/unlock manually. Never hold a lock across a
|
||||
condition variable wait without understanding why. If you think you need
|
||||
`memory_order_release`, you probably need `memory_order_acq_rel` and you
|
||||
should document why in a three-line comment above the operation.
|
||||
|
||||
If a data structure is touched by multiple threads, its access pattern must
|
||||
be documented at the declaration site, not in a PR description. "This is
|
||||
only written under the lock, read atomically elsewhere" goes in the header.
|
||||
|
||||
---
|
||||
|
||||
## What clang-tidy enforces
|
||||
|
||||
We run with `-Wall -Wextra -Wpedantic` and a `.clang-tidy` config. Zero
|
||||
warnings. Not "zero warnings except for that one file." Zero.
|
||||
|
||||
The following are non-negotiable:
|
||||
- Every `if`/`for`/`while` body has braces
|
||||
- `auto` variables that are never modified are `const auto`
|
||||
- Variables are initialized at declaration
|
||||
- No unused includes
|
||||
- No redundant declarations
|
||||
|
||||
If clang-tidy suggests a fix and you disagree, you're wrong. Apply the fix.
|
||||
The only acceptable override is `// NOLINT` with a justification comment —
|
||||
and if you write that more than twice in a file, the reviewer will ask you to
|
||||
rethink your design.
|
||||
|
||||
---
|
||||
|
||||
## What we reject
|
||||
|
||||
- **Comments that narrate the code.** `// Increment counter` above `i++`
|
||||
is an insult. Delete it.
|
||||
|
||||
- **Dead code.** No commented-out blocks. No `#if 0`. If it's not used,
|
||||
it doesn't exist. Git remembers.
|
||||
|
||||
- **Premature abstraction.** Three identical lines do not need a
|
||||
function. Ten do. The threshold is somewhere in between and you should
|
||||
err on the side of duplication.
|
||||
|
||||
- **C heritage.** `printf`, `malloc`, `NULL`, raw `char*` strings,
|
||||
`#define` constants. The 1970s called. Don't answer.
|
||||
|
||||
- **Over-engineering.** The build backend doesn't need a plugin
|
||||
architecture. The lexer doesn't need a state machine framework.
|
||||
Solve the problem in front of you, not the one you imagine someone
|
||||
might have in three years.
|
||||
|
||||
- **Cleverness.** If your solution makes you feel smart, it's wrong.
|
||||
The best code is the code you forget about because it never breaks.
|
||||
|
||||
---
|
||||
|
||||
Kappa does one thing: build your system from source, init-agnostically.
|
||||
Everything in this style guide exists to keep that codebase small, fast, and
|
||||
comprehensible. If a rule conflicts with that goal, the goal wins — but
|
||||
you'd better have a good reason, and you'd better write it down.
|
||||
Reference in New Issue
Block a user