Turicreate: Question about building Turi on 10.13.5

[avlaskin]

Hi,
I was following the guide for building Turi, but got some compilation errors on 10.13.5 using Xcode 9.4.

Errors (warnings removed):

/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/System/Library/Frameworks/CoreVideo.framework/Headers/CVPixelBuffer.h:256:205: error: array parameter is missing a nullability type
      specifier (_Nonnull, _Nullable, or _Null_unspecified) [-Werror,-Wnullability-completeness-on-arrays]
typedef void (*CVPixelBufferReleasePlanarBytesCallback)( void * CV_NULLABLE releaseRefCon, const void * CV_NULLABLE dataPtr, size_t dataSize, size_t numberOfPlanes, const void * CV_NULLABLE planeAddresses[] );
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/System/Library/Frameworks/CoreVideo.framework/Headers/CVPixelBuffer.h:286:40: error: array parameter is missing a nullability type
      specifier (_Nonnull, _Nullable, or _Null_unspecified) [-Werror,-Wnullability-completeness-on-arrays]
    void * CV_NULLABLE planeBaseAddress[],
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/System/Library/Frameworks/CoreVideo.framework/Headers/CVPixelBuffer.h:287:22: error: array parameter is missing a nullability type
      specifier (_Nonnull, _Nullable, or _Null_unspecified) [-Werror,-Wnullability-completeness-on-arrays]
    size_t planeWidth[],
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/System/Library/Frameworks/CoreVideo.framework/Headers/CVPixelBuffer.h:288:23: error: array parameter is missing a nullability type
      specifier (_Nonnull, _Nullable, or _Null_unspecified) [-Werror,-Wnullability-completeness-on-arrays]
    size_t planeHeight[],
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/System/Library/Frameworks/CoreVideo.framework/Headers/CVPixelBuffer.h:289:28: error: array parameter is missing a nullability type
      specifier (_Nonnull, _Nullable, or _Null_unspecified) [-Werror,-Wnullability-completeness-on-arrays]
    size_t planeBytesPerRow[],

78 warnings and 5 errors generated.
make[2]: *** [src/toolkits/image_deep_feature_extractor/CMakeFiles/image_deep_feature_extractor.dir/mlmodel_image_feature_extractor.mm.o] Error 1
make[1]: *** [src/toolkits/image_deep_feature_extractor/CMakeFiles/image_deep_feature_extractor.dir/all] Error 2

Clang:

Clang --version:
Apple LLVM version 9.1.0 (clang-902.0.30)
Target: x86_64-apple-darwin17.6.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

Any ideas what am I doing wrong?

[mdenty]

Your problem may be answered here : https://stackoverflow.com/questions/37691049/xcode-compile-flag-to-supress-nullability-warnings-not-working

Try to disable nullability-completeness-on-arrays.

[gustavla]

@avlaskin Sorry that you are having build issues on 10.13.5. I think @mdenty is onto what is happening, which is that we are using strict compilation flags that turn build warnings into build errors. This is great for keeping your code in good shape and finding bugs, but can also reduce the chance of a successful build. I will see if I can reproduce this today.

[gustavla]

This is strange. It looks like Core Video adds nullability only if the environment supports it. This is determined in CoreVideo.framework/Versions/A/Headers/CVBase.h:

#if (TARGET_OS_IPHONE || TARGET_OS_MAC) && defined(__has_feature) && __has_feature(nullability)
    #define COREVIDEO_DECLARE_NULLABILITY 1
#endif                                                                                         

For some reason, this #if must be evaluating to false on @avlaskin's environment, while still complaining that nullability is missing (so the environment obviously supports nullability).

[znation]

Open question: Do we still expect latest master to build on Xcode 9 (as of the 5.0b1 release), or do we now require Xcode 10? If the latter, let's document it in the build requirements. Either way, let's make sure that:

• This issue won't repro on 4.x branches with Xcode 9
• This issue won't repro on 5.x branches/current master with Xcode 10

And if this issue only repros on Xcode 9 on latest master, that may be a won't fix at this point. @nickjong, @hoytak, @gustavla, what are your thoughts on this? I am thinking it makes sense to restrict building to the latest Xcode (since that's the only build environment most of us are using), which should also work on the previous OS (so Xcode 10 on 10.13 should still work).

[igiloh]

Since Xcode 10 is still a beta, I would say no.
Even if we decide to support only latest Xcode - I wouldn't count beta versions in this definition.
Once 10.14 Mojave and Xcode 10 are officially release - we can stop supporting Xcode 9 (IMHO).

[znation]

Looking into this now.

[avlaskin]

@gustavla, here is how it is defined on 10.13.5. Essentially same definition:

#if (TARGET_OS_IPHONE || TARGET_OS_MAC) && defined(__has_feature) && __has_feature(nullability)
    #define COREVIDEO_DECLARE_NULLABILITY 1
#endif

I checked and can see that COREVIDEO_DECLARE_NULLABILITY is indeed defined by base code using the following construct in the CVPixelBuffer.h:

#if defined(COREVIDEO_DECLARE_NULLABILITY) && (COREVIDEO_DECLARE_NULLABILITY == 1)
    #warning "COREVIDEO_DECLARE_NULLABILITY is defined."
#else
    #warning "COREVIDEO_DECLARE_NULLABILITY is NOT defined."
#endif

Spills:

CVPixelBuffer.h:27:6: error: 
      "COREVIDEO_DECLARE_NULLABILITY is defined." [-Werror,-W#warnings]
    #warning "COREVIDEO_DECLARE_NULLABILITY is defined."

[gustavla]

@avlaskin That's strange! Then I don't understand why it's complaining that it is not getting nullability declarations. Can you do this around the same place?

#define STR_HELPER(x) #x                              
#define STR(x) STR_HELPER(x)                          
#pragma message("CV_NULLABILITY " STR(CV_NULLABILITY))

What does it say?

[avlaskin]

CoreVideo.framework/Headers/CVPixelBuffer.h:32:9: warning: CV_NULLABILITY CV_NULLABILITY
      [-W#pragma-messages]
#pragma message("CV_NULLABILITY " STR(CV_NULLABILITY))

And imho you wanted to check these: CV_NULLABLE :

warning: CV_NULLABLE _Nullable
      [-W#pragma-messages]
#pragma message("CV_NULLABLE " STR(CV_NULLABLE))

Which essentially means that it is defined in the expected way. In fact compiler seems complaining about different part of declaration. I know this is too wild, but I got it compiled using this modification (additions are CV_NULLABLE inside array brackets):

typedef void (*CVPixelBufferReleasePlanarBytesCallback)( void * CV_NULLABLE releaseRefCon, const void * CV_NULLABLE dataPtr, size_t dataSize, size_t numberOfPlanes, const void * CV_NULLABLE planeAddresses[CV_NULLABLE] );

CV_EXPORT CVReturn CVPixelBufferCreateWithPlanarBytes(
    CFAllocatorRef CV_NULLABLE allocator,
    size_t width,
    size_t height,
    OSType pixelFormatType,
    void * CV_NULLABLE dataPtr, // pass a pointer to a plane descriptor block, or NULL
    size_t dataSize, // pass size if planes are contiguous, NULL if not
    size_t numberOfPlanes,
    void * CV_NULLABLE planeBaseAddress[CV_NULLABLE],
    size_t planeWidth[CV_NULLABLE],
    size_t planeHeight[CV_NULLABLE],
    size_t planeBytesPerRow[CV_NULLABLE],
    ...

[gustavla]

@avlaskin That's some really great debugging, thanks!

You are right, it's only the arrays with [] it's complaining about and not the pointers with *. I had never seen the syntax for where to place the nullability in this setting before, but it looks like you are right that you place it inside the empty brackets. This alone suggests that these are genuinely missing from the CVPixelBuffer.h. Basically, it looks like it's complying with nullability except for -Wnullability-completeness-on-arrays.

However, I'm not sure how to reconcile this with the fact that I can't seem to repro this. If we can confirm this with a second repro, we can probably just add a well-placed exception for -Wnullability-completeness-on-arrays. However, it still feels like something is off here.

[znation]

@gustavla This repros for me on macOS 10.13.5 with Xcode 9.4. I'm working on a fix.

[avlaskin]

Pleasure to help, guys. Our iMac Pro has huge GPU inside and it's utilisation so far was 3%(mainly for text rendering). I am sure, it would be a big win for many people around the globe to make a use of our powerful GPUs using Turi Create.

[znation]

@avlaskin To clarify, GPU support in Turi Create requires macOS 10.14+ (which was announced at WWDC this year). Compiling on 10.13 won't help for this use case, as some of the Metal framework APIs we depend on are newly available in the 10.14 release. However, compiling on 10.13 will allow all other functionality to work as expected (including CPU execution of all toolkits).

[nickjong]

It looks like #675 may resolve this issue, too. My 10.13 partition is still on 10.13.4 and Xcode 9.3, but I cannot reproduce this issue after that fix.

Edit: scratch that, I can reproduce this issue now using public 10.13.5 and Xcode 9.4.1, with and without #675...

[znation]

I have a fix for this.

[avlaskin]

@znation Thanks for clarifiing that. Because the read me in section GPU support states:
"_Turi Create automatically utilizes Mac GPUs for the following tasks (requires macOS 10.13+):_"
However in section Version 5.0 it says :
"_GPU Acceleration on Macs (10.14+)_"
So it was not quite clear that Turi creat requires 10.14.

[znation]

Thanks for pointing that out @avlaskin. I updated the readme in https://github.com/apple/turicreate/pull/727 -- to clarify, GPU support on macOS requires 10.14+, but Turi Create will still work on 10.12 and 10.13, and fall back to CPU for those tasks.

[b-straub]

For me the the build problem described here is still open using Xcode 10b2 on OSX 10.14b2.
I have to add target_compile_options(image_deep_feature_extractor PUBLIC "-Wno-nullability-completeness") to the image_deep_feature_extractor make list to get this library build.

[znation]

@b-straub What's the commit hash you're on (where you can still repro this build issue)? I'll sync to that commit and investigate.

[nickjong]

Note also that this last round of build fixes may require you to run ./configure --cleanup on an existing checkout, or otherwise to invalidate the appropriate CMakeCache.txt

[znation]

I usually run git clean -f -d -x after making sure my repo doesn't contain any non-committed files I care about, to clean out the repo back to a fresh clone state. That's worth a try after syncing forward with any CMakeLists.txt changes and hitting build failures. @b-straub if the issue still repros for you after cleaning out the CMake cache, let us know what commit you're on and we'll try to repro it.

[b-straub]

Indeed after cleaning all the build is working.

[znation]

Good to know, thanks @b-straub! Sounds like we can leave this issue closed.