facebook / buck2

Build system, successor to Buck
https://buck2.build/
Apache License 2.0
3.33k stars 194 forks source link

Disable stripping stage if no strip binary was provided. #595

Closed zadlg closed 2 months ago

zadlg commented 2 months ago

Disable stripping stage if no strip binary was provided.

CxxToolchainInfo has a field called binary_utilities_info which points to some binary utilities such as strip or objcopy.

Similar to https://github.com/facebook/buck2/pull/594, according to the documentation, CxxToolchainInfo.binary_utilities_info is optional.

However, in the cxx_library implementation, _strip_objects is called regardless of the CxxToolchainInfo.binary_utilities_info value.

This leads to the following error:

Caused by:
    Traceback (most recent call last):
      File <builtin>, in <module>
      * prelude/rules.bzl:101, in buck2_compatibility_shim
          return impl(ctx)
      * prelude/cxx/cxx.bzl:191, in cxx_library_impl
          output = cxx_library_parameterized(ctx, params)
      * prelude/cxx/cxx_library.bzl:371, in cxx_library_parameterized
          compiled_srcs = cxx_compile_srcs(
      * prelude/cxx/cxx_library.bzl:914, in cxx_compile_srcs
          pic = _get_library_compile_output(ctx, pic_cxx_outs, impl_params.extra_link_i...
      * prelude/cxx/cxx_library.bzl:866, in _get_library_compile_output
          stripped_objects = _strip_objects(ctx, objects)
      * prelude/cxx/cxx_library.bzl:1108, in _strip_objects
          outs.append(strip_debug_info(ctx, base + ".stripped.o", obj))
      * prelude/linking/strip.bzl:67, in strip_debug_info
          return _strip_debug_info(
    error: Object of type `NoneType` has no attribute `strip`
      --> prelude/linking/strip.bzl:16:13
       |
    16 |     strip = cxx_toolchain.binary_utilities_info.strip
       |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This commit prevents this bug from happening by making CxxToolchainInfo.binary_utilities_info truely optional.

facebook-github-bot commented 2 months ago

@JakobDegen has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.