Skip to content
VLSI Mentor

Python Part II · Module 36 · Python for VLSI Design Review

Reviewing Tool Control: Reproducibility, Environment, Cleanup

The layer that launches the tool is the one a reviewer can test most directly and usually does not, because reading it tells you almost nothing: the code that builds a command line looks correct whether or not the command line it builds can be run again. So the review is a single action — take the cmd.sh the tool left behind, paste it into a shell, and see whether the failure comes back. Of five specimens here, one does.

Advanced25 min readPythonPart IIAutomationCode ReviewReproducibility

Part II · Module 36 · Chapter 36.3 · Reviewing Tool Control: Reproducibility, Environment, Cleanup

1. Five Properties, One of Them Testable in Seconds

Azvya Education Pvt. Ltd.VLSI Mentor
Snippet
PROPERTIES = [
    ("the command is recorded", "paste cmd.sh and run it", "seconds"),
    ("the environment is explicit", "read cmd.sh for exports", "seconds"),
    ("every call is bounded", "walk the AST for timeout=", "a minute"),
    ("the paths are portable", "grep for absolute and ~ paths", "a minute"),
    ("the run dir survives failure", "make it fail, then look", "a minute"),
]

print(f"{'property':<32} {'how a reviewer checks it':<28} cost")
for what, how, cost in PROPERTIES:
    print(f"{what:<32} {how:<28} {cost}")

print()
print("properties                     :", len(PROPERTIES))
print("checkable without reading code :", 3)
print("the first one subsumes          ")
print("  two of the others             :", "the cd and the exports")
print()
print("so the review's first action is  :", "run cmd.sh")
Azvya Education Pvt. Ltd.VLSI Mentor
Snippet
property                         how a reviewer checks it     cost
the command is recorded          paste cmd.sh and run it      seconds
the environment is explicit      read cmd.sh for exports      seconds
every call is bounded            walk the AST for timeout=    a minute
the paths are portable           grep for absolute and ~ paths a minute
the run dir survives failure     make it fail, then look      a minute

properties                     : 5
checkable without reading code : 3
the first one subsumes          
  two of the others             : the cd and the exports

so the review's first action is  : run cmd.sh

Running cmd.sh tests three of the five properties at once, because a command that reproduces has necessarily recorded its directory and its environment. That is why the annotation names it as the single action.

2. The Annotation

Azvya Education Pvt. Ltd.VLSI Mentor
Snippet
# can a reviewer paste cmd.sh and reproduce the failure?
Azvya Education Pvt. Ltd.VLSI Mentor
Snippet
WHO = [
    ("the author, on their machine", "almost always", "their cwd, their env"),
    ("the author, a week later", "often", "the env may have moved"),
    ("a reviewer, on their machine", "rarely", "different cwd and env"),
    ("CI, from a clean checkout", "the real test", "nothing is set"),
]

print(f"{'who tries to reproduce':<32} {'succeeds':<16} what they have")
for who, succeeds, have in WHO:
    print(f"{who:<32} {succeeds:<16} {have}")

print()
print("the author is the worst tester of this:",
      "their environment is already correct")
print("the reviewer is a good proxy for      :", "CI and for the next owner")
print()
print("so 'it works for me' means            :", "nothing about cmd.sh")
print("and the review is                     :", "the first honest test")
Azvya Education Pvt. Ltd.VLSI Mentor
Snippet
who tries to reproduce           succeeds         what they have
the author, on their machine     almost always    their cwd, their env
the author, a week later         often            the env may have moved
a reviewer, on their machine     rarely           different cwd and env
CI, from a clean checkout        the real test    nothing is set

the author is the worst tester of this: their environment is already correct
the reviewer is a good proxy for      : CI and for the next owner

so 'it works for me' means            : nothing about cmd.sh
and the review is                     : the first honest test

The author is the worst possible tester of reproducibility because their environment is already correct — every variable set, every path relative to a directory they happen to be in. A reviewer on a different machine is the first person for whom cmd.sh has to actually work, which makes the review the earliest honest test and not a formality.

3. Five Specimens, One Reproduces

Azvya Education Pvt. Ltd.VLSI Mentor
Snippet
import os
import shlex
import subprocess
import sys
from pathlib import Path

# The "tool" the specimens invoke. Its exit codes say WHY it failed, so a
# failed reproduction is diagnosable rather than just unsuccessful.
TOOL = '''import os
import sys

args = sys.argv[1:]
if os.environ.get("FLOW_MODE", "") != "strict":
    print("MODE NOT SET")
    sys.exit(4)
if not os.path.isfile("rtl/files.f"):
    print("NO FILELIST")
    sys.exit(3)
if "--label" not in args:
    print("NO LABEL")
    sys.exit(2)
label = args[args.index("--label") + 1]
if label != "nightly run":
    print(f"BAD LABEL {label!r}")
    sys.exit(2)
print("REPRODUCED")
sys.exit(1)
'''

run_dir = Path("run")
(run_dir / "rtl").mkdir(parents=True, exist_ok=True)
(run_dir / "rtl" / "files.f").write_text("rtl/a.v\n", encoding="utf-8")
(run_dir / "tool.py").write_text(TOOL, encoding="utf-8")

PY = shlex.quote(sys.executable)
HERE = shlex.quote(str(run_dir.resolve()))
LABEL = shlex.quote("nightly run")
# The TOOL path is absolute in every specimen, so the only thing `cd` affects
# is how the tool resolves its own inputs — which is the defect under review.
TOOL_PATH = shlex.quote(str((run_dir / "tool.py").resolve()))

SPECIMENS = {
    "complete": f'#!/bin/sh\ncd {HERE}\nexport FLOW_MODE=strict\n'
                f'{PY} {TOOL_PATH} --label {LABEL}\n',
    "no cd": f'#!/bin/sh\nexport FLOW_MODE=strict\n'
             f'{PY} {TOOL_PATH} --label {LABEL}\n',
    "no exports": f'#!/bin/sh\ncd {HERE}\n'
                  f'{PY} {TOOL_PATH} --label {LABEL}\n',
    "unquoted arg": f'#!/bin/sh\ncd {HERE}\nexport FLOW_MODE=strict\n'
                    f'{PY} {TOOL_PATH} --label nightly run\n',
    "bare python": f'#!/bin/sh\ncd {HERE}\nexport FLOW_MODE=strict\n'
                   f'python {TOOL_PATH} --label {LABEL}\n',
}

print(f"{'cmd.sh specimen':<16} {'rc':>4} {'first line':<24} reproduces?")
reproduced = []
for name, body in SPECIMENS.items():
    Path("cmd.sh").write_text(body, encoding="utf-8")
    os.chmod("cmd.sh", 0o755)
    r = subprocess.run(["/bin/sh", "cmd.sh"], stdout=subprocess.PIPE,
                       stderr=subprocess.STDOUT, text=True)
    first = (r.stdout.strip().splitlines() or ["(nothing)"])[0]
    ok = r.returncode == 1 and "REPRODUCED" in r.stdout
    if ok:
        reproduced.append(name)
    print(f"{name:<16} {r.returncode:>4} {first[:24]:<24} {ok}")

print()
print("specimens    :", len(SPECIMENS))
print("that reproduce:", len(reproduced), reproduced)
print()
print("every failure has a DIFFERENT exit code:", True)
print("so a failed reproduction is            :", "diagnosable")
Azvya Education Pvt. Ltd.VLSI Mentor
Snippet
cmd.sh specimen    rc first line               reproduces?
complete            1 REPRODUCED               True
no cd               3 NO FILELIST              False
no exports          4 MODE NOT SET             False
unquoted arg        2 BAD LABEL 'nightly'      False
bare python       127 cmd.sh: line 4: python:  False

specimens    : 5
that reproduce: 1 ['complete']

every failure has a DIFFERENT exit code: True
so a failed reproduction is            : diagnosable

One of five reproduces, and each of the four failures is distinguishable. That property is not accidental: the specimen tool was written to exit a different code for each precondition, which is Chapter 33.8's contract applied to a diagnostic. A tool that exits 1 for everything would make all four failures look like "it did not reproduce" and a reviewer could not say why.

Each failure is worth naming because the review comment differs:

4. Four Ways a cmd.sh Fails

Azvya Education Pvt. Ltd.VLSI Mentor
Snippet
FAILURES = [
    ("no cd", 3, "NO FILELIST",
     "entries resolve against the reviewer's cwd — 35.7 §4",
     "record `cd` to the run dir"),
    ("no exports", 4, "MODE NOT SET",
     "the tool read a variable the script never recorded — 34.5",
     "export every variable the tool reads"),
    ("unquoted arg", 2, "BAD LABEL 'nightly'",
     "a space split one argument into two",
     "build the line with shlex.quote — 23.1 §6"),
    ("bare python", 127, "command not found",
     "`python` does not exist here — 32.3 §3",
     "record sys.executable, not 'python'"),
]

print(f"{'specimen':<14} {'rc':>4} {'symptom':<22} review comment")
for name, rc, symptom, _, fix in FAILURES:
    print(f"{name:<14} {rc:>4} {symptom:<22} {fix}")

print()
print("the four causes:")
for name, _, _, cause, _ in FAILURES:
    print(f"  {name:<14} {cause}")

print()
print("the subtlest one      :", "unquoted arg")
print("because the command    ")
print("  LOOKS right          :", "--label nightly run")
print("and the tool reports   :", "BAD LABEL 'nightly'")
print()
print("rc 127 is produced by  :", "the shell, not the tool")
print("which distinguishes    :",
      "a missing interpreter from a tool that ran and refused")
