Skip to content

packs: light pack builds end-to-end — don't emit @import/module dep for module-less dir-scan packs #482

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
apotema merged 2 commits into from
Jul 1, 2026
Merged

packs: light pack builds end-to-end — don't emit @import/module dep for module-less dir-scan packs #482

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f799612
packs: light pack builds end-to-end — filter module-less packs from m…
apotema Jul 1, 2026
a4ad56f
packs: reject pack.labelle on a decl-module plugin (CodeRabbit #482)
apotema Jul 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
201 changes: 201 additions & 0 deletions src/plugin_manifest.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,41 @@ pub fn loadPackFromDir(
return error.PackManifestUnknownVersion;
}

// ── Mutual-exclusivity guard: pack XOR decl-module plugin (CodeRabbit, #481) ─
//
// A `pack.labelle` marks a *light pack*: a module-less, directory-scanned
// convention bundle whose entire contribution is its scanned/namespaced
// registry entries. A *decl-module plugin* is the opposite: a real Zig
// package that the generated build wires in as a module and `main.zig`
// reaches via `@import("<name>")`. The `declModulePlugins` filter in
// root.zig treats "carries pack.labelle" as the sole light-pack predicate
// and DROPS every such plugin from the module wiring. If a plugin
// erroneously carried BOTH a `pack.labelle` AND decl-module content, it
// would be silently stripped of its module wiring — a confusing failure.
//
// The two forms are mutually exclusive by design (a pack is module-less),
// so reject the conflict here, at load time, before `pack_entries` is even
// built — that keeps `declModulePlugins` fed a clean either/or split.
//
// Detection basis: presence of ANY decl-module signal in the same dir —
// - `plugin.labelle` — the decl-module plugin manifest (the clearest
// signal; the two manifests must never coexist), OR
// - `build.zig` — a decl-module plugin ships a build script; a light
// pack ships none, OR
// - `src/root.zig` — the importable module's exports.
// Any one is sufficient; a clean light pack has none of them.
if (try packDirHasDeclModuleContent(allocator, pack_dir)) {
std.log.warn(
"labelle: pack '{s}' at {s} carries a pack.labelle but ALSO ships decl-module content\n" ++
" (a plugin.labelle, a build.zig, or a src/root.zig).\n" ++
" A light pack (pack.labelle) and a decl-module plugin are mutually exclusive:\n" ++
" a pack is module-less and contributes only its scanned convention dirs.\n" ++
" Use ONE form — either a pack.labelle OR a decl-module plugin, not both.\n",
.{ expected_name, pack_dir },
);
return error.PackAndPluginManifestConflict;
}

return PackManifest{
.name = parsed.name,
.manifest_version = parsed.manifest_version,
Expand All @@ -269,6 +304,10 @@ pub fn loadPackFromDir(
// error.PluginManifestUnsafeDirName — convention_dir name is not a safe relative segment
// error.PluginManifestMissingExtension — copy_and_scan entry omitted its required extension
// error.PluginManifestUnknownVersion — manifest_version is < 1 or > what we support
//
// The pack-manifest path (`loadPackFromDir`) additionally raises:
// error.PackAndPluginManifestConflict — a pack.labelle dir ALSO ships
// decl-module content (mutually exclusive)

/// Read and parse `plugin.labelle` for the given plugin if it exists.
///
Expand Down Expand Up @@ -432,6 +471,40 @@ pub fn loadFromDir(
};
}

/// Returns true iff `pack_dir` contains any decl-module-plugin signal,
/// i.e. content that only a real Zig-package plugin (not a light pack)
/// would ship. Used by the pack/plugin mutual-exclusivity guard.
///
/// Signals (any one is sufficient):
/// - `plugin.labelle` — the decl-module plugin manifest
/// - `build.zig` — a plugin's build script
/// - `src/root.zig` — the importable module's exports
///
/// A clean light pack has none of these; it ships only convention dirs
/// (`components/ events/ prefabs/ hooks/`) and its `pack.labelle`.
pub fn packDirHasDeclModuleContent(allocator: std.mem.Allocator, pack_dir: []const u8) !bool {
const cwd = std.Io.Dir.cwd();
const io = config.globalIo();
const signals = [_][]const u8{
"plugin.labelle",
"build.zig",
"src" ++ std.fs.path.sep_str ++ "root.zig",
};
for (signals) |rel| {
const path = try std.fs.path.join(allocator, &.{ pack_dir, rel });
defer allocator.free(path);
if (cwd.access(io, path, .{})) |_| {
return true;
} else |err| switch (err) {
error.FileNotFound => {},
// Any other access error (permissions, etc.) is treated as
// "no reliable signal" rather than failing the whole generate.
else => {},
}
}
return false;
}

pub fn isReservedDirName(name: []const u8) bool {
for (RESERVED_DIR_NAMES) |reserved| {
if (std.mem.eql(u8, name, reserved)) return true;
Expand Down Expand Up @@ -1154,3 +1227,131 @@ test "loadPackFromDir: errors on unsupported manifest_version" {
const result = loadPackFromDir(testing.allocator, tmp_path, "citizens");
try testing.expectError(error.PackManifestUnknownVersion, result);
}

// ── pack XOR decl-module plugin mutual-exclusivity guard (#481) ─────

fn writeFileIn(tmp_dir: std.Io.Dir, name: []const u8, body: []const u8) !void {
var f = try tmp_dir.createFile(testing.io, name, .{});
defer f.close(testing.io);
try f.writeStreamingAll(testing.io, body);
}

test "loadPackFromDir: rejects a pack.labelle dir that also has plugin.labelle" {
var tmp = testing.tmpDir(.{});
defer tmp.cleanup();

try writePackManifestFile(tmp.dir,
\\.{
\\ .name = "citizens",
\\ .manifest_version = 1,
\\ .convention_dirs = .copy_and_scan,
\\}
);
// Decl-module signal: a plugin.labelle alongside the pack.labelle.
try writeFileIn(tmp.dir, "plugin.labelle",
\\.{ .name = "citizens", .manifest_version = 1 }
);

const tmp_path = try tmp.dir.realPathFileAlloc(testing.io, ".", testing.allocator);
defer testing.allocator.free(tmp_path);

const result = loadPackFromDir(testing.allocator, tmp_path, "citizens");
try testing.expectError(error.PackAndPluginManifestConflict, result);
}

test "loadPackFromDir: rejects a pack.labelle dir that also has build.zig" {
var tmp = testing.tmpDir(.{});
defer tmp.cleanup();

try writePackManifestFile(tmp.dir,
\\.{
\\ .name = "citizens",
\\ .manifest_version = 1,
\\ .convention_dirs = .copy_and_scan,
\\}
);
// Decl-module signal: a build.zig means a real Zig package, not a pack.
try writeFileIn(tmp.dir, "build.zig",
\\pub fn build(b: *@import("std").Build) void { _ = b; }
);

const tmp_path = try tmp.dir.realPathFileAlloc(testing.io, ".", testing.allocator);
defer testing.allocator.free(tmp_path);

const result = loadPackFromDir(testing.allocator, tmp_path, "citizens");
try testing.expectError(error.PackAndPluginManifestConflict, result);
}

test "loadPackFromDir: rejects a pack.labelle dir that also has src/root.zig" {
var tmp = testing.tmpDir(.{});
defer tmp.cleanup();

try writePackManifestFile(tmp.dir,
\\.{
\\ .name = "citizens",
\\ .manifest_version = 1,
\\ .convention_dirs = .copy_and_scan,
\\}
);
// Decl-module signal: an importable src/root.zig.
try tmp.dir.createDirPath(testing.io, "src");
var src_dir = try tmp.dir.openDir(testing.io, "src", .{});
defer src_dir.close(testing.io);
try writeFileIn(src_dir, "root.zig",
\\pub const marker = 1;
);

const tmp_path = try tmp.dir.realPathFileAlloc(testing.io, ".", testing.allocator);
defer testing.allocator.free(tmp_path);

const result = loadPackFromDir(testing.allocator, tmp_path, "citizens");
try testing.expectError(error.PackAndPluginManifestConflict, result);
}

test "loadPackFromDir: a clean light pack (no decl-module content) still loads" {
var tmp = testing.tmpDir(.{});
defer tmp.cleanup();

// Only a pack.labelle + a convention dir — the genuine light-pack shape.
try writePackManifestFile(tmp.dir,
\\.{
\\ .name = "citizens",
\\ .manifest_version = 1,
\\ .convention_dirs = .copy_and_scan,
\\}
);
try tmp.dir.createDirPath(testing.io, "components");

const tmp_path = try tmp.dir.realPathFileAlloc(testing.io, ".", testing.allocator);
defer testing.allocator.free(tmp_path);

var manifest = (try loadPackFromDir(testing.allocator, tmp_path, "citizens")).?;
defer manifest.deinit();

try testing.expectEqualStrings("citizens", manifest.name);
}

test "loadFromDir: a clean decl-module plugin (no pack.labelle) still loads" {
// The mutual-exclusivity guard lives on the pack path; a decl-module
// plugin with a build.zig + src/root.zig but NO pack.labelle loads fine
// through the plugin.labelle path.
Comment on lines +1335 to +1337
var tmp = testing.tmpDir(.{});
defer tmp.cleanup();

try writeManifestFile(tmp.dir,
\\.{ .name = "fsm", .manifest_version = 1 }
);
try writeFileIn(tmp.dir, "build.zig",
\\pub fn build(b: *@import("std").Build) void { _ = b; }
);

const tmp_path = try tmp.dir.realPathFileAlloc(testing.io, ".", testing.allocator);
defer testing.allocator.free(tmp_path);

var manifest = (try loadFromDir(testing.allocator, tmp_path, "fsm")).?;
defer manifest.deinit();
try testing.expectEqualStrings("fsm", manifest.name);

// And it must NOT parse as a pack (no pack.labelle present).
try testing.expect((try loadPackFromDir(testing.allocator, tmp_path, "fsm")) == null);
}
101 changes: 96 additions & 5 deletions src/root.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,46 @@ fn scanPackSubdir(
return scanner.copyAndScanAbs(allocator, src, dst, ext);
}

/// Select the decl-module subset of `plugins` (labelle-assembler#481).
///
/// A game's `.plugins` list mixes two kinds of dependency:
///
/// - **decl-module plugins** — a real Zig package (`plugin.labelle` +
/// `src/root.zig` exports) that the generated build wires in as a module
/// and the generated `main.zig` reaches via `@import("<name>")`.
/// - **light packs** — module-less, directory-scanned convention bundles
/// (they carry `pack.labelle`, so their name appears in `pack_names`).
/// A light pack has NO `build.zig` and NO importable module; its entire
/// contribution is the already-scanned + `<pack>__`-namespaced registry
/// entries (#439/#440). Emitting `@import("<pack>")` or a
/// `b.dependency("labelle_<pack>", …)` for one fails to resolve and breaks
/// `labelle build` — the gap this ticket closes.
///
/// Returns the subset of `plugins` whose name is NOT in `pack_names`, i.e. the
/// plugins that DO ship an importable module. The order of `plugins` is
/// preserved. The returned slice is caller-owned (`allocator.free`); the
/// `PluginDep` elements are shallow copies that alias the input's strings.
pub fn declModulePlugins(
allocator: std.mem.Allocator,
plugins: []const config.PluginDep,
pack_names: []const []const u8,
) ![]config.PluginDep {
var out: std.ArrayList(config.PluginDep) = .empty;
errdefer out.deinit(allocator);
try out.ensureTotalCapacity(allocator, plugins.len);
for (plugins) |plugin| {
var is_light_pack = false;
for (pack_names) |pack_name| {
if (std.mem.eql(u8, pack_name, plugin.name)) {
is_light_pack = true;
break;
}
}
if (!is_light_pack) out.appendAssumeCapacity(plugin);
}
return out.toOwnedSlice(allocator);
}

/// Scan a pack's convention subdirs (Packs RFC §4, labelle-assembler#439).
///
/// Copies `<pack_src_dir>/{components,events,prefabs,hooks}/` into
Expand Down Expand Up @@ -1272,6 +1312,42 @@ pub fn generate(
pack_scans.items,
);

// ── Module-plugin filter — light packs are dir-scan-only (#481) ────
//
// A *light pack* (module-less, carrying `pack.labelle`) contributes to
// the build ONLY via the dir-scan registry entries collected above
// (`pack_scans`): its components/events/prefabs/hooks are already
// copied, `<pack>__`-namespaced (#440), validated (#441), and wired
// into the SAME unified registries the game root feeds (#439). It ships
// NO importable Zig module and has NO `build.zig`, so it must NOT appear
// in any codegen/build site that emits `@import("<name>")` or a
// `b.dependency("labelle_<name>", …)` — the ComponentRegistryWithPlugins
// / SystemRegistry args, plugin controllers/events, the build.zig module
// graph, and the build.zig.zon deps. Emitting either for a module-less
// pack fails to resolve and breaks `labelle build` (the #481 gap).
//
// Only genuine decl-module plugins (a `plugin.labelle` with `src/root.zig`
// exports) get the module import + build dep. Every light pack carries a
// `pack.labelle` and therefore appears in `pack_entries` (parsed near the
// top of `generate()`), so membership there IS the light-pack predicate.
//
// Build a filtered plugin list and run the module-emitting phase against
// a `cfg` copy whose `.plugins` excludes light packs (`cfg_modules`). The
// scan/copy loops that need the FULL declared list already ran above, and
// the project-description sidecars (flow catalog, feature manifest) keep
// the original `cfg` so they still describe every declared plugin/pack.
var pack_names_for_filter: std.ArrayList([]const u8) = .empty;
defer pack_names_for_filter.deinit(allocator);
try pack_names_for_filter.ensureTotalCapacity(allocator, pack_entries.items.len);
for (pack_entries.items) |e| pack_names_for_filter.appendAssumeCapacity(e.plugin.name);

const module_plugins = try declModulePlugins(allocator, cfg.plugins, pack_names_for_filter.items);
Comment on lines +1339 to +1344

@gemini-code-assist gemini-code-assist Bot Jul 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Since the exact number of pack entries (pack_entries.items.len) is known beforehand, we can allocate a slice directly using allocator.alloc instead of using a dynamic std.ArrayList. This simplifies the code, improves readability, and avoids the overhead of ArrayList capacity tracking.

    const pack_names_for_filter = try allocator.alloc([]const u8, pack_entries.items.len);
    defer allocator.free(pack_names_for_filter);
    for (pack_entries.items, 0..) |e, i| pack_names_for_filter[i] = e.plugin.name;

    const module_plugins = try declModulePlugins(allocator, cfg.plugins, pack_names_for_filter);

defer allocator.free(module_plugins);
// A `cfg` view whose `.plugins` is the decl-module subset. Passed to every
// site that emits a Zig-module import or a build-file module dependency.
var cfg_modules = cfg;
cfg_modules.plugins = module_plugins;
Comment on lines +1339 to +1349

@coderabbitai coderabbitai Bot Jul 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Look for how pack.labelle vs plugin.labelle / module presence are distinguished.
fd -t f 'plugin_manifest.zig' -x cat -n {}
rg -nP -C3 'loadPackOptional|pack\.labelle|plugin\.labelle' src/

Repository: labelle-toolkit/labelle-assembler

Length of output: 50389

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the module-wiring logic around declModulePlugins and its callers.
ast-grep outline src/root.zig --view expanded | sed -n '1,260p'

printf '\n--- declModulePlugins references ---\n'
rg -n -C 4 'declModulePlugins|module_plugins|pack_entries|loadPackOptional|loadOptional|b\.dependency|`@import`' src/root.zig

Repository: labelle-toolkit/labelle-assembler

Length of output: 23742

Reject pack.labelle on decl-module plugins
pack_entries is the light-pack set, so any plugin with a pack.labelle is filtered out of declModulePlugins and loses its @import/b.dependency wiring. Add a guard here or in the manifest loader if both manifests are meant to be mutually exclusive.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/root.zig` around lines 1339 - 1349, The decl-module plugin selection is
currently using pack entries that may include light-pack-only plugins, so any
plugin with a pack.labelle gets dropped from declModulePlugins and loses
import/dependency wiring. Add an explicit rejection or validation in root.zig
around declModulePlugins or in the manifest-loading path so decl-module plugins
cannot carry pack.labelle, and ensure cfg_modules only receives the mutually
exclusive decl-module subset.


// ── Flow-node discovery (RFC-FLOW-VOCABULARY phase 2) ──────────────
//
// Discover `pub const FlowNodes` + `pub const PinStyles` +
Expand Down Expand Up @@ -1301,7 +1377,9 @@ pub fn generate(
defer allocator.free(scripts_target_for_flow);
var plugin_flow_decls = try main_zig.discoverPluginFlowDecls(
allocator,
cfg,
// Light packs contribute no decl-module `FlowNodes` (they have no
// `src/root.zig`); walk only the decl-module plugins (#481).
cfg_modules,
game_dir,
scripts_target_for_flow,
script_scan.getEntries(),
Expand All @@ -1327,7 +1405,9 @@ pub fn generate(
defer flow_result.deinit();

// Generate build.zig.zon
const zon = try build_files.generateBuildZigZon(allocator, cfg, target_dir, output_dir, game_dir, .{
// `cfg_modules` (not `cfg`): a light pack has no `build.zig`/module, so it
// must not become a `.labelle_<name> = .{ .path }` dep (#481).
const zon = try build_files.generateBuildZigZon(allocator, cfg_modules, target_dir, output_dir, game_dir, .{
// The tests target runs second — additive merge so the exe
// target's deps (chosen-backend, plugins) survive. Issue #83.
.recreate_deps = !is_tests_target,
Expand All @@ -1348,7 +1428,10 @@ pub fn generate(
defer main_zig.freePromotedScripts(allocator, promoted_scripts);

// Generate build.zig
const build_zig = try build_files.generateBuildZig(allocator, cfg, .{
// `cfg_modules` (not `cfg`): light packs get no `b.dependency` /
// `overrideImport` module wiring — their contribution is dir-scan
// registry entries, not an importable module (#481).
const build_zig = try build_files.generateBuildZig(allocator, cfg_modules, .{
.is_tests_target = is_tests_target,
.promoted_scripts = promoted_scripts,
// Manifest-driven backend splice (assembler#378): pass the project
Expand Down Expand Up @@ -1379,7 +1462,11 @@ pub fn generate(
// the file header on `main_zig.writePluginEventsBlock` for why the
// builtin `@Union(.auto, …)` path was retired (zero-field result is
// uninstantiable, breaks the plugin-controllers example).
var plugin_events = try main_zig.discoverPluginEvents(allocator, cfg, game_dir);
// `cfg_modules`: only decl-module plugins have a `src/root.zig` with an
// `Events` block; a light pack's events arrive via `pack_scans`, already
// namespaced, so it contributes nothing here and must not be resolved as a
// module (#481).
var plugin_events = try main_zig.discoverPluginEvents(allocator, cfg_modules, game_dir);
defer plugin_events.deinit();

// Emit the `game.zig` shim — a tiny re-export module that surfaces
Expand Down Expand Up @@ -1524,7 +1611,11 @@ pub fn generate(
const main_zig_content = try main_zig.generateMainZigFromTemplate(
allocator,
engine_template,
cfg,
// `cfg_modules`: the plugin `@import` sites inside main.zig
// (ComponentRegistryWithPlugins / SystemRegistry / plugin
// controllers) must skip light packs — their items are wired in
// via `main_template.pack_scans` above, not a module import (#481).
cfg_modules,
backend_tmpl,
merged_entries,
prefab_names,
Expand Down
Loading
Loading