Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

[#212] Check for deployment target #241

Merged
merged 7 commits into from

2 participants

@fabiopelosin

Going throughout pull request because I'm not sure of some points.

lib/cocoapods/specification.rb
((9 lines not shown))
end
- @platform = Platform.new(name, options)
+ @platform = Platform.new(name)
@fabiopelosin Owner

Is there any reason why @platform should receive the options in a specification. The options are currently used to pass the deployment target, Which is a Gem::Version instead in a specification the deployment target is a Gem::Requirement. This allows to specify deployment targets as:

  s.platform = :ios, '>= 5.0'
  s.platform = :ios, '~> 5.0'

Or something like (for discontinued pods):

  s.platform = :ios, ['>= 4.0', '< 6.0']

On the other using the Gem:Version would work assuming that the requirement is always >=.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@alloy
Owner

I’ll check this out tomorrow.

@fabiopelosin
Owner

I'm starting to think that just using the Gem::Version might be the best approach. Xcode treats a deployment target as a >= requirement so it should suffice for pods as well. There are some cases where it does make sense to have more granular requirements (like some libraries that are a shim back-porting new features) however those cases are better left to the programmer.

@alloy
Owner

Oops sorry, I totally forgot to check it. We had queensday on monday, I shouldn't respond to anything on such days :)

I'm starting to think that just using the Gem::Version might be the best approach. Xcode treats a deployment target as a >= requirement so it should suffice for pods as well.

Good point, let's just go with that for now.

fabiopelosin added some commits
@fabiopelosin fabiopelosin Merge branch 'master' into deployment-target-check
* master:
  [Installer] Don't generate documenation if already installed
  [Command::Spec::Linter] Fix - Skip documentation generation
  [Config#doc] Renamed to generate_docs
  [Command::Spec::Linter] Bug fix.
  [# 242 Command::Spec::Linter] Refactoring
  [#246 | Resolver] Validate plaforms against each target.
  [Resolver] Check target plafroms instead of podfile.

Conflicts:
	lib/cocoapods/resolver.rb
50405dc
@fabiopelosin fabiopelosin [Platform] Simplify deployment target specification.
- Allow to specify the deployment target after initialization
43e79bd
@fabiopelosin fabiopelosin [Platform#==] Check for deployment target. b86625e
@fabiopelosin fabiopelosin [Specification#deployment_target=] Switch to Pod::Version 8553b9e
@fabiopelosin fabiopelosin [Resolver] Support for deployment target check. 6c75b65
@fabiopelosin
Owner

Oops sorry, I totally forgot to check it. We had queensday on monday, I shouldn't respond to anything on such days :)

Nothing to be sorry about :-) Queensday looks like a really nice event!

@fabiopelosin fabiopelosin commented on the diff
lib/cocoapods/platform.rb
@@ -8,22 +8,29 @@ def self.osx
new :osx
end
- attr_reader :options
+ attr_reader :options, :deployment_target
@fabiopelosin Owner

I think that the options attribute could be removed because it is not currently used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@fabiopelosin fabiopelosin commented on the diff
lib/cocoapods/platform.rb
((9 lines not shown))
@symbolic_name = symbolic_name
- @options = options
+ if deployment_target
+ version = deployment_target.is_a?(Hash) ? deployment_target[:deployment_target] : deployment_target # backwards compatibility from 0.6
+ @deployment_target = Pod::Version.create(version)
+ end
end
@fabiopelosin Owner

With this change also in the podfile the platform can be defined as

platform :ios, '5.0'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@fabiopelosin fabiopelosin commented on the diff
lib/cocoapods/specification.rb
((13 lines not shown))
end
attr_reader :platform
+ def platforms
+ @platform.nil? ? @define_for_platforms.map { |platfrom| Platform.new(platfrom, @deployment_target[platfrom]) } : [platform]
+ end
+
@fabiopelosin Owner

I added this method to avoid clients from being aware that if the platform is nil they should look for :ios and :osx.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@fabiopelosin fabiopelosin was assigned
@alloy
Owner

Looks all good to me. Were there any specific questions you still wanted answered that I might have missed?

Btw, regarding setting up the feeds server, I will try to get to that tomorrow, it’s been a hectic week :)

@fabiopelosin
Owner

Looks all good to me. Were there any specific questions you still wanted answered that I might have missed?

Cool, I'm merging.

Btw, regarding setting up the feeds server, I will try to get to that tomorrow

Great :-)

it’s been a hectic week :)

Indeed...

@fabiopelosin fabiopelosin merged commit fdcbd63 into master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 30, 2012
  1. @fabiopelosin
Commits on May 3, 2012
  1. @fabiopelosin

    Merge branch 'master' into deployment-target-check

    fabiopelosin authored
    * master:
      [Installer] Don't generate documenation if already installed
      [Command::Spec::Linter] Fix - Skip documentation generation
      [Config#doc] Renamed to generate_docs
      [Command::Spec::Linter] Bug fix.
      [# 242 Command::Spec::Linter] Refactoring
      [#246 | Resolver] Validate plaforms against each target.
      [Resolver] Check target plafroms instead of podfile.
    
    Conflicts:
    	lib/cocoapods/resolver.rb
  2. @fabiopelosin

    [Platform] Simplify deployment target specification.

    fabiopelosin authored
    - Allow to specify the deployment target after initialization
  3. @fabiopelosin
  4. @fabiopelosin
  5. @fabiopelosin
  6. @fabiopelosin
This page is out of date. Refresh to see the latest.
View
20 lib/cocoapods/command/spec.rb
@@ -532,16 +532,17 @@ def spec_template(data)
s.description = 'An optional longer description of #{data[:name]}.'
- # If this Pod runs only on iOS or OS X, then specify that with one of
- # these, or none if it runs on both platforms.
- # If the pod runs on both plafroms but presents different deployment
- # targets, source files, etc. create two different pods: `#{data[:name]}-iOS'
- # and `#{data[:name]}-OSX'.
+ # If this Pod runs only on iOS or OS X, then specify the platform and
+ # the deployment target.
#
+ # s.platform = :ios, '5.0'
# s.platform = :ios
- # s.platform = :ios, { :deployment_target => "5.0" }
- # s.platform = :osx
- # s.platform = :osx, { :deployment_target => "10.7" }
+
+ # If this Pod runs on boths platforms, then specify the deployment
+ # targets.
+ #
+ # s.ios.deployment_target = '5.0'
+ # s.osx.deployment_target = '10.7'
# A list of resources included with the Pod. These are copied into the
# target bundle with a build phase script.
@@ -582,13 +583,14 @@ def spec_template(data)
#
# s.dependency 'JSONKit', '~> 1.4'
- # ――― EXTRA VALUES ―――――――――――――――――――――――――――――――――――――――――――――――――――――――――― #
+ # ――― EXTRA VALUES ―――――――――――――――――――――――――――――――――――――――――――――――――――――――――― #
# If you need to specify any other build settings, add them to the
# xcconfig hash.
#
# s.xcconfig = { 'HEADER_SEARCH_PATHS' => '$(SDKROOT)/usr/include/libxml2' }
+ # ――― INFO ―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――― #
end
SPEC
end
View
21 lib/cocoapods/platform.rb
@@ -8,22 +8,29 @@ def self.osx
new :osx
end
- attr_reader :options
+ attr_reader :options, :deployment_target
@fabiopelosin Owner

I think that the options attribute could be removed because it is not currently used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
- def initialize(symbolic_name, options = {})
+ def initialize(symbolic_name, deployment_target = nil)
@symbolic_name = symbolic_name
- @options = options
+ if deployment_target
+ version = deployment_target.is_a?(Hash) ? deployment_target[:deployment_target] : deployment_target # backwards compatibility from 0.6
+ @deployment_target = Pod::Version.create(version)
+ end
end
@fabiopelosin Owner

With this change also in the podfile the platform can be defined as

platform :ios, '5.0'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
def name
@symbolic_name
end
+ def deployment_target= (version)
+ @deployment_target = Pod::Version.create(version)
+ end
+
def ==(other_platform_or_symbolic_name)
if other_platform_or_symbolic_name.is_a?(Symbol)
@symbolic_name == other_platform_or_symbolic_name
else
- self == (other_platform_or_symbolic_name.name)
+ self.name == (other_platform_or_symbolic_name.name) && self.deployment_target == other_platform_or_symbolic_name.deployment_target
end
end
@@ -51,12 +58,6 @@ def nil?
name.nil?
end
- def deployment_target
- if (options && opt = options[:deployment_target])
- Pod::Version.new(opt)
- end
- end
-
def requires_legacy_ios_archs?
return unless deployment_target
(name == :ios) && (deployment_target < Pod::Version.new("4.3"))
View
4 lib/cocoapods/resolver.rb
@@ -84,8 +84,8 @@ def find_dependency_sets(dependent_specification, dependencies, target_definitio
end
def validate_platform!(spec, target)
- unless target.platform.support?(spec.platform)
- raise Informative, "[!] The platform required by the target `#{target.name}' `#{target.platform}' does not match that of #{spec} `#{spec.platform}'".red
+ unless spec.platforms.any? { |platform| target.platform.support?(platform) }
+ raise Informative, "[!] The platform of the target `#{target.name}' (#{target.platform}) is not compatible with `#{spec}' which has a minimun requirement of #{spec.platforms.join(' - ')}.".red
end
end
end
View
21 lib/cocoapods/specification.rb
@@ -34,6 +34,7 @@ def post_initialize
@define_for_platforms = [:osx, :ios]
@clean_paths, @subspecs = [], []
@dependencies, @source_files, @resources = { :ios => [], :osx => [] }, { :ios => [], :osx => [] }, { :ios => [], :osx => [] }
+ @deployment_target = {}
@platform = Platform.new(nil)
@xcconfig = { :ios => Xcodeproj::Config.new, :osx => Xcodeproj::Config.new }
@compiler_flags = { :ios => '', :osx => '' }
@@ -114,17 +115,14 @@ def header_dir
end
def platform=(platform)
- if platform.class == Array
- name = platform[0]
- options = platform[1]
- else
- name = platform
- options = nil
- end
- @platform = Platform.new(name, options)
+ @platform = Platform.new(*platform)
end
attr_reader :platform
+ def platforms
+ @platform.nil? ? @define_for_platforms.map { |platfrom| Platform.new(platfrom, @deployment_target[platfrom]) } : [platform]
+ end
+
@fabiopelosin Owner

I added this method to avoid clients from being aware that if the platform is nil they should look for :ios and :osx.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
def requires_arc=(requires_arc)
self.compiler_flags = '-fobjc-arc' if requires_arc
@requires_arc = requires_arc
@@ -157,7 +155,7 @@ def initialize(specification, platform)
@specification, @platform = specification, platform
end
- %w{ source_files= resource= resources= xcconfig= framework= frameworks= library= libraries= compiler_flags= dependency }.each do |method|
+ %w{ source_files= resource= resources= xcconfig= framework= frameworks= library= libraries= compiler_flags= deployment_target= dependency }.each do |method|
define_method(method) do |args|
@specification._on_platform(@platform) do
@specification.send(method, args)
@@ -181,6 +179,11 @@ def source_files=(patterns)
end
attr_reader :source_files
+ def deployment_target=(version)
+ raise Informative, "The deployment target must be defined per platform like s.ios.deployment_target = '5.0'" unless @define_for_platforms.count == 1
+ @deployment_target[@define_for_platforms.first] = version
+ end
+
def resources=(patterns)
@define_for_platforms.each do |platform|
@resources[platform] = pattern_list(patterns)
View
76 spec/unit/platform_spec.rb
@@ -18,36 +18,44 @@
@platform.should == Pod::Platform.new(:ios)
end
+ it "can be compared for equality with another platform with the same symbolic name and the same deployment target" do
+ @platform.should.not == Pod::Platform.new(:ios, '4.0')
+ Pod::Platform.new(:ios, '4.0').should == Pod::Platform.new(:ios, '4.0')
+ end
+
it "can be compared for equality with a matching symbolic name (backwards compatibility reasons)" do
@platform.should == :ios
end
it "presents an accurate string representation" do
- @platform.to_s.should == "iOS"
+ @platform.to_s.should == "iOS"
Pod::Platform.new(:osx).to_s.should == 'OS X'
Pod::Platform.new(nil).to_s.should == "iOS - OS X"
- Pod::Platform.new(:ios, { :deployment_target => '5.0.0' }).to_s.should == 'iOS 5.0.0'
- Pod::Platform.new(:osx, { :deployment_target => '10.7' }).to_s.should == 'OS X 10.7'
- end
-
- it "correctly indicates if it supports another platfrom" do
- ios4 = Pod::Platform.new(:ios, { :deployment_target => '4.0.0' })
- ios5 = Pod::Platform.new(:ios, { :deployment_target => '5.0.0' })
- ios5.should.support?(ios4)
- ios4.should.not.support?(ios5)
- osx6 = Pod::Platform.new(:osx, { :deployment_target => '10.6' })
- osx7 = Pod::Platform.new(:osx, { :deployment_target => '10.7' })
- osx7.should.support?(osx6)
- osx6.should.not.support?(osx7)
- both = Pod::Platform.new(nil)
- both.should.support?(ios4)
- both.should.support?(osx6)
- both.should.support?(nil)
+ Pod::Platform.new(:ios, '5.0.0').to_s.should == 'iOS 5.0.0'
+ Pod::Platform.new(:osx, '10.7').to_s.should == 'OS X 10.7'
end
it "uses it's name as it's symbold version" do
@platform.to_sym.should == :ios
end
+
+ it "allows to specify the deployment target on initialization" do
+ p = Pod::Platform.new(:ios, '4.0.0')
+ p.deployment_target.should == Pod::Version.new('4.0.0')
+ end
+
+ it "allows to specify the deployment target in a hash on initialization (backwards compatibility from 0.6)" do
+ p = Pod::Platform.new(:ios, { :deployment_target => '4.0.0' })
+ p.deployment_target.should == Pod::Version.new('4.0.0')
+ end
+
+ it "allows to specify the deployment target after initialization" do
+ p = Pod::Platform.new(:ios, '4.0.0')
+ p.deployment_target = '4.0.0'
+ p.deployment_target.should == Pod::Version.new('4.0.0')
+ p.deployment_target = Pod::Version.new('4.0.0')
+ p.deployment_target.should == Pod::Version.new('4.0.0')
+ end
end
describe "Pod::Platform with a nil value" do
@@ -59,3 +67,35 @@
@platform.should.be.nil
end
end
+
+describe "Pod::Platform#support?" do
+ it "supports another platform is with the same operating system" do
+ p1 = Pod::Platform.new(:ios)
+ p2 = Pod::Platform.new(:ios)
+ p1.should.support?(p2)
+
+ p1 = Pod::Platform.new(:osx)
+ p2 = Pod::Platform.new(:osx)
+ p1.should.support?(p2)
+ end
+
+ it "supports a nil platform" do
+ p1 = Pod::Platform.new(:ios)
+ p1.should.support?(nil)
+ end
+
+ it "supports a platform with a lower or equal deployment_target" do
+ p1 = Pod::Platform.new(:ios, '5.0')
+ p2 = Pod::Platform.new(:ios, '4.0')
+ p1.should.support?(p1)
+ p1.should.support?(p2)
+ p2.should.not.support?(p1)
+ end
+
+ it "supports a platform regardless of the deployment_target if one of the two does not specify it" do
+ p1 = Pod::Platform.new(:ios)
+ p2 = Pod::Platform.new(:ios, '4.0')
+ p1.should.support?(p2)
+ p2.should.support?(p1)
+ end
+end
View
22 spec/unit/resolver_spec.rb
@@ -39,7 +39,7 @@
lambda { @resolver.resolve }.should.not.raise
end
- it "raises once any of the dependencies does not match the platform of the root spec (Podfile)" do
+ it "raises once any of the dependencies does not match the platform of its podfile target" do
set = Pod::Spec::Set.new(config.repos_dir + 'master/ASIHTTPRequest')
@resolver.cached_sets['ASIHTTPRequest'] = set
@@ -59,27 +59,15 @@ def set.specification; spec = super; spec.platform = @stubbed_platform; spec; en
lambda { @resolver.resolve }.should.raise Pod::Informative
end
- it "does not raise if all of the dependencies have a deployment target equal or lower of the root spec (Podfile)" do
+ it "raises once any of the dependencies does not have a deployment_target compatible with its podfile target" do
set = Pod::Spec::Set.new(config.repos_dir + 'master/ASIHTTPRequest')
@resolver.cached_sets['ASIHTTPRequest'] = set
+ @podfile.platform :ios, "4.0"
- def set.stub_platform=(platform); @stubbed_platform = platform; end
- def set.specification; spec = super; spec.platform = @stubbed_platform; spec; end
-
- @podfile.platform :ios, { :deployment_target => "4.0.0" }
- set.stub_platform = :ios, { :deployment_target => "4.0.0" }
+ Pod::Specification.any_instance.stubs(:platforms).returns([ Pod::Platform.new(:ios, '4.0'), Pod::Platform.new(:osx, '10.7') ])
lambda { @resolver.resolve }.should.not.raise
- end
-
- it "raises once any of the dependencies requires a higher deployment target of the root spec (Podfile)" do
- set = Pod::Spec::Set.new(config.repos_dir + 'master/ASIHTTPRequest')
- @resolver.cached_sets['ASIHTTPRequest'] = set
-
- def set.stub_platform=(platform); @stubbed_platform = platform; end
- def set.specification; spec = super; spec.platform = @stubbed_platform; spec; end
- @podfile.platform :ios, { :deployment_target => "4.0.0" }
- set.stub_platform = :ios, { :deployment_target => "5.0.0" }
+ Pod::Specification.any_instance.stubs(:platforms).returns([ Pod::Platform.new(:ios, '5.0'), Pod::Platform.new(:osx, '10.7') ])
lambda { @resolver.resolve }.should.raise Pod::Informative
end
View
29 spec/unit/specification_spec.rb
@@ -154,6 +154,19 @@
@spec.platform.should == :ios
end
+ it "returns the platform and the deployment target" do
+ @spec.platform = :ios, '4.0'
+ @spec.platform.should == :ios
+ @spec.platform.deployment_target.should == Pod::Version.new('4.0')
+ end
+
+ it "returns the platfroms for which the pod is supported" do
+ @spec.platform = :ios, '4.0'
+ @spec.platforms.count.should == 1
+ @spec.platforms.first.should == :ios
+ @spec.platforms.first.deployment_target.should == Pod::Version.new('4.0')
+ end
+
it "returns the license of the Pod" do
@spec.license = {
:type => 'MIT',
@@ -168,7 +181,7 @@
:text => 'Permission is hereby granted ...'
}
end
-
+
it "returns the license of the Pod specified in the old format" do
@spec.license = 'MIT'
@spec.license.should == {
@@ -183,7 +196,7 @@
'--project-company', '"Company Name"',
'--company-id', 'com.company',
'--ignore', 'Common',
- '--ignore', '.m']
+ '--ignore', '.m']
}
@spec.documentation[:html].should == 'http://EXAMPLE/#{@name}/documentation'
@spec.documentation[:appledoc].should == ['--project-name', '#{@name}',
@@ -288,11 +301,11 @@
s.source_files = "."
end
end
-
+
it "is marked as local" do
@spec.should.be.local
end
-
+
it "it returns the expanded local path" do
@spec.local_path.should == fixture("integration/JSONKit")
end
@@ -364,6 +377,8 @@
s.ios.dependency 'JSONKit'
s.osx.dependency 'SSZipArchive'
+
+ s.ios.deployment_target = '4.0'
end
end
@@ -382,6 +397,12 @@
}
end
+ it "returns the list of the supported platfroms and deployment targets" do
+ @spec.platforms.count.should == 2
+ @spec.platforms.should.include? Pod::Platform.new(:osx)
+ @spec.platforms.should.include? Pod::Platform.new(:ios, '4.0')
+ end
+
it "returns the same list of compiler flags for each platform" do
@spec.compiler_flags.should == {
:ios => ' -Wdeprecated-implementations -fobjc-arc',
Something went wrong with that request. Please try again.