Don't include inttypes if compiling for Mac/iOS

[pietbrauer]

In ObjectiveGit have recurring problems (see libgit2/objective-git#436 (comment) and libgit2/objective-git#542) with Xcode complaining with Include of non-modular header inside framework module 'ObjectiveGit.git2.sys.filter' this is due to the fact that libgit2 is including inttypes.h in common.h. There is no need to import this header on iOS and Mac platforms and thus I excluded it.

This fixes our recurring compilation issues and still works. I am happy to discuss this change as it has some implications. I also find this way of solving the problem pretty naive but after days of trying is the only solution I could come up with.

[carlosmn]

This seems dodgy. This is the system-provided header, which is used to build libgit2 itself. If you're calling it from something else, this shouldn't have any effect.

I would rather recommend stopping the inclusion of this file in your build system if it's causing those issues by defining _INTTYPES_H_.

[pietbrauer]

It's included in the simulator environment there is no way I can stop it. 😬

[carlosmn]

If you can't stop this file from being included in the build, how does libgit2 not asking for it help? Is libgit2 having th #include statement causing the issue rather than the contents of the file? If the file is included by a different step (or prepended by the build system in some way) the it would have defined the include guard and thus this would be a no-op.

[pietbrauer]

The problem is that frameworks should not include external files in the header. I only know of this restriction in OS X/iOS frameworks. Other libraries have solved this issue by moving the import/include into the implementation file (.m or .c).

[carlosmn]

So it is the text of the include. That seems really odd.

But this isn't the only header we include. We also #include <sys/types.h> in git2/types.h. Why isn't that an issue?

Even so, I would much rather not rely on the OS specificiation, as this kind of thing changes with the whims of Apple's dev tools. We should be checking for _INTTYPES_H_ being defined and include it if not. There should be a comment explaining why we can't just let the header do it itself.

[pietbrauer]

Yeah, I tried that and it is including it because it is not defined before that so it is included and therefore generates the error.

it seems to me that the types that are needed (not sure which, actually) are already defined and thus the import can be omitted.

As I said other people have solved this by moving the imports. https://github.com/AFNetworking/AFNetworking/issues/2205

[pietbrauer]

Ok, I updated the PullRequest, it seems to work if I check for defined(_INTTYPES_H_) (I tried with defined(INCLUDE_INTTYPES_H_). Is that ok with you?

[carlosmn]

Yes, _INTTYPES_H_ is the right one, which is the one defined by the header. But because that's exactly the very first check in the header file, there should be a comment about why it's necessary for us to do it. Otherwise we'll likely remove it at some point.

[pietbrauer]

👍 makes sense, added a comment.

[carlosmn]

I asked around and it looks like this come from clang getting fancy and introducing modules in C. They (fortunately) accept that it's not realistic to change all the libraries we already have, so they allow for user-defined mappings between the include and the module. It unfortunately doesn't seem to include <inttypes> but that should be the way to go. Either here or in objective-git.

[pietbrauer]

Yeah, I think thats the underlying issue. Are you referring to introducing a modulemap?

[carlosmn]

This is a system header, so it should be provided by the OS, but yeah, libgit2 or objective-git would presumably have to tell clang what to translate that header to in modules.

[carlosmn]

/cc @joshaber where would be the place to add this kind of mapping? Should this be in the objective-git project? Or do we have to make clang happy ourselves?

[pietbrauer]

The problem is you can't tell the module map something like:

module std {
  header <inttypes.h>
}

It will just complain that <> is not allowed. If you use header "inttypes.h" instead if complains that it does not know where that is.

[joshaber]

@carlosmn Sadly I don't know much about clang modules except that they're a world of pain 😞

[pietbrauer]

I dug a bit more into this and found out that clang inttypes gets imported too and checking for it resolves the problem in objective-git.

[carlosmn]

If clang is making it so hard to do anything else, let's go with the current version. Could you squash this all together into a single commit?

[pietbrauer]

@carlosmn Done!