-
Notifications
You must be signed in to change notification settings - Fork 80
add std,alloc feature flag to support no_std build
#111
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
Conversation
|
Thanks! One comment: I'd prefer to remove |
follow bavshin-f5's comment: nginx#111 (comment) copied implementation: Display for ngx_str_t, and test <= bavshin-f5/ngx-rust@3f054e3 add_to_ngx_table <= bavshin-f5/ngx-rust@ae2ee99
|
Thank you for the advice. |
follow bavshin-f5's comment: nginx#111 (comment) copied implementation: Display for ngx_str_t, and test <= bavshin-f5/ngx-rust@3f054e3 add_to_ngx_table <= bavshin-f5/ngx-rust@ae2ee99
follow bavshin-f5's comment: nginx#111 (comment) copied implementation: Display for ngx_str_t, and test <= bavshin-f5/ngx-rust@3f054e3 add_to_ngx_table <= bavshin-f5/ngx-rust@ae2ee99
bavshin-f5
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comments for Request::headers_in_iterator/Request::headers_out_iterator need to be updated.
Please, squash all the fixup commits and clean up the history.
I believe this PR can be reorganized into 3-4 commits with distinct logical changes. For example,
- Replacing
stdwithcoreandalloc - Rework of the header iterator
- Removal of the remaining
Stringuses - Feature flags and documentation
|
Thank you for the reviewing. I'll fix the points one by one, and after that do squash. |
|
chores:
|
Worker processes crash when running awssig example code:
That's the only example that uses the header iterators, the crash is likely in |
|
commit squashed. |
bavshin-f5
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost ready, just a couple more issues.
98cb85d and 0174f10 fail to build. Please, consider reordering the commits as shown below to make each commit buildable:
- fix: refactor
NgxListIteratorto removeStringusage - fix: replace other
Stringusage by bavshin-f5's impl
(use alloc::string::ToStringaddition in the test code should be moved to theno_stdcommit) - chore: add Cargo features
- fix: replace
stdtocoreand add config out
Proposed changes
add features
std,allocto prepare forno_stdbuild support.allocfeature means memory allocation used. Implementation differs depending onstdfeature:std: usestdcrate components. (e.g.std::string::String)std: usealloccrate components. (e.g.alloc::string::String)stdfeature meansstdcrate components used. Currently the only difference betweenstdandallocis impl ofstd::error::Error(core::error::Erroris not in Rust 1.79.0), which is not used.allocis the transitional feature: while it enablesno_stdbuild, it allows memory allocation usingstd::alloc::Systeminternally. We can gradually replacealloccrate components by nginx-based ones.For these features, many
stdimports are replaced bycoreones.Related issue (enhancement): #109
Checklist
Before creating a PR, run through this checklist and mark each as complete.