#73 Scala Native Curl based backend #105

pcejrowski · 2018-06-04T20:28:52Z

#73 This is Curl based backend for Scala Native.

adamw · 2018-06-05T07:44:40Z

Looks really promising, thanks for your work!
I just removed the dependency on URLDecoder yesterday, this should be helpful here.

Do I understand correctly that no tests are runnable currently? If so, maybe instead of moving them, the native subproject could override the test sbt settings, remove the dependency on scalatest and switch off the tests (at least until scalatest-native is there)?

pcejrowski · 2018-06-05T17:54:58Z

@adamw Thanks! There was one more dependency on URLDecoder left -- I'm removing it in #109 .
I will try following the tests' setup pattern from coreJS.

pcejrowski · 2018-06-06T20:04:23Z

I have started using Curl based encoder (89a393e) but I'm still having one test failing:

[info] - host www.mikołak.net should serialize to http://www.xn--mikoak-6db.net *** FAILED ***
[info]   "http://www.[miko%C5%82ak.ne]" was not equal to "http://www.[xn--mikoak-6db.net]" (UriTests.scala:104)
[info]   Analysis:
[info]   "http://www.[miko%C5%82ak.ne]" -> "http://www.[xn--mikoak-6db.net]"

but when I run curl -v 'www.mikołak.net' it also says: * IDN support not present, can't parse Unicode domains.
I'm green in those topics, so any help is welcome.

…Linking

adamw · 2018-06-07T11:43:27Z

Ah see, curl is yet another backend which doesn't support IDN domains ;)

But I suppose if the IDN Java class is not available, there are only two possibilities: (1) copy its functionality into our codebase (2) live with the fact that - at least for now - IDN domains won't work :)

pcejrowski · 2018-06-07T18:52:22Z

I have just added test of the backend, but because of the Native scalatest status: [error] (Nativetest / executeTests) The test program never connected to sbt. even though the response is read by Curl (visible in Curl's log), so we might get stuck here.
I'll try copying IDN.toASCII into Sttp's codebase to close the previous issue.

At this moment, the main feature left is request body support.

guymers · 2018-06-22T23:19:10Z

Would it be easier to use libidn instead of copying IDN.toASCII?

Maybe the test problem is caused by native ScalaTest not handling tests that return futures? Does uTest (only testing library that seems to have a released native version) have the same problem?

pcejrowski · 2018-06-23T20:32:27Z

@guymers - I've just added IDN based on libidn. Thanks for the suggestion.

pcejrowski · 2018-06-24T13:31:21Z

I have just added significant part of tests that I was able to transfer from shared HttpTest. I changed AsyncFreeSpec into FreeSpec and it works. All tests that are commented out are failing for now. I plan to work on them in the next days.
It still requires some clean up of test classes, since some of them are currently redundant.

pcejrowski · 2018-07-18T19:55:23Z

I have just bumped Scala Native version to 3.8 and removed encoding based on Curl, since URLEncoder is added in SN3.8.

@densh Thanks for the explanation. I'll experiment around async tests.

adamw · 2018-07-18T20:18:55Z

@pcejrowski I wanted to convert the tests to a synchronous version, however this won't work with scala.js, as it's single-threaded and the only way to do something in parallel there is to work with Futures. So the tests need to be based on AsyncFreeSpec.

So if it turns out it's not possible to use AsyncFreeSpec with scala-native, let's just go with the copied tests approach. Better this than nothing :)

pcejrowski · 2018-07-18T20:52:19Z

@adamw yeah, I have been playing around AsyncFreeSpec that explicitly waits for the block passed into def in(f: Future[Assertion]), but haven't got any progress.

adamw · 2018-07-19T06:02:31Z

@pcejrowski let me know how it goes - I can either merge the PR as is, or wait for the results of your trials :)

adamw · 2018-07-19T06:05:01Z

build.sbt

@@ -29,6 +29,13 @@ val commonJSSettings = Seq(
  }
 ) ++ browserTestSettings

