Repository navigation
flow: read variables.yaml directly, delete variables.json - #4800
Merged
maliberty merged 2 commits intoNov 8, 2026
Merged
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>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
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.
Member
🔍 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), 2 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>
Collaborator
Author
|
@maliberty @vvbandeira FYI fewer papercuts |
maliberty
approved these changes
Nov 8, 2026
oharboe
added a commit
to oharboe/OpenROAD-flow-scripts
that referenced
this pull request
Nov 8, 2026
variables.json is no longer read since The-OpenROAD-Project#4800, so it is deleted in a pull request of its own rather than kept in step here. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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