search

jpsim / Yams

A Sweet and Swifty YAML parser.
https://jpsim.com/Yams
MIT License
1.12k stars 144 forks source link

Yams: make `CYaml` private to Yams #343

Closed compnerd closed 2 years ago

compnerd commented 2 years ago

This changes how CYaml is built and used within Yams. As there are no interfaces from CYaml being directly exposed, we can statically link the library. However, this still would cause a problem as the CYaml import would be serialized into the module requiring that the module is available when building anything which consumes Yams. To avoid that, change the import CYaml instances to @_implementationOnly. This allows for CYaml to not be required at the consumer site.

While in the area of the CMake build system, clean up some of the build system to use language specific options. This is required to avoid the de-duplication of the options across languages and enables linking CYaml statically into the library. Note that on Windows static linking of Swift libraries is not yet properly supported and this does not enable that nor use that - it is statically linking C content.

This now allows building Yams dynamically as a single library target. On Windows, this is a negligible space savings of ~16KiB.

compnerd commented 2 years ago

@jpsim this is likely something which is going to require some uglish workarounds for older versions of Swift. What do you think is the best way forward with such a change?

compnerd commented 2 years ago

@jpsim ping?

jpsim commented 2 years ago

this is likely something which is going to require some uglish workarounds for older versions of Swift

More specifically, looks like this would bump the minimum version of Swift supported when building Yams to Swift 5.4, which was released 9 months ago. That seems like an acceptable amount of time to support older Swift versions to me.

Would you like to handle the removal of Swift < 5.4 CI jobs & code, or shall I?

jpsim commented 2 years ago

Started working on bumping the minimum supported Swift version here: https://github.com/jpsim/Yams/pull/347

compnerd commented 2 years ago

I can take a stab at the codepaths for the older releases (if any) once the CI support is there.

jpsim commented 2 years ago

I can take a stab at the codepaths for the older releases (if any) once the CI support is there.

Not sure what you mean, I don't think there will be code changes needed for Swift 5.4/5.5.

compnerd commented 2 years ago

Not sure what you mean, I don't think there will be code changes needed for Swift 5.4/5.5.

Ah right; there is only a single location (Tests/YamsTest/TopLevelDecoderTests.swift) that has a check for something other than 5.6.

jpsim commented 2 years ago

@compnerd please rebase to get the changes from https://github.com/jpsim/Yams/pull/347 to require Swift 5.4.

jpsim commented 2 years ago

Thanks @compnerd. Do you need me to tag a release?

compnerd commented 2 years ago

That would be great - hopefully then I should be able to get this integrated into the swift-driver build.

jpsim commented 2 years ago

Release tagged: https://github.com/jpsim/Yams/releases/tag/5.0.0