Skip to content

Conversation

@openroad-ci
Copy link
Collaborator

Fix Dependency Installer and Improve Build System

This PR introduces a series of improvements to the build system, focusing on making dependency handling more robust, predictable, and user-friendly. The changes address build failures in custom environments and enhance the clarity of the installation summary.

Bug Fix

This PR resolves a critical build failure that occurred when dependencies were installed in a custom directory using the -prefix option (e.g., ./dependencies). The root cause was that the CMake configuration did not fully respect custom dependency paths, leading to linker errors. By enforcing the correct CMake policies, this change ensures that the project builds reliably, regardless of where the dependencies are located.

Key Changes

1. Robust Dependency Handling with Custom Prefixes

File changed: etc/DependencyInstaller.sh

The dependency installation script has been refactored to correctly prioritize custom installation prefixes provided with the -prefix flag.

  • Prefix-Aware Dependency Checks: The script now checks for dependencies within the specified prefix before searching system-wide paths. This ensures that a custom-built toolchain is used consistently.
  • Installation into Prefix: If a dependency is not found in the prefix, it will be installed there, even if a version exists on the system. This guarantees that the build environment is self-contained and avoids mixing libraries from different sources.
  • Relative Path Support: The --prefix argument now accepts relative paths, which are automatically converted to absolute paths. This adds flexibility when running the script from different directories.

2. Enhanced Installation Summary

image

File changed: etc/DependencyInstaller.sh

The installation summary table has been significantly improved to provide a clearer and more informative overview of the dependency landscape:

  • New "Path" Column: The summary now includes the installation path for each dependency, making it easy to see where each tool is located.
  • Improved Column Headers: "System Version" has been renamed to "Found" for better clarity.
  • Compact and Readable Format: The table's column widths have been reduced, and long paths are now automatically truncated to prevent formatting issues.

3. Consistent CMake Dependency Resolution

File changed: CMakeLists.txt

The root CMakeLists.txt has been updated to ensure consistent behavior across all environments, from local development to CI.

  • CMP0144 Policy: The CMP0144 policy has been added to ensure that find_package correctly uses <PACKAGENAME>_ROOT variables. This was the root cause of build failures when using a custom prefix, as CMake was ignoring some of the provided paths.
  • Clarifying Comments: Detailed comments have been added to explain the roles of both CMP0074 and CMP0144, documenting why both are necessary for backward and forward compatibility in CMake's dependency resolution.

@github-actions
Copy link
Contributor

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

@sombraSoft
Copy link
Contributor

@mguthaus FYI

@mguthaus
Copy link
Contributor

I also see some issues with removing files now:

/tmp/DependencyInstaller-orfs-4G9buw ~/orfs-upstream
rm: remove write-protected regular file 'verilator/.git/objects/pack/pack-1bba8259082364fd9eca4c7d20c41d90086becd2.idx'? y
rm: remove write-protected regular file 'verilator/.git/objects/pack/pack-1bba8259082364fd9eca4c7d20c41d90086becd2.rev'? y
rm: remove write-protected regular file 'verilator/.git/objects/pack/pack-1bba8259082364fd9eca4c7d20c41d90086becd2.pack'? y

It likely gets run as root in the CI but running as a user has this issue.

@maliberty
Copy link
Member

@sombraSoft holding on the above concern

@maliberty maliberty added the waiting on op Information has been requested from the Issue Author label Oct 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting on op Information has been requested from the Issue Author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants