Patch dependencies whose latest versions are no longer compatible with 1.75

[mxgrey]

This PR attempts to fix the CI for version 1.75, locking down specific versions of dependencies whose latest versions are no longer compatible.

[mxgrey]

@koonpeng this PR fixes a CI failure related to a dependency problem, but it seems there's still a test failure related to a panic for the Join operation.

I'd suggest merging this in despite the CI still not passing, and then you can continue working on the implementation of Join to fix the remaining panic.

[koonpeng]

Shouldn't we pin home in cargo.toml instead of this?

[mxgrey]

Pinning and committing it inside the Cargo.toml would lock all downstream users into this specific version. That could cause dependency conflicts if they have other dependencies that use a newer version of home.

The specific version of home is only needed in order to compile the library with Rust version 1.75, which is something that we're testing in our CI just in case we ever want to release a version of bevy_impulse on the ROS build farm.

There's a more detailed explanation of the same problem here. In the ros2-rust case we found a simpler solution, but that simpler solution doesn't apply to bevy_impulse.

[koonpeng]

After reading up on the cause of the issue and exploring various solutions, I think the best approach would be to add --ignore-rust-version in CI. It appears this is a fundamental design decision for rust (for now, 1.84 will support a MSRV-aware resolver). The core of the issue is that bumping the MSRV is designed to be a minor change even though it breaks existing builds. Pretty much every solution we can do has some problems

• Hack the CI to use older home (this PR). This doesn't really address the issue, users on rust 1.75 that cargo add bevy_impulse -F=diagram will not be able to build, it's hard to say that we support 1.75 if downstream needs to run the same hack.
• Pin home to 0.5.9. This would support rust 1.75, but would break if there is a package that require home >0.5.9. Cargo only support multiple versions of the same package if they have different major versions, since this is a minor bump, cargo will refuse to build. User would have to patch bevy_impulse if they want to use both bevy_impulse and another package that requires newer home.
• Patch home to use 0.5.9. This has the same issue as this PR in that the patch will not affect downstream users, they would have to add the patch to their Cargo.toml as well.
• Commit Cargo.lock. Will not be propagated to downstream users as well.
• Downgrade cel-interpreter to 0.8.1 which still works with rust 1.75, but it is missing important features that we need.

[mxgrey]

users on rust 1.75 that cargo add bevy_impulse -F=diagram will not be able to build, it's hard to say that we support 1.75 if downstream needs to run the same hack.

Our support for 1.75 is specifically targeted at environments where all available crates are selected to be compatible with Rust 1.75, e.g. a Debian-based crate registry, which is what we expect the build farm to be like if/when it rolls out official support for Rust crates. In such a registry the home crate would be limited to 0.5.9 and so cargo would find that version during its normal lookup without us needing to make any changes to the Cargo.toml.

I am not trying to accommodate users who have 1.75 installed locally and are trying to use cargo without running rustup. Those users are welcome to take advantage of this same technique in their own downstream crates to deal with their unusual use case.

I'm surprised that --ignore-rust-version would help since that means the maintainers of home increased the MSRV without actually needing to.. maybe that's just a standard practice of the home maintainers. But adding that flag will not help in the future if we ever get a dependency that actually requires a newer Rust feature. We will just get a confusing compilation error later in the build process. If that happens we'll end up going back to this workaround anyway.

Please give one more round of consideration to this approach.

[koonpeng]

I think if we ever get a dependency that no longer compiles on 1.75, then we need to consider maintaining multiple branches. For the case of a third party cargo registry, this shouldn't affect them as they should not see the updated dependency. But I think that if we are testing for that scenario, then we should test with that third party registry instead of trying to replicate that from crates.io, debian, being known to do a lot of patching on their repository, home 0.5.9 might not be the same home 0.5.9 on the debian repository. debian might even use home 0.5.11 but patch it so it compiles on rustc 1.75.

In the end, I don't think there is any good solution until either cargo supports a MSRV aware resolver (it is scheduled on the next release), or when we can test with the third party registry.

[mxgrey]

if we are testing for that scenario, then we should test with that third party registry instead of trying to replicate that from crates.io

That registry doesn't exist yet. It's a hypothetical target, so we have no choice but to attempt to replicate it from crates.io. The alternative would be to not try at all, in which case we may find ourselves in a year needing to claw back a dependency or a new language feature that could have been avoided if we were aware that it was going to be incompatible with 1.75.

debian, being known to do a lot of patching on their repository, home 0.5.9 might not be the same home 0.5.9 on the debian repository.

We don't need to care about exactly what version they use, what we need is for our dependencies specifically inside of the 1.75 workflow to be compatible with Rust version 1.75 so we can ensure that new language features don't crawl into our own code and also ensure that some API-comaptible version of each of our dependencies is compatible with 1.75.

This PR does not embody a perfect solution to the problem, but it is the only feasible approach to damage minimization that anyone has put forward so far.

In your PR #27 you've added several new dependencies, but also added --ignore-rust-version to the CI for 1.75, so we don't actually know if the versions of these added dependencies will compile with 1.75 or if they've already pinned an MSRV above 1.75, similar to home.

If you're strongly opposed to the approach in this PR and can't find an alternative that meets the needs of giving some assurance that we'll remain compatible with the environment that we expect the build farm to provide, then I can add this topic to the agenda of the next PMC meeting. There the committee can discuss whether the hypothetical target of the build farm is important and what an acceptable set of trade-offs would be for targeting it.

[koonpeng]

I think there are pros and cons to both ways, the third party registry would not contain packages that can't be build on 1.75, and because MSRV is a minor change, even if a new update bumps it and it no longer builds on 1.75 without --ignore-rust-version, it shouldn't affect users on the third party registry as either the new version would be patched so it works or not be updated on the registry.

I think something to consider is setting up a separate repo that keep tracks of crates that potentially breaks on the build farm, maybe provide a github action to "build on the build farm" (or what the environment would look like when it is ready). Technically this breakage would affect any crates that transitively depends on home, a package that has over 90 million downloads, so this would help other people that want to release on the build farm as well.

I guess for now what we can do is to move the patching to a script, build_debian-ros.sh or similar and call the script from the CI. With a future plan to move this to the "build on build farm" gh action.

[mxgrey]

I guess for now what we can do is to move the patching to a script, build_debian-ros.sh or similar and call the script from the CI. With a future plan to move this to the "build on build farm" gh action.

That's a good idea. As you saw I've updated the PR with this approach. I've also removed --ignore-rust-version from the CI, and it's still green. I'll merge this now since I think all the short-term concerns are addressed. The long-term ideal solution to this problem will probably need to wait until the build farm has rolled out support for Rust.