New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: Enable BUILD_LIBRARY_FOR_DISTRIBUTION for Release configuration #1110
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ 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
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
@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. 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). |
@ffried |
@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 @ffried, @sushichop, as an alternative and if you think that it will cost problems, let's try to have a separate branch where |
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).
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. 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: 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! 🙂 |
@dive Heck, let's just create that branch and see where it leads. 🙃 Here's the branch: binary_frameworks Please let us know how it works out. 🙂 |
Yap. Any feedback is always welcome🙂 |
@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
@ffried, cannot agree more. But this is the current situation, and I believe not many developers care about it in real life.
@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.
@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. |
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
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... 🙃
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).
Looking forward to it. 🙂 |
I've marked this PR as |
@ffried, we try every two or three months :) But this topic is complicated, at least for us. These are the main issues with have:
So, this is a complex combination of Xcode & |
@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 |
You are welcome! For me, It is always interesting to hear how it works in other companies too.
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.
I hope you got a nice bonus for this 😃. It saved you a lot of time and money.
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. |
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: Imagine if there was a file (let's call it But I'm just a dreamer dreaming of a world with usable and clever tools... 😉 |
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). 👍 |
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. |
Small update about the current state. We released applications with
But, there is one important note: I will update the conversation when I have more news. |
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 Let's wait for Swift 6... |
Sorry about that @dive. We'll need to wait then |
@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. |
Hello there, Long time no see! :) I have intermediate good news, I was able to build CocoaLumberjack with 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! |
Hi, @bpoplauschi! Sorry for the delayed answer. Somehow I missed the message. And thank you for the invitation! |
@dive sounds good, just let me know if / when this changes, so I can send you an invite, as you are welcome :) |
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:
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? |
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: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 withBUILD_LIBRARY_FOR_DISTRIBUTION = YES
option. In this pull request, theBUILD_LIBRARY_FOR_DISTRIBUTION
option is enabled for theRelease
configuration.This is the year when we blame Apple for missing documentation and this is the case. In general, this option does the following:
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