Skip to content

feat: default to Rust foundryup#15498

Open
stevencartavia wants to merge 9 commits into
masterfrom
steven/default-foundryup-migration
Open

feat: default to Rust foundryup#15498
stevencartavia wants to merge 9 commits into
masterfrom
steven/default-foundryup-migration

Conversation

@stevencartavia

@stevencartavia stevencartavia commented Jun 30, 2026

Copy link
Copy Markdown
Member

Migrates foundryup to 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.

@mablr mablr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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 mattsse left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

Comment thread foundryup/foundryup Outdated
Comment thread foundryup/foundryup Outdated
mablr
mablr previously approved these changes Jul 1, 2026

@mablr mablr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

Comment thread foundryup/foundryup Outdated
Comment thread foundryup/foundryup Outdated
Comment thread foundryup/foundryup Outdated
Comment thread foundryup/foundryup Outdated
@DaniPopes

Copy link
Copy Markdown
Member

why doesnt migration just replace foundryup script with the rust executable directly? i don't think we want to allow going back to the bash script, it's not that big of a migration

@stevencartavia stevencartavia marked this pull request as draft July 2, 2026 17:30

@mablr mablr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM overall, just one question.

Comment thread foundryup/foundryup Outdated
@stevencartavia stevencartavia marked this pull request as ready for review July 3, 2026 03:39
@mablr mablr requested review from figtracer, mablr and mattsse July 3, 2026 08:38

@mablr mablr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@DaniPopes DaniPopes left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

once we switched the redirects, we should also deprecate the foundryup/install

@mattsse mattsse left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

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

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

5 participants