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

Generate targets with same build settings as Xcode (related to #164) #166

Merged
merged 57 commits into from
Nov 20, 2014

Conversation

mrackwitz
Copy link
Member

With the new version of the toolchain and Xcode, there were introduced new target types (iOS Framework) and some new build settings (i.e. if the target uses Swift.). We have to ensure that the generated targets build settings match those Xcode will create. This PR is related to #164.

Therefore I created a sample project, with a target for each type, dumped their build settings to xcconfig files, built an integration spec, and changed the constants so that the build settings will match the parsed xcconfig files.

Currently there will fail some specs, where we might need to discuss, how Xcodeproj should behave.

ToDos

For easier code review I opened a PR on my fork for these points.

  • Dump build settings in a Xcode version specific directory
  • Deserialize xcconfigs on demand for ProjectHelper::common_build_settings instead of manually decompose them in the Constants
  • Move dumped build settings in another directory, because they will be used for more then being spec fixtures
  • Edit the gemspec to preserve the files in the packaged gem
  • Allow to specify the Xcode version as an argument for ProjectHelper::common_build_settings, use the current given by default
  • Update for Xcode 6.1
  • Let handle CocoaPods the build setting DNS_BLOCK_ASSERTIONS=1 for Release configuration, if needed, as it is not included by Xcode by default anymore and so should not be included in targets created with Xcodeproj
  • Add an entry to the CHANGELOG.md

@fabiopelosin
Copy link
Member

👍 Terrific work!

@fabiopelosin
Copy link
Member

Note: the build on Ruby 2.0 is succeeding even though some test failed because an issue of the simpelcov gem which is fixed in master (related to e5a69ed). The build on 1.8.7 has been erring because the installation of Ruby is taking too much time.

@mrackwitz
Copy link
Member Author

Here is a short list of all failing tests:

Xcodeproj::Project
  Initialization from scratch
    - it should have DNS_BLOCK_ASSERTIONS=1 flag in Release configuration [FAILED]

Xcodeproj::Project::Object::AbstractTarget
  Helpers
    #add_build_configuration
      - configures new build configurations according to the given type [FAILED]

Both fail because DNS_BLOCK_ASSERTIONS is not part of the build settings created by Xcode and will not be included in the generated settings. It's afaik not a default build setting, but still makes sense, but this is probably out of scope of Xcodeproj's purpose. What should we do here?


Xcodeproj::Project::Object::AbstractTarget
  Helpers
    - returns the deployment target specified in its build configuration [FAILED]

Expects that IPHONEOS_DEPLOYMENT_TARGET equals 4.3, if no other value was given. This was listed in the constants settings, but there is also a special handling in common_build_settings. Do we really need both? If so, where should we implement a fallback value?


Xcodeproj::Project::ProjectHelper
  ::common_build_settings
    - returns the build settings for an application by default [FAILED]

My changes to common_build_settings broke that. Do we want to keep this behavior for backwards compatibility?

@fabiopelosin
Copy link
Member

My take:

  1. We should closely match what Xcode does, if any setting makes sense for CocoaPods, it should handled there.
  2. I think that that setting is there for a legacy CocoaPods related reason and thus it can go away and we should match whatever is the Xcode default.
  3. I don't think so.

We need also to check that those settings work fine on Xcode 5 as long as we will support it. Otherwise we could have a temporary compatibility section.

It would be also nice to document and automate to the possible extent the process of updating the settings to a new version of Xcode.

@fabiopelosin
Copy link
Member

Please document all the changes (especially the breaking ones) on the Changelog.

@fabiopelosin
Copy link
Member

Regarding 1, we should ensure that a proper solution is implemented in CocoaPods before the release of this patch.

@fabiopelosin
Copy link
Member

@fabiopelosin
Copy link
Member

@mrackwitz I think that xcodeproj should keep track of the different build settings of each Xcode version. In that way it will be possible to support multiple versions of Xcode at the same time. Is this something that you want to do, or do you prefer me to take over this patch?

@mrackwitz
Copy link
Member Author

I think that xcodeproj should keep track of the different build settings of each Xcode version. In that way it will be possible to support multiple versions of Xcode at the same time. Is this something that you want to do, or do you prefer me to take over this patch?

Let's discuss how we would do it first.
I see the following requirements:

  • We need to keep track of the build settings for each Xcode version.
  • ProjectHelper::common_build_settings needs a new parameter to specify the Xcode version. Default should be probably the last known. It should return the matching build settings.
  • The last used Xcode version to modify a project has to be passed to common_build_settings by calls from Xcodeproj itself where applicable, i.e. for AbstractTarget#add_build_configuration

Furthermore I think, the build settings could been managed in a better separated / maintainable manner than the current constants module as they are subject to change and will probably bloat the code.

If only those build settings are valid outputs, which you can generate with Xcode itself, then the simpler way would be to just deserialze the dumped xcconfigs on-demand. But this is the less comprehensible way, as you can see now how the settings are composed.


In the end there is the question why we need to keep track of different versions at all? As soon as Apple releases a new version, they will deprecate the previous one. We could just suppport always the latest version, and developers who need to stay on an older Xcode version could still use an older gem version. I know, that there will be the challenge of organizing versions and dependencies. But let's make this point as transparent as possible.

@mrackwitz
Copy link
Member Author

It would be also nice to document and automate to the possible extent the process of updating the settings to a new version of Xcode.

The "current" process would enfold to manually create targets. This is error-prone. But I don't see so far a way to automate this process with AppleScript/Automator/…, which would be maintainable.

Has anyone helpful ideas regarding this point?

@fabiopelosin
Copy link
Member

I see the following requirements: ...

👍

If only those build settings are valid outputs, which you can generate with Xcode itself, then the simpler way would be to just deserialze the dumped xcconfigs on-demand. But this is the less comprehensible way, as you can see now how the settings are composed.

I was thinking about storing them in a JSON/xcconfig file and to add an Xcodeproj subcommand to output all the build settings per build configuration of a project.

In the end there is the question why we need to keep track of different versions at all?

This would allow to provide support for new versions of Xcode as soon as they are released and it automated to the possible extent is not such a maintenance burden in my opinion

The "current" process would enfold to manually create targets. This is error-prone. But I don't see so far a way to automate this process with AppleScript/Automator/…, which would be maintainable.

Manual creation of the reference project is the only possibility that I can see. The serialisation can be automated though.

@mrackwitz
Copy link
Member Author

I was thinking about storing them in a JSON/xcconfig file and to add an Xcodeproj subcommand to output all the build settings per build configuration of a project.

Manual creation of the reference project is the only possibility that I can see. The serialisation can be automated though.

I had already built an yet unreleased tool on top of Xcodeproj, which does the serialization. But as this thing is very feature-bloated and further requirements here introduce unneccessary debt, I contributed instead some of the logic as CLAide Subcommand to xcodeproj.

To continue those efforts to automate to the possible extent, I added some rake tasks:

  • rake common_build_settings:new_project deletes the fixture project and re-creates by Xcodeproj a new empty project in place of it, and finally opens it.
  • rake common_build_settings:targets guides through the manual creation of the targets.
  • rake common_build_settings:dump calls the new xcodeproj config-dump with correct arguments to dump the build settings in the fixture directory.
  • rake common_build_settings:rebuild does all this steps in this order.

@fabiopelosin
Copy link
Member

This is terrific work.

@fabiopelosin
Copy link
Member

@mrackwitz can we split this PR in smaller parts to get the bulk of it merged?

@segiddins
Copy link
Member

We can’t automatically merge this pull request.

I tried to rebase but I'm not familiar enough with the changes to have gotten it right. /c @fabiopelosin, @mrackwitz

@segiddins segiddins added and removed d2:moderate labels Aug 24, 2014
@mrackwitz
Copy link
Member Author

@fabiopelosin @segiddins @kylef: I've already begun to work on the subtasks. I hope that I will have some time this week to finish it and then merge the changes from master back.
The only thing, I can imagine, to break it down, would be if Apple surprises us with the release of the GM.

@fabiopelosin
Copy link
Member

@alloy I suggested to provide support for multiple Xcode versions because it would aide the transition during RCs and because it appeared easy to implement. This is regardless wether we decide to expose the selection of the Xcode version in CocoaPods.

@fabiopelosin
Copy link
Member

Related: CocoaPods/Core#152

@kylef
Copy link
Contributor

kylef commented Sep 12, 2014

Sorry, this is going to need rebasing too.

@fabiopelosin
Copy link
Member

This patch looks good to me... I think that the work of ext_build_settings_multiple should be discussed in a separated PR

Will be passed from new_target via configuration_list to common_build_settings
Conflicts:
	lib/xcodeproj/project/project_helper.rb
Removed because it is not supported anymore: Xcodeproj should generate build settings, which are the same as those from Xcode.
We only catch StandardError. `confirm` allows to escape with a SystemExit.
mrackwitz added a commit to mrackwitz/Xcodeproj that referenced this pull request Nov 20, 2014
@@ -1,5 +1,44 @@
# Xcodeproj Changelog

## 0.21.0
Copy link
Member

Choose a reason for hiding this comment

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

Master


targets.each do |name, attributes|
begin
sh "printf '#{name}' | pbcopy"
Copy link
Member

Choose a reason for hiding this comment

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

?

Copy link
Member

Choose a reason for hiding this comment

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

NSA IS WATCHING!

Copy link
Member Author

Choose a reason for hiding this comment

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

see above:

+ subtitle "Each target name will been copied to your clipboard."
+ confirm "Make sure you have nothing unsaved there"

Because this whole process is not completely automated and requires user interaction.


# Match further common settings by key sets
keys = [type, platform, target_product_type, language].compact
key_combinations = (1..keys.length).map { |n| keys.combination(n).to_a }.reduce([], :+)
Copy link
Member

Choose a reason for hiding this comment

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

Wow, so intens :D Can this be simplified for casual reading improvement?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, flat_map would help.

@alloy
Copy link
Member

alloy commented Nov 20, 2014

Looks good to go to me 👍

desc "Create a new empty project"
task :new_project => [:prepare] do
verbose false
require 'xcodeproj'
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably use Bundler.require instead. Otherwise if the Rakefile isn't ran with bundler we will potentially be running the wrong Xcodeproj.

Most of the other tasks in our Rakefile's protect against this, such as running bacon via sh "bundle exec bacon #{args.spec_file}".

kylef added a commit that referenced this pull request Nov 20, 2014
Generate targets with same build settings as Xcode (related to #164)
@kylef kylef merged commit 768a228 into CocoaPods:master Nov 20, 2014
@kylef
Copy link
Contributor

kylef commented Nov 20, 2014

:shipit:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants