feat: configurable ldflags

[jsun-splunk]

configure_make() and make() both resolves LDFLAGS to CxxToolsInfo.cxx_linker_executable. This hard codes the linker executable flags and does not provide a way to use the cxx_linker_shared flags. While this works for most use cases, it does not work for all. An example is when we enable --force-pic which appends -pie to our executable linker flags. -pie is only applicable when linking executables and will fail on shared libraries since it cannot find a main() function.

This MR solves this problem by introducing 2 new attributes to configure_make() and make().

• executable_ldflags_vars
• shared_ldflags_vars

Each is a string_list of variable names that the target project uses for executable ldflags and shared ldflags. For example, in openssl3 BIN_LDFLAGS is used for executable ldflags and LIB_LDFLAGS is used for shared ldflags.

by overwriting the respective *_LDFLAGS using make vars mapped to cxx_linker_executable and cxx_shared_executable we can enforce the mapping of appropriate linker flags.

[jsun-splunk]

The ci failures on this PR don't seem related to this change. The same error occurs on other recent PRs (e.g. #1223). I also did test run on main and the errors on macos still occurs. See #1228 and https://buildkite.com/bazel/rules-foreign-cc/builds/5761#0190be8c-a6b9-4df3-b99a-54831c1e37ef.

I tried looking into the error but I cannot replicate it locally. Running

bazel test -s --sandbox_debug //glib:glib_build_test

locally passes. However, I noticed my local resolves to local_config_cc/cc_wrapper.sh for the compiler and ci resolved to local_config_apple_cc/wrapped_clang for the compiler. I'm not sure how to replicate ci exactly locally.

[jsun-splunk]

@jsharpe - would you mind reviewing? TIA!

[jsun-splunk]

@jsharpe any chance of getting some feedback on this?

[jsharpe]

Sorry for the delay in getting round to reviewing this - I've not had much time to devote to maintaining these rules and the priority was getting CI back to green first.

In an ideal world this wouldn't be needed but the reality is since third party build systems can do literally anything then it does make sense to be able to override the ldflags.
I think it'd be good if you could also plumb this through for meson and ninja rules too? Specifically if this was present for the meson rules we could actually fix the macOS CI issues that had popped up I think because the issue there is due to ldflags from the toolchain propagating through to meson's setup where its not expecting to have them set if I've understood the issue there correctly.

[jsharpe]

nit: this is true for autotools generated makefiles but in general a makefile doesn't have to accept the environment variable based overrides for any given flags.

[jsun-splunk]

Yes, I understand. The 2 flags I've added for the different linker flags are injected to the build_script.sh as make vars (not env vars), which can be used for autotools or non-autotools makefiles. Is there anything specific you would like me to update in the description?

[allsey87]

I think the issue raised in this PR is also present in the Meson rule. Linking shared libraries is also complete ignored.
https://github.com/bazelbuild/rules_foreign_cc/blob/509b5aa6d655b7ca9a19db57aa82e0aee53ee4ed/foreign_cc/meson.bzl#L94-L96

If we are to consider a solution to this problem, I think it should also take cmake and meson into account. Also, does this PR consider the situation where we are building shared library with configure_make which in turn needs to create executables as part of its testing routines for the compiler?

[jsun-splunk]

Thanks for reviewing @jsharpe.

I had another look at the macOS CI issue with glib and meson. The issue is not a foreign_cc one but a meson one. I've raised a fix for it at #1260.

Also, I agree configurable LDFLAGS should be extended to meson, ninja and even cmake. However, I don't have the expertise in those build tools to make the update. I also don't know how to test any updates I would make. I'm testing configure_make against openssl3, which doesn't have a official meson/ninja/cmake build. Would you entertain accepting this PR without updates to other tools?

@allsey87 - given we are still using linker flags produced by the resolved cc_toolchain it should still support building shared libraries with configure_make and using in downstream cc rules.

[jsun-splunk]

Looks like this is already supported in cmake in some form --> https://github.com/bazelbuild/rules_foreign_cc/blob/main/foreign_cc/private/cmake_script.bzl#L355

[jsun-splunk]

Looks like github had an outage when I last pushed. Is there anyway to retry or trigger another buildkite run without having to do a dummy commit?

[jsun-splunk]

@jsharpe - any chance for another review on this?