-
Notifications
You must be signed in to change notification settings - Fork 127
Prepare release of v1.21.0 (polkadot-stable2512) #881
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
clangenb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks for handling this.
Regarding your no_std build error, I suppose that we could try changing the return type here to the value mentioned in the error message:
| fn panic(_panic: &PanicInfo<'_>) -> ! { |
.github/workflows/ci.yml
Outdated
| RUSTFLAGS='--cfg substrate_runtime' cargo build --release -p test-no-std --features api-client, | ||
| RUSTFLAGS='--cfg substrate_runtime' cargo build --release -p test-no-std --features compose-macros, | ||
| RUSTFLAGS='--cfg substrate_runtime' cargo build --release -p test-no-std --features node-api, | ||
| RUSTFLAGS='--cfg substrate_runtime' cargo build --release -p test-no-std --features primitives, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be able to set the env at the step level here with:
- name: ${{ matrix.check }}
env:
RUSTFLAGS: '--cfg substrate_runtime'
run: ${{ matrix.check }}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found yet another approach that works with cargo directly by providing a config.toml.
I like this a bit more as it is more general, but so far I only managed to get it working for the wasm32 builds. For no-std I have not (yet) managed to find a solution that works with a config.toml 😞
I have tried this but I could not get this to work. The error message originates from |
I have been looking into it: my superficial impression is that it might not be possible to plainly compile to So maybe we should drop pure no_std currently, and envision risk-v as a potential alternative in the future. |
|
Sorry, a bit late but to my understanding the following is happening: In commit paritytech/polkadot-sdk@93852b1 the RUSTFLAG Our problem is now that some crates, such as sp-storage, are still on the version previous to the commit mentioned above and some crates, such as sp-state-machine already include this commit. At least I'm quite sure that's the problem for the compile errors for the no-std primitives build (https://github.com/scs/substrate-api-client/actions/runs/21025146379/job/60447849672?pr=881#logs). Not sure what's happening with sp-io, but I think there's a good chance that the version mismatch is the problem there as well. Either way - sucks. Nothing we can do but Any preferences? |
I think I would opt for "leave as is" for now. |
haerdib
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, let's release either way. Two comments, otherwise it's perfect. Thank you very much!
.github/workflows/ci.yml
Outdated
| with: | ||
| key: ${{ matrix.check }} | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not necessary, is it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critically important whitespaces! I removed them...poor whitespaces 😭 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha 😆
| A: When specifying your own state query, you must provide the return type of the state you're trying to retrieve. This is because the api-client only gets bytes from the node and must be able to deserialize these properly. That is not possible without knowing the type to decode to. This type may be for example a simple `u64` for retrieving the `Balance` of an account. But careful: If you're looking at the pallet code and its return type, don't forget to take the Query type into consideration. The `OptionQuery` for example automatically wraps the return type into an `Option` (see the [substrate docs "Handling query return values"](https://docs.substrate.io/build/runtime-storage/) for more information). Alternatively, you can always double check via [polkadot.js](https://polkadot.js.org/). | ||
| If you're importing a value directly from the runtime, as it's done in this [example](https://github.com/scs/substrate-api-client/blob/fb108a7d1994705bbca50233e3bc66cec3726523/examples/examples/subscribe_events.rs#L25-L27), remember to adapt it to the node you are querying from. | ||
|
|
||
| 4. Q: I get a compilation error when compiling for `no-std` or `wasm`. For example "unresolved import `sp_core::storage::Storage`" in `sp-state-machine`. What is wrong? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add the explanation about the crate Version mismatch, such that user know they can / should switch to master if it's a problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I expanded the comment a bit.
haerdib
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeyy, thanks for bearing with me!
|
@clangenb v1.21.0 is now published on crates.io. Also see https://github.com/scs/substrate-api-client/releases/tag/v1.21.0 |
DO NOT MERGE
substrate_runtimeRustflag paritytech/polkadot-sdk#10514no-stderror before, I would suggest to create the release regardless and add a note in the release notes.