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

DDTTYLoggeer's AppKit detection doesn't work & Path to CLIColor.h include wrong #488

Closed
weissi opened this issue Apr 1, 2015 · 4 comments

Comments

@weissi
Copy link
Contributor

weissi commented Apr 1, 2015

Problem: AppKit Dependency

CocoaLumberjack's DDTTYLoggeer is in theory able to work without having AppKit.framework linked. It tries to check whether AppKit is linked by using #elif __has_include(<AppKit/NSColor.h>). Unfortunately this doesn't work, Apple's clang seems to always find the AppKit includes even if it's using AppKit (verified using Xcode 6.1 and Xcode 6.3 beta 4's clang versions).

The following demo program

$ cat /tmp/test.m
#if __has_include(<AppKit/NSColor.h>)
#    error AppKit available
#endif
int main() {
    return 0;
}

compiled as

$ clang -o /tmp/test /tmp/test.m
/tmp/test.m:2:6: error: AppKit available
#    error AppKit available
     ^
1 error generated.

does say the AppKit header files are available but there's no -framework AppKit or anything else passed on clang's command line.

Therefore, I think CocoaLumberjack's #elif __has_include(<AppKit/NSColor.h>) will always be true on OS X and which leads to the #else case (where CLIColor.h is included) to never be used. That creates a dependency on AppKit in DDLog.

Problem: Path to CLIColor.h

Additionally the code to use the CLIColor version is

#else
    // OS X CLI
    #import "CLIColor.h"
    #define DDColor CLIColor
    #define DDMakeColor(r, g, b) [CLIColor colorWithCalibratedRed:(r/255.0f) green:(g/255.0f) blue:(b/255.0f) alpha:1.0f]
#endif

but the #import to CLIColor.h should really be CLI/CLIColor.h otherwise it won't find CLIColor.h.

Possible Solution

  • A preprocessor flag DD_NO_APPKIT or DD_USE_CLICOLOR or similar that DDTTYLogger would be compiled with CLIColor instead of NSColor to remove the AppKit dependency
@rivera-ernesto
Copy link
Member

Couldn't it be that AppKit is getting added somewhere else in the project? Build steps, @import's, etc.?

@weissi
Copy link
Contributor Author

weissi commented Apr 1, 2015

@rivera-ernesto I'm just compiling one file (/tmp/test.m) from the command line. There's no project or anything else involved.

Try it for yourself and just paste this:

cat > /tmp/test.m <<EOF
#if __has_include(<AppKit/NSColor.h>)
#    error AppKit available
#endif
int main() {
    return 0;
}
EOF
xcrun clang -c /tmp/test.m

@rivera-ernesto
Copy link
Member

I see. What about he included CLI demo?

What about something like

#ifdef DD_CLI

Then people can define DD_CLI before importing the library to force "CLI-mode".

As for that path, I think that is because you're not using a project. We could modify as you propose but it may break "regular" projects. Or we can use the same DD_CLI to import it using a different path.

Also DD_CLI should probably be named something more expressive.

weissi added a commit to weissi/CocoaLumberjack that referenced this issue Apr 18, 2015
@weissi
Copy link
Contributor Author

weissi commented Apr 18, 2015

@rivera-ernesto Sorry for the late reply, I'm proposing PR #497 to fix this issue. Let me know what you think.

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

No branches or pull requests

2 participants