JetBrains / markdown

Markdown parser written in kotlin
Apache License 2.0
682 stars 75 forks source link

Add Kotlin Native targets #59

Closed ajalt closed 3 years ago

ajalt commented 3 years ago

This PR adds support for Linux, MacOS, Windows, and iOS native platforms.

Most of the native-specific code deals with URL resolution and percent encoding. Since K/N doesn't include any of this functionality in the standard library, I implemented the WHATWG URL specification with minor modifications to allow relative urls without an explicit host. Deviations from the WHATWG spec are called out explicitly with comments.

I restricted all the new K/N code to only the subset of POSIX that is supported on all platforms. It's located in an intermediate source set named nativeMain/nativeTest which each target depends on.

Note that IntelliJ doesn't currently understand intermediate source sets, so it won't resolve references to platform.posix etc. even though gradle can build correctly. Some projects add some hacks to their build scripts to trick IntelliJ into treating the intermediate source set as whichever target you're running on. I didn't add any hack like that to this PR, but I can do so if you'd like.

I updated the GH Actions workflow to run tests on Linux, Windows and MacOS systems in order to test all build targets.

Unfortunately, the way this project loads test resources is not compatible with running iOS tests on the simulator, so I have the iOS target disabled right now. There may be a way to copy test resources on to the simulator before tests run, but I don't have a mac, so I can't try it out.

I think the best fix would be to stop loading test data from the file system. Most tests that do so right now could be converted into regular unit tests (e.g. links.md could be a separate unit test for each line in the file). Large integration tests like gitBook.md could be removed to disabled. That approach would also allow us to run unit tests for JS in the browser. If you'd like to go that route, I can convert #54 to embed the spec data directly into tests and remove the json resources.

saket commented 3 years ago

This is great @ajalt. Having a multiplatform markdown parser is going to be very sweet!

valich commented 3 years ago

Hey, thanks for the contribution and sorry for the delay. LGTM, I think we should merge this first and then rework the tests to enable ios testing. I believe we can leave some file-based testing at least for performance tests, which we can run on specific architectures only. Also, I running macOS native tests was failing for me until I've installed XCode (not cmdline tools), but I believe that's expected, according to the kotlin issue tracker.

A couple of questions: 1) Why not enable ios artifact w/o tests? 2) How do you plan to depend on this library natively? I believe we'll need to set up some binaries publication as well?

ajalt commented 3 years ago

Awesome, I'm looking forward to using this!

  1. Because without tests, we can't confirm that the ios target is working correctly, and I didn't want to publish a potentially broken target.
  2. I'm going to depend on this library with normal Gradle Kotlin MPP dependencies. That means you'll have to build and publish this various targets to the same maven repo. For my KMP projects, I use GitHub Actions to publishing to Maven Central from windows, mac, and linux runners. If you want to do the same, I can set up the actions for you. If you're using something else like Team City, I can't offer much advice.
valich commented 3 years ago

I just noticed PR did not update publications so I thought that might have been intentional. I am in the process of transition from bintray to Maven Central; I hope I'll be able to set up publishing of respective targets myself, but will ask for help if I fail, thanks!

ajalt commented 3 years ago

Here's the publishing action I use for one of my projects.

I use a Windows runner to publish the mingw target, and a MacOS runner to publish everything else. MacOS can cross compile the linux targets, but you could also publish those on a separate linux runner. Once all targets are finished uploading, I close and release all the maven artifacts at once.