fix: bazel manpage compilation#10729
Conversation
Signed-off-by: Jack Luar <39641663+luarss@users.noreply.github.com>
There was a problem hiding this comment.
Code Review
This pull request adds support for installing and locating generated man pages in Bazel builds. Specifically, it updates bazel/install.sh and packaging/BUILD.bazel to copy man pages to the installation directory, and modifies src/OpenRoad.cc to look for them in bazel-bin/docs when running via Bazel. The review feedback suggests dereferencing symlinks using cp -rfL and cleaning up stale directories during installation, as well as using the non-throwing overload of std::filesystem::is_directory to prevent potential exceptions.
| if [ -n "$MAN_CAT_SRC" ] && [ -d "$MAN_CAT_SRC" ]; then | ||
| MAN_DIR="$ABS_DEST/share/openroad/man" | ||
| mkdir -p "$MAN_DIR" | ||
| cp -rf "$MAN_CAT_SRC" "$MAN_DIR/" | ||
| MAN_HTML_SRC="$(rlocation openroad/docs/html 2>/dev/null || true)" | ||
| if [ -n "$MAN_HTML_SRC" ] && [ -d "$MAN_HTML_SRC" ]; then | ||
| cp -rf "$MAN_HTML_SRC" "$MAN_DIR/" | ||
| fi | ||
| echo "OpenROAD man pages installed to $MAN_DIR" | ||
| fi |
There was a problem hiding this comment.
When copying directories from Bazel's runfiles, the source files are typically symlinks. Using cp -rf without dereferencing (-L) will copy these symlinks as-is, resulting in broken symlinks in the final installation directory. Additionally, if a previous installation exists, stale man pages are not cleaned up.
To resolve these issues, clean up the target directories before copying and use cp -rfL to dereference the symlinks.
| if [ -n "$MAN_CAT_SRC" ] && [ -d "$MAN_CAT_SRC" ]; then | |
| MAN_DIR="$ABS_DEST/share/openroad/man" | |
| mkdir -p "$MAN_DIR" | |
| cp -rf "$MAN_CAT_SRC" "$MAN_DIR/" | |
| MAN_HTML_SRC="$(rlocation openroad/docs/html 2>/dev/null || true)" | |
| if [ -n "$MAN_HTML_SRC" ] && [ -d "$MAN_HTML_SRC" ]; then | |
| cp -rf "$MAN_HTML_SRC" "$MAN_DIR/" | |
| fi | |
| echo "OpenROAD man pages installed to $MAN_DIR" | |
| fi | |
| if [ -n "$MAN_CAT_SRC" ] && [ -d "$MAN_CAT_SRC" ]; then | |
| MAN_DIR="$ABS_DEST/share/openroad/man" | |
| mkdir -p "$MAN_DIR" | |
| rm -rf "$MAN_DIR/cat" | |
| cp -rfL "$MAN_CAT_SRC" "$MAN_DIR/" | |
| MAN_HTML_SRC="$(rlocation openroad/docs/html 2>/dev/null || true)" | |
| if [ -n "$MAN_HTML_SRC" ] && [ -d "$MAN_HTML_SRC" ]; then | |
| rm -rf "$MAN_DIR/html" | |
| cp -rfL "$MAN_HTML_SRC" "$MAN_DIR/" | |
| fi | |
| echo "OpenROAD man pages installed to $MAN_DIR" | |
| fi |
| if (std::filesystem::is_directory(docs_path)) { | ||
| return docs_path; | ||
| } |
There was a problem hiding this comment.
std::filesystem::is_directory can throw a std::filesystem::filesystem_error exception if the path is inaccessible, contains a loop of symlinks, or encounters other OS-level errors. To ensure robust error handling and prevent potential crashes, use the non-throwing overload that accepts an std::error_code parameter.
std::error_code ec;
if (std::filesystem::is_directory(docs_path, ec)) {
return docs_path;
}
Summary
src/OpenRoad.cc: CheckBUILD_WORKSPACE_DIRECTORYingetDocsPath()forbazel runcase(ifdef BAZEL_BUILD).
packaging/BUILD.bazel: Add//docs:man_pagestoinstalldata deps.bazel/install.sh: Copycat/andhtml/to$DEST_DIR/share/openroad/man/.Type of Change
Impact
[How does this change the tool's behavior?]
Verification
./etc/Build.sh).Related Issues
fixes #10640