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

Fix PINCaching compiling in Xcode 12.0b6 (#275) #281

Merged
merged 1 commit into from
Sep 29, 2020

Conversation

SAGESSE-CN
Copy link
Contributor

@garrettmoon
Copy link
Collaborator

@SAGESSE-CN I'm not seeing any errors compiling on Xcode 12 beta 6. Can you provide more info?

@garrettmoon
Copy link
Collaborator

Specifically, if you could point out how I can reproduce it, that would be very helpful!

@bluemachine
Copy link

When we install the pod PINRemoteImage - it installs with following pods

Installing PINCache (3.0.1-beta.8)
Installing PINOperation (1.1.2)
Installing PINRemoteImage (3.0.0)

To update as suggested - when we install PINCache it updates to latest version but installs an older version of PINRemoteImage and followings pods.

Installing FLAnimatedImage (1.0.12)
Installing PINCache 3.0.1 (was 3.0.1-beta.8)
Installing PINOperation 1.2 (was 1.1.2)
Installing PINRemoteImage 2.1.4 (was 3.0.0)

This gives 4 build errors - screenshots below
Screenshot 2020-09-08 at 20 50 41
Screenshot 2020-09-08 at 20 50 52
Screenshot 2020-09-08 at 20 51 00
Screenshot 2020-09-08 at 20 51 10

Hope this helps in recreating the issue.

@SAGESSE-CN
Copy link
Contributor Author

SAGESSE-CN commented Sep 9, 2020

Specifically, if you could point out how I can reproduce it, that would be very helpful!

Reproduce

In any dependent PINCache project.

PINMemoryCache *cache = [[PINMemoryCache alloc] init];
cache.didAddObjectBlock = ^(PINMemoryCache *cache, NSString * _Nonnull key, id  _Nullable object) {
    // do something.
};

Reason

The compile error in PINCache project was fixed in #276.

But that solution was not the best, any users for using PINCache may need to make a lot of changes.

The block signature is void(^)(PINMemoryCache *, NSString *, id), this is a correct signature, can't limit users must to use void(^)(id<PINCaching>, NSString *, id).

A better solution is to add the __kindof into id<PINCaching>, user not need to modify code, just simply update the Cocoapods dependency.

About why must to add __kindof, see reference.

Thx!

@Plnda
Copy link

Plnda commented Sep 23, 2020

Could we get some traction on this PR, What is currently blocking it?

@natebirkholz
Copy link

What is holding up approval of this PR? I cannot get my work done without it.

Copy link
Collaborator

@garrettmoon garrettmoon left a comment

Choose a reason for hiding this comment

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

Thank you for the explanation and fix! Hopefully this will make downstream dependencies a little easier to manage.

@garrettmoon garrettmoon merged commit 37831e4 into pinterest:master Sep 29, 2020
@natebirkholz
Copy link

Reference: https://reviews.llvm.org/D66831#2019125

I get the latest with your changes and I still get two incompatible pointers.

PINCache.m line 109;
[self->_memoryCache objectForKeyAsync:key completion:^(PINMemoryCache
and line 232:
[_diskCache synchronouslyLockFileAccessWhileExecutingBlock:^(PINDiskCache *diskCache)

@natebirkholz
Copy link

@SAGESSE-CN
Copy link
Contributor Author

Reference: https://reviews.llvm.org/D66831#2019125

@SAGESSE-CN

Can you provide more information, a demo that depyon to pincache?

@natebirkholz
Copy link

Reference: https://reviews.llvm.org/D66831#2019125

@SAGESSE-CN

Can you provide more information, a demo that depyon to pincache?

@SAGESSE-CN My bad, the pod updates but something somewhere in this large legacy project I took over is specifying an earlier version of PINCache, so in my Xcode project it is an old version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants