From 3f6120b09cd8e4964508e71270e31297f3c670ed Mon Sep 17 00:00:00 2001
From: Chunyan Song <csong@twitter.com>
Date: Thu, 7 Mar 2013 08:44:47 -0800
Subject: [PATCH] Enforce explicit include to extract Thrift files from
 dependencies Compile thrift files from dependency jars ONLY when it's
 explicitly included in the <dependencyConfig> element

RB_ID=129964
---
 scrooge-maven-plugin/demo/pom.xml             | 31 +++++++------------
 .../com/twitter/AbstractMavenScroogeMojo.java |  6 ++--
 2 files changed, 13 insertions(+), 24 deletions(-)

diff --git a/scrooge-maven-plugin/demo/pom.xml b/scrooge-maven-plugin/demo/pom.xml
index 2ea703546..ea1abca64 100644
--- a/scrooge-maven-plugin/demo/pom.xml
+++ b/scrooge-maven-plugin/demo/pom.xml
@@ -10,27 +10,19 @@
     <git.dir>${project.basedir}/../../../.git</git.dir>
   </properties>
   <dependencies>
-    <!-- how to add thrift dependencies -->
-    <!-- if the artifact has "idl" classifer, you can specify it
-    <dependency>
-      <groupId>com.twitter</groupId>
-      <artifactId>foo</artifactId>
-      <version>0.1.0</version>
-      <scope>compile</scope>
-      <classifier>idl</classifier>
-    </dependency>
-    -->
+    <!-- Project dependencies -->
 
-    <!-- if the artifact does not have "idl" classifier, then you need to
-         - add it as a dependency here.
-         - add the artifactId as <dependencyConfig> in scrooge-maven-plugin, as seen later
+    <!-- Notes to users:
+    Suppose your project depends on some artifact from which you want to pull
+    out the thrift files to compile using Scrooge. Add the dependency as usual
+    and add the artifact to the whitelist in scrooge-maven-plugin configuration
+    Here we are using event-logger-thrift as an example
+    -->
     <dependency>
       <groupId>com.twitter</groupId>
-      <artifactId>bar</artifactId>
-      <version>1.0.1</version>
-      <scope>compile</scope>
+      <artifactId>event-logger-thrift</artifactId>
+      <version>0.0.3</version>
     </dependency>
-    -->
 
     <!-- common library dependencies -->
     <dependency>
@@ -165,11 +157,10 @@
             <thriftOpt>--ostrich</thriftOpt>
             <thriftOpt>-v</thriftOpt>
           </thriftOpts>
-          <!-- tell scrooge to extract thrifts from these artifacts as well
+          <!-- tell scrooge to extract thrifts from these artifacts -->
           <dependencyConfig>
-            <include>bar</include>
+            <include>event-logger-thrift</include>
           </dependencyConfig>
-          -->
         </configuration>
         <executions>
           <execution>
diff --git a/scrooge-maven-plugin/src/main/java/com/twitter/AbstractMavenScroogeMojo.java b/scrooge-maven-plugin/src/main/java/com/twitter/AbstractMavenScroogeMojo.java
index eded5cb56..ad2d0a228 100644
--- a/scrooge-maven-plugin/src/main/java/com/twitter/AbstractMavenScroogeMojo.java
+++ b/scrooge-maven-plugin/src/main/java/com/twitter/AbstractMavenScroogeMojo.java
@@ -257,10 +257,8 @@ private Set<File> findThriftFiles() throws IOException {
   private Set<File> findThriftDependencies(Set<String> whitelist) throws IOException {
     Set<File> thriftDependencies = new HashSet<File>();
     for(Artifact artifact : (Collection<Artifact>)project.getArtifacts()) {
-      String classifier = artifact.getClassifier();
       String artifactId = artifact.getArtifactId();
-      if (classifier != null && classifier.equals("idl")
-              || whitelist.contains(artifactId)) {
+      if (whitelist.contains(artifactId)) {
         thriftDependencies.add(artifact.getFile());
       }
     }
@@ -325,7 +323,7 @@ protected List<File> getRecursiveThriftFiles(MavenProject project, String output
    * Walk project references recursively, adding thrift files to the provided list.
    */
   List<File> getRecursiveThriftFiles(MavenProject project, String outputDirectory, List<File> files) throws IOException {
-    if (!dependencyConfig.contains(project.getArtifactId())) {
+    if (dependencyConfig.contains(project.getArtifactId())) {
       File dir = new File(new File(project.getFile().getParent(), "target"), outputDirectory);
       if (dir.exists()) {
         try {