build: Improve macOS build and dependency installation#10113
Conversation
![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 enhances macOS support by adding documentation for Python virtual environments and improving dependency management in the build scripts. Key changes include switching to tcl-tk@8, adding icu4c support, and implementing more robust checks for Homebrew-installed tools. Review feedback identifies a variable naming inconsistency in DependencyInstaller.sh that could hinder parallel builds and suggests using brew --prefix in Build.sh to ensure portability across Intel and Apple Silicon architectures.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Welcome to OpenROAD! Thanks for opening your first PR.
Before we review:
- Contribution Guide: https://openroad.readthedocs.io/en/latest/contrib/contributing.html
- Build Instructions: https://openroad.readthedocs.io/en/latest/contrib/BuildWithCMake.html
Please ensure:
- CI passes
- Code is properly formatted
- Tests are included where applicable
A maintainer will review shortly!
ref The-OpenROAD-Project#9895 - Add macOS venv setup documentation - Improve Build.sh with better package validation and env config - Update DependencyInstaller.sh for macOS (threads, icu4c, tcl-tk@8) - Use pip instead of pipx Signed-off-by: Sahil Jaiswal <jaiswalsahil7741@gmail.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Sahil Jaiswal <112881021+Sahil7741@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Sahil Jaiswal <112881021+Sahil7741@users.noreply.github.com>
Sahil7741
force-pushed
the
fix/macos-local-build
branch
from
69dccb9 to
73deb5f
Compare
|
@maliberty @luarss please review |
|
clang-tidy review says "All clean, LGTM! 👍" |
maliberty
left a comment
maliberty
left a comment
There was a problem hiding this comment.
This looks fine but you should be aware we are moving towards bazel where this is much simpler.
2 participants
ref #9895