diff options
author | cpeyser <cpeyser@google.com> | 2017-11-14 08:30:01 -0800 |
---|---|---|
committer | Copybara-Service <copybara-piper@google.com> | 2017-11-14 08:31:57 -0800 |
commit | a9808c08c793b96fc9259efa06be5bdbbb9b0316 (patch) | |
tree | 9083de4dc2f9bf7f98ec1cce42bb3a346147f7b3 | |
parent | 4fd788115bdd3e4fa3861bcb4b9caab480543da5 (diff) |
Move CppConfiguration#getBuiltinIncludeDirectories to CcToolchainProvider.
PiperOrigin-RevId: 175682806
3 files changed, 149 insertions, 88 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java index 860983a233..f7fc13e4dc 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java @@ -43,6 +43,8 @@ import com.google.devtools.build.lib.analysis.config.CompilationMode; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.cmdline.LabelSyntaxException; +import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; @@ -95,6 +97,85 @@ public class CcToolchain implements RuleConfiguredTargetFactory { } } + private static final String SYSROOT_START = "%sysroot%/"; + private static final String WORKSPACE_START = "%workspace%/"; + private static final String CROSSTOOL_START = "%crosstool_top%/"; + private static final String PACKAGE_START = "%package("; + private static final String PACKAGE_END = ")%"; + + /** + * Resolve the given include directory. + * + * <p>If it starts with %sysroot%/, that part is replaced with the actual sysroot. + * + * <p>If it starts with %workspace%/, that part is replaced with the empty string (essentially + * making it relative to the build directory). + * + * <p>If it starts with %crosstool_top%/ or is any relative path, it is interpreted relative to + * the crosstool top. The use of assumed-crosstool-relative specifications is considered + * deprecated, and all such uses should eventually be replaced by "%crosstool_top%/". + * + * <p>If it is of the form %package(@repository//my/package)%/folder, then it is interpreted as + * the named folder in the appropriate package. All of the normal package syntax is supported. The + * /folder part is optional. + * + * <p>It is illegal if it starts with a % and does not match any of the above forms to avoid + * accidentally silently ignoring misspelled prefixes. + * + * <p>If it is absolute, it remains unchanged. + */ + static PathFragment resolveIncludeDir( + String s, PathFragment sysroot, PathFragment crosstoolTopPathFragment) + throws InvalidConfigurationException { + PathFragment pathPrefix; + String pathString; + int packageEndIndex = s.indexOf(PACKAGE_END); + if (packageEndIndex != -1 && s.startsWith(PACKAGE_START)) { + String packageString = s.substring(PACKAGE_START.length(), packageEndIndex); + try { + pathPrefix = PackageIdentifier.parse(packageString).getSourceRoot(); + } catch (LabelSyntaxException e) { + throw new InvalidConfigurationException("The package '" + packageString + "' is not valid"); + } + int pathStartIndex = packageEndIndex + PACKAGE_END.length(); + if (pathStartIndex + 1 < s.length()) { + if (s.charAt(pathStartIndex) != '/') { + throw new InvalidConfigurationException( + "The path in the package for '" + s + "' is not valid"); + } + pathString = s.substring(pathStartIndex + 1, s.length()); + } else { + pathString = ""; + } + } else if (s.startsWith(SYSROOT_START)) { + if (sysroot == null) { + throw new InvalidConfigurationException( + "A %sysroot% prefix is only allowed if the " + "default_sysroot option is set"); + } + pathPrefix = sysroot; + pathString = s.substring(SYSROOT_START.length(), s.length()); + } else if (s.startsWith(WORKSPACE_START)) { + pathPrefix = PathFragment.EMPTY_FRAGMENT; + pathString = s.substring(WORKSPACE_START.length(), s.length()); + } else { + pathPrefix = crosstoolTopPathFragment; + if (s.startsWith(CROSSTOOL_START)) { + pathString = s.substring(CROSSTOOL_START.length(), s.length()); + } else if (s.startsWith("%")) { + throw new InvalidConfigurationException( + "The include path '" + s + "' has an " + "unrecognized %prefix%"); + } else { + pathString = s; + } + } + + PathFragment path = PathFragment.create(pathString); + if (!path.isNormalized()) { + throw new InvalidConfigurationException("The include path '" + s + "' is not normalized."); + } + return pathPrefix.getRelative(path); + } + /* * This function checks the format of the input profile data and converts it to * the indexed format (.profdata) if necessary. @@ -383,12 +464,17 @@ public class CcToolchain implements RuleConfiguredTargetFactory { PathFragment sysroot = calculateSysroot(ruleContext, toolchainInfo.getDefaultSysroot()); - ImmutableList<PathFragment> builtInIncludeDirectories = null; - try { - builtInIncludeDirectories = cppConfiguration.getBuiltInIncludeDirectories(sysroot); - } catch (InvalidConfigurationException e) { - ruleContext.ruleError(e.getMessage()); + ImmutableList.Builder<PathFragment> builtInIncludeDirectoriesBuilder = ImmutableList.builder(); + for (String s : toolchainInfo.getRawBuiltInIncludeDirectories()) { + try { + builtInIncludeDirectoriesBuilder.add( + resolveIncludeDir(s, sysroot, toolchainInfo.getCrosstoolTopPathFragment())); + } catch (InvalidConfigurationException e) { + ruleContext.ruleError(e.getMessage()); + } } + ImmutableList<PathFragment> builtInIncludeDirectories = + builtInIncludeDirectoriesBuilder.build(); coverageEnvironment.add( Pair.of( diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java index 653ab3727e..b2faac2146 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java @@ -33,8 +33,6 @@ import com.google.devtools.build.lib.analysis.config.InvalidConfigurationExcepti import com.google.devtools.build.lib.analysis.config.PatchTransition; import com.google.devtools.build.lib.analysis.config.PerLabelOptions; import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.build.lib.cmdline.LabelSyntaxException; -import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; @@ -423,85 +421,6 @@ public class CppConfiguration extends BuildConfiguration.Fragment { return LinkingMode.valueOf(mode.name()); } - private static final String SYSROOT_START = "%sysroot%/"; - private static final String WORKSPACE_START = "%workspace%/"; - private static final String CROSSTOOL_START = "%crosstool_top%/"; - private static final String PACKAGE_START = "%package("; - private static final String PACKAGE_END = ")%"; - - /** - * Resolve the given include directory. - * - * <p>If it starts with %sysroot%/, that part is replaced with the actual sysroot. - * - * <p>If it starts with %workspace%/, that part is replaced with the empty string - * (essentially making it relative to the build directory). - * - * <p>If it starts with %crosstool_top%/ or is any relative path, it is - * interpreted relative to the crosstool top. The use of assumed-crosstool-relative - * specifications is considered deprecated, and all such uses should eventually - * be replaced by "%crosstool_top%/". - * - * <p>If it is of the form %package(@repository//my/package)%/folder, then it is - * interpreted as the named folder in the appropriate package. All of the normal - * package syntax is supported. The /folder part is optional. - * - * <p>It is illegal if it starts with a % and does not match any of the above - * forms to avoid accidentally silently ignoring misspelled prefixes. - * - * <p>If it is absolute, it remains unchanged. - */ - static PathFragment resolveIncludeDir(String s, PathFragment sysroot, - PathFragment crosstoolTopPathFragment) throws InvalidConfigurationException { - PathFragment pathPrefix; - String pathString; - int packageEndIndex = s.indexOf(PACKAGE_END); - if (packageEndIndex != -1 && s.startsWith(PACKAGE_START)) { - String packageString = s.substring(PACKAGE_START.length(), packageEndIndex); - try { - pathPrefix = PackageIdentifier.parse(packageString).getSourceRoot(); - } catch (LabelSyntaxException e) { - throw new InvalidConfigurationException("The package '" + packageString + "' is not valid"); - } - int pathStartIndex = packageEndIndex + PACKAGE_END.length(); - if (pathStartIndex + 1 < s.length()) { - if (s.charAt(pathStartIndex) != '/') { - throw new InvalidConfigurationException( - "The path in the package for '" + s + "' is not valid"); - } - pathString = s.substring(pathStartIndex + 1, s.length()); - } else { - pathString = ""; - } - } else if (s.startsWith(SYSROOT_START)) { - if (sysroot == null) { - throw new InvalidConfigurationException("A %sysroot% prefix is only allowed if the " - + "default_sysroot option is set"); - } - pathPrefix = sysroot; - pathString = s.substring(SYSROOT_START.length(), s.length()); - } else if (s.startsWith(WORKSPACE_START)) { - pathPrefix = PathFragment.EMPTY_FRAGMENT; - pathString = s.substring(WORKSPACE_START.length(), s.length()); - } else { - pathPrefix = crosstoolTopPathFragment; - if (s.startsWith(CROSSTOOL_START)) { - pathString = s.substring(CROSSTOOL_START.length(), s.length()); - } else if (s.startsWith("%")) { - throw new InvalidConfigurationException( - "The include path '" + s + "' has an " + "unrecognized %prefix%"); - } else { - pathString = s; - } - } - - PathFragment path = PathFragment.create(pathString); - if (!path.isNormalized()) { - throw new InvalidConfigurationException("The include path '" + s + "' is not normalized."); - } - return pathPrefix.getRelative(path); - } - static ImmutableList<OptionalFlag> convertOptionalOptions( List<CrosstoolConfig.CToolchain.OptionalFlag> optionalFlagList) { ImmutableList.Builder<OptionalFlag> result = ImmutableList.builder(); @@ -684,12 +603,16 @@ public class CppConfiguration extends BuildConfiguration.Fragment { * Returns the built-in list of system include paths for the toolchain compiler. All paths in this * list should be relative to the exec directory. They may be absolute if they are also installed * on the remote build nodes or for local compilation. + * + * <p>TODO(b/64384912): Migrate skylark callers to + * CcToolchainProvider#getBuiltinIncludeDirectories and delete this method. */ - public ImmutableList<PathFragment> getBuiltInIncludeDirectories(PathFragment sysroot) + private ImmutableList<PathFragment> getBuiltInIncludeDirectories(PathFragment sysroot) throws InvalidConfigurationException { ImmutableList.Builder<PathFragment> builtInIncludeDirectoriesBuilder = ImmutableList.builder(); for (String s : cppToolchainInfo.getRawBuiltInIncludeDirectories()) { - builtInIncludeDirectoriesBuilder.add(resolveIncludeDir(s, sysroot, crosstoolTopPathFragment)); + builtInIncludeDirectoriesBuilder.add( + CcToolchain.resolveIncludeDir(s, sysroot, crosstoolTopPathFragment)); } return builtInIncludeDirectoriesBuilder.build(); } diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java index 1a29eb551f..d0b6c04f53 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.rules.cpp; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.fail; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.analysis.ConfiguredTarget; @@ -84,6 +85,57 @@ public class CcToolchainTest extends BuildViewTestCase { getConfiguredTarget("//a:a"); } + public void assertInvalidIncludeDirectoryMessage(String entry, String messageRegex) + throws Exception { + try { + scratch.overwriteFile( + "a/BUILD", + "filegroup(", + " name='empty')", + "cc_toolchain(", + " name = 'b',", + " cpu = 'k8',", + " all_files = ':banana',", + " compiler_files = ':empty',", + " dwp_files = ':empty',", + " linker_files = ':empty',", + " strip_files = ':empty',", + " objcopy_files = ':empty',", + " dynamic_runtime_libs = [':empty'],", + " static_runtime_libs = [':empty'])"); + + getAnalysisMock() + .ccSupport() + .setupCrosstool( + mockToolsConfig, + CrosstoolConfig.CToolchain.newBuilder() + .addCxxBuiltinIncludeDirectory(entry) + .buildPartial()); + + useConfiguration(); + + ConfiguredTarget target = getConfiguredTarget("//a:b"); + CcToolchainProvider toolchainProvider = + (CcToolchainProvider) target.get(ToolchainInfo.PROVIDER); + // Must call this function to actually see if there's an error with the directories. + toolchainProvider.getBuiltInIncludeDirectories(); + + fail("C++ configuration creation succeeded unexpectedly"); + } catch (AssertionError e) { + assertThat(e).hasMessageThat().containsMatch(messageRegex); + } + } + + @Test + public void testInvalidIncludeDirectory() throws Exception { + assertInvalidIncludeDirectoryMessage("%package(//a", "has an unrecognized %prefix%"); + assertInvalidIncludeDirectoryMessage("%package(//a@@a)%", "The package '//a@@a' is not valid"); + assertInvalidIncludeDirectoryMessage( + "%package(//a)%foo", "The path in the package.*is not valid"); + assertInvalidIncludeDirectoryMessage( + "%package(//a)%/../bar", "The include path.*is not normalized"); + } + @Test public void testModuleMapAttribute() throws Exception { scratchConfiguredTarget("modules/map", "c", |