Skip to content

[SPARK-21592][BUILD] Skip maven-compiler-plugin main and test compilations in Maven build#18750

Closed
gslowikowski wants to merge 1 commit intoapache:masterfrom
gslowikowski:remove-redundant-compilation-from-maven
Closed

[SPARK-21592][BUILD] Skip maven-compiler-plugin main and test compilations in Maven build#18750
gslowikowski wants to merge 1 commit intoapache:masterfrom
gslowikowski:remove-redundant-compilation-from-maven

Conversation

@gslowikowski
Copy link
Contributor

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).

pom.xml Outdated
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I forgot about it.

pom.xml Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Why is this necessary/OK?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

@gslowikowski
Copy link
Contributor Author

Do you want me to update pull?

@srowen
Copy link
Member

srowen commented Jul 27, 2017

Yeah then I can run tests

@gslowikowski gslowikowski force-pushed the remove-redundant-compilation-from-maven branch from c8638bf to d9dfe3a Compare July 27, 2017 19:57
@gslowikowski
Copy link
Contributor Author

gslowikowski commented Jul 27, 2017

Updated.

BTW Travis build fails on the last phase - dev/lint-java script.

@srowen
Copy link
Member

srowen commented Jul 28, 2017

Likewise @vanzin any thoughts on this one? because it touches the compilation and build

@srowen
Copy link
Member

srowen commented Jul 29, 2017

This will need a JIRA @gslowikowski , by the way

@SparkQA
Copy link

SparkQA commented Jul 29, 2017

Test build #3861 has finished for PR 18750 at commit d9dfe3a.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@srowen
Copy link
Member

srowen commented Aug 1, 2017

@gslowikowski I'll merge this if you'll make a JIRA and link it here via the title

@gslowikowski
Copy link
Contributor Author

@srowen
Copy link
Member

srowen commented Aug 1, 2017

Looks good @gslowikowski , just put [SPARK-21592] at the start of the title to link them.

@gslowikowski gslowikowski changed the title Skip maven-compiler-plugin main and test compilations in Maven build [SPARK-21592][BUILD] Skip maven-compiler-plugin main and test compilations in Maven build Aug 1, 2017
…tions

scala-maven-plugin in incremental mode compiles Scala and Java classes. There is no need to execute maven-compiler-plugin goals to compile Java.
@gslowikowski gslowikowski force-pushed the remove-redundant-compilation-from-maven branch from d9dfe3a to 0b36e30 Compare August 1, 2017 11:47
@srowen
Copy link
Member

srowen commented Aug 1, 2017

Merged to master. I'll watch the builds to make sure it's really happy with this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants