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

asm-casts: Run on no_std #307

Merged

Conversation

chrysn
Copy link
Contributor

@chrysn chrysn commented Sep 20, 2020

There's nothing in the asm-casts crate that necessitates std (and the std::marker::PhantomData was changed to its alias core::marker::PhantomData). This PR just sets #![no_std] and switches over the PhantomData import.

This allows code produced with asm casts to be used in no_std environments. (Admittedly, that's a bit far-fetched as there's no official support for cross-platform transpilation yet, see #305 -- but it's another stepping stone towards rust-lang/rust-bindgen#1344).

I've tested this in a practical application (it builds and runs -- but I can't tell yet whether it actually executes any place with those casts), and run the local cargo test which runs the crate-internal coverage combo successfully.

@ahomescu
Copy link
Contributor

I approved this, but we need to see what's up with that failing test (might not be caused by your change).

@chrysn
Copy link
Contributor Author

chrysn commented Sep 21, 2020

It's the ##[error]Input required and not supplied: token error I've seen in other failed build runs as well. (The later ##[error]Cannot read property 'replace' of null might be a follow-up error from the token not being set, and thus possibly null).

Looks to me like

token: ${{ secrets.GH_ACTIONS_CONTROL_PRIV_REPOS }}
is missing its input; can't do much about that from the outside as that probably needs configuring somewhere in the CI setup.

@thedataking
Copy link
Contributor

The failing test is because the forked version does not have access to the repo secret needed to pass the GitHub Actions tests. I tried disabling GitHub Actions on third-party PRs but apparently I wasn't successful. Merging.

@thedataking thedataking merged commit bde9c8c into immunant:master Sep 22, 2020
@chrysn
Copy link
Contributor Author

chrysn commented Sep 22, 2020

Thanks for merging.

Would it be possible to upload this to crates.io? I'm approaching the release of the new c2rust based version of riot-sys, and cargo insists all dependencies are uploaded.

(The c2rust which users need to have installed depends on, basically, all the other PRs I currently have open, but that's not so bad because that dependency is outside cargo anyway as c2rust can't be a build dependency AFAIU, but the c2rust-asm-casts are a regular dependency).

@ahomescu
Copy link
Contributor

Pinging @rinon for crates.io releases.

@rinon rinon self-assigned this Sep 23, 2020
@chrysn
Copy link
Contributor Author

chrysn commented Oct 13, 2020

@rinon, is there anything left to do / where I contribute to getting this to crates.io? That dependency is currently the largest stopper to a new round of RIOT sys-/wrapper crates being published.

@chrysn chrysn deleted the nostd-c2rust-asm-casts branch October 13, 2020 06:51
@rinon
Copy link
Contributor

rinon commented Oct 21, 2020

@chrysn new version is published, sorry it took so long.

@chrysn
Copy link
Contributor Author

chrysn commented Oct 21, 2020

I still see https://crates.io/crates/c2rust-asm-casts as being the 0.1.1 of last 2019-10-30, might something have gone wrong during the upload?

@rinon
Copy link
Contributor

rinon commented Oct 21, 2020

I was dumb and missed this crate in the publish. Should be good now @chrysn

chrysn added a commit to RIOT-OS/rust-riot-sys that referenced this pull request May 31, 2022
chrysn added a commit to RIOT-OS/rust-riot-sys that referenced this pull request May 31, 2022
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 this pull request may close these issues.

4 participants