aristocratos / btop

A monitor of resources
Apache License 2.0
21.38k stars 656 forks source link

[BUG] Build broken on Catalina and below: linking errors, unconditional usage of non-existing headers #682

Open barracuda156 opened 12 months ago

barracuda156 commented 12 months ago

Fails to build on Catalina and below: https://trac.macports.org/ticket/68871

On El Capitan down this fails (at least):

src/osx/sensors.cpp:22:10: fatal error: 'IOKit/hidsystem/IOHIDEventSystemClient.h' file not found
#include <IOKit/hidsystem/IOHIDEventSystemClient.h>
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/osx/sensors.cpp:22:10: note: did not find header 'hidsystem/IOHIDEventSystemClient.h' in framework 'IOKit' (loaded from '/System/Library/Frameworks')
1 warning and 1 error generated.
make: *** [obj/osx/sensors.o] Error 1

On Catalina through Sierra linking fails:

Linking and optimizing binary...
undef: __ZNSt3__120__libcpp_atomic_waitEPVKvx
undef: __ZNSt3__123__libcpp_atomic_monitorEPVKv
undef: __ZNSt3__123__cxx_atomic_notify_oneEPVKv
Undefined symbols for architecture x86_64:
  "std::__1::__libcpp_atomic_wait(void const volatile*, long long)", referenced from:
      Runner::_runner(void*) in lto.o
  "std::__1::__libcpp_atomic_monitor(void const volatile*)", referenced from:
      Runner::_runner(void*) in lto.o
  "std::__1::__cxx_atomic_notify_one(void const volatile*)", referenced from:
      Runner::run(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, bool, bool) in lto.o
      Runner::stop() in lto.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [btop] Error 1

I will test on PowerPC tomorrow, but expect the same headers errors at the very least.

imwints commented 11 months ago

I had the same error when I tested the CI file for the cmake build. I don't remember exactly what thw issue was, but from the top of my head I would say your linker doesn't know where your standard C++ library is. When you install llvm through homebrew for example the installation prints out addition LDFLAGS to append to the linker, like -L/usr/local/lib/...

barracuda156 commented 11 months ago

@imwints If that was the case, the build would fail on all macOS versions, but it does not: https://ports.macports.org/port/btop/details (see port health)

imwints commented 11 months ago

@barracuda156

Mmh yeah,I didn't look close enough. I reproduces a similar report with another std::__1::__libcpp... symbol, it happens when omitting the -stdlib=libc++ flag. It is likely that the ld command doesn't know about the standard library you compiler is using by default. And then don't forget to tell it where the standard library is, when it's in a non-default directory

imwints commented 11 months ago

And for the first issue, I'd be interested if the error is reproducible with cmake aswell, because in comparison to the make build cmake adds some macOS sdk paths by default

Edit: Ok the macports build also passes the sdk flags so this shouldn't be an issue

barracuda156 commented 11 months ago

-stdlib=libc++

@imwints MacOS from 10.7 onwards has it own native libc++, so location will be a default one (in sysroot), unless it is explicitly modified (which is possible via legacysupport port group, for example, but seldom needed).

Normally the compiler will pick correct flags to link with its standard runtime library. If it was not the case, everything would direly fail to link, and we would need to pass flags manually every time. (It is not impossible however that specifically here something breaks linking, or triggers some obscure bug in clang etc.) I can try switching from Apple clang to LLVM one to see if that makes any difference.

imwints commented 11 months ago

You can have a look here for LLVM

I still would try to be verbose about some options, probably

LDFLAGS="-stdlib=libc++ -v"

and have a look if properly linked with -lc++ and the directories point to the wanted library

barracuda156 commented 11 months ago

@imwints The link you refer is a build on macOS-12 (for some reason using mismatching SDK), unless I miss something. We have no failures on macOS-12, so it is not expected to fail. Could you confirm the build works on 10.15 on your end with any compiler?

imwints commented 11 months ago

for some reason using mismatching SDK

That is good to know, I have no experience with macOS at all, I just played around in CI with it and got it working somehow. (I also feel a bit unqualified answering here). I don't know how I would update or change the SDK.

From the github docs I read that there are only runners for macOS 11 through 13 and I don't have access to Apple hardware... :(

barracuda156 commented 11 months ago

@imwints My point was that we cannot infer from success of builds on macOS 11+ (which are successful both on your CI and our buildbots) that anything prior to macOS 11 shall automatically work, and if it does not, there is something wrong with Macports setup (something could be wrong, as always, but AFAICT it is unlikely here). I have no testing system for Intel at hand, but I can set it up with Catalina to deal with linking issues. (Since the cause of the error is unclear at the moment, I cannot really make a PR with a blind attempt to fix it, since our CI will not reveal relevant outcomes either, it will be clear only from buildbots.)

aristocratos commented 11 months ago

@barracuda156 Brew has a btop build for Catalina, and as far as I remember when we implemented MacOS support, Catalina was the "lowest" possible version I got it to build on.

Not sure if @joske got it to build on anything lower back then?

Might be that changes made since then to for example sensor detection for M1 macs might have hardened the dependencies even more.

I have however never documented a minimum requierd MacOS version, since I haven't invested enough time in exploring if it's possible to get it compiling on Mojave and below.

joske commented 11 months ago

I don't remember what version of macOS I was using at the time. I don't have access to anything below Monterrey ATM. I do have an old El Capitan macbook, but it just can't compile btop (IIRC could not install dependencies via brew, as El Capitan is just too old and unsupported).

barracuda156 commented 11 months ago

