bazel: split Qt GUI into separate :openroad-qt target (field-tested respin of #10384) by oharboe · Pull Request #10465 · The-OpenROAD-Project/OpenROAD

oharboe · 2026-05-20T05:48:21Z

Summary

Respin of the closed-unmerged #10384, with a linker-correctness bugfix that field testing surfaced. Adds a dedicated :openroad-qt cc_binary so the choice between CLI and Qt becomes a target choice, not a build configuration.

New :openroad-qt cc_binary alongside :openroad — pick the one you want, no flag flip needed.
--//:platform=gui //:openroad continues to work as an additive flag (no removal); :openroad-qt is just preferred because it doesn't invalidate :openroad's cache.
New //src/gui:gui_stub splits src/stub.cpp out of :gui. Linked only by CLI :openroad / :_openroadpy.so. This is the bugfix vs. add an openroad-qt as an alternative to GUI Qt config to reduce build times #10384.

Why the gui_stub split (vs. #10384)

#10384 left src/stub.cpp inside :gui. With --allow-multiple-definition in effect (LLVM toolchain default), :openroad-qt ended up linking both:

the real Qt implementations from //src/gui:gui_qt, and
the stub implementations from :gui (pulled transitively via :ui modules)

…for symbols like Gui::enabled, startGui, initGui, Painter::getOptions. The stub silently won — binary built, GUI never opened at runtime. Splitting the stub into its own cc_library, depped only by the CLI binary, makes the link unambiguous.

Caught while field-testing the branch downstream. The rationale comment is in src/gui/BUILD so the next person doesn't re-merge them.

Why `:openroad-qt` (recap from #10384)

A skylib string_flag has no --host_ form, so the platform setting is silently dropped under Bazel's exec transition. Rule libraries that pull OpenROAD as a build-time tool (cfg = "exec") got a different binary than bazelisk build //:openroad under a downstream that set --@openroad//:platform=gui — same label, two binaries, two ~30min LTO links per consumer.

After this change, @openroad//:openroad denotes one binary regardless of who pulls it or in what configuration; @openroad//:openroad-qt is the opt-in Qt variant.

Migration

Interactive Qt use: bazelisk build @openroad//:openroad-qt
Build-time tool use (rule libraries via cfg = "exec"): no change.
--@openroad//:platform=gui still works but is no longer recommended.

Test plan

CI: bazelisk build //:openroad (default path, what tool consumers pull)
CI: bazelisk test //:openroad_qt_build_test (pins :openroad-qt compilability)
CI: macOS workflow builds :openroad-qt directly
Manual: bazelisk run //:openroad-qt -- -gui opens the Qt GUI (this is what regressed under add an openroad-qt as an alternative to GUI Qt config to reduce build times #10384)
Manual: bazelisk run //:openroad -- <existing-flow> unchanged

Replace the :platform string_flag (cli/gui) with two cc_binary targets: :openroad (CLI, default — what build-time tool consumers should pull) and :openroad-qt (Qt GUI variant — opt-in for interactive use). The choice between GUI variants becomes a target choice, not a build configuration. `--//:platform=gui //:openroad` continues to work as an additive flag path; :openroad-qt is preferred for ad-hoc GUI builds because it does not invalidate :openroad's cache the way the flag does. Why this change --------------- A skylib string_flag has no --host_ form, so the platform setting is silently dropped under Bazel's exec transition. Rule libraries that pull OpenROAD as a build-time tool (cfg = "exec") therefore got a *different* OpenROAD binary than `bazelisk build //:openroad` produced under a downstream that set `--@openroad//:platform=gui` -- same label, two binaries. With LTO, that is two ~30 min link steps for any consumer whose CI builds both directly and via flow stages. Beyond compile cost, the conceptual problem is target-name overload: @openroad//:openroad should denote one binary. Today, the binary it denotes depends on the consumer's transition. gui_stub: linker correctness fix vs. the closed The-OpenROAD-Project#10384 ------------------------------------------------------ The closed PR The-OpenROAD-Project#10384 left src/stub.cpp inside :gui. With --allow-multiple-definition in effect (LLVM toolchain default), :openroad-qt — which links //src/gui:gui_qt for the real Qt impls AND pulls :gui transitively via :ui modules — ended up with BOTH the stub and gui_qt definitions of Gui::enabled/startGui/initGui/ Painter::getOptions at link time, and the stub silently won. The binary built, but the GUI never opened at runtime. Field testing this branch caught it. Fix: split the stub into a new //src/gui:gui_stub cc_library that only the CLI :openroad binary (and :_openroadpy.so) pulls. :openroad-qt links gui_qt alone for those symbols. A build_test pins :openroad-qt's compilability since no downstream consumer would otherwise exercise it in CI. What changes ------------ - BUILD.bazel: add :openroad-qt cc_binary, split :openroad_lib into CLI (:openroad_lib) + Qt (:openroad_lib_qt) variants compiled with BUILD_GUI=false/true respectively, hoist shared deps into OPENROAD_MAIN_DEPS, add build_test for :openroad-qt. - src/gui/BUILD: factor src/stub.cpp out of :gui into new :gui_stub cc_library (see linker-correctness rationale above). What does not change -------------------- - CMake build (intentional; CMake is on its own deprecation path). - C++ source. The lone BUILD_GUI consumer (OpenRoad::getGUICompileOption() at src/OpenRoad.cc:753-756) is unmodified. - :_openroadpy.so — already CLI-only in practice; now references :openroad_lib (the CLI variant) and :gui_stub. Migration for downstreams ------------------------- - For interactive Qt use: bazelisk build @openroad//:openroad-qt - For build-time tool use (rule libraries pulling OpenROAD via cfg = "exec"): no change — @openroad//:openroad remains the CLI. - --@openroad//:platform=gui still works as an additive flag, but is no longer the recommended path because :openroad-qt does not invalidate :openroad's cache. Closes the gap from the (closed-unmerged) The-OpenROAD-Project#10384 with the field-tested linker fix folded in. Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

gemini-code-assist

Code Review

This pull request refactors the Bazel build system to better support separate CLI and Qt GUI variants of OpenROAD. Key changes include the introduction of an openroad-qt binary target, the creation of OPENROAD_MAIN_DEPS to share common dependencies, and the isolation of GUI stubs into a standalone gui_stub library to prevent link-time symbol conflicts. Feedback focused on refining the gui_stub target by removing redundant source files and unused dependencies, as well as ensuring consistent dependency naming for the ODB library.

github-actions · 2026-05-20T05:51:01Z

clang-tidy review says "All clean, LGTM! 👍"

The aggregate //src/odb target was removed (The-OpenROAD-Project#10416, "odb: remove the aggregate \"odb\" target and use the fine-grained targets"). gui_stub was the lone holdout depending on the old label. Use the fine-grained //src/odb/src/db target — that's what its sources (stub.cpp and bufferTreeDescriptor.h) actually need for odb/db.h, odb/geom.h, and odb/PtrSetMap.h. Found by local build of :openroad after rebasing onto current master. :openroad-qt was unaffected (does not dep :gui_stub). Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

oharboe · 2026-05-20T06:09:49Z

Local build verification:

bazelisk build //:openroad ✅
bazelisk build //:openroad-qt ✅

Initial :openroad analysis failed because the rebased patch still referenced the aggregate //src/odb target that was removed in #10416. Fixed in 290e1c6 — :gui_stub now deps the fine-grained //src/odb/src/db (which is what stub.cpp/bufferTreeDescriptor.h actually need for odb/db.h, odb/geom.h, odb/PtrSetMap.h).

:openroad-qt was unaffected (does not dep :gui_stub).

github-actions · 2026-05-20T06:12:04Z

clang-tidy review says "All clean, LGTM! 👍"

hzeller · 2026-05-20T07:01:17Z

CC @QuantamHD has he has the most experience with builds with and without UI.

oharboe · 2026-05-20T07:02:38Z

@hzeller Gemini is right here or should I ignore?

oharboe · 2026-05-20T07:40:03Z

Suckerpunched by openroad changes, will bump and rebase patch downstream, test and update here

Both were copy-paste leftovers from :gui that gemini-code-assist flagged on The-OpenROAD-Project#10465: - ":tcl" in srcs would cause a multiple-definition error for gui_tcl_inits at link time (already provided by :gui, which :gui_stub deps), and src/stub.cpp doesn't reference the symbol. - "@spdlog" is unused — src/stub.cpp doesn't include any spdlog headers. Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

github-actions · 2026-05-20T09:59:42Z

clang-tidy review says "All clean, LGTM! 👍"

oharboe · 2026-05-20T10:01:26Z

/gemini review

gemini-code-assist

Code Review

This pull request refactors the Bazel build configuration to decouple the CLI and Qt GUI variants of OpenROAD. Key changes include the introduction of a dedicated openroad-qt binary target, the splitting of openroad_lib into CLI and Qt-specific versions, and the isolation of GUI stub implementations into a separate :gui_stub library to prevent symbol collisions. A build test has also been added to verify the compilability of the new Qt target. I have no feedback to provide as there were no review comments to assess.

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

oharboe · 2026-05-20T10:06:09Z

@maliberty Mac CI outage:

WARNING: Download from https://ftp.gnu.org/gnu/glpk/glpk-5.0.tar.gz failed: class java.io.IOException Connect timed out

github-actions · 2026-05-20T10:09:18Z

clang-tidy review says "All clean, LGTM! 👍"

After the :gui/:gui_stub split, libweb.so has undefined references to gui::Gui::get(), setHeadlessViewer, setChartFactory, BufferTree, etc. The :openroad and :_openroadpy.so binaries already pull in //src/gui:gui_stub explicitly to satisfy those; the cc_test targets in src/web/test/BUILD did not, so all eight of them (glyph_cache_test, snap_test, save_image_test, save_report_test, clock_tree_report_test, request_handler_test, tile_generator_test, debug_graphics_test) failed to link with --no-allow-shlib-undefined. Add //src/gui:gui_stub to each cc_test's deps. The Qt variant is not affected because no test target links //src/gui:gui_qt. Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

github-actions · 2026-05-20T12:18:27Z

clang-tidy review says "All clean, LGTM! 👍"

…rmp/rsz Same fix pattern as the prior //src/web/test commit: after the :gui/:gui_stub split, libgui.so has undefined references that the test binaries must resolve themselves. Add //src/gui:gui_stub to the 24 cc_test targets that transitively link libgui.so but did not already pull in the stub: src/cts/test:cts_unittest src/cut/test:CutGTests, :cut_abc_test src/dbSta/test:dbsta_unittest, :test_read_verilog, :test_write_verilog src/gpl/test:mbff_test src/mpl/test:mpl_block_macro_channels_unittest src/odb/test/cpp:TestSwapMaster, :TestSwapMasterUnusedPort, :TestWriteReadDbHier src/rmp/test:RmpGTests, :rmp_abc_unittest src/rsz/test:TestBufRem1, :TestBufferRemoval2, :TestBufferRemoval3, :TestInsertBuffer, :TestNestedJournal, :TestResizer, :TestResizerMt, :rsz_buffer_removal, :rsz_buffer_removal2, :rsz_buffer_removal3, :rsz_insert_buffer bazelisk test --keep_going //src/... now passes 1586/1586. Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

maliberty · 2026-05-20T14:33:38Z

I'll review this as it is cross-cutting.

GlobalRouter stored gui::HeatMapSourceHandle members directly, which pulled gui symbols into every consumer of :grt and forced downstream tests to link //src/gui:gui_stub. Repurpose the existing-but-unused AbstractRoutingCongestionDataSource into a single virtual void invalidate(). Store unique_ptr<AbstractRoutingCongestionDataSource> in GlobalRouter. Add a HeatMapHandleAdapter in :ui that wraps the gui handle and forwards invalidate() -> invalidateInstances(). Drop #include "gui/heatMap.h" from GlobalRouter.cpp and the gui forward-decl from GlobalRouter.h. Pattern matches AbstractFastRouteRenderer / AbstractGrouteRenderer: abstract base in :grt, concrete gui-aware adapter in :ui. CMake: move gui_heatmap_core from grt_lib to the swig grt target. Signed-off-by: Matt Liberty <mliberty@precisioninno.com>

:gpl was a monolith that compiled graphicsImpl.cpp (gui::Gui, gui::Painter, gui::Chart) and MakeReplace.cpp (Tcl + gui factory) directly, plus the swig/tcl glue. As a result :gpl pulled gui symbols into every consumer and forced tests to link gui_stub. Mirror the structure already used by :dpl, :grt, :mpl: split out a :ui target containing MakeReplace.cpp, graphicsImpl.cpp/h, swig and tcl rules, deps on //src/gui and //:ord. Drop gui/tcl/ord from :gpl deps. Add :gpl_private_hdrs (dpl pattern) for the five headers shared between :gpl and :ui: nesterovBase.h, nesterovPlace.h, placerBase.h, point.h, routeBase.h. Both targets dep it; no source duplication. Top-level BUILD.bazel adds //src/gpl:ui to OPENROAD_LIBRARY_DEPS and to :_openroadpy.so deps, matching how every other module's :ui is wired in. CMake already split gpl_lib (core, no gui) from gpl (swig, gui), so no CMake change needed. Signed-off-by: Matt Liberty <mliberty@precisioninno.com>

No source under src/par/ references any gui:: symbol or includes any gui/ header. The dep was inherited from older code and was the only reason //src/gui leaked into //src/mpl/test:mpl_block_macro_channels_unittest via :mpl -> :par. Signed-off-by: Matt Liberty <mliberty@precisioninno.com>

Revert the workaround added in 69e9a3b. With :grt and :gpl decoupled from gui (prior commits) and the stale par->gui dep removed, none of these tests transitively need gui symbols any more. Affected targets: src/cts/test:cts_unittest src/cut/test:CutGTests, :cut_abc_test src/dbSta/test:dbsta_unittest, :test_read_verilog, :test_write_verilog src/gpl/test:mbff_test src/mpl/test:mpl_block_macro_channels_unittest src/odb/test/cpp:TestSwapMaster, :TestSwapMasterUnusedPort, :TestWriteReadDbHier src/rmp/test:RmpGTests, :rmp_abc_unittest src/rsz/test:TestBufRem1, :TestBufferRemoval2, :TestBufferRemoval3, :TestInsertBuffer, :TestNestedJournal, :TestResizer, :TestResizerMt, :rsz_buffer_removal, :rsz_buffer_removal2, :rsz_buffer_removal3, :rsz_insert_buffer All 24 build and pass without :gui_stub. Signed-off-by: Matt Liberty <mliberty@precisioninno.com>

maliberty · 2026-05-20T17:23:40Z

Fixed the residual gui dependencies rather than sprinkle more gui dependencies around.

github-actions

clang-tidy made some suggestions

oharboe · 2026-05-20T17:36:08Z

@maliberty needs a tidy

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>

The clang-tidy CI action lints every changed file including headers, but HeatMapHandleAdapter.h had no compile_commands.json entry (only TUs do), so clang-tidy ran with no -I flags and failed to resolve gui/heatMap.h. The adapter has a single user; move the class into MakeGlobalRouter.cpp and delete the header. Signed-off-by: Matt Liberty <mliberty@precisioninno.com>

github-actions · 2026-05-20T18:02:41Z

clang-tidy review says "All clean, LGTM! 👍"

Add gui_descriptors to TEST_LIBS so every test in src/web/test/cpp picks up the gui/include directory (web links gui_descriptors PRIVATE, so the include path didn't propagate to consumers). Also register the three tests that existed in Bazel but not in CMake: TestClockTreeReport, TestSaveImage, TestSnap. Signed-off-by: Matt Liberty <mliberty@precisioninno.com>

github-actions · 2026-05-20T18:47:01Z

clang-tidy review says "All clean, LGTM! 👍"

oharboe · 2026-05-20T20:23:40Z

😌

oharboe requested review from a team as code owners May 20, 2026 05:48

oharboe requested review from gadfort and osamahammad21 May 20, 2026 05:48

github-actions Bot added the size/M label May 20, 2026

gemini-code-assist Bot reviewed May 20, 2026

View reviewed changes

Comment thread src/gui/BUILD Outdated

Comment thread src/gui/BUILD Outdated

Comment thread src/gui/BUILD Outdated

gemini-code-assist Bot reviewed May 20, 2026

View reviewed changes

ci: retrigger Mac-Build (ftp.gnu.org glpk-5.0 download flake)

e09ecdd

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

oharboe requested a review from a team as a code owner May 20, 2026 12:15

oharboe requested review from a team as code owners May 20, 2026 13:10

maliberty requested review from maliberty and removed request for LucasYuki, gadfort, jhkim-pii, joaomai, osamahammad21 and precisionmoon May 20, 2026 14:32

maliberty added 4 commits May 20, 2026 17:21

maliberty requested review from a team as code owners May 20, 2026 17:22

maliberty requested a review from eder-matheus May 20, 2026 17:22

maliberty approved these changes May 20, 2026

View reviewed changes

maliberty enabled auto-merge May 20, 2026 17:27

github-actions Bot reviewed May 20, 2026

View reviewed changes

Comment thread src/grt/src/HeatMapHandleAdapter.h Outdated

Comment thread src/grt/src/HeatMapHandleAdapter.h Outdated

maliberty added 2 commits May 20, 2026 17:43

grt: clang-tidy

d91141c

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>

maliberty disabled auto-merge May 20, 2026 17:55

maliberty removed the request for review from eder-matheus May 20, 2026 18:31

maliberty merged commit d088e22 into The-OpenROAD-Project:master May 20, 2026
15 of 16 checks passed

oharboe mentioned this pull request May 21, 2026

[feature request] Build flow should not depend on the Qt / web GUI sources #10453

Closed

Conversation

oharboe commented May 20, 2026

Summary

Why the gui_stub split (vs. #10384)

Why :openroad-qt (recap from #10384)

Migration

Test plan

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented May 20, 2026

Uh oh!

oharboe commented May 20, 2026

Uh oh!

github-actions Bot commented May 20, 2026

Uh oh!

hzeller commented May 20, 2026

Uh oh!

oharboe commented May 20, 2026

Uh oh!

oharboe commented May 20, 2026

Uh oh!

github-actions Bot commented May 20, 2026

Uh oh!

oharboe commented May 20, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

oharboe commented May 20, 2026

Uh oh!

github-actions Bot commented May 20, 2026

Uh oh!

github-actions Bot commented May 20, 2026

Uh oh!

maliberty commented May 20, 2026

Uh oh!

maliberty commented May 20, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

oharboe commented May 20, 2026

Uh oh!

github-actions Bot commented May 20, 2026

Uh oh!

github-actions Bot commented May 20, 2026

Uh oh!

Uh oh!

oharboe commented May 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Why `:openroad-qt` (recap from #10384)