Compare commits
7
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
5d9c677cf7 | ||
|
|
1e3a490c12 | ||
|
|
d4ea14ca5c | ||
|
|
62c1a95ead | ||
|
|
e67462ab53 | ||
|
|
e5dc54d0ec | ||
|
|
a5c3990d12 |
+21
-4
@@ -107,15 +107,32 @@ jobs:
|
||||
# Conventional Commits. `.releaserc.yml` has always derived the
|
||||
# version from the commit type; until now nothing checked that the
|
||||
# type was one it recognises, so a malformed subject silently meant
|
||||
# "no release". BEFORE is the push's previous tip and is absent or
|
||||
# all-zeros for a new branch, in which case only the tip is linted.
|
||||
# "no release".
|
||||
#
|
||||
# **On a `pull_request` there is no `before`.** Gitea leaves
|
||||
# `github.event.before` empty for one, so this step fell through to
|
||||
# bare `make commit-check`, which lints `git log -1` — the tip
|
||||
# alone. Every other commit the branch would bring was first
|
||||
# examined by *main's* post-merge run, which is a green PR that
|
||||
# stops being true after the merge, and which happened twice (#254).
|
||||
# The PR's base is the stand-in: the range below already excludes
|
||||
# what the base shares with the branch, because base advances on
|
||||
# main and those commits stay reachable from it.
|
||||
#
|
||||
# Both are handed to the shell rather than chosen in an expression:
|
||||
# `github.event.issue.number` in unclaim.yml is this repo's proof
|
||||
# that payload fields resolve, and the shell then falls back to
|
||||
# today's behaviour for a dispatch run or a missing field instead of
|
||||
# depending on how `&&`/`||` treat an absent context.
|
||||
- name: Commit messages
|
||||
working-directory: /src
|
||||
env:
|
||||
BEFORE: ${{ github.event.before }}
|
||||
PR_BASE: ${{ github.event.pull_request.base.sha }}
|
||||
PUSH_BEFORE: ${{ github.event.before }}
|
||||
run: |
|
||||
set -eu
|
||||
if [ -n "${BEFORE:-}" ] && [ "${BEFORE#0000000}" = "$BEFORE" ] \
|
||||
BEFORE="${PR_BASE:-${PUSH_BEFORE:-}}"
|
||||
if [ -n "$BEFORE" ] && [ "${BEFORE#0000000}" = "$BEFORE" ] \
|
||||
&& git cat-file -e "$BEFORE^{commit}" 2>/dev/null; then
|
||||
make commit-check RANGE="$BEFORE..$SHA"
|
||||
else
|
||||
|
||||
@@ -68,7 +68,7 @@ jobs:
|
||||
# claim with a test behind it now (cmd/indexbuild/deps_test.go),
|
||||
# because the v3 migration quietly broke it and this job was where
|
||||
# that surfaced.
|
||||
image: golang:1.25
|
||||
image: golang:1.26
|
||||
# This host path must exist on the runner and be listed verbatim in
|
||||
# act_runner's container.valid_volumes. It holds explore-staging/
|
||||
# (counts.bin + state.json) and yj.db — the checkpoint that makes
|
||||
@@ -148,6 +148,49 @@ jobs:
|
||||
sha256sum /tmp/core-index.db.zst | tee /tmp/core-index.db.zst.sha256
|
||||
ls -lh /tmp/core-index.db.zst
|
||||
|
||||
# Nothing is published until it has been imported by the code that
|
||||
# imports it on a user's machine. The exporter and the importer are
|
||||
# two descriptions of one storage format, and every other tier tests
|
||||
# the importer against a *fixture* rather than against the file being
|
||||
# shipped — a second description free to be wrong in the same
|
||||
# direction as the code reading it.
|
||||
#
|
||||
# That is how #258 reached everyone: the importer positioned its batch
|
||||
# walk with a Go `string` cursor against this file's 16-byte `mbid`
|
||||
# column, and SQLite neither coerces between TEXT and BLOB nor
|
||||
# complains about the comparison — so the walk merged no rows and
|
||||
# never advanced, and no install could finish its first index build.
|
||||
# The fixture guarding that walk writes the old text encoding, and the
|
||||
# only compact fixture is one row, below the batch size, so the bound
|
||||
# query never ran. Both were green throughout.
|
||||
#
|
||||
# Running it here is also what keeps the failure cheap: the previous
|
||||
# artifact stays published while this runs, so a failure costs one
|
||||
# stale catalog rather than an empty one for every install.
|
||||
#
|
||||
# `-tags indexbuild` because this container has no GTK and the default
|
||||
# tag set links the app through Wails. The `--- PASS` grep is not
|
||||
# decoration — the test skips without the path, and a skip is
|
||||
# indistinguishable from a pass in a summary line.
|
||||
- name: Import the exported artifact as a client does
|
||||
if: steps.maintain.outputs.complete == 'true' && steps.maintain.outputs.changed == 'true'
|
||||
working-directory: /src
|
||||
env:
|
||||
YJ_CORE_INDEX_ARTIFACT: /tmp/core-index.db.zst
|
||||
run: |
|
||||
set -eu
|
||||
log=/tmp/import-check.log
|
||||
if ! go test -tags indexbuild -count=1 -timeout 30m -v \
|
||||
-run TestImportPublishedArtifact ./backend/explore/ > "$log" 2>&1;
|
||||
then
|
||||
tail -60 "$log"
|
||||
echo "::error::The artifact does not import; not publishing it."
|
||||
exit 1
|
||||
fi
|
||||
cat "$log"
|
||||
grep -qF -- 'PASS: TestImportPublishedArtifact' "$log"
|
||||
echo "::notice::The artifact imports as a client would merge it."
|
||||
|
||||
- name: Publish to the Gitea package registry
|
||||
if: steps.maintain.outputs.complete == 'true' && steps.maintain.outputs.changed == 'true'
|
||||
run: |
|
||||
|
||||
@@ -57,6 +57,21 @@ behind `YJ_TESTCTL=1`, which `scripts/dev-headless.sh` sets and
|
||||
staging the work that would produce it — job progress, download
|
||||
progress, scan progress. It calls `events.Deliver`, which *errors*
|
||||
when the event reaches nobody, so a `200` means it really arrived.
|
||||
- **State you stage, you own** (#168). Nothing resets those stores, so
|
||||
clear yours in `test.afterEach` with the same event that staged it
|
||||
(`emit('JobsChanged', [])`) — the store replaces its list from every
|
||||
snapshot, so `testctl` needs no special case. **Measured: this does
|
||||
not currently cross a spec boundary**, because every test gets a fresh
|
||||
page and `JobStore.init()` refetches `GetJobs()` from a backend
|
||||
registry that `/__test/emit` never writes to. Stated anyway, because
|
||||
it costs one line and the leak needs only one spec that keeps a page
|
||||
alive — but do not cite #168 for a symptom you have not reproduced.
|
||||
- **Measure against the thing next to you, not an absolute
|
||||
coordinate.** An absolute number in a shell measurement is also a
|
||||
claim about everything above it — `contentTop === 0` quietly asserts
|
||||
"and no background job is running", which is not what that spec was
|
||||
about or could arrange, while `contentTop === jobBandBottom` is true
|
||||
either way. This is the half of #168 that stands on its own.
|
||||
- **`restore` is slow** (~40 s in the suite) because it copies every
|
||||
table. Prefer snapshotting once and restoring only when a spec
|
||||
genuinely mutates state.
|
||||
|
||||
@@ -845,6 +845,22 @@ that is quietly empty.
|
||||
top-N, exact match, FTS search, popularity batch, the CAA map — and
|
||||
asserts each returns something with a dashed id. A missed conversion
|
||||
site shows up there and essentially nowhere else.
|
||||
- **A comparison is typed on *both* sides, and a parameter is the half
|
||||
that gets forgotten.** The paragraph above is about a literal; the
|
||||
artifact merge positioned its batch walk with a Go `string` cursor
|
||||
against the artifact's byte column, and SQLite answered rather than
|
||||
complained: `mbid > ?` with a text key is true of every row, so the
|
||||
bound the walk looked up was the same every time and the cursor
|
||||
never advanced, while `mbid <= ?` is false of every row, so no batch
|
||||
merged at all. The import looped indefinitely at 100% CPU behind a
|
||||
progress bar reading "0 of 1,077,893 rows", merged nothing and
|
||||
raised nothing (#258). Nothing caught it because the fixture that
|
||||
guards the walk writes the old text form and the only compact one is
|
||||
a single row — below `artifactMergeBatch`, so the bound query never
|
||||
ran. `artifactKey` types the cursor to the artifact's own encoding
|
||||
now, and the walk fails loudly when its bound does not strictly
|
||||
advance, because the failure mode here is silence rather than a
|
||||
wrong answer.
|
||||
|
||||
**The artifact is read in either encoding.** A published artifact
|
||||
carries whichever form the exporter that built it used, and there is one
|
||||
|
||||
@@ -1,8 +1,10 @@
|
||||
package explore
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"database/sql"
|
||||
"database/sql/driver"
|
||||
"errors"
|
||||
"fmt"
|
||||
"os"
|
||||
@@ -283,6 +285,25 @@ func (si *SearchIndex) importCoreArtifact(ctx context.Context, path string) erro
|
||||
}
|
||||
|
||||
merged, mergeErr := si.mergeArtifactRows(ctx, info.rows)
|
||||
|
||||
// Every row the artifact declares has to land. The walk partitions
|
||||
// the artifact's key space, so a total short of info.rows does not
|
||||
// mean the artifact was smaller than it said -- it means a predicate
|
||||
// filtered rows out, and the catalog is quietly partial. Equality
|
||||
// rather than a lower bound because RowsAffected counts an upsert
|
||||
// that changes nothing, and a row already merged locally is counted
|
||||
// again here.
|
||||
//
|
||||
// One reachable case, so this is not merely a tripwire: a row whose
|
||||
// mbid is empty is excluded by `mbid > ?` in both encodings, and an
|
||||
// artifact carrying one would otherwise import as complete.
|
||||
if mergeErr == nil && merged != info.rows {
|
||||
mergeErr = fmt.Errorf(
|
||||
"%w: merged %d of %d rows — a row the artifact holds was not selected",
|
||||
ErrArtifactUnusable, merged, info.rows,
|
||||
)
|
||||
}
|
||||
|
||||
if mergeErr == nil {
|
||||
si.mergeArtifactCredits(ctx)
|
||||
}
|
||||
@@ -348,8 +369,12 @@ func (si *SearchIndex) analyzeIndex() {
|
||||
// is an index range scan and a cancelled import leaves committed work
|
||||
// behind rather than rolling it all back.
|
||||
func (si *SearchIndex) mergeArtifactRows(ctx context.Context, total int) (int, error) {
|
||||
// Asked once, because it is a property of the file and it decides
|
||||
// how the walk's own comparisons are typed. See artifactKey.
|
||||
storesText := si.artifactStoresText()
|
||||
|
||||
selectColumns := artifactSelectColumns(
|
||||
si.artifactStoresText(), si.artifactHasTotals(),
|
||||
storesText, si.artifactHasTotals(),
|
||||
)
|
||||
|
||||
insertSQL := `
|
||||
@@ -367,7 +392,7 @@ func (si *SearchIndex) mergeArtifactRows(ctx context.Context, total int) (int, e
|
||||
WHERE mbid > ? AND mbid <= ?` + upsertIndexConflictSQL
|
||||
|
||||
var (
|
||||
cursor string
|
||||
cursor artifactKey
|
||||
merged int
|
||||
)
|
||||
|
||||
@@ -376,17 +401,30 @@ func (si *SearchIndex) mergeArtifactRows(ctx context.Context, total int) (int, e
|
||||
return merged, err
|
||||
}
|
||||
|
||||
upper, hasUpper, err := si.artifactBatchBound(cursor)
|
||||
upper, hasUpper, err := si.artifactBatchBound(storesText, cursor)
|
||||
if err != nil {
|
||||
return merged, err
|
||||
}
|
||||
|
||||
if hasUpper && bytes.Compare(upper, cursor) <= 0 {
|
||||
// The predicate matched the cursor itself, so the walk can
|
||||
// never advance. SQLite says nothing when a comparison is
|
||||
// made between types it will not coerce - the query simply
|
||||
// answers wrongly - so a mismatch here would otherwise spin
|
||||
// forever behind an unmoving progress bar. Fail instead.
|
||||
return merged, fmt.Errorf(
|
||||
"%w: artifact walk did not advance past %x",
|
||||
ErrArtifactUnusable, []byte(cursor),
|
||||
)
|
||||
}
|
||||
|
||||
var res sql.Result
|
||||
|
||||
if hasUpper {
|
||||
res, err = si.db.ExecContext(insertRangeSQL, cursor, upper)
|
||||
res, err = si.db.ExecContext(insertRangeSQL,
|
||||
cursor.bind(storesText), upper.bind(storesText))
|
||||
} else {
|
||||
res, err = si.db.ExecContext(insertSQL, cursor)
|
||||
res, err = si.db.ExecContext(insertSQL, cursor.bind(storesText))
|
||||
}
|
||||
|
||||
if err != nil {
|
||||
@@ -413,26 +451,65 @@ func (si *SearchIndex) mergeArtifactRows(ctx context.Context, total int) (int, e
|
||||
}
|
||||
}
|
||||
|
||||
// artifactKey is one MBID as the attached artifact stores it: 16 raw
|
||||
// bytes in a compact artifact, the dashed 36-character form in one
|
||||
// published before that storage change.
|
||||
//
|
||||
// It is a type with a bind method rather than a string because the
|
||||
// comparison it feeds is typed, and the wrong type is silent. SQLite
|
||||
// does not coerce between TEXT and BLOB and orders every blob after
|
||||
// every text value, so a cursor bound as text against a byte column
|
||||
// makes `mbid > ?` true of the whole table - the walk rediscovers the
|
||||
// same batch bound forever, and `mbid <= ?` false of the whole table,
|
||||
// so no batch merges at all. Nothing errors; the import simply never
|
||||
// finishes. bind is the one place that knows which form the column is
|
||||
// in, decided by artifactStoresText, which asks the artifact rather than
|
||||
// trusting a version number.
|
||||
type artifactKey []byte
|
||||
|
||||
// bind renders the key as a statement argument in the artifact's own
|
||||
// encoding.
|
||||
func (k artifactKey) bind(storesText bool) driver.Value {
|
||||
if storesText {
|
||||
return string(k)
|
||||
}
|
||||
|
||||
// Never nil. database/sql converts a nil []byte to SQL NULL, and
|
||||
// `mbid > NULL` is NULL for every row - so an unset cursor would
|
||||
// agree with nothing and import nothing, which is the same silently
|
||||
// empty merge this type exists to prevent, one type over.
|
||||
if k == nil {
|
||||
return []byte{}
|
||||
}
|
||||
|
||||
return []byte(k)
|
||||
}
|
||||
|
||||
// artifactBatchBound returns the MBID that ends the next batch, and
|
||||
// whether one exists — no bound means the remainder is the last batch.
|
||||
func (si *SearchIndex) artifactBatchBound(cursor string) (string, bool, error) {
|
||||
var bound string
|
||||
//
|
||||
// The bound is read out of the artifact and handed back as an
|
||||
// artifactKey, because it becomes the next comparison the walk makes.
|
||||
func (si *SearchIndex) artifactBatchBound(
|
||||
storesText bool, cursor artifactKey,
|
||||
) (artifactKey, bool, error) {
|
||||
var bound []byte
|
||||
|
||||
err := si.db.QueryRowWriter(
|
||||
`SELECT mbid FROM core.explore_index
|
||||
WHERE mbid > ? ORDER BY mbid LIMIT 1 OFFSET ?`,
|
||||
cursor, artifactMergeBatch-1,
|
||||
cursor.bind(storesText), artifactMergeBatch-1,
|
||||
).Scan(&bound)
|
||||
|
||||
if errors.Is(err, sql.ErrNoRows) {
|
||||
return "", false, nil
|
||||
return nil, false, nil
|
||||
}
|
||||
|
||||
if err != nil {
|
||||
return "", false, fmt.Errorf("%w: batch bound: %w", ErrArtifactUnusable, err)
|
||||
return nil, false, fmt.Errorf("%w: batch bound: %w", ErrArtifactUnusable, err)
|
||||
}
|
||||
|
||||
return bound, true, nil
|
||||
return artifactKey(bound), true, nil
|
||||
}
|
||||
|
||||
// stampArtifactMeta records what the merge established: the catalog half
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package explore
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"database/sql"
|
||||
"encoding/hex"
|
||||
@@ -71,13 +72,7 @@ func writeTestArtifact(
|
||||
}
|
||||
}
|
||||
|
||||
for k, v := range meta {
|
||||
if _, err := db.Exec(
|
||||
`INSERT INTO artifact_meta (key, value) VALUES (?, ?)`, k, v,
|
||||
); err != nil {
|
||||
t.Fatalf("stamp artifact meta: %v", err)
|
||||
}
|
||||
}
|
||||
stampArtifactMeta(t, db, meta)
|
||||
|
||||
for _, r := range rows {
|
||||
if _, err := db.Exec(`
|
||||
@@ -93,6 +88,101 @@ func writeTestArtifact(
|
||||
return path
|
||||
}
|
||||
|
||||
// compactArtifactSchema is the artifact cmd/indexexport publishes: the
|
||||
// catalog's ids as 16 raw bytes, its entity types as codes, and the
|
||||
// per-release-group total_tracks the exporter added after the first
|
||||
// artifact was shipped.
|
||||
//
|
||||
// It matters that a fixture carries this encoding and not the older
|
||||
// text one, because SQLite does not coerce between TEXT and BLOB and
|
||||
// every comparison the importer makes against an mbid is therefore
|
||||
// encoding-sensitive. writeTestArtifact above is the *other* fixture:
|
||||
// it still writes the text form, which is what the first published
|
||||
// artifact carries and what the importer must keep reading.
|
||||
var compactArtifactSchema = []string{
|
||||
`CREATE TABLE explore_index (
|
||||
entity_type INTEGER NOT NULL,
|
||||
mbid BLOB NOT NULL,
|
||||
title TEXT NOT NULL,
|
||||
artist_name TEXT NOT NULL,
|
||||
artist_mbid BLOB NOT NULL,
|
||||
aliases TEXT NOT NULL DEFAULT '',
|
||||
popularity INTEGER NOT NULL DEFAULT 0,
|
||||
listener_count INTEGER NOT NULL DEFAULT 0,
|
||||
duration INTEGER NOT NULL DEFAULT 0,
|
||||
caa_release_mbid BLOB NOT NULL DEFAULT x'',
|
||||
release_name TEXT NOT NULL DEFAULT '',
|
||||
primary_type TEXT NOT NULL DEFAULT '',
|
||||
secondary_types TEXT NOT NULL DEFAULT '',
|
||||
release_date TEXT NOT NULL DEFAULT '',
|
||||
total_tracks INTEGER NOT NULL DEFAULT 0,
|
||||
artist_type TEXT NOT NULL DEFAULT '',
|
||||
country TEXT NOT NULL DEFAULT '',
|
||||
disambiguation TEXT NOT NULL DEFAULT '',
|
||||
sort_name TEXT NOT NULL DEFAULT '',
|
||||
discog_fetched INTEGER NOT NULL DEFAULT 0,
|
||||
PRIMARY KEY (mbid)
|
||||
) WITHOUT ROWID`,
|
||||
`CREATE TABLE artifact_meta (
|
||||
key TEXT PRIMARY KEY,
|
||||
value TEXT NOT NULL
|
||||
)`,
|
||||
}
|
||||
|
||||
// writeCompactTestArtifact builds the artifact the exporter publishes
|
||||
// today, in its own encoding, so the importer is exercised against what
|
||||
// a client actually downloads rather than against what it was written
|
||||
// for.
|
||||
func writeCompactTestArtifact(
|
||||
t *testing.T, meta map[string]string, rows []artifactRow,
|
||||
) string {
|
||||
t.Helper()
|
||||
|
||||
path := filepath.Join(t.TempDir(), "core-index.db")
|
||||
|
||||
db, err := sql.Open("sqlite", "file:"+path)
|
||||
if err != nil {
|
||||
t.Fatalf("open artifact: %v", err)
|
||||
}
|
||||
|
||||
defer func() { _ = db.Close() }()
|
||||
|
||||
for _, stmt := range compactArtifactSchema {
|
||||
if _, err := db.Exec(stmt); err != nil {
|
||||
t.Fatalf("create artifact schema: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
stampArtifactMeta(t, db, meta)
|
||||
|
||||
for _, r := range rows {
|
||||
if _, err := db.Exec(`
|
||||
INSERT INTO explore_index
|
||||
(entity_type, mbid, title, artist_name, artist_mbid, popularity)
|
||||
VALUES (?, ?, ?, ?, ?, ?)`,
|
||||
entityCode(r.entityType), mbidBytes(r.mbid), r.title,
|
||||
r.artistName, mbidBytes(r.artistMBID), r.popularity,
|
||||
); err != nil {
|
||||
t.Fatalf("insert artifact row: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
return path
|
||||
}
|
||||
|
||||
// stampArtifactMeta writes the artifact_meta rows a fixture declares.
|
||||
func stampArtifactMeta(t *testing.T, db *sql.DB, meta map[string]string) {
|
||||
t.Helper()
|
||||
|
||||
for k, v := range meta {
|
||||
if _, err := db.Exec(
|
||||
`INSERT INTO artifact_meta (key, value) VALUES (?, ?)`, k, v,
|
||||
); err != nil {
|
||||
t.Fatalf("stamp artifact meta: %v", err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// validMeta is the artifact_meta a well-formed artifact carries.
|
||||
func validMeta() map[string]string {
|
||||
return map[string]string{
|
||||
@@ -270,6 +360,71 @@ func TestImportCoreArtifactBatchWalkCoversAllRows(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestImportCoreArtifactBatchWalkCoversAllRowsCompact is the batch walk
|
||||
// on the encoding the exporter actually publishes.
|
||||
//
|
||||
// The walk positions itself by comparing the artifact's own mbid column
|
||||
// against the last id it reached, and that column holds 16 raw bytes.
|
||||
// SQLite does not coerce between TEXT and BLOB, and a blob sorts after
|
||||
// every text value, so a cursor bound as text is a predicate that either
|
||||
// matches every row or none: `mbid > ?` with an empty text key is true
|
||||
// of the whole table, so
|
||||
// the 100th row is always the 100th row and the bound never advances,
|
||||
// while `mbid <= <text>` is false of the whole table, so no batch ever
|
||||
// merges. The result is not a wrong import but an unbounded loop that
|
||||
// merges nothing and never fails.
|
||||
//
|
||||
// Both encodings are covered on purpose. The walk was only ever tested
|
||||
// against the text fixture above, which is why it shipped broken on the
|
||||
// one the clients download.
|
||||
func TestImportCoreArtifactBatchWalkCoversAllRowsCompact(t *testing.T) {
|
||||
db := database.NewTestDB(t)
|
||||
si := NewSearchIndex(db, nil, nil, testLogger())
|
||||
|
||||
original := artifactMergeBatch
|
||||
artifactMergeBatch = 100
|
||||
|
||||
t.Cleanup(func() { artifactMergeBatch = original })
|
||||
|
||||
const total = 337
|
||||
|
||||
rows := make([]artifactRow, 0, total)
|
||||
for i := range total {
|
||||
rows = append(rows, artifactRow{
|
||||
entityType: EntityRecording,
|
||||
mbid: syntheticMBID(i),
|
||||
title: "Song",
|
||||
artistName: "Artist",
|
||||
artistMBID: artA,
|
||||
popularity: i,
|
||||
})
|
||||
}
|
||||
|
||||
path := writeCompactTestArtifact(t, validMeta(), rows)
|
||||
|
||||
if err := si.importCoreArtifact(context.Background(), path); err != nil {
|
||||
t.Fatalf("importCoreArtifact: %v", err)
|
||||
}
|
||||
|
||||
var got, top int
|
||||
|
||||
if err := db.QueryRowWriter(
|
||||
"SELECT COUNT(*), MAX(popularity) FROM explore_index",
|
||||
).Scan(&got, &top); err != nil {
|
||||
t.Fatalf("count rows: %v", err)
|
||||
}
|
||||
|
||||
if got != total {
|
||||
t.Errorf("merged %d rows, want %d", got, total)
|
||||
}
|
||||
|
||||
// A count alone would pass if the walk re-merged the same first
|
||||
// batch forever, so the far end of the artifact is checked too.
|
||||
if top != total-1 {
|
||||
t.Errorf("highest popularity = %d, want %d", top, total-1)
|
||||
}
|
||||
}
|
||||
|
||||
func TestImportCoreArtifactRejectsBadArtifacts(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
@@ -454,61 +609,9 @@ func TestArtifactColumnsMatchExporter(t *testing.T) {
|
||||
// the importer decides by asking the artifact, not by trusting a
|
||||
// version number, and both must land identically.
|
||||
func TestImportCoreArtifactAcceptsBothEncodings(t *testing.T) {
|
||||
compact := filepath.Join(t.TempDir(), "core-index.db")
|
||||
|
||||
db, err := sql.Open("sqlite", "file:"+compact)
|
||||
if err != nil {
|
||||
t.Fatalf("open artifact: %v", err)
|
||||
}
|
||||
|
||||
if _, err := db.Exec(`CREATE TABLE explore_index (
|
||||
entity_type INTEGER NOT NULL,
|
||||
mbid BLOB NOT NULL,
|
||||
title TEXT NOT NULL,
|
||||
artist_name TEXT NOT NULL,
|
||||
artist_mbid BLOB NOT NULL,
|
||||
aliases TEXT NOT NULL DEFAULT '',
|
||||
popularity INTEGER NOT NULL DEFAULT 0,
|
||||
listener_count INTEGER NOT NULL DEFAULT 0,
|
||||
duration INTEGER NOT NULL DEFAULT 0,
|
||||
caa_release_mbid BLOB NOT NULL DEFAULT x'',
|
||||
release_name TEXT NOT NULL DEFAULT '',
|
||||
primary_type TEXT NOT NULL DEFAULT '',
|
||||
secondary_types TEXT NOT NULL DEFAULT '',
|
||||
release_date TEXT NOT NULL DEFAULT '',
|
||||
artist_type TEXT NOT NULL DEFAULT '',
|
||||
country TEXT NOT NULL DEFAULT '',
|
||||
disambiguation TEXT NOT NULL DEFAULT '',
|
||||
sort_name TEXT NOT NULL DEFAULT '',
|
||||
discog_fetched INTEGER NOT NULL DEFAULT 0,
|
||||
PRIMARY KEY (mbid)
|
||||
)`); err != nil {
|
||||
t.Fatalf("create artifact table: %v", err)
|
||||
}
|
||||
|
||||
if _, err := db.Exec(
|
||||
`CREATE TABLE artifact_meta (key TEXT PRIMARY KEY, value TEXT NOT NULL)`,
|
||||
); err != nil {
|
||||
t.Fatalf("create artifact meta: %v", err)
|
||||
}
|
||||
|
||||
for k, v := range validMeta() {
|
||||
if _, err := db.Exec(
|
||||
"INSERT INTO artifact_meta (key, value) VALUES (?, ?)", k, v,
|
||||
); err != nil {
|
||||
t.Fatalf("write artifact meta: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
if _, err := db.Exec(`
|
||||
INSERT INTO explore_index (entity_type, mbid, title, artist_name, artist_mbid, popularity)
|
||||
VALUES (1, ?, 'Artist A', 'Artist A', ?, 5000)`,
|
||||
mbidBytes(artA), mbidBytes(artA),
|
||||
); err != nil {
|
||||
t.Fatalf("write artifact row: %v", err)
|
||||
}
|
||||
|
||||
_ = db.Close()
|
||||
compact := writeCompactTestArtifact(t, validMeta(), []artifactRow{
|
||||
{EntityArtist, artA, "Artist A", "Artist A", artA, 5000},
|
||||
})
|
||||
|
||||
live := database.NewTestDB(t)
|
||||
si := NewSearchIndex(live, nil, nil, testLogger())
|
||||
@@ -748,3 +851,88 @@ func TestImportCoreArtifactWithoutCredits(t *testing.T) {
|
||||
t.Errorf("credit refs = %d, want 0", refs)
|
||||
}
|
||||
}
|
||||
|
||||
// TestImportCoreArtifactRefusesAMergeThatLosesRows is the count guard's
|
||||
// positive case.
|
||||
//
|
||||
// The walk's predicates partition the artifact's key space, so a merge
|
||||
// that lands fewer rows than the artifact declares means a predicate
|
||||
// dropped some — and the failure is a catalog that looks populated and
|
||||
// is missing things nobody can name. An empty mbid is the reachable
|
||||
// way to get there: `mbid > ?` is false of it in both encodings, so it
|
||||
// is never selected, and nothing else in the import would notice.
|
||||
func TestImportCoreArtifactRefusesAMergeThatLosesRows(t *testing.T) {
|
||||
db := database.NewTestDB(t)
|
||||
si := NewSearchIndex(db, nil, nil, testLogger())
|
||||
|
||||
path := writeCompactTestArtifact(t, validMeta(), []artifactRow{
|
||||
{EntityArtist, artA, "Artist A", "Artist A", artA, 5000},
|
||||
{EntityArtist, "", "Nameless", "Artist A", artA, 4000},
|
||||
})
|
||||
|
||||
err := si.importCoreArtifact(context.Background(), path)
|
||||
if err == nil {
|
||||
t.Fatal("a merge that lost a row was reported as a complete import")
|
||||
}
|
||||
|
||||
if !strings.Contains(err.Error(), "merged 1 of 2 rows") {
|
||||
t.Errorf("error = %v, want it to name the shortfall", err)
|
||||
}
|
||||
|
||||
// And the same rule as every other rejection: a failed merge must not
|
||||
// leave the index claiming it has a catalog, or the real build would
|
||||
// never run again.
|
||||
if si.hasMeta(dumpImportDoneKey) {
|
||||
t.Error("a failed import still stamped dump_import_done")
|
||||
}
|
||||
}
|
||||
|
||||
// TestArtifactKeyBindsInTheArtifactsOwnEncoding pins the one place the
|
||||
// batch walk's comparison type is decided.
|
||||
//
|
||||
// Every wrong answer is silent, which is why it is worth pinning all
|
||||
// four. SQLite does not coerce TEXT to BLOB and orders every blob after
|
||||
// every text value, so a text key against a byte column makes
|
||||
// `mbid > ?` true of the whole artifact - the cursor never advances and
|
||||
// the walk spins forever without merging a row - while a byte key
|
||||
// against a text column makes it false of the whole artifact, so every
|
||||
// batch merges nothing and the import "succeeds" empty. An unset cursor
|
||||
// is the same fault once more: database/sql converts a nil []byte to
|
||||
// SQL NULL, and `mbid > NULL` matches no row at all.
|
||||
func TestArtifactKeyBindsInTheArtifactsOwnEncoding(t *testing.T) {
|
||||
raw := mbidBytes(artA)
|
||||
|
||||
for _, tt := range []struct {
|
||||
name string
|
||||
key artifactKey
|
||||
want []byte
|
||||
}{
|
||||
{"unset", nil, []byte{}},
|
||||
{"set", artifactKey(raw), raw},
|
||||
} {
|
||||
t.Run("bytes/"+tt.name, func(t *testing.T) {
|
||||
got, ok := tt.key.bind(false).([]byte)
|
||||
if !ok {
|
||||
t.Fatalf("bind(false) = %T, want []byte", tt.key.bind(false))
|
||||
}
|
||||
|
||||
if got == nil {
|
||||
t.Fatal("bound to SQL NULL, which matches no row")
|
||||
}
|
||||
|
||||
if !bytes.Equal(got, tt.want) {
|
||||
t.Errorf("bind(false) = %x, want %x", got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// The dashed form is what an artifact published before the storage
|
||||
// change carries, and it has to compare as text against text.
|
||||
if got := artifactKey(nil).bind(true); got != "" {
|
||||
t.Errorf("bind(true) on an unset cursor = %#v, want an empty string", got)
|
||||
}
|
||||
|
||||
if got := artifactKey(artA).bind(true); got != artA {
|
||||
t.Errorf("bind(true) = %#v, want %q", got, artA)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,266 @@
|
||||
package explore
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"io"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"yellowjacket/backend/database"
|
||||
)
|
||||
|
||||
// Import of the artifact we actually publish, as a client imports it.
|
||||
//
|
||||
// Every other test here builds a fixture, and a fixture is a second
|
||||
// description of the storage format that can be wrong in the same
|
||||
// direction as the code reading it. That is how #258 shipped: the
|
||||
// importer positioned its batch walk with a Go `string` cursor against
|
||||
// the artifact's 16-byte `mbid` column, and SQLite neither coerces
|
||||
// between TEXT and BLOB nor complains about the comparison — so the walk
|
||||
// merged nothing and never advanced, and no install could finish its
|
||||
// first index build. The fixture that guards the walk writes the old
|
||||
// text encoding; the only compact fixture is one row, below the batch
|
||||
// size, so the bound query never ran. Both passed throughout.
|
||||
//
|
||||
// So this one takes the published file and runs the client's own path
|
||||
// over it — checksum, decompress, merge — and asserts that what the
|
||||
// artifact holds is what the client ends up with.
|
||||
//
|
||||
// It skips without the path, so an ordinary test run pays nothing for
|
||||
// it, and the publish job is where it is meant to run:
|
||||
//
|
||||
// YJ_CORE_INDEX_ARTIFACT=/tmp/core-index.db.zst \
|
||||
// go test -tags indexbuild -run TestImportPublishedArtifact \
|
||||
// ./backend/explore/
|
||||
//
|
||||
// The indexbuild tag is not incidental: that job's container has no GTK,
|
||||
// and the default tag set links the app through Wails.
|
||||
|
||||
// publishedArtifactEnv points at the published artifact: the compressed
|
||||
// core-index.db.zst, or the unpacked core-index.db.
|
||||
const publishedArtifactEnv = "YJ_CORE_INDEX_ARTIFACT"
|
||||
|
||||
// artifactTotals is the pair this test compares across the boundary.
|
||||
//
|
||||
// Rows is the whole point — a merge that lands fewer of them than the
|
||||
// artifact declares is a catalog that looks populated and is missing
|
||||
// things nobody can name — and popularity is the half whose absence was
|
||||
// reported when it happened, because it arrives only through the merge.
|
||||
type artifactTotals struct {
|
||||
rows int
|
||||
withListen int
|
||||
}
|
||||
|
||||
func TestImportPublishedArtifact(t *testing.T) {
|
||||
published := strings.TrimSpace(os.Getenv(publishedArtifactEnv))
|
||||
if published == "" {
|
||||
t.Skipf("set %s=<core-index.db.zst> to import the published artifact",
|
||||
publishedArtifactEnv)
|
||||
}
|
||||
|
||||
if _, err := os.Stat(published); err != nil {
|
||||
t.Fatalf("%s: %v", publishedArtifactEnv, err)
|
||||
}
|
||||
|
||||
// A file-backed database rather than NewTestDB's in-memory one: the
|
||||
// artifact is ~135MB and a million rows, which is not a thing to hold
|
||||
// in RAM inside a test. YJ_HOME is how NewDB is pointed somewhere
|
||||
// disposable, and going through NewDB means this is the constructor,
|
||||
// the schema and the read pool the app itself opens.
|
||||
//
|
||||
// Nothing closes it, because nothing can: `DB` has no Close and the
|
||||
// app's handles are process-lifetime by design. The directory is
|
||||
// unlinked at cleanup and the file goes with it.
|
||||
t.Setenv("YJ_HOME", t.TempDir())
|
||||
|
||||
db, err := database.NewDB(testLogger())
|
||||
if err != nil {
|
||||
t.Fatalf("open database: %v", err)
|
||||
}
|
||||
|
||||
si := NewSearchIndex(db, nil, nil, testLogger())
|
||||
|
||||
// The checksum the publisher shipped, if it shipped one. Every
|
||||
// client verifies it and refuses the artifact when it does not
|
||||
// match, so a wrong one breaks Explore for everyone who has not
|
||||
// already imported — and nothing else would see it, because the
|
||||
// comparison is between two files only the publisher has.
|
||||
if want, ok := publishedChecksum(published); ok {
|
||||
got, err := fileSHA256(published)
|
||||
if err != nil {
|
||||
t.Fatalf("checksum the artifact: %v", err)
|
||||
}
|
||||
|
||||
if got != want {
|
||||
t.Errorf("published artifact hashes to %s, but its .sha256 says %s",
|
||||
got, want)
|
||||
}
|
||||
}
|
||||
|
||||
unpacked := unpackPublishedArtifact(t, si, published)
|
||||
|
||||
want, err := artifactTotalsOf(unpacked)
|
||||
if err != nil {
|
||||
t.Fatalf("count the artifact's rows: %v", err)
|
||||
}
|
||||
|
||||
if err := si.importCoreArtifact(context.Background(), unpacked); err != nil {
|
||||
t.Fatalf("importCoreArtifact: %v", err)
|
||||
}
|
||||
|
||||
got, err := indexTotalsOf(db)
|
||||
if err != nil {
|
||||
t.Fatalf("count the index's rows: %v", err)
|
||||
}
|
||||
|
||||
if got.rows != want.rows {
|
||||
t.Errorf("merged %d rows, but the artifact holds %d",
|
||||
got.rows, want.rows)
|
||||
}
|
||||
|
||||
if got.withListen != want.withListen {
|
||||
t.Errorf("%d rows carry a listen count, but the artifact holds %d of them",
|
||||
got.withListen, want.withListen)
|
||||
}
|
||||
|
||||
// The FTS index is rebuilt from the table once the merge is done, and
|
||||
// it is what search actually reads: a merge that lands without it
|
||||
// leaves Explore silently matching nothing, which is the state #258
|
||||
// produced by a different route.
|
||||
var indexed int
|
||||
if err := db.QueryRowWriter(
|
||||
"SELECT COUNT(*) FROM explore_index_fts",
|
||||
).Scan(&indexed); err != nil {
|
||||
t.Fatalf("count the FTS index: %v", err)
|
||||
}
|
||||
|
||||
if indexed != got.rows {
|
||||
t.Errorf("FTS index holds %d rows against the table's %d",
|
||||
indexed, got.rows)
|
||||
}
|
||||
|
||||
// And one row read back through the app's own path, which is the
|
||||
// other direction of every conversion the merge makes: a byte MBID
|
||||
// out of the table, the app's dashed form, and back in as a lookup.
|
||||
var raw []byte
|
||||
if err := db.QueryRowWriter(`
|
||||
SELECT mbid FROM explore_index
|
||||
WHERE entity_type = 1 /* artist */ AND popularity > 0
|
||||
ORDER BY popularity DESC LIMIT 1`).Scan(&raw); err != nil {
|
||||
t.Fatalf("read a stored mbid: %v", err)
|
||||
}
|
||||
|
||||
dashed, err := mbidFromBytes(raw)
|
||||
if err != nil {
|
||||
t.Fatalf("the stored mbid is not one: %v", err)
|
||||
}
|
||||
|
||||
artist := si.LookupArtistByMBID(dashed)
|
||||
if artist == nil {
|
||||
t.Fatalf("the artifact's most popular artist %s does not look up", dashed)
|
||||
}
|
||||
|
||||
if artist.Popularity == 0 {
|
||||
t.Errorf("artist %s came back with no popularity", dashed)
|
||||
}
|
||||
}
|
||||
|
||||
// publishedChecksum reads the sha256 the publisher wrote beside the
|
||||
// artifact, in `sha256sum` output form. A missing file is not a
|
||||
// failure: it is only there when the artifact came from the publish job.
|
||||
func publishedChecksum(path string) (string, bool) {
|
||||
body, err := os.ReadFile(path + ".sha256")
|
||||
if err != nil {
|
||||
return "", false
|
||||
}
|
||||
|
||||
sum := strings.TrimSpace(string(body))
|
||||
if i := strings.IndexAny(sum, " \t"); i > 0 {
|
||||
sum = sum[:i]
|
||||
}
|
||||
|
||||
if len(sum) != 64 {
|
||||
return "", false
|
||||
}
|
||||
|
||||
return strings.ToLower(sum), true
|
||||
}
|
||||
|
||||
// unpackPublishedArtifact returns a path to the unpacked database,
|
||||
// going through the client's own decompression when it is handed the
|
||||
// compressed file that is actually published.
|
||||
func unpackPublishedArtifact(t *testing.T, si *SearchIndex, path string) string {
|
||||
t.Helper()
|
||||
|
||||
if strings.HasSuffix(path, ".db") {
|
||||
return path
|
||||
}
|
||||
|
||||
// Copied into the test's own directory first: decompress writes
|
||||
// beside the compressed file, and the publisher's directory is not
|
||||
// this test's to write in.
|
||||
staging := t.TempDir()
|
||||
dst := filepath.Join(staging, coreArtifactFile)
|
||||
|
||||
src, err := os.Open(path)
|
||||
if err != nil {
|
||||
t.Fatalf("open the published artifact: %v", err)
|
||||
}
|
||||
|
||||
defer func() { _ = src.Close() }()
|
||||
|
||||
out, err := os.Create(dst)
|
||||
if err != nil {
|
||||
t.Fatalf("create a staging copy: %v", err)
|
||||
}
|
||||
|
||||
if _, err := io.Copy(out, src); err != nil {
|
||||
t.Fatalf("copy the published artifact: %v", err)
|
||||
}
|
||||
|
||||
if err := out.Close(); err != nil {
|
||||
t.Fatalf("close the staging copy: %v", err)
|
||||
}
|
||||
|
||||
fetcher := &artifactFetcher{si: si, stagingDir: staging}
|
||||
|
||||
if err := fetcher.decompress(context.Background()); err != nil {
|
||||
t.Fatalf("decompress the published artifact: %v", err)
|
||||
}
|
||||
|
||||
return fetcher.unpackedPath()
|
||||
}
|
||||
|
||||
// artifactTotalsOf counts what an artifact file holds, read directly so
|
||||
// the numbers do not depend on anything the client does.
|
||||
func artifactTotalsOf(path string) (artifactTotals, error) {
|
||||
db, err := sql.Open("sqlite", "file:"+path+"?mode=ro")
|
||||
if err != nil {
|
||||
return artifactTotals{}, err
|
||||
}
|
||||
|
||||
defer func() { _ = db.Close() }()
|
||||
|
||||
var totals artifactTotals
|
||||
|
||||
err = db.QueryRow(`SELECT COUNT(*), COALESCE(SUM(popularity > 0), 0)
|
||||
FROM explore_index`).Scan(&totals.rows, &totals.withListen)
|
||||
if err != nil {
|
||||
return artifactTotals{}, err
|
||||
}
|
||||
|
||||
return totals, nil
|
||||
}
|
||||
|
||||
// indexTotalsOf counts what the client ended up with.
|
||||
func indexTotalsOf(db *database.DB) (artifactTotals, error) {
|
||||
var totals artifactTotals
|
||||
|
||||
err := db.QueryRowWriter(`SELECT COUNT(*), COALESCE(SUM(popularity > 0), 0)
|
||||
FROM explore_index`).Scan(&totals.rows, &totals.withListen)
|
||||
|
||||
return totals, err
|
||||
}
|
||||
@@ -60,6 +60,23 @@ const PHONE = { width: 424, height: 439 };
|
||||
const DESKTOP = { width: 1100, height: 800 };
|
||||
|
||||
test.describe('background jobs on a phone', () => {
|
||||
/**
|
||||
* **State a spec stages is the spec's to clear.** `/__test/emit` writes
|
||||
* to a store nothing resets, so the event that staged a job is the
|
||||
* event that clears it — `JobStore` replaces its whole list from every
|
||||
* snapshot, so `testctl` needs no special case.
|
||||
*
|
||||
* **Measured on #168: this does not currently outlive the page.** Every
|
||||
* test gets a fresh page, and `JobStore.init()` refetches `GetJobs()`
|
||||
* from a backend registry that `/__test/emit` never writes to, so the
|
||||
* staged job is gone before the next spec starts. Ownership is stated
|
||||
* rather than a live leak repaired — the leak needs a page that
|
||||
* survives its own spec, and there is none today.
|
||||
*/
|
||||
test.afterEach(async ({ testctl }) => {
|
||||
await testctl.emit('JobsChanged', []);
|
||||
});
|
||||
|
||||
test('are shown in the band, without opening anything', async ({
|
||||
app,
|
||||
testctl,
|
||||
|
||||
@@ -102,6 +102,22 @@ const collapsed = (page: Page) =>
|
||||
}));
|
||||
|
||||
test.describe('the top bar fits the window', () => {
|
||||
/**
|
||||
* **State a spec stages is the spec's to clear** (#168). `/__test/emit`
|
||||
* writes to a store nothing resets, and this file stages the widest job
|
||||
* in the app, so it puts it back — with the same event, since the store
|
||||
* replaces its whole list from every snapshot.
|
||||
*
|
||||
* **Measured: it does not currently outlive the page.** Every test gets
|
||||
* a fresh page and `JobStore.init()` refetches `GetJobs()` from a
|
||||
* backend registry `/__test/emit` never writes to, so nothing is being
|
||||
* repaired here; the rule is stated because it costs one line and the
|
||||
* leak would need only one spec that keeps a page alive.
|
||||
*/
|
||||
test.afterEach(async ({ testctl }) => {
|
||||
await testctl.emit('JobsChanged', []);
|
||||
});
|
||||
|
||||
/**
|
||||
* The phone's answer, which is not "it fits" (#57).
|
||||
*
|
||||
|
||||
+62
-7
@@ -82,23 +82,69 @@ targets="$({ make -pqRr 2>/dev/null || true; } |
|
||||
# happened to break there, and a check that fails on reflow gets
|
||||
# disabled rather than fixed.
|
||||
#
|
||||
# **An inline span may be hard-wrapped, and then the mention is split
|
||||
# across two lines.** `make` at the end of one line and its target at
|
||||
# the start of the next is one code span to Markdown and two strings to
|
||||
# a per-line regex, so the target was invisible — and these docs are
|
||||
# mostly hard-wrapped prose, so the wrap is what the author does not
|
||||
# think about. Lines are therefore joined while the span is still open,
|
||||
# which is what an odd number of backticks means.
|
||||
#
|
||||
# Joining re-opens the reflow trap above unless it is bounded, so it is
|
||||
# bounded three ways: a fence flushes first (a fenced command is already
|
||||
# whole, and joining inside one would break the line-start rule), a
|
||||
# blank line flushes (CommonMark does not allow a blank line inside a
|
||||
# code span, so nothing legitimate is split by one), and so does a file
|
||||
# boundary. A stray odd backtick in prose therefore costs one paragraph
|
||||
# of over-matching rather than the rest of the file.
|
||||
#
|
||||
# AGENTS.md is deliberately not in this list: it is a symlink to
|
||||
# CLAUDE.md, asserted above, so scanning it would report every failure
|
||||
# twice under two names.
|
||||
mentioned="$(printf '%s\n' "$docs" |
|
||||
xargs awk '
|
||||
FNR == 1 { fence = 0 }
|
||||
/^```/ { fence = !fence; next }
|
||||
{
|
||||
rest = $0
|
||||
function scan(text, rest) {
|
||||
rest = text
|
||||
while (match(rest, /`make [a-z][a-z0-9-]*/)) {
|
||||
print substr(rest, RSTART + 6, RLENGTH - 6)
|
||||
rest = substr(rest, RSTART + RLENGTH)
|
||||
}
|
||||
if (fence && match($0, /^make [a-z][a-z0-9-]*/)) {
|
||||
print substr($0, 6, RLENGTH - 5)
|
||||
}
|
||||
|
||||
function lineStart(text) {
|
||||
if (match(text, /^make [a-z][a-z0-9-]*/)) {
|
||||
print substr(text, 6, RLENGTH - 5)
|
||||
}
|
||||
}
|
||||
|
||||
function ticks(s, n, i) {
|
||||
n = 0
|
||||
for (i = 1; i <= length(s); i++) {
|
||||
if (substr(s, i, 1) == "`") n++
|
||||
}
|
||||
return n
|
||||
}
|
||||
|
||||
function flush() {
|
||||
if (buf == "") return
|
||||
scan(buf)
|
||||
if (fence) lineStart(buf)
|
||||
buf = ""
|
||||
}
|
||||
|
||||
FNR == 1 { flush(); fence = 0 }
|
||||
|
||||
/^```/ { flush(); fence = !fence; next }
|
||||
|
||||
/^[[:space:]]*$/ { flush(); next }
|
||||
|
||||
{
|
||||
if (fence) { scan($0); lineStart($0); next }
|
||||
buf = (buf == "" ? $0 : buf " " $0)
|
||||
if (ticks(buf) % 2 == 0) flush()
|
||||
}
|
||||
|
||||
END { flush() }
|
||||
' | sort -u)"
|
||||
|
||||
missing=""
|
||||
@@ -113,7 +159,16 @@ if [ -n "$missing" ]; then
|
||||
echo "skill-check: the docs name make targets that do not exist:" >&2
|
||||
for t in $missing; do
|
||||
echo " make $t" >&2
|
||||
printf '%s\n' "$docs" | xargs grep -ln "make $t" | sed 's/^/ /' >&2
|
||||
# `make <t>` on one line first, because that is where a target is
|
||||
# normally named and it is the precise answer. The bare name is the
|
||||
# fallback, and it exists because the parser above can now find a
|
||||
# mention that *this* grep cannot: a wrapped span has `make` and its
|
||||
# target on different lines. Without it a missing target reported no
|
||||
# file at all, and `set -o pipefail` turned the empty grep into exit
|
||||
# 123, before the line telling the author what to do.
|
||||
hits="$(printf '%s\n' "$docs" | xargs grep -ln "make $t" 2>/dev/null || true)"
|
||||
[ -n "$hits" ] || hits="$(printf '%s\n' "$docs" | xargs grep -ln -- "$t" 2>/dev/null || true)"
|
||||
[ -n "$hits" ] && printf '%s\n' "$hits" | sed 's/^/ /' >&2
|
||||
done
|
||||
echo "Fix the docs, or restore the target." >&2
|
||||
exit 1
|
||||
|
||||
Reference in New Issue
Block a user