You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by gslowikowski <gi...@git.apache.org> on 2017/07/27 12:51:53 UTC

[GitHub] spark pull request #18750: Skip maven-compiler-plugin main and test compilat...

GitHub user gslowikowski opened a pull request:

    https://github.com/apache/spark/pull/18750

    Skip maven-compiler-plugin main and test compilations in Maven build

    `scala-maven-plugin` in `incremental` mode compiles `Scala` and `Java` classes. There is no need to execute `maven-compiler-plugin` goals to compile (in fact recompile) `Java`.
    
    This change reduces compilation time (over 10% on my machine).

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/gslowikowski/spark remove-redundant-compilation-from-maven

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/18750.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #18750
    
----
commit c8638bff14fd7bf4a9f87a948e82ec67aba222b4
Author: Grzegorz Slowikowski <gs...@gmail.com>
Date:   2017-07-27T12:42:15Z

    Skip maven-compiler-plugin main and test compilations.
    
    scala-maven-plugin in incremental mode compiles Scala and Java classes. There is no need to execute maven-compiler-plugin goals to compile Java.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18750: Skip maven-compiler-plugin main and test compilat...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18750#discussion_r129843018
  
    --- Diff: pom.xml ---
    @@ -2022,14 +2020,8 @@
               <artifactId>maven-compiler-plugin</artifactId>
               <version>3.6.1</version>
               <configuration>
    -            <source>${java.version}</source>
    -            <target>${java.version}</target>
    -            <encoding>UTF-8</encoding>
    -            <maxmem>1024m</maxmem>
    -            <fork>true</fork>
    -            <compilerArgs>
    -              <arg>-Xlint:all,-serial,-path</arg>
    -            </compilerArgs>
    +            <skipMain>true</skipMain> <!-- skip compile -->
    --- End diff --
    
    I think you may be right. However if so you can remove the configuration of `maven-compiler-plugin` in the two submodules that further refine the config.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18750: Skip maven-compiler-plugin main and test compilations in...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:

    https://github.com/apache/spark/pull/18750
  
    Looks good @gslowikowski , just put `[SPARK-21592]` at the start of the title to link them.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18750: Skip maven-compiler-plugin main and test compilations in...

Posted by gslowikowski <gi...@git.apache.org>.
Github user gslowikowski commented on the issue:

    https://github.com/apache/spark/pull/18750
  
    Created https://issues.apache.org/jira/browse/SPARK-21592


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18750: Skip maven-compiler-plugin main and test compilat...

Posted by gslowikowski <gi...@git.apache.org>.
Github user gslowikowski commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18750#discussion_r129848461
  
    --- Diff: pom.xml ---
    @@ -2022,14 +2020,8 @@
               <artifactId>maven-compiler-plugin</artifactId>
               <version>3.6.1</version>
               <configuration>
    -            <source>${java.version}</source>
    -            <target>${java.version}</target>
    -            <encoding>UTF-8</encoding>
    -            <maxmem>1024m</maxmem>
    -            <fork>true</fork>
    -            <compilerArgs>
    -              <arg>-Xlint:all,-serial,-path</arg>
    -            </compilerArgs>
    +            <skipMain>true</skipMain> <!-- skip compile -->
    --- End diff --
    
    Yes, I forgot about it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18750: Skip maven-compiler-plugin main and test compilations in...

Posted by gslowikowski <gi...@git.apache.org>.
Github user gslowikowski commented on the issue:

    https://github.com/apache/spark/pull/18750
  
    Updated.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18750: [SPARK-21592][BUILD] Skip maven-compiler-plugin main and...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:

    https://github.com/apache/spark/pull/18750
  
    Merged to master. I'll watch the builds to make sure it's really happy with this change.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18750: Skip maven-compiler-plugin main and test compilations in...

Posted by gslowikowski <gi...@git.apache.org>.
Github user gslowikowski commented on the issue:

    https://github.com/apache/spark/pull/18750
  
    Do you want me to update pull?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18750: Skip maven-compiler-plugin main and test compilations in...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:

    https://github.com/apache/spark/pull/18750
  
    @gslowikowski I'll merge this if you'll make a JIRA and link it here via the title


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18750: Skip maven-compiler-plugin main and test compilations in...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18750
  
    **[Test build #3861 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3861/testReport)** for PR 18750 at commit [`d9dfe3a`](https://github.com/apache/spark/commit/d9dfe3a147dd5530315572c523588d55c28120de).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18750: Skip maven-compiler-plugin main and test compilations in...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18750
  
    Can one of the admins verify this patch?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18750: Skip maven-compiler-plugin main and test compilat...

Posted by gslowikowski <gi...@git.apache.org>.
Github user gslowikowski commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18750#discussion_r129849368
  
    --- Diff: pom.xml ---
    @@ -1972,14 +1972,12 @@
                 </execution>
                 <execution>
                   <id>scala-compile-first</id>
    -              <phase>process-resources</phase>
                   <goals>
                     <goal>compile</goal>
                   </goals>
                 </execution>
                 <execution>
                   <id>scala-test-compile-first</id>
    -              <phase>process-test-resources</phase>
    --- End diff --
    
    Changing phase to one before `compile` is very old trick, used when `scala-maven-plugin` compiles only Scala classes and this compilation must be performed before Java classes compilation (by `maven-compiler-plugin`) because Java classes depend on Scala classes.
    In incremental compilation mode SBT compiler is used internally and it compiles everything. Because `maven-compiler-plugin` is disabled (compilations are skipped), there is no need to change default phase.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18750: Skip maven-compiler-plugin main and test compilations in...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18750
  
    **[Test build #3861 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3861/testReport)** for PR 18750 at commit [`d9dfe3a`](https://github.com/apache/spark/commit/d9dfe3a147dd5530315572c523588d55c28120de).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18750: Skip maven-compiler-plugin main and test compilations in...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:

    https://github.com/apache/spark/pull/18750
  
    This will need a JIRA @gslowikowski , by the way


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18750: [SPARK-21592][BUILD] Skip maven-compiler-plugin m...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/18750


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18750: Skip maven-compiler-plugin main and test compilations in...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:

    https://github.com/apache/spark/pull/18750
  
    Yeah then I can run tests


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18750: Skip maven-compiler-plugin main and test compilat...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18750#discussion_r129843046
  
    --- Diff: pom.xml ---
    @@ -1972,14 +1972,12 @@
                 </execution>
                 <execution>
                   <id>scala-compile-first</id>
    -              <phase>process-resources</phase>
                   <goals>
                     <goal>compile</goal>
                   </goals>
                 </execution>
                 <execution>
                   <id>scala-test-compile-first</id>
    -              <phase>process-test-resources</phase>
    --- End diff --
    
    Why is this necessary/OK?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18750: Skip maven-compiler-plugin main and test compilations in...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:

    https://github.com/apache/spark/pull/18750
  
    Likewise @vanzin any thoughts on this one? because it touches the compilation and build


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org