Repository navigation
Conversation
variables.json was added in The-OpenROAD-Project#4113 for The-OpenROAD-Project#4103: OpenROAD's `bazel test test/...` ran the flow through bazel-orfs, and non_stage_variables.py, exec'd from util.tcl inside the Bazel sandbox, found no PyYAML. OpenROAD has since dropped bazel-orfs and test/orfs (The-OpenROAD-Project/OpenROAD#11546), ORFS no longer carries Bazel build files (c4d858d), and bazel-orfs sets PYTHON_EXE to a Python that has PyYAML for every action. A plain make run uses the host python3, and the DependencyInstaller (etc/requirements-common.in) and flake.nix install PyYAML, as they did before The-OpenROAD-Project#4113. defaults.py and non_stage_variables.py go back to reading variables.yaml as they did before The-OpenROAD-Project#4113, and variables.json, yaml_to_json.py, the CI check that kept them in sync and the linguist-generated attribute go away. Both scripts print the same lines as before, in the YAML's order rather than sorted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
There was a problem hiding this comment.
Code Review
This pull request removes the variables.json file and its conversion script, migrating the variable definitions entirely to YAML. The Python scripts defaults.py and non_stage_variables.py are updated to import yaml and load variables.yaml directly using yaml.safe_load instead of parsing JSON. All review comments have been filtered out as they suggest one-off cleanups regarding encoding arguments, which are out of scope according to the repository's style guide. Consequently, there is no additional feedback to provide.
🔍 QoR checkMetrics reflect the PR merge build — i.e. what will land on the target branch. Commit 62 design(s) checked — 0 with regression(s), 0 without a comparable baseline. |
variables.json changes with nearly every variables.yaml edit on master, so deleting it here conflicts with master again and again while this is in review. Keep the file as it is on master; nothing in the flow reads it after this change, and a separate pull request deletes it once this one has landed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018PFLaCzFtFbnWE4nPTDwpC Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
|
@maliberty @vvbandeira FYI fewer papercuts |
Before:
flow/scripts/variables.jsonwas a generated copy ofvariables.yamlcommitted to git, withyaml_to_json.pyto regenerate it and a CI check to catch a forgotten regeneration.Now:
defaults.pyandnon_stage_variables.pyreadvariables.yaml, and the JSON, the converter and the check are gone.Why the JSON was added: #4113, for #4103. OpenROAD's
bazel test test/...ran the flow through bazel-orfs.non_stage_variables.py, whichutil.tclruns inside the Bazel sandbox, found no PyYAML.Why it is no longer needed:
test/orfsin bazel: retire bazel-orfs and test/orfs OpenROAD#11546.PYTHON_EXEto a Python with PyYAML for every action.makerun uses the hostpython3. The DependencyInstaller (etc/requirements-common.in) andflake.nixinstall PyYAML for it, as they did before fix: remove PyYAML runtime dependency from Bazel flow scripts #4113.Checked: on master,
defaults.pyandnon_stage_variables.py <stage>print the same lines as the JSON version for every stage, in the YAML's order instead of sorted.make print-*on asap7 gcd works.variables.json stays for now: this PR leaves
flow/scripts/variables.jsonuntouched, so it does not conflict with the constantvariables.jsonchurn on master while it is in review. Nothing reads it after this PR.variables.jsonis deleted in a separate PR after this one lands.🤖 Generated with Claude Code