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

CocoaLumberjack 2.0.0-rc does not build with Pods #433

Closed
Legoless opened this issue Jan 5, 2015 · 25 comments
Closed

CocoaLumberjack 2.0.0-rc does not build with Pods #433

Legoless opened this issue Jan 5, 2015 · 25 comments
Milestone

Comments

@Legoless
Copy link

Legoless commented Jan 5, 2015

I updated the CocoaLumberjack to 2.0.0-rc from 2.0.0-beta4 and now the project does not build anymore. This are the two errors I get:

DDLog.h:344:1: Property with 'retain (or strong)' attribute must be of object type
DDLog.h:539:1: Property with 'retain (or strong)' attribute must be of object type

Apparently these lines were changed to point to objects. Is there any specific reason that those properties are marked as strong?

@property (nonatomic, DISPATCH_QUEUE_REFERENCE_TYPE, readonly) dispatch_queue_t loggerQueue;
@ipeisong
Copy link

ipeisong commented Jan 7, 2015

See the same issue

@jasperblues
Copy link

Also getting this.

See which issue?

@Legoless
Copy link
Author

Legoless commented Jan 9, 2015

So, according to this: http://stackoverflow.com/questions/8904206/what-property-should-i-use-for-a-dispatch-queue-after-arc they should be objects. But even that my deployment target is iOS 7.0, compiler throws this error.

The problem is apparently with CocoaPods's linking system and should add !COCOAPODS to the checking macro.

@rivera-ernesto
Copy link
Member

We had this problem before (#410) and was "patched" by #412.

Then it seemed not to be necessary anymore in #420.

Any ideas @foozmeat?

@foozmeat
Copy link
Contributor

@Legoless Can you tell us what version of cocoapods you're using? Cocoapods previously overrode OS_OBJECT_USE_OBJC which would cause checks for dispatch_queue_t's type to act incorrectly. My patch in #420 corrected the check that CLJ uses but it relies on you using a recent version of Cocoapods.

@Legoless
Copy link
Author

I should be on the pre version of CocoaPods (using --pre) and updating CocoaPods every day.

@foozmeat
Copy link
Contributor

Alright, if you open Classes/DDLog.h you should see

#if OS_OBJECT_HAVE_OBJC_SUPPORT
     #define DISPATCH_QUEUE_REFERENCE_TYPE strong
#else
     #define DISPATCH_QUEUE_REFERENCE_TYPE assign

Which line is Xcode syntax highlighting? Also, can you tell us what your SDK and Deployment targets are?

@Legoless
Copy link
Author

It's the top one of course, thats where the error is. Using SDK 8.1 and Deployment target 7.0.

@foozmeat
Copy link
Contributor

OK after doing some digging I see where the error is. The CLJ podspec specifies the following

  s.ios.deployment_target = '5.0'
  s.osx.deployment_target = '10.7'

According to the Cocoapods docs this causes cocoapods to set OS_OBJECT_USE_OBJC=0 which will cause dispatch_queue_t to require assign instead of strong.

If the above podspec settings are correct for CLJ 2.0 then my patch will need to be reverted and the check will have to behave differently under cocoapods. This feels pretty gross to me. I wonder if the check should instead look at Deployment target to pick the correct macro.

If CLJ 2.0 requires newer deployment targets then the podspec can be adjusted and this error will go away.

@danydev
Copy link

danydev commented Jan 20, 2015

Same here, pointing to last commit causes the reported error. Are you planning guys to fix it soon? I would love to avoid forking it for my current project! :)

@rivera-ernesto
Copy link
Member

We should then revert #420 @foozmeat ? For the foreseeable future we may have to support iOS 5/OSX 10.7.

@foozmeat
Copy link
Contributor

foozmeat commented Feb 3, 2015

You'll need to change the check to something like the following.

#if OS_OBJECT_HAVE_OBJC_SUPPORT || __IPHONE_OS_VERSION_MIN_REQUIRED >= 60000
  #define DISPATCH_QUEUE_REFERENCE_TYPE strong
#else
  #define DISPATCH_QUEUE_REFERENCE_TYPE assign
#endif

If you revert the patch then Cocoapods users using an SDK > 6.0 will get an error.

@bpoplauschi bpoplauschi added this to the 2.0.0 milestone Feb 9, 2015
@bpoplauschi
Copy link
Member

