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.

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

WIP: Support Scala Native #239

Open
wants to merge 53 commits into
base: main
Choose a base branch
from
Open

WIP: Support Scala Native #239

wants to merge 53 commits into from

Conversation

lolgab
Copy link

@lolgab lolgab commented Sep 15, 2023

Fixes #192

Current Status

✅ compiles successfully for all platforms
✅ links successfully for all platforms
jvm / jdk-client tests passing
jvm / ember-client tests NOT passing
js / ember-client tests NOT passing
native / ember-client tests NOT passing
🤘 figure out yaml: since the original circe yaml is not cross-platform, we're using @armanbilge's fork: https://github.com/armanbilge/circe-scala-yaml (nothing bad with it per se; and I don't know if we should be expecting cross-platform builds from the upstream we should not be expecting it from the upstream since it's based on a java library)
❌ stop using a snapshot version of http4s (when websocket support in Ember client lands in a release version)
✅ stop using a snapshot version of fs2 (when clientside mTLS fix lands in a release version)

Ember client

❌ need to fix the websocket issue:

  • stream terminates prematurely due to an exception
  • that exception is currently "masked" but the stream seems to be missing the final frame

native

✅ Ember client (the underlying s2n connection) is not now enabling the mutual TLS and fails when the server requests it
❌ client key password is not supported (s2n doesn't do it) - I'm assuming it's not critical for the vast majority of use cases, though. Currently an error is raised if the password is specified in the configuration. If we want to add support for it, we'll have to parse and decrypt the pem file ourselves.

In order to be able to run the linked binary I did this locally:

brew install s2n openssl
export LD_LIBRARY_PATH=/opt/homebrew/Cellar/s2n/1.3.56/lib/:/opt/homebrew/Cellar/openssl@1.1/1.1.1u/

❌ This needs to be part of the build (for CI). See https://github.com/armanbilge/scala-native-config-brew for reference.

@joan38 joan38 marked this pull request as ready for review September 19, 2023 15:25
domaspoliakas and others added 10 commits October 9, 2023 11:54
# Conflicts:
#	kubernetes-client/src/com/goyeau/kubernetes/client/KubernetesClient.scala
# Conflicts:
#	kubernetes-client/src/com/goyeau/kubernetes/client/KubernetesClient.scala
# Conflicts:
#	build.sc
#	project/Dependencies.sc
# Conflicts:
#	kubernetes-client/src/com/goyeau/kubernetes/client/KubernetesClient.scala
…e of `PodsApi#execStream`, minor refactoring
# Conflicts:
#	.github/workflows/ci.yml
#	.mill-version
#	build.sc
#	project/Dependencies.sc
@lolgab
Copy link
Author

lolgab commented Nov 25, 2023

What do you think about using ember instead of jdk http client?
Ember is the only one that is supported on all platforms. Otherwise I would need to add platform specific code to use ember on native and js and continue to use jdk client on JVM.
Let me know :)

@lolgab
Copy link
Author

lolgab commented Nov 25, 2023

I just noticed that this repo uses javax for TLS. We need to find a substitute for it to support it on native.

@yurique
Copy link
Collaborator

yurique commented Nov 25, 2023

What do you think about using ember instead of jdk http client? Ember is the only one that is supported on all platforms. Otherwise I would need to add platform specific code to use ember on native and js and continue to use jdk client on JVM. Let me know :)

I was thinking to add a "bring your own Client" constructor somehow. We can leave the JDK client as the default (as it is now) - in the jvm platform. Obviously it won't be available in other platforms - it will have to be Ember instead.

Or we could make Ember the default in all platforms, and add the jdk client as an alternative on jvm.

@joan38 what do you think?

I just noticed that this repo uses javax for TLS. We need to find a substitute for it to support it on native.

Ember is using TLSContext, which seems to be available on all platforms:

https://github.com/typelevel/fs2/blob/main/io/native/src/main/scala/fs2/io/net/tls/TLSContextPlatform.scala
https://github.com/typelevel/fs2/blob/main/io/js/src/main/scala/fs2/io/net/tls/TLSContextPlatform.scala

This should be all we need, right? We just need to create the TSLContext from the config.

@yurique
Copy link
Collaborator

yurique commented Nov 25, 2023

Do we have a problem? :)
https://http4s.org/v0.23/docs/integrations.html

image

@joan38
Copy link
Owner

joan38 commented Nov 25, 2023

Web socket support was the whole reason we went with the jdk client.

@yurique
Copy link
Collaborator

yurique commented Nov 25, 2023

Yeah...
On the brighter side, there's this: http4s/http4s#7261

@yurique
Copy link
Collaborator

yurique commented Nov 25, 2023

and this: http4s/http4s#7196 - there's even a snapshot available

@yurique
Copy link
Collaborator

yurique commented Nov 25, 2023

now, how do I tell mill to look at the sonatype snapshots? :)
or better yet – is there a way to enable metals in the build.sc? (after adding crossplatformity, IDEA is no longer able to import the build at all)

@lolgab
Copy link
Author

lolgab commented Nov 25, 2023

now, how do I tell mill to look at the sonatype snapshots? :)

def repositoriesTask = T.task {
  super.repositoriesTask() ++ Seq(coursier.Repositories.sonatype("snapshots"), coursier.Repositories.sonatypeS01("snapshots"))
}

@yurique
Copy link
Collaborator

yurique commented Nov 25, 2023

trying it with that snapshot version, got it to compile, but js linking fails:

kubernetes-client[2.13.10].js.test.fastLinkJSTest There were linking errors

now I need to tell mill to give me more details

@lolgab
Copy link
Author

lolgab commented Nov 25, 2023

Feel free to push more commits to this branch of course :)

@yurique
Copy link
Collaborator

yurique commented Nov 25, 2023

Feel free to push more commits to this branch of course :)

Will do! :) I was thinking if I should branch off, but if you're not actively working on it today I'd rather push here.

@joan38
Copy link
Owner

joan38 commented Nov 27, 2023

Sounds like we need to replace all the T.ctx by T.ctx().
Not sure why the command is not working locally for you

@yurique
Copy link
Collaborator

yurique commented Nov 27, 2023

Sounds like we need to replace all the T.ctx by T.ctx(). Not sure why the command is not working locally for you

Ha, I'm blind. Those were just warnings. The action fails because of the same error I'm getting locally:

Cannot resolve all. Try `mill resolve _` to see what's available.

It must have changed in mill 0.11

@yurique
Copy link
Collaborator

yurique commented Nov 27, 2023

@lolgab can we have something like src-js-native in addition to src-js and src-native?

@yurique
Copy link
Collaborator

yurique commented Nov 27, 2023

for anyone interested: ./mill all __.checkStyle __.docJar (0.10.x) --> ./mill __.checkStyle + __.docJar (0.11.x)

@yurique
Copy link
Collaborator

yurique commented Nov 28, 2023

I changed the CI job to matrix. Is this too much now? :)
https://github.com/joan38/kubernetes-client/actions/runs/7012375769

@lolgab
Copy link
Author

lolgab commented Nov 28, 2023

I changed the CI job to matrix. Is this too much now? :) https://github.com/joan38/kubernetes-client/actions/runs/7012375769

I would expand the whole matrix only in the latest Kubernetes version and then have a job with jvm - 2.13 (or 3) for the other k8s versions.

@lolgab
Copy link
Author

lolgab commented Nov 28, 2023

@lolgab can we have something like src-js-native in addition to src-js and src-native?

Out of the box it's not supported I think. I have it in my mill-crossplatform plugin but it's not worth it to add here.
You can manually add the sources section to js and native:

def sources = super.sources() ++ Seq(millSourcePath / "src-js-native")

@yurique
Copy link
Collaborator

yurique commented Nov 28, 2023

def sources = super.sources() ++ Seq(millSourcePath / "src-js-native")

That's what I was looking for, thanks! :)

Though now I'm realizing that the ember client builder in all three private object PlatformSpecific is identical, so I could move it into the object KubernetesClient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Question: Scala Native build
4 participants