Skip to content

Conversation

@ScrapCodes
Copy link
Member

This patch get rid of _2.10 in the artifact ids and adapts the install plugin to include it instead.
This will make it easy to cross build for scala 2.10 and 2.11.

There was no way(I know) to avoid copying the same code everywhere. (Maven oddities... )

@ScrapCodes
Copy link
Member Author

@pwendell If this is okay to do, then there is no need of a special maven plugin for this task.

@ScrapCodes ScrapCodes changed the title [SPARK-3437] Support crossbuilding in maven. [SPARK-3437][BUILD] Support crossbuilding in maven. Sep 8, 2014
@SparkQA
Copy link

SparkQA commented Sep 8, 2014

QA tests have started for PR 2318 at commit 6c8f9f6.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 8, 2014

QA tests have finished for PR 2318 at commit 6c8f9f6.

  • This patch fails unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

This patch get rid of _2.10 in the artifact ids and adapts the install plugin to include it instead.
This will make it easy to cross build for scala 2.10 and 2.11.
@SparkQA
Copy link

SparkQA commented Sep 8, 2014

QA tests have started for PR 2318 at commit ed315ef.

  • This patch merges cleanly.

Copy link
Member

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 classifier could 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?

Copy link
Contributor

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?

Copy link
Member

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 classifier instead, there's no point changing conventions if they're not already being changed.

Copy link
Member Author

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.

<?xml version="1.0" encoding="UTF-8"?>
<project xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd" xmlns="http://maven.apache.org/POM/4.0.0"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
  <modelVersion>4.0.0</modelVersion>
  <groupId>org.apache.spark</groupId>
  <artifactId>spark-core_2.10</artifactId>
  <version>1.2.0-SNAPSHOT</version>
  <description>POM was created from install:install-file</description>
</project>

No dependency information, I am not sure if we can live without that.

@SparkQA
Copy link

SparkQA commented Sep 8, 2014

QA tests have finished for PR 2318 at commit ed315ef.

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

@pwendell
Copy link
Contributor

pwendell commented Sep 8, 2014

I like this approach better than the previous one of modifying the maven build with bash - but I haven't tested it locally yet. I do wonder a bit about how this works in sbt (is this inherited for an sbt/sbt publish-local?). Also, it would be really good to see if this could be done in a way that doesn't require adding this big build definition in every project.

@vanzin
Copy link
Contributor

vanzin commented Sep 8, 2014

@ScrapCodes the amount of copy & paste that Maven sometimes seem to enforce on people always boggles my mind... have you tried alternative approaches?

I played a little bit with a couple of things and this approach seems to have made some progress: https://gist.github.com/vanzin/1ada891e573b89ff8c45

Namely, I saw the executions of the maven-install-plugin when building core/, and projects dependending on core were picking up the right dependency (with the scala version appended to the artifactId).

@srowen
Copy link
Member

srowen commented Sep 8, 2014

@vanzin I have a feeling I'm missing something, but does this config need to be repeated more than once to begin with, instead of being defined in the parent once? Is it not going to work in this situation given what the plugin is trying to do with the artifact ID?

@vanzin
Copy link
Contributor

vanzin commented Sep 8, 2014

@srowen the problem is that maven is stup... err, peculiar.

If you declare the plugin in the parent pom's "pluginManagement" section, it will try to execute the plugin when building the parent pom itself. Since the parent pom has packaging "pom", it will fail, since there are no jars. So you need a way to only add the plugin to the build in certain situations.

The logical thing would have the plugin only be activated for children, but there's no option I can find to do that. The next would be to only bind the plugin to a certain packaging type, but again, no dice.

So you have to resort to profiles. You could have a profile activated by a property, but then that doesn't work when the property is set in the child pom (instead of the command line). So you're left with checking for some file on disk, like my approach. I checked for "src/main/scala", but if that's too generic, we can use a tag file (and create that file in all projects where we want the plugin to be activated).

@srowen
Copy link
Member

srowen commented Sep 8, 2014

@vanzin I'm not suggesting putting it in <plugins> but just configuring it in <pluginManagement> in the parent. That's where it's configured now. It would not make it execute on the parent. You still need to enable it in each submodule in <plugins>, but not copy config in each submodule. At least that works fine in general and hope it works here.

@vanzin
Copy link
Contributor

vanzin commented Sep 8, 2014

I'm not suggesting putting it in but just configuring it in pluginManagement

That's what I meant. If you do that, it will be executed for the "spark_parent" pom and will fail. (BTW that's the first thing I tried.)

@srowen
Copy link
Member

srowen commented Sep 8, 2014

pluginManagement by itself never activates a plugin. But crumbs I suppose
the install plugin is inherited as an active plugin from Maven's own
default POM. Aha, well, configure it in the parent, set config skip=true
there, and set it to false in children? That has worked for me elsewhere.
On Sep 8, 2014 11:47 PM, "Marcelo Vanzin" notifications@github.com wrote:

I'm not suggesting putting it in but just configuring it in

That's what I meant. If you do that, it will be executed for the
"spark_parent" pom and will fail. (BTW that's the first thing I tried.)


Reply to this email directly or view it on GitHub
#2318 (comment).

@vanzin
Copy link
Contributor

vanzin commented Sep 8, 2014

I don't see a "skip" config option for the install-file goal, but if it exists / works, sure, that's an option.

@srowen
Copy link
Member

srowen commented Sep 9, 2014

@vanzin So yeah, it doesn't have a skip configuration, darn. How about this?

  • Declare plugin in <pluginManagement> in the parent with all <configuration>
  • Omit <goals> in this parent POM declaration
  • In children, repeat the <plugin> declaration in <build>, without <configuration>, but with an <execution> with the same <id> that adds the <goals>

The idea is that you can't avoid the plugin being active but can wait to connect the install-file goal to a phase until the children. It might be sufficient to just omit <phase> until the children.

@ScrapCodes
Copy link
Member Author

Hey All,
Please correct me if I am wrong, my conclusion is that this also will not help because pom published is not appropriate(Since it does not have dependency info, which is I guess needed by downstream projects). So IMO writing a plugin is the only option now. I have started writing one here https://github.com/ScrapCodes/scala-install-plugin. It is cool in the sense, it publishes effective pom that is: if artifact was build using a profile its effective pom of dependencies(will have correct properties values set in etc..) will be different than one published with a different profile. I am not expert here and totally confused, my only source of information is seeing what others have done.

@vanzin
Copy link
Contributor

vanzin commented Sep 10, 2014

If there's no way for the maven-install-plugin to fix the existing pom (instead of generating a minimal pom with no dependencies), then I guess I don't see any other solution. (If only you could use properties in the artifactId declaration...)

@ScrapCodes
Copy link
Member Author

#2357 Does this with the help of a custom maven plugin.

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.

5 participants