aristocratos / btop

A monitor of resources
Apache License 2.0
21.36k stars 655 forks source link

Build error after #796 changes NVML_DEVICE_NAME_BUFFER_SIZE to RSMI_DEVICE_NAME_BUFFER_SIZE #837

Closed sbradnick closed 7 months ago

sbradnick commented 7 months ago

Read the README.md and search for similar issues before posting a bug report!

Any bug that can be solved by just reading the prerequisites section of the README will likely be ignored.

Describe the bug

Post 20240218 (6c66740), which was working fine, it seems #796 breaks building with the following error:

[   11s] Compiling src/linux/btop_collect.cpp
[   11s] src/linux/btop_collect.cpp: In function ‘bool Gpu::Rsmi::collect(Gpu::gpu_info*)’:
[   11s] src/linux/btop_collect.cpp:1400:51: error: ‘RSMI_DEVICE_NAME_BUFFER_SIZE’ was not declared in this scope; did you mean ‘NVML_DEVICE_NAME_BUFFER_SIZE’?
[   11s]  1400 |                                         char name[RSMI_DEVICE_NAME_BUFFER_SIZE];
[   11s]       |                                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
[   11s]       |                                                   NVML_DEVICE_NAME_BUFFER_SIZE
[   11s] src/linux/btop_collect.cpp:1401:63: error: ‘name’ was not declared in this scope; did you mean ‘tzname’?
[   11s]  1401 |                                 result = rsmi_dev_name_get(i, name, RSMI_DEVICE_NAME_BUFFER_SIZE);
[   11s]       |                                                               ^~~~
[   11s]       |                                                               tzname
[   11s] make: *** [Makefile:394: obj/linux/btop_collect.o] Error 1

To Reproduce

Running a standard build via OBS with ROCm v6.1.0 (but pretty sure it fails w/ v5.6.1 as well)

Expected behavior

Successful build w/o needing to patch RSMI_DEVICE_NAME_BUFFER_SIZE back to NVML_DEVICE_NAME_BUFFER_SIZE

My test machine is running off the iGPU of a 7800x3d, btop doesn't report the GPU @ 100% and it seems to report sensibly if I launch glxgears to get the GPU to do more than show my :0 screen. I also have a NVIDIA RTX 3070ti in it, but it's for GPU passthrough and not in use here.

Screenshots

Info (please complete the following information):

Additional context

Contents of ~/.config/btop/btop.log

Note: The snap uses: ~/snap/btop/current/.config/btop

(try running btop with --debug flag if btop.log is empty)

GDB Backtrace

If btop++ is crashing at start the following steps could be helpful:

(Extra helpful if compiled with make OPTFLAGS="-O0 -g")

  1. run (linux): gdb btop (macos): lldb btop

  2. r to run, wait for crash and press enter if prompted, CTRL+L to clear screen if needed.

  3. (gdb): thread apply all bt (lldb): bt all to get backtrace for all threads

  4. Copy and paste the backtrace here:

aristocratos commented 7 months ago

@sbradnick Do you wanna try moving line 163 in btop_collect.cpp:

#define RSMI_DEVICE_NAME_BUFFER_SIZE 128

up two lines so it looks like this:

namespace Rsmi {
    #define RSMI_DEVICE_NAME_BUFFER_SIZE 128
    #if !defined(RSMI_STATIC)
        //? RSMI defines, structs & typedefs
        #define RSMI_MAX_NUM_FREQUENCIES_V5  32

and then try compiling again.

sbradnick commented 7 months ago

@sbradnick Do you wanna try moving line 163 in btop_collect.cpp:

#define RSMI_DEVICE_NAME_BUFFER_SIZE 128

up two lines so it looks like this:

namespace Rsmi {
    #define RSMI_DEVICE_NAME_BUFFER_SIZE 128
  #if !defined(RSMI_STATIC)
      //? RSMI defines, structs & typedefs
      #define RSMI_MAX_NUM_FREQUENCIES_V5  32

and then try compiling again.

That makes the build successful. Only other oddity is how different the spacing is (tabs vs. spaces) for that line vs. the others and what the expected indentation should be (if it matters), when comparing to your "so it looks like this" example. Thanks for getting back to me! :smile:

btop_collect cpp_spacing_1714587724

aristocratos commented 7 months ago

@sbradnick

That makes the build successful.

Great :)

Only other oddity is how different the spacing is (tabs vs. spaces) for that line vs. the others and what the expected indentation should be

That's probably just github messing up the formatting when I moved it around in the code snippet.

Anyways, fixed in latest commits.