Make default OSX toolchain respect the use_libtool flag#736
Conversation
30e47cb to
a3ff742
Compare
| # compile.inputs = { hello_cc } | ||
| compile_action.inputs().contains("{package}/hello.cc") | ||
|
|
||
| # compile.outputs = { hello.o } (mock toolchain doesn't generate .d) |
There was a problem hiding this comment.
what are these changes for?
I think @c-mita might have a better fix if you're just trying to resolve an unrelated failure here.
There was a problem hiding this comment.
Yeah this is a drive-by fix because master is red https://buildkite.com/bazel/rules-cc/builds/5396
|
@keith any opinions here? This seems ~correct to me -- honoring this flag if set, defaulting to libtool but falling back more gracefully to ar without completely pretending ar = libtool. |
keith
left a comment
There was a problem hiding this comment.
yea i think accepting this flag is fine to do here too
|
addressed @keith 's feedback and undid the test fixes |
| libtool = None | ||
| if darwin: | ||
| overridden_tools["gcc"] = "cc_wrapper.sh" | ||
| overridden_tools["ar"] = _find_generic(repository_ctx, "libtool", "LIBTOOL", overridden_tools) | ||
| libtool = _find_generic(repository_ctx, "libtool", "LIBTOOL", overridden_tools) |
There was a problem hiding this comment.
| libtool = None | |
| if darwin: | |
| overridden_tools["gcc"] = "cc_wrapper.sh" | |
| overridden_tools["ar"] = _find_generic(repository_ctx, "libtool", "LIBTOOL", overridden_tools) | |
| libtool = _find_generic(repository_ctx, "libtool", "LIBTOOL", overridden_tools) | |
| libtool = _find_generic(repository_ctx, "libtool", "LIBTOOL", overridden_tools) | |
| if darwin: | |
| overridden_tools["gcc"] = "cc_wrapper.sh" |
i think now that you're using a different name this is safe?
7d0186c to
a2279cf
Compare
| "toolchain_identifier": attr.string(mandatory = True), | ||
| "unfiltered_compile_flags": attr.string_list(), | ||
| "_use_libtool_on_macos": attr.label( | ||
| default = "@rules_cc//cc/toolchains/args/archiver_flags:use_libtool_on_macos", |
There was a problem hiding this comment.
we should move this flag to the config settings directory with the other apple flag (and i guess leave an alias in this location for now to be nice)
No description provided.