Down icon indicating copy to clipboard operation
Down copied to clipboard

Make Down portable as a built Apple Framework

Open erikkerber opened this issue 2 years ago • 7 comments

Resolves #120

The Down library as-is does not generate a "portable" framework, as described very well in this blog post. The result is that anyone who attempts to build and consume Down as an Apple Framework needs to add the cmark directory from source into the SWIFT_INCLUDE_PATHS

SWIFT_INCLUDE_PATHS = $(inherited) $(PROJECT_DIR)/Carthage/Checkouts/down/Source/cmark

Additionally, having matching module names and types exposes the Down library to this bug: https://bugs.swift.org/browse/SR-14195.

This only manifests when the framework is built for distribution, but it does make building with library evolution/BUILD_LIBRARY_FOR_DISTRIBUTION = YES impossible.

This change does the following:

  • Builds cmark as a clang submodule using a custom modulemap for both Down/cmark
  • Renames the module to "DownLib" from "Down" to avoid SR-14195

The renaming of the module is critical for portability, but could easily be dropped from this PR if that's something that is too aggressive at the moment.

erikkerber avatar Jul 27 '21 17:07 erikkerber

I have an issue when importing this library into my Swift Package. This PR fixes this.

larryonoff avatar Jul 28 '21 15:07 larryonoff

There's some issues with Travis CI that need working out, I'll fix it this weekend.

johnxnguyen avatar Jul 30 '21 07:07 johnxnguyen

I'll make PR today. Maybe there're a bit simpler solution.

larryonoff avatar Jul 30 '21 07:07 larryonoff

Please see https://github.com/johnxnguyen/Down/pull/268

larryonoff avatar Jul 30 '21 10:07 larryonoff

@johnxnguyen any plans to merge this PR? Being able to build a working xcframework would be really helpful!

MarcoEidinger avatar Sep 24 '21 13:09 MarcoEidinger

@johnxnguyen any plans to merge this PR? Being able to build a working xcframework would be really helpful!

I just copied Down to our local repos for the moment. :(

larryonoff avatar Sep 24 '21 13:09 larryonoff

The test target is failing to build, I would like to resolve this first before merging.

johnxnguyen avatar Sep 27 '21 06:09 johnxnguyen

Sorry friends as I did not steward this to completion.

@johnxnguyen I rebased off master to see if the error still reproduced and, if so, actually get a CI result that hasn't been time expired.

If you think this is better off just being closed, that is fine as well. We now consume and build Down via Bazel, so the portable pre-built framework is no longer "needed" by us. Happy to land this if it helps others though.

erikkerber avatar Feb 15 '23 20:02 erikkerber