Skip to content

Commit

Permalink
Fix handling of packages with unexpected files.
Browse files Browse the repository at this point in the history
Some file browsers helpfully create extra hidden files in any
directory they view. It may have been better to just ignore anything
without a module.json, but #106
made module.json optional, so technically any directory is a valid
module now and we need a new schema revision to change that behavior.
We could alternatively add an explicit module list to prefab.json, but
that would also require a new schema revision.

For now, just ignore all files (fixes the .DS_Store case) and any
hidden directories (just in case).
  • Loading branch information
DanAlbert committed Sep 14, 2020
1 parent ce16536 commit 6b0eefb
Show file tree
Hide file tree
Showing 12 changed files with 62 additions and 1 deletion.
13 changes: 12 additions & 1 deletion api/src/main/kotlin/com/google/prefab/api/Package.kt
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,18 @@ class Package(val path: Path) {
* The list of modules in this package.
*/
val modules: List<Module> =
moduleDir.listFiles()?.map { Module(it.toPath(), this) }
// Some file browsers helpfully create extra hidden files in any
// directory they view. It may have been better to just ignore anything
// without a module.json, but https://github.com/google/prefab/pull/106
// made module.json optional, so technically any directory is a valid
// module now and we need a new schema revision to change that behavior.
// We could alternatively add an explicit module list to prefab.json,
// but that would also require a new schema revision.
//
// For now, just ignore all files (fixes the .DS_Store case) and any
// hidden directories (just in case).
moduleDir.listFiles()?.filter { it.isDirectory && !it.isHidden }
?.map { Module(it.toPath(), this) }
?: throw RuntimeException(
"Unable to retrieve file list for $moduleDir"
)
Expand Down
20 changes: 20 additions & 0 deletions cli/src/test/kotlin/com/google/prefab/cli/PackageTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,26 @@ class PackageTest {
)
}

@Test
fun `can load package with unexpected files`() {
val packagePath = Paths.get(
this.javaClass.getResource("packages/has_unexpected_files").toURI()
)
val pkg = Package(packagePath)
assertEquals(packagePath, pkg.path)
assertEquals("has_unexpected_files", pkg.name)
assertEquals(emptyList(), pkg.dependencies)

assertEquals(1, pkg.modules.size)

val bar = pkg.modules.single()

assertEquals("bar", bar.name)
assertEquals(packagePath.resolve("modules/bar"), bar.path)
assertEquals("libbar", bar.libraryNameForPlatform(android))
assertEquals(emptyList(), bar.linkLibsForPlatform(android))
}

@Test
fun `package with unsupported platforms does not load`() {
assertFailsWith(UnsupportedPlatformException::class) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"abi": "arm64-v8a",
"api": 21,
"ndk": 19,
"stl": "c++_shared"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"abi": "armeabi-v7a",
"api": 16,
"ndk": 19,
"stl": "c++_shared"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"abi": "x86",
"api": 16,
"ndk": 19,
"stl": "c++_shared"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"abi": "x86_64",
"api": 21,
"ndk": 19,
"stl": "c++_shared"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"schema_version": 1,
"name": "has_unexpected_files",
"version": "1",
"dependencies": []
}

0 comments on commit 6b0eefb

Please sign in to comment.