-
Notifications
You must be signed in to change notification settings - Fork 64
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
add std
,alloc
feature flag to support no_std
build
#111
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
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
std
withcore
andalloc
- Rework of the header iterator
- Removal of the remaining
String
uses - 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. |
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
NgxListIterator
to removeString
usage - fix: replace other
String
usage by bavshin-f5's impl
(use alloc::string::ToString
addition in the test code should be moved to theno_std
commit) - chore: add Cargo features
- fix: replace
std
tocore
and add config out
Proposed changes
add features
std
,alloc
to prepare forno_std
build support.alloc
feature means memory allocation used. Implementation differs depending onstd
feature:std
: usestd
crate components. (e.g.std::string::String
)std
: usealloc
crate components. (e.g.alloc::string::String
)std
feature meansstd
crate components used. Currently the only difference betweenstd
andalloc
is impl ofstd::error::Error
(core::error::Error
is not in Rust 1.79.0), which is not used.alloc
is the transitional feature: while it enablesno_std
build, it allows memory allocation usingstd::alloc::System
internally. We can gradually replacealloc
crate components by nginx-based ones.For these features, many
std
imports are replaced bycore
ones.Related issue (enhancement): #109
Checklist
Before creating a PR, run through this checklist and mark each as complete.