Recommend files() instead of fileTree() in protobuf dependency (#294)

We **incorrectly** recommended the use of:
```gradle
protobuf fileTree("ext/")
```
With this syntax Gradle will collect all files under the given directory and emit them as individual files, leaving the plugin unable to tell what the root directory is, thus dumping the files under the same directory.

If we just pass the directory to the configuration, the current implementation will correctly handle the copying of a whole directory, preserving its structure:
``` gradle
protobuf files("ext/")
```

This PR includes a test to reproduce the issue, and updates tests and examples to use the latter syntax. A warning will be printed if the former syntax is detected.

Resolves #248

Author: zhangkun83

.gitignore

@@ -4,4 +4,4 @@ protobuf-gradle-plugin.i*
 gradle.properties
 /.idea
 local.properties
-
+**/*~

README.md

@@ -453,7 +453,11 @@ proto files (if any). Example:
 
 ```gradle
 dependencies {
+  // protos can be from a local package,
   protobuf files('lib/protos.tar.gz')
+  // ... a local directory,
+  protobuf files('ext/')   // NEVER use fileTree(). See issue #248.
+  // ... or an artifact from a repository
   testProtobuf 'com.example:published-protos:1.0.0'
 }
 ```

examples/exampleKotlinDslProject/build.gradle.kts

@@ -7,7 +7,7 @@ import org.gradle.kotlin.dsl.provider.gradleKotlinDslOf
 plugins {
     java
     idea
-    id("com.google.protobuf") version "0.8.8-SNAPSHOT"
+    id("com.google.protobuf") version "0.8.8"
 }
 
 repositories {
@@ -34,10 +34,10 @@ dependencies {
     // Extra proto source files besides the ones residing under
     // "src/main".
     protobuf(files("lib/protos.tar.gz"))
-    protobuf(fileTree("ext/"))
+    protobuf(files("ext/"))
 
     // Adding dependency for configuration from custom sourceSet
-    "sampleProtobuf"(fileTree("ext/"))
+    "sampleProtobuf"(files("ext/"))
 
     testCompile("junit:junit:4.12")
     // Extra proto source files for test besides the ones residing under

examples/exampleProject/build.gradle

@@ -15,7 +15,7 @@ buildscript {
     maven { url "https://plugins.gradle.org/m2/" }
   }
   dependencies {
-    classpath 'com.google.protobuf:protobuf-gradle-plugin:0.8.4'
+    classpath 'com.google.protobuf:protobuf-gradle-plugin:0.8.8'
   }
 }
 
@@ -31,7 +31,7 @@ dependencies {
   // Extra proto source files besides the ones residing under
   // "src/main".
   protobuf files("lib/protos.tar.gz")
-  protobuf fileTree("ext/")
+  protobuf files("ext/")
 
   testCompile 'junit:junit:4.12'
   // Extra proto source files for test besides the ones residing under

src/main/groovy/com/google/protobuf/gradle/ProtobufExtract.groovy

@@ -56,6 +56,7 @@ class ProtobufExtract extends DefaultTask {
   @TaskAction
   void extract() {
     destDir.mkdir()
+    boolean warningLogged = false
     inputs.files.each { file ->
       logger.debug "Extracting protos from ${file} to ${destDir}"
       if (file.isDirectory()) {
@@ -67,6 +68,15 @@ class ProtobufExtract extends DefaultTask {
           into(destDir)
         }
       } else if (file.path.endsWith('.proto')) {
+        if (!warningLogged) {
+          warningLogged = true
+          project.logger.warn "proto file '${file.path}' directly specified in configuration. " +
+              "It's likely you specified files('path/to/foo.proto') or " +
+              "fileTree('path/to/directory') in protobuf or compile configuration. " +
+              "This makes you vulnerable to " +
+              "https://github.com/google/protobuf-gradle-plugin/issues/248. " +
+              "Please use files('path/to/directory') instead."
+        }
         project.copy {
           includeEmptyDirs(false)
           from(file.path)

testProject/src/main/java/Foo.java

@@ -20,9 +20,11 @@ public static List<MessageLite> getDefaultInstances() {
     list.add(More.MoreMsg.getDefaultInstance());
     list.add(More.Foo.getDefaultInstance());
     // from ext/test1.proto
-    list.add(Test1.Test1Msg.getDefaultInstance());
+    list.add(test1.Test1.Test1Msg.getDefaultInstance());
+    // from ext/ext1/test1.proto
+    list.add(ext1.Ext1Test1.Ext1Test1Msg.getDefaultInstance());
     // from ext/test2.proto
-    list.add(Test2.Test2Msg.getDefaultInstance());
+    list.add(test2.Test2.Test2Msg.getDefaultInstance());
     return list;
   }
 }

testProject/src/test/java/FooTest.java

@@ -4,7 +4,7 @@
 public class FooTest {
   @org.junit.Test
   public void testMainProtos() {
-    assertEquals(11, Foo.getDefaultInstances().size());
+    assertEquals(12, Foo.getDefaultInstances().size());
   }
 
   @org.junit.Test

testProjectBase/build_base.gradle

@@ -28,7 +28,7 @@ def protobufDep = 'com.google.protobuf:protobuf-java:3.0.0'
 
 dependencies {
   protobuf files("lib/protos.tar.gz")
-  protobuf fileTree("ext/")
+  protobuf files("ext/")
   testProtobuf files("lib/protos-test.tar.gz")
 
   compile protobufDep

testProjectBase/ext/ext1/test1.proto

@@ -0,0 +1,8 @@
+syntax = "proto3";
+
+option java_package = "ext1";
+option java_outer_classname = "Ext1Test1";
+
+message Ext1Test1Msg {
+    string bar = 1;
+}

testProjectBase/ext/test1.proto

@@ -7,6 +7,8 @@
  */
 syntax = "proto3";
 
+option java_package = "test1";
+
 message Test1Msg {
     string bar = 1;
 }

testProjectBase/ext/test2.proto

@@ -1,5 +1,7 @@
 syntax = "proto3";
 
+option java_package = "test2";
+
 message Test2Msg {
     string bar = 1;
 }

testProjectBase/src/main/proto/com/example/tutorial/sample.proto

@@ -4,10 +4,16 @@ option java_package = "com.example.tutorial";
 option java_outer_classname = "OuterSample";
 option java_multiple_files = true;
 
+import "test1.proto";
+import "ext1/test1.proto";
+import "test2.proto";
 
 message Msg {
     string foo = 1;
     SecondMsg blah = 2;
+    Test1Msg test1 = 3;
+    Test2Msg test2 = 4;
+    Ext1Test1Msg ext1test1 = 5;
 }
 
 message SecondMsg {

testProjectJavaAndKotlin/src/test/java/CallKotlinClass.java

@@ -5,6 +5,6 @@ public class CallKotlinClass {
   @org.junit.Test
   public void testMainProtosKotlin() {
     // call kotlin class from java
-    assertEquals(11, new KotlinFoo().getDefaultInstances().size());
+    assertEquals(12, new KotlinFoo().getDefaultInstances().size());
   }
 }

testProjectJavaAndKotlin/src/test/kotlin/CallJavaClass.kt

@@ -4,6 +4,6 @@ import org.junit.Assert.assertTrue
 class CallJavaClass {
   @org.junit.Test
   fun testCallJavaFoo() {
-    assertEquals(11, Foo.getDefaultInstances().size)
+    assertEquals(12, Foo.getDefaultInstances().size)
   }
 }

testProjectKotlin/src/main/kotlin/KotlinFoo.kt

@@ -18,9 +18,11 @@ class KotlinFoo {
     list.add(More.MoreMsg.getDefaultInstance());
     list.add(More.Foo.getDefaultInstance());
     // from ext/test1.proto
-    list.add(Test1.Test1Msg.getDefaultInstance());
+    list.add(test1.Test1.Test1Msg.getDefaultInstance());
+    // from ext/ext1/test1.proto
+    list.add(ext1.Ext1Test1.Ext1Test1Msg.getDefaultInstance());
     // from ext/test2.proto
-    list.add(Test2.Test2Msg.getDefaultInstance());
+    list.add(test2.Test2.Test2Msg.getDefaultInstance());
     return list
   }
 }

testProjectKotlin/src/test/kotlin/KotlinFooTest.kt

@@ -4,7 +4,7 @@ import org.junit.Assert.assertTrue
 class KotlinFooTest {
   @org.junit.Test
   fun testMainProtos() {
-    assertEquals(11, KotlinFoo().getDefaultInstances().size)
+    assertEquals(12, KotlinFoo().getDefaultInstances().size)
   }
 
   @org.junit.Test

testProjectKotlinDslBase/build.gradle.kts

@@ -60,7 +60,7 @@ val protobufDep = "com.google.protobuf:protobuf-java:3.0.0"
 
 dependencies {
     protobuf(files("lib/protos.tar.gz"))
-    protobuf(fileTree("ext/"))
+    protobuf(files("ext/"))
     testProtobuf(files("lib/protos-test.tar.gz"))
 
     compile(protobufDep)

testProjectLite/build.gradle

@@ -12,6 +12,7 @@ targetCompatibility = JavaVersion.VERSION_1_7
 
 dependencies {
     compile 'com.google.protobuf:protobuf-lite:3.0.0'
+    protobuf files("ext/")
     testCompile 'junit:junit:4.12'
 }