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.

Sign up for GitHub

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

Jump to bottom

Upgrade to Rust 1.68.2 #2494

Merged
merged 2 commits into from
Apr 6, 2023
Merged

Upgrade to Rust 1.68.2 #2494

merged 2 commits into from
Apr 6, 2023

Conversation

link2xt
Copy link
Contributor

@link2xt link2xt commented Mar 10, 2023 •
edited
Loading

Runs on Android 11 32-bit powered by a Snapdragon 801 (32-bit processor) as reported by @gerryfrancis

It is reported that it does not build with NDK 21 by @adbenitez. Probably only r23 is supported now: rust-lang/rust#103673 Older than r23 does not have libunwind, and newer does not support Android API 16.

@link2xt
Copy link
Contributor Author

link2xt commented Mar 10, 2023

Release notes say
"As previously announced, Android platform support in Rust is now targeting NDK r25, which corresponds to a minimum supported API level of 19 (KitKat)." but looks like it still works for our NDK r23c.

@link2xt link2xt mentioned this pull request Mar 10, 2023
Merged
adbenitez
adbenitez approved these changes Mar 10, 2023
View reviewed changes
Copy link
Member

@adbenitez adbenitez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be good if before merging someone with an android 4 device can give a try to this to be sure this works fine

@link2xt link2xt force-pushed the link2xt/rust-1.68 branch 2 times, most recently from b4975e1 to 9bdc057 Compare March 10, 2023 19:59
@deltachat deltachat deleted a comment from github-actions bot Mar 10, 2023
@deltachat deltachat deleted a comment from github-actions bot Mar 10, 2023
@link2xt
Copy link
Contributor Author

link2xt commented Mar 10, 2023 •
edited
Loading

Works for me on 64-bit ARM, so can be merged once @r10s or someone else (who has Android 4 around?) tests this on Android 4.

By the way we will be able to use newer Rust with NDK r23c (the latest supporting Android 4) because Rust is going to stabilize build-std feature at some point, allowing us to rebuild standard library against NDK of our own choose.

@github-actions
Copy link

To test the changes in this pull request, install this apk:
📦 app-preview.apk

@flub
Copy link
Member

flub commented Mar 11, 2023

So what does this mean for the MSRV of deltachat? Is this now bumped to 1.68 or still on 1.63? Where do we keep track of this?

@link2xt
Copy link
Contributor Author

link2xt commented Mar 11, 2023 •
edited
Loading

So what does this mean for the MSRV of deltachat? Is this now bumped to 1.68 or still on 1.63? Where do we keep track of this?

MSRV is tracked in the deltachat-core-rust CI, it is still at 1.63:
https://github.com/deltachat/deltachat-core-rust/blob/a853b8283a789e944f7bb3ce6270679e93b52153/.github/workflows/ci.yml#L81-L88

The version used to build Android was decoupled from the core in #2440.
Each platform can choose its own version of Rust and upgrade at its own pace. MSRV can be bumped once all platfroms are ok with it. Currently iOS is blocking increasing MSRV above 1.64.0: deltachat/deltachat-ios#1771

@r10s
Copy link
Member

r10s commented Mar 11, 2023

i am currently building this and will try out on my android 4.4 (kitkat, api19) device - i do not have an android 4.1 (jellybean, api16) device, but iirc we got a donation at some point, should be at bb33

@r10s
Copy link
Member

r10s commented Mar 11, 2023 •
edited
Loading

interim status: up to now, i did not manage to make this pr working on my old android device nor in any emulator :/

this is not necessarily related to this pr, idk yet.

one always has all kind of issues when trying out old androids (battery, broken usb ports, android studio and emulator issues ...) - where i succeeded to transfer an apk, that crashes - however, one has to make sure that it does not crashes on master then

otoh, i just installed #2493 on a nexusS with KitKat - and that works. but i am failing to connect this device now ...

so, no green light yet - or did someone else manage to make this run on sth. before Lollipop?

@flub @dignifiedquire @link2xt is rust1.68 important for the current iroh ongoings? otherwise i'd like to postpone the tests, they're very time consuming.

@dignifiedquire
Copy link
Member

@flub @dignifiedquire @link2xt is rust1.68 important for the current iroh ongoings? otherwise i'd like to postpone the tests, they're very time consuming.

no need from irohs side atm

@link2xt
Copy link
Contributor Author

link2xt commented Mar 11, 2023

This can be postponed, not urgent at all, especially since iOS is blocking MSRV update too.

@github-actions
Copy link

To test the changes in this pull request, install this apk:
📦 app-preview.apk

@link2xt link2xt changed the title Upgrade to Rust 1.68.0 Upgrade to Rust 1.68.1 Mar 24, 2023
@github-actions
Copy link

To test the changes in this pull request, install this apk:
📦 app-preview.apk

@github-actions
Copy link

To test the changes in this pull request, install this apk:
📦 app-preview.apk

@github-actions
Copy link

To test the changes in this pull request, install this apk:
📦 app-preview.apk

@github-actions
Copy link

To test the changes in this pull request, install this apk:
📦 app-preview.apk

@link2xt link2xt mentioned this pull request Mar 27, 2023
Merged
@link2xt link2xt changed the title Upgrade to Rust 1.68.1 Upgrade to Rust 1.68.2 Mar 31, 2023
@github-actions
Copy link

To test the changes in this pull request, install this apk:
📦 app-preview.apk

@github-actions
Copy link

github-actions bot commented Apr 3, 2023

To test the changes in this pull request, install this apk:
📦 app-preview.apk

@github-actions
Copy link

github-actions bot commented Apr 4, 2023

To test the changes in this pull request, install this apk:
📦 app-preview.apk

@link2xt
Copy link
Contributor Author

link2xt commented Apr 5, 2023

@r10s This is tested on 32-bit androids so once you confirm that building with rust 1.68.2 works for you after running scripts/install-toolchains.sh, I think it can be merged. Then we will have nightly builds again.

r10s
r10s approved these changes Apr 6, 2023
View reviewed changes
Copy link
Member

@r10s r10s left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iirc building is not an issue, it was just that i could not verify nor falsify working on android4

i think, we can give it a try, ppl with android4 devices may complain in time, once we have nightlies again :)

@link2xt
Update to Rust 1.68.2
f6562c4
@link2xt
Remove workaround for Rust built against r22b NDK
fa7812a 
Rust 1.68.0 targets NDK r25, so the workaround is no longer needed.
@link2xt link2xt merged commit fa7812a into master Apr 6, 2023
@link2xt link2xt deleted the link2xt/rust-1.68 branch April 6, 2023 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adbenitez adbenitez adbenitez approved these changes

@r10s r10s r10s approved these changes

@Hocuri Hocuri Awaiting requested review from Hocuri

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@link2xt @flub @r10s @dignifiedquire @adbenitez