Skip to content

Conversation

@graydon
Copy link
Contributor

@graydon graydon commented Mar 27, 2025

Part of stellar/rs-soroban-sdk#1428 -- just moving us to rust 1.84 and wasm32v1-none for the wasm build (and fixing clippy fallout).

@graydon graydon requested a review from a team as a code owner March 27, 2025 04:46
@github-project-automation github-project-automation bot moved this to Backlog (Not Ready) in DevX Mar 27, 2025
Copy link
Member

@leighmcculloch leighmcculloch left a comment

Choose a reason for hiding this comment

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

One change is needed, commented inline.

Also, we need to think about how this will work for folks with older contracts that use an older version of rustc. So I think for the contract build command we need to choose the target based on the version of Rust installed and in use.

Otherwise lgtm.

We'll want to delay merging this closer to / after when the SDK is released so that the init commands that generate the contract Cargo.toml will require a late enough Rust version.

@graydon
Copy link
Contributor Author

graydon commented Mar 28, 2025

One change is needed, commented inline.

Pushed a fix that undoes the backticks and adds a local clippy-allow. I think this is what you wanted?

Also, we need to think about how this will work for folks with older contracts that use an older version of rustc. So I think for the contract build command we need to choose the target based on the version of Rust installed and in use.

Not sure I understand this -- it's a rare contract that's going to not work on 1.84. Are we typically promising "never upgrading rust versions"?

We'll want to delay merging this closer to / after when the SDK is released so that the init commands that generate the contract Cargo.toml will require a late enough Rust version.

Ok.

@leighmcculloch
Copy link
Member

I separated the rust upgrade into:

@leighmcculloch leighmcculloch changed the title Adapt to rust 1.84 and wasm32v1-none target Adapt to wasm32v1-none target Apr 15, 2025
@leighmcculloch
Copy link
Member

leighmcculloch commented Apr 15, 2025

Are we typically promising "never upgrading rust versions"?

No, but existing contracts should be able to continue to build with the new tooling, and existing contracts explicitly refuse to build on anything newer than 1.81. Anyone who has already built contracts, and is building a new contract that they presumedly want to use a new SDK and new CLI for, when they go back and run build on their recent (not actually that old) contract will see build errors.

I think we need a small tweak to branch on the version of Rust. If the version of Rust in use is >= 1.84, use wasm32v1-none, if < 1.84, use wasm32-unknown-unknown. I was wondering if we need to also branch on the SDK version in the contract, but I don't think so.

cc @stellar/devx

@leighmcculloch
Copy link
Member

leighmcculloch commented Apr 15, 2025

A mistake that might be easy for devs to make is that they could have scripts that pull the wasm file from the old target directory, and that file could still be present on disk but not updating. Some ideas:

  • CLI output an ℹ️ text block informing the user about the change.
  • Or, if the CLI detects a target/wasm32-unknown-unknown when building for wasm32v1-none, or vice-versa, the CLI could delete the other before building, outputting an ℹ️ either informing that it has happened or ❓ checking and waiting for the user to confirm to do it (probably the better thing to do).

@graydon
Copy link
Contributor Author

graydon commented Apr 15, 2025

If the version of Rust in use is >= 1.84, use wasm32v1-none, if < 1.84, use wasm32-unknown-unknown.

I think you mean "if < 1.82" right? Versions 1.82 and 1.83 are, AIUI, something we don't ever want anyone compiling with, because they don't support wasm32v1-none but do enable the objectionable target features when building for wasm32-unknown-unknown.

@leighmcculloch
Copy link
Member

Yes right. I wasn't sure what to do with those versions. But I guess the cli errors?

@janewang
Copy link
Contributor

fyi @ElliotFriend @briwylde08 We may need to update the dev docs as well. This changes the wasm path. Even our getting started guide may need to provide note for this

@ElliotFriend
Copy link
Contributor

Love to see this getting closer! Thanks for the hard work everyone!

@fnando
Copy link
Member

fnando commented May 2, 2025

Closing this in favor of #2022

@fnando fnando closed this May 2, 2025
@github-project-automation github-project-automation bot moved this from Backlog (Not Ready) to Done in DevX May 2, 2025
@fnando fnando deleted the wasm32v1-none branch August 14, 2025 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

6 participants