Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
concatime opened this issue Oct 25, 2021 · 19 comments · May be fixed by #580
Open

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

concatime opened this issue Oct 25, 2021 · 19 comments · May be fixed by #580
Assignees

Comments

@concatime
Copy link

concatime commented Oct 25, 2021

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
Copy link
Author

concatime commented Oct 25, 2021

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
Copy link
Author

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 concatime changed the title libsync includes sys/cdefs.h libsync includes the nonexistant sys/cdefs.h in musl Oct 25, 2021
@rakuco rakuco changed the title libsync includes the nonexistant sys/cdefs.h in musl chromium: libsync includes the nonexistant sys/cdefs.h in musl Oct 25, 2021
@concatime
Copy link
Author

concatime commented Oct 25, 2021

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
Copy link
Collaborator

kraj commented Oct 27, 2021

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
Copy link
Author

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
Copy link
Author

concatime commented Oct 29, 2021

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
Copy link
Author

concatime commented Oct 29, 2021

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
Copy link
Author

concatime commented Oct 30, 2021

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
Copy link
Collaborator

kraj commented Oct 30, 2021

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
Copy link
Author

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
Copy link
Collaborator

kraj commented Oct 30, 2021

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
Copy link
Author

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.

concatime added a commit to concatime/meta-browser that referenced this issue Oct 30, 2021
@concatime concatime linked a pull request Oct 30, 2021 that will close this issue
concatime added a commit to concatime/meta-browser that referenced this issue Oct 30, 2021
concatime added a commit to concatime/meta-browser that referenced this issue Dec 5, 2021
concatime added a commit to concatime/meta-browser that referenced this issue Jul 3, 2022
@LEDZB
Copy link

LEDZB commented Jan 18, 2023

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
Copy link
Collaborator

kraj commented Jan 18, 2023

@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
Copy link

LEDZB commented Jan 18, 2023

@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
Copy link
Collaborator

kraj commented Jan 18, 2023

@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
Copy link

LEDZB commented Jan 18, 2023

@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
Copy link
Author

@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
Copy link

LEDZB commented Jan 29, 2023

@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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

3 participants