Skip to content

build: Add Python virtual environment guard to Build.sh#9820

Open
Divinesoumyadip wants to merge 2 commits intoThe-OpenROAD-Project:masterfrom
Divinesoumyadip:python-venv-guard
Open

build: Add Python virtual environment guard to Build.sh#9820
Divinesoumyadip wants to merge 2 commits intoThe-OpenROAD-Project:masterfrom
Divinesoumyadip:python-venv-guard

Conversation

@Divinesoumyadip
Copy link
Contributor

Adds a safety check to Build.sh to detect if the user is operating inside a Python Virtual Environment .

OpenROAD relies heavily on Python bindings and external PIP packages. On modern Linux distributions (like Ubuntu 24.04 with PEP-668), attempting to install dependencies globally can break OS-level package managers. Beginners frequently fall into this trap, resulting in corrupted local environments and support tickets.
The script now checks for a registered virtual environment. If none is found, it issues a highly visible warning with exact instructions on how to create one (python3 -m venv or_env), pauses for 3 seconds to ensure the user reads it, and then proceeds.

Directly supports the Onboarding Simplification initiative by preemptively stopping developers from destroying their local Python environments during the initial build phase.

Signed-off-by: Divinesoumyadip <soumyacode7@gmail.com>
@github-actions
Copy link
Contributor

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

Copy link
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 introduces a helpful safety check to Build.sh that warns users when they are not in a Python virtual environment. This is a great improvement for preventing environment-related issues, especially for new contributors. The implementation is straightforward. I've added a couple of suggestions to improve the script's robustness: one to ensure tput is available before use, and another to restore the POSIX-compliant trailing newline at the end of the file.

etc/Build.sh Outdated
# ==============================================================================
# PYTHON VIRTUAL ENVIRONMENT CHECK
# ==============================================================================
if [[ -t 1 ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The script assumes tput is always available. If tput is not installed, this will produce command not found errors to stderr. It's safer to check for the existence of tput before attempting to use it.

Suggested change
if [[ -t 1 ]]; then
if [[ -t 1 ]] && command -v tput >/dev/null; then

fi
eval cmake "${cmakeOptions}" -B "${buildDir}" .
eval time cmake --build "${buildDir}" -j "${numThreads}"
eval time cmake --build "${buildDir}" -j "${numThreads}" No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The trailing newline has been removed from this file. According to POSIX standards, text files should end with a newline character to ensure they are processed correctly by various tools. Please restore the trailing newline.

@maliberty
Copy link
Member

What external packages are being updated during Build.sh?

Adds a check for tput availability to prevent stderr noise and ensures the file ends with a POSIX-compliant newline.

Signed-off-by: Divinesoumyadip <soumyacode7@gmail.com>
@github-actions
Copy link
Contributor

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

@Divinesoumyadip
Copy link
Contributor Author

What external packages are being updated during Build.sh?

Hi @maliberty, that is a fair point. Build.sh itself doesn't directly update external PIP packages.However, I've noticed that new contributors on modern distributions (like Ubuntu 24.04+) often hit the externally-managed-environment (PEP-668) error immediately after a successful build when they try to install flow dependencies like pyverilog or utl.
Since the build process links against the active Python interpreter, adding this guard to Build.sh serves as a "pre-flight check." It ensures beginners have a safe, non-system workspace established before they invest time in the compilation, ultimately reducing environment-related support tickets later in the onboarding process.

@maliberty
Copy link
Member

Could you give an example of such a problem report? Is utl also a pip module or are you referring to the src/utl module? Where is pyverilog used?

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.

2 participants