Skip to content

Conversation

@pwendell
Copy link
Contributor

DON'T MERGE ME - TESTING ON JENKINS

@pwendell
Copy link
Contributor Author

Jenkins, test this please.

@SparkQA
Copy link

SparkQA commented Aug 31, 2015

Test build #41816 has finished for PR 8531 at commit 9cf442d.

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

@pwendell
Copy link
Contributor Author

Jenkins, retest this please.

@pwendell
Copy link
Contributor Author

Jenkins, test this please.

@SparkQA
Copy link

SparkQA commented Aug 31, 2015

Test build #41817 has finished for PR 8531 at commit 5b2910b.

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

@SparkQA
Copy link

SparkQA commented Aug 31, 2015

Test build #41818 has finished for PR 8531 at commit 5b2910b.

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

@JoshRosen
Copy link
Contributor

Given that we just had a build break in only one of the Hadoop branches due to a dep. change, I'm thinking that this script should generate four dependencies files, one for each Hadoop profile that we test on Jenkins.

@SparkQA
Copy link

SparkQA commented Nov 3, 2015

Test build #44874 has started for PR 8531 at commit f4243b9.

@SparkQA
Copy link

SparkQA commented Nov 3, 2015

Test build #44876 has finished for PR 8531 at commit 8b973bf.

  • This patch fails executing the dev/run-tests script.
  • This patch merges cleanly.
  • This patch adds no public classes.

@pwendell
Copy link
Contributor Author

pwendell commented Nov 3, 2015

Jenkins, test this please.

@pwendell
Copy link
Contributor Author

pwendell commented Nov 3, 2015

Jenkins, test this please.

@SparkQA
Copy link

SparkQA commented Nov 3, 2015

Test build #44886 has finished for PR 8531 at commit 2f4d3e5.

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

@pwendell
Copy link
Contributor Author

pwendell commented Nov 3, 2015

Jenkins, retest this please.

@SparkQA
Copy link

SparkQA commented Nov 3, 2015

Test build #44889 has finished for PR 8531 at commit 2f4d3e5.

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

@SparkQA
Copy link

SparkQA commented Nov 3, 2015

Test build #44895 has finished for PR 8531 at commit ca686a3.

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

Copy link
Member

Choose a reason for hiding this comment

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

We don't need runtime as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, I think you're right. According to the maven-dependency-plugin docs (https://maven.apache.org/plugins/maven-dependency-plugin/list-mojo.html#includeScope):

Scope to include. An Empty string indicates all scopes (default). The scopes being interpreted are the scopes as Maven sees them, not as specified in the pom. In summary:

  • runtime scope gives runtime and compile dependencies,
  • compile scope gives compile, provided, and system dependencies,
  • test (default) scope gives all dependencies,
  • provided scope just gives provided dependencies,
  • system scope just gives system dependencies.

Based on this language, it seems like we want to be using runtime here so that we include both compile and runtime scope dependencies but do not exclude provided ones.

@SparkQA
Copy link

SparkQA commented Nov 3, 2015

Test build #44907 has finished for PR 8531 at commit 37230f0.

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

@pwendell
Copy link
Contributor Author

pwendell commented Nov 3, 2015

Jenkins, test this please.

@SparkQA
Copy link

SparkQA commented Nov 3, 2015

Test build #44950 has finished for PR 8531 at commit 1d91634.

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

@JoshRosen
Copy link
Contributor

Are you sure that we should sort the classpath? If two JARs provide the same classes then the ordering might matter and the relative classpath ordering might change as a consequence of adding a new dependency. Could also leave that up to followup, though, since this patch is massively useful as-is.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm thinking of taking over this PR and am considering dropping this logic since it adds complexity and might not be a huge performance issue in practice. Let me know if you disagree.

@asfgit asfgit closed this in 27a42c7 Dec 30, 2015
JoshRosen added a commit to JoshRosen/spark that referenced this pull request Jan 10, 2016
…r new pull requests

This patch adds a new build check which enumerates Spark's resolved runtime classpath and saves it to a file, then diffs against that file to detect whether pull requests have introduced dependency changes. The aim of this check is to make it simpler to reason about whether pull request which modify the build have introduced new dependencies or changed transitive dependencies in a way that affects the final classpath.

This supplants the checks added in SPARK-4123 / apache#5093, which are currently disabled due to bugs.

This patch is based on pwendell's work in apache#8531.

Closes apache#8531.

Author: Josh Rosen <joshrosen@databricks.com>
Author: Patrick Wendell <patrick@databricks.com>

Closes apache#10461 from JoshRosen/SPARK-10359.
JoshRosen added a commit to JoshRosen/spark that referenced this pull request Jan 10, 2016
…r new pull requests

This patch adds a new build check which enumerates Spark's resolved runtime classpath and saves it to a file, then diffs against that file to detect whether pull requests have introduced dependency changes. The aim of this check is to make it simpler to reason about whether pull request which modify the build have introduced new dependencies or changed transitive dependencies in a way that affects the final classpath.

This supplants the checks added in SPARK-4123 / apache#5093, which are currently disabled due to bugs.

This patch is based on pwendell's work in apache#8531.

Closes apache#8531.

Author: Josh Rosen <joshrosen@databricks.com>
Author: Patrick Wendell <patrick@databricks.com>

Closes apache#10461 from JoshRosen/SPARK-10359.
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.

4 participants