install: check GUI deps before building#9772
Conversation
Check for required xcb system packages before starting the expensive Bazel build. Fails fast with a helpful error message showing the exact apt install command on Ubuntu-based systems. This follows the principle of least astonishment: rather than failing deep in the build with cryptic errors, check upfront and tell the user exactly what to install. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
|
@gadfort FYI |
|
clang-tidy review says "All clean, LGTM! 👍" |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request aims to add a pre-build check for GUI dependencies to fail fast. While the dependency checking logic itself is sound, its placement within bazel/install.sh means it will run after the expensive Bazel build, not before. The bazelisk run //:install command first builds all dependencies of the //:install target (which includes the main OpenROAD application) and only then executes the script. This defeats the primary goal of failing fast. The check should be moved to a mechanism that runs before the build is initiated.
| # Check for required system dependencies before expensive build. | ||
| # Each tool checks its own deps (separation of concerns). | ||
| # | ||
| # Currently only Ubuntu/Debian is checked. Dependency checking for | ||
| # other platforms (macOS, RHEL, Fedora, etc.) is not implemented | ||
| # because we cannot test them. Contributions welcome. | ||
| check_ubuntu_deps() { | ||
| local missing=() | ||
| # GUI deps (xcb libraries needed for Qt) | ||
| for pkg in libxcb-icccm4-dev libxcb-image0-dev libxcb-keysyms1-dev \ | ||
| libxcb-render-util0-dev libxcb-xinerama0-dev libxcb-xkb-dev; do | ||
| if ! dpkg -s "$pkg" &>/dev/null 2>&1; then | ||
| missing+=("$pkg") | ||
| fi | ||
| done | ||
| if [[ ${#missing[@]} -gt 0 ]]; then | ||
| echo "ERROR: Missing dependencies for OpenROAD GUI build." | ||
| echo "" | ||
| echo "On Ubuntu this would be:" | ||
| echo " sudo apt install ${missing[*]}" | ||
| exit 1 | ||
| fi | ||
| } | ||
|
|
||
| if command -v dpkg &>/dev/null; then | ||
| check_ubuntu_deps | ||
| fi |
There was a problem hiding this comment.
This dependency check is intended to run before the build, but its placement in this script will cause it to run after the build. When bazelisk run //:install is executed, Bazel first builds all the dependencies of the //:install target. Since this script consumes the build artifact openroad.tar, the expensive build will complete before this check is ever run, defeating the purpose of failing fast.
To fix this, the dependency check logic should be moved to a script or mechanism that is executed before the bazelisk build or bazelisk run command that triggers the main application build.
|
Closing this — the dep check is unnecessary. The OpenROAD Bazel build produces a statically linked binary with no runtime dependency on xcb or Qt system packages: GUI works fine without any of the checked packages installed. Bazel handles Qt/xcb internally. |
|
@maliberty Do we have any dependencies that we need to install on a blank ubuntu anymore? I have installed the dependencies on my machines long ago, so I can't check. |
|
If you build with the gui then you do need a bunch see #8532 (still in progress) |
2 participants
Check for required xcb system packages before starting the expensive Bazel build. Fails fast with a helpful error message showing the exact apt install command on Ubuntu-based systems.
This follows the principle of least astonishment: rather than failing deep in the build with cryptic errors, check upfront and tell the user exactly what to install.