diff options
5 files changed, 137 insertions, 63 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java index 94b085d51f..b3fd9279d9 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java @@ -22,6 +22,7 @@ import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.bazel.rules.android.ndkcrosstools.AndroidNdkCrosstools; import com.google.devtools.build.lib.bazel.rules.android.ndkcrosstools.AndroidNdkCrosstools.NdkCrosstoolsException; import com.google.devtools.build.lib.bazel.rules.android.ndkcrosstools.ApiLevel; +import com.google.devtools.build.lib.bazel.rules.android.ndkcrosstools.NdkMajorRevision; import com.google.devtools.build.lib.bazel.rules.android.ndkcrosstools.NdkPaths; import com.google.devtools.build.lib.bazel.rules.android.ndkcrosstools.NdkRelease; import com.google.devtools.build.lib.bazel.rules.android.ndkcrosstools.StlImpl; @@ -63,7 +64,7 @@ public class AndroidNdkRepositoryFunction extends RepositoryFunction { private static final PathFragment PLATFORMS_DIR = new PathFragment("platforms"); private static final Iterable<String> PATH_ENV_VAR_AS_LIST = ImmutableList.of(PATH_ENV_VAR); - + private static final class CrosstoolStlPair { private final CrosstoolRelease crosstoolRelease; @@ -165,26 +166,36 @@ public class AndroidNdkRepositoryFunction extends RepositoryFunction { apiLevelString = apiLevels.first().toString(); } - ApiLevel apiLevel; - apiLevel = - AndroidNdkCrosstools.KNOWN_NDK_MAJOR_REVISIONS - .get(ndkRelease.majorRevision) - .apiLevel(env.getListener(), ruleName, apiLevelString); - + // NDK minor revisions should be backwards compatible within a major revision, the crosstools + // we generate don't care about the minor revision. + NdkMajorRevision ndkMajorRevision; if (!ndkRelease.isValid) { - env.getListener().handle(Event.warn(String.format( - "The revision of the Android NDK given in android_ndk_repository rule '%s' could not be " - + "determined (the revision string found is '%s'). " - + "Defaulting to Android NDK revision %s", ruleName, ndkRelease.rawRelease, - AndroidNdkCrosstools.LATEST_KNOWN_REVISION))); + String warningMessage = + String.format( + "The revision of the Android NDK referenced by android_ndk_repository rule '%s' " + + "could not be determined (the revision string found is '%s'). Defaulting to " + + "revision %s.", + ruleName, ndkRelease.rawRelease, AndroidNdkCrosstools.LATEST_KNOWN_REVISION.getKey()); + env.getListener().handle(Event.warn(warningMessage)); + ndkMajorRevision = AndroidNdkCrosstools.LATEST_KNOWN_REVISION.getValue(); + } else if (!AndroidNdkCrosstools.isKnownNDKRevision(ndkRelease)) { + String warningMessage = + String.format( + "The major revision of the Android NDK referenced by android_ndk_repository rule " + + "'%s' is %s. The major revisions supported by Bazel are %s. Defaulting to " + + "revision %s.", + ruleName, + ndkRelease.majorRevision, + AndroidNdkCrosstools.KNOWN_NDK_MAJOR_REVISIONS.keySet(), + AndroidNdkCrosstools.LATEST_KNOWN_REVISION.getKey()); + env.getListener().handle(Event.warn(warningMessage)); + ndkMajorRevision = AndroidNdkCrosstools.LATEST_KNOWN_REVISION.getValue(); + } else { + ndkMajorRevision = + AndroidNdkCrosstools.KNOWN_NDK_MAJOR_REVISIONS.get(ndkRelease.majorRevision); } - if (!AndroidNdkCrosstools.isKnownNDKRevision(ndkRelease)) { - env.getListener().handle(Event.warn(String.format( - "Bazel Android NDK crosstools are based on Android NDK revision %s. " - + "The revision of the Android NDK given in android_ndk_repository rule '%s' is '%s'", - AndroidNdkCrosstools.LATEST_KNOWN_REVISION, ruleName, ndkRelease.rawRelease))); - } + ApiLevel apiLevel = ndkMajorRevision.apiLevel(env.getListener(), ruleName, apiLevelString); ImmutableList.Builder<CrosstoolStlPair> crosstoolsAndStls = ImmutableList.builder(); try { @@ -193,13 +204,8 @@ public class AndroidNdkRepositoryFunction extends RepositoryFunction { NdkPaths ndkPaths = new NdkPaths(ruleName, hostPlatform, apiLevel); for (StlImpl stlImpl : StlImpls.get(ndkPaths)) { - - CrosstoolRelease crosstoolRelease = AndroidNdkCrosstools.create( - ndkRelease, - ndkPaths, - stlImpl, - hostPlatform); - + CrosstoolRelease crosstoolRelease = + ndkMajorRevision.crosstoolRelease(ndkPaths, stlImpl, hostPlatform); crosstoolsAndStls.add(new CrosstoolStlPair(crosstoolRelease, stlImpl)); } @@ -246,7 +252,7 @@ public class AndroidNdkRepositoryFunction extends RepositoryFunction { } String toolchainName = createToolchainName(crosstoolStlPair.stlImpl.getName()); - + ccToolchainSuites.append(ccToolchainSuiteTemplate .replace("%toolchainName%", toolchainName) .replace("%toolchainMap%", toolchainMap.toString().trim()) @@ -277,7 +283,7 @@ public class AndroidNdkRepositoryFunction extends RepositoryFunction { static String createToolchainName(String stlName) { return TOOLCHAIN_NAME_PREFIX + stlName; } - + private static String createCcToolchainRule(String ccToolchainTemplate, CToolchain toolchain) { // TODO(bazel-team): It's unfortunate to have to extract data from a CToolchain proto like this. @@ -345,7 +351,7 @@ public class AndroidNdkRepositoryFunction extends RepositoryFunction { // For NDK r10e releaseFilePath = directory.getRelative("RELEASE.TXT"); } - + SkyKey releaseFileKey = FileValue.key(RootedPath.toRootedPath(directory, releaseFilePath)); String releaseFileContent; diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/AndroidNdkCrosstools.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/AndroidNdkCrosstools.java index 187f005028..fab8dfbbce 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/AndroidNdkCrosstools.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/AndroidNdkCrosstools.java @@ -21,7 +21,7 @@ import com.google.devtools.build.lib.bazel.rules.android.ndkcrosstools.r11.NdkMa import com.google.devtools.build.lib.bazel.rules.android.ndkcrosstools.r12.NdkMajorRevisionR12; import com.google.devtools.build.lib.bazel.rules.android.ndkcrosstools.r13.NdkMajorRevisionR13; import com.google.devtools.build.lib.util.OS; -import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CrosstoolRelease; +import java.util.Map; /** * Helper methods for generating a CrosstoolRelease proto for the Android NDK based on a particular @@ -38,8 +38,8 @@ public final class AndroidNdkCrosstools { "11", new NdkMajorRevisionR11(), "12", new NdkMajorRevisionR12(), "13", new NdkMajorRevisionR13()); - public static final String LATEST_KNOWN_REVISION = - Iterables.getLast(KNOWN_NDK_MAJOR_REVISIONS.keySet()); + public static final Map.Entry<String, NdkMajorRevision> LATEST_KNOWN_REVISION = + Iterables.getLast(KNOWN_NDK_MAJOR_REVISIONS.entrySet()); /** * Exception thrown when there is an error creating the crosstools file. @@ -50,23 +50,6 @@ public final class AndroidNdkCrosstools { } } - public static CrosstoolRelease create( - NdkRelease ndkRelease, - NdkPaths ndkPaths, - StlImpl stlImpl, - String hostPlatform) { - NdkMajorRevision ndkMajorRevision; - if (ndkRelease.isValid) { - // NDK minor revisions should be backwards compatible within a major revision, so it should be - // enough to check the major revision of the release. - ndkMajorRevision = KNOWN_NDK_MAJOR_REVISIONS.get(ndkRelease.majorRevision); - } else { - // If the NDK revision isn't valid, try using the latest one we know about. - ndkMajorRevision = KNOWN_NDK_MAJOR_REVISIONS.get(LATEST_KNOWN_REVISION); - } - return ndkMajorRevision.crosstoolRelease(ndkPaths, stlImpl, hostPlatform); - } - public static String getHostPlatform(NdkRelease ndkRelease) throws NdkCrosstoolsException { String hostOs; switch (OS.getCurrent()) { @@ -99,5 +82,5 @@ public final class AndroidNdkCrosstools { public static boolean isKnownNDKRevision(NdkRelease ndkRelease) { return KNOWN_NDK_MAJOR_REVISIONS.containsKey(ndkRelease.majorRevision); - } + } }
\ No newline at end of file diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/NdkRelease.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/NdkRelease.java index ec39fdc0da..31540b3d6d 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/NdkRelease.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/NdkRelease.java @@ -51,14 +51,19 @@ public class NdkRelease { } String revision = properties.getProperty(REVISION_PROPERTY); String[] revisionParsed = revision.split("\\."); - return new NdkRelease( - revision, // raw revision - true, // isValid - revisionParsed[0], // major revision - revisionParsed[1], // minor revision - null, // release candidate - true // is64 bit. 32-bit NDKs are provided for only windows. - ); + if (revisionParsed.length < 2) { + // Unable to parse Pkg.Revision. Return invalid NdkRelease. + return new NdkRelease(revision, false, null, null, null, false); + } else { + return new NdkRelease( + revision, // raw revision + true, // isValid + revisionParsed[0], // major revision + revisionParsed[1], // minor revision + null, // release candidate + true // is64 bit. 32-bit NDKs are provided for only windows. + ); + } } /** diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryTest.java index b27efd7fc3..6fc9fbf831 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryTest.java @@ -20,6 +20,7 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; import com.google.devtools.build.lib.analysis.FilesToRunProvider; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; +import com.google.devtools.build.lib.testutil.MoreAsserts; import com.google.devtools.build.lib.testutil.TestRuleClassProvider; import com.google.devtools.build.lib.vfs.FileSystemUtils; import org.junit.Before; @@ -74,4 +75,79 @@ public class AndroidNdkRepositoryTest extends BuildViewTestCase { .doesNotContain( "src external/androidndk/ndk/platforms/android-22/arch-x86/usr/lib/libandroid.so"); } + + @Test + public void testInvalidNdkReleaseTxt() throws Exception { + scratchPlatformsDirectories("arch-x86", 24); + FileSystemUtils.appendIsoLatin1( + scratch.resolve("WORKSPACE"), + "android_ndk_repository(", + " name = 'androidndk',", + " path = '/ndk',", + " api_level = 24,", + ")"); + + scratch.deleteFile("/ndk/source.properties"); + scratch.file("/ndk/RELEASE.TXT", "not a valid release string"); + + invalidatePackages(); + + assertThat(getConfiguredTarget("@androidndk//:files")).isNotNull(); + MoreAsserts.assertContainsEvent( + eventCollector, + "The revision of the Android NDK referenced by android_ndk_repository rule 'androidndk' " + + "could not be determined (the revision string found is 'not a valid release string')." + + " Defaulting to revision 13."); + } + + @Test + public void testInvalidNdkSourceProperties() throws Exception { + scratchPlatformsDirectories("arch-x86", 24); + FileSystemUtils.appendIsoLatin1( + scratch.resolve("WORKSPACE"), + "android_ndk_repository(", + " name = 'androidndk',", + " path = '/ndk',", + " api_level = 24,", + ")"); + + scratch.overwriteFile( + "/ndk/source.properties", + "Pkg.Desc = Android NDK", + "Pkg.Revision = invalid package revision"); + + invalidatePackages(); + + assertThat(getConfiguredTarget("@androidndk//:files")).isNotNull(); + MoreAsserts.assertContainsEvent( + eventCollector, + "The revision of the Android NDK referenced by android_ndk_repository rule 'androidndk' " + + "could not be determined (the revision string found is 'invalid package revision'). " + + "Defaulting to revision 13."); + } + + @Test + public void testUnsupportedNdkVersion() throws Exception { + scratchPlatformsDirectories("arch-x86", 24); + FileSystemUtils.appendIsoLatin1( + scratch.resolve("WORKSPACE"), + "android_ndk_repository(", + " name = 'androidndk',", + " path = '/ndk',", + " api_level = 24,", + ")"); + + scratch.overwriteFile( + "/ndk/source.properties", + "Pkg.Desc = Android NDK", + "Pkg.Revision = 14.0.3675639-beta2"); + invalidatePackages(); + + assertThat(getConfiguredTarget("@androidndk//:files")).isNotNull(); + MoreAsserts.assertContainsEvent( + eventCollector, + "The major revision of the Android NDK referenced by android_ndk_repository rule " + + "'androidndk' is 14. The major revisions supported by Bazel are [10, 11, 12, 13]. " + + "Defaulting to revision 13."); + } } diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/AndroidNdkCrosstoolsTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/AndroidNdkCrosstoolsTest.java index 09c369904d..c2d85ff144 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/AndroidNdkCrosstoolsTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/AndroidNdkCrosstoolsTest.java @@ -73,6 +73,10 @@ public class AndroidNdkCrosstoolsTest { this.ndkDirectoriesFilename = ndkDirectoriesFilename; } + NdkMajorRevision getNdkMajorRevision() { + return AndroidNdkCrosstools.KNOWN_NDK_MAJOR_REVISIONS.get(ndkRelease.majorRevision); + } + ImmutableSet<String> getNdkFiles() throws IOException { String ndkFilesFileContent = ResourceFileLoader.loadResource(AndroidNdkCrosstoolsTest.class, ndkFilesFilename); @@ -134,7 +138,7 @@ public class AndroidNdkCrosstoolsTest { for (StlImpl ndkStlImpl : StlImpls.get(ndkPaths)) { // Protos are immutable, so this can be shared between tests. CrosstoolRelease crosstool = - AndroidNdkCrosstools.create(params.ndkRelease, ndkPaths, ndkStlImpl, HOST_PLATFORM); + params.getNdkMajorRevision().crosstoolRelease(ndkPaths, ndkStlImpl, HOST_PLATFORM); crosstools.add(crosstool); stlFilegroupsBuilder.putAll(ndkStlImpl.getFilegroupNamesAndFilegroupFileGlobPatterns()); } @@ -144,18 +148,18 @@ public class AndroidNdkCrosstoolsTest { ndkFiles = params.getNdkFiles(); ndkDirectories = params.getNdkDirectories(); } - + @Test public void testPathsExist() throws Exception { for (CrosstoolRelease crosstool : crosstoolReleases) { for (CToolchain toolchain : crosstool.getToolchainList()) { - + // Test that all tool paths exist. for (ToolPath toolpath : toolchain.getToolPathList()) { assertThat(ndkFiles).contains(toolpath.getPath()); } - + // Test that all cxx_builtin_include_directory paths exist. for (String includeDirectory : toolchain.getCxxBuiltinIncludeDirectoryList()) { // Special case for builtin_sysroot. @@ -164,13 +168,13 @@ public class AndroidNdkCrosstoolsTest { assertThat(ndkDirectories).contains(path); } } - + // Test that the builtin_sysroot path exists. { String builtinSysroot = NdkPaths.stripRepositoryPrefix(toolchain.getBuiltinSysroot()); assertThat(ndkDirectories).contains(builtinSysroot); } - + // Test that all include directories added through unfiltered_cxx_flag exist. for (String flag : toolchain.getUnfilteredCxxFlagList()) { if (!flag.equals("-isystem")) { |