Fix compilation for Android and clean up _FILE_OFFSET_BITS #if-ery

[alexeikh]

This pull requests cleans up _FILE_OFFSET_BITS #if-ery in operations.cpp, fixes compilation with new Android NDKs for old Android APIs (issue #65) and, thanks to @Lastique's patch, provides better error checking in resize_file() function.

Differences from other pull requests, like #52 and #67:

• Instead of taking easy approach like

#if !MY_SETUP
// I don't care about the mess here.
#endif 

, which may eventually lead to #if-ery like

#if !SETUP_3
#if !SETUP_2
#if !SETUP_1
...
#else
...
#endif
#endif 
#endif 

, I am making attempt to clean the mess and provide "flat" #if-ery like

#if SETUP_1
...
#elif SETUP_2
...
#else
...
#endif

• During the cleanup, it turns out the either _FILE_OFFSET_BITS, or __USE_FILE_OFFSET64, or both (!) is always defined, thus it is possible to greatly simplify the #if-ery.

• I am checking if __ANDROID_API__ is defined. Reason: older Android NDKs, that use non-unified headers, do not define __ANDROID_API__.

• Unlike operations: Don't set _FILE_OFFSET_BITS for older Android versions #52, I am including @Lastique's patch for resize_file() from Don't define _FILE_OFFSET_BITS=64 on Google Android NDK up to API level 21 #67.

• Unlike Don't define _FILE_OFFSET_BITS=64 on Google Android NDK up to API level 21 #67, I do not define _FILE_OFFSET_BITS if compiling with Crystax Android NDK. The reason is that I haven't found any proof that Crystax will somehow provide 64-bit truncate for Android APIs < 21. On the contrary, I found that on Crystax off_t is 32-bit: https:/crystax/android-platform-bionic#lp32-abi-bugs.

[rcdailey]

Thanks for the fixes. Which release of boost will this be available in? Will we have to wait for a 1.67 or can we get it in a hotfix 1.66.1?

[Lastique]

IMO, 1.66.1 is highly unlikely since 1.67 is on the way. I would hope someone with permissions will be able to merge it to develop and then to master before 1.67 is out, but no guarantees on that.

[MaartenBent]

This doesn't work with toolchains 21, 22 and 23, failing with error:

C:\Android\toolchain21\bin/../lib/gcc/arm-linux-androideabi/4.9.x/../../../../include/c++/4.9.x\cstdio:107:11: error: no member named 'fgetpos' in the global namespace
  using ::fgetpos;
        ~~^
C:\Android\toolchain21\bin/../lib/gcc/arm-linux-androideabi/4.9.x/../../../../include/c++/4.9.x\cstdio:117:11: error: no member named 'fsetpos' in the global namespace
  using ::fsetpos;
        ~~^
2 errors generated.

Changing the guard to __ANDROID_API__ < 24 fixes this.

See also this documentation that mentions API 24 is the first API that has all functions included in <stdio.h>.

[alexeikh]

@MaartenBent, thanks for the heads up.

The issue with fgetpos and fsetpos seems to be this one: android/ndk#480 . Android NDK developers fixed it in r16 with this patch: https://android.googlesource.com/platform/ndk/+/d5acdb0ee5ad536d543b509d83cfa79b9d01b3b1 , but only for libc++. I think, they did it as an exception, because compilation failure on just #include <cstdio> is really ridiculous. They refuse to fix it for gnustl, because they are deprecating gnustl in favor of libc++.

It means that the issue still exists for Android APIs 21-23 on:

• NDK r16 and above with gnustl.
• NDK r14 and r15 with any STL and enabled Unified Headers.

@MaartenBent, judging from you error messages, you are using gnustl.

@MaartenBent, I agree, it's better to change the guard to __ANDROID_API__ < 24. I will update the pull request.

It's possible to get 64-bit truncate for APIs 21-23, but it seems to require some complicated logic. I am not sure it's worth the hassle. And I don't want to further complicate and delay the review of this pull request. If anyone needs 64-bit truncate particularly for API range 21-23, I think he/she should make it as a separate pull request.

[Bjoe]

It compiles with android API level 19 and the verification if the file is not to big for resizing does indeed looks like a good fix, so I can execute my App on an Android with a higher API level (with 64-Bit support).

With the last change, is this really sure that this is compiling with gnustl and API level 24? And when I check the Roadmap from Android they planned:

NDK r17
Estimated release: Q1 2018
Default to libc++

NDK r18
Estimated release: Q2 2018
Remove non-libc++ STLs
Remove GCC

So Q2/2018 it's not far away... gnustl will be removed.
Maybe you add another check which STL libraries is used like
(__ANDROID_API__ < 21 && _LIBCPP_VERSION) || (__ANDROID_API__ < 24 && __GLIBCXX__)
or something like that. See also Pre-defined C/C++ Compiler Macros
Btw ... should I close #65 ?

[MaartenBent]

With the last change, is this really sure that this is compiling with gnustl and API level 24

For me it compiles successfully with API 21 and API 24. I do not define the stl when creating the toolchain so gnustl is used as default.

Maybe you add another check which STL libraries is used like

This doesn't work because __GLIBCXX__ is not yet defined, from your link:

One of the standard header files must be included before any of the following macros are defined.

After moving #include <cstdlib> to the beginning of the file it works. I did not check if _LIBCPP_VERSION is defined correctly when using libc++, or if moving the include has any side effects.

[Lastique]

After you included a system header, it's already too late to define _FILE_OFFSET_BITS.

[alexeikh]

Btw ... should I close #65 ?

@Bjoe: No, of course not. First, you have very good description of the issue. With relevant error messages and code snippets. Second, if you close the issue - it may confuse people, someone may get an idea that the issue is fixed.

[alexeikh]

Dear Boost.Filesystem maintainers, @Beman and @pdimov.

I submitted this pull request several days ago. How can I get it reviewed?

[alexeikh]

Congratulations with the International Space Day, everyone. According to the Git logs, @Beman was on Earth 2 days ago and made some commits to the repository.

On this nice day, can I ask @Beman for some attention to this pull request?

[alexeikh]

This pull request is waiting for review 2 months already.

@Beman , @pdimov : Boost 1.67 is out, maybe now you have time to review and merge the patch?

Maybe you see some issue in the patch? And have suggestion for improvement?

[pdimov]

Looks good to me. If @Beman doesn't object, I'll merge that in a few days.

[alexeikh]

Hurrah! :) Thanks for the merging!