patches: Remove sys/cdefs.h header dependency

[meator]

Please see the description of the patch I added for an in length explanation.

Most distros dealing with musl already have a way around this. For example Alpine has the bsd-compat-headers and Chimera Linux has musl-bsd-headers, which provide a small simple sys/cdefs.h replacement. But it is a hack. Alpine's header even #warnings its users that it should not be included.

I have faced issues while trying to cross compile my fork on musl, since the target environment does not have sys/cdefs.h. I'm fairly certain that the same issue can occur in nmeum/android-tools, but it is a pain to cross compile nmeum/android-tools, I didn't want to cross compile all of its dependencies so I have not tested this.

Alternative solutions include:

• do nothing, musl users will have to install bsd-compat-headers or an equivalent package (which might sometimes be pulled in transitively, as it is pulled in Alpine CI)

  Cross sysroots will have to have sys/cdefs.h available too, which isn't mandatory and for example musl-cross-make, which is used by my fork, does not provide this header (since musl doesn't provide it).

• vendor a sys/cdefs.h file

  Theoretically, overriding this project-wide and vendoring an empty file should work fine. A more targetted solution would be adding this to include path only where it's needed (libbase/test_utils.cpp of libbase) and vendoring a sys/cdefs.h which has at least some of the stuff its users would expect (Chimera's would be a good pick).

  But if what I've written in the patch description is correct, sys/cdefs.h is only needed for potential __BIONIC__ macro definition, so an empty header file should be sufficient.

  Chimera's cdefs.h suitable for vendoring

  #ifndef _SYS_CDEFS_H_
  #define _SYS_CDEFS_H_
  
  #undef __P
  #undef __PMT
  
  #define __P(args)	args
  #define __PMT(args)	args
  
  #define __CONCAT(x,y)	x ## y
  #define __STRING(x)	#x
  
  #ifdef  __cplusplus
  # define __BEGIN_DECLS	extern "C" {
  # define __END_DECLS	}
  #else
  # define __BEGIN_DECLS
  # define __END_DECLS
  #endif
  
  #if defined(__GNUC__) && !defined(__cplusplus)
  # define __THROW	__attribute__ ((__nothrow__))
  # define __NTH(fct)	__attribute__ ((__nothrow__)) fct
  #else
  # define __THROW
  # define __NTH(fct)     fct
  #endif
  
  #define __CONCAT(x,y)   x ## y
  #define __STRING(x)     #x
  
  #endif /* _SYS_CDEFS_H_ */

• do __BIONIC__ detection in CMake instead of in the header

  This solution is described in the patch description.

This is a 37.0.0 issue, 36.0.1 and earlier did not use sys/cdefs.h.

[luk1337]

can you update patches/libbase/0002-remove-glibc-internal-headers-cdefs.h.patch instead?

[Biswa96]

libbase/test_utils.cpp file is not required in cmake. I will remove it later with an exclude rule before creating tarball.

[meator]

libbase/test_utils.cpp file is not required in cmake. I will remove it later with an exclude rule before creating tarball.

So you're saying that the problem can be solved by the following?

diff --git a/vendor/CMakeLists.libbase.txt b/vendor/CMakeLists.libbase.txt
index 08e0706..9210352 100644
--- a/vendor/CMakeLists.libbase.txt
+++ b/vendor/CMakeLists.libbase.txt
@@ -14,7 +14,6 @@ add_library(libbase STATIC
     libbase/stringprintf.cpp
     libbase/strings.cpp
     libbase/threads.cpp
-    libbase/test_utils.cpp
     libbase/errors_unix.cpp)
 
 target_include_directories(libbase PUBLIC

[luk1337]

libbase/test_utils.cpp file is not required in cmake. I will remove it later with an exclude rule before creating tarball.

So you're saying that the problem can be solved by the following?

diff --git a/vendor/CMakeLists.libbase.txt b/vendor/CMakeLists.libbase.txt
index 08e0706..9210352 100644
--- a/vendor/CMakeLists.libbase.txt
+++ b/vendor/CMakeLists.libbase.txt
@@ -14,7 +14,6 @@ add_library(libbase STATIC
     libbase/stringprintf.cpp
     libbase/strings.cpp
     libbase/threads.cpp
-    libbase/test_utils.cpp
     libbase/errors_unix.cpp)
 
 target_include_directories(libbase PUBLIC

it should.

[Biswa96]

So you're saying that the problem can be solved by the following?

Yes, I have tried it with both Chimera Linux and Void Linux (musl).

[meator]

I also compiled for Void glibc, alpine x86_64 -> ppc64le cross and Windows x86_64. I suspected that test_ files might not be necessary, but I assumed that it's there for a reason and did not investigate further. I did not do proper testing of this change, but it really looks like omitting this should cause no harm.

Can you push this to master? This change is not appropriate for this PR and it is simple enough that me making another PR should not be necessary. I'll also close this PR, but discussion can continue here.