Azvya Education Pvt. Ltd.VLSI Mentor
Snippet
specimen         rc symptom                review comment
no cd             3 NO FILELIST            record `cd` to the run dir
no exports        4 MODE NOT SET           export every variable the tool reads
unquoted arg      2 BAD LABEL 'nightly'    build the line with shlex.quote — 23.1 §6
bare python     127 command not found      record sys.executable, not 'python'

the four causes:
  no cd          entries resolve against the reviewer's cwd — 35.7 §4
  no exports     the tool read a variable the script never recorded — 34.5
  unquoted arg   a space split one argument into two
  bare python    `python` does not exist here — 32.3 §3

the subtlest one      : unquoted arg
because the command    
  LOOKS right          : --label nightly run
and the tool reports   : BAD LABEL 'nightly'

rc 127 is produced by  : the shell, not the tool
which distinguishes    : a missing interpreter from a tool that ran and refused

The unquoted argument is the subtlest because the recorded line reads correctly to a human — --label nightly run is what the author typed — and the shell splits it into two words, so the tool receives nightly and an unexpected run. Chapter 23.1 Section 6's shlex.quote exists for exactly this, and a reviewer can spot it by looking for any recorded argument containing a space without quotes around it.

rc 127 comes from the shell, not from the tool, which is a useful distinction when triaging a failed reproduction: 127 means the command was never started, so the problem is the recorded interpreter or PATH, and any other code means the tool ran and declined.

5. The Environment the Tool Actually Reads

Azvya Education Pvt. Ltd.VLSI Mentor
Snippet
import ast

SPECIMEN = '''import os
import subprocess
from pathlib import Path


def run(test, seed):
    sim = os.environ["FLOW_SIM"]
    root = Path(os.environ.get("FLOW_RUN_ROOT", "runs"))
    extra = os.environ.get("FLOW_SIM_ARGS", "").split()
    os.environ["UVM_HOME"] = "/opt/uvm-1.2"
    return subprocess.run([sim, *extra], cwd=root,
                          env={**os.environ, "SEED": str(seed)})
'''

RECORDED = {"FLOW_SIM", "FLOW_RUN_ROOT"}      # what cmd.sh exports


def env_reads(tree):
    reads, writes = set(), set()
    for n in ast.walk(tree):
        if isinstance(n, ast.Subscript) and "environ" in ast.dump(n.value) \
           and isinstance(n.slice, ast.Constant):
            if isinstance(n.ctx, ast.Store):
                writes.add(n.slice.value)
            else:
                reads.add(n.slice.value)
        if isinstance(n, ast.Call) and isinstance(n.func, ast.Attribute) \
           and n.func.attr == "get" and "environ" in ast.dump(n.func) \
           and n.args and isinstance(n.args[0], ast.Constant):
            reads.add(n.args[0].value)
    return reads, writes


reads, writes = env_reads(ast.parse(SPECIMEN))

print("variables the tool READS :", sorted(reads))
print("variables the tool WRITES:", sorted(writes))
print("variables cmd.sh RECORDS :", sorted(RECORDED))
print()
print("read but not recorded    :", sorted(reads - RECORDED))
print("recorded but not read    :", sorted(RECORDED - reads))
print()
print("the unrecorded one        :", "FLOW_SIM_ARGS")
print("its default is            :", '"" — so it is silently empty — 34.5 §3')
print("what a reviewer sees in   ")
print("  cmd.sh                  :", "nothing at all about it")
print()
print("and the variable the tool WRITES:", sorted(writes))
print("which cmd.sh need not record    :", "the tool sets it itself")
Azvya Education Pvt. Ltd.VLSI Mentor
Snippet
variables the tool READS : ['FLOW_RUN_ROOT', 'FLOW_SIM', 'FLOW_SIM_ARGS']
variables the tool WRITES: ['UVM_HOME']
variables cmd.sh RECORDS : ['FLOW_RUN_ROOT', 'FLOW_SIM']

read but not recorded    : ['FLOW_SIM_ARGS']
recorded but not read    : []

the unrecorded one        : FLOW_SIM_ARGS
its default is            : "" — so it is silently empty — 34.5 §3
what a reviewer sees in   
  cmd.sh                  : nothing at all about it

and the variable the tool WRITES: ['UVM_HOME']
which cmd.sh need not record    : the tool sets it itself

One variable is read and not recorded, and it is the dangerous kind. FLOW_SIM_ARGS defaults to the empty string, so a run with it set and a reproduction without it both work — and produce different simulations. Chapter 34.5 Section 3 measured that os.environ.get(name, "") treats set-but-empty as a value; here the same shape makes a reproduction silently incomplete.

The read/write distinction is worth making in a review. UVM_HOME is set by the tool, so cmd.sh does not need to record it — recording it would be harmless but would also hide the fact that the tool controls it, and a reviewer who sees it exported might reasonably assume the site sets it.

6. Is Every Call Bounded?

Azvya Education Pvt. Ltd.VLSI Mentor
Snippet
import ast
import subprocess

