[SPARK-4123][Project Infra]: Show new dependencies added in pull requests #5093

brennonyork · 2015-03-19T21:22:03Z

Starting work on this, but need to find a way to ensure that, after doing a checkout from apache/master, we can successfully return to the current checkout. I believe that git rev-parse HEAD will get me what I want, but pushing this PR up to test what the Jenkins boxes are seeing.

SparkQA · 2015-03-19T21:23:13Z

Test build #28886 has finished for PR 5093 at commit ae83b90.

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

SparkQA · 2015-03-19T21:33:09Z

Test build #28887 has finished for PR 5093 at commit 2bca3c3.

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

nchammas · 2015-03-19T21:38:54Z

dev/tests/pr_new_dependencies.sh

Do we need to do any kind of cleaning between the two Maven operations?

Honestly I'm unsure. I believe we're going to need to run a mvn clean compile dependency:build-classpath for each to work properly, but @pwendell was running it without the compile step which I couldn't reproduce locally without an install as well. Going to test now and see what Jenkins tells me.

SparkQA · 2015-03-19T22:03:18Z

Test build #28888 has finished for PR 5093 at commit 94d3547.

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

SparkQA · 2015-03-19T22:13:20Z

Test build #28890 has started for PR 5093 at commit dae7ba8.

This patch merges cleanly.

SparkQA · 2015-03-19T22:43:31Z

Test build #28894 has started for PR 5093 at commit 63a35c9.

This patch merges cleanly.

SparkQA · 2015-03-19T22:58:37Z

Test build #28895 has started for PR 5093 at commit f8011d8.

This patch merges cleanly.

SparkQA · 2015-03-19T23:03:09Z

Test build #28896 has finished for PR 5093 at commit 126ce61.

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

pwendell · 2015-03-20T22:53:07Z

dev/tests/pr_new_dependencies.sh

It would actually be good to show additions and removals of dependencies (in some cases a library will get a new version, which ends up here as an addition and removal). I updated the JIRA title to reflect that.

Was thinking the same thing actually. I'll make sure to include that before this WIP is completed. Thanks!

SparkQA · 2015-03-23T17:38:11Z

Test build #29006 has finished for PR 5093 at commit 291a8fe.

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

SparkQA · 2015-03-23T17:48:14Z

Test build #29007 has started for PR 5093 at commit b98f78c.

This patch merges cleanly.

SparkQA · 2015-03-23T17:58:06Z

Test build #29008 has finished for PR 5093 at commit 6912584.

This patch passes all tests.

SparkQA · 2015-03-23T18:17:43Z

Test build #29014 has finished for PR 5093 at commit f2abc8c.

This patch passes all tests.

SparkQA · 2015-03-23T18:32:37Z

Test build #29017 has finished for PR 5093 at commit 56f74a8.

This patch passes all tests.

… deps

SparkQA · 2015-03-26T16:59:08Z

Test build #29241 has finished for PR 5093 at commit 27bb9b5.

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

SparkQA · 2015-03-26T18:18:19Z

Test build #29247 has finished for PR 5093 at commit e229802.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.
This patch adds the following new dependencies:
- <

SparkQA · 2015-03-26T18:53:39Z

Test build #29248 has finished for PR 5093 at commit 0074566.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.
This patch adds the following new dependencies:
- clojure-1.6.0.jar

SparkQA · 2015-03-26T19:28:48Z

Test build #29250 has finished for PR 5093 at commit 7aa1dee.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.
This patch adds the following new dependencies:
- clojure-1.6.0.jar

SparkQA · 2015-03-26T20:58:20Z

Test build #29251 has finished for PR 5093 at commit dadd941.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.
This patch does not change any dependencies.

brennonyork · 2015-03-26T22:59:19Z

@pwendell @srowen @nchammas

All complete. You can check out build 29250 up a few to get what the output would be like if a new dependency were added. One issue which I'd love to get some opinion on...

Right now the initial post message to Github (i.e. the "Test build started + patch merges cleanly") will take up to 20 minutes to post if any pom.xml files were changed because it will then run and build both the current PR plus the master branch. This is purely because the "This patch merges cleanly" output is from a pr_test and runs in the core test loop. The easiest option to reflect the original way things have been posted would be to move the pr_merge_ability test out of the main test loop and have it execute independently. The other option would be to merely post the "Test build started at ..." message and remove the "merges cleanly" portion to be posted with in the post-test message.

I'll admit I'm more in favor of the latter option as I think it keeps things clean as well as the fact that "merges cleanly" is slightly ambiguous given that Github reports on this as well. Thoughts? Whichever way we go I can get that final change up and then I'd say this is ready for review into master.

nchammas · 2015-03-26T23:11:03Z

I have advocated in the past for removing the redundant "merges cleanly" message, but IIRC some people found it useful and wanted to keep it.

One way or the other, yes, the mergability test needs to run only once. And more generally, we want the "tests started" message to post as quickly as possible so that contributors know the CI is working.

If that means not posting dependency changes until all tests are done, I personally think that's fair.

srowen · 2015-03-27T13:15:57Z

dev/tests/pr_new_dependencies.sh

Pretty trivial but the third arg could be described here. I assume the other two scripts simply ignore the third arg.

srowen · 2015-03-27T13:22:25Z

I personally favor a "tests have started" message quickly, yes, but I don't find it essential to print the merge status in that initial message, no. All of those results can be printed in the second "tests have finished" message IMHO. So I think I agree with you.

…nged mvn call to build/mvn

brennonyork · 2015-03-27T17:36:23Z

Thanks for the update guys. Per the consensus I moved the "tests have started" message to before the PR tests run. Also, @srowen, updated all items per your comments. Any additional thoughts all?

SparkQA · 2015-03-27T19:20:24Z

Test build #29314 has finished for PR 5093 at commit 42e243e.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.
This patch adds the following new dependencies:
- avro-1.7.7.jar
This patch removes the following dependencies:
- avro-1.7.6.jar

srowen · 2015-03-28T06:59:32Z

That's looking good to me. It seems to work -- here we actually see the small possibility of a false positive. A separate change updated Avro to 1.7.7 at about the same time that this test ran. I don't think it's a big deal since we can easily reason about it or retest if in doubt.

I'll wait a bit for comments but I like it.

JoshRosen · 2015-12-23T23:47:05Z

I've taken this over at #10461, where I found a clean way to speed up the checks to a point where they run near instantaneously without having to modify or delete source files.

…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 / #5093, which are currently disabled due to bugs. This patch is based on pwendell's work in #8531. Closes #8531. Author: Josh Rosen <joshrosen@databricks.com> Author: Patrick Wendell <patrick@databricks.com> Closes #10461 from JoshRosen/SPARK-10359.

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

Brennon York added 2 commits March 19, 2015 17:17

beginning work on pr test to add new dependencies

4110993

removed jenkins tests to grab some values from the jenkins box

ae83b90

added a git checkout 'git rev-parse HEAD' to the end of each pr test

2bca3c3

nchammas reviewed Mar 19, 2015
View reviewed changes

adding the new dependencies script into the test mix

94d3547

Capturing output directly from dependency builds

dae7ba8

updated new dependencies test

63a35c9

minor updates and style changes

f8011d8

removing new deps test to isolate why jenkins isn't posting messages

126ce61

pwendell reviewed Mar 20, 2015
View reviewed changes

updated location of maven binary

291a8fe

added the removed deps and echo output for jenkins testing

b98f78c

Brennon York added 2 commits March 23, 2015 10:54

removed the error to dev/null

c610d42

added this_mssg echo output

6912584

removed the git checkout

f2abc8c

updated the unop for ensuring a test is available

56f74a8

Brennon York added 2 commits March 23, 2015 11:56

Merge remote-tracking branch 'upstream/master' into SPARK-4123

17379a5

commenting out new deps script

97e5cfb

changed the assembly pom to test whether the pr test will pick up new…

27bb9b5

… deps

updated to print the new dependency found

e229802

fixed minor echo issue with quotes

0074566

set new dendencies into a <code> block, removed the bash debugging flag

7aa1dee

reverted assembly pom, put the regular test suite back in play

dadd941

brennonyork changed the title ~~[SPARK-4123][Project Infra][WIP]: Show new dependencies added in pull requests~~ [SPARK-4123][Project Infra]: Show new dependencies added in pull requests Mar 26, 2015

srowen reviewed Mar 27, 2015
View reviewed changes

moved starting test output to before pr tests, fixed indentation, cha…

42e243e

…nged mvn call to build/mvn

asfgit closed this in 55153f5 Mar 29, 2015

JoshRosen mentioned this pull request Dec 23, 2015

[SPARK-10359] Enumerate dependencies in a file and diff against it for new pull requests #10461

Closed

[SPARK-4123][Project Infra]: Show new dependencies added in pull requests #5093

[SPARK-4123][Project Infra]: Show new dependencies added in pull requests #5093

Uh oh!

Conversation

brennonyork commented Mar 19, 2015

Uh oh!

SparkQA commented Mar 19, 2015

Uh oh!

SparkQA commented Mar 19, 2015

Uh oh!

nchammas Mar 19, 2015

Choose a reason for hiding this comment

Uh oh!

brennonyork Mar 19, 2015

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Mar 19, 2015

Uh oh!

SparkQA commented Mar 19, 2015

Uh oh!

SparkQA commented Mar 19, 2015

Uh oh!

SparkQA commented Mar 19, 2015

Uh oh!

SparkQA commented Mar 19, 2015

Uh oh!

pwendell Mar 20, 2015

Choose a reason for hiding this comment

Uh oh!

brennonyork Mar 23, 2015

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Mar 23, 2015

Uh oh!

SparkQA commented Mar 23, 2015

Uh oh!

SparkQA commented Mar 23, 2015

Uh oh!

SparkQA commented Mar 23, 2015

Uh oh!

SparkQA commented Mar 23, 2015

Uh oh!

SparkQA commented Mar 26, 2015

Uh oh!

SparkQA commented Mar 26, 2015

Uh oh!

SparkQA commented Mar 26, 2015

Uh oh!

SparkQA commented Mar 26, 2015

Uh oh!

SparkQA commented Mar 26, 2015

Uh oh!

brennonyork commented Mar 26, 2015

Uh oh!

nchammas commented Mar 26, 2015

Uh oh!

srowen Mar 27, 2015

Choose a reason for hiding this comment

Uh oh!

srowen commented Mar 27, 2015

Uh oh!

brennonyork commented Mar 27, 2015

Uh oh!

SparkQA commented Mar 27, 2015

Uh oh!

srowen commented Mar 28, 2015

Uh oh!

JoshRosen commented Dec 23, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants