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

[WIP] Resolver #2637

Merged
merged 44 commits into from Oct 24, 2014

Conversation

Projects
None yet
3 participants
@segiddins
Member

segiddins commented Oct 9, 2014

Closes #978, #1489, #2002, #2318, #2506.

Depends upon changes in CocoaPods/Core#186.

This branch is very much a work in progress, but I wanted to give us a place to discuss the final steps of integrating the resolver back into CocoaPods. To just test out the dependency resolution process, without having to pod install a million times, I recommend https://github.com/segiddins/cocoapods-dependencies (it even has an --ignore-lockfile option).

This PR won't be merged for a while, so I welcome any and all feedback.


Examples of improvement in this PR:

Podfile:

platform :ios, "7.0"
inhibit_all_warnings!
pod 'RestKit'
pod 'AFNetworking', '~> 1.2.0'

master:

[!] Unable to satisfy the following requirements:

- `AFNetworking (~> 1.3.0)` required by `RestKit/Network (0.23.3)`
- `AFNetworking (~> 1.2.0)` required by `Podfile`

resolver:

---
- AFNetworking (1.2.1)
- RestKit (0.20.1):
  - RestKit/Core
- RestKit/Core (0.20.1):
  - RestKit/CoreData
  - RestKit/Network
  - RestKit/ObjectMapping
- RestKit/CoreData (0.20.1)
- RestKit/Network (0.20.1):
  - AFNetworking (~> 1.2.0)
  - RestKit/ObjectMapping
  - RestKit/Support
  - SOCKit
- RestKit/ObjectMapping (0.20.1)
- RestKit/Support (0.20.1):
  - TransitionKit (= 1.1.0)
- SOCKit (1.1)
- TransitionKit (1.1.0)

Podfile:

platform :ios, "7.0"
inhibit_all_warnings!
pod 'RestKit'
pod 'AFAmazonS3Client'
pod 'CargoBay'
pod 'AFOAuth2Client'

master:

[!] Unable to satisfy the following requirements:

- `AFNetworking (~> 1.3.0)` required by `RestKit/Network (0.23.3)`
- `AFNetworking (~> 1.2.0)` required by `Podfile`

resolver:

---
- AFAmazonS3Client (1.0.1):
  - AFNetworking (~> 1.3)
- AFNetworking (1.3.4)
- AFOAuth2Client (0.1.2):
  - AFNetworking (~> 1.3)
- CargoBay (1.0.0):
  - AFNetworking (~> 1.0)
- ISO8601DateFormatterValueTransformer (0.6.0):
  - RKValueTransformers (~> 1.1.0)
- RKValueTransformers (1.1.0)
- RestKit (0.23.3):
  - RestKit/Core
- RestKit/Core (0.23.3):
  - RestKit/CoreData
  - RestKit/Network
  - RestKit/ObjectMapping
- RestKit/CoreData (0.23.3):
  - RestKit/ObjectMapping
- RestKit/Network (0.23.3):
  - AFNetworking (~> 1.3.0)
  - RestKit/ObjectMapping
  - RestKit/Support
  - SOCKit
- RestKit/ObjectMapping (0.23.3):
  - ISO8601DateFormatterValueTransformer (~> 0.6.0)
  - RKValueTransformers (~> 1.1.0)
  - RestKit/Support
- RestKit/Support (0.23.3):
  - TransitionKit (= 2.1.0)
- SOCKit (1.1)
- TransitionKit (2.1.0)

Podfile:

platform :ios, "7.0"
inhibit_all_warnings!
pod 'AFAmazonS3Client'
pod 'CargoBay'
pod 'AFOAuth2Client'

master:

[!] Unable to satisfy the following requirements:

- `AFNetworking (~> 2.0)` required by `AFAmazonS3Client (2.0.0)`
- `AFNetworking/Serialization` required by `AFNetworking (2.4.1)`
- `AFNetworking/Security` required by `AFNetworking (2.4.1)`
- `AFNetworking/Reachability` required by `AFNetworking (2.4.1)`
- `AFNetworking/NSURLConnection` required by `AFNetworking (2.4.1)`
- `AFNetworking/NSURLSession` required by `AFNetworking (2.4.1)`
- `AFNetworking/UIKit` required by `AFNetworking (2.4.1)`
- `AFNetworking/Serialization` required by `AFNetworking/NSURLConnection (2.4.1)`
- `AFNetworking/Reachability` required by `AFNetworking/NSURLConnection (2.4.1)`
- `AFNetworking/Security` required by `AFNetworking/NSURLConnection (2.4.1)`
- `AFNetworking/Serialization` required by `AFNetworking/NSURLSession (2.4.1)`
- `AFNetworking/Reachability` required by `AFNetworking/NSURLSession (2.4.1)`
- `AFNetworking/Security` required by `AFNetworking/NSURLSession (2.4.1)`
- `AFNetworking/NSURLConnection` required by `AFNetworking/UIKit (2.4.1)`
- `AFNetworking/NSURLSession` required by `AFNetworking/UIKit (2.4.1)`
- `AFNetworking (~> 2.2)` required by `CargoBay (2.1.0)`
- `AFNetworking (~> 1.3)` required by `AFOAuth2Client (0.1.2)`

resolver:

---
- AFAmazonS3Client (1.0.1):
  - AFNetworking (~> 1.3)
- AFNetworking (1.3.4)
- AFOAuth2Client (0.1.2):
  - AFNetworking (~> 1.3)
- CargoBay (1.0.0):
  - AFNetworking (~> 1.0)

Major (non-code) holdups:

  • Name for resolver gem (Molinillo)
  • Launch version for resolver gem (0.1.0)
  • Specs for conflict Podfiles that we now support
  • Performance torture testing
  • More complete documentation
  • Analyzer specs for handling implicitly locked dependencies (#2318)
  • Fine tuning UI hooks in the resolver to output what we want
  • Code review of the resolver gem itself (CocoaPods/Molinillo#2)
  • Fix existing specs with circular dependencies (CocoaPods/Molinillo#6)
  • Have a 2nd Core team member comfortable with the inner workings of this PR and Molinillo
@segiddins

This comment has been minimized.

Show comment
Hide comment
@segiddins

segiddins Oct 9, 2014

Member

We have a few features in the current resolver that dont really map cleanly to the new one:

  1. Showing what the dependent_spec is when we can't find a spec set
  2. Showing what required a dependency that led to a conflict
  3. Warning when multiple sources contain the same version of a pod
Member

segiddins commented Oct 9, 2014

We have a few features in the current resolver that dont really map cleanly to the new one:

  1. Showing what the dependent_spec is when we can't find a spec set
  2. Showing what required a dependency that led to a conflict
  3. Warning when multiple sources contain the same version of a pod
Show outdated Hide outdated cocoapods.gemspec
Show outdated Hide outdated lib/cocoapods/command.rb
# Sort dependencies so that the ones that are easiest to resolve are first.
# Easiest to resolve is (usually) defined by:
# 1) Is this dependency already activated?
# 2) How relaxed are the requirements?

This comment has been minimized.

@alloy

alloy Oct 10, 2014

Member

I assume that this means: the firmer the version requirements, the easiest it is to resolve? i.e. '= 1.2.3' is pretty clear, whereas '~> 1.2.3' could change depending on other dependencies in the graph?

@alloy

alloy Oct 10, 2014

Member

I assume that this means: the firmer the version requirements, the easiest it is to resolve? i.e. '= 1.2.3' is pretty clear, whereas '~> 1.2.3' could change depending on other dependencies in the graph?

This comment has been minimized.

@segiddins

segiddins Oct 10, 2014

Member

Yup. This is handled in the third fourth part of the sorting array, where we go by the total number of possibilities for the dependency.

@segiddins

segiddins Oct 10, 2014

Member

Yup. This is handled in the third fourth part of the sorting array, where we go by the total number of possibilities for the dependency.

@alloy

This comment has been minimized.

Show comment
Hide comment
@alloy

alloy Oct 10, 2014

Member

We have a few features in the current resolver that dont really map cleanly to the new one:

  1. Showing what the dependent_spec is when we can't find a spec set
  2. Showing what required a dependency that led to a conflict
  3. Warning when multiple sources contain the same version of a pod

Are you looking for input here? Because we obviously still need this functionality :)

Member

alloy commented Oct 10, 2014

We have a few features in the current resolver that dont really map cleanly to the new one:

  1. Showing what the dependent_spec is when we can't find a spec set
  2. Showing what required a dependency that led to a conflict
  3. Warning when multiple sources contain the same version of a pod

Are you looking for input here? Because we obviously still need this functionality :)

@alloy

This comment has been minimized.

Show comment
Hide comment
@alloy

alloy Oct 10, 2014

Member

One thing I’d like to see is some kind of way to easily see which methods are delegate methods of which Resolver module. I can’t find a nice pattern to do it in the method name like Cocoa does, so at the very least can you define ‘groups’ of methods with the YARD grouping comments?

Member

alloy commented Oct 10, 2014

One thing I’d like to see is some kind of way to easily see which methods are delegate methods of which Resolver module. I can’t find a nice pattern to do it in the method name like Cocoa does, so at the very least can you define ‘groups’ of methods with the YARD grouping comments?

@segiddins

This comment has been minimized.

Show comment
Hide comment
@segiddins

segiddins Oct 10, 2014

Member

It's both a note to myself and an open call for ideas :)

-Samuel E. Giddins

On Oct 10, 2014, at 12:54 AM, Eloy Durán notifications@github.com wrote:

We have a few features in the current resolver that dont really map cleanly to the new one:

  1. Showing what the dependent_spec is when we can't find a spec set
  2. Showing what required a dependency that led to a conflict
  3. Warning when multiple sources contain the same version of a pod

Are you looking for input here? Because we obviously still need this functionality :)


