Skip to content

We're not using runfiles anymore, don't use associated macro define.#10634

Open
hzeller wants to merge 3 commits into
The-OpenROAD-Project:masterfrom
hzeller:feature-20260611-bazel-build
Open

We're not using runfiles anymore, don't use associated macro define.#10634
hzeller wants to merge 3 commits into
The-OpenROAD-Project:masterfrom
hzeller:feature-20260611-bazel-build

Conversation

@hzeller

@hzeller hzeller commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

We used to use runfiles to work with tcl dependencies. Since we don't need that anymore, just add a regular BAZEL_BUILD define to choose the behavior in the bazel build and remove the unneeded dependency.

We used to use runfiles to work with tcl dependencies. Since we don't
need that anymore, just add a regular `BAZEL_BUILD` define to
choose the behavior in the bazel build and remove the unneeded
dependency.

Signed-off-by: Henner Zeller <h.zeller@acm.org>
@hzeller hzeller requested a review from a team as a code owner June 10, 2026 22:37
@hzeller hzeller requested a review from maliberty June 10, 2026 22:37

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request replaces the preprocessor macro BAZEL_CURRENT_REPOSITORY with BAZEL_BUILD in BUILD.bazel and src/Main.cc. The reviewer feedback highlights several opportunities for cleanup now that Bazel runfiles are no longer used. Specifically, it is recommended to remove obsolete dependencies in BUILD.bazel (@rules_cc//cc/runfiles and @boost.dll), eliminate the unused boost/dll/runtime_symbol_info.hpp header, and completely remove the dead setupBazelRunfilesEnvironment() function and its invocation.

Comment thread BUILD.bazel
Comment thread src/Main.cc
Comment thread src/Main.cc Outdated
Comment thread src/Main.cc Outdated
@maliberty

Copy link
Copy Markdown
Member

Gemini comments seem worth answering/fixing

@hzeller

hzeller commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator Author

Yes, I was about to fix these in a separate commit as it was unclear why this was used, as they were added very recently.

Peter @gadfort, you recently added these setup runfile functions and I am wondering why ? We're not using tcl from runfiles in a while now, the resources are all compiled-in (since #10094), so the openroad binary can be copied anywhere without needing any runfiles.

But the change #10591 looks deliberate, so maybe something else is going on ? Can you shed some light, Peter ?

Since tcl resources are compmiled-in, we don't need that anymore.

Signed-off-by: Henner Zeller <h.zeller@acm.org>
@hzeller

hzeller commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator Author

Removed the set-up in 40677ea
@gadfort please have a look

Signed-off-by: Henner Zeller <h.zeller@acm.org>
@hzeller hzeller requested review from a team as code owners June 11, 2026 07:59
@hzeller

hzeller commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator Author

oops, accidentally added an unrealated boost cleanup into this PR (separate commit), but yeah, might as well keep it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants