Merge branch 'feat/modules-name-no-node' into bulk-c
This commit is contained in:
@@ -1,3 +0,0 @@
|
||||
# The modules the check covers (novox/hq ADR 0507 §9, issue 1838), one a line; issue 1838 widens it.
|
||||
pacman
|
||||
kernel
|
||||
+41
-16
@@ -1,12 +1,14 @@
|
||||
// Command particulars keeps this mesh's node names out of the modules it covers (novox/hq ADR 0507 §9,
|
||||
// issue 1838): a module is for any mesh, so no manifest, file, code string, identifier, comment, test name or
|
||||
// fixture of a covered module names a node of this one.
|
||||
// fixture of any module names a node of this one.
|
||||
//
|
||||
// The names are in the file `particulars` at the catalogue's root, one a line, `#` comments — the one place
|
||||
// they may be written. The modules covered are in `covered` at checks/particulars/covered, one a line. A name is matched
|
||||
// whole and in any case; a hyphen, an underscore or a CamelCase boundary parts one, so a longer word holding a name is another word.
|
||||
// The forge is not a node name: its host (git.novox.be) and the organisation path (novox/<repo>) are blanked
|
||||
// before a line is read.
|
||||
// they may be written. Every directory under modules/ is read, with no list of covered modules, so a new module
|
||||
// is covered from its first commit. A name is matched whole and in any case; a hyphen, an underscore or a
|
||||
// CamelCase boundary parts one, so a longer word holding a name is another word.
|
||||
// Three names hold a node name and are not one, so they are blanked before a line is read: the forge's host
|
||||
// (git.novox.be) and organisation path (novox/<repo>), the hardware model (asus-zephyrus-g14, ROG Zephyrus G14)
|
||||
// and the software's own name (Novox Mesh, novox-mesh).
|
||||
//
|
||||
// go run . <catalogue root>
|
||||
//
|
||||
@@ -29,6 +31,11 @@ import (
|
||||
// (testdata/novox/x, /srv/novox/data) is not blanked.
|
||||
var forge = regexp.MustCompile(`(?i)git\.novox\.be(?:/novox/[\w.-]+)?|(?:^|[^/\w.-])novox/[\w.-]+`)
|
||||
|
||||
// ownNames are the hardware model's and the software's own names, blanked before a line is read: the model
|
||||
// in any separator and case (asus-zephyrus-g14, ASUS_ZEPHYRUS_G14, ROG Zephyrus G14), the software as two
|
||||
// words or hyphened, whole (Novox Mesh, novox mesh, novox-mesh), so novox-mesh-vault stays a node name joined.
|
||||
var ownNames = regexp.MustCompile(`(?i)(?:asus[-_ ])?zephyrus[-_ ]g14|(?:^|[^\w-])novox[- ]mesh(?:$|[^\w-])`)
|
||||
|
||||
func main() {
|
||||
root := "."
|
||||
if len(os.Args) > 1 {
|
||||
@@ -43,18 +50,18 @@ func check(root string) int {
|
||||
if err == nil && len(names) == 0 {
|
||||
err = fmt.Errorf("particulars lists no name — the check would pass on anything")
|
||||
}
|
||||
var covered []string
|
||||
var modules []string
|
||||
if err == nil {
|
||||
covered, err = readList(filepath.Join(root, "checks/particulars/covered"))
|
||||
if err == nil && len(covered) == 0 {
|
||||
err = fmt.Errorf("covered lists no module — the check would pass on anything")
|
||||
modules, err = moduleDirs(root)
|
||||
if err == nil && len(modules) == 0 {
|
||||
err = fmt.Errorf("modules/ holds no module — the check would pass on anything")
|
||||
}
|
||||
}
|
||||
if err != nil {
|
||||
fmt.Fprintln(os.Stderr, "particulars:", err)
|
||||
return 2
|
||||
}
|
||||
findings, err := run(root, names, covered)
|
||||
findings, err := run(root, names, modules)
|
||||
if err != nil {
|
||||
fmt.Fprintln(os.Stderr, "particulars:", err)
|
||||
return 2
|
||||
@@ -66,7 +73,7 @@ func check(root string) int {
|
||||
fmt.Printf("particulars: %d node name(s) in a module for any mesh (novox/hq ADR 0507 §9) — say what it shows, not where it was seen\n", len(findings))
|
||||
return 1
|
||||
}
|
||||
fmt.Printf("particulars: none of %d node names in the %d covered modules\n", len(names), len(covered))
|
||||
fmt.Printf("particulars: none of %d node names in the %d modules\n", len(names), len(modules))
|
||||
return 0
|
||||
}
|
||||
|
||||
@@ -89,10 +96,25 @@ func readList(path string) ([]string, error) {
|
||||
return out, sc.Err()
|
||||
}
|
||||
|
||||
// run reads every text file of each covered module and returns the findings, sorted.
|
||||
func run(root string, names, covered []string) ([]string, error) {
|
||||
// moduleDirs returns every directory under the catalogue's modules/, sorted.
|
||||
func moduleDirs(root string) ([]string, error) {
|
||||
entries, err := os.ReadDir(filepath.Join(root, "modules"))
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
var out []string
|
||||
for _, m := range covered {
|
||||
for _, e := range entries {
|
||||
if e.IsDir() {
|
||||
out = append(out, e.Name())
|
||||
}
|
||||
}
|
||||
return out, nil
|
||||
}
|
||||
|
||||
// run reads every text file of each module named and returns the findings, sorted.
|
||||
func run(root string, names, modules []string) ([]string, error) {
|
||||
var out []string
|
||||
for _, m := range modules {
|
||||
dir := filepath.Join(root, "modules", m)
|
||||
err := filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error {
|
||||
if err != nil {
|
||||
@@ -143,9 +165,12 @@ func scan(file, src string, names []string) []string {
|
||||
}
|
||||
|
||||
// hits returns the node names one line holds: as a word (a hyphen or underscore parts it), as a part of a
|
||||
// CamelCase identifier (TestAce, novoxTree), the forge's host and organisation path blanked first.
|
||||
// CamelCase identifier (TestAce, novoxTree), the forge's host and organisation path and the hardware's and the
|
||||
// software's own names blanked first.
|
||||
func hits(line string, names []string) []string {
|
||||
line = forge.ReplaceAllStringFunc(line, func(s string) string { return strings.Repeat(" ", len(s)) })
|
||||
blank := func(s string) string { return strings.Repeat(" ", len(s)) }
|
||||
line = forge.ReplaceAllStringFunc(line, blank)
|
||||
line = ownNames.ReplaceAllStringFunc(line, blank)
|
||||
var out []string
|
||||
for _, n := range names {
|
||||
q := regexp.QuoteMeta(n)
|
||||
|
||||
@@ -82,7 +82,6 @@ func TestCheckReturnsOneOnAFindingAndTwoOnAnUnreadableList(t *testing.T) {
|
||||
os.WriteFile(p, []byte(s), 0o644)
|
||||
}
|
||||
write("particulars", "ace\n")
|
||||
write("checks/particulars/covered", "kernel\n")
|
||||
write("modules/kernel/a", "clean\n")
|
||||
if got := check(root); got != 0 {
|
||||
t.Errorf("clean = %d, want 0", got)
|
||||
@@ -97,7 +96,7 @@ func TestCheckReturnsOneOnAFindingAndTwoOnAnUnreadableList(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestOnlyCoveredModulesAreRead(t *testing.T) {
|
||||
func TestRunReadsOnlyTheModulesItIsGiven(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
write := func(p, s string) {
|
||||
p = filepath.Join(root, p)
|
||||
@@ -114,3 +113,59 @@ func TestOnlyCoveredModulesAreRead(t *testing.T) {
|
||||
t.Errorf("got %v", got)
|
||||
}
|
||||
}
|
||||
|
||||
// 1838/AC1: the check reads every directory under modules/ and no list of covered modules, so a module is
|
||||
// covered from its first commit.
|
||||
func TestEveryModuleIsReadWithoutAList(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
write := func(p, s string) {
|
||||
p = filepath.Join(root, p)
|
||||
os.MkdirAll(filepath.Dir(p), 0o755)
|
||||
os.WriteFile(p, []byte(s), 0o644)
|
||||
}
|
||||
write("particulars", "novox\n")
|
||||
write("modules/kernel/files/a", "clean\n")
|
||||
if got := check(root); got != 0 {
|
||||
t.Errorf("clean, no covered list = %d, want 0", got)
|
||||
}
|
||||
write("modules/new-module/files/a", "on novox\n")
|
||||
if got := check(root); got != 1 {
|
||||
t.Errorf("a finding in a module no list names = %d, want 1", got)
|
||||
}
|
||||
}
|
||||
|
||||
// 1838/AC2: the hardware model's own name and the software's own name are not node names.
|
||||
func TestTheHardwareModelAndTheSoftwareAreNotNodeNames(t *testing.T) {
|
||||
for _, s := range []string{
|
||||
"modules/asus-zephyrus-g14/README.md",
|
||||
"the asus-zephyrus-g14 module",
|
||||
"ASUS_ZEPHYRUS_G14=1",
|
||||
"product_family: ROG Zephyrus G14",
|
||||
"# This machine is a node of a Novox mesh",
|
||||
"Novox Mesh turns a handful of personal machines into one system",
|
||||
"<!-- novox mesh: this file was changed on two nodes",
|
||||
`const sealInfo = "novox-mesh sealed box v1"`,
|
||||
"module git.novox.be/novox/mesh-catalog/modules/kernel",
|
||||
} {
|
||||
if got := scan("f", s, names); len(got) != 0 {
|
||||
t.Errorf("%q refused: %v", s, got)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// 1838/AC2: a node name standing alone is still one, beside the hardware's or the software's name too.
|
||||
func TestANodeNameBesideTheHardwareOrTheSoftwareIsRefused(t *testing.T) {
|
||||
cases := map[string]string{
|
||||
"g14": "the asus-zephyrus-g14 module on g14",
|
||||
"shanks": "Novox Mesh on shanks",
|
||||
"novox": "resolves under novox.internal",
|
||||
"ace": "ROG Zephyrus G14 beside ace",
|
||||
"novox mesh-ish": "mesh.mod.gitea.call.x.novox-mesh-vault",
|
||||
}
|
||||
for want, s := range cases {
|
||||
got := scan("modules/m/a.go", s, names)
|
||||
if len(got) != 1 || !strings.HasPrefix(got[0], "modules/m/a.go:1: ") {
|
||||
t.Errorf("%s: %q gave %v, want one finding naming file:line", want, s, got)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -38,7 +38,7 @@ the SDK, or the catalogue's bus, forge or build agent) resolves with `replay` or
|
||||
needs a `reason`. A refused change changes nothing and says which rule refused it.
|
||||
|
||||
**Who works on it** (hq ADR 0284). A move to `diagnosing` or `fixing` names in `by` the issue's **worker**: who
|
||||
works on it and its node, as `who@node` (such as `fix-399@g14` or `jochen@shanks`, kept in lower case). The issue
|
||||
works on it and its node, as `who@node` (such as `fix-399@node-b` or `operator@node-d`, kept in lower case). The issue
|
||||
keeps its worker (`worker`: `by`, `since`, `active`) while it stays at either status; any other status ends the
|
||||
work. `get` shows it, with `worker-stale`, and each `list` row shows `worker`, `since` and `stale`. A move to either
|
||||
status by someone else while the worker is current is refused, naming the worker; with `take-over` saying why, the
|
||||
@@ -342,7 +342,7 @@ when. A change whose event the bus did not take is kept, and its answer says the
|
||||
ADR 0275 §5 records the caller from the bus credential, never from an argument. **Who acted is the bus user
|
||||
that called** (hq issue 365): a call on a tool's caller-named subject carries the caller's bus user as its last
|
||||
token, the bus lets each user publish there with its own name alone, and the tool runner hands the module that
|
||||
user from the subject the call arrived on (mesh-sdk `stdio.CallerOf`) — `shanks.node-tools` for an agent on
|
||||
user from the subject the call arrived on (mesh-sdk `stdio.CallerOf`) — `node-d.node-tools` for an agent on
|
||||
that machine's console, `person.<name>` for a person's credential. A call on the subject that names no caller
|
||||
(served beside it for one release, hq issue 464), or through a runtime from before issue 365, is recorded as
|
||||
by `unproven`. No verb takes an argument naming a caller, and none can be given one. The caller is only as sound
|
||||
@@ -351,7 +351,8 @@ as the credential, which ADR 0266 keeps agents from reading (hq issue 333).
|
||||
## Settings
|
||||
|
||||
`duplicate-check` (`on`; `off` stops the triage of new issues), `judge-model` (`sonnet`; `haiku` or `opus`) and
|
||||
`forge-owner` (the forge's owner of the repositories `fixed-by` names, for asking a delivery) steer the triage;
|
||||
`forge-owner` (the forge's owner of the repositories `fixed-by` names, for asking a delivery: required, with no
|
||||
default, novox/hq ADR 0507 §9) steer the triage;
|
||||
`stuck-after` (`2h`) is how long a merged fix may take to be delivered before it is said;
|
||||
`delivered-not-resolved-after` (`1h`) how long an issue may stay in fixing once every fix is delivered before it is
|
||||
said (hq issue 483).
|
||||
|
||||
@@ -427,9 +427,16 @@ func nonZero(t *time.Time) *time.Time {
|
||||
// DeliveryState asks mesh-delivery for one pull request's delivery.
|
||||
type DeliveryState func(repository string, number string) (Delivery, error)
|
||||
|
||||
// errNoForgeOwner is what asking the forge says when no forge owner is set: the forge is not asked under a
|
||||
// name nobody gave (novox/hq ADR 0507 §9).
|
||||
var errNoForgeOwner = errors.New("forge-owner is not set: the forge is not asked (set the module's forge-owner)")
|
||||
|
||||
// busDeliveryState asks mesh-delivery's checks over the bus.
|
||||
func busDeliveryState(owner string) DeliveryState {
|
||||
return func(repository, number string) (Delivery, error) {
|
||||
if owner == "" {
|
||||
return Delivery{}, errNoForgeOwner
|
||||
}
|
||||
raw, err := stdio.Ask("seat:mesh-delivery.checks", map[string]any{"repository": owner + "/" + repository, "number": number})
|
||||
if err != nil {
|
||||
return Delivery{}, err
|
||||
|
||||
@@ -362,3 +362,15 @@ func TestATransitionIsTakenUnderEveryNameTheTrackerHears(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// With no forge owner, the forge is not asked under a name nobody gave: the delivery and the approval each say
|
||||
// the owner is not set (novox/hq ADR 0507 §9).
|
||||
// 1838/AC4
|
||||
func TestAnUnsetForgeOwnerAsksNoForgeAndSaysWhy(t *testing.T) {
|
||||
if _, err := busDeliveryState("")("mesh-catalog", "5"); err == nil || !strings.Contains(err.Error(), "forge-owner is not set") {
|
||||
t.Errorf("delivery with no owner: err %v, want it to say forge-owner is not set", err)
|
||||
}
|
||||
if _, _, err := busApproval(func() string { return "" })("mesh-catalog", 5); err == nil || !strings.Contains(err.Error(), "forge-owner is not set") {
|
||||
t.Errorf("approval with no owner: err %v, want it to say forge-owner is not set", err)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -638,7 +638,11 @@ func verdictApproves(body, commit string) bool {
|
||||
// busApproval asks the forge's reviews of a pull request over the bus (gitea_pull_reviews).
|
||||
func busApproval(owner func() string) Approval {
|
||||
return func(repository string, number int) (time.Time, bool, error) {
|
||||
raw, err := stdio.Ask("gitea.gitea_pull_reviews", map[string]any{"owner": owner(), "repo": repository, "number": number})
|
||||
o := owner()
|
||||
if o == "" {
|
||||
return time.Time{}, false, errNoForgeOwner
|
||||
}
|
||||
raw, err := stdio.Ask("gitea.gitea_pull_reviews", map[string]any{"owner": o, "repo": repository, "number": number})
|
||||
if err != nil {
|
||||
return time.Time{}, false, err
|
||||
}
|
||||
|
||||
@@ -39,7 +39,8 @@ type Settings struct {
|
||||
// JudgeModel is the model the duplicate check asks; DefaultJudgeModel when empty.
|
||||
JudgeModel string `json:"judge-model,omitempty"`
|
||||
// ForgeOwner is the owner on the forge of the repositories fixed-by names, for asking a pull request's
|
||||
// delivery (ADR 0279 §3).
|
||||
// delivery (ADR 0279 §3): the operator's, a required config value with no default (ADR 0507 §9); empty,
|
||||
// the forge is not asked and each asking says so.
|
||||
ForgeOwner string `json:"forge-owner,omitempty"`
|
||||
// StuckAfter is how long a fix may be merged and not delivered before it is said stuck (ADR 0281), as a
|
||||
// duration such as 2h; DefaultStuckAfter when empty.
|
||||
@@ -99,7 +100,7 @@ func main() {
|
||||
if s, err := t.Settings(); err == nil && strings.TrimSpace(s.ForgeOwner) != "" {
|
||||
return strings.TrimSpace(s.ForgeOwner)
|
||||
}
|
||||
return "novox"
|
||||
return ""
|
||||
}
|
||||
t.Deliveries = func(repository, number string) (Delivery, error) {
|
||||
return busDeliveryState(forgeOwner())(repository, number)
|
||||
@@ -533,7 +534,7 @@ func Tools(t *Tracker) []stdio.Tool {
|
||||
{Name: Seat + ".move",
|
||||
Description: "Change an issue's status — " + statuses + " — with what that status needs, under the " +
|
||||
"development cycle's rules. Move an issue to diagnosing or fixing when you start work on it, with by: " +
|
||||
"who works on it and its node, as who@node (such as fix-399@g14); the issue shows it until it moves on. " +
|
||||
"who works on it and its node, as who@node (such as fix-399@node-b); the issue shows it until it moves on. " +
|
||||
"Refused while someone else works on it, naming them, unless take-over says why (noted on the issue); a " +
|
||||
"worker who did nothing on it for a day may be taken over from without a reason, and with no pull request open " +
|
||||
"the answer says it was abandoned. located needs located-in, owners on " +
|
||||
@@ -551,7 +552,7 @@ func Tools(t *Tracker) []stdio.Tool {
|
||||
"replay": text("for a core issue resolved: the replay's id in mesh-lab's replays register, such as R361"),
|
||||
"replay-none": text("for a core issue resolved: why no replay is possible"),
|
||||
"reason": text("for wontfix: why it will not be fixed"),
|
||||
"by": text("for diagnosing and fixing: who works on it and its node, as who@node, such as fix-399@g14 or jochen@shanks"),
|
||||
"by": text("for diagnosing and fixing: who works on it and its node, as who@node, such as fix-399@node-b or operator@node-d"),
|
||||
"take-over": text("for diagnosing and fixing, when someone else works on it: why you take it over"),
|
||||
}),
|
||||
Run: func(m map[string]any) (any, error) {
|
||||
@@ -734,7 +735,7 @@ func Tools(t *Tracker) []stdio.Tool {
|
||||
"operator's channel asks it to say in a condition's message which issue tracks it. Refused: a key not of a condition " +
|
||||
"key's shape.",
|
||||
Input: schema(nil, map[string]any{
|
||||
"conditions": texts("only these condition keys, such as module.mounts.shanks.unhealthy; all when not given"),
|
||||
"conditions": texts("only these condition keys, such as module.mounts.node-d.unhealthy; all when not given"),
|
||||
}),
|
||||
Run: func(m map[string]any) (any, error) {
|
||||
a, err := readArgs("tracking", m, "conditions")
|
||||
|
||||
@@ -270,3 +270,40 @@ func TestEveryCodeModuleOfTheCatalogueIsAnOwner(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// The forge's owner differs between meshes, so it is a config value the operator gives, with no default naming
|
||||
// this mesh, and the template the config merges into holds none either (novox/hq ADR 0507 §9).
|
||||
// 1838/AC4
|
||||
func TestTheForgeOwnerIsARequiredValueWithNoDefault(t *testing.T) {
|
||||
raw, err := os.ReadFile(filepath.Join("..", "..", "module.json"))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
var m struct {
|
||||
Config map[string]map[string]any `json:"config"`
|
||||
}
|
||||
if err := json.Unmarshal(raw, &m); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
owner, ok := m.Config["forge-owner"]
|
||||
if !ok {
|
||||
t.Fatal("no forge-owner config value")
|
||||
}
|
||||
if d, has := owner["default"]; has {
|
||||
t.Errorf("forge-owner has the default %v: the forge's owner is the operator's, with no default", d)
|
||||
}
|
||||
if owner["required"] != true {
|
||||
t.Errorf("forge-owner is not required: an unset owner would compose and ask no forge in silence")
|
||||
}
|
||||
tmpl, err := os.ReadFile(filepath.Join("..", "..", "files", "settings.json"))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
var s map[string]any
|
||||
if err := json.Unmarshal(tmpl, &s); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if v, has := s["forge-owner"]; has {
|
||||
t.Errorf("files/settings.json holds forge-owner %v: the value comes from the config only", v)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -393,7 +393,7 @@ func (t *Tracker) move(by string, a MoveAsk) (*Moved, error) {
|
||||
if worked(to) {
|
||||
if strings.TrimSpace(a.By) == "" {
|
||||
return nil, fmt.Errorf("a move to %s needs by: who works on it and its node, as who@node, such as "+
|
||||
"fix-399@g14 or jochen@shanks", to)
|
||||
"fix-399@node-b or operator@node-d", to)
|
||||
}
|
||||
if worker, err = checkWorker(a.By); err != nil {
|
||||
return nil, err
|
||||
|
||||
@@ -53,7 +53,7 @@ func checkWorkerField(field, s string) (string, error) {
|
||||
s = strings.ToLower(s)
|
||||
if !workerName.MatchString(s) {
|
||||
return "", fmt.Errorf("%s %q is not who works on it and its node: give who@node, the agent or person and the "+
|
||||
"machine it works on, such as fix-399@g14 or jochen@shanks", field, s)
|
||||
"machine it works on, such as fix-399@node-b or operator@node-d", field, s)
|
||||
}
|
||||
return s, nil
|
||||
}
|
||||
|
||||
@@ -161,7 +161,6 @@
|
||||
],
|
||||
"duplicate-check": "on",
|
||||
"judge-model": "sonnet",
|
||||
"forge-owner": "novox",
|
||||
"stuck-after": "2h",
|
||||
"worker-stale-after": "24h",
|
||||
"delivered-not-resolved-after": "1h",
|
||||
|
||||
@@ -267,9 +267,9 @@
|
||||
},
|
||||
"forge-owner": {
|
||||
"type": "string",
|
||||
"description": "The owner on the forge of the repositories an issue's fixed-by names, for asking after a pull request's delivery (novox/hq ADR 0279 §3); empty asks no forge.",
|
||||
"description": "The owner on the forge of the repositories an issue's fixed-by names, for asking after a pull request's delivery (novox/hq ADR 0279 §3), such as the organisation that holds them: the operator's, with no default (novox/hq ADR 0507 §9).",
|
||||
"trusted": true,
|
||||
"default": "novox"
|
||||
"required": true
|
||||
},
|
||||
"stuck-after": {
|
||||
"type": "string",
|
||||
|
||||
Reference in New Issue
Block a user