Reply to this email directly or view it on GitHub.=

Member

segiddins commented Oct 10, 2014

It's both a note to myself and an open call for ideas :)

-Samuel E. Giddins

On Oct 10, 2014, at 12:54 AM, Eloy Durán notifications@github.com wrote:

We have a few features in the current resolver that dont really map cleanly to the new one:

  1. Showing what the dependent_spec is when we can't find a spec set
  2. Showing what required a dependency that led to a conflict
  3. Warning when multiple sources contain the same version of a pod

Are you looking for input here? Because we obviously still need this functionality :)


Reply to this email directly or view it on GitHub.=

@alloy

This comment has been minimized.

Show comment
Hide comment
@alloy

alloy Oct 10, 2014

Member

Regarding CocoaPods/Core#73, I can’t find a test for this in the patches. Am I overlooking it or is it missing?

Member

alloy commented Oct 10, 2014

Regarding CocoaPods/Core#73, I can’t find a test for this in the patches. Am I overlooking it or is it missing?

@segiddins

This comment has been minimized.

Show comment
Hide comment
@segiddins

segiddins Oct 10, 2014

Member

Those tests were added in a previous PR and are enabled in this one.

-Samuel E. Giddins

On Oct 10, 2014, at 1:03 AM, Eloy Durán notifications@github.com wrote:

Regarding CocoaPods/Core#73, I can’t find a test for this in the patches. Am I overlooking it or is it missing?


Reply to this email directly or view it on GitHub.=

Member

segiddins commented Oct 10, 2014

Those tests were added in a previous PR and are enabled in this one.

-Samuel E. Giddins

On Oct 10, 2014, at 1:03 AM, Eloy Durán notifications@github.com wrote:

Regarding CocoaPods/Core#73, I can’t find a test for this in the patches. Am I overlooking it or is it missing?


Reply to this email directly or view it on GitHub.=

@alloy

This comment has been minimized.

Show comment
Hide comment
@alloy

alloy Oct 10, 2014

Member

Those tests were added in a previous PR and are enabled in this one.

Ok perfect.

Member

alloy commented Oct 10, 2014

Those tests were added in a previous PR and are enabled in this one.

Ok perfect.

@segiddins segiddins referenced this pull request Oct 10, 2014

Merged

[WIP] Resolver #186

segiddins added a commit that referenced this pull request Oct 13, 2014

@segiddins

This comment has been minimized.

Show comment
Hide comment
@segiddins

segiddins Oct 13, 2014

Member

We have a few features in the current resolver that dont really map cleanly to the new one:

  1. Showing what the dependent_spec is when we can't find a spec set
  2. Showing what required a dependency that led to a conflict
  3. Warning when multiple sources contain the same version of a pod

