Skip to content

A few setup fixes#2

Draft
entpiske wants to merge 2 commits into
mainfrom
ap-fixes
Draft

A few setup fixes#2
entpiske wants to merge 2 commits into
mainfrom
ap-fixes

Conversation

@entpiske

Copy link
Copy Markdown
Member

No description provided.

entpiske and others added 2 commits June 11, 2026 10:11
Replicates the macOS rust install from d4fc0d9 to the Ubuntu (apt)
and Arch (pacman) platforms, since rust is required for building
Ruby from source.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@andrepiske

Copy link
Copy Markdown

Code review by Claude (Claude Code)

🤖 This review was performed by Claude (Claude Code), not by the PR author.

The PR does two things: (1) adds a rust install to build_setup.sh for macOS/Ubuntu/Arch, and (2) sets mise settings ruby.compile=false before installing Ruby in mise_setup.sh. The intent (prefer a precompiled Ruby, with build deps available as a fallback) is sound, and mise settings ruby.compile=false is valid syntax on current mise (verified on 2026.6.1). But there are several real issues:

1. "rust is required for building Ruby from source" is inaccurate (all 3 platforms)

Rust is not required to build Ruby. Ruby's ./configure only uses Rust to enable YJIT; if rustc is missing it prints a warning and builds Ruby without YJIT — the build still succeeds. So the comments (and the commit message of 4cf0a6f) overstate the dependency.

2. The rust install is redundant in the common path

ruby.compile=false makes mise prefer precompiled binaries (it only falls back to source compilation when no precompiled binary exists, per the mise docs). Precompiled Ruby binaries already ship with YJIT. So in the normal case Ruby is downloaded, never compiled, and the freshly-installed rust/libyaml/build-essential are never touched. The rust install only does anything on the fallback-compile path — and even then only enables an optional feature.

3. Ubuntu's apt rustc is likely too old to actually enable YJIT

On Ubuntu LTS, apt-get install rustc installs a distro Rust that is frequently too old for YJIT's minimum Rust version. So on the one path where rust would matter (fallback source compile), sudo apt-get install -y -qq rustc cargo may not even achieve the stated goal — YJIT would still be skipped. mise/ruby-build generally expect a recent rustup-managed toolchain.

4. Pre-existing gap this PR makes more likely to bite (Ubuntu)

build_setup.sh installs the source-build dev headers (libssl-dev libreadline-dev zlib1g-dev libyaml-dev) only inside the if ! dpkg -s build-essential branch. On a machine where build-essential is already present but those -dev headers aren't, they never get installed. Now that compile=false + ruby@latest makes the source-compile fallback a real code path (precompiled binaries for the newest release often lag by days/weeks), a fallback build could fail with missing openssl/yaml headers. Consider decoupling the header install from the build-essential check.

Minor

  • Ubuntu check/install mismatch: the guard is dpkg -s rustc but it installs rustc cargo. If rustc is present but cargo isn't, cargo is skipped. (For YJIT only rustc is needed anyway, so installing cargo is also unnecessary.)
  • macOS brew install rust: the rust formula works but can conflict with a rustup-managed toolchain on PATH; rustup is Homebrew's/ruby-build's recommended route.
  • Style: echo 'setting mise ruby.compile=false' uses a raw, unindented echo while the rest of the file uses the fmt_* helpers and 2-space-indented messages.

Recommendation

Decide which path you actually want:

  • If the goal is fast, precompiled Ruby (which compile=false achieves), the rust installs add install time and complexity for no benefit in the success path — consider dropping them, or at minimum fix the comments to "optional, enables YJIT on the fallback source build."
  • If you want reliable YJIT on the fallback build, apt rustc won't reliably deliver it — prefer rustup, and move the Ubuntu -dev headers out of the build-essential conditional so the fallback compile can't fail on missing headers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants