[Cache] Use git ls-remote to skip full clones for branch dependencies
#5386
Conversation
| @@ -21,7 +21,7 @@ class Request | |||
|
|
|||
| # @return [Hash<Symbol, String>] The download parameters for this request. | |||
| # | |||
| attr_reader :params | |||
| attr_accessor :params | |||
There was a problem hiding this comment.
i'd rather keep Request immutable and just construct a new instance instead of mutating?
There was a problem hiding this comment.
I was torn on this. I agree that adding mutation here isn't the nicest thing. But it seemed weird to me that the request instance that's held by the user of the downloader would stop being "truthful". Looking at the call sites for Downloder.download the request is never even saved to a variable so I think I'll change it to be immutable again.
|
🎆 you can actually change the Gemfile to point to your branch for now so we can try this out :D |
|
@segiddins done! |
|
@segiddins I'm not sure how to go about adding tests for this, do you have any ideas? |
|
In https://github.com/CocoaPods/CocoaPods/blob/master/spec/unit/downloader/cache_spec.rb, maybe just make sure the options are preprocessed? |
This adds the posibility of caches hits for git branch depedencies. Previously, since pods didn't know the commit the branch points it always did a full clone of the repo. Using `ls-remote` we can check the commit the branch points to and reuse cached version of the pods if available.
| if can_cache | ||
| raise ArgumentError, 'Must provide a `cache_path` when caching.' unless cache_path | ||
| cache = Cache.new(cache_path) | ||
| result = cache.download_pod(request) | ||
| else | ||
| raise ArgumentError, 'Must provide a `target` when caching is disabled.' unless target |
There was a problem hiding this comment.
@segiddins this is a bit out of scope in this PR but I stumbled across it writing the test. Does this seem right to you?
There was a problem hiding this comment.
If I recall this is because otherwise you don't know where to download to
There was a problem hiding this comment.
exactly. I stumbled upon it throwing a random error farther down the call chain so I copied the check for cache_path.
I've added a simple test btw. Considering the coverage over at cocoapods-downloader I think this is enough, but I can add more if you disagree.
|
👍🏻 |
|
I'm trying to use cocoapods right form the sources (setting my Gemfile as But now I'm getting an error on pod install, and looks like the problem is caused by this merge.
|
|
Ok, nevermind, I added |
This adds the posibility of caches hits for git branch dependencies.
Previously, since pods didn't know the commit the branch points it
always did a full clone of the repo. Using
ls-remotewe can check thecommit the branch points to and reuse cached version of the pods if
available.
Related to #5376
TODO