feat: default to Rust foundryup#15498
Conversation
mablr
left a comment
There was a problem hiding this comment.
With migration now enabled by default, a first-run foundryup --version bootstraps the sidecar and writes all of the bootstrap/status output to stdout before the version line, including instructions to add the internal sidecar path to PATH. I reproduced this with a fresh FOUNDRY_DIR stdout contained the migration/install messages and only then foundryup 0.0.5 .... We should probably redirect the bootstrap/install chatter to stderr.
mattsse
left a comment
There was a problem hiding this comment.
Thanks for working on the migration. I found one behavior regression that should be fixed before flipping the default.
migrate_and_exec now calls ensure_rust_foundryup for every non---update invocation before execing the Rust sidecar. That makes previously local/offline commands like foundryup --help, foundryup --version, foundryup --list, and foundryup --use <installed-version> require a successful first-time sidecar bootstrap. If the user is offline, GitHub is unavailable, or the bootstrap download fails, those commands fail before they can print local help/version info or inspect already-installed versions. The Rust binary itself treats --list/--use as offline paths, but the shim prevents reaching those paths unless the sidecar already exists.
A fix would be to keep truly local commands on a local path until the sidecar exists. For example, handle -h|--help and -v|--version directly in the shim, and either route -l|--list / -u|--use through the legacy Bash implementation when rust_foundryup_bin is missing, or only attempt bootstrap for those commands after confirming the sidecar is already installed. Please also add a top-level shim test that stubs install_rust_foundryup to fail and verifies the offline commands still succeed or use the legacy path instead of trying to download.
mablr
left a comment
There was a problem hiding this comment.
Invalid options may still trigger a Rust sidecar bootstrap before failing, but it's a narrow corner case. It's good to go as-is.
|
why doesnt migration just replace |
mablr
left a comment
There was a problem hiding this comment.
LGTM overall, just one question.
DaniPopes
left a comment
There was a problem hiding this comment.
once we switched the redirects, we should also deprecate the foundryup/install
Migrates
foundryupto the Rust rewrite. The bash script is now a thin bootstrap: on first run it installs the pinned Rust foundryup (v0.0.6) and replaces itself with the binary, so all subsequent runs use the Rust foundryup directly.