WIP: Enable BUILD_LIBRARY_FOR_DISTRIBUTION for Release configuration

[dive]

New Pull Request Checklist

• I have read and understood the CONTRIBUTING guide

• I have read the Documentation

• I have searched for a similar pull request in the project and found none

• I have updated this branch with the latest master to avoid conflicts (via merge from master or rebase)

• I have added the required tests to prove the fix/feature I am adding

• I have updated the documentation (if necessary)

• I have run the tests and they pass

• I have run the lint and it passes (pod lib lint)

Pull Request Description

Hello there,

The problem

Right now, if you will try to reuse CocoaLumberjack built with Swift 5.1 Toolchain with Xcode 11.2 (11.2.1GM) you get an error:

./Badoo/Platform/Foundation/source/logging/BPFLog.swift:6:8: 
    Module compiled with Swift 5.1 cannot be imported by the Swift 5.1.2 compiler: 
    ./Badoo/ThirdParty/Carthage/Build/iOS/CocoaLumberjackSwift.framework/Modules/CocoaLumberjackSwift.swiftmodule/x86_64-apple-ios-simulator.swiftmodule

Solution

Swift 5.1 is here with Module Stability support, and to be able to reuse CocoaLumberjack binaries between Swift Toolchains >= 5.1 we have to build it with BUILD_LIBRARY_FOR_DISTRIBUTION = YES option. In this pull request, the BUILD_LIBRARY_FOR_DISTRIBUTION option is enabled for the Release configuration.

This is the year when we blame Apple for missing documentation and this is the case. In general, this option does the following:

Ensures that your libraries are built for distribution. For Swift, this enables support for library evolution and generation of a module interface file.

If you want to know more, check "Binary Frameworks in Swift" session from WWDC'19.

We tested the solution, and it works just fine. You can build CocoaLumberjack with Swift 5.1 Toolchain and it will work/link for both Xcode 11.1-11.2 (aka, Swift 5.1 and Swift 5.1.1).

Cheers,
Artem | Badoo

[codecov-io]

Codecov Report

Merging #1110 into master will decrease coverage by 0.62%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1110      +/-   ##
==========================================
- Coverage   37.54%   36.92%   -0.63%     
==========================================
  Files          23       23              
  Lines        3870     3870              
==========================================
- Hits         1453     1429      -24     
- Misses       2417     2441      +24

Impacted Files	Coverage Δ
Sources/CocoaLumberjack/DDOSLogger.m	20.83% <0%> (-50%)	⬇️
Tests/CocoaLumberjackTests/DDAtomicCounterTests.m	98.14% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f92141d...b5bd812. Read the comment docs.

[sushichop]

@dive
Thanks for your pull request. swiftinterface has come!
Then, IMHO, BUILD_LIBRARY_FOR_DISTRIBUTION is valid not only for release but also for debug configuration.
So we can write BUILD_LIBRARY_FOR_DISTRIBUTION = YES to Configs/Module-Shared.xcconfig🙂

Thanks.

[ffried]

@sushichop @dive I'd be careful with this setting. There is a lot that can go wrong when enabling this setting. I think in one of the WWDC Talks (or release notes?) Apple recommends it mainly for binary distributed frameworks (such as the new xcframeworks). For libraries that are open source and ship the source code, it introduces overhead that is not really necessary.
This is mainly because, apart from watching out for source compatibility, we would then also have to check binary compatibility of each new version.

E.g. it becomes difficult to determine what version of the framework is actually used (since Xcode / Carthage / [...] might use a cached binary instead of re-building the sources). Then we might also miss out on fixes in the Swift compiler, because (like in this case) the source code is not re-compiled but instead uses the pre-compiled version (created by an older version of Swift).

So IMHO we should not use this setting (for now).

[sushichop]

@ffried
Hmm..., you have a point there🙂

[dive]

@ffried, I agree, the setting has some side effects. And yes, it is mainly for binary distributions (that's the case for us, we use Carthage). But I am not quite sure about any overheads for cases when the library is used via the source-code. In the case of the source-code distributions, you are more or less safe because the Swift Toolchain is always aligned between dependencies and your code-base (interfaces are stable, etc.).

But, for the users of the binaries, this is important because we do not have to rebuild CocoaLumberjack every time we migrate our internal code-base to the new Xcode version or change the toolchain. The most complicated scenario, for example, is to support two Xcode/Toolchain versions at the same time.
Swift Module Stability is not as stable as we want it to see of course (SR-11704, SR-11466), but I think it is important to support this feature for end-users of this beautiful framework.

@ffried, @sushichop, as an alternative and if you think that it will cost problems, let's try to have a separate branch where BUILD_LIBRARY_FOR_DISTRIBUTION is on for distribution configurations and mention it in the documentation. At least, we will have a chance to test it and gather feedback.

[ffried]

@dive

And yes, it is mainly for binary distributions (that's the case for us, we use Carthage).

In this case, Carthage is the one serving binaries - not us. Also, Carthage would be in the position to add this build setting before building here (maybe with an optional fallback in case the build fails with this setting).

But I am not quite sure about any overheads for cases when the library is used via the source-code. In the case of the source-code distributions, you are more or less safe because the Swift Toolchain is always aligned between dependencies and your code-base (interfaces are stable, etc.).

I'm not sure I understand your point here. If we enable this setting, we give up control over what compiler was used for a given binary. Like in your case, it might be a 5.1 compiler or a 5.1.2 compiler or a 5.1.1 compiler. We simply don't know. We also give up quite a few optimizations.
Also, we actually cannot guarantee that our library is used via source-code.

IMHO, the problem here lies more in the fact, that Carthage is already "faking" binaries. In one of the bigger (closed source) projects I'm working, we also used Carthage in the beginning (we then switched to our own dependency management, but that's a different story). But we never let Carthage build any binaries. This way, supporting multiple Xcode/Toolchains is very easy. Since Xcode anyways rebuilds the dependencies, we never ran into any problems.

Adding the fact, that module stability, as you mentioned yourself, is not quite stable yet (from a compiler perspective), we should even more be careful with this setting.

Your idea of creating a branch where this setting is enabled could work for now, but adds the burden of keeping this branch up-to-date. But I could also live with just creating that branch for now and seeing where it leads. 🙃

In general, I think we should look at Apple's strategy here:
They have introduced two methods of integrating 3rd party frameworks in Xcode this year: Swift Package Manager and Binary Frameworks. The former is for open source projects and the latter for closed source projects. So from my point of view, SPM is what we primarily need to support and we added that support not long ago.

Again, Carthage could simply inject that build setting before building and support this for their own purposes - with all the benefits and downsides this setting has.

Also: That's just my opinion on this topic. If the team here decides differently, that's ok as well! 🙂

[ffried]

@dive Heck, let's just create that branch and see where it leads. 🙃
As long as no one requires it to be up-to-date (with master) at all times, this should be fine. I'll see if I can rebase it from time to time.

Here's the branch: binary_frameworks

Please let us know how it works out. 🙂

[sushichop]

@dive Heck, let's just create that branch and see where it leads. 🙃

Yap. Any feedback is always welcome🙂

[dive]

I'm not sure I understand your point here. If we enable this setting, we give up control over what compiler was used for a given binary. Like in your case, it might be a 5.1 compiler or a 5.1.2 compiler or a 5.1.1 compiler. We simply don't know. We also give up quite a few optimizations.
Also, we actually cannot guarantee that our library is used via source-code.

@ffried, yeah, I think we are talking about two different things. I was talking about the fact that there are not much differences and side-effects when you build CocoaLumberjack together with the application you are going to ship.

IMHO, the problem here lies more in the fact, that Carthage is already "faking" binaries...

@ffried, cannot agree more. But this is the current situation, and I believe not many developers care about it in real life.

But we never let Carthage build any binaries. This way, supporting multiple Xcode/Toolchains is very easy. Since Xcode anyways rebuilds the dependencies, we never ran into any problems.

@ffried, this is exactly what we are trying to avoid. Do not rebuild 3rd party binaries during development flows and continuous integration. For big applications, this is an unnecessary overhead.

@dive Heck, let's just create that branch and see where it leads. 🙃
As long as no one requires it to be up-to-date (with master) at all times, this should be fine. I'll see if I can rebase it from time to time.

@ffried, @sushichop, thanks! We are going to switch to the branch and test in the production environment. I will keep you posted about results here, in the comments. If you are curious, we are going to switch Bumble & Badoo applications. They are quite big, so, I hope, the first results will be available very soon.

[ffried]

@dive

I was talking about the fact that there are not much differences and side-effects when you build CocoaLumberjack together with the application you are going to ship.

That was exactly my point. You don't know that this is the case. In your case, Carthage decides whether this is the case or not. E.g. it might actually not re-build CocoaLumberjack when you update to a new version of Xcode.

But this is the current situation, and I believe not many developers care about it in real life.

Unfortunately they don't, yea. Many of them probably have no idea what's even happening under the hood. But that's a different story... 🙃

Do not rebuild 3rd party binaries during development flows and continuous integration. For big applications, this is an unnecessary overhead.

Is it, though? Xcode has become pretty good at deciding whether or not to re-build a certain dependency. At least that's my experience. In our medium-sized (almost pure-swift) 130kLOC project, we have clean builds that take around 7-10mins and incremental builds that take around 10-20s. Our OpenSource dependencies (including CocoaLumberjack) are basically never re-built in incremental builds (unless the environment changes, like with new Xcode versions).
Just out of curiosity: Have you tried letting Xcode decide whether or not to re-build the dependencies? If so, how big's the difference?

thanks! We are going to switch to the branch and test in the production environment. I will keep you posted about results here, in the comments. If you are curious, we are going to switch Bumble & Badoo applications. They are quite big, so, I hope, the first results will be available very soon.

Looking forward to it. 🙂

[ffried]

I've marked this PR as WIP and added the necessary labels so this won't be marked as stale.

[dive]

Do not rebuild 3rd party binaries during development flows and continuous integration. For big applications, this is an unnecessary overhead.

Is it, though? Xcode has become pretty good at deciding whether or not to re-build a certain dependency. At least that's my experience. In our medium-sized (almost pure-swift) 130kLOC project, we have clean builds that take around 7-10mins and incremental builds that take around 10-20s. Our OpenSource dependencies (including CocoaLumberjack) are basically never re-built in incremental builds (unless the environment changes, like with new Xcode versions).
Just out of curiosity: Have you tried letting Xcode decide whether or not to re-build the dependencies? If so, how big's the difference?

@ffried, we try every two or three months :) But this topic is complicated, at least for us. These are the main issues with have:

