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

[RFC] Expanding `use_frameworks!` to allow specifying static (or dynamic) linking #9099

Closed
dnkoutso opened this issue Aug 11, 2019 · 9 comments · Fixed by #9119
Closed

[RFC] Expanding `use_frameworks!` to allow specifying static (or dynamic) linking #9099

dnkoutso opened this issue Aug 11, 2019 · 9 comments · Fixed by #9119
Milestone

Comments

@dnkoutso
Copy link
Contributor

@dnkoutso dnkoutso commented Aug 11, 2019

Expanding use_frameworks! DSL (specifying linkage style)

Introduction

CocoaPods today provides little space for pod consumers to customize how they want their pods to be linked. As of today, pod consumers are only able to customize different packaging (static library vs framework) via the use_frameworks! DSL. The linking style happens to be a side-effect of that decision without giving pod consumers a proper way to configure it.

This RFC aims to expand the use_frameworks! DSL in order to give pod consumers a little bit more control and allow them to specify the link style they want if they decide to integrate their pods using frameworks.

Motivation

Leveraging the .framework bundle structure has proven to be very useful. Using frameworks was also a hard requirement in the earlier days of Swift.

Here's the definition of a framework from Apple:

A framework is a hierarchical directory that encapsulates shared resources, such as a dynamic shared library, nib files, image files, localized strings, header files, and reference documentation in a single package. Multiple applications can use all of these resources simultaneously. The system loads them into memory as needed and shares the one copy of the resource among all applications whenever possible.

Up until recently, a .framework mostly contained a dynamic shared library which was loaded at runtime when the application launched. However, this could cause issues when the number of dynamic libraries grows high enough because the app would take longer to boot. In some extreme cases, the app would be killed off by the system if it took too long to load all dynamically shared libraries.

Since CocoaPods introduced the static_framework DSL, only pod authors could leverage the .framework structure but instead force static linkage. On the other hand, pod consumers still have no way of specifying that they want to use .framework packaging but instead statically link their pods into their app executable.

Giving this option to pod consumers would give them more control and allow them to leverage both the .framework structure but also avoid some of the aforementioned issues.

Proposed solution

Extend the existing use_frameworks! DSL in order to allow pod consumers to specify how they want linking to happen via a hash.

Detailed design

Core gem changes

Move BuildType class into core gem

The recently introduced BuildType class encapsulates all of packaging and linking concepts into one very simple and well tested class. This can be leveraged as part of this work in order for it to be used as an easy to understand model for storing all information related to a target definition's desired build type.

Thus, as part of this proposal, the BuildType class and its specs will move over to the CocoaPods/Core gem.

Expanding use_frameworks!

The current TargetDefinition use_frameworks! method is defined as such:

def use_frameworks!(flag = true)
  set_hash_value('uses_frameworks', flag)  
end

It stores the given flag as a Boolean in the target definition's hash.

The re-defined method will accept both a Boolean and a Hash and instead serialize an instance of BuildType class into the target definition's internal hash and will contain both the packaging and linkage information.

# Sets whether the target definition's pods should be built as frameworks.
#
# @param [Boolean, Hash] option
#        Whether pods that are integrated in this target should be built as frameworks. If the option is a
#        boolean then the value affects both packaging and linkage styles. If set to true, then dynamic frameworks
#        are used and if it's set to false, then static libraries are used. If the option is a hash then
#        `:framework` packaging is implied and the user configures the `:linkage` style to use.
#
# @return [void]
#
def use_frameworks!(option = true)
  value = case option
          when true, false
            option ? BuildType.dynamic_framework : BuildType.static_library
          when Hash
            BuildType.new(:linkage => option.fetch(:linkage), :packaging => :framework)
          else
            raise ArgumentError, "Got `#{option.inspect}`, should be a boolean or hash."
          end
  set_hash_value('uses_frameworks', value.to_hash)
end

As you can see, the 'uses_frameworks' key stored in the target definition's hash will not change in order to maintain backwards compatibility for existing Podfile files that may have been serialized (for example, into yaml).

Furthermore, the default value of the option parameter will remain true also in order to maintain backwards compatibility with all of the existing configurations that currently specify use_frameworks! in their Podfile.

An additional method will be introduced in the TargetDefinition class that returns the (desired) build type it requires:

# The (desired) build type for the pods integrated in this target definition. Defaults to static libraries and can
# only be overridden through Pod::Podfile::DSL#use_frameworks!.
#
# @return [BuildType]
#
def build_type
  value = get_hash_value('uses_frameworks', root? ? BuildType.static_library : parent.build_type)
  case value
  when true, false
    value ? BuildType.dynamic_framework : BuildType.static_library
  when Hash
    BuildType.new(:linkage => value.fetch(:linkage), :packaging => value.fetch(:packaging))
  when BuildType
    value
  else
    raise ArgumentError, "Got `#{value.inspect}`, should be a boolean, hash or BuildType."
  end
end

Finally, the TargetDefinition#uses_frameworks? method will also need to be updated to work with the new hash that is now being stored:

-# @return [Bool] whether the target definition should build
-#         a framework.
+# @return [Bool] whether the target definition pods should be built as frameworks.
 #
 def uses_frameworks?
   if internal_hash['uses_frameworks'].nil?
     root? ? false : parent.uses_frameworks?
   else
-    get_hash_value('uses_frameworks')
+    build_type.framework?
   end
 end

With the above changes the following examples are now possible:

  pod 'AFNetworking', '~> 1.0'

  target 'MyApp' do
    use_frameworks!
  end

  target 'MyApp2' do
    use_frameworks! :linkage => :dynamic
  end

  target 'MyApp3' do
    use_frameworks! :linkage => :static
  end