(3) is complete, (2) is accomplished by CocoaPods/Molinillo#3.
(1) is the only one of these remaining where I'm unsure of how to implement it in a clean way.

Member

segiddins commented Oct 13, 2014

We have a few features in the current resolver that dont really map cleanly to the new one:

  1. Showing what the dependent_spec is when we can't find a spec set
  2. Showing what required a dependency that led to a conflict
  3. Warning when multiple sources contain the same version of a pod

(3) is complete, (2) is accomplished by CocoaPods/Molinillo#3.
(1) is the only one of these remaining where I'm unsure of how to implement it in a clean way.

@segiddins segiddins added this to the 0.35.0 milestone Oct 14, 2014

@segiddins

This comment has been minimized.

Show comment
Hide comment
@segiddins

segiddins Oct 15, 2014

Member

@kylef @alloy please take a look at the failing integration tests and let me know if you're ok with the changes!

Member

segiddins commented Oct 15, 2014

@kylef @alloy please take a look at the failing integration tests and let me know if you're ok with the changes!

@segiddins

This comment has been minimized.

Show comment
Hide comment
@segiddins

segiddins Oct 15, 2014

Member

@kylef @alloy please take a look at the rubocop failures at https://travis-ci.org/CocoaPods/CocoaPods/builds/38080546#L1770, and the changes to the integration specs at CocoaPods/cocoapods-integration-specs@14c1483

Member

segiddins commented Oct 15, 2014

@kylef @alloy please take a look at the rubocop failures at https://travis-ci.org/CocoaPods/CocoaPods/builds/38080546#L1770, and the changes to the integration specs at CocoaPods/cocoapods-integration-specs@14c1483

segiddins added a commit that referenced this pull request Oct 15, 2014

segiddins added a commit that referenced this pull request Oct 16, 2014

segiddins added a commit that referenced this pull request Oct 23, 2014

segiddins added a commit that referenced this pull request Oct 23, 2014

@alloy

This comment has been minimized.

Show comment
Hide comment
@alloy

alloy Oct 23, 2014

Member

Here’s an example Podfile and some changes in the Podfile.lock file after changing from 0.34.4 to the resolver branch.

The fact that Expecta 0.3.1 was previously activated while the Podfile explicitly requires 0.3.0 is a hilarious improvement, so there’s that 😝 👍

Regarding the HEAD changes, how are you planning on locking specific SCM commits? I.e. in the cases where the :head option is used, the next installation should (eventually, it doesn’t right now either) install the exact same revision.

Member

alloy commented Oct 23, 2014

Here’s an example Podfile and some changes in the Podfile.lock file after changing from 0.34.4 to the resolver branch.

The fact that Expecta 0.3.1 was previously activated while the Podfile explicitly requires 0.3.0 is a hilarious improvement, so there’s that 😝 👍

Regarding the HEAD changes, how are you planning on locking specific SCM commits? I.e. in the cases where the :head option is used, the next installation should (eventually, it doesn’t right now either) install the exact same revision.

@segiddins

This comment has been minimized.

Show comment
Hide comment
@segiddins

segiddins Oct 23, 2014

Member

@alloy I think that behavior would belong in the analyzer, in generate_versions_locking_dependencies

Member

segiddins commented Oct 23, 2014

@alloy I think that behavior would belong in the analyzer, in generate_versions_locking_dependencies

segiddins added some commits Oct 13, 2014

[Resolver] Fix handling of pre-release specs
This was tough to do, since it really adversely affected performance on unresolvable podfiles. Hence the optimization of making sure that a pod's dependencies with the same root name are pinned to the same version in `dependencies_for`

\c @kylef, @alloy
[ResolverSpec] Add uses a Podfile requirement even when a previously …
…declared dependency has a different requirement

segiddins added a commit that referenced this pull request Oct 24, 2014

@segiddins segiddins merged commit 0a6e8a1 into master Oct 24, 2014

1 check was pending

continuous-integration/travis-ci The Travis CI build is in progress
Details

@segiddins segiddins deleted the resolver branch Oct 24, 2014

segiddins added a commit to CocoaPods/cocoapods-integration-specs that referenced this pull request Oct 24, 2014

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