Closed Bjoe closed 6 years ago
I have seen there is a ticket in track https://svn.boost.org/trac10/ticket/13230 . It is most likely the same issue
@Bjoe Any idea what patch they're talking about in the issue you linked? They didn't seem to include a link to a github commit or pull request that I can see.
@rcdailey Hi. I think my patch .... see above: android-boost-1_65_1-patches.patch.txt .... I didn't create a pull request yet because my patch fixes the build issue (!) not the problem ! I'm still waiting for any contributors from the boost filesystem project to give me a answer, if I should create a pull request ... maybe in the next time I will created one...
Btw ... #56 and boost.filesystem compile problem on Android is the same issue
Update: I confirmed via response from an NDK developer that this issue is indeed a boost.filesystem problem, not an NDK bug.
When will this issue get some attention? It's been months.
Disclaimer: I'm not a Boost.Filesystem maintainer.
From the preprocessed output, there is no declaration of ::truncate
, which I think is supposed to be there regardless of whether we define _FILE_OFFSET_BITS
or __USE_FILE_OFFSET64
or not. This is part of POSIX API, and, if I understand correctly, it was also provided by an older NDK for the target Android API version you use. So, the fact that it is missing seems to me a bug in NDK.
Granted, we shouldn't define __USE_FILE_OFFSET64
ourselves as this is an internal macro specific to libc. The public POSIX macro is _FILE_OFFSET_BITS
, which is the one we should define. But this may be a workaround for some broken libc, I don't know. If this is __USE_FILE_OFFSET64
that breaks NDK then we should remove it in this case.
this is the mistake:
if I understand correctly, it was also provided by an older NDK for the target Android API version you use
no, it wasn't. in older NDKs, _FILE_OFFSET_BITS
and __USE_FILE_OFFSET64
were ignored. so you just used to get a 32-bit off_t and 32-bit functions regardless. the current NDK supports _FILE_OFFSET_BITS=64
, but at the cost of various degrees of support at different API levels.
if you want the old behavior back 100%, just don't define _FILE_OFFSET_BITS
/__USE_FILE_OFFSET64
if defined(__ANDROID__)
.
alternatively, at the cost of ABI issues on your side, you could check __ANDROID_API_LEVEL__
and enable __FILE_OFFSET_BITS=64
if you're being built targeting a new enough API level to have 64-bit versions of all the functions you need. but if you have any off_t
s in your ABI, that will be an ABI break.
That NDK removes pieces of otherwise available APIs when _FILE_OFFSET_BITS
is defined is a bug of NDK. It should either support this mode in full or not (in which case it should use 32-bit offsets throughout the API regardless of the macro).
I guess, we could consider _FILE_OFFSET_BITS
broken in the NDK and not define it. Although I wonder if other Android SDKs like CrystaX do not suffer from this problem. We might want to apply this workaround specifically for the Google NDK.
That NDK removes pieces of otherwise available APIs when _FILE_OFFSET_BITS is defined is a bug of NDK.
you think so (and it's a not entirely invalid argument), but others argue the opposite: that if they ask for _FILE_OFFSET_BITS=64
they should get as much as was available at their API level, because API 26 is a long time to wait for real-world app developers. (and this was, after all, how the Android platform worked during those years: with only a small subset of the functionality available. almost no-one needs it all.)
an argument in favor of the behavior we chose for the NDK is that it's trivial for dissenters to get what they want (just stop defining _FILE_OFFSET_BITS
) but non-trivial for the others to get partial support on old API levels (they'd have to touch a lot of different NDK header files).
Could someone test if https://github.com/boostorg/filesystem/pull/67 fixes the problem?
@enh I understand your reasoning but I disagree. When the user defines _FILE_OFFSET_BITS=64
he wants to have the full support for 64-bit offsets. In older API versions this is not possible to provide, so the _FILE_OFFSET_BITS=64
cannot and should not be supported. The interested parties that need particular 64-bit APIs that happen to be available in older API versions can already use off64_t
and accordingly named functions like lseek64
.
@Lastique I can confirm your changes fix the problem. I checked out your branch on your fork and compiled for the following platform using Android NDK:
Android NDK Version: r16b
Architecture: armeabi-v7a
API: 15
Toolchain: Clang
STL: libc++ (LLVM)
I applied your patch to boost 1.66.0 for my test. Patch file I used is available at the bottom of this post. Can we get this hotfixed into 1.66?
diff --git a/src/operations.cpp b/src/operations.cpp
index 035612c..df9aa42 100644
--- a/src/operations.cpp
+++ b/src/operations.cpp
@@ -10,7 +10,9 @@
//--------------------------------------------------------------------------------------//
-// define 64-bit offset macros BEFORE including boost/config.hpp (see ticket #5355)
+// define 64-bit offset macros BEFORE including boost/config.hpp (see ticket #5355)
+// Google Android NDK is broken though as it doesn't provide truncate() declaration when _FILE_OFFSET_BITS=64 is defined (https://github.com/boostorg/filesystem/issues/65)
+#if !defined(__ANDROID__) || (__ANDROID_API__+0) >= 21 || defined(__CRYSTAX__)
#if !(defined(__HP_aCC) && defined(_ILP32) && !defined(_STATVFS_ACPP_PROBLEMS_FIXED))
#define _FILE_OFFSET_BITS 64 // at worst, these defines may have no effect,
#endif
@@ -28,6 +30,7 @@
#else
#define _FILE_OFFSET_BITS 64
#endif
+#endif // !defined(__ANDROID__) || (__ANDROID_API__+0) >= 21 || defined(__CRYSTAX__)
// define BOOST_FILESYSTEM_SOURCE so that <boost/filesystem/config.hpp> knows
// the library is being built (possibly exporting rather than importing code)
@@ -44,7 +47,8 @@
#include <boost/filesystem/operations.hpp>
#include <boost/scoped_array.hpp>
#include <boost/detail/workaround.hpp>
-#include <vector>
+#include <limits>
+#include <vector>
#include <cstdlib> // for malloc, free
#include <cstring>
#include <cstdio> // for remove, rename
@@ -1615,6 +1619,12 @@ namespace detail
BOOST_FILESYSTEM_DECL
void resize_file(const path& p, uintmax_t size, system::error_code* ec)
{
+# if defined(BOOST_POSIX_API)
+ if (BOOST_UNLIKELY(size > static_cast< uintmax_t >((std::numeric_limits< off_t >::max)()))) {
+ error(system::errc::file_too_large, p, ec, "boost::filesystem::resize_file");
+ return;
+ }
+# endif
error(!BOOST_RESIZE_FILE(p.c_str(), size) ? BOOST_ERRNO : 0, p, ec,
"boost::filesystem::resize_file");
}
Now when #67 has been merged, this issue can be closed, I think.
Yep. I close this. Thanks @alexeikh
Boost 1.65.1 Compiler: clang from android NDK clang_version__ "5.0.300080 " NDK version: 16.0.4442984 Build system: cmake (3.6.4111459) Std librariy: libc++_static Build machine: Ubuntu 16.04.3 LTS
Since Google introduced unified headers the following compile error occurs to build boost::filesystem for example Android Level 19:
Before they introduced the unified headers the header file unistd.h didn't care about the defines _FILE_OFFSET_BITS or __USE_FILE_OFFSET64. Here a snippit from unistd.h:
Since they introduced unified headers, unistd.h snippet:
Now they care about the define __USE_FILE_OFFSET64 and truncate for 64 Bit is not implemented before Android API level 21.
We compile and use boost::filesystem for Android API level 19 long time ago and we didn't have any problems. So I decided to "if def" the define of __USE_FILE_OFFSET64 around (see snippet):
to skip this so that we have the same behavior as before, I think.
I create a patch file but I will not create a pull request because I think this is not the right solution and you should decided if this is a proper fix or not. Then I can create a pull request, if needed.
I added the patch file and a preprocessor output of the compile error in operations.cpp
operations.ii.txt
android-boost-1_65_1-patches.patch.txt
You need some help or additional information, let me know.