print("=== what boundedness buys")
try:
    subprocess.run(["sleep", "5"], timeout=0.2)
    print("  timeout=0.2 on `sleep 5`: returned normally")
except subprocess.TimeoutExpired:
    print("  timeout=0.2 on `sleep 5`: TimeoutExpired raised")
print("  with no timeout at all   :",
      "the call returns when the tool decides, or never")

SPECIMEN = '''import subprocess


def build(sources):
    return subprocess.run(["iverilog", "-o", "a.vvp"] + sources, timeout=600)


def simulate(vvp):
    return subprocess.run(["vvp", vvp])


def lint(path):
    return subprocess.check_output(["lint", path])


def legacy(cmd):
    return subprocess.call(cmd, shell=True)
'''

RUNNERS = {"run", "call", "check_output", "check_call", "Popen"}


def audit_calls(src):
    rows = []
    for n in ast.walk(ast.parse(src)):
        if isinstance(n, ast.Call) and isinstance(n.func, ast.Attribute) \
           and n.func.attr in RUNNERS:
            kw = {k.arg for k in n.keywords}
            rows.append({"call": f"subprocess.{n.func.attr}",
                         "line": n.lineno,
                         "bounded": "timeout" in kw,
                         "shell": "shell" in kw})
    return rows


rows = audit_calls(SPECIMEN)
print()
print(f"{'call':<24} {'line':>5} {'bounded':>9} {'shell=True':>11}")
for r in rows:
    print(f"{r['call']:<24} {r['line']:>5} {str(r['bounded']):>9} "
          f"{str(r['shell']):>11}")

print()
print("tool invocations  :", len(rows))
print("bounded           :", sum(1 for r in rows if r["bounded"]))
print("UNBOUNDED         :", sum(1 for r in rows if not r["bounded"]))
print("with shell=True   :", sum(1 for r in rows if r["shell"]))
print()
print("the unbounded simulate() means:", "a hung simulation hangs the flow")
print("and 33.12's pre-flight cannot ")
print("  help because                :", "the hang happens during the run")
Azvya Education Pvt. Ltd.VLSI Mentor
Snippet
=== what boundedness buys
  timeout=0.2 on `sleep 5`: TimeoutExpired raised
  with no timeout at all   : the call returns when the tool decides, or never

call                      line   bounded  shell=True
subprocess.run               5      True       False
subprocess.run               9     False       False
subprocess.check_output     13     False       False
subprocess.call             17     False        True

tool invocations  : 4
bounded           : 1
UNBOUNDED         : 3
with shell=True   : 1

the unbounded simulate() means: a hung simulation hangs the flow
and 33.12's pre-flight cannot 
  help because                : the hang happens during the run

One of four calls is bounded. The audit is six lines of ast and produces a finding per call site with a line number, which is the form a review comment wants — and the unbounded simulate() is the one that matters, because a simulation is exactly the call that can hang.

The shell=True on the last row is a second finding from the same walk. It means the command is a string the shell will re-parse, so Chapter 23.1's quoting problem returns and a filename containing a space or a ; changes what runs. A reviewer finding both on one line — unbounded and shell=True — has found the highest-risk invocation in the file without reading what it does.

7. Does the Run Directory Survive a Failure?

Azvya Education Pvt. Ltd.VLSI Mentor
Snippet
import shutil
from pathlib import Path

SCRATCH = {"obj", "work", "csrc"}
EVIDENCE_SUFFIX = {".log", ".sh", ".json"}


def build_run_dir(name):
    d = Path(name)
    for sub in SCRATCH:
        (d / sub).mkdir(parents=True, exist_ok=True)
        (d / sub / "big.tmp").write_text("x", encoding="utf-8")
    d.mkdir(parents=True, exist_ok=True)
    for f in ("sim.log", "compile.log", "cmd.sh", "results.json",
              "scratch.tmp"):
        (d / f).write_text("x", encoding="utf-8")
    return d


def cleanup_everything(d):
    """Specimen A: tidy, and useless after a failure."""
    shutil.rmtree(d)


def cleanup_nothing(d):
    """Specimen B: keeps everything, including gigabytes of scratch."""
    return


def cleanup_selective(d):
    """Specimen C: 33.13's rule."""
    for child in sorted(Path(d).iterdir()):
        if child.is_dir() and child.name in SCRATCH:
            shutil.rmtree(child)
        elif child.is_file() and child.suffix not in EVIDENCE_SUFFIX:
            child.unlink()


SPECIMENS = [("remove the run dir", cleanup_everything),
             ("keep everything", cleanup_nothing),
             ("remove scratch only", cleanup_selective)]

NEEDED = {"cmd.sh", "sim.log", "compile.log"}

print(f"{'cleanup specimen':<22} {'dir exists':<12} {'kept':<34} usable?")
for label, fn in SPECIMENS:
    d = build_run_dir(f"run_{label.split()[0]}")
    fn(d)
    exists = d.exists()
    kept = sorted(p.name for p in d.iterdir() if p.is_file()) if exists else []
    usable = NEEDED <= set(kept)
    print(f"{label:<22} {str(exists):<12} {str(kept)[:34]:<34} {usable}")

print()
print("what a reviewer needs after a failure:", sorted(NEEDED))
print("specimens that provide it            :", 1)
print()
print("specimen A is                        :", "tidy and unusable")
print("specimen B is                        :", "usable and fills the volume")
print("specimen C is                        :", "33.13 §7's classification")
Azvya Education Pvt. Ltd.VLSI Mentor
Snippet
cleanup specimen       dir exists   kept                               usable?
remove the run dir     False        []                                 False
keep everything        True         ['cmd.sh', 'compile.log', 'results True
remove scratch only    True         ['cmd.sh', 'compile.log', 'results True

what a reviewer needs after a failure: ['cmd.sh', 'compile.log', 'sim.log']
specimens that provide it            : 1

specimen A is                        : tidy and unusable
specimen B is                        : usable and fills the volume
specimen C is                        : 33.13 §7's classification

Two of three cleanups leave a usable run directory and one removes it entirely. Specimen A is the one to flag: shutil.rmtree(run_dir) in a finally is the tidiest-looking line in a flow and it deletes the only evidence a failure produced.

The difference between B and C is not usability but volume — both keep what a reviewer needs, and only C removes the scratch. So the review has two separate questions here, and conflating them is how a team ends up with A: is the evidence kept is about correctness, and is the scratch removed is about disk, and the answer to the first must never be traded for the second.

8. The Tool-Control Review, Assembled

Azvya Education Pvt. Ltd.VLSI Mentor
Snippet
import ast
from dataclasses import dataclass
from pathlib import Path

RUNNERS = {"run", "call", "check_output", "check_call", "Popen"}


@dataclass(frozen=True)
class Finding:
    area: str
    severity: str
    detail: str


def review_tool_control(src, cmd_sh):
    findings = []
    tree = ast.parse(src)

    # --- environment: read against recorded
    reads = set()
    for n in ast.walk(tree):
        if isinstance(n, ast.Subscript) and "environ" in ast.dump(n.value) \
           and isinstance(n.slice, ast.Constant) \
           and not isinstance(n.ctx, ast.Store):
            reads.add(n.slice.value)
        if isinstance(n, ast.Call) and isinstance(n.func, ast.Attribute) \
           and n.func.attr == "get" and "environ" in ast.dump(n.func) \
           and n.args and isinstance(n.args[0], ast.Constant):
            reads.add(n.args[0].value)
    recorded = {line.split("=", 1)[0].removeprefix("export ").strip()
                for line in cmd_sh.splitlines()
                if line.startswith("export ")}
    for name in sorted(reads - recorded):
        findings.append(Finding("environment", "major",
                                f"${name} is read and not exported by cmd.sh"))

    # --- boundedness and shell
    for n in ast.walk(tree):
        if isinstance(n, ast.Call) and isinstance(n.func, ast.Attribute) \
           and n.func.attr in RUNNERS:
            kw = {k.arg for k in n.keywords}
            if "timeout" not in kw:
                findings.append(Finding(
                    "boundedness", "major",
                    f"line {n.lineno}: subprocess.{n.func.attr} has no "
                    f"timeout"))
            if "shell" in kw:
                findings.append(Finding(
                    "quoting", "major",
                    f"line {n.lineno}: shell=True re-parses the command"))

    # --- cmd.sh contents
    if "cd " not in cmd_sh:
        findings.append(Finding("reproducibility", "blocker",
                                "cmd.sh has no `cd`: it depends on the cwd"))
    for line in cmd_sh.splitlines():
        if line.startswith("python ") or " python " in line:
            findings.append(Finding("reproducibility", "blocker",
                                    "cmd.sh invokes bare `python` — 32.3 §3"))
    # an argument with a space and no quoting
    for line in cmd_sh.splitlines():
        if line.startswith(("#", "cd ", "export ")):
            continue
        if "--label" in line and "'" not in line and '"' not in line:
            findings.append(Finding("quoting", "blocker",
                                    "an argument with a space is unquoted"))

    # --- portability
    for n in ast.walk(tree):
        if isinstance(n, ast.Constant) and isinstance(n.value, str):
            if n.value.startswith("/") and n.value.count("/") > 1:
                findings.append(Finding("portability", "minor",
                                        f"absolute path literal "
                                        f"{n.value!r}"))
            if n.value.startswith("~"):
                findings.append(Finding("portability", "major",
                                        f"per-user path {n.value!r}"))
    return findings


SPECIMEN_SRC = '''import os
import subprocess
from pathlib import Path

ROOT = "/proj/chip_a/rtl"


def run(test, seed):
    sim = os.environ["FLOW_SIM"]
    extra = os.environ.get("FLOW_SIM_ARGS", "").split()
    subprocess.run([sim, *extra, test])
    subprocess.call(f"grep -c UVM_ERROR {test}.log", shell=True)
'''

SPECIMEN_CMD = ("#!/bin/sh\n"
                "export FLOW_SIM=iverilog\n"
                "python tool.py --label nightly run\n")

findings = review_tool_control(SPECIMEN_SRC, SPECIMEN_CMD)
ORDER = {"blocker": 0, "major": 1, "minor": 2}

print("tool-control review")
for f in sorted(findings, key=lambda f: (ORDER[f.severity], f.area, f.detail)):
    print(f"  [{f.severity:<7}] {f.area:<16} {f.detail}")

print()
print("findings  :", len(findings))
for sev in ("blocker", "major", "minor"):
    print(f"  {sev:<8} {sum(1 for f in findings if f.severity == sev)}")
print()
print("areas covered:", sorted({f.area for f in findings}))
print("lines of specimen:", len(SPECIMEN_SRC.strip().splitlines()))
Azvya Education Pvt. Ltd.VLSI Mentor
Snippet
tool-control review
  [blocker] quoting          an argument with a space is unquoted
  [blocker] reproducibility  cmd.sh has no `cd`: it depends on the cwd
  [blocker] reproducibility  cmd.sh invokes bare `python` — 32.3 §3
  [major  ] boundedness      line 11: subprocess.run has no timeout
  [major  ] boundedness      line 12: subprocess.call has no timeout
  [major  ] environment      $FLOW_SIM_ARGS is read and not exported by cmd.sh
  [major  ] quoting          line 12: shell=True re-parses the command
  [minor  ] portability      absolute path literal '/proj/chip_a/rtl'

findings  : 8
  blocker  3
  major    4
  minor    1

areas covered: ['boundedness', 'environment', 'portability', 'quoting', 'reproducibility']
lines of specimen: 12

Eight findings in eleven lines, three of them blockers. The three blockers are all in cmd.sh rather than in the code, which is the chapter's point: the most serious defects in tool control are in the artefact that records what happened, and no amount of reading the invocation code finds them.

Note that line 11 produces two findings — unbounded and shell=True — and the report lists them separately rather than merging. A reviewer fixes them differently, so a combined comment would get half-addressed.

9. Are the Paths Portable?

The fourth property in Section 1 got one minor finding in Section 8, and it deserves a pass of its own, because path literals are not one defect class but four, with different severities:

Azvya Education Pvt. Ltd.VLSI Mentor
Snippet
import ast
from collections import Counter

SPECIMEN = "\n".join([
    "import os",
    "from pathlib import Path",
    "",
    'SIM = "/tools/cadence/xcelium/19.09/bin/xrun"',
    'SCRATCH = "/proj/chip_a/users/azhar/runs"',
    'BASELINE = "~/.flow/baseline.json"',
    'FILELIST = "rtl/files.f"',
    'LOG = os.environ.get("FLOW_RUN_ROOT", "runs") + "/sim.log"',
])


def classify_path(value):
    """Four classes, three severities. Most specific test first."""
    if value.startswith("~"):
        return "per-user", "blocker", "depends on who ran it"
    if value.startswith("/") and any(c.isdigit() for c in value) \
       and value.count("/") > 3:
        return "versioned tool", "blocker", "breaks on upgrade day"
    if value.startswith("/"):
        return "absolute", "major", "one machine's layout"
    if "/" in value:
        return "relative", "minor", "needs a declared root"
    return None, None, None


rows = []
for node in ast.walk(ast.parse(SPECIMEN)):
    if isinstance(node, ast.Constant) and isinstance(node.value, str):
        cls, sev, why = classify_path(node.value)
        if cls:
            rows.append((node.value, cls, sev, why))

print(f"{'literal':<38} {'class':<15} {'severity':<8} why")
for value, cls, sev, why in rows:
    print(f"{value:<38} {cls:<15} {sev:<8} {why}")

print()
print("path literals found :", len(rows))
for cls, n in sorted(Counter(r[1] for r in rows).items()):
    print(f"  {cls:<15} {n}")
print("portable as written :", 0)
Azvya Education Pvt. Ltd.VLSI Mentor
Snippet
literal                                class           severity why
/tools/cadence/xcelium/19.09/bin/xrun  versioned tool  blocker  breaks on upgrade day
/proj/chip_a/users/azhar/runs          absolute        major    one machine's layout
~/.flow/baseline.json                  per-user        blocker  depends on who ran it
rtl/files.f                            relative        minor    needs a declared root
/sim.log                               absolute        major    one machine's layout

path literals found : 5
  absolute        2
  per-user        1
  relative        1
  versioned tool  1
portable as written : 0

Five literals, four classes, none portable as written — and the last row is a false positive worth keeping rather than quietly tidying away.

/sim.log is not a path. It is the right-hand operand of os.environ.get("FLOW_RUN_ROOT", "runs") + "/sim.log" — a concatenation fragment that merely begins with a slash, which is exactly the over-count Chapter 34.1 Section 3 measured when a naive scan reported eleven distinct paths where seven were real. This audit inherits that flaw honestly: it looks at string constants without looking at what they are joined to.

That is acceptable in a review instrument as long as the reviewer knows it. Four of the five findings are real, and the fifth is a prompt to look at the last line — which is worth looking at anyway, because that line does path arithmetic with + instead of /:

Azvya Education Pvt. Ltd.VLSI Mentor
Snippet
import os
from pathlib import Path

os.environ.pop("FLOW_RUN_ROOT", None)
root = os.environ.get("FLOW_RUN_ROOT", "runs")
print("without a trailing slash")
print("  string +  :", repr(root + "/sim.log"))
print("  Path /    :", repr(str(Path(root) / "sim.log")))

os.environ["FLOW_RUN_ROOT"] = "runs/"
root = os.environ["FLOW_RUN_ROOT"]
joined = root + "/sim.log"
paved = str(Path(root) / "sim.log")
print()
print("with a trailing slash in the variable")
print("  string +  :", repr(joined))
print("  Path /    :", repr(paved))
del os.environ["FLOW_RUN_ROOT"]

print()
print("the string form doubles the slash:", "//" in joined)
print("the Path form does not           :", "//" not in paved)
print("the two spellings are equal      :", joined == paved)
print("and they name one file           :", True)
Azvya Education Pvt. Ltd.VLSI Mentor
Snippet
without a trailing slash
  string +  : 'runs/sim.log'
  Path /    : 'runs/sim.log'

with a trailing slash in the variable
  string +  : 'runs//sim.log'
  Path /    : 'runs/sim.log'

the string form doubles the slash: True
the Path form does not           : True
the two spellings are equal      : False
and they name one file           : True

A trailing slash in the variable turns + into runs//sim.log where / gives runs/sim.log. Nothing fails: the doubled slash opens the same file on POSIX. The two strings are simply unequal while naming one file, which is precisely the duplicate Chapter 35.7 Section 7 measured — resolve() collapses ., .. and //, and a checker comparing raw strings does not.

So the review comment on that line is not about style. Joining with + produces values that compare unequal while being equal, and the flow's own duplicate detection goes blind to them.

10. Predict, Run, Observe

Azvya Education Pvt. Ltd.VLSI Mentor
Snippet
import os
import shlex
import subprocess
import sys
from pathlib import Path

Path("d").mkdir(exist_ok=True)
(Path("d") / "marker").write_text("here\n", encoding="utf-8")

# A
Path("a.sh").write_text("#!/bin/sh\ncat marker\n", encoding="utf-8")
r = subprocess.run(["/bin/sh", "a.sh"], stdout=subprocess.PIPE,
                   stderr=subprocess.DEVNULL, text=True)
print("A:", r.returncode, repr(r.stdout.strip()))

# B
Path("b.sh").write_text(f"#!/bin/sh\ncd {shlex.quote('d')}\ncat marker\n",
                        encoding="utf-8")
r = subprocess.run(["/bin/sh", "b.sh"], stdout=subprocess.PIPE,
                   stderr=subprocess.DEVNULL, text=True)
print("B:", r.returncode, repr(r.stdout.strip()))

# C
Path("c.sh").write_text("#!/bin/sh\npython -c 'print(1)'\n", encoding="utf-8")
r = subprocess.run(["/bin/sh", "c.sh"], stdout=subprocess.DEVNULL,
                   stderr=subprocess.DEVNULL)
print("C:", r.returncode)

# D
try:
    subprocess.run(["sleep", "5"], timeout=0.2)
    print("D:", "returned")
except subprocess.TimeoutExpired:
    print("D:", "TimeoutExpired")

The verified output:

Azvya Education Pvt. Ltd.VLSI Mentor
Snippet
A: 1 ''
B: 0 'here'
C: 127
D: TimeoutExpired

A — no cd, so cat marker fails: the file is in d/ and the script runs where it was invoked. That is the "no cd" specimen in two values.

B — with the cd, the same command works. One line is the whole difference between a cmd.sh that reproduces and one that does not.

C — bare python gives 127, the shell's command-not-found. Chapter 32.3 Section 3 established that python does not exist on this machine, and a cmd.sh recording it is unrunnable here no matter how correct the rest is.

D — timeout=0.2 raises TimeoutExpired rather than waiting. The exception is the point: a bounded call fails in a way the caller can handle, and an unbounded one has no failure mode at all, which is why "it has never hung" is not evidence.

11. Exercises

Exercise 1 — five properties. Check each one for a script your team owns.

Exercise 2 — three at once. Explain why running cmd.sh tests more than one property.

Exercise 3 — the author is the worst tester. Say why, in one sentence.

Exercise 4 — be the reviewer. Take a colleague's cmd.sh and run it on your machine.

Exercise 5 — diagnosable failures. Make your tool exit a different code per precondition.

Exercise 6 — no cd. Delete the cd from a working cmd.sh and record what breaks.

Exercise 7 — no exports. Do the same for one export and record the symptom.

Exercise 8 — the unquoted argument. Put a space in a label and see how the tool receives it.

Exercise 9 — bare python. Confirm what python does on your machine and what rc you get.

Exercise 10 — rc 127. Explain what it tells you that any other code does not.

Exercise 11 — check for absences. Write the four-item presence list for cmd.sh.

Exercise 12 — compose the two reviews. Use Chapter 36.1's input audit to produce the variable list, then check cmd.sh against it.

Exercise 13 — read against recorded. Run the comparison on your own tool.

Exercise 14 — the silently empty variable. Find one with an empty default and say what a reproduction without it produces.

Exercise 15 — read or written. Separate the variables your tool sets from the ones it consumes.

Exercise 16 — audit boundedness. Walk your own source for subprocess calls and count the bounded ones.

Exercise 17 — the call that can hang. Identify it and say what a timeout would do.

Exercise 18 — shell=True. Find one and describe what a filename with a space does.

Exercise 19 — the highest-risk line. Find a call that is both unbounded and shell=True.

Exercise 20 — three cleanups. Implement all three and check which leave a usable directory.

Exercise 21 — tidy and unusable. Search your flow for an rmtree of a run directory.

Exercise 22 — two separate questions. State them, and say why the answers must not be traded.

Exercise 23 — eight findings. Run the assembled review on one of your scripts.

Exercise 24 — where the blockers were. Note which artefact they were in.

Exercise 25 — two findings on one line. Explain why they are reported separately.

Exercise 26 — it has never hung. Explain why that is not evidence of boundedness.

Exercise 27 — four classes. Classify every path literal in one of your own tools.

Exercise 28 — the versioned one. Find a tool path with a version in it and describe the upgrade-day failure.

Exercise 29 — per-user before absolute. Explain why the Callout ranks them that way.

Exercise 30 — the relative one. Say what it needs before it means anything.

Exercise 31 — the false positive. Find a concatenation fragment your audit reports as a path.

Exercise 32 — follow it anyway. Explain why that finding is still worth acting on.

Exercise 33 — + against /. Join a path both ways with a trailing slash in the variable.

Exercise 34 — the invisible duplicate. Say what a doubled slash does to a string-comparing duplicate check, and name the chapter that measured it.

12. Summary

  • Running cmd.sh tests three of the five properties at once, because a command that reproduces has necessarily recorded its directory and environment.
  • The author is the worst tester of reproducibility — their environment is already correct — so the review is the first honest test and a proxy for CI.
  • One of five cmd.sh specimens reproduced, and each of the four failures had a different exit code because the specimen tool exits a distinct code per precondition.
  • No cd makes entries resolve against the reviewer's working directory — Chapter 35.7 Section 4's finding arriving in the recorded command.
  • No export leaves the tool reading a variable nobody recorded.
  • An unquoted argument is the subtlest defect: --label nightly run reads correctly and the shell splits it, so the tool receives nightly.
  • Bare python gives rc 127 from the shell, which distinguishes a command that never started from a tool that ran and declined.
  • Review cmd.sh for what it does not contain. All four defects are absences, and absences are what reading misses.
  • One variable read and not recorded is the dangerous kind when its default is "": the run and the reproduction both work and simulate differently.
  • Separate the variables the tool sets from the ones it reads — recording the first hides the fact that the tool controls it.
  • One of four subprocess calls was bounded. The audit is six lines of ast and names a line number per finding.
  • Unbounded plus shell=True on one line is the highest-risk invocation in a file, findable without reading what it does.
  • Two of three cleanups left a usable run directory and one removed it. shutil.rmtree(run_dir) is the tidiest-looking line in a flow and deletes the only evidence a failure produced.
  • "Is the evidence kept" and "is the scratch removed" are separate questions, and the first must never be traded for the second.
  • Eight findings in eleven lines, and all three blockers were in cmd.sh rather than in the code.
  • "It has never hung" is not evidence of boundedness — an unbounded call has
  • Path literals are four classes with three severities: per-user and versioned-tool are blockers, absolute is major, relative is minor and needs a declared root.
  • None of the five literals in the specimen was portable as written.
  • Severity is about who is blocked: an absolute path is wrong for everyone identically, a per-user path is wrong differently for each person — so it works for its author and reads the wrong baseline for everyone else.
  • One finding was a false positive — /sim.log is a concatenation fragment, the same over-count Chapter 34.1 Section 3 measured — and it is still worth following, because the line it points at joins paths with +.
  • + gives runs//sim.log where / gives runs/sim.log. Both open one file and the strings are unequal, which is exactly the duplicate Chapter 35.7 Section 7's string comparison cannot see. no failure mode to observe.

The next chapter reviews the system that calls all of this many times at once, where the question stops being about one run and becomes whether every job is accounted for.

Where this fits

Part of the Python curriculum.