Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions scripts/regressions/tap-resolution-contract.sh
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@
# Allowed sites:
# - src/core/forge.zig the host-shaped seam (URLs, parse, auth)
# - src/core/tap.zig the row/orchestration caller of the seam
# - src/tap_slug.zig the canonical-identity leaf: strip and
# re-prefix live together so they cannot
# drift, and a leaf is the only tier both
# core/ and db/ (the repair migration) can
# import
# - src/cli/migrate/keg.zig on-disk Cellar path
# (<prefix>/Library/Taps/<user>/homebrew-<repo>) —
# Homebrew's filesystem layout, not a URL
Expand Down Expand Up @@ -59,6 +64,7 @@ bs=$'\\'
filtered=$(printf '%s\n' "$all" |
grep -v '^src/core/forge\.zig:' |
grep -v '^src/core/tap\.zig:' |
grep -v '^src/tap_slug\.zig:' |
grep -v '^src/cli/migrate/keg\.zig:' |
grep -v '^src/net/client\.zig:' |
grep -vE ":[0-9]+:[[:space:]]*${bs}${bs}" |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
#!/usr/bin/env bash
# Lock tap-slug canonicalisation: every spelling of a tap is one tap.
#
# Homebrew downcases a tap reference and strips a leading `homebrew-`
# from the repo component before addressing the git repo as
# `homebrew-<repo>`. malt used to keep the user's spelling verbatim, so
# a slug that already carried the prefix resolved to
# `github.com/<user>/homebrew-homebrew-<repo>` (404), and the same
# missing canonicalisation split one tap's identity across two rows.
#
# Pinned behaviour:
# 1. Resolve — a `homebrew-`-prefixed slug reaches the `.rb` probe
# instead of 404ing on the tap repo. Keyed on error *text*, not exit
# code: both the broken and fixed runs exit non-zero on a formula
# that does not exist; only the broken one blames the repo.
# 2. Identity — registering one spelling and removing another leaves
# no row behind. `untap` is silent about a miss, so the registry
# listing is the assertion.
#
# Uses a deliberately absent formula, so nothing is installed and a
# single API round trip covers the whole run. Throwaway MALT_PREFIX,
# never the live one.
#
# Usage: scripts/regressions/tap-slug-canonicalisation-myx-installation-fails.sh
# Requirements: a built malt binary; one GitHub API call.

set -euo pipefail

ROOT=$(cd "$(dirname "$0")/../.." && pwd)
cd "$ROOT"

MALT_BIN=${MALT_BIN:-$ROOT/zig-out/bin/malt}
if [[ ! -x "$MALT_BIN" ]]; then
printf 'FAIL: malt binary not found at %s — run "zig build" first.\n' "$MALT_BIN" >&2
exit 1
fi

# The override must name a tap whose slug ALREADY carries `homebrew-`.
TAP_PREFIXED=${MALT_TAP_REGRESSION:-indaco/homebrew-tap}
TAP_BARE=${TAP_PREFIXED/\/homebrew-//}

PREFIX=$(mktemp -d)
trap 'rm -rf "$PREFIX"' EXIT
export MALT_PREFIX="$PREFIX"
export MALT_GITHUB_TOKEN=${MALT_GITHUB_TOKEN:-$(gh auth token 2>/dev/null || true)}

# Skip loud rather than fail: the anonymous 60/hr cap makes an untokened
# run indistinguishable from the bug. Same gate as tap_head_etag_304.sh.
if [[ -z "$MALT_GITHUB_TOKEN" ]]; then
printf 'SKIP: MALT_GITHUB_TOKEN unset — a rate-limited resolve is\n' >&2
printf ' indistinguishable from the 404 this test looks for.\n' >&2
exit 0
fi

# (1) resolve
out=$("$MALT_BIN" install --dry-run "$TAP_PREFIXED/nope-not-a-formula" 2>&1 || true)
# An environment failure is not a regression — say so and stop.
if grep -qE 'rate limit reached|Network failure' <<<"$out"; then
printf 'SKIP: GitHub unreachable or rate-limited:\n%s\n' "$out" >&2
exit 0
fi
if grep -q '404 for the tap repo' <<<"$out"; then
printf "FAIL: '%s' still resolves to a doubled homebrew- prefix:\n\n" "$TAP_PREFIXED" >&2
printf '%s\n' "$out" >&2
exit 1
fi
if ! grep -q 'Tap formula/cask not found' <<<"$out"; then
printf 'FAIL: unexpected resolve output for %s:\n\n' "$TAP_PREFIXED" >&2
printf '%s\n' "$out" >&2
exit 1
fi

# (2) identity
if ! reg_out=$("$MALT_BIN" tap "$TAP_BARE" 2>&1); then
printf 'SKIP: could not register %s:\n%s\n' "$TAP_BARE" "$reg_out" >&2
exit 0
fi
"$MALT_BIN" untap "$TAP_PREFIXED" >/dev/null 2>&1 || true
listing=$("$MALT_BIN" tap --list 2>&1 || true)
if grep -qi -- "$TAP_BARE" <<<"$listing"; then
printf "FAIL: '%s' survived untap of '%s' — two rows for one tap:\n\n" \
"$TAP_BARE" "$TAP_PREFIXED" >&2
printf '%s\n' "$listing" >&2
exit 1
fi

echo "OK: prefixed and bare spellings resolve alike and share one tap identity"
13 changes: 6 additions & 7 deletions scripts/regressions/tap_no_inline_homebrew_synthesis.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
# else.
#
# Allowed sites:
# - src/core/tap.zig single-point fallback in `effectiveOwnerRepo`
# (the cold path during initial `mt tap`).
# - src/db/schema.zig v8→v9 migration backfill needs the prefix
# literal to derive (owner, "homebrew-" || repo)
# from the existing slug.
# - src/tap_slug.zig the one synthesis, beside the prefix strip
# it has to stay consistent with. Both the
# cold-path fallback in `effectiveOwnerRepo`
# and the repair migration call it, which is
# why it sits in a leaf.
# - src/cli/migrate/keg.zig `<prefix>/Library/Taps/<user>/homebrew-<repo>`
# on-disk path — Homebrew's filesystem layout,
# not a URL.
Expand All @@ -29,8 +29,7 @@ cd "$ROOT"
hits=$(grep -rn --include='*.zig' -F 'homebrew-{' src || true)

filtered=$(printf '%s\n' "$hits" |
grep -v '^src/core/tap\.zig:' |
grep -v '^src/db/schema\.zig:' |
grep -v '^src/tap_slug\.zig:' |
grep -v '^src/cli/migrate/keg\.zig:' |
grep -v '^$' || true)

Expand Down
8 changes: 6 additions & 2 deletions src/cli/install.zig
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const plist_mod = @import("../core/services/plist.zig");
const supervisor_mod = @import("../core/services/supervisor.zig");
const signals = @import("../core/signals.zig");
const store_mod = @import("../core/store.zig");
const tap_slug = @import("../tap_slug.zig");
const lock_mod = @import("../db/lock.zig");
const lock_report = @import("lock_report.zig");
const schema = @import("../db/schema.zig");
Expand Down Expand Up @@ -306,8 +307,11 @@ fn caskPresent(ctx: *const AppCtx, prefix: []const u8, token: []const u8) bool {
fn tapPackagePresent(ctx: *const AppCtx, prefix: []const u8, name: []const u8) bool {
const parts = args_mod.parseTapName(name) orelse return false;

var slug_buf: [256]u8 = undefined;
const slug = std.fmt.bufPrint(&slug_buf, "{s}/{s}", .{ parts.user, parts.repo }) catch return false;
// Must key the probe the same way the install path records the row.
var slug_raw_buf: [tap_slug.max_slug_len]u8 = undefined;
const slug_raw = std.fmt.bufPrint(&slug_raw_buf, "{s}/{s}", .{ parts.user, parts.repo }) catch return false;
var slug_buf: [tap_slug.max_slug_len]u8 = undefined;
const slug = tap_slug.canonicalTapSlug(&slug_buf, slug_raw) orelse return false;

var db = openExistingDb(ctx, prefix) orelse return false;
defer db.close();
Expand Down
11 changes: 9 additions & 2 deletions src/cli/install/local.zig
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const cellar_mod = @import("../../core/cellar.zig");
const hash = @import("../../core/hash.zig");
const linker_mod = @import("../../core/linker.zig");
const tap_mod = @import("../../core/tap.zig");

const forge = @import("../../core/forge.zig");
const tap_cache = @import("../../core/tap_cache.zig");
const sqlite = @import("../../db/sqlite.zig");
Expand Down Expand Up @@ -226,8 +227,14 @@ fn installTapRb(
return InstallError.FormulaNotFound;
};

var tap_slug_buf: [128]u8 = undefined;
const tap_slug = std.fmt.bufPrint(&tap_slug_buf, "{s}/{s}", .{ parts.user, parts.repo }) catch
// Single construction point for the tap key: the registry row,
// `kegs.tap`/`casks.tap` and the resolve URLs all inherit it, so
// canonicalizing here is what keeps one tap from splitting in two.
var tap_slug_raw_buf: [tap_mod.max_slug_len]u8 = undefined;
const tap_slug_raw = std.fmt.bufPrint(&tap_slug_raw_buf, "{s}/{s}", .{ parts.user, parts.repo }) catch
return InstallError.FormulaNotFound;
var tap_slug_buf: [tap_mod.max_slug_len]u8 = undefined;
const tap_slug = tap_mod.canonicalTapSlug(&tap_slug_buf, tap_slug_raw) orelse
return InstallError.FormulaNotFound;

// An already-recorded package needs no `.rb` at all - the post-parse
Expand Down
6 changes: 6 additions & 0 deletions src/cli/list.zig
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const schema = @import("../db/schema.zig");
const sqlite = @import("../db/sqlite.zig");
const atomic = @import("../fs/atomic.zig");
const dirsize = @import("../fs/dirsize.zig");
const tap_slug = @import("../tap_slug.zig");
const color = @import("../ui/color.zig");
const output = @import("../ui/output.zig");
const help = @import("help.zig");
Expand Down Expand Up @@ -53,6 +54,11 @@ pub fn execute(ctx: *const AppCtx, args: []const []const u8) !void {
tap_filter = arg["--tap=".len..];
}
}
// Rows are stored canonical; fold the filter so every spelling of a
// tap selects the same set.
var tap_filter_buf: [tap_slug.max_slug_len]u8 = undefined;
if (tap_filter) |raw| tap_filter = tap_slug.canonicalTapSlug(&tap_filter_buf, raw) orelse raw;

const json_mode = output.isJson();

// If neither specified, show both
Expand Down
40 changes: 32 additions & 8 deletions src/cli/outdated.zig
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const formula_mod = @import("../core/formula.zig");
const schema = @import("../db/schema.zig");
const sqlite = @import("../db/sqlite.zig");
const atomic = @import("../fs/atomic.zig");
const tap_slug = @import("../tap_slug.zig");
const api_mod = @import("../net/api.zig");
const client_mod = @import("../net/client.zig");
const color = @import("../ui/color.zig");
Expand Down Expand Up @@ -248,10 +249,12 @@ pub fn planEmit(
/// Trim ASCII whitespace from a `--tap` label; return null when the
/// trimmed result is empty so the caller can fail with a precise
/// error instead of running the DB lookup against `""`.
pub fn normalizeTapLabel(label: []const u8) ?[]const u8 {
pub fn normalizeTapLabel(buf: []u8, label: []const u8) ?[]const u8 {
const trimmed = std.mem.trim(u8, label, " \t\n\r");
if (trimmed.len == 0) return null;
return trimmed;
// Rows are stored canonical; fold the filter so `--tap User/Homebrew-X`
// and `--tap user/x` select the same set.
return tap_slug.canonicalTapSlug(buf, trimmed) orelse trimmed;
}

/// "All clear" summary line for the current scope, or null when at
Expand Down Expand Up @@ -661,14 +664,34 @@ test "planEmit recomputes when --tap= narrows the scope (equals form)" {
}

test "normalizeTapLabel returns null for empty and whitespace-only labels" {
try std.testing.expectEqual(@as(?[]const u8, null), normalizeTapLabel(""));
try std.testing.expectEqual(@as(?[]const u8, null), normalizeTapLabel(" "));
try std.testing.expectEqual(@as(?[]const u8, null), normalizeTapLabel("\t\n"));
var buf: [160]u8 = undefined;
try std.testing.expectEqual(@as(?[]const u8, null), normalizeTapLabel(&buf, ""));
try std.testing.expectEqual(@as(?[]const u8, null), normalizeTapLabel(&buf, " "));
try std.testing.expectEqual(@as(?[]const u8, null), normalizeTapLabel(&buf, "\t\n"));
}

test "normalizeTapLabel trims surrounding whitespace from a valid label" {
try std.testing.expectEqualStrings("user/repo", normalizeTapLabel(" user/repo ").?);
try std.testing.expectEqualStrings("user/repo", normalizeTapLabel("user/repo").?);
var buf: [160]u8 = undefined;
try std.testing.expectEqualStrings("user/repo", normalizeTapLabel(&buf, " user/repo ").?);
try std.testing.expectEqualStrings("user/repo", normalizeTapLabel(&buf, "user/repo").?);
}

test "normalizeTapLabel passes through a label it cannot fold" {
// No slash means no tap identity to fold onto; the raw value is bound
// and simply matches nothing, rather than silently selecting a row.
var buf: [160]u8 = undefined;
try std.testing.expectEqualStrings("noslash", normalizeTapLabel(&buf, " noslash ").?);
// Too long for the buffer — same pass-through, no truncation.
var tiny: [4]u8 = undefined;
try std.testing.expectEqualStrings("user/repo", normalizeTapLabel(&tiny, "user/repo").?);
}

test "normalizeTapLabel folds a --tap label onto the stored tap identity" {
// `--tap` is matched against rows written in canonical form, so the
// user's spelling has to be folded before it is bound.
var buf: [160]u8 = undefined;
try std.testing.expectEqualStrings("indaco/tap", normalizeTapLabel(&buf, " Indaco/Homebrew-Tap ").?);
try std.testing.expectEqualStrings("homebrew/core", normalizeTapLabel(&buf, "Homebrew/homebrew-core").?);
}

test "summaryMessage suppresses 'all up to date' when any row was printed" {
Expand Down Expand Up @@ -866,8 +889,9 @@ pub fn execute(ctx: *const AppCtx, allocator: std.mem.Allocator, args: []const [

// Validate --tap before any cache/network I/O so a typo never
// writes a partial snapshot or warms an API cache for nothing.
var tap_label_buf: [tap_slug.max_slug_len]u8 = undefined;
if (tap_filter) |raw_label| {
const label = normalizeTapLabel(raw_label) orelse {
const label = normalizeTapLabel(&tap_label_buf, raw_label) orelse {
output.err("--tap requires a non-empty label (e.g. `--tap user/repo`)", .{});
return error.Aborted;
};
Expand Down
15 changes: 9 additions & 6 deletions src/cli/outdated/rows.zig
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ pub fn loadFormulaRows(
const sql: [:0]const u8 = switch (filter) {
.all => "SELECT name, version, revision, tap, pinned FROM kegs ORDER BY name;",
.pinned_only => "SELECT name, version, revision, tap, pinned FROM kegs WHERE pinned = 1 ORDER BY name;",
// NOCASE so `--tap User/Repo` resolves a row stored lowercase.
// NOCASE: redundant now both sides are canonical, kept for
// hand-edited or un-migrated DBs. See `tapExists`.
.by_tap => "SELECT name, version, revision, tap, pinned FROM kegs WHERE tap = ?1 COLLATE NOCASE ORDER BY name;",
};
const bind: ?[]const u8 = switch (filter) {
Expand All @@ -77,7 +78,8 @@ pub fn loadCaskRows(
const sql: [:0]const u8 = switch (filter) {
.all => "SELECT token, version, 0 AS revision, tap, pinned FROM casks ORDER BY token;",
.pinned_only => "SELECT token, version, 0 AS revision, tap, pinned FROM casks WHERE pinned = 1 ORDER BY token;",
// NOCASE so `--tap User/Repo` resolves a row stored lowercase.
// NOCASE: redundant now both sides are canonical, kept for
// hand-edited or un-migrated DBs. See `tapExists`.
.by_tap => "SELECT token, version, 0 AS revision, tap, pinned FROM casks WHERE tap = ?1 COLLATE NOCASE ORDER BY token;",
};
const bind: ?[]const u8 = switch (filter) {
Expand All @@ -94,10 +96,11 @@ pub fn loadCaskRows(
/// `untap`ped while keeping their installs.
pub fn tapExists(db: *sqlite.Database, label: []const u8) !bool {
// Three sources, single round-trip: `?1` is reused across the
// UNION ALL legs; `COLLATE NOCASE` matches `--tap User/Repo`
// against a lowercase row; `LIMIT 1` short-circuits after the
// first match. Caller propagates `error.PrepareFailed` so a
// broken schema is diagnosed distinctly from a typo.
// UNION ALL legs; `LIMIT 1` short-circuits after the first match.
// `COLLATE NOCASE` predates canonicalization and is now redundant —
// kept so hand-edited or un-migrated DBs still match. Caller
// propagates `error.PrepareFailed` so a broken schema is diagnosed
// distinctly from a typo.
var stmt = try db.prepare(
\\SELECT 1 FROM taps WHERE name = ?1 COLLATE NOCASE
\\UNION ALL
Expand Down
10 changes: 7 additions & 3 deletions src/cli/tap.zig
Original file line number Diff line number Diff line change
Expand Up @@ -840,11 +840,15 @@ fn run(ctx: *const AppCtx, allocator: std.mem.Allocator, args: []const []const u
return;
}

const name = positional.?;
validateTapName(name) catch {
output.err("Invalid tap '{s}'. Expected: user/repo with [A-Za-z0-9._-]", .{name});
const raw_name = positional.?;
validateTapName(raw_name) catch {
output.err("Invalid tap '{s}'. Expected: user/repo with [A-Za-z0-9._-]", .{raw_name});
return error.Aborted;
};
// One identity per tap: fold here so add and remove agree on the row
// key no matter which spelling the user typed.
var name_buf: [tap_mod.max_slug_len]u8 = undefined;
const name = tap_mod.canonicalTapSlug(&name_buf, raw_name) orelse raw_name;

switch (action) {
.add => {
Expand Down
Loading
Loading