Skip to content
This repository was archived by the owner on Mar 23, 2024. It is now read-only.

Support no_std (Rust 1.64 and later) (Take 2) #14

Merged
merged 3 commits into from
Sep 24, 2022

Conversation

chrysn
Copy link
Contributor

@chrysn chrysn commented Jul 31, 2022

Closes: #12 (see previous discussion there)
Replaces: #13

Based on the discussion in #13 I've revisited some assumptions, and removed parts of the PR that were based on my wrong understanding of proc-macro crates.

The patches are now in a sequence that, by running CI on them, shows that the test for no-std behavior alone fails, whereas the second patch fixes it.

As before, this breaks support for Rust versions < 1.64, so CI will report all but the "Build on no_std" tests as failing -- so this only makes sense to merge at the point when 1.64 is stable (at which the "nightly" in the test can be switched over).

@chrysn chrysn force-pushed the no-std-3 branch 2 times, most recently from 0acaa3c to e294368 Compare July 31, 2022 18:54
@chrysn
Copy link
Contributor Author

chrysn commented Jul 31, 2022

Relevant build reports, now that I've made the github actions happy:

(But of course everything else is not yet on 1.64, so several other tests fail).

Copy link
Owner

@upsuper upsuper left a comment

Choose a reason for hiding this comment

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

I'm suggesting trybuild to support specifying target. And if that proceeds, it would be easier to add and run such tests, and there wouldn't be a need to have an additional check task.

Let's see how that goes.

@chrysn
Copy link
Contributor Author

chrysn commented Aug 6, 2022 via email

@upsuper
Copy link
Owner

upsuper commented Aug 7, 2022

No, because trybuild already generates test crate automatically. If it can generate a test crate that uses a specific target, the test would be runnable with plain cargo test, and the only change needed for CI task is to install the needed target.

@chrysn
Copy link
Contributor Author

chrysn commented Sep 10, 2022

Short of patching trybuild (which I don't have any capacity to do right now), is there anything I can do to advance this issue?

@upsuper
Copy link
Owner

upsuper commented Sep 11, 2022

Nothing you need to do. I'll probably just merge it when 1.64 reaches stable.

@chrysn
Copy link
Contributor Author

chrysn commented Sep 22, 2022

With 1.64 released, I've been running a round of CI tests at RIOT-OS/rust-riot-wrappers#10 (comment) in parallel to local tests -- unsurprisingly, there were no surprises.

I've added one commit updating all mentions of required Rust versions to reflect the strategy indicated in the last comment, and taken the liberty to add a rust-version = "1.64" as that enhances the user experience in the error case.

Would you consider making a cstr release soon after merging this?

@upsuper upsuper merged commit bac7fde into upsuper:master Sep 24, 2022
@upsuper
Copy link
Owner

upsuper commented Sep 24, 2022

New version published.

@chrysn
Copy link
Contributor Author

chrysn commented Sep 24, 2022

Thanks a bunch!

@chrysn chrysn deleted the no-std-3 branch September 24, 2022 17:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Supporting no_std
2 participants