99c145df11c04f80820690fd67e9499f296b4d7d
braney
Sat Aug 1 12:58:39 2026 -0700
harvesters: resolve #defines per file, not pooled refs #37923 refs #37925 refs #37838
The first nightly run reported a new URL parameter, hggw_term, that had been in
the tree since the hgGateway redesign. It was not new; the harvester had been
answering the question differently in two checkouts of the same commit.
SEARCH_TERM is "hggw_term" in hgGateway and "hgcd_term" in hgChooseDb. The
#define table was pooled across the whole tree with setdefault, so the winner
was whichever file the filesystem walk reached first, and hgGateway's reads came
out as hgcd_term in my working tree and hggw_term in a fresh clone. For a
nightly cron that means mail whenever a directory listing changes order, which
is worse than no cron.
So a name a file defines itself now wins, and a name the tree defines
inconsistently resolves to {NAME} instead of to a guess. This is the call
bdc473369e1 already made for char * constants, for the same reason and with the
same tradeoff written up in the CONST_RE comment: an honest {ident} beats a
confident wrong answer. Verified by harvesting both trees and diffing.
Twelve names leave the URL baseline as a result, the gisaidTable and hgg_
prefix families, which were pooled values from sibling CGIs rather than reads
in the file they were attributed to. Recovering those properly means following
the #include chain to the header that defines them, which is not done here.
The URL catalog carried hgt_tSearch twice, once correctly as the track search
variable and once as hgGateway's search term, which was this bug showing up in
the curated half. --check did not catch the duplicate because it only looks
within a section. hgGateway now has hggw_term and hgChooseDb hgcd_term, both
confirmed at their call sites and in their CGI's excludeVars.
diff --git src/hg/utils/cartTrackVarCatalog/harvestCartVars.py src/hg/utils/cartTrackVarCatalog/harvestCartVars.py
index fc0b674e40b..dac403f0309 100755
--- src/hg/utils/cartTrackVarCatalog/harvestCartVars.py
+++ src/hg/utils/cartTrackVarCatalog/harvestCartVars.py
@@ -1,395 +1,434 @@
#!/usr/bin/env python3
"""harvestCartVars.py - find track-scoped cart variables in the kent tree.
Refs #37838. This is the mechanical half of the cart variable inventory: it
scans the source for the two ways a track-scoped cart name gets built and
reports every suffix it finds, attributed to the function that reads or writes
it. cartTrackVarCatalog.py (next to this file) is the curated half. Run this
first when the tree has moved, then reconcile what falls out against the
catalog.
The two signals:
1. cart*ClosestToHome(cart, tdb, parentLevel, "suffix")
The suffix argument is track-scoped by construction, so the 4th argument
of any such call is a cart variable name.
2. safef(buf, sizeof buf, "%s.%s", track, SUFFIX)
and the "%s.%s.%s" and "%s.<literal>" variants. Note the suffix is the
SECOND vararg, not the first: the first one is the track name.
Macro identifiers are resolved against every #define in the scanned trees plus
inc/, lib/ and hg/inc/, chased up to five levels deep so that things like
GRAY_LEVEL_SCORE_MIN -> SCORE_MIN -> "scoreMin" come out as strings.
What it cannot resolve it reports rather than drops:
{ident} an identifier with no #define found, usually a local variable
holding a name computed at run time
EXPR:... a computed expression, e.g. a ternary
Those are signal, not noise. They mark the places where the name is built at
run time, which is exactly where the hierarchy nests one level deeper:
filter.<field>, decorator.<name>.<var>, <track>.<species>.
Output needs curation. The scan cannot tell a cart variable from a table
name, a filename suffix or an SQL fragment, so expect to throw away things
like .bai, _gold and .tbi by hand.
Usage:
harvestCartVars.py --by-func # grouped by function, for reading
harvestCartVars.py --by-var # grouped by variable, with sites
harvestCartVars.py --json recs.json # raw records
harvestCartVars.py --dirs hg/hgc,hg/hgTables --by-func
"""
import argparse
import collections
import json
import os
import re
import sys
# The tree to scan. KENT_SRC lets a nightly run point at a pristine
# checkout instead of somebody's working tree, where a stray .c file or a
# half-finished edit would show up as a finding.
ROOT = os.environ.get("KENT_SRC") or os.path.expanduser("~/kent/src")
# Everything that draws or configures a track. hgc and hgTables are in the
# default list because they read and write per-track vars too, which is easy
# to forget.
DEFAULT_DIRS = "hg/lib,hg/hgTracks,hg/hgTrackUi,hg/cgilib,hg/hgc,hg/hgTables"
# Extra trees mined for #define values only, not scanned for call sites.
MACRO_DIRS = ["inc", "lib", "hg/inc"]
# ---------------------------------------------------------------------------
# macro table
# ---------------------------------------------------------------------------
def build_macros(dirs):
- """Map every #define that resolves to a string literal, chasing aliases."""
+ """(name -> literal, names defined inconsistently) over the scanned dirs.
+
+ The second return value exists because a pooled table gives a name that two
+ files define differently whichever value the directory listing reached
+ first, so the answer changes between two checkouts of the same commit.
+ Those names resolve to {NAME} unless the file being scanned defines them
+ itself, the same call the per-file char * constants make.
+ """
macro = {}
+ conflict = set()
chains = []
def_re = re.compile(
r'^\s*#\s*define\s+([A-Za-z_][A-Za-z0-9_]*)\s+'
r'("(?:[^"\\]|\\.)*")\s*(?:/[/*].*)?$')
chain_re = re.compile(
r'^\s*#\s*define\s+([A-Za-z_][A-Za-z0-9_]*)\s+'
r'([A-Za-z_][A-Za-z0-9_]*)\s*(?:/[/*].*)?$')
for d in list(dirs) + MACRO_DIRS:
p = os.path.join(ROOT, d)
if not os.path.isdir(p):
continue
for fn in os.listdir(p):
if not fn.endswith((".h", ".c")):
continue
for line in open(os.path.join(p, fn), errors="replace"):
m = def_re.match(line)
if m:
- macro.setdefault(m.group(1), m.group(2)[1:-1])
+ name, val = m.group(1), m.group(2)[1:-1]
+ if name in macro and macro[name] != val:
+ conflict.add(name)
+ macro.setdefault(name, val)
continue
m = chain_re.match(line)
if m:
chains.append((m.group(1), m.group(2)))
for _ in range(5):
for a, b in chains:
if a not in macro and b in macro:
macro[a] = macro[b]
- return macro
+ if b in conflict:
+ conflict.add(a) # alias of an ambiguous name
+ return macro, conflict
+
+
+# #define NAME "literal" in the file being scanned. Its own definition is the
+# one that file means, whatever the rest of the tree says.
+LOCAL_DEFINE_RE = re.compile(
+ r'^[ \t]*#[ \t]*define[ \t]+([A-Za-z_]\w*)[ \t]+'
+ r'("(?:[^"\\]|\\.)*")[ \t]*(?:/[/*].*)?$', re.M)
+
+
+def local_defines(text):
+ # CONST_RE next door captures inside the quotes, so strip them here to
+ # match: localconst holds bare values.
+ return {m.group(1): m.group(2)[1:-1]
+ for m in LOCAL_DEFINE_RE.finditer(text)}
# ---------------------------------------------------------------------------
# C parsing, such as it is
# ---------------------------------------------------------------------------
CONST_RE = re.compile(
r'(?:static\s+)?(?:const\s+)?char\s*\*\s*([A-Za-z_][A-Za-z0-9_]*)\s*=\s*'
r'"((?:[^"\\]|\\.)*)"')
# Kent style puts function bodies at column 0, so a bare "safef(...)" at
# column 0 looks like a function definition. Requiring a return type plus
# whitespace (or a star) before the name is what keeps that from matching.
FUNCDEF_RE = re.compile(
r'^(?:static\s+)?(?:INLINE\s+)?(?:const\s+)?'
r'(?:struct\s+[A-Za-z_]\w*|unsigned\s+\w+|[A-Za-z_]\w*)'
r'(?:\s+\**\s*|\s*\*+\s*)'
r'([A-Za-z_]\w*)\s*\([^;]*$')
CTH_RE = re.compile(r'\bcart\w*ClosestToHome\s*\(')
FMT_RE = re.compile(
r'\b(?:safef|dyStringPrintf|sqlDyStringPrintf|printf|jsInlineF)\s*\(')
def split_args(s):
"""Split a C argument list at top-level commas, respecting strings."""
out, depth, cur, i, instr = [], 0, "", 0, False
while i < len(s):
c = s[i]
if instr:
cur += c
if c == "\\":
cur += s[i+1:i+2]
i += 2
continue
if c == '"':
instr = False
i += 1
continue
if c == '"':
instr = True
cur += c
i += 1
continue
if c in "([{":
depth += 1
elif c in ")]}":
depth -= 1
if c == "," and depth == 0:
out.append(cur.strip())
cur = ""
i += 1
continue
cur += c
i += 1
if cur.strip():
out.append(cur.strip())
return out
-def resolve(arg, localconst, macro):
- """Turn an argument into the string it evaluates to, if we can."""
+def resolve(arg, localconst, macro, conflict=None):
+ """Turn an argument into the string it evaluates to, if we can.
+
+ conflict is the set of names the tree defines inconsistently; without the
+ file's own definition to go on, those stay {NAME} rather than taking
+ whichever value was seen first.
+ """
arg = arg.strip()
if re.fullmatch(r'"((?:[^"\\]|\\.)*)"', arg):
return arg[1:-1]
toks = re.findall(r'"(?:[^"\\]|\\.)*"|[A-Za-z_][A-Za-z0-9_]*', arg)
plain = re.sub(r'"(?:[^"\\]|\\.)*"|[A-Za-z_][A-Za-z0-9_]*|\s+', '', arg)
if plain == "" and toks:
# nothing but literals and identifiers, i.e. C string concatenation
vals = []
for t in toks:
if t.startswith('"'):
vals.append(t[1:-1])
elif t in localconst:
vals.append(localconst[t])
+ elif conflict and t in conflict:
+ vals.append("{" + t + "}")
elif t in macro:
vals.append(macro[t])
else:
vals.append("{" + t + "}")
return "".join(vals)
return "EXPR:" + re.sub(r'\s+', ' ', arg)[:60]
def match_close(txt, i):
"""Index of the paren that closes the one at i."""
depth, j, instr = 0, i, False
while j < len(txt):
c = txt[j]
if instr:
if c == "\\":
j += 2
continue
if c == '"':
instr = False
elif c == '"':
instr = True
elif c == "(":
depth += 1
elif c == ")":
depth -= 1
if depth == 0:
return j
j += 1
return len(txt) - 1
def enclosing_functions(lines):
"""Map 1-based line number to the name of the function containing it."""
encl = [None] * (len(lines) + 2)
cur = None
for idx, line in enumerate(lines):
if (line and not line[0].isspace()
and not line.startswith(("#", "/", "*", "}", "{"))):
m = FUNCDEF_RE.match(line)
if m:
cur = m.group(1)
encl[idx + 1] = cur
return encl
-def scan_file(fp, rel, macro):
+def scan_file(fp, rel, macro, conflict=None):
"""Return one record per track-scoped cart name found in one file."""
txt = open(fp, errors="replace").read()
+ # The file's own #defines join its char * constants: both are what this
+ # file means, whatever the rest of the tree calls the same name.
localconst = {m.group(1): m.group(2) for m in CONST_RE.finditer(txt)}
+ localconst.update(local_defines(txt))
encl = enclosing_functions(txt.split("\n"))
starts = [0]
for i, ch in enumerate(txt):
if ch == "\n":
starts.append(i + 1)
def lineno(pos):
lo, hi = 0, len(starts) - 1
while lo < hi:
mid = (lo + hi + 1) // 2
if starts[mid] <= pos:
lo = mid
else:
hi = mid - 1
return lo + 1
recs = []
for m in CTH_RE.finditer(txt):
i = m.end() - 1
args = split_args(txt[i+1:match_close(txt, i)])
if len(args) >= 4:
ln = lineno(m.start())
- recs.append(dict(var=resolve(args[3], localconst, macro),
+ recs.append(dict(var=resolve(args[3], localconst, macro,
+ conflict),
file=rel, line=ln, func=encl[ln], how="cth"))
for m in FMT_RE.finditer(txt):
i = m.end() - 1
args = split_args(txt[i+1:match_close(txt, i)])
ln = lineno(m.start())
fi = None
for k, a in enumerate(args):
if a.strip().startswith('"'):
fi = k
break
if fi is None:
continue
- fmt = resolve(args[fi], localconst, macro)
+ fmt = resolve(args[fi], localconst, macro, conflict)
rest = args[fi+1:]
mm = re.match(r'^%s([._])(.*)$', fmt or "")
if not mm:
continue
sep, tail = mm.group(1), mm.group(2)
if "%" not in tail and tail:
recs.append(dict(var=sep+tail, file=rel, line=ln,
func=encl[ln], how="fmtlit"))
elif tail == "%s" and len(rest) >= 2:
# "%s.%s", track, SUFFIX -> the suffix is the SECOND vararg
- v = resolve(rest[1], localconst, macro)
+ v = resolve(rest[1], localconst, macro, conflict)
if v and not v.startswith("EXPR:"):
recs.append(dict(var=sep+v, file=rel, line=ln,
func=encl[ln], how="fmt"))
elif tail == "%s.%s" and len(rest) >= 3:
- v1 = resolve(rest[1], localconst, macro)
- v2 = resolve(rest[2], localconst, macro)
+ v1 = resolve(rest[1], localconst, macro, conflict)
+ v2 = resolve(rest[2], localconst, macro, conflict)
if not v1.startswith("EXPR:") and not v2.startswith("EXPR:"):
recs.append(dict(var=sep+v1+"."+v2, file=rel, line=ln,
func=encl[ln], how="fmt3"))
return recs
# ---------------------------------------------------------------------------
# entry point for the catalog next door
# ---------------------------------------------------------------------------
def harvest(dirs=None, quiet=False):
"""Scan the tree and return the raw records.
cartTrackVarCatalog.py --reconcile imports this, so the scan has one
definition rather than one here and a second one written out by hand.
"""
dirs = dirs or [d.strip() for d in DEFAULT_DIRS.split(",") if d.strip()]
- macro = build_macros(dirs)
+ macro, conflict = build_macros(dirs)
files = []
for d in dirs:
p = os.path.join(ROOT, d)
if not os.path.isdir(p):
sys.exit("no such directory: %s" % p)
for fn in sorted(os.listdir(p)):
if fn.endswith(".c"):
files.append(os.path.join(p, fn))
records = []
for fp in files:
- records.extend(scan_file(fp, os.path.relpath(fp, ROOT), macro))
+ records.extend(scan_file(fp, os.path.relpath(fp, ROOT), macro,
+ conflict))
if not quiet:
print("scanned %d files in %d dirs, %d macros, %d records"
% (len(files), len(dirs), len(macro), len(records)),
file=sys.stderr)
return records
def resolved(records):
"""name -> first file:line, for the names the scan resolved to a literal.
An EXPR: or {ident} record marks a name built at run time, which is signal
for a person reading the harvester output but cannot be compared against a
catalog of literal names, so it is dropped here.
"""
out = {}
for r in sorted(records, key=lambda r: (r["file"], r["line"])):
var = r["var"]
if var.startswith("EXPR:") or "{" in var:
continue
out.setdefault(var, "%s:%d" % (r["file"], r["line"]))
return out
# ---------------------------------------------------------------------------
# main
# ---------------------------------------------------------------------------
def main():
ap = argparse.ArgumentParser(
description=__doc__,
formatter_class=argparse.RawDescriptionHelpFormatter)
ap.add_argument("--dirs", default=DEFAULT_DIRS,
help="comma-separated dirs under the kent src root to "
"scan (default: %s)" % DEFAULT_DIRS)
ap.add_argument("--json", metavar="FILE", help="write raw records as JSON")
ap.add_argument("--by-func", action="store_true",
help="group by file and function")
ap.add_argument("--by-var", action="store_true",
help="group by variable, listing where each is used")
ap.add_argument("--keep-unresolved", action="store_true",
help="include {ident} and EXPR: entries in the groupings")
args = ap.parse_args()
dirs = [d.strip() for d in args.dirs.split(",") if d.strip()]
records = harvest(dirs)
def wanted(var):
if args.keep_unresolved:
return True
return not var.startswith("EXPR:") and "{" not in var
if args.json:
with open(args.json, "w") as f:
json.dump(records, f, indent=1)
print("wrote %s" % args.json, file=sys.stderr)
if args.by_func:
byfunc = collections.defaultdict(set)
for r in records:
if wanted(r["var"]):
byfunc[(r["file"], r["func"])].add(r["var"])
for k in sorted(byfunc):
print("%s %s()" % (k[0], k[1]))
print(" " + ", ".join(sorted(byfunc[k])))
if args.by_var:
byvar = collections.defaultdict(set)
for r in records:
if wanted(r["var"]):
byvar[r["var"]].add("%s:%d" % (r["file"], r["line"]))
for k in sorted(byvar, key=str.lower):
print("%-34s %d %s"
% (k, len(byvar[k]), " ".join(sorted(byvar[k])[:4])))
if not (args.json or args.by_func or args.by_var):
print("nothing to do; pass --by-func, --by-var or --json",
file=sys.stderr)
return 1
return 0
if __name__ == "__main__":
sys.exit(main())