OSSystems / meta-browser

OpenEmbedded/Yocto BSP layer for Web Browsers
MIT License
186 stars 195 forks source link

chromium: libsync includes the nonexistant `sys/cdefs.h` in musl #578

Open concatime opened 3 years ago

concatime commented 3 years ago

Hi, when building Chromium v94.0.4606.61, I get:

In file included from ../chromium-94.0.4606.61/third_party/libsync/src/sync.c:30:
../chromium-94.0.4606.61/third_party/libsync/src/include/sync/sync.h:22:10: fatal error: 'sys/cdefs.h' file not found
#include <sys/cdefs.h>
         ^~~~~~~~~~~~~

Shouldn’t there be a patch for libsync in the musl directory too ?

Also... thank you for the awesomeness of this project, very helpful!

concatime commented 3 years ago

Something along these lines:

diff --git a/third_party/libsync/src/include/sync/sync.h b/third_party/libsync/src/include/sync/sync.h
index 50ed0ac5..014ea72d 100644
--- a/third_party/libsync/src/include/sync/sync.h
+++ b/third_party/libsync/src/include/sync/sync.h
@@ -19,12 +19,13 @@
 #ifndef __SYS_CORE_SYNC_H
 #define __SYS_CORE_SYNC_H

-#include <sys/cdefs.h>
 #include <stdint.h>

 #include <linux/types.h>

