Skip to content

[SPARK-9015] [BUILD] Clean project import in scala ide #7375

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from
Closed

[SPARK-9015] [BUILD] Clean project import in scala ide #7375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
80fbdc5
remove groovy and gmavenplus plugin
jendap May 11, 2015
999a068
remove some maven warnings in scala ide
jendap May 14, 2015
c8a54db
remove java compiler warnings from java tests
jendap May 14, 2015
f47d856
remove spark-1.4-staging repository
jendap Jul 13, 2015
312007e
scala-maven-plugin itself add scala sources by default
jendap Jul 13, 2015
5a83e07
Revert "remove java compiler warnings from java tests"
jendap Jul 13, 2015
c4b4c0f
fix whitespaces
jendap Jul 15, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 53 additions & 77 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@
<aws.kinesis.client.version>1.2.1</aws.kinesis.client.version>
<commons.httpclient.version>4.3.2</commons.httpclient.version>
<commons.math3.version>3.4.1</commons.math3.version>
<test_classpath_file>${project.build.directory}/spark-test-classpath.txt</test_classpath_file>
<scala.version>2.10.4</scala.version>
<scala.binary.version>2.10</scala.binary.version>
<jline.version>${scala.version}</jline.version>
Expand Down Expand Up @@ -283,18 +282,6 @@
<enabled>false</enabled>
</snapshots>
</repository>
<!-- TODO: This can be deleted after Spark 1.4 is posted -->
<repository>
<id>spark-1.4-staging</id>
<name>Spark 1.4 RC4 Staging Repository</name>
<url>https://repository.apache.org/content/repositories/orgapachespark-1112</url>
<releases>
<enabled>true</enabled>
</releases>
<snapshots>
<enabled>false</enabled>
</snapshots>
</repository>
</repositories>
<pluginRepositories>
<pluginRepository>
Expand All @@ -318,17 +305,6 @@
<artifactId>unused</artifactId>
<version>1.0.0</version>
</dependency>
<!--
This depndency has been added to provided scope as it is needed for executing build
specific groovy scripts using gmaven+ and not required for downstream project building
with spark.
-->
<dependency>
<groupId>org.codehaus.groovy</groupId>
<artifactId>groovy-all</artifactId>
<version>2.3.7</version>
<scope>provided</scope>
</dependency>
<!--
This is needed by the scalatest plugin, and so is declared here to be available in
all child modules, just as scalatest is run in all children
Expand Down Expand Up @@ -1406,6 +1382,58 @@
<artifactId>maven-deploy-plugin</artifactId>
<version>2.8.2</version>
</plugin>
<!-- This plugin's configuration is used to store Eclipse m2e settings only. -->
<!-- It has no influence on the Maven build itself. -->
<plugin>
<groupId>org.eclipse.m2e</groupId>
<artifactId>lifecycle-mapping</artifactId>
<version>1.0.0</version>
<configuration>
<lifecycleMappingMetadata>
<pluginExecutions>
<pluginExecution>
<pluginExecutionFilter>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-dependency-plugin</artifactId>
<versionRange>[2.8,)</versionRange>
<goals>
<goal>build-classpath</goal>
</goals>
</pluginExecutionFilter>
<action>
<ignore></ignore>
</action>
</pluginExecution>
<pluginExecution>
<pluginExecutionFilter>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-jar-plugin</artifactId>
<versionRange>[2.6,)</versionRange>
<goals>
<goal>test-jar</goal>
</goals>
</pluginExecutionFilter>
<action>
<ignore></ignore>
</action>
</pluginExecution>
<pluginExecution>
<pluginExecutionFilter>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-antrun-plugin</artifactId>
<versionRange>[1.8,)</versionRange>
<goals>
<goal>run</goal>
</goals>
</pluginExecutionFilter>
<action>
<ignore></ignore>
</action>
</pluginExecution>
Copy link
Member

@srowen srowen Jul 13, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be unneeded, but how do you know? even I dont' know what this plugin is about.

Copy link
Contributor Author

@jendap jendap Jul 13, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The result is needed. The groovy plugin not. The trick is 6 lines above in this commit - maven-dependency-plugin can save to file. The "outputProperty" is changed to "outputFile". It does exactly the same thing as the gradle script.

I have double checked it. It does the same thing. The same file is there with the same content.

The advantage of this is therefore smaller maven file and faster build.

Copy link
Member

@srowen srowen Jul 13, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ScrapCodes you added this in daaca14 -- looks OK to you?

Copy link
Member

@ScrapCodes ScrapCodes Jul 14, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not aware of this back then, did it even exist in the 2.9 version of maven dependency plugin ? Anyway the proof that this change is legit is that - it passes the maven test build especially the distributed ones in core(and there are lots of them).

</pluginExecutions>
</lifecycleMappingMetadata>
</configuration>
</plugin>
</plugins>
</pluginManagement>

Expand All @@ -1423,34 +1451,12 @@
</goals>
<configuration>
<includeScope>test</includeScope>
<outputFile>${test_classpath_file}</outputFile>
<outputProperty>test_classpath</outputProperty>
</configuration>
</execution>
</executions>
</plugin>

<!-- This plugin reads a file into maven property. And it lets us write groovy !! -->
<plugin>
<groupId>org.codehaus.gmavenplus</groupId>
<artifactId>gmavenplus-plugin</artifactId>
<version>1.5</version>
<executions>
<execution>
<phase>process-test-classes</phase>
<goals>
<goal>execute</goal>
</goals>
<configuration>
<scripts>
<script><![CDATA[
def file = new File(project.properties.test_classpath_file)
project.properties.test_classpath = file.getText().split().join(":")
]]></script>
</scripts>
</configuration>
</execution>
</executions>
</plugin>
<!--
The shade plug-in is used here to create effective pom's (see SPARK-3812), and also
remove references from the shaded libraries from artifacts published by Spark.
Expand Down Expand Up @@ -1517,36 +1523,6 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-enforcer-plugin</artifactId>
</plugin>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>build-helper-maven-plugin</artifactId>
<executions>
<execution>
<id>add-scala-sources</id>
<phase>generate-sources</phase>
<goals>
<goal>add-source</goal>
</goals>
<configuration>
<sources>
<source>src/main/scala</source>
</sources>
</configuration>
</execution>
<execution>
<id>add-scala-test-sources</id>
<phase>generate-test-sources</phase>
<goals>
<goal>add-test-source</goal>
</goals>
<configuration>
<sources>
<source>src/test/scala</source>
</sources>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
<groupId>net.alchim31.maven</groupId>
<artifactId>scala-maven-plugin</artifactId>
Expand Down
2 changes: 0 additions & 2 deletions repl/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@
</goals>
<configuration>
<sources>
<source>src/main/scala</source>
<source>${extra.source.dir}</source>
</sources>
</configuration>
Expand All @@ -146,7 +145,6 @@
</goals>
<configuration>
<sources>
<source>src/test/scala</source>
<source>${extra.testsource.dir}</source>
</sources>
</configuration>
Expand Down
1 change: 0 additions & 1 deletion sql/core/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@
</goals>
<configuration>
<sources>
<source>src/test/scala</source>
<source>src/test/gen-java</source>
</sources>
</configuration>
Expand Down
1 change: 0 additions & 1 deletion sql/hive/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@
</goals>
<configuration>
<sources>
<source>src/test/scala</source>
<source>compatibility/src/test/scala</source>
Copy link
Member

@srowen srowen Jul 13, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK as long as we're sure this adds to rather than overrides <sources> from the parent.

Copy link
Contributor

@JoshRosen JoshRosen Jul 13, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I've been bitten by this before; we may have to pass an option to ensure overriding.

Copy link
Contributor

@JoshRosen JoshRosen Jul 13, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Err, to ensure addition.

Copy link
Contributor Author

@jendap jendap Jul 13, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not needed anymore. It did hurt you in the past because the add-sources was done by build-helper-maven-plugin only. Overriding the configuration here has overriden the setting from parent. That is why one had to repeat it all the time or the dir set in parent was lost.

Now however the addition happens because there are two plugins "adding sources".

BTW: We can keep it there. It does not hurt. It is just a cleanup.

</sources>
</configuration>
Expand Down
4 changes: 0 additions & 4 deletions tools/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,6 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-source-plugin</artifactId>
</plugin>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>build-helper-maven-plugin</artifactId>
</plugin>
</plugins>
Copy link
Member

@srowen srowen Jul 13, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure this isn't needed? this module has src/main/scala code

</build>
</project>