-
Notifications
You must be signed in to change notification settings - Fork 50
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
build: Add build for ios and android #44
build: Add build for ios and android #44
Conversation
Work funded by the Government of Ontario. Signed-off-by: blu3beri <blu3beri@proton.me>
9466fe2
to
bebbc42
Compare
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.
Where does it mention they only accept security PRs? Not sure what to do if we can't get it in. This hasn't been solved for indy-credx yet a while ago?
@@ -8,3 +8,6 @@ members = [ | |||
[profile.release] | |||
lto = true | |||
codegen-units = 1 | |||
|
|||
[patch.crates-io] | |||
openssl-src = { git = "https://github.com/blu3beri/openssl-src-rs", branch = "release/111" } |
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.
Maybe add a comment here why it's patched?
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.
Where does it mention they only accept security PRs? Not sure what to do if we can't get it in. This hasn't been solved for indy-credx yet a while ago?
Here for example: alexcrichton/openssl-src-rs#130 (comment)
I don't think it was fixed for credx, I vaguely remember the issue from before. Would be nice if we can just have a patch file in this repo with the fix. Can check for that later on.
We may be able to say it IS pressing as otherwise we'll have forks that don't necesarily improve security 🤷
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.
Yeah I will create a PR with it. If it is merged I will remove the fix but leave this in for now.
EDIT: opened here alexcrichton/openssl-src-rs#175
Here for example: alexcrichton/openssl-src-rs#130 (comment) I don't think it was fixed for credx, I vaguely remember the issue from before. Would be nice if we can just have a patch file in this repo with the fix. Can check for that later on. |
However, this works for now. |
What would be the effort to upgrade to openssl 3? |
It is coming from |
Work funded by the Government of Ontario. Signed-off-by: blu3beri <blu3beri@proton.me>
@TimoGlastra Can we merge this? I think the comments are resolved. |
Go for it |
Depends on #43
closes #45
closes #46
aarch64-apple-ios-sim
. Since they do not accept non-security fixes for this branch we can not get this upstream, unless we depend upon openssl 3.Work funded by the Government of Ontario.
Signed-off-by: blu3beri blu3beri@proton.me