Be less aggressive in interpreting flags as response files

[cocreature]

Both -Xlinker and -install_name will often receive flags starting with
@, specifically @loader_path or @rpath. Bazel 4.0 started
interpreting those as response files which broke our build.

This PR addresses this by hardcoding those flags. This is a pretty
ugly solution so I’m very open to better ideas. That said, there is
some precedence for doing that in
rules-haskell.

[keith]

The thought at the time we made this decision was that this form of linker flag should be replaced with -Wl,-install_name,@foo

[cocreature]

Those flags are not under our control. In this case, it is the Haskell compiler GHC that is passing them https://gitlab.haskell.org/ghc/ghc/-/blob/ab1b49108e2665eb9ccdccffde0099d1785315ee/compiler/main/SysTools.hs#L376 I do agree that in an ideal world, changing the flags would be preferable but I think this is a good example that this isn’t really feasible for users in a lot of cases.

[cocreature]

I guess one option would be for rules_haskell’s cc wrapper to do the rewriting but I’m not sure that’s better than handling it in Bazel itself.

[keith]

Ah interesting. I do think it would be safe to make that compiler change, but that wouldn't solve this in the short term. At least 1 problem with this change is that there are a few other places that also duplicate this logic, see the original change and conversation for details #12265

[aherrmann]

@keith GHC's reasoning for preferring -Xlinker over -Wl,... is documented here:

Note [-Xlinker -rpath vs -Wl,-rpath]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

-Wl takes a comma-separated list of options which in the case of
-Wl,-rpath -Wl,some,path,with,commas parses the path with commas
as separate options.
Buck, the build system, produces paths with commas in them.

-Xlinker doesn't have this disadvantage and as far as I can tell
it is supported by both gcc and clang. Anecdotally nvcc supports
-Xlinker, but not -Wl.

[keith]

ugh!! thanks for the link. I've never heard that buck could produce commas in paths, any idea where I can see more details on that? FWIW I think a change like this would be accepted as long as it covered all the cases (although I'm not a googler so I can't make that call overall), it's just we didn't have a compelling reason to do the extra work in the previous case, so we didn't want to enumerate a list of all possible flags that could have a @ after them

[aherrmann]

@keith I'm not familiar with Buck myself. What I understand, Buck can append comma separated flavors to targets and file names, e.g. sometarget#default,static, see also this GHC related issue on Buck's issue tracker here.

FWIW I think a change like this would be accepted as long as it covered all the cases (although I'm not a googler so I can't make that call overall)

The ones I'm aware of are -install_name, e.g. -install_name @rpath/..., and -rpath, e.g. -rpath @loader_path/....

[keith]

I actually meant which scripts not which args. There are 3 scripts which have similar logic for this unfortunately. But it depends on what rules_haskell calls through how much of an issue that will be. I believe I solved this for wrapped_clang already with this logic

bazel/tools/osx/crosstool/wrapped_clang.cc

Lines 275 to 278 in caf1355

	// Ignore non-file args such as '@loader_path/...'
	if (!original_file.good()) {
	return false;
	}

You should be able to implement something similar in tools/cpp/osx_cc_wrapper.sh.tpl and tools/cpp/osx_cc_wrapper.sh (both are used in different places unfortunately)

Instead of specifying the flags that can have arguments starting with @ I think you can do something like what's being done in the C++ with:

diff --git i/tools/cpp/osx_cc_wrapper.sh w/tools/cpp/osx_cc_wrapper.sh
index 8c9c111a75..23604c7c7e 100755
--- i/tools/cpp/osx_cc_wrapper.sh
+++ w/tools/cpp/osx_cc_wrapper.sh
@@ -53,7 +53,7 @@ function parse_option() {
 
 # let parse the option list
 for i in "$@"; do
-    if [[ "$i" = @* ]]; then
+    if [[ "$i" = @* && -r "$i" ]]; then
         while IFS= read -r opt
         do
             parse_option "$opt"

[aherrmann]

I actually meant which scripts not which args.

Oh, I see, sorry, misread.

But it depends on what rules_haskell calls through how much of an issue that will be.

Just for completeness, rules_haskell doesn't call any of these scripts explicitly. It calls the C compiler provided by Bazel's cc toolchain. I.e.

cc_common.get_tool_for_action(
        feature_configuration = feature_configuration,
        action_name = ACTION_NAMES.c_compile,
)

That may be one of Bazel's cc-wrappers. So far @bazel_tools//tools/cpp:osx_cc_wrapper.sh.tpl was the only script that triggered this issue. But, it makes sense to try and cover all instances.

Instead of specifying the flags that can have arguments starting with @ I think you can do something like what's being done in the C++ with [...]

Looks good to me.

[aherrmann]

@keith @cocreature I've opened #13148 to replace this PR with @keith's suggestions from above applied.

[brentleyjones]

@bazel-io fork 5.1