Skip to content

Commit cd5f4c7

Browse files
committed
fix: address gemini-code-assist nits on The-OpenROAD-Project#4229
Two follow-ups from gemini-code-assist review of PR The-OpenROAD-Project#4229; the other five nits no longer apply (three targeted bazel/yosys-check.sh and bazel/make-yosys-netlist.sh which moved upstream to bazel-orfs in commit 432edcd63; two were already addressed in commit c18e424 when gds/gds.gz were added and the _export_design_files helper was factored out). - flow/designs/design.bzl: make _export_design_files() idempotent via a sentinel filegroup. design() and files() both call it, and a BUILD file may legitimately call files() more than once (e.g. files("verilog") and files("lef") in the same package). A second native.exports_files over the same paths is a duplicate-target error, so the sentinel short-circuits subsequent calls in the same package. The sentinel rule itself is private to the package. - flow/scripts/synth.sh: capture $(realpath "$2") once into log_file rather than re-running it for both the stderr append and the tee target in the genElapsedTime.py epilogue. Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
1 parent c44b22d commit cd5f4c7

2 files changed

Lines changed: 18 additions & 2 deletions

File tree

flow/designs/design.bzl

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ _GROUPS = {
1919
# gds/gds.gz are inputs in hierarchical flows via ADDITIONAL_GDS.
2020
_EXPORTED_EXTS = ["v", "sv", "svh", "tcl", "sdc", "def", "cfg", "lef", "lib", "gds", "gds.gz"]
2121

22+
_EXPORTS_SENTINEL = "_orfs_design_exports_sentinel"
23+
2224
def _export_design_files():
2325
"""Publicly export per-file labels for cross-package references.
2426
@@ -28,13 +30,26 @@ def _export_design_files():
2830
resolve only if the source package calls exports_files() on the
2931
individual files — being part of a public filegroup is not
3032
sufficient.
33+
34+
Idempotent: design() and files() both call this, and a BUILD file
35+
may legitimately call files() more than once (e.g. files("verilog")
36+
and files("lef") in the same package). A second native.exports_files
37+
over the same paths is a duplicate-target error, so a sentinel rule
38+
short-circuits subsequent calls within the same package.
3139
"""
40+
if _EXPORTS_SENTINEL in native.existing_rules():
41+
return
3242
exported = native.glob(
3343
["*.{}".format(e) for e in _EXPORTED_EXTS],
3444
allow_empty = True,
3545
)
3646
if exported:
3747
native.exports_files(exported, visibility = ["//visibility:public"])
48+
native.filegroup(
49+
name = _EXPORTS_SENTINEL,
50+
srcs = [],
51+
visibility = ["//visibility:private"],
52+
)
3853

3954
def design(config = "config.mk", user_arguments = [], local_arguments = []):
4055
"""Standard BUILD body for flow/designs/<platform>/<design>/.

flow/scripts/synth.sh

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ $PYTHON_EXE "$SCRIPTS_DIR/run_command.py" --log "$(realpath $2)" --append --tee
1414
# stderr into the log rather than silently dropping it, so a real bug
1515
# in the helper is still discoverable after the fact.
1616
stage=$(basename "$2" .log)
17+
log_file=$(realpath "$2")
1718
"$PYTHON_EXE" "$UTILS_DIR/genElapsedTime.py" --match "$stage" -d "$LOG_DIR" \
18-
2>>"$(realpath "$2")" \
19-
| tee -a "$(realpath "$2")" || true
19+
2>>"$log_file" \
20+
| tee -a "$log_file" || true

0 commit comments

Comments
 (0)