Skip to content
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

Why is MSRV 1.79? #76

Closed
nfejzic opened this issue Oct 26, 2024 · 10 comments · Fixed by #82
Closed

Why is MSRV 1.79? #76

nfejzic opened this issue Oct 26, 2024 · 10 comments · Fixed by #82

Comments

@nfejzic
Copy link

nfejzic commented Oct 26, 2024

Hello! Nice work with the crate, really like it!

I have a question regarding the minimum supported rust version, which is at the moment of this writing set to 1.79. I tried building the crate on my machine using version 1.75 and it builds without any problems, tests run as well.

What is the reasoning behind setting the minimum supported Rust version to 1.79?

@nfejzic
Copy link
Author

nfejzic commented Oct 27, 2024

I just noticed that tests do not pass with Rust 1.75, std::num::NonZero is missing. So I assume that's the reason?

@tqwewe
Copy link
Owner

tqwewe commented Oct 27, 2024

Ah yeah when I saw your issue initially I was trying to remember whats the reason, but as you've pointed out, its because of the NonZero types. I believe if you compile with the remote feature flag, you'll encounter errors since its used within the actor swarm registration error

@nfejzic
Copy link
Author

nfejzic commented Oct 27, 2024

Ok, so using it without the remote feature should work then with older rust versions as well? Would it be too much of a burden to document (and guarantee, maintain, etc.) the minimum supported version when not using remote? I would totally understand if it's too much work though 😄.

@tqwewe
Copy link
Owner

tqwewe commented Oct 27, 2024

Absolutely, its a great suggestion! I added the MSRV before remote was behind a feature flag, but now that it is I can definitely improve documentation around this.

Does Rust let you use kameo on older versions even though the Cargo.toml file has rust-version = "1.79"? Would I need to remove that config from the Cargo toml?

@nfejzic
Copy link
Author

nfejzic commented Oct 27, 2024

Awesome, thank you!

No, it won't build with older versions if rust-version = "1.79" is present. Now that you mention it, it might be worth suggesting to cargo team such feature. Similar how we can have optional dependencies linked to features 🤔.

@nfejzic
Copy link
Author

nfejzic commented Nov 1, 2024

@tqwewe I opened an issue for Cargo: rust-lang/cargo#14770. While waiting for the response, I assume even if this feature is enabled, it won't be available in older cargo versions, making it irrelevant for this issue.

Have you thought about any alternatives? Maybe use rust_version crate to check if rust version is at least 1.79, otherwise emit a compile_error? 🤔

EDIT: Another possible workaround could be to factor out the remote module to a separate crate as an extension for kameo. This would make it possible to have separate MSRV versions for kameo and kameo_remote

@tqwewe
Copy link
Owner

tqwewe commented Nov 6, 2024

Hi sorry for the late reply. Thanks for taking the time to open the issue on cargo GH
I haven't heard of rust_version crate but this could definitely work.

I also am open to the idea of kameo_remote. Is there a particular reason upgrading Rust version isn't possible in your case?

@nfejzic
Copy link
Author

nfejzic commented Nov 6, 2024

I also am open to the idea of kameo_remote. Is there a particular reason upgrading Rust version isn't possible in your case?

That would be nice! As for the reason, we're currently developing software for embedded devices and we use Yocto for this purpose. Unfortunately, upgrading Rust in such environments is not always easy and straightforward. We are working on it, so maybe we will succeed and this won't be necessary, but for now we can't use the crate because it doesn't build with lower rust versions.

@tqwewe
Copy link
Owner

tqwewe commented Nov 15, 2024

Sorry for the delay, this should be resolved in 0.13 of kameo

@nfejzic
Copy link
Author

nfejzic commented Nov 15, 2024

Sorry for the delay, this should be resolved in 0.13 of kameo

No problem, thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants