Skip to content

etc: move yosys readline dep from -common to -base#10569

Open
mguthaus wants to merge 2 commits into
The-OpenROAD-Project:masterfrom
mguthaus:readline-base-move
Open

etc: move yosys readline dep from -common to -base#10569
mguthaus wants to merge 2 commits into
The-OpenROAD-Project:masterfrom
mguthaus:readline-base-move

Conversation

@mguthaus
Copy link
Copy Markdown
Contributor

Move the libreadline-dev / readline-devel / readline install out of the per-build _install_yosys_dependencies() helper (invoked during the -common phase) and into the per-platform -base package lists where the other system -dev libs already live.

Fixes ORFS #4266.

Why

-common is documented (and setup.sh arranges) as the unprivileged phase: it installs into ${PREFIX:-/usr/local} and setup.sh calls it via sudo -u $SUDO_USER. The previous _install_yosys_dependencies() helper, however, ran apt-get -y install libreadline-dev (and the yum/zypper/brew equivalents). On Linux those require root, so the standard "sudo ./setup.sh" install path (and any other non-root -common -prefix=… flow) failed with:

[INFO] Installing libreadline-dev for yosys...
E: Could not open lock file /var/lib/dpkg/lock-frontend - open (13: Permission denied)
[ERROR] Failed to execute: apt-get -y install --no-install-recommends libreadline-dev

The original commit that introduced _install_yosys_dependencies() noted that OpenROAD itself uses vendored linenoise and dropped readline from -base. That's still true for OpenROAD, but yosys needs libreadline-dev at build time and that has to come from a privileged install regardless. So the right home is -base, not -common.

What this changes

  • Add libreadline-dev to _install_ubuntu_packages.
  • Add readline-devel to _install_rhel_packages.
  • Add readline-devel to _install_opensuse_packages.
  • Add readline to _install_darwin_packages (brew).
  • Delete _install_yosys_dependencies() and its call site in _install_yosys.
  • Short comment above _install_yosys pointing to where the dep now lives.

Tiny growth of -base (libreadline-dev is small) in exchange for restoring the non-root -common workflow.

Testing

  • sudo ./setup.sh on Ubuntu 24.04: completes -base and -common cleanly.
  • ./build_openroad.sh --local afterwards: yosys builds and links against libreadline as before.

The previous _install_yosys_dependencies() helper was invoked from
_install_yosys() during the -common phase. -common is meant to be
runnable without root (it installs into ${PREFIX:-/usr/local} and
setup.sh deliberately runs it as $SUDO_USER), but the helper called
apt-get/yum/zypper/brew, all of which need root on Linux. As a
result, the standard "setup.sh as sudo" install path (and any
unprivileged -common -prefix=... use case) failed with:

    [INFO] Installing libreadline-dev for yosys...
    E: Could not open lock file /var/lib/dpkg/lock-frontend ...
    [ERROR] Failed to execute: apt-get -y install --no-install-recommends libreadline-dev

Move the dep into the per-platform -base package lists
(_install_ubuntu_packages, _install_rhel_packages,
_install_opensuse_packages, _install_darwin_packages) where the
other system -dev libs already live and where root is already
required, and drop _install_yosys_dependencies entirely. The original
"readline isn't in -base anymore because OpenROAD uses vendored
linenoise" rationale is fine for OpenROAD itself, but yosys still
needs it at build time and it has to be installed by the privileged
phase regardless. Slightly enlarges -base (libreadline-dev is small)
in exchange for making non-root -common work again.

Fixes The-OpenROAD-Project/OpenROAD-flow-scripts#4266.

Signed-off-by: Matthew Guthaus <mrg@ucsc.edu>
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the installation of Yosys compile-time readline dependencies by moving them from a separate helper function into the per-platform base package installation functions (Ubuntu, RHEL, openSUSE, and Darwin). This prevents running root-only commands during the unprivileged common phase. Feedback indicates that the libreadline-dev dependency was missed in the Debian package installer, which should be added to avoid build failures on Debian systems.

Comment thread etc/DependencyInstaller.sh
Missed the Debian path in the previous commit. Same rationale: yosys
needs libreadline at build time and the install has to happen in the
privileged -base phase, not -common. Caught in PR review.

Signed-off-by: Matthew Guthaus <mrg@ucsc.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

-common -prefix=… (non-root local install) fails: yosys readline dep does a root-only apt-get

2 participants