• We have a lot of 3rd party dependencies and some of them are distributed only as binaries. For us, it is a convenient way to manage all dependencies at once via the same flow;
• We use wholemodule compiler optimisation everywhere internally because incremental option still does not play well with mixed code-base (Swift + Objective C) and bridging headers. It is not directly related to the topic, but we have to execute clean builds even during development because sometimes we have strange issues with the module cache and cached artefacts. So, clean builds are unavoidable, and we do not want to multiply the clean compilation time by 3-5x due to 3rd parties;
• On CI we use clean builds only, even for unit-tests targets. We try to reuse Derived Data time-to-time and always have problems with flakiness and instabilities. This is too pricy for us to have flaky CI.

So, this is a complex combination of Xcode & xcbuild & Swift Toolchain issues, our code-base, legacy stuff, mistakes made in the past, priorities and weather, but clean builds are part of our flow. At least for now. But, in general, I think there is nothing wrong with using 3rd party modules as binaries (except for the issues with Carthage behaviour you mentioned above, but this is a different story).

[ffried]

@dive Thanks a lot for the insights here! Very interesting to hear about the experiences in other larger scale projects!

On our CI, we also do clean builds every time. But I don't care if the CI takes a few minutes longer. It's a powerful machine that can handle a few parallel builds and I don't need to wait for it.

As for the mixed code-base: this is why we separated our code. All of our frameworks (both 3rd party and our own) are either built with only Swift or only Objective-C (or some other form of C 😛). Hearing your experiences, it seems like this saves us from a lot of trouble 🙂

Again, I never spoke against binary 3rd party dependencies. I just don't like the thought of one dependency being distributed as both, binary and source. If some tool transforms source into binary, that's another thing, but then that tool should be responsible for creating the most reusable binary possible. So maybe you might also open an issue against Carthage and propose that they inject the BUILD_LIBRARY_FOR_DISTRIBUTION build setting when they build their binaries...?

[dive]

@ffried,

@dive Thanks a lot for the insights here! Very interesting to hear about the experiences in other larger scale projects!

You are welcome! For me, It is always interesting to hear how it works in other companies too.

On our CI, we also do clean builds every time. But I don't care if the CI takes a few minutes longer. It's a powerful machine that can handle a few parallel builds, and I don't need to wait for it.

It depends on the volume of jobs you have on CI and the size of your team. When you have around 100 jobs to integrate a branch or run all checks multiplied by 30-50 developers that deliver releases every week for 5 applications, then "a few minutes" become "a few hours", etc. So, yeah, there is no silver bullet, all these requirements are specific to the problem you are trying to solve.

As for the mixed code-base: this is why we separated our code. All of our frameworks (both 3rd party and our own) are either built with only Swift or only Objective-C (or some other form of C 😛). Hearing your experiences, it seems like this saves us from a lot of trouble 🙂.

I hope you got a nice bonus for this 😃. It saved you a lot of time and money.

So maybe you might also open an issue against Carthage and propose that they inject the BUILD_LIBRARY_FOR_DISTRIBUTION build setting when they build their binaries...?

I do not want to do it because this is precisely my general complaint about Carthage - they implicitly change build rules without any options to prevent it or be informed about it. There is a nice discussion in the issue I mentioned above about Carthage philosophy.
Also, I think that owners of frameworks have to be aware of such changes. Otherwise, we will have situations when something does not work due to BUILD_LIBRARY_FOR_DISTRIBUTION propagated by Carthage, but the owner is not aware. It would be hard to solve such issues or even convince the owner to take a look (you know, when you have that feeling that you own the code but, in the real-life, some 3rd party tools make unsupported changes you have to fix to).

[ffried]

@dive

So maybe you might also open an issue against Carthage and propose that they inject the BUILD_LIBRARY_FOR_DISTRIBUTION build setting when they build their binaries...?

I do not want to do it because this is precisely my general complaint about Carthage - they implicitly change build rules without any options to prevent it or be informed about it. There is a nice discussion in the issue I mentioned above about Carthage philosophy.
Also, I think that owners of frameworks have to be aware of such changes. Otherwise, we will have situations when something does not work due to BUILD_LIBRARY_FOR_DISTRIBUTION propagated by Carthage, but the owner is not aware. It would be hard to solve such issues or even convince the owner to take a look (you know, when you have that feeling that you own the code but, in the real-life, some 3rd party tools make unsupported changes you have to fix to).

In general, I agree here. But IMHO tools should be allowed to make adjustments to make things work. Cocoapods does so, Carthage does and even SPM does that. The problem with Carthage is, that they chose a different approach than the other two tools:
While Cocoapods and SPM provide defaults that can be overridden / adjusted by a framework owner and/or people who integrate the framework, Carthage started off as a "zero configuration" tool (which is a cool idea!) and then noticed, that they still had to make changes. And instead of introducing a way to configure these changes, they simply baked them into the tool.

