Skip to content

fix: run the version floors in generate/check/upgrade; normalize the bgfx remote; floor the 2.x core line (#739, #742) #746

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
Sep 18, 2026
Merged

fix: run the version floors in generate/check/upgrade; normalize the bgfx remote; floor the 2.x core line (#739, #742) #746

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
8387444
fix: apply the version floors in generate/check/upgrade, normalize th…
apotema Sep 18, 2026
582d0c1
fix: identify the provider from the package, unify remote normalizati…
apotema Sep 18, 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
25 changes: 13 additions & 12 deletions src/cache/fetch.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,19 +326,16 @@ fn archiveFetch(allocator: std.mem.Allocator, repo: []const u8, ref: []const u8,

/// Normalize a clone URL / host-path to a bare `host/owner/repo`: strip a
/// `git+` prefix, the scheme, any `?query`/`#fragment`, a trailing `.git`, and
/// a trailing slash. Idempotent for values already in host-path form.
/// trailing slashes. Idempotent for values already in host-path form.
///
/// Delegates to `config.normalizeRemote` so the identity used to CLONE and
/// the identity the version-floor / official-provider gates COMPARE are the
/// same function (#746 review). They used to be two copies that had drifted:
/// this one stripped `git+` and `?query`/`#fragment`, the gate's did not, so
/// `git+https://github.com/labelle-toolkit/labelle-bgfx?ref=main` fetched the
/// official backend and was then judged a custom provider.
fn repoHostPath(allocator: std.mem.Allocator, url: []const u8) ![]u8 {
var s = url;
if (std.mem.startsWith(u8, s, "git+")) s = s["git+".len..];
if (std.mem.startsWith(u8, s, "https://")) {
s = s["https://".len..];
} else if (std.mem.startsWith(u8, s, "http://")) {
s = s["http://".len..];
}
if (std.mem.indexOfAny(u8, s, "?#")) |i| s = s[0..i];
if (std.mem.endsWith(u8, s, ".git")) s = s[0 .. s.len - ".git".len];
if (std.mem.endsWith(u8, s, "/")) s = s[0 .. s.len - 1];
return allocator.dupe(u8, s);
return allocator.dupe(u8, config.normalizeRemote(url));
}

/// Filesystem-safe slug for a temp filename: non-alphanumerics → `-`.
Expand Down Expand Up @@ -427,6 +424,10 @@ test "repoHostPath: normalizes clone URLs and host-paths" {
.{ "https://github.com/labelle-toolkit/labelle-core.git", "github.com/labelle-toolkit/labelle-core" },
.{ "git+https://github.com/x/y?ref=v1.0#abc", "github.com/x/y" },
.{ "https://codeberg.org/a/b/", "codeberg.org/a/b" },
// Now shared with `config.normalizeRemote` (#746 review): the extra
// spellings that helper already folded must survive the delegation.
.{ "git://GitHub.com/x/y.git", "GitHub.com/x/y" },
.{ "github.com/x/y/?ref=main#frag", "github.com/x/y" },
}) |case| {
const got = try repoHostPath(alloc, case[0]);
defer alloc.free(got);
Expand Down
103 changes: 79 additions & 24 deletions src/cache_cmd.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

const std = @import("std");
const gen = @import("root.zig");
const version_floors = @import("version_floors.zig");
const cache = @import("cache.zig");
const config = @import("config.zig");

Expand Down Expand Up @@ -536,10 +537,12 @@ pub fn cmdUpgrade(allocator: std.mem.Allocator, io: std.Io, args: *std.process.A
defer arena.deinit();
const arena_alloc = arena.allocator();

// Parse project.labelle up front to fail fast on a malformed file —
// the parsed config itself is no longer needed (replaceVersionField
// edits the raw text and inserts omitted fields directly).
_ = readProjectConfig(arena_alloc, io, root) catch {
// Parse project.labelle up front to fail fast on a malformed file. The
// parsed config is not needed for the REWRITE (`replaceVersionField`
// edits the raw text and inserts omitted fields directly) — it is
// needed for the floor gate below, which judges the pins as they would
// be AFTER this upgrade.
const current = readProjectConfig(arena_alloc, io, root) catch {
std.log.err("labelle-assembler upgrade: failed to read project.labelle in '{s}'", .{root});
std.process.exit(1);
};
Expand All @@ -550,14 +553,24 @@ pub fn cmdUpgrade(allocator: std.mem.Allocator, io: std.Io, args: *std.process.A
std.process.exit(1);
};

// What this upgrade WOULD write, decided before anything is rewritten:
// the floor gate below judges the RESULTING trio + backend pairing, and
// a refusal must leave project.labelle untouched (#739). `null` = this
// field is not part of this upgrade and keeps its current pin.
var next_core: ?[]const u8 = null;
var next_engine: ?[]const u8 = null;
var next_gfx: ?[]const u8 = null;
var next_cli: ?[]const u8 = null;
// The log line the command prints once the pins are accepted.
var announce_set = false;
var announce_one: ?struct { pkg: []const u8, version: []const u8 } = null;

if (positionals.items.len == 0) {
std.log.info("labelle-assembler: upgrading to compatible set (core={s}, engine={s}, gfx={s}, cli={s})", .{
gen.CORE_VERSION, gen.ENGINE_VERSION, gen.GFX_VERSION, gen.CLI_VERSION,
});
content = try replaceVersionField(arena_alloc, content, "core_version", gen.CORE_VERSION);
content = try replaceVersionField(arena_alloc, content, "engine_version", gen.ENGINE_VERSION);
content = try replaceVersionField(arena_alloc, content, "gfx_version", gen.GFX_VERSION);
content = try replaceVersionField(arena_alloc, content, "labelle_version", gen.CLI_VERSION);
announce_set = true;
next_core = gen.CORE_VERSION;
next_engine = gen.ENGINE_VERSION;
next_gfx = gen.GFX_VERSION;
next_cli = gen.CLI_VERSION;
} else {
const pkg = positionals.items[0];

Expand All @@ -581,31 +594,73 @@ pub fn cmdUpgrade(allocator: std.mem.Allocator, io: std.Io, args: *std.process.A
const version = if (positionals.items.len > 1) positionals.items[1] else default_version;

if (std.mem.eql(u8, pkg, "core")) {
content = try replaceVersionField(arena_alloc, content, "core_version", version);
next_core = version;
} else if (std.mem.eql(u8, pkg, "engine")) {
content = try replaceVersionField(arena_alloc, content, "engine_version", version);
next_engine = version;
} else if (std.mem.eql(u8, pkg, "gfx")) {
content = try replaceVersionField(arena_alloc, content, "gfx_version", version);
next_gfx = version;
} else if (std.mem.eql(u8, pkg, "labelle") or std.mem.eql(u8, pkg, "cli")) {
content = try replaceVersionField(arena_alloc, content, "labelle_version", version);
next_cli = version;
} else if (std.mem.eql(u8, pkg, "all")) {
content = try replaceVersionField(arena_alloc, content, "core_version", gen.CORE_VERSION);
content = try replaceVersionField(arena_alloc, content, "engine_version", gen.ENGINE_VERSION);
content = try replaceVersionField(arena_alloc, content, "gfx_version", gen.GFX_VERSION);
content = try replaceVersionField(arena_alloc, content, "labelle_version", gen.CLI_VERSION);
next_core = gen.CORE_VERSION;
next_engine = gen.ENGINE_VERSION;
next_gfx = gen.GFX_VERSION;
next_cli = gen.CLI_VERSION;
announce_set = true;
} else {
std.log.err("labelle-assembler upgrade: unknown package '{s}' (packages: core, engine, gfx, cli, all)", .{pkg});
std.process.exit(2);
}
if (std.mem.eql(u8, pkg, "all")) {
std.log.info("labelle-assembler: upgrading all packages to the assembler's compatible set (core={s}, engine={s}, gfx={s}, cli={s})", .{
gen.CORE_VERSION, gen.ENGINE_VERSION, gen.GFX_VERSION, gen.CLI_VERSION,
});
if (!announce_set) announce_one = .{ .pkg = pkg, .version = version };
}

// ── cross-package version floors (#739) ──────────────────────────────
// `upgrade core <ver>` can move `.core_version` BELOW the floor of the
// backend the project already pins (or below the engine/gfx it is
// paired with) — the same pairing `init` refuses at the front door,
// arrived at from the side. Judged on the PROSPECTIVE pins, so the
// refusal happens before the rewrite lands: nothing is written.
//
// The gate REFUSES only when this upgrade actually moves a
// floor-bearing pin. An upgrade that touches none of them (`upgrade
// cli`) cannot make the pairing worse, and a project that is ALREADY
// below a floor — hand-edited, or upgraded by an older assembler — must
// still be able to bump its CLI pin, so that case warns instead of
// blocking an unrelated bump. (`upgrade all` / bare `upgrade` snap the
// whole trio to the curated set, which REPAIRS such a project.)
{
var prospective = current;
if (next_core) |v| prospective.core_version = v;
if (next_engine) |v| prospective.engine_version = v;
if (next_gfx) |v| prospective.gfx_version = v;
const touches_floor_pin = next_core != null or next_engine != null or next_gfx != null;
if (touches_floor_pin) {
version_floors.enforce(prospective, "labelle-assembler upgrade") catch |err| {
std.log.err("labelle-assembler upgrade: {s} — project.labelle left unchanged", .{@errorName(err)});
std.process.exit(2);
};
} else {
std.log.info("labelle-assembler: upgrading {s} to {s}", .{ pkg, version });
const v = version_floors.verdict(prospective) catch |err| {
std.log.err("labelle-assembler upgrade: version pins: {s}", .{@errorName(err)});
std.process.exit(2);
};
if (v.refused()) std.log.warn("labelle-assembler upgrade: this project's core/engine/gfx pins are already below a hard floor and cannot build — run 'labelle-assembler upgrade all' to snap them to the compatible set (this upgrade changes none of them)", .{});
}
}

if (announce_set) {
std.log.info("labelle-assembler: upgrading to compatible set (core={s}, engine={s}, gfx={s}, cli={s})", .{
gen.CORE_VERSION, gen.ENGINE_VERSION, gen.GFX_VERSION, gen.CLI_VERSION,
});
} else if (announce_one) |a| {
std.log.info("labelle-assembler: upgrading {s} to {s}", .{ a.pkg, a.version });
}

if (next_core) |v| content = try replaceVersionField(arena_alloc, content, "core_version", v);
if (next_engine) |v| content = try replaceVersionField(arena_alloc, content, "engine_version", v);
if (next_gfx) |v| content = try replaceVersionField(arena_alloc, content, "gfx_version", v);
if (next_cli) |v| content = try replaceVersionField(arena_alloc, content, "labelle_version", v);

std.Io.Dir.cwd().writeFile(io, .{ .sub_path = labelle_path, .data = content }) catch {
std.log.err("labelle-assembler upgrade: could not write '{s}'", .{labelle_path});
std.process.exit(1);
Expand Down
14 changes: 14 additions & 0 deletions src/check_cmd.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const scan = @import("codegen/scan.zig");
const idents = @import("codegen/idents.zig");
const check = @import("check.zig");
const scene_name_lint = @import("scene_name_lint.zig");
const version_floors = @import("version_floors.zig");

const ProjectConfig = config.ProjectConfig;

Expand Down Expand Up @@ -89,6 +90,19 @@ pub fn cmdCheck(allocator: std.mem.Allocator, io: std.Io, args: *std.process.Arg
defer arena_state.deinit();
const arena = arena_state.allocator();

// ── cross-package version floors (#739) ──────────────────────────────
// `check` is the OTHER door that resolves the same backend/core pairing
// `generate` does, and it is what CI runs first — so it must refuse the
// same pairings `generate` refuses, with the same diagnostic, rather
// than passing a project that cannot build. Parse failures are left to
// `runLint` below, which reports them with its own message.
if (readProjectConfig(arena, io, root)) |cfg| {
version_floors.enforce(cfg, "labelle-assembler check") catch |err| {
std.log.err("labelle-assembler check: {s} (in '{s}')", .{ @errorName(err), root });
std.process.exit(2);
};
} else |_| {}

const result = runLint(arena, io, root) catch |err| {
std.log.err("labelle-assembler check: {s} (in '{s}')", .{ @errorName(err), root });
std.process.exit(1);
Expand Down
109 changes: 109 additions & 0 deletions src/config.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,115 @@ pub fn pinAtLeast(pin: []const u8, floor: []const u8) error{UnparsableVersionPin
return p.order(f) != .lt;
}

/// The comparable identity of a package `repo` field: the remote with any
/// `git+` prefix, its scheme, any `?query`/`#fragment`, any trailing `/` and
/// a trailing `.git` removed.
///
/// `parseProjectConfig` preserves `PluginDep.repo` VERBATIM, and the fetch
/// path accepts every spelling of the same remote —
/// `github.com/labelle-toolkit/labelle-bgfx`,
/// `https://github.com/labelle-toolkit/labelle-bgfx` and either with a
/// `.git` suffix all clone the same repository. Any gate that asks "is this
/// the official provider?" must therefore compare the NORMALIZED form
/// (#742): comparing the raw string treated the `https://…/….git` spelling
/// of the official repo as a third-party provider and skipped the floor
/// checks that exist to name a bad pin.
///
/// THE SINGLE NORMALIZER (#746 review). `cache/fetch.zig`'s `repoHostPath`
/// — the function that decides which repository is actually CLONED — used
/// to carry its own copy of this logic, and the copies had drifted: the
/// fetcher stripped `git+` and `?query`/`#fragment`, this one did not. So
/// `git+https://github.com/labelle-toolkit/labelle-bgfx?ref=main` FETCHED
/// the official backend while every floor gate classified it as a custom
/// provider and waved incompatible release pins through. `repoHostPath` now
/// delegates here, so the identity a gate compares is by construction the
/// identity the fetcher resolves.
///
/// A `local:` path is NOT a remote — it is a filesystem checkout resolved
/// elsewhere — so it is returned untouched (scheme stripping would mangle
/// it, and two different local paths must never compare equal to a remote).
pub fn normalizeRemote(repo: []const u8) []const u8 {
if (std.mem.startsWith(u8, repo, "local:")) return repo;
var r = repo;
if (std.ascii.startsWithIgnoreCase(r, "git+")) r = r["git+".len..];
inline for (.{ "https://", "http://", "git://", "ssh://" }) |scheme| {
if (std.ascii.startsWithIgnoreCase(r, scheme)) {
r = r[scheme.len..];
break;
}
}
// A `?ref=`/`#sha` suffix selects a REVISION of the same repository, so
// it is not part of the repository's identity — and the fetch path drops
// it before cloning.
if (std.mem.indexOfAny(u8, r, "?#")) |i| r = r[0..i];
while (r.len > 0 and r[r.len - 1] == '/') r = r[0 .. r.len - 1];
if (std.mem.endsWith(u8, r, ".git")) r = r[0 .. r.len - ".git".len];
return r;
Comment thread
apotema marked this conversation as resolved.
Resolved
}

/// Two `repo` fields naming the same repository. Host is compared
/// case-INSENSITIVELY (DNS is case-folding); the path after it is compared
/// exactly, because forge paths are case-sensitive on every forge the
/// registry talks to — folding it would make `labelle-toolkit/Foo` and
/// `labelle-toolkit/foo` the same repo when they are not. `local:` paths
/// compare verbatim.
pub fn sameRemote(a: []const u8, b: []const u8) bool {
const x = normalizeRemote(a);
const y = normalizeRemote(b);
if (std.mem.startsWith(u8, x, "local:") or std.mem.startsWith(u8, y, "local:")) return std.mem.eql(u8, x, y);
const xs = std.mem.indexOfScalar(u8, x, '/') orelse x.len;
const ys = std.mem.indexOfScalar(u8, y, '/') orelse y.len;
if (!std.ascii.eqlIgnoreCase(x[0..xs], y[0..ys])) return false;
return std.mem.eql(u8, x[xs..], y[ys..]);
}

test "normalizeRemote/sameRemote fold every spelling of one remote, keep distinct repos distinct — #742" {
// The mechanism: the SPELLING is normalized away, so the official-repo
// gate sees one identity for all four forms the fetch path accepts.
const canonical = "github.com/labelle-toolkit/labelle-bgfx";
inline for (.{
"github.com/labelle-toolkit/labelle-bgfx",
"https://github.com/labelle-toolkit/labelle-bgfx",
"https://github.com/labelle-toolkit/labelle-bgfx.git",
"github.com/labelle-toolkit/labelle-bgfx.git",
"http://github.com/labelle-toolkit/labelle-bgfx/",
"git://GitHub.com/labelle-toolkit/labelle-bgfx.git",
"GITHUB.COM/labelle-toolkit/labelle-bgfx",
// A `?ref=`/`#sha` suffix selects a REVISION, not a repository, and
// `cache/fetch.zig` drops it before cloning — so the gates must fold
// it away too, or a `git+https://…?ref=main` spelling FETCHES the
// official backend while every floor check calls it third-party
// (#746 review).
"git+https://github.com/labelle-toolkit/labelle-bgfx?ref=main",
"git+https://github.com/labelle-toolkit/labelle-bgfx.git?ref=v0.21.0",
"https://github.com/labelle-toolkit/labelle-bgfx#deadbeef",
"git+https://github.com/labelle-toolkit/labelle-bgfx/?ref=main#frag",
}) |spelling| {
try std.testing.expect(sameRemote(spelling, canonical));
try std.testing.expect(sameRemote(canonical, spelling));
}
try std.testing.expectEqualStrings(canonical, normalizeRemote("https://github.com/labelle-toolkit/labelle-bgfx.git"));

// A genuinely different repo stays different — including a fork with the
// same trailing name, a case-differing PATH, and another host.
inline for (.{
"github.com/acme/labelle-bgfx",
"https://github.com/acme/labelle-bgfx.git",
"github.com/labelle-toolkit/labelle-bgfx-fork",
"github.com/labelle-toolkit/Labelle-Bgfx",
"gitlab.com/labelle-toolkit/labelle-bgfx",
}) |other| {
try std.testing.expect(!sameRemote(other, canonical));
}

// `local:` is a checkout path, not a remote: untouched, and never equal
// to a remote that happens to normalize similarly.
try std.testing.expectEqualStrings("local:../labelle-bgfx.git", normalizeRemote("local:../labelle-bgfx.git"));
try std.testing.expect(sameRemote("local:../labelle-bgfx", "local:../labelle-bgfx"));
try std.testing.expect(!sameRemote("local:../labelle-bgfx", "local:../other"));
try std.testing.expect(!sameRemote("local:github.com/labelle-toolkit/labelle-bgfx", canonical));
}

/// Map a package `version` string to the git ref to clone.
///
/// A semver-shaped version (`1.2.3`) maps to the published release tag
Expand Down
Loading
Loading