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.

Sign up for GitHub

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

Minimum deployment target #14

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+17 −10
Open

Minimum deployment target #14

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5092d1f
Fix a misspelling in the README
Vkt0r Nov 25, 2019
2ced8a4
Update the `ios.deployment_target` from the `Podfile`
Vkt0r Nov 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions PodMergeExample/MergeFile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ end

group 'MergedSwiftPods' do
pod 'SnapKit', '5.0.1'
pod 'Kingfisher', '~> 5.0'
pod 'SwiftyJSON', '5.0.0'

end

group 'AlamofireGroup' do
Expand Down
5 changes: 5 additions & 0 deletions PodMergeExample/PodMergeExample/ViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,17 @@ class ViewController: UIViewController {

let request = ImageRequest(url: URL(string: "https://github.com/grab/cocoapods-pod-merge")!)

let url = URL(string: "https://example.com/image.png")

override func viewDidLoad() {
super.viewDidLoad()

// SnapKit Usage
let box = UIView()
box.snp.makeConstraints { _ in }

let imageView = UIImageView(frame: .zero)
imageView.kf.setImage(with: url)
}
}

10 changes: 5 additions & 5 deletions PodMergeExample/Podfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ DEPENDENCIES:
- UI (from `MergedPods/UI`)

SPEC REPOS:
https://cdn.cocoapods.org/:
trunk:
Copy link

Choose a reason for hiding this comment

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

Do we need this change? I think we can continue to use the new Cocoapods CDN

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Uhmm weird, I've used the latest Cocoapods version so it should be using the CDN now. I'll check that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're good with trunk. Take a look at this closed issue CocoaPods/CocoaPods#9190.

- Nuke

EXTERNAL SOURCES:
Expand All @@ -27,11 +27,11 @@ EXTERNAL SOURCES:
:path: MergedPods/UI

SPEC CHECKSUMS:
AlamofireGroup: 113f3ab321b31b75a748378909f3f96305b771a2
MergedSwiftPods: 86f52fdd7411987cd39e6c80e709ef0de0126fd3
Networking: 844633d13d2328a829083b24ffaee99aea51c1de
AlamofireGroup: c7af15f9d1f646dd2d5793dd470efc1942bfc20b
MergedSwiftPods: 90c36ec9c0177f0cd19776a428e6e22d6df267a4
Networking: 63b67ccfde89eaf06f6e01218d294ec2c8b6c628
Nuke: 85fb80f8df0cb26c28d2f4e0cb7fb93bcd6548d3
UI: 2aa82721ee430cd2f0ab314904bbe1a281fa2ff9
UI: 82623fb675f17ffe5a5cd00602619ee4f2040cd7

PODFILE CHECKSUM: 88ffe01efb39e0f819cf88bf11786182ab6f4441

Expand Down
3 changes: 1 addition & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,10 +245,9 @@ This plugin is not a magic bullet that'll merge all your cocoapods into a single
* **Refrain from merging complex or specialized pods** (like pods written in C/C++). Such pods can have special build settings and compiler flags that can break the other pods that are merged with them.
* **Make sure** you add the required [flags](https://github.com/grab/cocoapods-pod-merge#special-flags) to relevant groups in your `MergeFile`.


## Troubleshooting

If the above guidelines still do not solve your problem, please [report it](https://github.com/grab/cocoapods-pod-merge/issues)! Merging Pods is a complex process, and the plugin does not cover all possible use cases or podspec formats. Any feedback or feature suggesions are also encouraged. Bug reports and pull requests are welcome.
If the above guidelines still do not solve your problem, please [report it](https://github.com/grab/cocoapods-pod-merge/issues)! Merging Pods is a complex process, and the plugin does not cover all possible use cases or podspec formats. Any feedback or feature suggestions are also encouraged. Bug reports and pull requests are welcome.

## License

Expand Down
7 changes: 4 additions & 3 deletions lib/cocoapods-pod-merge/Main.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ def merge(merged_framework_name, group_contents, podfile_info)

# Create the local podspec
Pod::UI.puts "\tCreating Podspec for the merged framework".magenta
create_podspec(merged_framework_name, pods_to_merge, PodspecInfo.new(frameworks.uniq, prefix_header_contents.uniq, private_header_files.uniq, resources.uniq, script_phases.uniq, compiler_flags.uniq, libraries.uniq, prepare_command.uniq, resource_bundles, vendored_libraries.uniq, swift_version), mixed_language_group)
create_podspec(merged_framework_name, pods_to_merge, PodspecInfo.new(frameworks.uniq, prefix_header_contents.uniq, private_header_files.uniq, resources.uniq, script_phases.uniq, compiler_flags.uniq, libraries.uniq, prepare_command.uniq, resource_bundles, vendored_libraries.uniq, swift_version), mixed_language_group, podfile_info)
Copy link

Choose a reason for hiding this comment

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

Where is thispodfile_info being created? 🤔

Copy link
Contributor Author

@Vkt0r Vkt0r Dec 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Oh, this was created in this line

cocoapods-pod-merge/lib/cocoapods-pod-merge/Main.rb

Line 49 in c9830f8

podfile_info = read_podfile
I only passed it to be able to extract the deployment target.


Pod::UI.puts 'Cleaning up cache'.cyan
FileUtils.rm_rf(CacheDirectory)
Expand Down Expand Up @@ -487,7 +487,7 @@ def generate_module_map(merged_framework_name, public_headers)
module_map.close
end

def create_podspec(merged_framework_name, pods_to_merge, podspec_info, mixed_language_group)
def create_podspec(merged_framework_name, pods_to_merge, podspec_info, mixed_language_group, podfile_info)
frameworks = podspec_info.frameworks
prefix_header_contents = podspec_info.prefix_header_contents
private_header_files = podspec_info.private_header_files
Expand All @@ -499,6 +499,7 @@ def create_podspec(merged_framework_name, pods_to_merge, podspec_info, mixed_lan
resource_bundles = podspec_info.resource_bundles
vendored_libraries = podspec_info.vendored_libraries
swift_versions = podspec_info.swift_versions
ios_deployment_target = podfile_info.platforms.find { |platform| platform.include? "ios"}.split(',')[1]
Copy link

Choose a reason for hiding this comment

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

What happens if the Podfile does not define a platform? Will this be able to get a default value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a very valid question! By default Cocoapods provide a default in case that one is not specified:

CocoaPods provides a default deployment target if one is not specified. The current default values are 4.3 for iOS, 10.6 for OS X, 9.0 for tvOS and 2.0 for watchOS.

Nevertheless, in this specific case, we should provide a default one in the case that is not specified in the Podfile. Let's do that of course!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Uhmm ran some experiments and one thing emerged. If I delete the MergedPods folder and run bundle exec pod install Cocoapods would raise an exception like the following in case the platform is not set in the Podfile:

It is necessary to specify the platform in the Podfile if not integrating.

If the project was integrated it already I think we're good if the platform is not specified.

The other case is when once it was integrated we comment it out the platform in the Podfile and run bundle exec pod install and Cocoapods would try to set a default platform target version for the Pods.xcodeproj like this:

[!] Automatically assigning platform `iOS` with version `11.0` on target `PodMergeExample` because no platform was specified. Please specify a platform for this target in your Podfile. See `https://guides.cocoapods.org/syntax/podfile.html#platform`.

Let me know your thoughts about it.


mergedPodspec = %(
Pod::Spec.new do |s|
Expand All @@ -510,7 +511,7 @@ def create_podspec(merged_framework_name, pods_to_merge, podspec_info, mixed_lan
s.license = { :type => 'MIT', :text => 'Merged Pods by cocoapods-pod-merge plugin ' }
s.author = { 'GrabTaxi Pte Ltd' => '[email protected]' }
s.source = { :git => 'https://github.com/grab/cocoapods-pod-merge', :tag => '1.0.0' }
s.ios.deployment_target = '8.0'
s.ios.deployment_target = #{ios_deployment_target}
s.source_files = 'Sources/**/*.{h,m,mm,swift}'
)

Expand Down