754f5637634694624fd359811c60513966f3c1a9 braney Sat Sep 5 07:58:46 2026 -0700 cartTrackVarCatalog: read a filename as a filename, not as a cart variable. The harvester looks for a track-scoped name built as safef(buf, size, "%s.%s", track, SUFFIX). Code that builds "%s.tmp" from a filename has exactly that shape, so every such site arrived as a name a person had to write down in cartVarsNotCataloged.txt as not-a-cart-variable. There were 15 of them, and they were arriving at a rate of one every few weeks: .tmp came in on 2026-08-27 with writeMergedHubFile, _ss.ps on 2026-09-04 with the RNA fold fix. The docstring predicted the class from the start and still asked for it to be thrown away by hand. harvestCartVars.py now answers the question with fileNameLike(), which asks whether the name's trailing dot-separated component is a file extension. FILE_SUFFIXES holds only the extensions the tree builds today plus tbi beside bai: each entry is a name nobody classifies again, so a guessed one adds a way to lose a real cart variable and buys nothing. Two cleverer tests were tried and rejected, and the reasons are in the docstring: the destination buffer's declaration does not decide it, since the .bai and .link.bb sites format into a plain buf and buffer while psName and tmpName are char[PATH_LEN]; and neither does the argument being formatted, which is a filename at some sites, a url at others and a table name at a third set. The records still carry these names, because a harvest that hides what it saw cannot be checked. What changed is that --reconcile no longer asks a person about them, and --update-baseline no longer writes them back. They are listed under --reconcile --verbose, and harvestCartVars.py --filenames prints the rule's claims with their call sites. Two things guard against the rule going wrong in the direction that would matter. --reconcile tests cataloged() before the filename rule, so a name the catalog describes can never be suppressed by it. And --check now fails if any cataloged name would be read as a filename, which is the case where the two halves of this file disagree about what a name is; verified with a planted row that it reports rather than passing. Baseline 70 names to 55. refs #37838 diff --git src/hg/utils/cartTrackVarCatalog/cartTrackVarCatalog.py src/hg/utils/cartTrackVarCatalog/cartTrackVarCatalog.py index 76077a7932b..0e8c0230dc8 100755 --- src/hg/utils/cartTrackVarCatalog/cartTrackVarCatalog.py +++ src/hg/utils/cartTrackVarCatalog/cartTrackVarCatalog.py @@ -1526,30 +1526,44 @@ for alias, target in (e.get("aliases") or {}).items(): if not vals: print("%s: alias %r on an entry with no values" % (name, alias), file=sys.stderr) bad += 1 elif target not in vals: print("%s: alias %r -> %r, which is not in values" % (name, alias, target), file=sys.stderr) bad += 1 elif alias in vals: print("%s: alias %r is also a value, so it cannot be " "translated" % (name, alias), file=sys.stderr) bad += 1 if e["type"] == "enum" and vals and not e.get("valuesSrc"): noSrc.append(name) + # The filename rule in the harvester decides that a name is not a cart + # variable, so it must never be able to say that about one the catalog + # describes. Nothing warns if it starts to: --reconcile checks + # cataloged() first, so such a name would simply stop being reported and + # keep its row. Checked here instead, where a new extension in + # FILE_SUFFIXES or a new catalog row shows up as an error. + h = harvestModule() + if h is not None: + for e in all_vars(): + if e["name"] and h.fileNameLike(e["name"]): + print("%s: the harvester's filename rule would read this " + "cataloged name as a filename; the two disagree about " + "what it is" % e["name"], file=sys.stderr) + bad += 1 print("values ok" if not bad else "values: %d problem(s)" % bad, file=out) # Not an error. It is the backlog: an enum whose values nobody has # checked against the C array that actually gates them. print("enums with no valuesSrc: %d (unverified value lists)" % len(noSrc), file=out) return bad def key(name): """A harvested and a cataloged name reduced to a comparable form. The leading separator goes because the harvester cannot always report it; see the module docstring. """ return name.lstrip("._") @@ -1662,67 +1676,87 @@ def write_baseline(names, sites=None, path=BASELINE_FILE): """Write the baseline, annotating each name with the file that reads it. The comment carries the file but not the line, so that ordinary edits above a call site do not rewrite hundreds of lines here and bury the one name that actually changed. """ with open(path, "w") as f: f.write("""\ # cartVarsNotCataloged.txt - names that harvestCartVars.py finds at a # cart*ClosestToHome() or safef("%s.%s") call site but that cartTrackVarCatalog.py # does not describe as a track-scoped cart variable. Refs #37838. # # Most are not cart variables at all: the scan cannot tell one from a table -# name, a filename suffix or an SQL fragment, so .bai, _gold and .tbi come out -# of it too. Some are cart variables that simply have not been cataloged yet. +# name or an SQL fragment, so _gold comes out of it too. Some are cart +# variables that simply have not been cataloged yet. +# +# Filenames are NOT in here. A "%s.tmp" built from a filename has the same +# shape as a "%s.heightPer" built from a track name, and 15 names of that kind +# used to sit below with a new one arriving every few weeks. The harvester now +# reads them from the trailing extension instead: harvestCartVars.py +# --filenames lists what that rule claims and explains it. # # cartTrackVarCatalog.py --reconcile complains about any harvested name in # neither the catalog nor this file, so this is what keeps a nightly run quiet # until something actually changes. Regenerate with --update-baseline, then read # the diff before committing: a name appearing here is a decision that it is not # a cart variable worth cataloging, and a name disappearing means its call site # went away. # # The first version of this file was accepted wholesale, as a snapshot of the # gap on the day reconcile learned to fail. So a name being in here is not # evidence that anybody has looked at it; only the ones added since, which # arrive a few at a time in a reviewable diff, carry that weight. # # Names are stored with the leading separator stripped, which is how reconcile # compares them. """) sites = sites or {} for n in sorted(names): where = sites.get(n, "").rsplit(":", 1)[0] if where: f.write("%-34s # %s\n" % (n, where)) else: f.write("%s\n" % n) -def harvested(): - """(name -> file:line) for every literal name the tree yields, or None. +def harvestModule(): + """The harvester next door, or None if it cannot be imported. - Keyed the same way the catalog is, so the two are directly comparable. + Imported by name inside a function rather than at module scope on purpose. + registryPages loads this file by path, and in that process the sibling + directory is not on sys.path, so a top-level import would break a consumer + that only wants the catalog and never asks for a harvest. """ try: import harvestCartVars as h except ImportError: print("harvestCartVars.py not importable from here", file=sys.stderr) return None + return h + + +def harvested(): + """(name -> file:line) for every literal name the tree yields, or None. + + Keyed the same way the catalog is, so the two are directly comparable. + """ + h = harvestModule() + if h is None: + return None out = {} for name, src in h.resolved(h.harvest(quiet=True)).items(): k = key(name) if k: out.setdefault(k, src) return out def reconcile(cat, out=sys.stdout, verbose=False): """Diff the catalog against the names the tree actually builds. The one thing here that needs a person is a name the tree builds that is in neither the catalog nor the baseline, because that is a track-scoped variable somebody added without saying what it is. A catalog entry with no call site found is not: several are read through a helper or spelled with a @@ -1733,51 +1767,68 @@ tree = harvested() if tree is None: return 1 # A scan that finds almost nothing is a broken scan, not a clean tree, and # the difference matters: pointed at the wrong KENT_SRC or an empty clone, # everything below would come up empty and report all clear forever. if len(tree) < MIN_TREE_NAMES: print("only %d names found: expected at least %d, so the scan is " "broken rather\nthan the tree being clean. Check KENT_SRC." % (len(tree), MIN_TREE_NAMES), file=out) return 1 cataloged, literals, patterns = cataloged_test() baseline = read_baseline() - - new = sorted(n for n in tree if not cataloged(n) and n not in baseline) + h = harvestModule() # harvested() above already proved it imports + + # A filename is not a cart variable, and the scan cannot tell the two + # apart: "%s.tmp" built from a filename has the shape of "%s.heightPer" + # built from a track name. h.fileNameLike() answers that from the + # trailing extension, so those names no longer need a baseline line each. + # The test comes after cataloged(), so a name the catalog describes is + # never hidden by it, and h.fileNameLike is checked against the catalog's + # own literals by --check. + files = sorted(n for n in tree + if not cataloged(n) and h.fileNameLike(n)) + new = sorted(n for n in tree if not cataloged(n) and n not in baseline + and not h.fileNameLike(n)) only_cat = sorted(n for n in literals if n not in tree) if verbose: print("catalog literal names %d" % len(literals), file=out) print("catalog patterns %d" % len(patterns), file=out) print("harvested names %d" % len(tree), file=out) print("baseline names %d" % len(baseline), file=out) print("\nin the catalog, no call site found (%d)" % len(only_cat), file=out) print(" (expected for a name read through a helper or built from a " "macro the\n scan cannot follow; anything else is an entry " "whose read has gone away)", file=out) for n in only_cat: print(" %s" % n, file=out) known = sorted(n for n in tree if n in baseline) print("\nharvested, in the baseline rather than the catalog (%d)" % len(known), file=out) for n in known: print(" %-30s %s" % (n, tree[n]), file=out) + print("\nharvested, read as a filename rather than a cart variable " + "(%d)" % len(files), file=out) + print(" (harvestCartVars.py --filenames explains the rule)", + file=out) + for n in files: + print(" %-30s %s" % (n, tree[n]), file=out) if new: print("\nbuilt by the tree, in neither the catalog nor the baseline " "(%d):" % len(new), file=out) print(" (add an entry to cartTrackVarCatalog.py if it is a " "track-scoped cart\n variable, otherwise accept it with " "--update-baseline)", file=out) for n in new: print(" %-30s %s" % (n, tree[n]), file=out) return len(new) # --------------------------------------------------------------------------- # HTML rendering # --------------------------------------------------------------------------- @@ -2074,32 +2125,36 @@ ap.add_argument("--verbose", action="store_true", help="with --reconcile, also print the standing drift " "that needs no action") ap.add_argument("--update-baseline", dest="updateBaseline", action="store_true", help="rewrite %s from the current tree; read the diff " "before committing it" % os.path.basename(BASELINE_FILE)) args = ap.parse_args() cat = build() if args.updateBaseline: tree = harvested() if tree is None: return 1 cataloged, _, _ = cataloged_test() + h = harvestModule() # harvested() above already proved it imports was = read_baseline() - now = set(n for n in tree if not cataloged(n)) + # The same two exclusions --reconcile makes, or accepting the backlog + # would write back every filename the harvester reads as a name. + now = set(n for n in tree + if not cataloged(n) and not h.fileNameLike(n)) write_baseline(now, tree) print("wrote %s: %d names, %d added, %d dropped" % (BASELINE_FILE, len(now), len(now - was), len(was - now))) return 0 if args.reconcile: return 1 if reconcile(cat, verbose=args.verbose) else 0 if args.json: with open(args.json, "w") as f: json.dump(cat, f, indent=1) print("wrote %s" % args.json) if args.html: with open(args.html, "w") as f: f.write(render_html(cat)) print("wrote %s" % args.html) if args.check or not (args.json or args.html):