-
Notifications
You must be signed in to change notification settings - Fork 2.7k
[Downloader::Git] Cache repos #248
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
Conversation
* master: [Command::Spec::Linter] Fix - Skip documentation generation [Config#doc] Renamed to generate_docs [Command::Spec::Linter] Bug fix.
* master: [Installer] Don't generate documenation if already installed
True, and it seems that the logic for it is not complex, so I’m inclined to agree with this feature. Just keep in mind that in the future we may switch to a real package system, i.e. have our own web service that stores and distributes packages with metadata, making us free from any SCM details, so try to keep this stuff as simple as possible. Why did you remove the github tarball code btw? |
I opened the pull only because it is a simple patch as I consider caching the pods playing with fire :-). Luckily git does all the heavy work.
I assumed the tarball logic was there to speed up downloads avoiding the whole git metadata. Leaving it would skip the proposed cache, which I think is way faster because once you download a pod any other version is a maximun an incremental git download. |
True, git is pretty good at it.
Hmm, actually, I don’t know what the exact use case was for introducing it, @lukeredpath can you explain it? |
- ensure that cache git repos are clean - added cache size limit
* master: [Command::Spec::Linter] fix for #podspec_warnings. [Command::Spec] Template layout fix. [Platform] Removed options accessor. [Executable] Made executed command more prominent. Spelling correctioSpelling correctionn [Resolver] Support for deployment target check. [Specification#deployment_target=] Switch to Pod::Version [Platform#==] Check for deployment target. [Platform] Simplify deployment target specification. [#212] Check for deployment target
@alloy @lukeredpath Any update on this? It speeds up a lot testing podfiles. |
@irrationalfab I think the cache feature is a good idea! Regarding the git zipballs, that’s really up-to @lukeredpath to answer, so, for now, I would say let’s leave that in. |
@alloy The problem is that if I leave the github tarball code this conditional intercepts all the github urls rendering the cache useless in 99% of the cases. Unless I misunderstood you and you are suggesting to leave the code and remove the conditional, but in that case the GitHub tarball is not used by anything. |
The motiviation behind the Github downloader using the tarball was for speed. I'd rather it was left in - it's not true that any github URLs will bypass the cache as the tarball download behaviour only happens if the If, in any instance its not doing this, then that should be considered a bug. |
@lukeredpath Thank you for the reply and for the correction. I should have been more meticulous. I've restored the the Pod::Downloader::Git::GitHub class and now I'll merge. |
[Downloader::Git] Cache repos
@irrationalfab A couple more questions:
|
Good point. Not sure even if we need on/off actually. I was thinking of reactivating the cache in the integration specs tests the whole thing.
Agreed as we purge the cache. The original ideas was to have something like homebrew that justs keeps adding caches (irrc).
Sure I was just thinking of a good way to do them. The TTD is slowing entering in my mindset thanks to Ruby :-) |
@alloy Do you think that I ask too much from people if I set the cache to 1Gb? |
@irrationalfab I really don’t think we need to worry about it all atm. Does it grow that fast? |
@alloy Sorry my bad, in the past the specs used to saturate the cache and I thought that this was the problem, instead is that setting the max cache size to 0 nukes everything. I have removed any configuration (lets see if somebody complains) and enabled the cache in the specs (It shaves 1/2 mins). |
No need for that :)
Yeah I’d rather have a complaint first, before we make it more complex for ourselves :) Sweet, good work! |
Added Base32 and Base64
[Downloader::Git] Cache repos
Update integration specs for #9125
I think that we could take advantage of Git to reduce execution times and bandwidth usage.
The idea is to store a clone of the remote somewhere in the disk and the update and clone this copy as a cache.