Update to the latest Linebender CI standard. #39
Conversation
There was a problem hiding this comment.
I was going to suggest moving to a standard gitignore, but it turns out that we don't really have one 🙃
| @@ -23,3 +23,6 @@ instant = "0.1" | |||
| byte-unit = "4.0.19" | |||
| inquire = "0.7" | |||
| ureq = "2.9.6" | |||
|
|
|||
| [target.'cfg(target_arch = "wasm32")'.dependencies] | |||
| getrandom = { version = "0.2.15", features = ["js"] } | |||
There was a problem hiding this comment.
We should probably have a comment here, because we don't use this dependency ourselves. I recognise that the corresponding Vello example almost certainly doesn't have such a comment.
There was a problem hiding this comment.
Are we using getrandom currently? I don't think the "example" code (e.g. with_winit) has been updated in a long time. I am assuming this was copy/pasted from vello which does use it?
There was a problem hiding this comment.
@simbleau rand requires a source of randomness. To get this, it uses the getrandom crate, but for WebAssembly, it doesn't enable anything by default for wasm32-unknown-unknown targets, so we have to tell it to enable something:
https://github.com/rust-random/getrandom?tab=readme-ov-file#webassembly-support
(The config here should be more specific and only happen for wasm32-unknown-unknown but that's a separate thing.)
There was a problem hiding this comment.
I've ran into that issue many times and am aware, but I don't see any new use of rand, or any changes to the example, making me wonder why it's being added now?
There was a problem hiding this comment.
Without looking, I assume because updating the CI builds more now for wasm32 and so this is related to that. But you can probably verify that.
There was a problem hiding this comment.
It was always required, the old CI just wasn't good enough to catch the issue.
| @@ -1,7 +1,3 @@ | |||
| # Remove Cargo.lock from gitignore if creating an executable, leave it for libraries | |||
| # More information here https://doc.rust-lang.org/cargo/guide/cargo-toml-vs-cargo-lock.html | |||
| Cargo.lock | |||
There was a problem hiding this comment.
@simbleau All Linebender crates (apart from stragglers like this one) have a Cargo.lock committed to git. The Rust team changed their guidance.
Also, see https://github.com/linebender/rfcs/blob/main/rfcs/0005-version-matrix.md
| @@ -23,3 +23,6 @@ instant = "0.1" | |||
| byte-unit = "4.0.19" | |||
| inquire = "0.7" | |||
| ureq = "2.9.6" | |||
|
|
|||
| [target.'cfg(target_arch = "wasm32")'.dependencies] | |||
| getrandom = { version = "0.2.15", features = ["js"] } | |||
There was a problem hiding this comment.
Are we using getrandom currently? I don't think the "example" code (e.g. with_winit) has been updated in a long time. I am assuming this was copy/pasted from vello which does use it?
As was pointed out in [velato#39](linebender/velato#39) this `js` feature specification can be more narrow than all of `wasm32` and should also be documented.
The changes can be best compared to [velato#39](linebender/velato#39).
As was pointed out in [velato#39](linebender/velato#39) this `js` feature specification can be more narrow than all of `wasm32` and should also be documented.
The new CI script can be best compared to vello.