diff --git a/.travis.yml b/.travis.yml index 17c3d79092..5e367d57c5 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,7 +11,7 @@ os: - osx env: - - V=0.16.0 + - V=0.17.2 before_install: - | diff --git a/go/platform/apple.bzl b/go/platform/apple.bzl index ab0489ae10..c598dcbd68 100644 --- a/go/platform/apple.bzl +++ b/go/platform/apple.bzl @@ -29,7 +29,7 @@ PLATFORMS = { def _apple_version_min(platform, version): return "-m" + platform.name_in_plist.lower() + "-version-min=" + version -def apple_ensure_options(ctx, env, tags, compiler_options, linker_options, target_gnu_system_name): +def apple_ensure_options(ctx, env, tags, compiler_option_lists, linker_option_lists, target_gnu_system_name): """apple_ensure_options ensures that, when building an Apple target, the proper environment, compiler flags and Go tags are correctly set.""" platform = PLATFORMS.get(target_gnu_system_name) @@ -39,7 +39,9 @@ def apple_ensure_options(ctx, env, tags, compiler_options, linker_options, targe tags.append("ios") # needed for stdlib building if platform in [apple_common.platform.ios_device, apple_common.platform.ios_simulator]: min_version = _apple_version_min(platform, "7.0") - compiler_options.append(min_version) - linker_options.append(min_version) + for compiler_options in compiler_option_lists: + compiler_options.append(min_version) + for linker_options in linker_option_lists: + linker_options.append(min_version) xcode_config = ctx.attr._xcode_config[apple_common.XcodeVersionConfig] env.update(apple_common.target_apple_env(xcode_config, platform)) diff --git a/go/private/actions/link.bzl b/go/private/actions/link.bzl index e079868ad1..425f8941a9 100644 --- a/go/private/actions/link.bzl +++ b/go/private/actions/link.bzl @@ -22,6 +22,8 @@ load( "@io_bazel_rules_go//go/private:mode.bzl", "LINKMODE_NORMAL", "LINKMODE_PLUGIN", + "extld_from_cc_toolchain", + "extldflags_from_cc_toolchain", ) load( "@io_bazel_rules_go//go/private:skylib/lib/shell.bzl", @@ -69,7 +71,7 @@ def emit_link( # Exclude -lstdc++ from link options. We don't want to link against it # unless we actually have some C++ code. _cgo_codegen will include it # in archives via CGO_LDFLAGS if it's needed. - extldflags = [f for f in go.cgo_tools.linker_options if f not in ("-lstdc++", "-lc++")] + extldflags = [f for f in extldflags_from_cc_toolchain(go) if f not in ("-lstdc++", "-lc++")] if go.coverage_enabled: extldflags.append("--coverage") @@ -78,8 +80,7 @@ def emit_link( tool_args = go.tool_args(go) # Add in any mode specific behaviours - extld = go.cgo_tools.compiler_executable - tool_args.add("-extld", extld) + tool_args.add_all(extld_from_cc_toolchain(go)) if go.mode.race: tool_args.add("-race") if go.mode.msan: @@ -180,12 +181,7 @@ def _bootstrap_link(go, archive, executable, gc_linkopts): arguments = [args], mnemonic = "GoLink", command = "export GOROOT=\"$(pwd)\"/{} && {} \"$@\"".format(shell.quote(go.root), shell.quote(go.go.path)), - env = { - # workaround: go link tool needs some features of gcc to complete the job on Arm platform. - # So, PATH for 'gcc' is required here on Arm platform. - "PATH": go.cgo_tools.compiler_path, - "GOROOT_FINAL": "GOROOT", - }, + env = {"GOROOT_FINAL": "GOROOT"}, ) def _extract_extldflags(gc_linkopts, extldflags): diff --git a/go/private/context.bzl b/go/private/context.bzl index 1e8d565333..3d4c1ea57d 100644 --- a/go/private/context.bzl +++ b/go/private/context.bzl @@ -16,6 +16,14 @@ load( "@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain", ) +load( + "@bazel_tools//tools/build_defs/cc:action_names.bzl", + "CPP_COMPILE_ACTION_NAME", + "CPP_LINK_DYNAMIC_LIBRARY_ACTION_NAME", + "CPP_LINK_EXECUTABLE_ACTION_NAME", + "CPP_LINK_STATIC_LIBRARY_ACTION_NAME", + "C_COMPILE_ACTION_NAME", +) load( "@io_bazel_rules_go//go/private:providers.bzl", "EXPLICIT_PATH", @@ -52,6 +60,7 @@ load( ) GoContext = provider() +_GoContextData = provider() _COMPILER_OPTIONS_BLACKLIST = { "-fcolor-diagnostics": None, @@ -247,7 +256,7 @@ def go_context(ctx, attr = None): host_only = getattr(attr, "_hostonly", False) - context_data = attr._go_context_data + context_data = attr._go_context_data[_GoContextData] mode = get_mode(ctx, host_only, toolchain, context_data) tags = list(context_data.tags) if mode.race: @@ -270,7 +279,6 @@ def go_context(ctx, attr = None): "GOROOT": goroot, "GOROOT_FINAL": "GOROOT", "CGO_ENABLED": "0" if mode.pure else "1", - "PATH": context_data.cgo_tools.compiler_path, }) # TODO(jayconrod): remove this. It's way too broad. Everything should @@ -331,42 +339,178 @@ def go_context(ctx, attr = None): _ctx = ctx, # TODO: All uses of this should be removed ) -def _go_context_data(ctx): - cpp = find_cpp_toolchain(ctx) - features = ctx.features - compiler_options = _filter_options( - cpp.compiler_options() + cpp.unfiltered_compiler_options(features), +def _go_context_data_impl(ctx): + # TODO(jayconrod): find a way to get a list of files that comprise the + # toolchain (to be inputs into actions that need it). + # ctx.files._cc_toolchain won't work when cc toolchain resolution + # is switched on. + cc_toolchain = find_cpp_toolchain(ctx) + feature_configuration = cc_common.configure_features( + cc_toolchain = cc_toolchain, + requested_features = ctx.features, + unsupported_features = ctx.disabled_features, + ) + + # TODO(jayconrod): keep the environment separate for different actions. + env = {} + + c_compile_variables = cc_common.create_compile_variables( + feature_configuration = feature_configuration, + cc_toolchain = cc_toolchain, + ) + c_compiler_path = cc_common.get_tool_for_action( + feature_configuration = feature_configuration, + action_name = C_COMPILE_ACTION_NAME, + ) + c_compile_options = _filter_options( + cc_common.get_memory_inefficient_command_line( + feature_configuration = feature_configuration, + action_name = C_COMPILE_ACTION_NAME, + variables = c_compile_variables, + ), _COMPILER_OPTIONS_BLACKLIST, ) - linker_options = _filter_options( - cpp.mostly_static_link_options(False), + env.update(cc_common.get_environment_variables( + feature_configuration = feature_configuration, + action_name = C_COMPILE_ACTION_NAME, + variables = c_compile_variables, + )) + + cxx_compile_variables = cc_common.create_compile_variables( + feature_configuration = feature_configuration, + cc_toolchain = cc_toolchain, + ) + cxx_compile_options = _filter_options( + cc_common.get_memory_inefficient_command_line( + feature_configuration = feature_configuration, + action_name = CPP_COMPILE_ACTION_NAME, + variables = cxx_compile_variables, + ), + _COMPILER_OPTIONS_BLACKLIST, + ) + env.update(cc_common.get_environment_variables( + feature_configuration = feature_configuration, + action_name = CPP_COMPILE_ACTION_NAME, + variables = cxx_compile_variables, + )) + + ld_executable_variables = cc_common.create_link_variables( + feature_configuration = feature_configuration, + cc_toolchain = cc_toolchain, + is_linking_dynamic_library = False, + ) + ld_executable_path = cc_common.get_tool_for_action( + feature_configuration = feature_configuration, + action_name = CPP_LINK_EXECUTABLE_ACTION_NAME, + ) + ld_executable_options = _filter_options( + cc_common.get_memory_inefficient_command_line( + feature_configuration = feature_configuration, + action_name = CPP_LINK_EXECUTABLE_ACTION_NAME, + variables = ld_executable_variables, + ), _LINKER_OPTIONS_BLACKLIST, ) + env.update(cc_common.get_environment_variables( + feature_configuration = feature_configuration, + action_name = CPP_LINK_EXECUTABLE_ACTION_NAME, + variables = ld_executable_variables, + )) + + # We don't collect options for static libraries. Go always links with + # "ar" in "c-archive" mode. We can set the ar executable path with + # -extar, but the options are hard-coded to something like -q -c -s. + ld_static_lib_variables = cc_common.create_link_variables( + feature_configuration = feature_configuration, + cc_toolchain = cc_toolchain, + is_linking_dynamic_library = False, + ) + ld_static_lib_path = cc_common.get_tool_for_action( + feature_configuration = feature_configuration, + action_name = CPP_LINK_STATIC_LIBRARY_ACTION_NAME, + ) + env.update(cc_common.get_environment_variables( + feature_configuration = feature_configuration, + action_name = CPP_LINK_STATIC_LIBRARY_ACTION_NAME, + variables = ld_static_lib_variables, + )) + + ld_dynamic_lib_variables = cc_common.create_link_variables( + feature_configuration = feature_configuration, + cc_toolchain = cc_toolchain, + is_linking_dynamic_library = True, + ) + ld_dynamic_lib_path = cc_common.get_tool_for_action( + feature_configuration = feature_configuration, + action_name = CPP_LINK_DYNAMIC_LIBRARY_ACTION_NAME, + ) + ld_dynamic_lib_options = _filter_options( + cc_common.get_memory_inefficient_command_line( + feature_configuration = feature_configuration, + action_name = CPP_LINK_DYNAMIC_LIBRARY_ACTION_NAME, + variables = ld_dynamic_lib_variables, + ), + _LINKER_OPTIONS_BLACKLIST, + ) + env.update(cc_common.get_environment_variables( + feature_configuration = feature_configuration, + action_name = CPP_LINK_DYNAMIC_LIBRARY_ACTION_NAME, + variables = ld_dynamic_lib_variables, + )) - env = {} tags = [] if "gotags" in ctx.var: tags = ctx.var["gotags"].split(",") - apple_ensure_options(ctx, env, tags, compiler_options, linker_options, cpp.target_gnu_system_name) - compiler_path, _ = cpp.ld_executable.rsplit("/", 1) - return struct( + apple_ensure_options( + ctx, + env, + tags, + (c_compile_options, cxx_compile_options), + (ld_executable_options, ld_dynamic_lib_options), + cc_toolchain.target_gnu_system_name, + ) + + # Add C toolchain directories to PATH. + # On ARM, go tool link uses some features of gcc to complete its work, + # so PATH is needed on ARM. + path_set = {} + if "PATH" in env: + for p in env["PATH"].split(ctx.configuration.host_path_separator): + path_set[p] = None + for tool_path in [c_compiler_path, ld_executable_path, ld_static_lib_path, ld_dynamic_lib_path]: + tool_dir, _, _ = tool_path.rpartition("/") + path_set[tool_dir] = None + paths = sorted(path_set.keys()) + if ctx.configuration.host_path_separator == ":": + # HACK: ":" is a proxy for a UNIX-like host. + # The tools returned above may be bash scripts that reference commands + # in directories we might not otherwise include. For example, + # on macOS, wrapped_ar calls dirname. + if "/bin" not in path_set: + paths.append("/bin") + if "/usr/bin" not in path_set: + paths.append("/usr/bin") + env["PATH"] = ctx.configuration.host_path_separator.join(paths) + + return [_GoContextData( strip = ctx.attr.strip, crosstool = ctx.files._cc_toolchain, tags = tags, env = env, cgo_tools = struct( - compiler_path = compiler_path, - compiler_executable = cpp.compiler_executable, - ld_executable = cpp.ld_executable, - compiler_options = compiler_options, - linker_options = linker_options, - options = compiler_options + linker_options, - c_options = cpp.c_options(), + c_compiler_path = c_compiler_path, + c_compile_options = c_compile_options, + cxx_compile_options = cxx_compile_options, + ld_executable_path = ld_executable_path, + ld_executable_options = ld_executable_options, + ld_static_lib_path = ld_static_lib_path, + ld_dynamic_lib_path = ld_dynamic_lib_path, + ld_dynamic_lib_options = ld_dynamic_lib_options, ), - ) + )] go_context_data = rule( - _go_context_data, + _go_context_data_impl, attrs = { "strip": attr.string(mandatory = True), "_cc_toolchain": attr.label(default = "@bazel_tools//tools/cpp:current_cc_toolchain"), diff --git a/go/private/mode.bzl b/go/private/mode.bzl index 2b47c91f38..271bd14113 100644 --- a/go/private/mode.bzl +++ b/go/private/mode.bzl @@ -208,3 +208,30 @@ def link_mode_args(mode): if platform in _LINK_PLUGIN_PLATFORMS: args.append("-dynlink") return args + +def extldflags_from_cc_toolchain(go): + if go.mode.link in (LINKMODE_SHARED, LINKMODE_PLUGIN, LINKMODE_C_SHARED): + return go.cgo_tools.ld_dynamic_lib_options + else: + # NOTE: in c-archive mode, -extldflags are ignored by the linker. + # However, we still need to set them for cgo, which links a binary + # in each package. We use the executable options for this. + return go.cgo_tools.ld_executable_options + +def extld_from_cc_toolchain(go): + if go.mode.link in (LINKMODE_SHARED, LINKMODE_PLUGIN, LINKMODE_C_SHARED): + return ["-extld", go.cgo_tools.ld_dynamic_lib_path] + elif go.mode.link == LINKMODE_C_ARCHIVE: + if go.mode.goos == "darwin": + # TODO(jayconrod): on macOS, set -extar. At this time, wrapped_ar is + # a bash script without a shebang line, so we can't execute it. We + # use /usr/bin/ar (the default) instead. + return [] + else: + return ["-extar", go.cgo_tools.ld_static_lib_path] + else: + # NOTE: In c-archive mode, we should probably set -extar. However, + # on macOS, Bazel returns wrapped_ar, which is not executable. + # /usr/bin/ar (the default) should be visible though, and we have a + # hack in link.go to strip out non-reproducible stuff. + return ["-extld", go.cgo_tools.ld_executable_path] diff --git a/go/private/rules/cgo.bzl b/go/private/rules/cgo.bzl index 7a0a0117b1..71d1a49183 100644 --- a/go/private/rules/cgo.bzl +++ b/go/private/rules/cgo.bzl @@ -39,13 +39,14 @@ load( "@io_bazel_rules_go//go/private:mode.bzl", "LINKMODE_C_ARCHIVE", "LINKMODE_C_SHARED", + "extldflags_from_cc_toolchain", "mode_string", "new_mode", ) load( "@io_bazel_rules_go//go/platform:list.bzl", - "GOOS", "GOARCH", + "GOOS", "GOOS_GOARCH", "MSAN_GOOS_GOARCH", "RACE_GOOS_GOARCH", @@ -143,9 +144,9 @@ def _cgo_codegen_impl(ctx): go = go_context(ctx) if not go.cgo_tools: fail("Go toolchain does not support cgo") - linkopts = go.cgo_tools.linker_options + ctx.attr.linkopts - cppopts = go.cgo_tools.compiler_options + ctx.attr.cppopts - copts = go.cgo_tools.c_options + ctx.attr.copts + linkopts = extldflags_from_cc_toolchain(go) + ctx.attr.linkopts + cppopts = list(ctx.attr.cppopts) + copts = go.cgo_tools.c_compile_options + ctx.attr.copts deps = depset([], order = "topological") cgo_export_h = go.declare_file(go, path = "_cgo_export.h") cgo_export_c = go.declare_file(go, path = "_cgo_export.c") @@ -262,7 +263,7 @@ def _cgo_codegen_impl(ctx): # TODO(jayconrod): do we need to set this here, or only in _cgo_import? # go build does it here. env = go.env - env["CC"] = go.cgo_tools.compiler_executable + env["CC"] = go.cgo_tools.c_compiler_path env["CGO_LDFLAGS"] = " ".join(linkopts) cc_args.add_all(cppopts) diff --git a/go/private/rules/stdlib.bzl b/go/private/rules/stdlib.bzl index 0269f68e2f..164ce4bc8e 100644 --- a/go/private/rules/stdlib.bzl +++ b/go/private/rules/stdlib.bzl @@ -26,8 +26,9 @@ load( ) load( "@io_bazel_rules_go//go/private:mode.bzl", - "link_mode_args", "LINKMODE_NORMAL", + "extldflags_from_cc_toolchain", + "link_mode_args", ) def _stdlib_library_to_source(go, attr, source, merge): @@ -64,10 +65,9 @@ def _build_stdlib(go, attr): go.actions.write(root_file, "") env = go.env env.update({ - "CC": go.cgo_tools.compiler_executable, - "CGO_CPPFLAGS": " ".join(go.cgo_tools.compiler_options), - "CGO_CFLAGS": " ".join(go.cgo_tools.c_options), - "CGO_LDFLAGS": " ".join(go.cgo_tools.linker_options), + "CC": go.cgo_tools.c_compiler_path, + "CGO_CFLAGS": " ".join(go.cgo_tools.c_compile_options), + "CGO_LDFLAGS": " ".join(extldflags_from_cc_toolchain(go)), }) inputs = (go.sdk.srcs + go.sdk.headers + diff --git a/go/tools/builders/env.go b/go/tools/builders/env.go index 762b596915..d3f1f09990 100644 --- a/go/tools/builders/env.go +++ b/go/tools/builders/env.go @@ -187,7 +187,15 @@ func splitArgs(args []string) (builderArgs []string, toolArgs []string) { // absolute paths to work correctly. Most notably, golang on Windows cannot // handle relative paths to files whose absolute path is > ~250 chars, while // it can handle absolute paths. See http://goo.gl/eqeWjm. +// +// Note that strings that begin with "__BAZEL_" are not absolutized. These are +// used on macOS for paths that the compiler wrapper (wrapped_clang) is +// supposed to know about. func abs(path string) string { + if strings.HasPrefix(path, "__BAZEL_") { + return path + } + if abs, err := filepath.Abs(path); err != nil { return path } else { diff --git a/proto/compiler.bzl b/proto/compiler.bzl index 261eb6140b..200173086a 100644 --- a/proto/compiler.bzl +++ b/proto/compiler.bzl @@ -41,7 +41,9 @@ def go_proto_compile(go, compiler, proto, imports, importpath): args.add("-importpath", importpath) args.add("-out_path", outpath) args.add("-plugin", compiler.plugin) - args.add("-compiler_path", go.cgo_tools.compiler_path) + + # TODO(jayconrod): can we just use go.env instead? + args.add("-compiler_path", go.cgo_tools.c_compiler_path.rpartition("/")[0]) args.add_all(compiler.options, before_each = "--option") if compiler.import_path_option: args.add_all([importpath], before_each = "--option", format_each = "import_path=%s")