bazelbuild / reclient

Apache License 2.0
68 stars 17 forks source link

unused var bug in goma_subprocess.patch macro #59

Closed daniel-sudz closed 3 months ago

daniel-sudz commented 4 months ago

Not sure why this does not repro in CI. Note that reclient patches https://chromium.googlesource.com/infra/goma/client/+/5de52a5ece34ce6a08797565cff270ddead936b3/client/subprocess.cc in https://github.com/bazelbuild/reclient/blob/main/third_party/patches/goma/goma_subprocess.patch. This introduces the following variable in the namespace:

+std::mutex popen_mutex;

however the usage of the variable is guarded by

 #ifdef _WIN32

So when the macro is not triggered, an unused variable warning is emitted which converts to an error based on the -Werror flag in this repo:

FAILED: obj/client/subprocess_lib/subprocess.obj 
..\..\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe /nologo /showIncludes /FC "-imsvc../../../../../../Program Files/Microsoft Visual Studio/2022/Enterprise/VC/Tools/MSVC/14.40.33807/include" "-imsvc../../../../../../Program Files/Microsoft Visual Studio/2022/Enterprise/VC/Tools/MSVC/14.40.33807/ATLMFC/include" "-imsvc../../../../../../Program Files/Microsoft Visual Studio/2022/Enterprise/VC/Auxiliary/VS/include" "-imsvc../../../../../../Program Files (x86)/Windows Kits/10/include/10.0.20348.0/ucrt" "-imsvc../../../../../../Program Files (x86)/Windows Kits/10/include/10.0.20348.0/um" "-imsvc../../../../../../Program Files (x86)/Windows Kits/10/include/10.0.20348.0/shared" "-imsvc../../../../../../Program Files (x86)/Windows Kits/10/include/10.0.20348.0/winrt" "-imsvc../../../../../../Program Files (x86)/Windows Kits/10/include/10.0.20348.0/cppwinrt" "-imsvc../../../../../../Program Files (x86)/Windows Kits/NETFXSDK/4.8/include/um" -D_HAS_EXCEPTIONS=0 "-DCR_CLANG_REVISION=\"llvmorg-18-init-4631-gd50b56d1-1\"" -DNDEBUG -DNDEBUG -DENABLE_LZMA -DHAVE_COUNTERZ=1 -DWIN32_LEAN_AND_MEAN -DNOMINMAX -D_ATL_NO_OPENGL -D_WINDOWS -DCERT_CHAIN_PARA_HAS_EXTRA_FIELDS -DNTDDI_VERSION=0x06030000 -DPSAPI_VERSION=1 -DWIN32 -D_SECURE_ATL -D_UNICODE -DUNICODE -D_WIN32_WINNT=0x0603 -DWINVER=0x0603 -DHAVE_ZLIB -DGOOGLE_GLOG_DLL_DECL= -DGOOGLE_GLOG_IS_A_DLL=0 -DGOOGLE_GLOG_DLL_DECL_FOR_UNITTESTS= -DGLOG_NO_ABBREVIATED_SEVERITIES -D_CRT_RAND_S -DGOOGLE_PROTOBUF_NO_RTTI -DGOOGLE_PROTOBUF_NO_STATIC_INITIALIZER -DHAVE_ZLIB -I../.. -Igen -I../../base -I../../third_party/abseil/src -I../../third_party/gtest/googletest/include -I../../third_party/chromium_base -I../../lib -I../../third_party/protobuf/protobuf/src -Igen -Igen/third_party/protobuf/protobuf/src -I../../third_party/protobuf/protobuf/src -I../../third_party/protobuf/protobuf -I../../third_party/utf8_range/src -I../../third_party/zlib -I../../third_party/zlib/contrib -I../../third_party/config/glog/win -I../../third_party/glog/src -I../../third_party/glog/src/windows -I../../third_party/xz/src/liblzma/api -I../../client -I../../third_party/jsoncpp/source/include -Werror /MT /WX /Zi /Gy /EHsc /FIIntrin.h -m64 /W4 /wd4127 /wd4244 -Wno-sign-compare -Wimplicit-fallthrough -Wthread-safety -Wno-unused-parameter -Werror /Ox /Ot /Oy -Wno-implicit-int-float-conversion -Wno-deprecated -Wno-implicit-fallthrough -Wno-missing-field-initializers -Wno-sign-compare -Wno-unused-parameter -Wno-bitfield-width -Wno-unused-local-typedef -Wno-microsoft-unqualified-friend -Wno-implicit-int-float-conversion /wd4267 /GR- -Wno-deprecated-declarations /c ../../client/subprocess.cc /Foobj/client/subprocess_lib/subprocess.obj /Fd"obj/client/subprocess_lib_cc.pdb"
../../client/subprocess.cc(44,12): error: unused variable 'popen_mutex' [-Werror,-Wunused-variable]
   44 | std::mutex popen_mutex;
      |            ^~~~~~~~~~~
daniel-sudz commented 4 months ago

this should just be a small fix to update the patch file that I'm happy to do

gkousik commented 3 months ago

Closing this out since the referenced PR is now merged.