Imagine if there was a file (let's call it Cartconf), where we could adjust how Carthage treats our project. In the current case, there could be an opt-out option for BUILD_LIBRARY_FOR_DISTRIBUTION where owners could permanently disable the injection of this flag by Carthage.

But I'm just a dreamer dreaming of a world with usable and clever tools... 😉

[bpoplauschi]

Guys, great talk here. I only read it diagonally, but I do like how you've approached it (dedicated branch vs having the setting in our releases). 👍

[bpoplauschi]

PS: we can wait and see how other open source frameworks handle this. I did a quick search (https://github.com/search?q=BUILD_LIBRARY_FOR_DISTRIBUTION&type=Code) but didn't see any open source having this on.

[dive]

Small update about the current state. We released applications with CocoaLumberjack built from binary_framework branch. There are no crashes, misbehaviours or any other issues so far. To make the picture more precise, this is how we use CocoaLumberjack during development and on production:

• General: we link CocoaLumberjack as a binary dependency built with Carthage; DDFileLogger is always turned on for all configurations;
• Development: we use DDOSLogger by default with the option to log to the file. Did not notice any problems during development, collecting log on CI or from test devices;
• Production: we attach the last log in the rotation to our crash-reports as additional information. No regressions or differences with previous versions.

But, there is one important note: CocoaLumberjack was built with Apple Swift version 5.1 (swiftlang-1100.0.270.13 clang-1100.0.33.7) and we use the same Swift Toolchain to build our applications for distribution.
So, the next step is to switch our applications to the Apple Swift version 5.1.2 (swiftlang-1100.0.278 clang-1100.0.33.9) toolchain but keep the same binary for CocoaLumberjack. After this, we will be able to check the "module stability" feature between toolchains, and, perhaps, this is a proper test for the technology.

I will update the conversation when I have more news.

[dive]

Hello there,

Quick update.

I think we can postpone this PR for a long time. Just tested with Xcode 11.4 and you still cannot import frameworks with swiftinterface (produced by BUILD_LIBRARY_FOR_DISTRIBUTION) that were built with Swift 5.1 Toolchain.

Let's wait for Swift 6...

[bpoplauschi]

Sorry about that @dive. We'll need to wait then

[bpoplauschi]

@dive Even if this PR couldn't get merged (yet), I think you could definitely bring value to our project, so we would like to invite you to become a maintainer – no pressure to accept! You can pitch in with what seems comfortable: comment on open issues/PRs, triage, improve documentation, write your own PRs. Let me know if you are interested.

[dive]

Hello there,

Long time no see! :) I have intermediate good news, I was able to build CocoaLumberjack with BUILD_LIBRARY_FOR_DISTRIBUTION option from the branch and re-use it with both Xcode 11.7 (Swift 5.2) and Xcode 12 (Swift 5.3) with iOS support >= 12. The framework was built with Xcode 11.7 (Swift 5.2 Toolchain).
So far, all our tests are passing, release/debug and other configurations work as expected.

I will update the message when Bumble application will be released with CocoaLumberjack in this configuration.

P.S. Thank you, @ffried, for keeping the branch up to date!

[dive]

@dive Even if this PR couldn't get merged (yet), I think you could definitely bring value to our project, so we would like to invite you to become a maintainer – no pressure to accept! You can pitch in with what seems comfortable: comment on open issues/PRs, triage, improve documentation, write your own PRs. Let me know if you are interested.

Hi, @bpoplauschi! Sorry for the delayed answer. Somehow I missed the message. And thank you for the invitation!
To be honest, I doubt that I will be able to devote any reasonable time to such activities. Anyway, I have switched to the "Be notified of all conversations" mode for the project, let's check what comes of it.

[bpoplauschi]

@dive sounds good, just let me know if / when this changes, so I can send you an invite, as you are welcome :)

[XabierGoros]

Hi guys,

I'm not quite sure if this issue is related to the following one, but just in case, talking about library evolution itself I'm trying to compile my project using Xcode 13 beta 3 and I get the following error:

❌ /.../DerivedData/SourcePackages/checkouts/CocoaLumberjack/Sources/CocoaLumberjackSwift/DDLog+Combine.swift:16:5: failed to build module 'Combine'; this SDK is not supported by the compiler (the SDK is built with 'Apple Swift version 5.5 (swiftlang-1300.0.24.14 clang-1300.0.25.10)', while this compiler is 'Apple Swift version 5.5 (swiftlang-1300.0.24.13 clang-1300.0.25.10)'). Please select a toolchain which matches the SDK.
#if canImport(Combine)

Since I'm consuming the source code of the project through SwiftPM, I'm confused of why I might be getting it. I've even tested (although pointless for this case) with the binary_frameworks branch with the same result. Does it mean that Apple theirselves have the same issue with the Combine library?