Fix build on OS X #159

Merged
merged 4 commits into from Nov 8, 2013

2 participants

@rivera-ernesto
CocoaLumberjack member

#156 and #121 were not tested on OS X.

  • Fix build on OS X
  • Test for OS X 10.9
  • Stop ignoring deprecation warnings.
@rivera-ernesto
CocoaLumberjack member

Came up with three cases:

a) Newer SDK's (iOS 7+/OS X 10.9+) on newer (iOS 7.0+/OS X 10.9+) runtime
queueLabel = dd_str_copy(dispatch_queue_get_label(DISPATCH_CURRENT_QUEUE_LABEL));

b) Systems where dispatch_get_current_queue is not yet deprecated and won't crash (< iOS 6.0/OS X 10.9)

dispatch_queue_t currentQueue = dispatch_get_current_queue();
queueLabel = dd_str_copy(dispatch_queue_get_label(currentQueue));

c) Give up
queueLabel = dd_str_copy("");

Any comments? @robbiehanson, @bpoplauschi, @kolyuchiy, @sprhawk?

@bpoplauschi
CocoaLumberjack member

I think DISPATCH_CURRENT_QUEUE_LABEL is not defined for OS X

@rivera-ernesto
CocoaLumberjack member

From MacOSX.platform/Developer/SDKs/MacOSX10.9.sdk/usr/include/dispatch/queue.h:

/*!
 * @const DISPATCH_CURRENT_QUEUE_LABEL
 * @discussion Constant to pass to the dispatch_queue_get_label() function to
 * retrieve the label of the current queue.
 */
#define DISPATCH_CURRENT_QUEUE_LABEL NULL

/*!
 * @function dispatch_queue_get_label
 *
 * @abstract
 * Returns the label of the given queue, as specified when the queue was
 * created, or the empty string if a NULL label was specified.
 *
 * Passing DISPATCH_CURRENT_QUEUE_LABEL will return the label of the current
 * queue.
 *
 * @param queue
 * The queue to query, or DISPATCH_CURRENT_QUEUE_LABEL.
 *
 * @result
 * The label of the queue.
 */
__OSX_AVAILABLE_STARTING(__MAC_10_6,__IPHONE_4_0)
DISPATCH_EXPORT DISPATCH_PURE DISPATCH_WARN_RESULT DISPATCH_NOTHROW
const char *
dispatch_queue_get_label(dispatch_queue_t queue);
@bpoplauschi
CocoaLumberjack member

I can't find it. Could you try building Xcode/LumberjackFramework/Desktop just to be sure?

@rivera-ernesto rivera-ernesto commented on the diff Nov 8, 2013
Lumberjack/DDLog.m
@@ -877,41 +878,42 @@ - (instancetype)initWithLogMsg:(NSString *)msg
machThreadID = pthread_mach_thread_np(pthread_self());
- // dispatch_get_current_queue() is deprecated and most importantly it
- // crashes sometimes.
-
-#if __IPHONE_OS_VERSION_MAX_ALLOWED >= __IPHONE_7_0
- // If compiling with iOS 7.0+ SDK for any deployment target
- if ([[[UIDevice currentDevice] systemVersion] compare:@"7.0" options:NSNumericSearch] != NSOrderedAscending) {
- // If runtime environment is iOS 7.0+
+ // Try to get the current queue's label
+
+ // a) Compiling against newer SDK's (iOS 7+/OS X 10.9+) where DISPATCH_CURRENT_QUEUE_LABEL is defined
+ // on a (iOS 7.0+/OS X 10.9+) runtime version
+ BOOL gotLabel = NO;
+ #ifdef DISPATCH_CURRENT_QUEUE_LABEL
@rivera-ernesto
CocoaLumberjack member
rivera-ernesto added a line comment Nov 8, 2013

It builds, but it would anyway because of this check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@bpoplauschi
CocoaLumberjack member

You're correct. I meant to say that because DISPATCH_CURRENT_QUEUE_LABEL doesn't exist on Mac OS, the Mac path from case a) is never executed. If you see DISPATCH_CURRENT_QUEUE_LABEL defined on Mac, could you tell me in which file?

@bpoplauschi
CocoaLumberjack member

Disregard my last comment. I just saw your definition of DISPATCH_CURRENT_QUEUE_LABEL. I'm still running Mountain Lion, that's why it's undefined for me.

@rivera-ernesto
CocoaLumberjack member

I'll have to leave now but you and @kolyuchiy can work directly on this pull request if you feel like (added you as collaborators in the fork).

@bpoplauschi
CocoaLumberjack member

@rivera-ernesto The pull request sounds right to me. I will go ahead and merge it.

@bpoplauschi bpoplauschi merged commit bf7f218 into CocoaLumberjack:master Nov 8, 2013
@bpoplauschi
CocoaLumberjack member

@robbiehanson @kolyuchiy @sprhawk you guys should still review and comment

@kolyuchiy kolyuchiy commented on the diff Nov 8, 2013
Lumberjack/DDLog.m
-
-#if __IPHONE_OS_VERSION_MAX_ALLOWED >= __IPHONE_7_0
- // If compiling with iOS 7.0+ SDK for any deployment target
- if ([[[UIDevice currentDevice] systemVersion] compare:@"7.0" options:NSNumericSearch] != NSOrderedAscending) {
- // If runtime environment is iOS 7.0+
+ // Try to get the current queue's label
+
+ // a) Compiling against newer SDK's (iOS 7+/OS X 10.9+) where DISPATCH_CURRENT_QUEUE_LABEL is defined
+ // on a (iOS 7.0+/OS X 10.9+) runtime version
+ BOOL gotLabel = NO;
+ #ifdef DISPATCH_CURRENT_QUEUE_LABEL
+ if (
+ #if TARGET_OS_IPHONE
+ [[[UIDevice currentDevice] systemVersion] compare:@"7.0" options:NSNumericSearch] != NSOrderedAscending // 7.0+
+ #else
+ [[NSApplication sharedApplication] respondsToSelector:@selector(occlusionState)] // No nice way to check for OS X 10.9+
@kolyuchiy
kolyuchiy added a line comment Nov 8, 2013

Does this bug actually reproduces on Mac OS? Maybe we don't have to check and just use dispatch_queue_get_label(currentQueue)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@rivera-ernesto
CocoaLumberjack member

I think that avoiding deprecated methods is good even if they don't crash on OS X.

@rivera-ernesto rivera-ernesto deleted the rivera-ernesto:fixosx_removeignoredeprecations branch Nov 11, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment