Skip to content
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

Bump Tests project to Xcode 13.2. #1288

Merged
merged 2 commits into from Feb 16, 2022

Conversation

jcbertin
Copy link
Contributor

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

This PR fixes warnings for Xcode 13.2.

@codecov
Copy link

codecov bot commented Feb 11, 2022

Codecov Report

Merging #1288 (402b4b7) into master (cff9d3e) will increase coverage by 0.15%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1288      +/-   ##
==========================================
+ Coverage   48.26%   48.42%   +0.15%     
==========================================
  Files          32       32              
  Lines        5200     5212      +12     
  Branches      488      489       +1     
==========================================
+ Hits         2510     2524      +14     
+ Misses       2501     2499       -2     
  Partials      189      189              
Impacted Files Coverage Δ
Sources/CocoaLumberjack/DDFileLogger.m 52.47% <0.00%> (-0.15%) ⬇️
Sources/CocoaLumberjack/DDLog.m 57.50% <0.00%> (+0.31%) ⬆️
Tests/CocoaLumberjackTests/DDFileLoggerTests.m 97.97% <0.00%> (+2.61%) ⬆️

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 cff9d3e...402b4b7. Read the comment docs.

};
name = Debug;
};
432B532B1AAE40EB00843E69 /* Release */ = {
isa = XCBuildConfiguration;
baseConfigurationReference = 435F03B22174AB9600A86B2D /* Module-Release.xcconfig */;
buildSettings = {
IPHONEOS_DEPLOYMENT_TARGET = 12.0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't add build settings to the project file. They are all in xcconfig files.

Tests/Tests.xcodeproj/project.pbxproj Outdated Show resolved Hide resolved
@lolgear
Copy link
Contributor

lolgear commented Feb 11, 2022

Why do we care of Tests.xcodeproj and why it still exists?
The question is wider.
Why do we still use xcodeproj?
If we have to ( and we must ) add support for new Xcodes, well, why do we need to support very old Xcodes?
Does anybody check that this project still compiles in Xcode 11? Xcode 10?
I mean that IF we are moving in this way ( add support for many new features ), why should we care about very outdated format xcodeproj?
We have a good alternative SPM manifest file Package.swift.

This PR shows that we have to keep xcconfigs, because, actually, Tests and Integration stuff are not connected ( but should ).
So, can we just rid whole project of xcodeproj as much as possible?
Move Integration stuff into Tests directory and run tests and integration tests by xcodebuild and SPM.

We should also remove support of third-party dependencies managers ( because nobody doesn't care about them and keeping their support is not a as good as it seems ). Keeping support of these managers means blocking people from new technologies ( and a way that vendor sees dependencies problem solving ).

@ffried
Copy link
Member

ffried commented Feb 14, 2022

Why do we care of Tests.xcodeproj and why it still exists? The question is wider. Why do we still use xcodeproj? If we have to ( and we must ) add support for new Xcodes, well, why do we need to support very old Xcodes? Does anybody check that this project still compiles in Xcode 11? Xcode 10? I mean that IF we are moving in this way ( add support for many new features ), why should we care about very outdated format xcodeproj? We have a good alternative SPM manifest file Package.swift.

This PR shows that we have to keep xcconfigs, because, actually, Tests and Integration stuff are not connected ( but should ). So, can we just rid whole project of xcodeproj as much as possible? Move Integration stuff into Tests directory and run tests and integration tests by xcodebuild and SPM.

We should also remove support of third-party dependencies managers ( because nobody doesn't care about them and keeping their support is not a as good as it seems ). Keeping support of these managers means blocking people from new technologies ( and a way that vendor sees dependencies problem solving ).

Simple answer: because this is not a (pure) Swift project.
There are enough usages out there in apps that use Objective-C. And while Swift (and SPM) is the future, there will always be projects that are very happy if their dependencies aren't all shut down or moved away to some technology they can't use. And this is sometimes also a business decision. Personally, I've worked in projects where management didn't want to spend the money porting everything to Swift and integrating Swift into the current setup would have been quite difficult.

@lolgear
Copy link
Contributor

lolgear commented Feb 14, 2022

@ffried
SPM is not a Swift-only solution.
It is a different format for delivering frameworks. Ok, libraries. Ok, whatever they call it.
You can't deliver a product to new OSes without new Xcode.
Well, you can, but you have to add SDK manually.
The last problem is integration of SPM into a project. Actually, you have to replace all imports of umbrella header with modern module imports. Modules are not only simple, but they are preventing bugs. And this is what LLVM forces you to do. You have to enable modules for C and ObjC.

But about this PR - it doesn't make sense to keep Tests.xcodeproj at all. It can be covered by SPM easily.
And all these integration tests... It also doesn't make sense to deliver CocoaLumberjack as framework, it is better to force people to integrate SPM as soon as possible or to use static library. It is utility, not a functional library, it can be integrated into other libraries/frameworks.

Actually, we don't need CocoaLumberjack as framework.
All binary frameworks use CocoaLumberjack as static library or as SPM static library.
All new projects use it as SPM module.
All legacy projects have to migrate to SPM module ( because they have to support new OSes and they have to use new Xcode ).

@ffried
Copy link
Member

ffried commented Feb 14, 2022

@lolgear

SPM is not a Swift-only solution.

I never said SPM was Swift-only (although it is heavily Swift-focused). I just said CocoaLumberjack is not.

The last problem is integration of SPM into a project. Actually, you have to replace all imports of umbrella header with modern module imports. Modules are not only simple, but they are preventing bugs. And this is what LLVM forces you to do. You have to enable modules for C and ObjC.

It also requires an "open" internet connection on your build servers and currently there is no easy way to mirror dependencies once there is an Xcode project with SPM dependencies. Again, similar to OSLog in #1289, SPM might be a fully featured replacement in the future, but it isn't yet. Not for all kinds of projects.

But about this PR - it doesn't make sense to keep Tests.xcodeproj at all. It can be covered by SPM easily.

Like I said, SPM manages some build settings that we manage ourselves in xcconfigs. This is fine for SPM, but we still do (and need to) support other ways of distribution which I'd like to have tested. We run our tests with SPM and Xcode btw. So both ways are tested.

All binary frameworks use CocoaLumberjack as static library or as SPM static library.

They do not: #1278

All new projects use it as SPM module.

Have any proof of that? Our issues show, that people are still using CocoaLumberjack without SPM - and I doubt they do that without having a reason for it.

All legacy projects have to migrate to SPM module ( because they have to support new OSes and they have to use new Xcode ).

Like I said above, once SPM is a fully featured replacement, this is a viable change - because we and our users don't loose anything. By then, other dependency managers might even be able to integrate with SPM as well, thus allowing us to drop Xcode projects.


Don't get me wrong, I'm a big fan of SPM. And I wouldn't dream of writing a new library using anything else but SPM - but I've written more scripts and code to work around the (current) limitations of SPM than I prefer. Especially when it comes to SPM in Xcode.
Let's think about this again, when we can build fully featured apps without needing Xcode projects. 😉

@lolgear
Copy link
Contributor

lolgear commented Feb 14, 2022

If your internet is not open, actually, you have two possibilities:

  1. Use vendor solution.
  2. fork OSS project to your own organization.

Second approach is less preferable, because you should maintain it.

"Have any proof of that? Our issues show, that people are still using CocoaLumberjack without SPM - and I doubt they do that without having a reason for it."

It is a fault of marketing. Reorder/emphasize SPM as the most preferable solution - and that is it, people will stop complain about frameworks at all.
Replace occurrences of umbrella headers with module imports - voila! people will stop complain about frameworks.

In my organization we use OSes forks and also heavily remove all frameworks. So, nothing special, but we have nearly a decade of development with all legacy stuff and all outdated practices in ObjC/Swift. And yes, project is a mix of ObjC/ObjC++/C/C++/Swift. ( With python, perl, ruby, bash in build phases/build scripts ).

All binary frameworks use CocoaLumberjack as static library or as SPM static library.
They do not: #1278

Hm, actually, I only observe misunderstanding of what SPM manifest file is and what products it delivers/distributes.
It is a mess of what CocoaPods does for us. ( But, of course, it is a fault of Apple? ). All umbrella headers and all these dependencies graphs with linkage between frameworks - no-no-no, we have to stop this madness.

My point is to not only rethink a current approach, but also force people not to use outdated stuff. ( Again, reorder sections/dependencies managers in Readme, emphasize that SPM is the best option ).
I once asked Apple about frameworks ( don't remember my problem, misunderstanding of its layout ) why this thing doesn't work. And Apple engineers answered that this layout is outdated, please, don't rely on it. It was 6 years ago I guess.
Maybe it is a bell that they are doing something new/modern to replace outdated frameworks and to solve problems with linkage and umbrella frameworks.

@lolgear
Copy link
Contributor

lolgear commented Feb 14, 2022

Ok, my point is that all OSS projects are open and their codebases are open, so, it means that you can't hide any source from people. That means that these OSS projects are distributed as static libraries. ( Nothing to hide ).
Previous madness with frameworks IS because of lack of a good linker or a good tool which properly links all binaries ( libraries ).
So, instead of supporting all outdated technologies and also tell people that "we are happy to be your valentine in crime", we can show a different way to them ( and start with Readme ).
I also don't understand why CocoaPods still ships their stuff without mention ( in Readme ) that SPM is the best option for dependencies. As an OSS project they have to deliver light. But they do opposite.

And all these "dynamic/static" stuff is not what I want to solve. It is, actually, a business problem ( xcframeworks vs SPM ).
One thing that you have to solve is xcframework with third-party oss dependencies ( sounds as oxymoron ).

@ffried
Copy link
Member

ffried commented Feb 14, 2022

If your internet is not open, actually, you have two possibilities:

  • Use vendor solution.
  • fork OSS project to your own organization.

Or possibility number 3: Make it work.
In our case (and likely the StackOverflow case as well), the "vendor" is simply a corporation that provides restricted build servers for their projects to make sure they control what sources are drawn into their projects. There is no "vendor solution", they don't provide one. There are usually ways to get URLs on an allow-list, but the process is usually more work than to hack your way around it by shipping a complete package. Now to do that with SPM, you need to mirror URLs, deliver local packages etc. Things that work quite fine with SPM itself, but not (yet) as soon as Xcode gets involved.

It is a fault of marketing. Reorder/emphasize SPM as the most preferable solution - and that is it, people will stop complain about frameworks at all.

That's something we can totally consider! But this is different from dropping support for everything else altogether!

Replace occurrences of umbrella headers with module imports - voila! people will stop complain about frameworks.

Umbrella headers are still there. They're just generated... 😉
Also, module imports aren't perfect either. And they add certain overhead - which is a problem Swift hasn't solved yet either.
But I agree that they are in most cases a better alternative.

I once asked Apple about frameworks ( don't remember my problem, misunderstanding of its layout ) why this thing doesn't work. And Apple engineers answered that this layout is outdated, please, don't rely on it. It was 6 years ago I guess.

It depends on what exact layout you mean here. I'd call it an implementation detail - which is something you should never rely on. The new XCFrameworks still use it internally - you just don't get to see it usually.

Maybe it is a bell that they are doing something new/modern to replace outdated frameworks and to solve problems with linkage and umbrella frameworks.

They did. SPM is on its way to solve this problem. At least for open source frameworks.

That means that these OSS projects are distributed as static libraries.

OSS != static libraries. There is a valid case for OSS libs to be dynamically linked - especially widely used libs.

I also don't understand why CocoaPods still ships their stuff without mention ( in Readme ) that SPM is the best option for dependencies. As an OSS project they have to deliver light. But they do opposite.

One big reason is that it can manage OSS and binary libs just the same - something SPM just recently learned and is still learning. Also (multi-platform) XCFrameworks are still quite buggy.

And all these "dynamic/static" stuff is not what I want to solve. It is, actually, a business problem ( xcframeworks vs SPM ).

This is a problem that first needs solving from Apple's side. There is also no clever equivalent of XCFrameworks on Linux. Once we can write all kinds of libs (OSS / closed source, dynamic / static) for all kinds of platforms (Darwin, Linux, ...) using Swift, then we see what we need to support to make CocoaLumberjack work with this as well (as good as possible).


Either way - let's draw the line here, we're far off the topic of this pull request. 🙂
I like the idea of moving the SPM section up in the README! But we should be careful with "discouraging" other ways before we're absolutely sure we're not shooting ourselves in the end. 😉
Especially because just because we don't like "other ways", doesn't mean they didn't gain experience that might eventually be useful...

@lolgear
Copy link
Contributor

lolgear commented Feb 14, 2022

@ffried
Yeah, let's draw a line :)
One last thing. I don't said I don't like other solutions. I said that they are, well, developed to support outdated stuff ( like frameworks-frameworks dependencies ). It is not valid anymore. You have a better ( and native ) alternative that do everything statically for you.
And it is a shame that OSS dependency manager doesn't respect its commercial ( yeah, commercial ) brother as the best option for many cases.
And SPM... And umbrella headers also a pain in it, yes.
Well, it is so broken ( was so broken ) in Xcode 12.6.1 that many things took a lot more time to solve than to use. But it is still the best alternative for a corporation that use private repos and checks all third-party dependencies for vulnerabilities.

So, let's consider Readme changes.

@ffried ffried merged commit a110fd5 into CocoaLumberjack:master Feb 16, 2022
@jcbertin jcbertin deleted the tests-xcode-13.2 branch February 16, 2022 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants