Skip to content

Improve macOS handling in setup and dependency installer scripts#4023

Merged
maliberty merged 2 commits into
The-OpenROAD-Project:masterfrom
Sahil7741:fix/macos-setup-installer
Mar 22, 2026
Merged

Improve macOS handling in setup and dependency installer scripts#4023
maliberty merged 2 commits into
The-OpenROAD-Project:masterfrom
Sahil7741:fix/macos-setup-installer

Conversation

@Sahil7741
Copy link
Copy Markdown
Contributor

This PR improves macOS behavior in the ORFS setup path:

  • Allow sudo on Linux and disallow on macOS
  • Fix submodule update to run with correct user per OS
  • Use sysctl for CPU count on macOS instead of nproc
  • Minor portability and error message improvements

Addresses #4022

Signed-off-by: Sahil Jaiswal <jaiswalsahil7741@gmail.com>
Comment thread setup.sh Outdated
IS_DARWIN=true
fi

if $IS_DARWIN && [[ $EUID -eq 0 ]]; then
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not sudo on macos?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually setup.sh invokes DependencyInstaller.sh and dependencies are installed via Homebrew which do not need sudo. Additionally DependencyInstaller.sh is build in such a way that on mac its recommend to not use sudo. If we run it using sudo then script fails and exits giving below error.
Look Line 410 of DependencyInstaller.sh

echo "ERROR: cannot install on macOS if you are root or using sudo (not recommended for brew)." >&2

Comment thread setup.sh Outdated
echo "This script must be run with sudo"
# macOS detection
IS_DARWIN=false
if [[ "$(uname)" == "Darwin" ]]; then
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we can follow existing coding style, it should be okay to repeat if [[ "$OSTYPE" == "darwin"* ]]; then multiple times

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I did the changes please check

Signed-off-by: Sahil Jaiswal <jaiswalsahil7741@gmail.com>
@maliberty maliberty enabled auto-merge March 22, 2026 15:23
@maliberty maliberty merged commit 51ad123 into The-OpenROAD-Project:master Mar 22, 2026
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants