-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-3437][BUILD] Support crossbuilding in maven. #2318
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,7 +26,7 @@ | |
| </parent> | ||
|
|
||
| <groupId>org.apache.spark</groupId> | ||
| <artifactId>spark-bagel_2.10</artifactId> | ||
| <artifactId>spark-bagel</artifactId> | ||
| <properties> | ||
| <sbt.project.name>bagel</sbt.project.name> | ||
| </properties> | ||
|
|
@@ -37,7 +37,7 @@ | |
| <dependencies> | ||
| <dependency> | ||
| <groupId>org.apache.spark</groupId> | ||
| <artifactId>spark-core_${scala.binary.version}</artifactId> | ||
| <artifactId>spark-core</artifactId> | ||
| <version>${project.version}</version> | ||
| </dependency> | ||
| <dependency> | ||
|
|
@@ -59,6 +59,53 @@ | |
| <outputDirectory>target/scala-${scala.binary.version}/classes</outputDirectory> | ||
| <testOutputDirectory>target/scala-${scala.binary.version}/test-classes</testOutputDirectory> | ||
| <plugins> | ||
| <plugin> | ||
| <groupId>org.apache.maven.plugins</groupId> | ||
| <artifactId>maven-install-plugin</artifactId> | ||
| <version>2.5.2</version> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should specify this version once in parent pom in |
||
| <executions> | ||
| <execution> | ||
| <id>1</id> | ||
| <goals> | ||
| <goal>install-file</goal> | ||
| </goals> | ||
| <phase>install</phase> | ||
| <configuration> | ||
| <file>${project.build.directory}/${project.build.finalName}.jar</file> | ||
| <artifactId>${project.artifactId}_${scala.binary.version}</artifactId> | ||
| </configuration> | ||
| </execution> | ||
| <execution> | ||
| <id>2</id> | ||
| <goals> | ||
| <goal>install-file</goal> | ||
| </goals> | ||
| <phase>install</phase> | ||
| <configuration> | ||
| <generatePom>false</generatePom> | ||
| <pomFile>pom.xml</pomFile> | ||
| <file>${project.build.directory}/${project.build.finalName}-sources.jar</file> | ||
| <artifactId>${project.artifactId}_${scala.binary.version}</artifactId> | ||
| <classifier>sources</classifier> | ||
| </configuration> | ||
| </execution> | ||
| <execution> | ||
| <id>3</id> | ||
| <goals> | ||
| <goal>install-file</goal> | ||
| </goals> | ||
| <phase>install</phase> | ||
| <configuration> | ||
| <generatePom>false</generatePom> | ||
| <pomFile>pom.xml</pomFile> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In fact it looks like all of this config can be common in the parent since it looks like it's written generically in terms of properties? |
||
| <file>${project.build.directory}/${project.build.finalName}-javadoc.jar</file> | ||
| <artifactId>${project.artifactId}_${scala.binary.version}</artifactId> | ||
| <classifier>javadoc</classifier> | ||
| </configuration> | ||
| </execution> | ||
| </executions> | ||
| </plugin> | ||
|
|
||
| <plugin> | ||
| <groupId>org.scalatest</groupId> | ||
| <artifactId>scalatest-maven-plugin</artifactId> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It goes without saying, but this introduces a sort of 'incompatibility' at the Maven level. Downstream projects have to change artifact and version when selecting between this and a future version with different artifact names. This doesn't automatically harmonize the Spark build with the version of Scala you're using, and Maven can't resolve dependency conflicts across differently named artifacts. These are kind of inevitable.
If an end goal is to publicly support Scala 2.10 and 2.11 simultaneously, then we still have to have several published artifacts at once right? this is what
classifiercould be used for. But I suppose it's worth asking if that's the intent? or is this going to may be happen all at once for Spark 2.x?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sean I think this approach actually modifies the published pom's to look different than the poms which are our own build. I.e. it appends the 2.11 or 2.10 suffix to the artifact in the published poms. Other than being horrendously verbose... this does seem to be a nicer approach IMO.
Since donstream projects only consume spark via the published poms, it seems okay to me. What is the specific case you're concerned about?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see now. That makes sense then, and leaves open the possibility of publishing parallel builds for Scala 2.10 / 2.11 under the current naming convention. Although maybe you could argue for using
classifierinstead, there's no point changing conventions if they're not already being changed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just checked the published pom had, just this inside it.
No dependency information, I am not sure if we can live without that.