diff --git a/compile.sh b/compile.sh index 4712355d48a6c6..f8cd131c0f20cd 100755 --- a/compile.sh +++ b/compile.sh @@ -61,7 +61,7 @@ if [[ $PLATFORM == "darwin" ]] && \ fi if [[ $PLATFORM == "windows" ]]; then - EXTRA_BAZEL_ARGS="${EXTRA_BAZEL_ARGS-} --cxxopt=/std:c++17 --host_cxxopt=/std:c++17" + EXTRA_BAZEL_ARGS="${EXTRA_BAZEL_ARGS-} --cxxopt=/std:c++17 --host_cxxopt=/std:c++17 --copt=/external:W0 --host_copt=/external:W0" fi source scripts/bootstrap/bootstrap.sh diff --git a/scripts/bootstrap/bootstrap.sh b/scripts/bootstrap/bootstrap.sh index 978f0d0ce5005b..9c779678010e4d 100755 --- a/scripts/bootstrap/bootstrap.sh +++ b/scripts/bootstrap/bootstrap.sh @@ -44,6 +44,7 @@ _BAZEL_ARGS="--spawn_strategy=standalone \ --enable_bzlmod \ --check_direct_dependencies=error \ --lockfile_mode=update \ + --features=external_include_paths --host_features=external_include_paths \ --override_repository=$(cat derived/maven/MAVEN_CANONICAL_REPO_NAME)=derived/maven \ --java_runtime_version=${JAVA_VERSION} \ --java_language_version=${JAVA_VERSION} \ diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java index 58b7b58e75cae7..f803a02216bffa 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java @@ -1133,7 +1133,10 @@ public void validateInclusions( private Iterable getValidationIgnoredDirs() { List cxxSystemIncludeDirs = getBuiltInIncludeDirectories(); - return Iterables.concat(cxxSystemIncludeDirs, ccCompilationContext.getSystemIncludeDirs()); + return Iterables.concat( + cxxSystemIncludeDirs, + ccCompilationContext.getSystemIncludeDirs(), + ccCompilationContext.getExternalIncludeDirs()); } @VisibleForTesting diff --git a/src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl b/src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl index 57d551c45842a7..12ab5780bddd36 100644 --- a/src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl +++ b/src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl @@ -510,7 +510,7 @@ def cc_binary_impl(ctx, additional_linkopts, force_linkstatic = False): cxx_flags = cc_helper.get_copts(ctx, feature_configuration, additional_make_variable_substitutions, attr = "cxxopts"), defines = cc_helper.defines(ctx, additional_make_variable_substitutions), local_defines = cc_helper.local_defines(ctx, additional_make_variable_substitutions) + cc_helper.get_local_defines_for_runfiles_lookup(ctx, ctx.attr.deps), - system_includes = cc_helper.system_include_dirs(ctx, additional_make_variable_substitutions), + includes = cc_helper.include_dirs(ctx, additional_make_variable_substitutions), private_hdrs = cc_helper.get_private_hdrs(ctx), public_hdrs = cc_helper.get_public_hdrs(ctx), copts_filter = cc_helper.copts_filter(ctx, additional_make_variable_substitutions), diff --git a/src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl b/src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl index ebca20e41e0986..92e58eb193833c 100644 --- a/src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl +++ b/src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl @@ -1062,7 +1062,7 @@ def _package_source_root(repository, package, sibling_repository_layout): repository = repository[1:] return paths.get_relative(paths.get_relative("external", repository), package) -def _system_include_dirs(ctx, additional_make_variable_substitutions): +def _include_dirs(ctx, additional_make_variable_substitutions): result = [] sibling_repository_layout = ctx.configuration.is_sibling_repository_layout() package = ctx.label.package @@ -1264,7 +1264,8 @@ cc_helper = struct( get_private_hdrs = _get_private_hdrs, get_public_hdrs = _get_public_hdrs, report_invalid_options = _report_invalid_options, - system_include_dirs = _system_include_dirs, + include_dirs = _include_dirs, + system_include_dirs = _include_dirs, # TODO: Remove uses of old name get_coverage_environment = _get_coverage_environment, create_cc_instrumented_files_info = _create_cc_instrumented_files_info, linkopts = _linkopts, diff --git a/src/main/starlark/builtins_bzl/common/cc/cc_library.bzl b/src/main/starlark/builtins_bzl/common/cc/cc_library.bzl index 70a8c843e49701..d29ad9d2b8b848 100755 --- a/src/main/starlark/builtins_bzl/common/cc/cc_library.bzl +++ b/src/main/starlark/builtins_bzl/common/cc/cc_library.bzl @@ -63,7 +63,7 @@ def _cc_library_impl(ctx): cxx_flags = cc_helper.get_copts(ctx, feature_configuration, additional_make_variable_substitutions, attr = "cxxopts"), defines = cc_helper.defines(ctx, additional_make_variable_substitutions), local_defines = cc_helper.local_defines(ctx, additional_make_variable_substitutions) + cc_helper.get_local_defines_for_runfiles_lookup(ctx, ctx.attr.deps + ctx.attr.implementation_deps), - system_includes = cc_helper.system_include_dirs(ctx, additional_make_variable_substitutions), + includes = cc_helper.include_dirs(ctx, additional_make_variable_substitutions), copts_filter = cc_helper.copts_filter(ctx, additional_make_variable_substitutions), purpose = "cc_library-compile", srcs = cc_helper.get_srcs(ctx), diff --git a/src/main/starlark/builtins_bzl/common/cc/compile/cc_compilation_helper.bzl b/src/main/starlark/builtins_bzl/common/cc/compile/cc_compilation_helper.bzl index b836938df8585f..3033e803fcb0d7 100644 --- a/src/main/starlark/builtins_bzl/common/cc/compile/cc_compilation_helper.bzl +++ b/src/main/starlark/builtins_bzl/common/cc/compile/cc_compilation_helper.bzl @@ -227,7 +227,10 @@ def _init_cc_compilation_context( external_include_dirs = [] declared_include_srcs = [] - if not external: + if not external and feature_configuration.is_requested("system_include_paths"): + system_include_dirs_for_context = system_include_dirs + include_dirs + include_dirs_for_context = [] + elif not external: system_include_dirs_for_context = list(system_include_dirs) include_dirs_for_context = list(include_dirs) else: diff --git a/src/main/starlark/builtins_bzl/common/objc/compilation_support.bzl b/src/main/starlark/builtins_bzl/common/objc/compilation_support.bzl index 5e22c1c3f66c0b..e1a7f40454faeb 100644 --- a/src/main/starlark/builtins_bzl/common/objc/compilation_support.bzl +++ b/src/main/starlark/builtins_bzl/common/objc/compilation_support.bzl @@ -105,7 +105,7 @@ def _build_common_variables( has_module_map = has_module_map, attr_linkopts = attr_linkopts, direct_cc_compilation_contexts = direct_cc_compilation_contexts, - includes = cc_helper.system_include_dirs(ctx, {}) if hasattr(ctx.attr, "includes") else [], + includes = cc_helper.include_dirs(ctx, {}) if hasattr(ctx.attr, "includes") else [], ) return struct( diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java index 3f8d7dd0df31f4..cd7d609ed9c954 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java @@ -573,7 +573,7 @@ public void testIsolatedIncludes() throws Exception { ConfiguredTarget foo = getConfiguredTarget("//bang:bang"); String includesRoot = "bang/bang_includes"; - assertThat(foo.get(CcInfo.PROVIDER).getCcCompilationContext().getSystemIncludeDirs()) + assertThat(foo.get(CcInfo.PROVIDER).getCcCompilationContext().getIncludeDirs()) .containsAtLeast( PathFragment.create(includesRoot), targetConfig.getGenfilesFragment(RepositoryName.MAIN).getRelative(includesRoot)); @@ -596,12 +596,12 @@ public void testDisabledGenfilesDontShowUpInSystemIncludePaths() throws Exceptio ConfiguredTarget foo = getConfiguredTarget("//bang:bang"); PathFragment genfilesDir = targetConfig.getGenfilesFragment(RepositoryName.MAIN).getRelative(includesRoot); - assertThat(foo.get(CcInfo.PROVIDER).getCcCompilationContext().getSystemIncludeDirs()) + assertThat(foo.get(CcInfo.PROVIDER).getCcCompilationContext().getIncludeDirs()) .contains(genfilesDir); useConfiguration("--incompatible_merge_genfiles_directory"); foo = getConfiguredTarget("//bang:bang"); - assertThat(foo.get(CcInfo.PROVIDER).getCcCompilationContext().getSystemIncludeDirs()) + assertThat(foo.get(CcInfo.PROVIDER).getCcCompilationContext().getIncludeDirs()) .doesNotContain(genfilesDir); } @@ -626,6 +626,7 @@ public void testUseIsystemForIncludes() throws Exception { name = "bang", srcs = ["bang.cc"], includes = ["bang_includes"], + features = ["system_include_paths"], ) """); @@ -644,6 +645,45 @@ public void testUseIsystemForIncludes() throws Exception { .containsExactlyElementsIn(expected); } + @Test + public void testUseIForIncludes() throws Exception { + // Tests the effect of --use_isystem_for_includes. + useConfiguration("--incompatible_merge_genfiles_directory=false"); + scratch.file( + "no_includes/BUILD", + """ + cc_library( + name = "no_includes", + srcs = ["no_includes.cc"], + ) + """); + ConfiguredTarget noIncludes = getConfiguredTarget("//no_includes:no_includes"); + + scratch.file( + "bang/BUILD", + """ + cc_library( + name = "bang", + srcs = ["bang.cc"], + includes = ["bang_includes"], + ) + """); + + ConfiguredTarget foo = getConfiguredTarget("//bang:bang"); + + String includesRoot = "bang/bang_includes"; + List expected = + new ImmutableList.Builder() + .addAll( + noIncludes.get(CcInfo.PROVIDER).getCcCompilationContext().getIncludeDirs()) + .add(PathFragment.create(includesRoot)) + .add(targetConfig.getGenfilesFragment(RepositoryName.MAIN).getRelative(includesRoot)) + .add(targetConfig.getBinFragment(RepositoryName.MAIN).getRelative(includesRoot)) + .build(); + assertThat(foo.get(CcInfo.PROVIDER).getCcCompilationContext().getIncludeDirs()) + .containsExactlyElementsIn(expected); + } + @Test public void testCcTestDisallowsAlwaysLink() throws Exception { scratch.file( diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java index 122518411d4057..2be34f25107732 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java @@ -1196,18 +1196,12 @@ public void testIncludePathOrder() throws Exception { assertContainsSublist( action.getCompilerOptions(), ImmutableList.of( - "-isystem", - "foo/foo", - "-isystem", - genfilesDir + "/foo/foo", - "-isystem", - binDir + "/foo/foo", - "-isystem", - "foo/bar", - "-isystem", - genfilesDir + "/foo/bar", - "-isystem", - binDir + "/foo/bar")); + "-Ifoo/foo", + "-I" + genfilesDir + "/foo/foo", + "-I" + binDir + "/foo/foo", + "-Ifoo/bar", + "-I" + genfilesDir + "/foo/bar", + "-I" + binDir + "/foo/bar")); } @Test @@ -1911,10 +1905,12 @@ public void testImplementationDepsCompilationContextIsNotPropagated() throws Exc assertThat(artifactsToStrings(libCompilationContext.getDeclaredIncludeSrcs())) .doesNotContain("src foo/implementation_dep.h"); - assertThat(pathfragmentsToStrings(libCompilationContext.getSystemIncludeDirs())) + assertThat(pathfragmentsToStrings(libCompilationContext.getIncludeDirs())) .contains("foo/public_dep"); - assertThat(pathfragmentsToStrings(libCompilationContext.getSystemIncludeDirs())) + assertThat(pathfragmentsToStrings(libCompilationContext.getIncludeDirs())) .contains("foo/interface_dep"); + assertThat(pathfragmentsToStrings(libCompilationContext.getIncludeDirs())) + .doesNotContain("foo/implementation_dep"); assertThat(pathfragmentsToStrings(libCompilationContext.getSystemIncludeDirs())) .doesNotContain("foo/implementation_dep"); @@ -1922,11 +1918,11 @@ public void testImplementationDepsCompilationContextIsNotPropagated() throws Exc getCppCompileAction("//foo:public_dep").getCcCompilationContext(); assertThat(artifactsToStrings(publicDepCompilationContext.getDeclaredIncludeSrcs())) .contains("src foo/interface_dep.h"); - assertThat(pathfragmentsToStrings(publicDepCompilationContext.getSystemIncludeDirs())) + assertThat(pathfragmentsToStrings(publicDepCompilationContext.getIncludeDirs())) .contains("foo/interface_dep"); assertThat(artifactsToStrings(publicDepCompilationContext.getDeclaredIncludeSrcs())) .contains("src foo/implementation_dep.h"); - assertThat(pathfragmentsToStrings(publicDepCompilationContext.getSystemIncludeDirs())) + assertThat(pathfragmentsToStrings(publicDepCompilationContext.getIncludeDirs())) .contains("foo/implementation_dep"); } diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/StarlarkCcCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/StarlarkCcCommonTest.java index 7d989167af2eee..bc5d5bde423ffe 100755 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/StarlarkCcCommonTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/StarlarkCcCommonTest.java @@ -1398,11 +1398,11 @@ def _impl(ctx): List mergedSystemIncludes = ((Depset) myInfo.getValue("merged_system_includes")).getSet(String.class).toList(); - assertThat(mergedSystemIncludes).containsAtLeast("foo/bar", "a/dep1/baz", "a/dep2/qux"); + assertThat(mergedSystemIncludes).contains("foo/bar"); List mergedIncludes = ((Depset) myInfo.getValue("merged_includes")).getSet(String.class).toList(); - assertThat(mergedIncludes).contains("baz/qux"); + assertThat(mergedIncludes).containsAtLeast("baz/qux", "a/dep1/baz", "a/dep2/qux"); List mergedQuoteIncludes = ((Depset) myInfo.getValue("merged_quote_includes")).getSet(String.class).toList(); diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java index d8c70c0042af42..7161ea8b4df47a 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java @@ -1397,7 +1397,7 @@ public void testIncludesDirsOfTransitiveCcDepsGetPassedToCompileAction() throws Iterables.concat( Iterables.transform( rootedIncludePaths("package/foo/bar"), - element -> ImmutableList.of("-isystem", element))))); + element -> ImmutableList.of("-I" + element))))); } @Test