Skip to content

Conversation

@jaceklaskowski
Copy link
Contributor

…o NoClassDefFoundError: org/spark-project/guava/collect/Maps

/cc @srowen @rxin

…o NoClassDefFoundError: org/spark-project/guava/collect/Maps
@srowen
Copy link
Member

srowen commented Jan 9, 2016

I'm not sure about that; I believe Guava is supposed to be provided here. Does it fix the issue though? @vanzin how is this one supposed to work re: shading now that we don't need Guava in the public API?

@jaceklaskowski
Copy link
Contributor Author

Yes, it does. I'm using the latest revision + the change. It's a serious issue since standalone Master cannot be started as of today.

I do not know how it's supposed to have been fixed, but that's exactly what helped to resolve the issue. Any help appreciated to make it better.

@srowen
Copy link
Member

srowen commented Jan 9, 2016

Yeah I think this is correct. The comment that was here previously suggested this was related to use of Optional in the public API, but I think it needs to stay in compile scope for the shading to work as desired. If there are no other thoughts today I'll merge

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't compile the default scope? i.e. couldn't you leave out the explicit compile scope here and just not specify a scope?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thought so, but the line has fixed standalone Master to start (after the line got removed in 659fd9d).

Copy link
Member

Choose a reason for hiding this comment

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

Isn't compile the default scope? i.e. couldn't you leave out the explicit compile scope here and just not specify a scope?

It's set to provided in the parent pom.xml:

spark/pom.xml

Line 365 in b23c452

<groupId>com.google.guava</groupId>

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, gotcha; (this is why I shouldn't review before morning coffee).

@JoshRosen
Copy link
Contributor

Jenkins, this is ok to test.

@JoshRosen
Copy link
Contributor

@jaceklaskowski, to help me understand the scope of this bug could you tell us how you built Spark in your failing example? Did you use the Maven or SBT builds? Which build profiles? I'm wondering because there it looks like there are some differences between how the two build tools handle the Guava dependency. Specifically, it looks like SBT always puts Guava in compile scope:

spark/pom.xml

Line 2302 in 3d77cff

<scope>compile</scope>

@jaceklaskowski
Copy link
Contributor Author

I'm using the following commands to do the build:

$ ./dev/change-scala-version.sh 2.11
$ ./build/mvn -Pyarn -Phadoop-2.6 -Dhadoop.version=2.7.1 -Dscala-2.11 -Phive -Phive-thriftserver -DskipTests clean install

(I've been using sbt but somehow it got "broken" lately, i.e. it doesn't build all modules - I'm going to report it later).

@SparkQA
Copy link

SparkQA commented Jan 9, 2016

Test build #49047 has finished for PR 10674 at commit 16c228d.

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

@srowen
Copy link
Member

srowen commented Jan 10, 2016

I'm going to merge this to fix the issue; @vanzin maybe you can swoop in and double-check what I did here to make sure the provided vs compile scope for Guava is still as it needs to be now that there is no Guava in the API.

@asfgit asfgit closed this in b78e028 Jan 10, 2016
@jaceklaskowski jaceklaskowski deleted the SPARK-12736 branch January 10, 2016 13:11
@vanzin
Copy link
Contributor

vanzin commented Jan 11, 2016

The fix is correct; that shouldn't have been removed in the first place (sorry I missed that). The change in that pom is so that the files actually get packaged somewhere. Otherwise the shade plugin just changes references in existing classes, but no jar would actually include the shaded classes.

BTW it would be nice to restore the original comment that explained that too.

@srowen
Copy link
Member

srowen commented Jan 11, 2016

Yeah the original comment was helpful, though it also kind of led me to believe the setting was connected to the class existing in the public API. That bit would have to be changed.

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.

6 participants