Skip to content

Conversation

@brennonyork
Copy link

We're seeing a bug sporadically in the new PR dependency comparison test whereby it notes that all dependencies are removed. This happens when the current PR is built, but the final, sorted, dependency file is left blank. I believe this is an error either in the way the git checkout calls have been or an error within the mvn build for that PR (again, likely related to the git checkout). As such I've set the checkouts to now force (with -f flag) which is more in line with what Jenkins currently does on the initial checkout.

Setting this as a WIP for now to trigger the build process myriad times to see if the issue still arises.

@brennonyork
Copy link
Author

/cc @pwendell @nchammas @shaneknapp

@brennonyork brennonyork changed the title [WIP][HOTFIX][SPARK-4123]: Fix bug in dependency removal issue [WIP][HOTFIX][SPARK-4123]: Fix bug in PR dependency (all deps. removed issue) Apr 9, 2015
@brennonyork
Copy link
Author

jenkins, retest this please

@SparkQA
Copy link

SparkQA commented Apr 10, 2015

Test build #29977 has finished for PR 5443 at commit 3f073d6.

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

@SparkQA
Copy link

SparkQA commented Apr 10, 2015

Test build #29978 has finished for PR 5443 at commit 3f073d6.

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

@pwendell
Copy link
Contributor

@brennonyork let me know when this is not WIP and I'll take a look. Seems like you are still testing a bit.

@SparkQA
Copy link

SparkQA commented Apr 10, 2015

Test build #30040 has finished for PR 5443 at commit f2186be.

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

@brennonyork
Copy link
Author

@shaneknapp can you help me understand how Jenkins is doing the checkouts? I'm seeing the PR builder outputting:

Building remotely on amp-jenkins-worker-06 (centos) in workspace /home/jenkins/workspace/SparkPullRequestBuilder
 > git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
 > git config remote.origin.url https://github.com/apache/spark.git # timeout=10
Fetching upstream changes from https://github.com/apache/spark.git
 > git --version # timeout=10
 > git fetch --tags --progress https://github.com/apache/spark.git +refs/pull/5443/*:refs/remotes/origin/pr/5443/* # timeout=15
 > git rev-parse origin/pr/5443/merge^{commit} # timeout=10
 > git branch -a --contains c5916336e6aff94dd3abfc9a0a41a2528c765fce # timeout=10
 > git rev-parse remotes/origin/pr/5443/merge^{commit} # timeout=10
Checking out Revision c5916336e6aff94dd3abfc9a0a41a2528c765fce (origin/pr/5443/merge)
 > git config core.sparsecheckout # timeout=10
 > git checkout -f c5916336e6aff94dd3abfc9a0a41a2528c765fce

although I'm a bit confused what checkout I should switch between if, say, I want to, from a PR, checkout the master branch, then switch back to the given PR branch, then possibly back to master, and finally back to the PR again.

I'm currently doing what I believe is correct here although there are times when the checkout from master back to the current PR fails, producing odd dependency reports. I've noticed that Jenkins is using the -f flag which I've added, but wanted to see if you had any thoughts into the matter.

Further, I've added echo statements to dump the ghprbActualCommit, the sha1, and the output of git rev-parse HEAD. Each are different commit hashes which makes me further think this is the cause for all the errors. Again, any advice?

EDIT: For reference here is an example of the output from Jenkins with the previous build with a few comments from me:

HEAD:  e994bf15bcb5d59357b24af572010195c5ec7ab4
GHPRB: f2186be090c9c73847e97cc9abf37fc62bbf7368
SHA1:  origin/pr/5443/merge
Running test: pr_merge_ability
Running test: pr_public_classes
Running test: pr_new_dependencies
# checkout of master (from git checkout master)
Previous HEAD position was e994bf1... Merge f2186be090c9c73847e97cc9abf37fc62bbf7368 into 18ca089bed41ce3e87deeb14206317863518c12c
Switched to branch 'master'
Note: checking out 'e994bf15bcb5d59357b24af572010195c5ec7ab4'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. Example:

  git checkout -b new_branch_name

# checkout from value of `git rev-parse HEAD` presumably back into the current PR head
HEAD is now at e994bf1... Merge f2186be090c9c73847e97cc9abf37fc62bbf7368 into 18ca089bed41ce3e87deeb14206317863518c12c

@pwendell
Copy link
Contributor

I think you may need to look at the source code to figure out what is going on:

https://github.com/jenkinsci/ghprb-plugin/blob/master/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java#L156

The current set of changes seem pretty straightforward, so maybe I can merge this now to see if the "-f" helps.

@asfgit asfgit closed this in 77eeb10 Apr 14, 2015
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.

3 participants