So immediate issues are the following:

  1. Non-existing header here:
    Compiling src/osx/btop_collect.cpp
    Compiling src/osx/sensors.cpp
    src/osx/sensors.cpp:22:10: fatal error: IOKit/hidsystem/IOHIDEventSystemClient.h: No such file or directory
    22 | #include <IOKit/hidsystem/IOHIDEventSystemClient.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    compilation terminated.
    make: *** [obj/osx/sensors.o] Error 1
    make: *** Waiting for unfinished jobs....

    Missing 32-bit defines here:

    src/osx/btop_collect.cpp:680:17: error: 'vm_statistics64' was not declared in this scope; did you mean 'vm_statistics_t'?
    680 |                 vm_statistics64 p;
      |                 ^~~~~~~~~~~~~~~
      |                 vm_statistics_t
    src/osx/btop_collect.cpp:681:52: error: 'HOST_VM_INFO64_COUNT' was not declared in this scope; did you mean 'HOST_VM_INFO_COUNT'?
    681 |                 mach_msg_type_number_t info_size = HOST_VM_INFO64_COUNT;
      |                                                    ^~~~~~~~~~~~~~~~~~~~
      |                                                    HOST_VM_INFO_COUNT
    src/osx/btop_collect.cpp:682:57: error: 'HOST_VM_INFO64' was not declared in this scope; did you mean 'HOST_VM_INFO'?
    682 |                 if (host_statistics64(mach_host_self(), HOST_VM_INFO64, (host_info64_t)&p, &info_size) == 0) {
      |                                                         ^~~~~~~~~~~~~~
      |                                                         HOST_VM_INFO
    src/osx/btop_collect.cpp:682:74: error: 'host_info64_t' was not declared in this scope; did you mean 'host_info_t'?
    682 |                 if (host_statistics64(mach_host_self(), HOST_VM_INFO64, (host_info64_t)&p, &info_size) == 0) {
      |                                                                          ^~~~~~~~~~~~~
      |                                                                          host_info_t
    src/osx/btop_collect.cpp:682:89: error: 'p' was not declared in this scope
    682 |                 if (host_statistics64(mach_host_self(), HOST_VM_INFO64, (host_info64_t)&p, &info_size) == 0) {
      |                                                                                         ^
    src/osx/btop_collect.cpp:682:21: error: 'host_statistics64' was not declared in this scope; did you mean 'host_statistics'?
    682 |                 if (host_statistics64(mach_host_self(), HOST_VM_INFO64, (host_info64_t)&p, &info_size) == 0) {
      |                     ^~~~~~~~~~~~~~~~~
      |                     host_statistics

    And some apparently missing headers here:

    src/osx/btop_collect.cpp:1123:17: error: 'rusage_info_current' was not declared in this scope
    1123 |                 rusage_info_current rusage;
      |                 ^~~~~~~~~~~~~~~~~~~
    src/osx/btop_collect.cpp:1124:42: error: 'RUSAGE_INFO_CURRENT' was not declared in this scope
    1124 |                 if (proc_pid_rusage(pid, RUSAGE_INFO_CURRENT, (void **)&rusage) == 0) {
      |                                          ^~~~~~~~~~~~~~~~~~~
    src/osx/btop_collect.cpp:1124:79: error: expected primary-expression before ')' token
    1124 |                 if (proc_pid_rusage(pid, RUSAGE_INFO_CURRENT, (void **)&rusage) == 0) {
      |                                                                               ^
    src/osx/btop_collect.cpp:1124:21: error: 'proc_pid_rusage' was not declared in this scope
    1124 |                 if (proc_pid_rusage(pid, RUSAGE_INFO_CURRENT, (void **)&rusage) == 0) {
      |                     ^~~~~~~~~~~~~~~
    src/osx/btop_collect.cpp:1126:69: error: expected primary-expression before '.' token
    1126 |                         detailed.io_read = floating_humanizer(rusage.ri_diskio_bytesread);
      |                                                                     ^
    src/osx/btop_collect.cpp:1127:70: error: expected primary-expression before '.' token
    1127 |                         detailed.io_write = floating_humanizer(rusage.ri_diskio_byteswritten);
      |                                                                      ^
    src/osx/btop_collect.cpp: In function 'std::vector<Proc::proc_info>& Proc::collect(bool)':
    src/osx/btop_collect.cpp:1133:29: warning: possibly dangling reference to a temporary [-Wdangling-reference]
    1133 |                 const auto &sorting = Config::getS("proc_sorting");
      |                             ^~~~~~~
    src/osx/btop_collect.cpp:1133:51: note: the temporary was destroyed at the end of the full expression 'Config::getS(std::basic_string<char>(((const char*)"proc_sorting"), std::allocator<char>()))'
    1133 |                 const auto &sorting = Config::getS("proc_sorting");
      |                                       ~~~~~~~~~~~~^~~~~~~~~~~~~~~~
    src/osx/btop_collect.cpp:1135:29: warning: possibly dangling reference to a temporary [-Wdangling-reference]
    1135 |                 const auto &filter = Config::getS("proc_filter");
      |                             ^~~~~~
    src/osx/btop_collect.cpp:1135:50: note: the temporary was destroyed at the end of the full expression 'Config::getS(std::basic_string<char>(((const char*)"proc_filter"), std::allocator<char>()))'
    1135 |                 const auto &filter = Config::getS("proc_filter");
      |                                      ~~~~~~~~~~~~^~~~~~~~~~~~~~~
    make: *** [obj/osx/btop_collect.o] Error 1

    Does it have some more generic Unix fallback? That could potentially work on older macOS.

barracuda156 commented 11 months ago

rusage_info_current is available on 10.10+: https://developer.apple.com/documentation/kernel/rusage_info_current

aristocratos commented 11 months ago

@barracuda156 Feel free to create a PR if you wanna work on adapting it for more backwards compatibility.

But I'm guessing there will be a wall at some point because of compiler compatibility (needs C++20 support).

barracuda156 commented 11 months ago

We have C++20 up to macOS 10.5 via GCC, and so far it is supported by upstream (PowerPC and Intel, 32- and 64-bit). I use gcc13 on my 10.6 system. New Clangs (I think clang-16) also build back to 10.5, though only for Intel (PowerPC is broken).

aristocratos commented 11 months ago

Well, having support for all but the first 5 versions should probably be good enough 😉

Ping me if you get stuck on anything and I'll try to help when I've got time.

Good luck 👍

barracuda156 commented 11 months ago

On a quick look (short on time tonight), it will not suffice just to fix compilation, since CPU detection has to be re-implemented for PowerPC. Not sure to what extent related info can be pulled from the system (some functions are restricted to the kernel), but given that apps like MenuMeters somehow work, rather detailed info can be collected.

For Intel is should be far easier; one thing which may be causing linking errors is that apparently filesystem is needed, and Apple libc++ does not have it until something like Darwin 16. It is likely fixable in Macports, we can use a newer libc++ which supports this extension.

joske commented 11 months ago

I tried building with make and gcc-13 on a Catalina (10.15.7) VM (64 bit), and it just worked. I never tried with any 32 bit version.

joske commented 11 months ago

For the El Capitan issue, IOKit/hidsystem/IOHIDEventSystemClient.h was not documented as far as I know. Also this code is only needed for Apple Silicon, so we can easily make this conditional on macOS versions above Big Sur only.

Edit: see https://github.com/aristocratos/btop/pull/690