make setup in json - #1591
make setup in json#1591TalBarYakar wants to merge 5 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1591 +/- ##
=======================================
Coverage 75.18% 75.18%
=======================================
Files 15 15
Lines 4255 4255
=======================================
Hits 3199 3199
Misses 1056 1056 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9772d18. Configure here.
| RUN chmod +x /workspace/.install/install_cmake.sh && /workspace/.install/install_cmake.sh | ||
| RUN bash /workspace/.install/install_script.sh | ||
| # rustc/cargo from rustup (install_script.sh ran getrust.sh) | ||
| ENV PATH="/root/.cargo/bin:${PATH}" |
There was a problem hiding this comment.
Bullseye Dockerfile drops install_cmake.sh upgrade step
High Severity
The old Dockerfile.bullseye ran install_cmake.sh after installing packages (which upgrades cmake to 3.25.1), but the new Dockerfile omits this step. Debian 11 ships cmake 3.18, likely too old for the project. Both Dockerfile.bookworm and Dockerfile.trixie still include the install_cmake.sh step, so this looks like an accidental omission. The legacy debian_gnu_linux_11.sh also called source install_cmake.sh, confirming the upgrade is needed.
Reviewed by Cursor Bugbot for commit 9772d18. Configure here.
| # python3 via brew's existing /opt/homebrew/bin entry. | ||
| if [ -n "${GITHUB_PATH:-}" ]; then | ||
| echo "$newpath" >> "$GITHUB_PATH" | ||
| fi |
There was a problem hiding this comment.
GITHUB_PATH receives export statement instead of directories
Medium Severity
The update_profile function writes $newpath (which is export PATH=...:$PATH) to $GITHUB_PATH. GitHub Actions expects $GITHUB_PATH to contain one raw directory path per line — not shell export statements. This means macOS CI runners won't actually get the intended directories prepended to their PATH in subsequent steps.
Reviewed by Cursor Bugbot for commit 9772d18. Configure here.
| # leaving that on PATH breaks anything needing cmake>=3 (e.g. cpu_features). | ||
| # This symlink is unconditional on purpose to override the 2.8 binary — | ||
| # the legacy Dockerfile.amazonlinux2 did exactly the same thing. | ||
| $MODE ln -sf "$(command -v cmake3)" /usr/bin/cmake |
There was a problem hiding this comment.
Amazon Linux 2 quirk missing openssl11 packages
High Severity
The legacy amazon_linux_2.sh installed openssl11 and openssl11-devel (OpenSSL 1.1.1) and symlinked openssl11 as /usr/bin/openssl. The new quirk omits these entirely. The abstract openssl_dev maps to the ancient openssl/openssl-devel (1.0.2) for yum, which is insufficient for building Rust crates and other components that require OpenSSL ≥ 1.1. The comment on line 353 of install_script.sh even notes "AmazonLinux2 needs openssl11" but the quirk doesn't deliver it.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 9772d18. Configure here.
|
|
||
| # Use system Python with venv (like the old flow-alpine workflow) | ||
| # uv's managed Python has clang-specific flags that cause build issues | ||
| RUN python3 -m venv /opt/.venv |
There was a problem hiding this comment.
Alpine venv missing --system-site-packages flag
Medium Severity
The old Alpine Dockerfile created the venv with uv venv --system-site-packages, making system-installed py3-cryptography, py3-numpy, and py3-psutil visible inside it. The new python3 -m venv /opt/.venv omits --system-site-packages, so those packages — installed by quirks/alpine.sh specifically to "avoid building the C extensions against musl from source" — become invisible in the venv. Any later pip install needing them will attempt a slow, fragile source build against musl instead.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 9772d18. Configure here.
1 participant
Note
Medium Risk
Medium risk because it rewires how build/test dependencies and Rust toolchains are installed across many Docker images and local
make bootstrap, which can cause CI/build breakages on specific distros or minimal base images.Overview
Unifies dependency installation by replacing the old
.install/install_script.shwith an OSNICK-aware installer that readsdependencies.yaml, resolves abstract deps to package-manager packages, installs only missing packages, and falls back to legacy.install/<distro>_<ver>.shscripts for non-migrated OSes.Adds OS-specific
quirkshooks (e.g. toolset gcc on Alma/Rocky, devtoolset+cmake3 on AmazonLinux2, musl extras on Alpine, PATH/profile handling on macOS) and enables CRB on EL9+ where needed for-develpackages.Updates build entrypoints: Dockerfiles switch from inline per-distro installs to
install_script.sh(plus apt retry config on Debian/Ubuntu), Rust is installed viagetrust.shfor non-Alpine, Alpine’s Python env switches topython3 -m venv+pip, and the repoMakefilereplacessetupwith a newbootstraptarget that runs the installer then creates a./venvand runs.install/common_installations.sh.Reviewed by Cursor Bugbot for commit b9ffac6. Bugbot is set up for automated code reviews on this repo. Configure here.