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

Support multiple CDN sources #771

Merged
merged 4 commits into from
Jul 27, 2024
Merged

Conversation

santam85
Copy link
Contributor

@santam85 santam85 commented May 14, 2024

Allows to declare multiple CDN sources in your Podfile, even if one of them does not fully mirror the trunk.
Removes the requirement to provide empty shards for the version metadata files of non-existing pods on a CDN repo.

Prevents errors like:

### Error

Errno::ENOENT - No such file or directory @ rb_sysopen - /Users/nathan/.cocoapods/repos/pods-private/all_pods_versions_4_0_1.txt

Fixes #768
Fixes CocoaPods/CocoaPods#12338

@igor-makarov
Copy link
Contributor

This doesn't look like working code - there seems to be at least one undefined merhod.

@maikelvdh
Copy link
Contributor

This doesn't look like working code - there seems to be at least one undefined merhod.

Would you mind to elaborate on it further? I have been leveraging similar patch for identical usecase as @santam85 and this seems to work for the use case.

@santam85
Copy link
Contributor Author

The added line is using local_file_okay? from

def local_file_okay?(partial_url)

@santam85
Copy link
Contributor Author

@orta @segiddins @paulb777 can you have a look at this one please?

@santam85
Copy link
Contributor Author

santam85 commented Jun 4, 2024

@igor-makarov I fixed the test errors triggered by the first iteration of my changes. Any other feedback on this PR? Do you think it's ready to be merged/released?

@santam85
Copy link
Contributor Author

@dnkoutso @amorde sorry for tagging but I noticed you were recently actively reviewing PRs, care to take a look at this one please?

lib/cocoapods-core/cdn_source.rb Outdated Show resolved Hide resolved
Accept @segiddings suggestion

Co-authored-by: Samuel Giddins <segiddins@segiddins.me>
@santam85
Copy link
Contributor Author

@segiddins thanks for the suggestion! I see the macos-11 agent seems to fail altogether in the tests, what's missing to merge and release this?

@santam85
Copy link
Contributor Author

https://github.blog/changelog/2024-05-20-actions-upcoming-changes-to-github-hosted-macos-runners/
MacOS 11 is removed from GHA supported stack, updating to a supported version.

…github-hosted-macos-runners/

MacOS 11 is removed from GHA supported stack, updating to a supported version.
@amorde
Copy link
Member

amorde commented Jul 23, 2024

In this scenario why are we requesting indices for shards that don't exist? I would think we should be using the metadata and list of pods for the repo to determine which shards are possible, so this shouldn't even happen

@amorde
Copy link
Member

amorde commented Jul 23, 2024

Unfortunately CocoaPods is basically unmaintained at this point so its hard to review changes like this when the impacts are unclear

@segiddins
Copy link
Member

This feels like a fairly benign change to me, given it is explicitly basically ignoring this one exception

@santam85
Copy link
Contributor Author

santam85 commented Jul 24, 2024

@segiddins Thanks for your approval. If there's consensus this is a desirable change, how can we get it released?

@santam85
Copy link
Contributor Author

In this scenario why are we requesting indices for shards that don't exist? I would think we should be using the metadata and list of pods for the repo to determine which shards are possible, so this shouldn't even happen

When using repositories different from the trunk, it might be that not all trunk are populated. The client attempts downloading the version shards from all sources.

@amorde
Copy link
Member

amorde commented Jul 27, 2024

In this scenario why are we requesting indices for shards that don't exist? I would think we should be using the metadata and list of pods for the repo to determine which shards are possible, so this shouldn't even happen

When using repositories different from the trunk, it might be that not all trunk are populated. The client attempts downloading the version shards from all sources.

Right I understand that the custom repo doesn't have the shards. I'm asking why the code is even trying to load those shards from a repo that doesn't have them. it should be deciding which things to load based on the settings of the repo.

Like @segiddins said though this seems fine to land regardless.

@amorde amorde merged commit ffea754 into CocoaPods:master Jul 27, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants