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

KTOR-5753: Enable linuxArm64 builds #3587

Merged
merged 1 commit into from
Jul 14, 2023
Merged

KTOR-5753: Enable linuxArm64 builds #3587

merged 1 commit into from
Jul 14, 2023

Conversation

bcmedeiros
Copy link
Contributor

@bcmedeiros bcmedeiros commented Apr 30, 2023 •
edited
Loading

Subsystem
Server and Client

Motivation
https://youtrack.jetbrains.com/issue/KTOR-5753

Solution
This is spike to enable linuxArm64 for ktor.

Caveats:

  • This is using kotlinx-coroutines 1.7.0-RC, so we should probably wait for the final GA release, and possibly also make this upgrade as a separate PR. Happy to do that if you guys think it's the best way done, Upgrade kotlinx-coroutines to 1.7.1 #3609. ✅
  • This is using an un-published version of kotlinx-html containing Enable linuxArm64 native target Kotlin/kotlinx.html#213, as the last published version does not support linuxArm64. Using kotlinx.html 0.9.0 ✅
  • I ended up copying a few files from linuxX64 to the the new linuxArm64. Are we planning to introduce a common linux source set? Done, files are not copied anymore. ✅

Any feedback? Happy to keep helping with this as I need ktor server running on linux arm for one of my projects.

@bcmedeiros bcmedeiros marked this pull request as draft April 30, 2023 14:04
@bcmedeiros bcmedeiros changed the title Enabling linuxArm64 builds KTOR-5753: Enabling linuxArm64 builds Apr 30, 2023
hfhbd
hfhbd reviewed May 8, 2023
View reviewed changes
ktor-network/linuxArm64/src/io/ktor/network/selector/SelectUtilsLinux.kt Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead copying, could you move this file into linuxMain?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll try that, I think I was trying linux before.

hfhbd
hfhbd reviewed May 8, 2023
View reviewed changes
gradle.properties Outdated
@@ -32,8 +32,8 @@ kotlin.native.binary.memoryModel=experimental
#kotlinx.atomicfu.enableJsIrTransformation=true

kotlin_version=1.8.10
coroutines_version=1.6.4
atomicfu_version=0.20.0
coroutines_version=1.7.0-RC
Copy link
Contributor

Choose a reason for hiding this comment

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

1.7.0 is released

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Raised #3609, which contains more things than the automatic one.

@hfhbd
Copy link
Contributor

hfhbd commented May 8, 2023

You should be able to move many copied files into linuxMain.

@bcmedeiros
Copy link
Contributor Author

You should be able to move many copied files into linuxMain.

@hfhbd The project didn't have a linux common folder set up, but I created one and could just move the files to a common place, everything is compiling now. Let me know what you think.

@bcmedeiros bcmedeiros changed the title KTOR-5753: Enabling linuxArm64 builds KTOR-5753: Enable linuxArm64 builds May 12, 2023
@hfhbd
Copy link
Contributor

hfhbd commented May 15, 2023 •
edited
Loading

I am okay with this, but I am not member of Ktor. Especially with serialization 1.5.1, we could add the linuxArm64 target to more ktor modules.

@bcmedeiros
Copy link
Contributor Author

I am okay with this, but I am not member of Ktor. Especially with serialization 1.5.1, we could add the linuxArm64 target to more ktor modules.

Nice, I'm planning to keep this PR up-to-date until it's ready to be merged. There are two blocker PRs in the description, one for coroutines update in this repo, and another one for kotlinx-html, so I think we need some ktor/JB sponsor here.

@bcmedeiros
Copy link
Contributor Author

Just rebased all the work on top of current main.

Main pending task to get this across now is Kotlin/kotlinx.html#213
Can someone from Jetbrains poke the guys on kotlinx.html over this one? We need a very quick merge and a release.

@hfhbd hfhbd mentioned this pull request Jun 13, 2023
Merged
@hfhbd hfhbd mentioned this pull request Jun 21, 2023
Closed
@bcmedeiros bcmedeiros marked this pull request as ready for review June 26, 2023 15:29
@bcmedeiros
Copy link
Contributor Author

@e5l this one is now ready to review.

@e5l e5l self-requested a review July 6, 2023 08:18
@e5l
Copy link
Member

e5l commented Jul 6, 2023

@bcmedeiros could you rebase it on main?

@bcmedeiros
Copy link
Contributor Author

@e5l I thought you'd squash merge it.
Do you want me to rebase it as as single commit? I can do that in about 30 min.

@e5l
Copy link
Member

e5l commented Jul 6, 2023

Thanks for the quick answer! No problem, I'll wait. The kotlinx.html is published, and I also want to run tests the rebased version

@bcmedeiros
Copy link
Contributor Author

@e5l just pushed a single commit with the same net result, let me know if you need anything else :)

@bcmedeiros
Copy link
Contributor Author

bcmedeiros commented Jul 6, 2023 •
edited
Loading

The kotlinx.html is published, and I also want to run tests the rebased version

I was already using the new pusblished version of kotlinx.html before the rebase.

@bcmedeiros
Copy link
Contributor Author

@e5l is any of that related to arm support? Should we split the kotlinx-html upgrade into a separate PR?

@hfhbd
Copy link
Contributor

hfhbd commented Jul 6, 2023 •
edited
Loading

Well, kotlinx.html does not specify the jvmtoolchain, so it uses the jvm from TeamCity, which is Java 11, but ktor requires Java 8. Ideally, kotlinx.html (and ktor) should specify the jvmtoolchain explicitly.
Kotlinx.html still requires a new release: Kotlin/kotlinx.html#226

@e5l
Copy link
Member

e5l commented Jul 6, 2023

@hfhbd, yep. WIP

@e5l
Copy link
Member

e5l commented Jul 6, 2023

@bcmedeiros, we can't. There was no arm version for 0.8.*. I will make new release soon

@e5l
Copy link
Member

e5l commented Jul 12, 2023

kotlinx.html 0.9.1 is out. Could you please update?

@bcmedeiros
Copy link
Contributor Author

@e5l done :)

@e5l e5l merged commit 46ac5de into ktorio:main Jul 14, 2023
@e5l
Copy link
Member

e5l commented Jul 14, 2023

@bcmedeiros, Awesome Job! Thanks. Merged

@bcmedeiros bcmedeiros deleted the linuxArm64 branch July 14, 2023 09:08
@hfhbd hfhbd mentioned this pull request Jul 17, 2023
Merged
@hfhbd
Copy link
Contributor

hfhbd commented Aug 2, 2023

https://repo1.maven.org/maven2/io/ktor/ktor-server-cio-linuxarm64/ 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hfhbd hfhbd hfhbd left review comments

@e5l e5l e5l approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@bcmedeiros @hfhbd @e5l