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

WIP/oliver/no_std #39

Closed
wants to merge 2 commits into from
Closed

Conversation

OliverUv
Copy link
Contributor

@OliverUv OliverUv commented Feb 1, 2023

Hello!

Is no_std (but not no_alloc) something you're interested in supporting?

Making this PR to gather input and because I'm having trouble getting travis-ci to run on my fork of the repo. Of course my hope is that this will be merged for an eventual 2.0 version of pem-rs.

Need to do some testing before this is considered for merging. (see the the FIXME on L52 of src/errors.rs)

@jcreekmore
Copy link
Owner

Yes, no_std support is something I am interested in. I have a few ideas for things that need to go into a 2.0 (like actually supporting headers as mentioned in #36 instead of just allowing the parse to happen without retrieving them) so I could see a 2.0 happening sooner rather than later.

I will have to take a closer look at your FIXME tomorrow when I am not on my phone. Also, I am not entirely sure that the Travis file is even working anymore. I have been meaning to convert over to GitHub Actions so I might do that shortly. Let me take a closer look at this all tomorrow.

@jcreekmore
Copy link
Owner

Alright, after spending a little time working through it, I added Github Actions to replace Travis since that didn't seem to be working. While doing that, I went ahead and am declaring that the next few PRs are all going to be working towards a 2.x, so I went ahead and moved to the 2021 edition and put the MSRV to 1.67. So, if you want to fix up your PR and rebase on top of current master, that should be a bit cleaner.

Also, unless you just really feel strongly about providing a source function for PemError in the no_std mode, I would just drop that impl PemError. I think I would rather wait and see if they are going to stabilize core::error soon (rust-lang/rust#103765) rather that provide a non-conforming implementation that will be another API bump to resolve.

@OliverUv
Copy link
Contributor Author

OliverUv commented Feb 6, 2023

Leaving out the no_std fn source impl until core::Error is stable sounds like the right approach.

Is it alright if I rebase this on top of for-2.x instead? I spotted a use std::fmt there that needs to be a use core::fmt for no_std compat.

@jcreekmore
Copy link
Owner

That is fine. Just point the merge request onto that branch as well. I am still poking at that when I get a chance (adding support for viewing/setting the PEM headers is making me change things a bit), but I will go ahead and get your stuff accepted into that branch before I pull that one into master and it will make it so that the other changes I make are also all compatible with no_std.

@OliverUv
Copy link
Contributor Author

OliverUv commented Feb 6, 2023

Closing to target for-2.x instead

@OliverUv OliverUv closed this Feb 6, 2023
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.

None yet

2 participants