@foozmeat and @rivera-ernesto please review #451

@foozmeat
Copy link
Contributor

foozmeat commented Feb 9, 2015

I believe that will produce the desired result regardless of Cocoapods.

rivera-ernesto pushed a commit that referenced this issue Feb 10, 2015
Fix #433 CocoaLumberjack 2.0.0-rc does not build with Pods
@Legoless
Copy link
Author

Sadly this does not fix the issue. We build for iOS 7 and up, so this check does not make any difference really. I'd see a better solution to be: #if COCOAPODS for now..

@rivera-ernesto
Copy link
Member

I think we need to revert to the original #412 fix then.

@oshamsy
Copy link
Contributor

oshamsy commented Feb 13, 2015

#if OS_OBJECT_USE_OBJC solves the compile error under CocoaPods. I think this is a good solution for now.

However, on iOS >= 6.0 and OS X >= 10.8 we will have the dispatch_release() behavior. This is because the current podspec specifies a deployment target of iOS 5 and OS X 10.7.

One option to get around this is to create two podspecs:
A podspec for those requiring support for iOS >= 5 and OS X >= 10.7.
A second podspec for those requiring support for iOS >= 6.0 and OS X >= 10.8

@foozmeat
Copy link
Contributor

@Legoless I don't understand why #if OS_OBJECT_HAVE_OBJC_SUPPORT || __IPHONE_OS_VERSION_MIN_REQUIRED >= 60000 doesn't work for you. It should eval to true on ios 6 and above.

@oshamsy
Copy link
Contributor

oshamsy commented Feb 13, 2015

Yes, but CocoaPods is setting OS_OBJECT_USE_OBJC flag to 0 per the info at: http://guides.cocoapods.org/syntax/podspec.html#deployment_target
Which causes the dispatch_release() behavior.

This is all due to the podspec specifying a deployment target of iOS 5 and OS X 10.7.

@foozmeat
Copy link
Contributor

Understood, I posted the exact same information higher up this thread :)

#433 (comment)

That's why I suggested adding a check for the minimum OS required. Take a look at how OS_OBJECT_HAVE_OBJC_SUPPORT is defined in object.h.

#ifndef OS_OBJECT_HAVE_OBJC_SUPPORT
#if defined(__OBJC2__) && !defined(__OBJC_GC__) && ( \
        __MAC_OS_X_VERSION_MIN_REQUIRED >= __MAC_10_8 || \
        __IPHONE_OS_VERSION_MIN_REQUIRED >= __IPHONE_6_0)
#define OS_OBJECT_HAVE_OBJC_SUPPORT 1
#else
#define OS_OBJECT_HAVE_OBJC_SUPPORT 0
#endif
#endif

Checking OS_OBJECT_HAVE_OBJC_SUPPORT and __IPHONE_OS_VERSION_MIN_REQUIRED should provide the same result with or without Cocoapods.

@oshamsy
Copy link
Contributor

oshamsy commented Feb 13, 2015

Thanks. I missed your earlier comment. I agree. Should evaluate to the same with or without CocoaPods.

However, checking OS_OBJECT_HAVE_OBJC_SUPPORT || __IPHONE_OS_VERSION_MIN_REQUIRED >= 60000 is true for my iOS 8 target, and results in a strong declaration for the loggerQueue.

This is at odds with the compiler flag that CocoPods passes in -DOS_OBJECT_USE_OBJC=0. Hence the compile error: property with 'retain (or strong)' attribute must be of object type

@Legoless
Copy link
Author

@foozmeat The conditions all go through correctly, but apparently it wont compile GCD properties as strong, even so. So the issue is somewhere else, probably in Pod settings rather.

rivera-ernesto pushed a commit that referenced this issue Feb 13, 2015
Fix #433 CocoaLumberjack 2.0.0-rc does not build with Pods
@rivera-ernesto
Copy link
Member

Please test if #455 fixed this.

@oshamsy
Copy link
Contributor

oshamsy commented Feb 15, 2015

Fixed for me. I tested with OS X 10.7 and 10.10, as well as iOS 5.0 and 8.1.

@danydev
Copy link

danydev commented Feb 15, 2015

Fixed for me, tested with project that has as deployment target iOS 8.0

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

No branches or pull requests

8 participants