This concludes the changes required for the Core gem.

CocoaPods gem changes

As mentioned, most of the work for supporting the notion of different packaging or linking types has already been completed through the aforementioned BuildType class that was introduced in the 1.7 release. This also includes variant support and CocoaPods will automatically split a pod that is consumed across targets with different build types for both packaging and linking.

Update BuildType references

Since the BuildType class will be moved as part of this proposal, all references must be updated in the CocoaPods gem. This should be straightforward find/replace.

Remove host_requires_frameworks boolean flag

The existing host_requires_frameworks boolean flag found in Target is no longer needed and it is redundant given that the initializer requires a BuildType instance to be passed in.

As part of this proposal, the Target class and it's subclasses will need to be updated to remove host_requires_frameworks boolean flag and instead update all callers to rely on the BuildType attribute.

Additionally, all initializers will be updated to require a BuildType as it currently appears to be optional and can accept a nil value which should never be the case.

Update analyzer

The analyzer will need to be updated in order to take into consideration the (desired) BuildType specified in the TargetDefinition when constructing PodTarget instances.

A new method will be added that calculates the BuildType to use:

# Calculates and returns the BuildType to use for the given spec. If the spec specifies `static_framework` then
# it is honored as long as the host build type also requires its pods to be integrated as frameworks.
#
# @param [Specification] spec
#        the spec to determine the BuildType for.
#
# @param [BuildType] host_build_type
#        The desired build type by the host of this pod target. If the pod target does not specify explicitly a
#        build type then the one from the host is used.
#
# @return [BuildType]
#
def determine_build_type(spec, host_build_type)
  if host_build_type.framework?
    root_spec = spec && spec.root
    if root_spec && root_spec.static_framework
      BuildType.static_framework
    else
      host_build_type
    end
  else
    BuildType.static_library
  end
end

The above method will be used by the analyzer when determining the BuildType to use for a PodTarget instance.

Backwards compatibility

Most of the concerns around backwards compatibility have already been covered in the 'Detailed Design' section especially regarding the serialization of the Podfile and ensuring that existing setups that use use_frameworks! continue to function after this proposal is implemented.

This proposal has no effect on podspec files and therefore it is considered a low-risk change.

Alternatives considered

An alternative was to introduce a new use_static_frameworks! DSL that would have the same effect. However, this was avoided due to the proliferation of DSL options in CocoaPods and would require additional linting in order to ensure it is mutually exclusive with the use_frameworks! option.

Another alternative as always is to do nothing in which case CocoaPods will remain very limited in its configuration options regarding linking and packaging during integration.

@dnkoutso dnkoutso added this to the 1.9.0 milestone Aug 11, 2019
@paulb777

This comment has been minimized.

Copy link
Member

@paulb777 paulb777 commented Aug 12, 2019

LGTM! - just a few nits ...

In the second paragraph, I think you mean "pod consumers" instead of "pod authors".

From a CocoaPods perspective, the "Since Xcode 9" could be changed to "Since CocoaPods and the introduction of the podspec attribute "static_framework"".

@amorde

This comment has been minimized.

Copy link
Member

@amorde amorde commented Aug 12, 2019

Looks great 👍

@amorde

This comment has been minimized.

Copy link
Member

@amorde amorde commented Aug 12, 2019

There's a TODO-PENDING-WIP in there still

@dnkoutso

This comment has been minimized.

Copy link
Contributor Author

@dnkoutso dnkoutso commented Aug 13, 2019

Yeap will take care of it after vacation.

@cltnschlosser

This comment has been minimized.

Copy link

@cltnschlosser cltnschlosser commented Aug 14, 2019

Would there be any way to override this setting at a per pod level?
Currently using static frameworks with this:

pre_install do |installer|
  installer.pod_targets.each do |pod|
    if !dynamic_frameworks.include?(pod.name) and !pod.build_as_static_framework?
      puts "Forcing static_framework true for #{pod.name}"
      def pod.build_as_static_framework?
        true
      end
      def pod.build_as_dynamic_framework?
        false
      end
    end
  end
end

The reason for the dynamic_frameworks list is that certain pods don't work as static frameworks, but it's still nice to get the benefit of static frameworks for the majority.

@dnkoutso

This comment has been minimized.

Copy link
Contributor Author

@dnkoutso dnkoutso commented Aug 14, 2019

@cltnschlosser yes! there is a proposal for this here https://docs.google.com/document/d/1lr2mHkwhNha5_0Wfa_pYL51MAxoro0LBxx_DUzuSu0Y/edit but I wanted to flesh out this one separately since its a much easier change.

@segiddins

This comment has been minimized.

Copy link
Member

@segiddins segiddins commented Aug 21, 2019

@dnkoutso I . thought you were also proposing allowing specifying packaging on a per-pod basis?

@dnkoutso

This comment has been minimized.

Copy link
Contributor Author

@dnkoutso dnkoutso commented Aug 21, 2019

@segiddins I decided to split it into two. See #9099 (comment). This change is much easier to implement and more certain.

Even if we decided to do per-pod linkage and packaging I suspect we would need an overarching way to customize all pods packaging and linking like use_frameworks! does today but for static linking.

@amorde

This comment has been minimized.

Copy link
Member

@amorde amorde commented Aug 22, 2019

Yeah this seems like a simple win and a natural 1st step towards the per-pod solution

@weibel weibel mentioned this issue Dec 13, 2019
1 of 1 task complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.