Skip to content

[Builder] Check verify_steps prerequisites are actually run - #1673

Open
mumallaeng wants to merge 1 commit into
Xilinx:devfrom
mumallaeng:check-verification-prereqs
Open

[Builder] Check verify_steps prerequisites are actually run#1673
mumallaeng wants to merge 1 commit into
Xilinx:devfrom
mumallaeng:check-verification-prereqs

Conversation

@mumallaeng

Copy link
Copy Markdown

Fix #1428.

Problem

Each VerificationStepType entry (verify_steps) is only ever triggered from inside one specific step - stitched_ip_rtlsim from step_create_stitched_ip, node_by_node_rtlsim from step_hw_ipgen, and so on. If the configured steps/start_step/stop_step excludes that step (e.g. stop_step set before phase_build_hardware while verify_steps still asks for node_by_node_rtlsim), the verification is silently never run. The build reports success with no indication that the requested check never happened - exactly the failure mode #1428 describes.

Fix

Added a verify_step_prereq check to run_all_config_checks in build_dataflow_checks.py. It resolves the actual list of steps that will run via resolve_build_steps (the same function build_dataflow.py uses internally) and flags any verify_steps entry whose owning phase or fine-grained step isn't in that resolved list.

resolve_build_steps is imported lazily via importlib.import_module rather than a top-level import, since build_dataflow.py imports build_dataflow_checks.py - a top-level import back would be circular. .ruff.toml enables PLC (including PLC0415, import-outside-top-level), so a plain function-local from ... import ... would fail lint; importlib.import_module isn't an Import/ImportFrom AST node, so it isn't flagged.

Testing

Ran the full test_build_dataflow_checks.py suite locally (setup-local.sh-style venv, no Docker/Vivado) - all 15 tests pass, including 3 new ones: a verify_steps entry whose phase is excluded by stop_step (errors), one whose phase is included (no error), and a custom steps list naming the fine-grained step directly instead of its phase (also no error, since the check accepts either). isort/black/ruff (pinned versions from .pre-commit-config.yaml) pass on both changed files.

Each verify_steps entry only ever gets triggered from inside one
specific step (e.g. stitched_ip_rtlsim from step_create_stitched_ip).
If the configured steps/start_step/stop_step leaves that step out,
the entry is silently never verified - the build reports success
with no indication that the requested check didn't happen.

Add a verify_step_prereq check to run_all_config_checks: resolve
the actual step list via resolve_build_steps and flag any
verify_steps entry whose owning phase or fine-grained step isn't
in it. resolve_build_steps is imported lazily via importlib, since
build_dataflow imports this module and a top-level import back
would be circular.

Verified with pytest locally (setup-local.sh-style venv, no
Docker/Vivado) - all 15 tests pass, including 3 new ones covering
the missing-phase error, the phase-present pass-through, and a
custom steps list naming the fine-grained step directly.

Signed-off-by: mumallaeng <mumallaeng@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant