-
Notifications
You must be signed in to change notification settings - Fork 349
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
Refactor Podfile DSL #81
Conversation
The current direction is to support the following syntax: workspace "MyWorkspace" do
project "MyProject" do
target "Tests" do
build_configuration "Development" do
pod 'Specta', '~> 0.2.1'
end
end
end
end |
Q1 Currently the workspace attribute accepts a name, should we switch to only the name?Which one of the following syntaxes do we want to support? A1.1project "MyProject" do
end No support for paths, name only. A1.2project "path/to/MyProject" do
end Support for paths, no extension. A1.3project "path/to/MyProject.xcodeproj" do
end Support for paths with extension. |
Q2 Is there any interest in supporting build configurations at the project and at the workspace level?The question is wether the following makes any sense? workspace "MyWorkspace" do
project "MyProject" do
build_configuration "Debug" do
pod 'Tweaks'
end
target 'Name' do
end
target 'Name2' do
end
end
end In this scenario all the targets of the projects will have |
@orta Thanks man, I need it! |
Is there a big problem with supporting all those options? (Although maybe A3 could be dropped.)
Yes. |
No, just double checking that it made sense. |
@kylef is using clever solution to define common dependencies among targets: https://github.com/kylef/KFData/blob/master/Tests/Podfile#L5-9. I'm hence wondering if a proper DSL attribute is desirable. Regarding the inheritance of the headers only (useful for bundled targets: i.e. tests), a dedicated attribute is definitely needed. |
Regarding the new syntax, are we all in phase with the following meaning of the different scopes?
The biggest question is for the case of
I personally prefer solution 1, namely force even newcomers to CP to explicitly declare on which target the pods must be added, and totally dropping the "implicit target" rule. It seems we agree on this (according to #1243) but I just wanted things to be clear on this new syntax and the fact that we totally drop the implicit thingy. |
Regarding inheritance, the question is still open: The solution of @kylef is clever, but a lot of CP users don't know a thing about Ruby.
💡 Maybe, instead of relying on inheritance (and introducing the abstract targets concept and all), a solution could be to be able to declare an block for an array of targets instead of just one? For example, imagine we have a project with 2 app targets (say one "Lite" version and one "Pro" version of our app) and a 3rd target being the Unit Tests targets. Then we could have:
Of course, same idea applies to If we add some read-only attributes to the
|
I want to highlight a point as a consequence of the discussion generated by CocoaPods/CocoaPods#840 (comment). If the workspace and the project can be implicit. The following means link every target of the user project with this dependency:
This is something which I really like, but is a different behaviour which could break existing setups (if the project had two targets, the second one would not have linked with the previous ones). I also agree with the point of @AliSoftware (CocoaPods/CocoaPods#840 (comment)) that the need of DSL attribute is mooted by the possibility of specifying dependencies at the project and at the workspace level. |
Re @AliSoftware (btw thanks for the feedback! Very appreciated) I don't like the solution of specifying an array of targets, and I think that it could be confusing and suboptimal in the long run. /c @alloy |
Actually the idea of the array of targets was inspired by the dependency declaration of the The typical example that I have in mind is some project of mine that is declined in multiple app targets (a white-labelled product with 5 or 6 variants, one per customer, for example) and one test target. Of course I will have common pods which must be added to all my app targets, but not in the test target. And I want to avoid repeating all the pods in each of those 5 or 6 app targets.
For this kind of case — which is fairly common (when you have mulitple app targets, you generally link most of the pods with all of those targets), here are the solutions I imagine Solution 1 : allow
|
Have to admit I didn't understand your point/idea here? |
Premise: I'm not sure of how typical this case could be with up to 6 targets sharing the same reps (I think that usually is 2 and rarely 3) My comment was about this proposal: project 'MyProject' do
pod 'A'
pod 'B'
test_target 'Tests' do
# This target is equivalent to the current `:exclusive => true`.
# In other words has only the headers of pod A and pod B visible
# but it doesn't link against them.
end
end In the above all the targets app1-5 will link against the pods A & B. Note that your proposal |
Agreed about your solution to use But let's say then that half of the pods are common to all 5 targets and the other half only common to some of them… what then? Note: I edited my previous comment to throw some more complexity in my example use case ("Pro Apps" for 2 of the 5 targets), to add to the debate about how the new DSL could address such case. I agree that this may seem a complex and far-fetched example, but the aim here is to see if the future DSL can handle such complex examples, and to experience/stress-test this new DSL 😉 |
Xcode can do the distinction, so I'm sure we could find a way for the |
What bother me with the |
I see your point, however there another consideration to take into account... |
Can you elaborate why you could possibly need headers to pods you don't link against? Why would you need to access, from your test target, to header of pods that are only in the app target? If the pod is not linked against the target, you won't be able to call it anyway, so what is the point of having the headers/API of the pod there? (Except if you tell Xcode to use your app to host your test bundle ("General" tab of the test target > dropdown menu "Target" in the right pane > choose "YourApp" instead of "None") and thus the libraries will be linked against the app thus code could be called from the tests, but in case the app is the host for the test target, Xcode will build the app as an implicit dependency first and the headers will be in the Products Directory anyway when building the test target then… so we would be able to import them from the tests, right?) Or am I missing something? |
By the way, your last comments imply that future For example I have many projects where a lot of common pods are used both in the app and in my unit tests. For example if I use For all those cases I don't want my test target to be "exclusive", on the contrary I want it to include the same pods (and possibly more) than my app target, so I would rely on the inheritance mechanism of the new DSL, declaring my pods at the higher level (project level) in my Sure if I really want my test target to inherit pods from the higher level I would be able to use
I believe that the main problem here is the vocabulary: the term chosen qualifies what type of target it is (test target vs. app target) but the effect it has is on how the target handles inheritance (inherit parent scope or not) which is not semantically related. Frankly I really rather prefer keeping the PS: here are some 🍻 to help you go thru all that reading I give to you… sorry for my very long messages and the headaches I may have given you 😄 |
Which is the default behaviour of Xcode and not an exceptional case.
In this case thus we should avoid double linking. Moreover access to the headers of the dependencies might be used for stubbing or setting up the test environment in other forms.
I'm not sure what you are referring to, but my understanding is that the headers search paths must be set in the xcconfig of the test target, otherwise the build will fail. |
We should support test targets hosted in another target and not. I agree that the nomenclature
🍻 I appreciate some good feedback! |
Actually I just realized that it seams that CocoaPods does not correctly export the All the details about correct Static Libraries configuration are explained here in this Apple TechNote. As you can see, Apple discourage the usage of the "Copy Headers" phase and suggest using "Copy Files" phase with a subpath of As a consequence of this, every library that copy/exports its headers this way make those headers available to other dependent targets, as the headers are copied in the "Products Directory" and are then automatically accessible using |
Whoa I wasn't aware of that... Thanks man! This works very well the only drawback is that you need to specify the import like the following:
I have created a sample proj on https://github.com/irrationalfab/CocoaPodsCore81 @alloy what is your take? |
Cool glad I taught you something interesting :) Actually the name to use in the My dream would be that when Apple & LLVM finally officialize their support for |
Nice! I'm of the opinion that we should switch to this behaviour... @alloy I would like your confirmation because I recall that we experimented with the |
@irrationalfab I’m confused. Last time I checked we don’t actually do anything with headers, we just set the search paths to @AliSoftware / @irrationalfab In the past we did copy and I experiment with both types of phases. At that time we concluded ‘copy headers’ is broken and we switched to ‘copy files’. |
@alloy We create the symlinks and set the header search paths. However with @AliSoftware approach we could skip the creation of the symlinks and the set of the search paths in the xcconfig. However I recall experimenting as well and bing bitten by them, but maybe was an error on my side. My conclusion is that we should experiment again if we don't recall the reason why they were not working... and if there is one document it. |
@AliSoftware I toyed around with modules as soon as they where announced however I think that we should not rely on them as they are not officially supported and provide limited benefit respect the risk. However if things change I think that we should switch to them asap. |
@irrationalfab The ticket is too noisy for me to focus on just that. Can you please point me to where this specifically is discussed?
No it was definitely not an error on our side, somebody specifically asked an Apple engineer and I documented it in the Xcodeproj API that was used to add source files (and which would add headers to the copy phase). Having said that, we should most definitely re-check such issues after each big Xcode release :) |
@alloy I recall some similar discussion... the @AliSoftware suggestion is detailed here #81 (comment) |
I totally agree and was not suggesting to migrate to them — as they are not ready to be used yet for 3rdP libs — but just pointing out that one of the numerous advantages of following Apple's recommandation of using the "Copy Files" phase instead of "Copy Headers" is that it will also be compatible with the Being able to use the same syntax (whether it is |
@irrationalfab I still don’t see it. We don’t use any of the Xcode ‘copy’ phases for the headers, because we don’t copy them at all. Instead we symlink them during the CP install process. So it’s still completely unclear to me what issue that we have this would solve, which means it might not be an efficient thing to spend time on. |
@alloy Here is what we are talking about: |
@AliSoftware Thanks! So we are using the copy phase after all. I think we should simply keep doing what we do atm (set the header search path to |
The copy phase is not needed and can be removed if needed iirc (is there just to properly setup the target). The benefit of @AliSoftware approach is that we would delegate to Xcode the responsibility of making headers visible... in detail the advantages are:
|
+1 for abstract targets. It should do more than the |
Closing due to extreme age. |
This is a living comment
TODO
workspace
and thexcodeproj
.Questions
Related issues
Discussion
Blocked issues