do not check file types too early.
a check was added to ensure that file types are not mixed in
built artifacts but the check was running too early, potentially
checking files that are not closed or even exists which can create
false positive.
Bug: 172550678
Test: Existing
Change-Id: I1c2cd8566751e5cdb318bc5ee1472d4c1ad8f3e2
diff --git a/build-system/gradle-core/src/main/java/com/android/build/api/variant/impl/BuiltArtifactsImpl.kt b/build-system/gradle-core/src/main/java/com/android/build/api/variant/impl/BuiltArtifactsImpl.kt
index 8ea27b4..64f0060 100644
--- a/build-system/gradle-core/src/main/java/com/android/build/api/variant/impl/BuiltArtifactsImpl.kt
+++ b/build-system/gradle-core/src/main/java/com/android/build/api/variant/impl/BuiltArtifactsImpl.kt
@@ -35,19 +35,21 @@
override val applicationId: String,
override val variantName: String,
override val elements: Collection<BuiltArtifactImpl>,
- val elementType: String? = initFileType(elements))
+ private val elementType: String? = null)
: CommonBuiltArtifacts, BuiltArtifacts, Serializable {
+ fun elementType():String? =
+ elementType ?: initFileType(elements)
+
companion object {
const val METADATA_FILE_NAME = "output-metadata.json"
private fun initFileType(elements: Collection<BuiltArtifactImpl>): String? {
val (files, directories) = elements
.asSequence()
- .map { FileSystems.getDefault().getPath(it.outputFile) }
- // ensure that the file exists and it can be determined as a File or Directory
- .filter { it.toFile().exists() && (Files.isRegularFile(it) || Files.isDirectory(it)) }
- .partition { Files.isRegularFile(it) }
+ .map { File(it.outputFile) }
+ .filter { it.exists() }
+ .partition { it.isFile }
if (files.isNotEmpty() && directories.isNotEmpty()) {
throw IllegalArgumentException("""
You cannot store both files and directories as a single artifact.
@@ -62,10 +64,10 @@
}
}
- private fun display(files: Collection<Path>, singular: String, plural: String): String {
+ private fun display(files: Collection<File>, singular: String, plural: String): String {
return if (files.size > 1)
- "${files.joinToString(",") { it.fileName.toString() }} are $plural"
- else "${files.first().fileName.toString()} is a $singular"
+ "${files.joinToString(",") { it.name }} are $plural"
+ else "${files.first().name} is a $singular"
}
}
@@ -115,7 +117,7 @@
variantOutputConfiguration = builtArtifact.variantOutputConfiguration
)
}.toList(),
- elementType)
+ elementType())
)
}
}
diff --git a/build-system/gradle-core/src/main/java/com/android/build/gradle/internal/test/BuiltArtifactsSplitOutputMatcher.kt b/build-system/gradle-core/src/main/java/com/android/build/gradle/internal/test/BuiltArtifactsSplitOutputMatcher.kt
index 843a372..94c0b38 100644
--- a/build-system/gradle-core/src/main/java/com/android/build/gradle/internal/test/BuiltArtifactsSplitOutputMatcher.kt
+++ b/build-system/gradle-core/src/main/java/com/android/build/gradle/internal/test/BuiltArtifactsSplitOutputMatcher.kt
@@ -64,7 +64,7 @@
outputFile = sourceBuiltArtifact.outputFile
)
},
- elementType = builtArtifacts.elementType
+ elementType = builtArtifacts.elementType()
)
// now look for a matching output file
return GenericBuiltArtifactsSplitOutputMatcher.computeBestOutput(
diff --git a/build-system/gradle-core/src/test/java/com/android/build/api/variant/impl/BuiltArtifactsImplTest.kt b/build-system/gradle-core/src/test/java/com/android/build/api/variant/impl/BuiltArtifactsImplTest.kt
index fa24eed..2d41434 100644
--- a/build-system/gradle-core/src/test/java/com/android/build/api/variant/impl/BuiltArtifactsImplTest.kt
+++ b/build-system/gradle-core/src/test/java/com/android/build/api/variant/impl/BuiltArtifactsImplTest.kt
@@ -401,7 +401,7 @@
createBuiltArtifacts(
BuiltArtifactImpl.make(folder.absolutePath),
BuiltArtifactImpl.make(file.absolutePath)
- )
+ ).saveToFile(tmpFolder.newFile())
} catch(e: IllegalArgumentException) {
Truth.assertThat(e.message).contains("${file.name} is a file")
Truth.assertThat(e.message).contains("${folder.name} is a directory")
@@ -420,7 +420,7 @@
BuiltArtifactImpl.make(folder.absolutePath),
BuiltArtifactImpl.make(fileOne.absolutePath),
BuiltArtifactImpl.make(fileTwo.absolutePath)
- )
+ ).saveToFile(tmpFolder.newFile())
} catch(e: IllegalArgumentException) {
Truth.assertThat(e.message).contains(
"${fileOne.name},${fileTwo.name} are files")
@@ -440,7 +440,7 @@
BuiltArtifactImpl.make(folderOne.absolutePath),
BuiltArtifactImpl.make(folderTwo.absolutePath),
BuiltArtifactImpl.make(file.absolutePath)
- )
+ ).saveToFile(tmpFolder.newFile())
} catch(e: IllegalArgumentException) {
Truth.assertThat(e.message).contains(
"${folderOne.name},${folderTwo.name} are directories")