-__BEGIN_DECLS
+#ifdef __cplusplus
+extern "C" {
+#endif

 struct sync_legacy_merge_data {
  int32_t fd2;
@@ -158,6 +159,8 @@ struct sync_pt_info *sync_pt_info(struct sync_fence_info_data *info,
                                   struct sync_pt_info *itr);
 void sync_fence_info_free(struct sync_fence_info_data *info);

-__END_DECLS
+#ifdef __cplusplus
+}
+#endif

 #endif /* __SYS_CORE_SYNC_H */
concatime commented 3 years ago

Another issue:

../chromium-94.0.4606.61/base/third_party/libevent/epoll.c:39:10: fatal error: 'sys/queue.h' file not found
#include <sys/queue.h>
         ^~~~~~~~~~~~~

solvable with:

diff --git a/base/third_party/libevent/BUILD.gn b/base/third_party/libevent/BUILD.gn
index abd3901d..ae2f020f 100644
--- a/base/third_party/libevent/BUILD.gn
+++ b/base/third_party/libevent/BUILD.gn
@@ -49,7 +49,7 @@ static_library("libevent") {
       "linux/config.h",
       "linux/event-config.h",
     ]
-    include_dirs = [ "linux" ]
+    include_dirs = [ "linux", "compat" ]
   } else if (is_android) {
     sources += [
       "android/config.h",
concatime commented 3 years ago

Same for:

../chromium-94.0.4606.61/third_party/crashpad/crashpad/compat/linux/sys/ptrace.h:20:10: fatal error: 'sys/cdefs.h' file not found
#include <sys/cdefs.h>
         ^~~~~~~~~~~~~

Fixable with:

diff --git a/third_party/crashpad/crashpad/compat/linux/sys/ptrace.h b/third_party/crashpad/crashpad/compat/linux/sys/ptrace.h
index f8be372c..c07e4c88 100644
--- a/third_party/crashpad/crashpad/compat/linux/sys/ptrace.h
+++ b/third_party/crashpad/crashpad/compat/linux/sys/ptrace.h
@@ -17,8 +17,6 @@

 #include_next <sys/ptrace.h>

-#include <sys/cdefs.h>
-
 // https://sourceware.org/bugzilla/show_bug.cgi?id=22433
 #if !defined(PTRACE_GET_THREAD_AREA) && !defined(PT_GET_THREAD_AREA) && \
     defined(__GLIBC__)
kraj commented 3 years ago

I built it for qemux86-64/musl using master and it built fine and I am on 94.0.4606.71 not 94.0.4606.61

concatime commented 3 years ago

I will try tomorrow with .71, but I don’t think it’s related. I rather think that I build chromium with a different set of flags. So maybe in your config, e.g. nasm is never built so you never face the compilation error. Same goes for libsync and libevent.

concatime commented 3 years ago

Or, if you are on Alpine, maybe you have the package bsd-compat-headers w/o even knowing? Because musl definitely does not include sys/cdefs.h as stated in their FAQ.

concatime commented 3 years ago

One issue with nasm:

ld.lld: error: undefined symbol: canonicalize_file_name
>>> referenced by realpath.c:58 (../chromium-94.0.4606.61/third_party/nasm/nasmlib/realpath.c:58)
>>>               obj/third_party/nasm/nasm/realpath.o:(nasm_realpath)
clang-13: error: linker command failed with exit code 1 (use -v to see invocation)

Here is the patch:

diff --git a/third_party/nasm/config/config-linux.h b/third_party/nasm/config/config-linux.h
index 9e59df4f..b96673ac 100644
--- a/third_party/nasm/config/config-linux.h
+++ b/third_party/nasm/config/config-linux.h
@@ -139,7 +139,7 @@
 #define HAVE_ACCESS 1

 /* Define to 1 if you have the `canonicalize_file_name' function. */
-#define HAVE_CANONICALIZE_FILE_NAME 1
+/* #undef HAVE_CANONICALIZE_FILE_NAME */

 /* Define to 1 if you have the `cpu_to_le16' intrinsic function. */
 /* #undef HAVE_CPU_TO_LE16 */
concatime commented 3 years ago

I confirm. With these four patches (nasm, crashpad, libsync and libevent) for musl, I’m able (~ish, but that’s for another story) to compile chromium. Would you accept if I submit them, @kraj ?

kraj commented 3 years ago

I confirm. With these three patches (nasm, libsync and libevent) for musl, I’m able (~ish, but that’s for another story) to compile chromium. Would you accept if I submit them, @kraj ?

Yes I will but I am still not sure why The build is working for me but not for You I am using arch Linux host and Ubuntu 18.04 hosts to build and it always builds and runs fine for qemux86-64/musl so help us understand how to reproduce the problem

concatime commented 3 years ago

I don’t know, but let’s try.

First, verify that neither sys/cdefs.h nor sys/queue.h are in your QEMU system.

For libevent:

So, for libevent to be built, you need one of dep_libevent, use_libevent and rtc_build_libevent variables to be true.

dep_libevent =
    !is_fuchsia && !is_win && !is_mac && !(is_nacl && !is_nacl_nonsfi)

# Determines whether message_pump_libevent should be used.
use_libevent = dep_libevent && !is_ios
  # Enable libevent task queues on platforms that support it.
  if (is_win || is_mac || is_ios || is_nacl || is_fuchsia ||
      target_cpu == "wasm") {
    rtc_enable_libevent = false
    rtc_build_libevent = false
  } else {
    rtc_enable_libevent = true
    rtc_build_libevent = !build_with_mozilla
  }
kraj commented 3 years ago

Can you try pasting output of bitbake -e chromium-x11 | grep ^DEPENDS=

and if bsd-headers is missing there then please add

DEPENDS:append:libc-musl = " bsd-headers"

to chromium-gn.inc and see if it helps.

concatime commented 3 years ago

Oh, sorry, I wasn’t clear. I’m building chromium myself, a.k.a w/o bitbake. But I’m using your awesome set of patches. Ideally, the patches should cover any build systems, and not rely on a specific one.

LEDZB commented 1 year ago

It means that the latest musl library does not have the library file <sys/cdefs.h>. I checked under the corresponding directory and found that it was not found.

kraj commented 1 year ago

@LEDZB @concatime I understand the problem now. In openembedded we install few BSD headers via bsd-headers package which provides sys/cdefs.h on OpenEmbedded builds thats why we do not see these errors, therefore these patches were not needed with OpenEmbedded/musl builds.

I am fine apply these patches if you can forward port them to current master and ensure that they are still needed for your builds and can apply cleanly.

LEDZB commented 1 year ago

@LEDZB @concatime I understand the problem now. In openembedded we install few BSD headers via bsd-headers package which provides sys/cdefs.h on OpenEmbedded builds thats why we do not see these errors, therefore these patches were not needed with OpenEmbedded/musl builds.

I am fine apply these patches if you can forward port them to current master and ensure that they are still needed for your builds and can apply cleanly.

I seem to understand what you mean, so when I need to use the functions that were included in <sys/cdefs.h> before, I don’t need to include <sys/cdefs.h> now? Just need to use it directly? Is my understanding correct

kraj commented 1 year ago

@LEDZB @concatime I understand the problem now. In openembedded we install few BSD headers via bsd-headers package which provides sys/cdefs.h on OpenEmbedded builds thats why we do not see these errors, therefore these patches were not needed with OpenEmbedded/musl builds. I am fine apply these patches if you can forward port them to current master and ensure that they are still needed for your builds and can apply cleanly.

I seem to understand what you mean, so when I need to use the functions that were included in <sys/cdefs.h> before, I don’t need to include <sys/cdefs.h> now? Just need to use it directly? Is my understanding correct

your build system sysroot should have sys/cdefs.h and you can use one pointed above if it does not.

LEDZB commented 1 year ago

@LEDZB @concatime I understand the problem now. In openembedded we install few BSD headers via bsd-headers package which provides sys/cdefs.h on OpenEmbedded builds thats why we do not see these errors, therefore these patches were not needed with OpenEmbedded/musl builds. I am fine apply these patches if you can forward port them to current master and ensure that they are still needed for your builds and can apply cleanly.

I seem to understand what you mean, so when I need to use the functions that were included in <sys/cdefs.h> before, I don’t need to include <sys/cdefs.h> now? Just need to use it directly? Is my understanding correct

your build system sysroot should have sys/cdefs.h and you can use one pointed above if it does not.

Ok, I understand what you mean, thank you very much

concatime commented 1 year ago

@kraj to be honest, I don't have time right now to submit and test a PR. But the patches linked above should work just fine.

LEDZB commented 1 year ago

@kraj to be honest, I don't have time right now to submit and test a PR. But the patches linked above should work just fine.

The patch you mentioned means delete <sys/cdefs.h> from the function library written by yourself?