Catch misspelled / wrong-stage env var references in flow scripts#4191
Catch misspelled / wrong-stage env var references in flow scripts#4191oharboe wants to merge 1 commit intoThe-OpenROAD-Project:masterfrom
Conversation
Add a typo guard in util.tcl: env_var_equals, env_var_exists_and_non_empty, env_var_or_empty, append_env_var, and source_env_var_if_exists now error if the variable name is not declared in variables.yaml for the current stage. erase_non_stage_variables populates the per-stage declared set; utility scripts that don't run it (e.g. open.tcl) fall back to the full set. Add declared_variables.py to emit the per-stage declared set as a sibling of non_stage_variables.py. Declare 6 helper-referenced vars that were missing from variables.yaml: DEF_FILE, V_FILE, LIB_MODEL, RTLMP_RPT_FILE, RTLMP_BLOCKAGE_FILE, SDC_FILE_CLOCK_PERIOD. Widen FOOTPRINT and FOOTPRINT_TCL to stages [floorplan, place]. Fixes a latent bug: io_placement.tcl and global_place.tcl read them in the place stage but they were declared floorplan-only and so were always silently empty after erase_non_stage_variables. Regenerate variables.json and docs/user/FlowVariables.md via yaml_to_json.py and generate-variables-docs.py. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
There was a problem hiding this comment.
Code Review
This pull request implements a typo guard for ORFS variables by validating environment variable lookups against declarations in variables.yaml. It introduces a Python script to fetch valid variables for specific stages and updates Tcl utility functions to enforce these checks. Additionally, several new variables are added and documented. Feedback was provided to improve the robustness of the Tcl execution logic by explicitly calling the Python interpreter and ensuring the output is correctly parsed as a list.
| set cmd [list $::env(SCRIPTS_DIR)/declared_variables.py] | ||
| if { $stage_name ne "" } { | ||
| lappend cmd $stage_name | ||
| } | ||
| set ::orfs_declared_vars [exec {*}$cmd] |
There was a problem hiding this comment.
The current implementation of orfs_load_declared_vars relies on the script being executable and correctly handled by the shell. For better robustness and portability across different environments, it is recommended to explicitly call python3 and use file join for path construction. Additionally, using split and string trim on the exec output ensures that the resulting variable list is clean and correctly formatted as a Tcl list, avoiding potential issues with trailing newlines or platform-specific line endings.
set cmd [list python3 [file join $::env(SCRIPTS_DIR) declared_variables.py]]
if { $stage_name ne "" } {
lappend cmd $stage_name
}
set ::orfs_declared_vars [split [string trim [exec {*}$cmd]] "\n"]
|
leave be for now |
Add a typo guard in util.tcl: env_var_equals,
env_var_exists_and_non_empty, env_var_or_empty, append_env_var, and source_env_var_if_exists now error if the variable name is not declared in variables.yaml for the current stage. erase_non_stage_variables populates the per-stage declared set; utility scripts that don't run it (e.g. open.tcl) fall back to the full set.
Add declared_variables.py to emit the per-stage declared set as a sibling of non_stage_variables.py.
Declare 6 helper-referenced vars that were missing from variables.yaml: DEF_FILE, V_FILE, LIB_MODEL, RTLMP_RPT_FILE, RTLMP_BLOCKAGE_FILE, SDC_FILE_CLOCK_PERIOD.
Widen FOOTPRINT and FOOTPRINT_TCL to stages [floorplan, place]. Fixes a latent bug: io_placement.tcl and global_place.tcl read them in the place stage but they were declared floorplan-only and so were always silently empty after erase_non_stage_variables.
Regenerate variables.json and docs/user/FlowVariables.md via yaml_to_json.py and generate-variables-docs.py.