+val commonNativeSettings = commonSmlBuildSettings ++ ossPublishSettings ++ Seq(
+  organization := "com.softwaremill.sttp",
+  scalaVersion := "2.11.12",


So scala native is based on 2.11? What about 2.12?

There is no 2.12 support.
2.12 support is tracked here: scala-native/scala-native#233

adamw · 2018-07-19T06:08:21Z

core/native/src/test/scala/com/softwaremill/sttp/testing/SyncHttpTest.scala

+
+import scala.concurrent.duration._
+
+trait SyncHttpTest


If this test stays as-is (copy-pasted), maybe you could add a comment explaining why it's copied and how to add how the tests cases differ from HttpTest

pcejrowski · 2018-07-19T17:59:34Z

@adamw
I have just added the comment you requested, but I see no hope for some sort of AsyncFreeSpec to be ready soon.
If you are ok with that, please merge it as-is.

adamw · 2018-07-20T16:05:04Z

@pcejrowski do you maybe know why I'm getting

[error] /Users/adamw/projects/sttp/core/native/target/scala-2.11/native/lib/optional/re2.cpp:61:10: fatal error: 're2/re2.h' file not found
[error] #include <re2/re2.h>

when trying to run the tests locally? (macos)

adamw · 2018-07-20T16:08:30Z

brew install re2 helped :)

adamw · 2018-07-20T16:16:25Z

@pcejrowski though now I've got another one :) I installed

$ brew install llvm
$ brew install bdw-gc re2 # optional

as specified on http://www.scala-native.org/en/latest/user/setup.html, but now I get:

[error] ld: library not found for -lidn
[error] clang: error: linker command failed with exit code 1 (use -v to see invocation)
[info] Linking native code (immix gc) (46 ms)
[info] Starting process '/Users/adamw/projects/sttp/core/native/target/scala-2.11/core-out' on port '63807'.
Exception in thread "Thread-488" java.io.IOException: Cannot run program "/Users/adamw/projects/sttp/core/native/target/scala-2.11/core-out": error=2, No such file or directory
	at java.lang.ProcessBuilder.start(ProcessBuilder.java:1048)
	at scala.sys.process.ProcessBuilderImpl$Simple.run(ProcessBuilderImpl.scala:71)
	at scala.sys.process.ProcessBuilderImpl$AbstractBuilder.run(ProcessBuilderImpl.scala:102)
	at scala.sys.process.ProcessBuilderImpl$AbstractBuilder.$anonfun$runBuffered$1(ProcessBuilderImpl.scala:150)
	at scala.runtime.java8.JFunction0$mcI$sp.apply(JFunction0$mcI$sp.java:12)
	at scala.sys.process.ProcessLogger$$anon$1.buffer(ProcessLogger.scala:99)
	at scala.sys.process.ProcessBuilderImpl$AbstractBuilder.runBuffered(ProcessBuilderImpl.scala:150)
	at scala.sys.process.ProcessBuilderImpl$AbstractBuilder.$bang(ProcessBuilderImpl.scala:116)
	at scala.scalanative.testinterface.ComRunner$$anon$1.run(ComRunner.scala:31)
Caused by: java.io.IOException: error=2, No such file or directory
	at java.lang.UNIXProcess.forkAndExec(Native Method)
	at java.lang.UNIXProcess.<init>(UNIXProcess.java:247)
	at java.lang.ProcessImpl.start(ProcessImpl.java:134)
	at java.lang.ProcessBuilder.start(ProcessBuilder.java:1029)
	... 8 more

Any pointers maybe where to look?

densh · 2018-07-20T16:21:37Z

@adamw

ld: library not found for -lidn

You need to install idn library using homebrew probably.

pcejrowski · 2018-07-20T16:23:49Z

@adamw Congrats on your first contact with Scala Native :)

brew install libidn might help. It's interesting that it worked fine on Travis.

adamw · 2018-07-20T16:38:25Z

@densh Thanks, trying :)
@pcejrowski I don't think the native version is built on travis: https://github.com/softwaremill/sttp/blob/master/.travis.yml#L22

adamw · 2018-07-20T16:39:42Z

@densh @pcejrowski new errors:

[error] Undefined symbols for architecture x86_64:
[error]   "_curl_mime_addpart", referenced from:
[error]       _com.softwaremill.sttp.curl.CurlApi$MimeHandleOps::addPart_ptr in com.softwaremill.sttp.curl.ll.o
[error]   "_curl_mime_data", referenced from:
[error]       _com.softwaremill.sttp.curl.CurlApi$MimePartHandleOps::withData_java.lang.String_i64_scala.scalanative.native.Zone_scala.Enumeration$Value in com.softwaremill.sttp.curl.ll.o
[error]   "_curl_mime_filename", referenced from:
[error]       _com.softwaremill.sttp.curl.CurlApi$MimePartHandleOps::withFileName_java.lang.String_scala.scalanative.native.Zone_scala.Enumeration$Value in com.softwaremill.sttp.curl.ll.o
[error]   "_curl_mime_headers", referenced from:
[error]       _com.softwaremill.sttp.curl.CurlApi$MimePartHandleOps::withHeaders_ptr_i32_scala.Enumeration$Value in com.softwaremill.sttp.curl.ll.o
[error]   "_curl_mime_init", referenced from:
[error]       _com.softwaremill.sttp.curl.CurlApi$CurlHandleOps::mime_ptr in com.softwaremill.sttp.curl.ll.o
[error]   "_curl_mime_name", referenced from:
[error]       _com.softwaremill.sttp.curl.CurlApi$MimePartHandleOps::withName_java.lang.String_scala.scalanative.native.Zone_scala.Enumeration$Value in com.softwaremill.sttp.curl.ll.o
[error]   "_curl_mime_type", referenced from:
[error]       _com.softwaremill.sttp.curl.CurlApi$MimePartHandleOps::withMimeType_java.lang.String_scala.scalanative.native.Zone_scala.Enumeration$Value in com.softwaremill.sttp.curl.ll.o
[error] ld: symbol(s) not found for architecture x86_64
[error] clang: error: linker command failed with exit code 1 (use -v to see invocation)

:)

pcejrowski · 2018-07-20T16:42:31Z

I think you might have an old version of Curl. I have 7.60.0.

pcejrowski · 2018-07-20T16:47:15Z

Curl is required in version 7.56.0+. I'm about to submit a PR with a note on that in docs.

#127

adamw · 2018-07-20T17:26:37Z

Ok, I've got curl 7.61.0, I've updated the PATH so that it's used, but I don't think sbt sees it, as it gives the same error. Anything special I need to set in sbt? Printing the env from within sbt gives the correct PATH.

adamw · 2018-07-20T17:30:31Z

The brew message says that:

For compilers to find this software you may need to set:
    LDFLAGS:  -L/usr/local/opt/curl/lib
    CPPFLAGS: -I/usr/local/opt/curl/include

so maybe I need to pass those flags to the compiler somehow?

adamw · 2018-07-20T18:13:49Z

nativeCompileOptions += "-I/usr/local/opt/curl/include",
    nativeLinkingOptions += "-L/usr/local/opt/curl/lib"

helped :)

adamw · 2018-07-20T19:02:19Z

@pcejrowski I've updated the SyncHttpTest and SyncHttpTestExtensions to include the same tests as in the "normal" versions. Some tests are failing - could you take a look?

I've also updated the README to include instructions on building with the scala-native backend.

adamw · 2018-07-20T19:15:37Z

Because the native backend setup is non-trivial, I've changed the default build not to include the native backend in the aggregate project by default. To include it, the STTP_NATIVE env variable must be defined. All is documented in the readme.

I'll be back on Tuesday :)

pcejrowski · 2018-07-20T20:32:20Z

@adamw got them to work in #129
Enjoy the weekend!

pcejrowski mentioned this pull request Jun 4, 2018

scala-native #73

Closed

pcejrowski changed the title ~~WIP: Scala Native Curl based backend~~ WIP: #73 Scala Native Curl based backend Jun 4, 2018

pcejrowski force-pushed the native-backend branch from 221d035 to 321bbc4 Compare June 5, 2018 17:37

pcejrowski force-pushed the native-backend branch 2 times, most recently from 6d0491d to 89a393e Compare June 6, 2018 19:55

pcejrowski added 10 commits June 6, 2018 22:45

Scala Native Curl based backend

c031777

removing URLDecoder's and fixing after rebase

c7f36d7

scalafmt reformatting

43ec7e5

fixing boxing of Ptr[_]'s and following redirects

64470b5

adding rootNative to rootProject aggregate

09ab2bd

fixing tests in coreNative

32bfb85

update of docs for coreNative

8572060

added implementation of tests for coreNative that fails during native…

fd1a128

…Linking

adding curl native encoding

26b235b

simplification of the AsyncFreeSpec in Scala Native tests

f9fae9c

pcejrowski force-pushed the native-backend branch from 2f4ff8b to f9fae9c Compare June 6, 2018 20:45

adding CurlBackend test and response body parsing

eb17640

removing commonSettings from root project

ca23831

adding IDN based on libidn

0610f7b

pcejrowski force-pushed the native-backend branch from bb1d714 to 0610f7b Compare June 23, 2018 21:21

incomplete tests for curl native backend

ce79400

more tests for native curl backend

4591138

pcejrowski added 4 commits July 18, 2018 21:24

bumping to Scala Native 3.8

d82c19f

switching to java.net.URLEncoder

8278d21

removing unnecessary numeric widenning

6236713

Merge branch 'master' into native-backend

d236e02

wrapping curl's perform rc into response monad

40e9415

adamw reviewed Jul 19, 2018

View reviewed changes

adding a comment on existance of SyncHttpTest

f9de73c

adamw merged commit 4726bdb into softwaremill:master Jul 20, 2018

pcejrowski deleted the native-backend branch July 20, 2018 16:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#73 Scala Native Curl based backend #105

#73 Scala Native Curl based backend #105

pcejrowski commented Jun 4, 2018 •

edited

adamw commented Jun 5, 2018

pcejrowski commented Jun 5, 2018

pcejrowski commented Jun 6, 2018

adamw commented Jun 7, 2018

pcejrowski commented Jun 7, 2018

guymers commented Jun 22, 2018

pcejrowski commented Jun 23, 2018

pcejrowski commented Jun 24, 2018

pcejrowski commented Jul 18, 2018

adamw commented Jul 18, 2018

pcejrowski commented Jul 18, 2018

adamw commented Jul 19, 2018

adamw Jul 19, 2018

pcejrowski Jul 19, 2018

adamw Jul 19, 2018

pcejrowski commented Jul 19, 2018

adamw commented Jul 20, 2018

adamw commented Jul 20, 2018

adamw commented Jul 20, 2018

densh commented Jul 20, 2018

pcejrowski commented Jul 20, 2018

adamw commented Jul 20, 2018

adamw commented Jul 20, 2018

pcejrowski commented Jul 20, 2018

pcejrowski commented Jul 20, 2018 •

edited

adamw commented Jul 20, 2018

adamw commented Jul 20, 2018

adamw commented Jul 20, 2018

adamw commented Jul 20, 2018

adamw commented Jul 20, 2018

pcejrowski commented Jul 20, 2018


		import scala.concurrent.duration._

		trait SyncHttpTest

#73 Scala Native Curl based backend #105

#73 Scala Native Curl based backend #105

Conversation

pcejrowski commented Jun 4, 2018 • edited

adamw commented Jun 5, 2018

pcejrowski commented Jun 5, 2018

pcejrowski commented Jun 6, 2018

adamw commented Jun 7, 2018

pcejrowski commented Jun 7, 2018

guymers commented Jun 22, 2018

pcejrowski commented Jun 23, 2018

pcejrowski commented Jun 24, 2018

pcejrowski commented Jul 18, 2018

adamw commented Jul 18, 2018

pcejrowski commented Jul 18, 2018

adamw commented Jul 19, 2018

adamw Jul 19, 2018

Choose a reason for hiding this comment

pcejrowski Jul 19, 2018

Choose a reason for hiding this comment

adamw Jul 19, 2018

Choose a reason for hiding this comment

pcejrowski commented Jul 19, 2018

adamw commented Jul 20, 2018

adamw commented Jul 20, 2018

adamw commented Jul 20, 2018

densh commented Jul 20, 2018

pcejrowski commented Jul 20, 2018

adamw commented Jul 20, 2018

adamw commented Jul 20, 2018

pcejrowski commented Jul 20, 2018

pcejrowski commented Jul 20, 2018 • edited

adamw commented Jul 20, 2018

adamw commented Jul 20, 2018

adamw commented Jul 20, 2018

adamw commented Jul 20, 2018

adamw commented Jul 20, 2018

pcejrowski commented Jul 20, 2018

pcejrowski commented Jun 4, 2018 •

edited

pcejrowski commented Jul 20, 2018 •

edited