build: Split the Ubuntu dev-dependency install script into common, huntsman, and wolf variants. - #336
Conversation
|
Too much diff to scan? Review this PR in Change Stack to start with the highest-impact changes. WalkthroughDevelopment dependency installation is refactored from a single generic Linux script to Ubuntu-specific modular scripts. A new common installer handles shared setup (apt update, base packages, uv), while Wolf and Huntsman installers handle component-specific packages. All references in the devcontainer and CI workflows are updated to call the appropriate scripts. ChangesUbuntu-specific installation scripts
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
tools/scripts/lib_install/ubuntu/install-dev-common.sh (2)
18-18: 💤 Low valueConsider adding
DEBIAN_FRONTEND=noninteractivetoapt-get updatefor consistency.While
apt-get updatetypically doesn't prompt interactively, settingDEBIAN_FRONTEND=noninteractivehere would match the pattern used on line 19 and ensure consistent non-interactive behaviour across all apt operations.♻️ Proposed change
-${privileged_command_prefix} apt-get update +DEBIAN_FRONTEND=noninteractive ${privileged_command_prefix} apt-get update🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/scripts/lib_install/ubuntu/install-dev-common.sh` at line 18, The apt-get update invocation using ${privileged_command_prefix} should be run non-interactively like the subsequent apt commands; update the command that invokes "apt-get update" (the line using the variable ${privileged_command_prefix}) to prefix it with DEBIAN_FRONTEND=noninteractive so it matches the non-interactive pattern used on the following line and ensures consistent behavior across all apt operations.
29-29: ⚡ Quick winConsider verifying the uv installer script integrity.
Piping a remote script directly to
shis convenient but introduces a supply-chain risk. While the URL uses HTTPS and points to the official Astral uv installer, consider adding checksum verification or pinning to a specific version if reproducibility and additional security assurance are priorities.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/scripts/lib_install/ubuntu/install-dev-common.sh` at line 29, The installer is being piped directly into sh which is a supply-chain risk; change the flow in install-dev-common.sh so you first download the script (referenced by the existing curl invocation), verify integrity (either fetch a published checksum/signature from the vendor and validate it or pin to a specific versioned installer URL), and only then execute the verified script; alternatively replace the piped URL with a stable, versioned release URL from Astral (or verify a GPG signature) before invoking sh..github/workflows/code-linting-checks.yaml (1)
47-50: Informational: uv is installed twice in jobs usinginstall-dev-common.sh.The workflow installs uv via the GitHub action (line 47), then
install-dev-common.shinstalls it again (line 50). The uv installer is idempotent, so this works correctly but performs redundant work. The same pattern applies tolint-python(lines 151-154). This redundancy likely existed before this refactor and is not a new issue.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/code-linting-checks.yaml around lines 47 - 50, Remove the duplicate uv installation by choosing one source of truth: either keep the GitHub action step "./tools/yscope-dev-utils/exports/github/actions/install-uv" or keep the script step named "Install dev dependencies" that runs "./tools/scripts/lib_install/ubuntu/install-dev-common.sh"; update the workflow to remove the other step, or alternatively modify install-dev-common.sh to detect an existing uv install and skip reinstalling (add an idempotent check before install). Also apply the same de-duplication for the lint-python installation (the corresponding install action and the install-dev-common.sh invocation)..devcontainer/Dockerfile (1)
8-9: Note:install-dev-common.shruns twice (once per component script).Since both wolf and huntsman internally call
install-dev-common.sh, the common setup (includingapt-get updateand base package checks) executes twice. This is slightly inefficient but maintains script independence, allowing each component script to run standalone. The duplicate installations are harmless (apt and uv are idempotent), so this trade-off is acceptable for maintainability.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.devcontainer/Dockerfile around lines 8 - 9, Both component installers (install-dev-wolf.sh and install-dev-huntsman.sh) call install-dev-common.sh, causing the common setup to run twice; change the Dockerfile so install-dev-common.sh runs only once before invoking the two component scripts, or modify the component scripts to respect an environment flag (e.g., SKIP_COMMON) and set that flag when calling the second script. Locate install-dev-wolf.sh, install-dev-huntsman.sh and install-dev-common.sh references in the diff and either (A) add a separate RUN invocation for install-dev-common.sh once and then run both component scripts, or (B) update the component scripts to check SKIP_COMMON and set SKIP_COMMON=1 in the Dockerfile for subsequent calls to avoid re-running the common installer.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.devcontainer/Dockerfile:
- Around line 8-9: Both component installers (install-dev-wolf.sh and
install-dev-huntsman.sh) call install-dev-common.sh, causing the common setup to
run twice; change the Dockerfile so install-dev-common.sh runs only once before
invoking the two component scripts, or modify the component scripts to respect
an environment flag (e.g., SKIP_COMMON) and set that flag when calling the
second script. Locate install-dev-wolf.sh, install-dev-huntsman.sh and
install-dev-common.sh references in the diff and either (A) add a separate RUN
invocation for install-dev-common.sh once and then run both component scripts,
or (B) update the component scripts to check SKIP_COMMON and set SKIP_COMMON=1
in the Dockerfile for subsequent calls to avoid re-running the common installer.
In @.github/workflows/code-linting-checks.yaml:
- Around line 47-50: Remove the duplicate uv installation by choosing one source
of truth: either keep the GitHub action step
"./tools/yscope-dev-utils/exports/github/actions/install-uv" or keep the script
step named "Install dev dependencies" that runs
"./tools/scripts/lib_install/ubuntu/install-dev-common.sh"; update the workflow
to remove the other step, or alternatively modify install-dev-common.sh to
detect an existing uv install and skip reinstalling (add an idempotent check
before install). Also apply the same de-duplication for the lint-python
installation (the corresponding install action and the install-dev-common.sh
invocation).
In `@tools/scripts/lib_install/ubuntu/install-dev-common.sh`:
- Line 18: The apt-get update invocation using ${privileged_command_prefix}
should be run non-interactively like the subsequent apt commands; update the
command that invokes "apt-get update" (the line using the variable
${privileged_command_prefix}) to prefix it with DEBIAN_FRONTEND=noninteractive
so it matches the non-interactive pattern used on the following line and ensures
consistent behavior across all apt operations.
- Line 29: The installer is being piped directly into sh which is a supply-chain
risk; change the flow in install-dev-common.sh so you first download the script
(referenced by the existing curl invocation), verify integrity (either fetch a
published checksum/signature from the vendor and validate it or pin to a
specific versioned installer URL), and only then execute the verified script;
alternatively replace the piped URL with a stable, versioned release URL from
Astral (or verify a GPG signature) before invoking sh.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e5ac8d1f-e8c4-4c38-9461-64de296655bd
📒 Files selected for processing (6)
.devcontainer/Dockerfile.github/workflows/code-linting-checks.yaml.github/workflows/tests.yamltools/scripts/lib_install/ubuntu/install-dev-common.shtools/scripts/lib_install/ubuntu/install-dev-huntsman.shtools/scripts/lib_install/ubuntu/install-dev-wolf.sh
2 participants
Description
tools/scripts/lib_install/linux/install-dev.shinstalled everything required to build Spider Wolf (C++), but Spider Huntsman (Rust) only needs a small subset of those dependencies. This PR splits the script so each consumer installs exactly what it needs, and renames the containing directory to reflect what the scripts actually target.Changes
install-dev.shinto three scripts:install-dev-common.sh— dependencies shared by all versions:ca-certificates,curl,git,python3,python3-pip,python3-venv, and theuvinstaller. Owns theapt-get updateand privilege-elevation preamble.install-dev-huntsman.sh— runs the common script, then installsgccandlibc6-dev, whichrustcrequires to link binaries (it invokes the system C compiler driver). Note:libc6-devis listed explicitly because it previously arrived transitively throughlibssl-dev/libmariadb-dev, which the huntsman variant no longer installs.install-dev-wolf.sh— runs the common script, then installs the C++ toolchain and native libraries (checkinstall,g++,gcc,jq,libcurl4,libcurl4-openssl-dev,libmariadb-dev,libssl-dev,make,openjdk-11-jdk,pkg-config) and performs the CMake install + version check, which are Wolf-only.tools/scripts/lib_install/linuxtotools/scripts/lib_install/ubuntu: the scripts useapt-getand Debian-family package names, which are not portable across Linux distributions, and all current consumers run Ubuntu (CI runners and the dev container). The new layout also telegraphs how support for another distribution would be added (a sibling directory with the same script names)..github/workflows/code-linting-checks.yaml:lint-commonandlint-pythonnow use the common script;lint-cppuses the wolf script;lint-rustuses the huntsman script..github/workflows/tests.yaml:wolf-testsuses the wolf script;huntsman-testsuses the huntsman script..devcontainer/Dockerfile: runs both the wolf and huntsman scripts since the dev container is a full development environment for both versions.Checklist
breaking change.
Validation performed
Summary by CodeRabbit
Release Notes