lcm-proj/lcm

lcm 1.5.0 build issue on linux

Closed this issue · 5 comments

chenrui333 commented

👋 trying to build the latest release, but run into some build issue. The error log is as below:

error build log 
  /tmp/lcm-20230505-24434-mkd4tk/lcm-1.5.0/lcm-lua/lualcm_lcm.c:577:9: error: ‘subscription’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
    577 |     if (lcm_unsubscribe(lcmu->lcm, subscription) != 0) {
        |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  In function ‘ensure_token_capacity’,

full build log, https://github.com/Homebrew/homebrew-core/actions/runs/4896032508/jobs/8753235249
relates to Homebrew/homebrew-core#130265

nosracd commented

I can reproduce the warnings on CI by switching to a release build. It looks like the warnings are specific to the GCC compiler. It would be good to fix those, but none of them look like serious issues unique to this release. I'm not familiar with homebrew's policies, but in order to get the homebrew PR unblocked, could you turn off Werror so the warnings don't cause the build to fail?

chenrui333 commented

yeah, that might be the only choice, but is that hard to fix in the codebase?

nosracd commented

It seems that it's not terribly difficult to fix it the warnings for GCC 11 (which is the version the homebrew job failed on). Fixing all the warnings through GCC 12 will be a bit trickier, since those appear in generated code.

However, even if we fix the warnings on master, would you still need a new tagged release (something like 1.5.1) that included the changes to work off of?

nosracd commented

I'm reopening this issue because it looks like there's actually a more fundamental issue that I've failed to address. I thought extra warnings and Werror were on for the homebrew build because it was configured that way for the homebrew build but that's not actually the case. Those warnings are being treated as errors because of this line in lcm-lua/CMakeLists.txt:

target_compile_options(lcm-lua PRIVATE -Werror -Wall -Wextra)

In general, my philosophy on warnings and werror has been

  • By default, Werror is off and warnings are the default level
  • On CI all (or most) warnings are on
  • On CI Werror is on

That way warnings are caught by CI but won't bother any user trying to use the latest version of their compiler.

I'd propose that we remove that entire line but I'd also be happy with just removing the -Werror if anyone feels differently.

Of course, that requires a code change and I'm guessing that @chenrui333 would prefer a workaround that doesn't require a code change so they can finish their homebrew PR. It seems that configuring a build directory with the argument -DCMAKE_C_FLAGS="-Wno-maybe-uninitialized" to cmake results in a successful build for me locally so I'd recommend adding that to the homebrew build configuration for LCM.

chenrui333 commented

It seems that configuring a build directory with the argument -DCMAKE_C_FLAGS="-Wno-maybe-uninitialized" to cmake results in a successful build for me locally so I'd recommend adding that to the homebrew build configuration for LCM.

that is an easy change on my end.

  • 🎉1