d2cc543ed2c0fc032bc0cf8527ecff098b075aef braney Sun Jul 26 17:24:31 2026 -0700 record the ticket that introduced each hg.conf variable refs #37925 Each setting now carries the Redmine ticket cited by the commit that added its read, or for a flag the commit that turned its default on. --sunset prints it per gate, --html links it, and harvestHgConf.py --tickets groups the settings by whether git can attribute them at all. Nothing looser is used: a commit that merely edits a line is not a commit about the setting on it. A second history walk dates the names that reach cfgOption through a macro, so hgConfAges.json is rebuilt and a refresh now costs about four minutes. No CGI behaviour changes. diff --git src/hg/utils/hgConfCatalog/harvestHgConf.py src/hg/utils/hgConfCatalog/harvestHgConf.py index b2df3cee310..df5594d19f2 100755 --- src/hg/utils/hgConfCatalog/harvestHgConf.py +++ src/hg/utils/hgConfCatalog/harvestHgConf.py @@ -84,30 +84,39 @@ # Mined for #define values in addition to everything under SCAN_ROOTS. MACRO_DIRS = ["inc", "hg/inc"] # The documented example configs shipped to mirrors. These are the closest # thing the tree has to hg.conf documentation today, and the thing the catalog # is reconciled against. DOC_FILES = ["product/ex.hg.conf", "product/minimal.hg.conf"] # Where the release version lives, and the file whose history gives the # date -> version mapping used by --age. VERSION_FILE = "hg/inc/versionInfo.h" CACHE = os.path.join(os.path.dirname(os.path.abspath(__file__)), "hgConfAges.json") +# Bumped when the shape of the cache changes. A cache written by an older +# version is still usable for dates, so a mismatch is reported rather than +# treated as an error, but the fields added since will be missing. +CACHE_SCHEMA = 2 + +# "refs #37925", "#37925", "fixes #37925". Three digits minimum, so a commit +# talking about #10 or a C preprocessor line does not read as a ticket. +TICKET_RE = re.compile(r'#\s*(\d{3,6})') + # How each accessor lays out its arguments. nameArg is the index of the # hg.conf name; defArg is the index of the compiled-in default, if any; # twoPart means the name is arg0 + "." + arg1. ACCESSORS = { "cfgOption": {"nameArg": 0, "defArg": None}, "cfgOptionDefault": {"nameArg": 0, "defArg": 1}, "cfgOptionBooleanDefault": {"nameArg": 0, "defArg": 1, "boolean": True}, "cfgVal": {"nameArg": 0, "defArg": None, "required": True}, "cfgOptionEnv": {"nameArg": 1, "defArg": None, "envArg": 0}, "cfgOptionEnvDefault": {"nameArg": 1, "defArg": 2, "envArg": 0}, "cfgOption2": {"twoPart": True, "defArg": None}, "cfgOptionDefault2": {"twoPart": True, "defArg": 2}, } # Reads whose name comes from a variable holding some other module's setting @@ -517,101 +526,222 @@ ver = None for stamp, v in timeline: if stamp <= ts: ver = v else: break return ver def current_version(): path = os.path.join(ROOT, VERSION_FILE) m = re.search(r'CGI_VERSION\s+"(\d+)"', open(path).read()) return int(m.group(1)) if m else None -def harvest_ages(refresh=False): - """First version each hg.conf variable was read in, and when a flag flipped. +def commit_messages(shas): + """sha -> full commit message, in batches so this is a few git calls.""" + msgs = {} + shas = sorted(set(shas)) + for i in range(0, len(shas), 200): + # \x01 between records and \x02 between the hash and the body, so a + # multi-line commit message can be split back apart safely. + raw = git("show", "-s", "--format=%x01%H%x02%B", *shas[i:i + 200]) + for rec in raw.split("\x01"): + if not rec.strip(): + continue + sha, _, body = rec.partition("\x02") + msgs[sha.strip()] = body + return msgs + + +def tickets_in(msg): + return sorted({int(m) for m in TICKET_RE.findall(msg or "")}) - One history traversal with -G'cfgOption', recording for each name the - earliest commit that added a line reading it, and for boolean flags the - earliest commit that added a read with a TRUE compiled-in default. That - second date is what turns a release gate's lifecycle into something the - tree knows rather than something a person maintains by hand: a gate is - introduced defaulting FALSE, flips to TRUE when the feature ships, and only - the removal deadline is left as a judgement call. - About two minutes, so the result is cached next to this script. +def walk_adds(pattern, matcher, paths=("hg", "lib")): + """Every commit that added a line matching, oldest first, per name. - Caveats. This dates the earliest surviving *read* of a name, which is the - right question for "how long has this been in the tree", but a name removed - and later reintroduced dates from the reintroduction. A flag whose default - flipped TRUE and then back to FALSE still reports the first flip. Reads - whose name comes from a macro are not datable at all and come back absent, - which --check reports rather than guesses at. + One -G traversal. matcher(line) returns the names that line introduces. + Returns {name: [(timestamp, sha)]} with consecutive duplicates collapsed, + so the first entry is the introducing commit and the rest are later + commits that touched a read of the same name. + """ + out = git("log", "--reverse", "-G", pattern, "--format=COMMIT %at %H", + "-p", "--unified=0", "--", *paths) + hits = {} + ts = sha = None + for line in out.splitlines(): + if line.startswith("COMMIT "): + f = line.split() + if len(f) >= 3: + ts, sha = int(f[1]), f[2] + continue + if not line.startswith("+") or ts is None: + continue + for name in matcher(line): + lst = hits.setdefault(name, []) + if not lst or lst[-1][1] != sha: + lst.append((ts, sha)) + return hits + + +def harvest_ages(names=None, refresh=False): + """When each hg.conf variable arrived, when a flag flipped, and under + which ticket. + + Two history traversals. The first is filtered on -G'cfgOption' and + records, for each name, the earliest commit that added a line reading it, + plus for boolean flags the earliest commit that added a read with a TRUE + compiled-in default. That second date is what turns a release gate's + lifecycle into something the tree knows rather than something a person + maintains by hand: a gate is introduced defaulting FALSE, flips to TRUE + when the feature ships, and only the removal deadline is left as a + judgement call. + + The second traversal exists because a read whose name comes from a macro + is invisible to the first. cfgOptionBooleanDefault(CFG_LOGIN_HTTPS, ...) + carries no literal, so login.https is dated from the #define instead, by + walking the names the first pass missed. That covers 274 of the 281 + catalogued settings; the remainder are the names built at run time, which + have no single birthday to find. + + Attribution. Each commit message is searched for a Redmine ticket, and + only two commits are allowed to speak for a setting: the one that added + the read, and for a flag the one that turned its default on. Nothing + else is used, and the reason is worth writing down. An earlier draft + fell back to the next later commit that touched the same read, which + tripled coverage and was wrong: it credited wiki.host and textSize to + #37838, a 2026 cart refactor that happened to touch those lines. A + commit that edits a line is not a commit about the setting on it. + + So roughly 40% of settings get a ticket and the rest honestly have none. + Of those, about half were added before the tree used Redmine at all and + can never have one. The rest are recent enough that somebody chose not + to cite a ticket, and --tickets lists them separately as the ones a human + could still fill in. + + About four minutes for both walks, so the result is cached next to this + script. + + Caveats. This dates the earliest surviving read of a name, which is the + right question for "how long has this been in the tree", but a name + removed and later reintroduced dates from the reintroduction. A flag + whose default flipped TRUE and then back to FALSE still reports the first + flip. A ticket number in a commit message is whatever the committer + typed, so a typo becomes a wrong ticket here. """ if not refresh and os.path.exists(CACHE): with open(CACHE) as f: ages = json.load(f) # The release version must come from the tree, never from the cache. # Every deadline in the sunset report is arithmetic against it, so a # cache built two releases ago would quietly move every deadline two # releases into the future. Keep the build-time value under cachedAt # so staleness can be reported rather than guessed at. ages["cachedAt"] = ages.get("current") ages["current"] = current_version() ages["stale"] = (ages["cachedAt"] is not None and ages["current"] is not None and ages["cachedAt"] < ages["current"]) + ages["oldSchema"] = ages.get("schema", 1) < CACHE_SCHEMA return ages timeline = version_timeline() - out = git("log", "--reverse", "-G", "cfgOption", - "--format=COMMIT %at %H", "-p", "--unified=0", "--", "hg", "lib") - # Any literal in a cfgOption* call on an added line. Deliberately looser - # than the real scan: here a false positive only mis-dates a name, while a - # miss loses it entirely. - pat = re.compile(r'cfg(?:Option[A-Za-z0-9]*|Val)\s*\(\s*' + + # Pass one: any literal in a cfgOption* call on an added line. + # Deliberately looser than the real scan, since here a false positive only + # mis-dates a name while a miss loses it entirely. + call_re = re.compile(r'cfg(?:Option[A-Za-z0-9]*|Val)\s*\(\s*' r'(?:"[^"]*"\s*,\s*)?"([^"]+)"') # cfgOptionBooleanDefault("name", TRUE) specifically, for the flip date. true_re = re.compile(r'cfgOptionBooleanDefault\s*\(\s*"([^"]+)"\s*,\s*' r'(TRUE|1)\s*\)') - first = {} - first_true = {} - ts = None - for line in out.splitlines(): - if line.startswith("COMMIT "): - ts = int(line.split()[1]) - continue - if not line.startswith("+") or ts is None: - continue - for name in pat.findall(line): - first.setdefault(name, ts) - for name, _ in true_re.findall(line): - first_true.setdefault(name, ts) - - ages = {"current": current_version(), + # Both come out of the same traversal. A flip is tagged with a prefix a + # setting name cannot contain, then split back out below, so that turning + # this into two walks costs nothing. + def reads_and_flips(line): + return (call_re.findall(line) + + ["\x00" + n for n, _ in true_re.findall(line)]) + + walked = walk_adds("cfgOption", reads_and_flips) + hits = {n: v for n, v in walked.items() if not n.startswith("\x00")} + flips = {n[1:]: v for n, v in walked.items() if n.startswith("\x00")} + + # Pass two: the catalogued names pass one never saw, matched as a quoted + # literal anywhere, which finds the macro definition that names them. + chased = {} + todo = sorted(n for n in (names or []) + if n not in hits and not n.startswith("{")) + if todo: + alt = "|".join(re.escape(n) for n in + sorted(todo, key=len, reverse=True)) + lit = {n: re.compile('"' + re.escape(n) + '"') for n in todo} + chased = walk_adds(alt, lambda line: [n for n, p in lit.items() + if p.search(line)]) + + msgs = commit_messages([s for lst in hits.values() for _, s in lst] + + [s for lst in flips.values() for _, s in lst] + + [s for lst in chased.values() for _, s in lst]) + + def record(lst, via): + ts, sha = lst[0] + return {"ts": ts, "version": version_at(ts, timeline), + "commit": sha[:11], "tickets": tickets_in(msgs.get(sha)), + "subject": (msgs.get(sha, "").splitlines() or [""])[0][:120], + "via": via} + + first = {n: record(lst, "call") for n, lst in hits.items()} + for n, lst in chased.items(): + first[n] = record(lst, "literal") + + ages = {"schema": CACHE_SCHEMA, + "current": current_version(), "timeline": timeline, - "first": {n: {"ts": t, "version": version_at(t, timeline)} - for n, t in first.items()}, - "firstTrue": {n: {"ts": t, "version": version_at(t, timeline)} - for n, t in first_true.items()}} + "first": first, + "firstTrue": {n: record(lst, "call") for n, lst in flips.items()}} with open(CACHE, "w") as f: json.dump(ages, f, indent=1, sort_keys=True) return ages +def ticket_for(name, ages): + """(tickets, kind) for a setting, where kind says what the tickets are. + + kind is "introduced" when the commit that added the read cites tickets, + "flip" when only the commit that turned the flag's default on does, and + None when neither names one. The list is every ticket that commit cited, + kept whole rather than reduced to the first, because a commit citing three + tickets has not told us which one asked for the setting. + """ + rec = (ages.get("first") or {}).get(name) or {} + if rec.get("tickets"): + return rec["tickets"], "introduced" + flip = (ages.get("firstTrue") or {}).get(name) or {} + if flip.get("tickets"): + return flip["tickets"], "flip" + return [], None + + +def cite(tickets, kind): + """Render a ticket list for a plain-text report.""" + if not tickets: + return "" + s = ", ".join("#%d" % t for t in tickets) + return s if kind == "introduced" else s + " (flip)" + + # --------------------------------------------------------------------------- # reporting # --------------------------------------------------------------------------- def report_reads(found, out=sys.stdout): print("\n=== hg.conf reads, by owning directory ===", file=out) for who in sorted(found["reads"]): recs = {} for rec in found["reads"][who]: recs.setdefault(rec["name"], rec) print("\n%s (%d)" % (who, len(recs)), file=out) for name in sorted(recs): rec = recs[name] tail = rec["func"] if rec.get("default") is not None: @@ -685,39 +815,123 @@ "and\n friends are read through cfgOption2 and never appear " "literally)", file=out) for n in sorted(documented - code): print(" %-40s %s" % (n, docs[n]["sites"][0]), file=out) def report_ages(found, ages, out=sys.stdout): names = by_name(found) cur = ages.get("current") first = ages.get("first", {}) print("\n=== first version seen (current tree: v%s) ===" % cur, file=out) known = [(first[n]["version"], n) for n in sorted(names) if n in first and first[n].get("version")] known.sort() for ver, name in known: - print(" v%-5s %-42s %d sites" % (ver, name, + tickets, kind = ticket_for(name, ages) + print(" v%-5s %-42s %-22s %d sites" + % (ver, name, cite(tickets, kind), len(names[name]["sites"])), file=out) missing = [n for n in sorted(names) if not n.startswith("{") and n not in first] if missing: print("\nnot datable from history (%d): read through a macro or " "renamed" % len(missing), file=out) for n in missing: print(" %s" % n, file=out) + dated = [n for _, n in known] + intro = [n for n in dated if ticket_for(n, ages)[1] == "introduced"] + print("\n%d of %d dated names cite a ticket in the commit that added them. " + "See --tickets\nfor the rest, which mostly predate the tree's use of " + "Redmine." % (len(intro), len(dated)), file=out) + + +# The tree started citing Redmine tickets in commit messages around here. A +# setting older than this cannot have one, so it is reported as out of scope +# rather than as a gap somebody should go and fill. +REDMINE_ERA = 270 + + +def report_tickets(found, ages, out=sys.stdout): + """Which ticket introduced each setting, and where that is not knowable. + + Only the commit that added the read, and for a flag the commit that turned + its default on, are allowed to answer. See harvest_ages for why nothing + looser is used. + """ + names = by_name(found) + first = ages.get("first", {}) + rows = [] + for name in sorted(names): + if name.startswith("{"): + continue + tickets, kind = ticket_for(name, ages) + rows.append({"name": name, "tickets": tickets, "kind": kind, + "version": first.get(name, {}).get("version"), + "commit": first.get(name, {}).get("commit")}) + + def show(sel): + for r in sorted(sel, key=lambda r: (r["version"] or 0, r["name"])): + print(" %-42s %-7s %-22s %s" + % (r["name"], "v%s" % r["version"] if r["version"] else "?", + cite(r["tickets"], r["kind"]), r["commit"] or ""), + file=out) + + print("\n=== the ticket that introduced each setting ===", file=out) + print("\nOnly two commits get to answer for a setting: the one that added " + "the read,\nand for a flag the one that turned its default on.", + file=out) + + have = [r for r in rows if r["kind"] == "introduced"] + print("\nATTRIBUTED, from the commit that added the read: %d" % len(have), + file=out) + show(have) + + flip = [r for r in rows if r["kind"] == "flip"] + print("\nATTRIBUTED, only from the commit that turned the default on: %d" + % len(flip), file=out) + print(" The ticket that shipped the feature, which is not necessarily " + "the one that\n asked for the flag.", file=out) + show(flip) + + # A dated name with no version is older than the first CGI_VERSION stamp + # the timeline reaches, so it belongs here rather than nowhere. + old = [r for r in rows if not r["kind"] + and (r["version"] or 0) < REDMINE_ERA] + print("\nBEFORE REDMINE (added before ~v%d, so there is no ticket to " + "find): %d" % (REDMINE_ERA, len(old)), file=out) + show(old) + + gap = [r for r in rows if not r["kind"] + and (r["version"] or 0) >= REDMINE_ERA] + print("\nNO TICKET CITED (added late enough that there probably was one): " + "%d" % len(gap), file=out) + print(" This is the actionable list. Each of these was added by a commit " + "that names\n no ticket, so the only way to fill it in is somebody " + "who remembers, or a\n search of Redmine for the setting name.", + file=out) + show(gap) + + undated = [n for n in sorted(names) + if not n.startswith("{") and n not in first] + if undated: + print("\nNO BIRTHDAY TO FIND (the name is built at run time): %d\n %s" + % (len(undated), ", ".join(undated)), file=out) + runtime = [n for n in sorted(names) if n.startswith("{")] + if runtime: + print("\nNot settings, so not attributed: %s" % ", ".join(runtime), + file=out) def counts(found): names = by_name(found) resolved = [n for n in names if not n.startswith("{")] return { "distinctNames": len(resolved), "unresolved": len(names) - len(resolved), "booleanFlags": len([n for n, d in names.items() if d["boolean"]]), "required": len([n for n, d in names.items() if d["required"]]), "envOverridable": len([n for n, d in names.items() if d["env"]]), "profileSuffixes": len(found["profiles"]), "prefixScans": len(found["prefixScans"]), "owningDirs": len(found["reads"]), "documented": len(parse_doc_files()), @@ -730,78 +944,92 @@ first = (ages or {}).get("first", {}) out = {} for name in sorted(names): d = names[name] rec = {"name": name, "sites": sorted(d["sites"]), "funcs": sorted(d["funcs"]), "defaults": sorted(d["defaults"]), "boolean": d["boolean"], "required": d["required"], "documented": name in docs} if d["env"]: rec["env"] = d["env"] if name in first: rec["firstVersion"] = first[name].get("version") + rec["firstCommit"] = first[name].get("commit") flip = (ages or {}).get("firstTrue", {}).get(name) if flip: rec["flippedVersion"] = flip.get("version") + rec["flippedCommit"] = flip.get("commit") + if ages: + tickets, kind = ticket_for(name, ages) + if tickets: + rec["tickets"] = tickets + rec["ticketFrom"] = kind out[name] = rec return {"names": out, "profiles": {s: sorted({p for p, _, _ in v}) for s, v in found["profiles"].items()}, "prefixScans": {k: sorted(set(v)) for k, v in found["prefixScans"].items()}, "documentedOnly": sorted(set(docs) - set(names)), "currentVersion": (ages or {}).get("current")} def main(): ap = argparse.ArgumentParser( description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) ap.add_argument("--names", action="store_true") ap.add_argument("--reads", action="store_true") ap.add_argument("--gates", action="store_true") ap.add_argument("--profiles", action="store_true") ap.add_argument("--docs", action="store_true") ap.add_argument("--age", action="store_true") + ap.add_argument("--tickets", action="store_true", + help="the ticket that introduced each setting") ap.add_argument("--refresh", action="store_true", - help="rebuild the age cache (walks history, ~2 minutes)") + help="rebuild the age cache (walks history, ~4 minutes)") ap.add_argument("--json") args = ap.parse_args() found, macro = harvest() ages = None - if args.age or args.gates or args.json or args.refresh: - ages = harvest_ages(refresh=args.refresh) + if args.age or args.gates or args.tickets or args.json or args.refresh: + # The scanned names let the refresh chase the ones whose reads name + # them through a macro, which the cfgOption filter cannot see. + ages = harvest_ages(names=sorted(by_name(found)), + refresh=args.refresh) if args.reads: report_reads(found) if args.gates: report_gates(found, ages) if args.profiles: report_profiles(found) if args.docs: report_docs(found) if args.age: report_ages(found, ages) + if args.tickets: + report_tickets(found, ages) if args.names: for n in sorted(by_name(found)): print(n) if args.json: with open(args.json, "w") as f: json.dump(as_json(found, ages), f, indent=1, sort_keys=True) print("wrote %s" % args.json) if not any([args.reads, args.gates, args.profiles, args.docs, args.age, - args.names, args.json]): + args.tickets, args.names, args.json]): c = counts(found) print("macros resolved %d" % len(macro)) for k in sorted(c): print("%-18s %s" % (k, c[k])) return 0 if __name__ == "__main__": sys.exit(main())