[JIRA] (JENKINS-43194) Lightweight checkout for PR merge jobs

2 views
Skip to first unread message

vivek.pandey@gmail.com (JIRA)

unread,
Feb 27, 2018, 12:21:04 AM2/27/18
to jenkinsc...@googlegroups.com
Vivek Pandey updated an issue
 
Jenkins / Improvement JENKINS-43194
Lightweight checkout for PR merge jobs
Change By: Vivek Pandey
Sprint: Blue Ocean 1.4 - beta 2, Pipeline - December March 2018
Add Comment Add Comment
 
This message was sent by Atlassian JIRA (v7.3.0#73011-sha1:3c73d0e)
Atlassian logo

vivek.pandey@gmail.com (JIRA)

unread,
Mar 12, 2018, 4:53:03 PM3/12/18
to jenkinsc...@googlegroups.com
Vivek Pandey updated an issue
Change By: Vivek Pandey
Sprint: Pipeline - Candidates, Blue Ocean 1.4 - beta 2 , Pipeline - March 2018

vivek.pandey@gmail.com (JIRA)

unread,
Mar 13, 2018, 12:09:08 PM3/13/18
to jenkinsc...@googlegroups.com
Vivek Pandey updated an issue
 
Jenkins / Improvement JENKINS-43194
Lightweight checkout for PR merge jobs
Change By: Vivek Pandey
Sprint: Pipeline - Candidates, Blue Ocean 1.4 - beta 2

bitwiseman@gmail.com (JIRA)

unread,
Feb 22, 2019, 4:46:02 PM2/22/19
to jenkinsc...@googlegroups.com
Liam Newman commented on Improvement JENKINS-43194
 
Re: Lightweight checkout for PR merge jobs

Jesse Glick 

Could you be more specific here?  It is unclear to me what you are describing.  

From what I can see, GitHub does the merge automatically and stores it in +refs/pull/<number>/merge.  If we ask for PR info via the GH API https://api.github.com/repos/bitwiseman/test-github-branch-plugin/pulls/1 ,  the merge_commit_sha matches the +refs/pull/<number>/merge branch.  The base and head remain unmodified.  We can then get the contents of the Jenkinsfile by specifying refs/pull/<number>/merge.

This seems to work when Jenkinsfile is modified as part of the commit and also when it is not modified:

refs/pull/1/merge - Jenkinsfile modified
refs/pull/2/merge - Readme modified

I'm new to this area so, I'm sure there's some aspect of this that I'm missing.

 

This message was sent by Atlassian Jira (v7.11.2#711002-sha1:fdc329d)

bitwiseman@gmail.com (JIRA)

unread,
Feb 22, 2019, 4:46:03 PM2/22/19
to jenkinsc...@googlegroups.com
Liam Newman assigned an issue to Liam Newman
 
Change By: Liam Newman
Assignee: rsandell Liam Newman

jglick@cloudbees.com (JIRA)

unread,
Feb 22, 2019, 5:47:03 PM2/22/19
to jenkinsc...@googlegroups.com
Jesse Glick commented on Improvement JENKINS-43194
 
Re: Lightweight checkout for PR merge jobs

Do not use merge_commit_sha from the GH API. It is deprecated and unreliable. Anyway that or +refs/pull/<number>/merge could not suit our purposes since we track a particular PR commit and a particular base branch commit as part of the SCM revision (what checkout scm uses), not just whatever happens to be recent-ish at the time you make the API call.

bitwiseman@gmail.com (JIRA)

unread,
Feb 22, 2019, 10:42:03 PM2/22/19
to jenkinsc...@googlegroups.com

Okay, as I figured there's things I don't know. That said...

merge_commit_sha is not deprecated: https://developer.github.com/changes/2013-04-25-deprecating-merge-commit-sha/ - they un-deprecated it about 2 years ago. 

How do you mean unreliable?  merge_commit_sha isn't updated until you at least curl to pull request page on the github site - which seems odd, but okay.  Then again the API for PRs as a whole doesn't seem to want update until I do that - the head revision and other info remains out of date until I look at the PR page.  So, the merge_commit_sha is seems about as reliable as anything else.  

could not suit our purposes since we track a particular PR commit and a particular base branch commit as part of the SCM revision

Could you expand on this? We're talking about using the GitHub API to get the contents of Jenkinsfile at a specific revision, right? In the case of the merge_commit_sha commit, it should have two parents that we can check and they should match up to the head and base commits.

jglick@cloudbees.com (JIRA)

unread,
Feb 25, 2019, 11:12:04 AM2/25/19
to jenkinsc...@googlegroups.com

In the case of the merge_commit_sha commit, it should have two parents that we can check and they should match up to the head and base commits.

It should have two parents, and they should be some PR commit and some base branch commit. Not necessarily the ones encoded in PullRequestSCMRevision.

bitwiseman@gmail.com (JIRA)

unread,
Feb 25, 2019, 2:51:05 PM2/25/19
to jenkinsc...@googlegroups.com

Jesse Glick
Just to keep this on track my original question was:
From what I can see merge_commit_sha should be what we're using for "Merge Strategy" pipeline jobs, what am I missing?

Points covered so far:
1. (resolved) Deprecated - they later decided not to deprecate it.
2. (workaround possible) Not generated consistently - May require a little hacking to trigger update, but that is doable.
3. (open) Not verifiably accurate - The parents of merge_commit_sha should be the PR head and base. You said maybe not. I don't understand your point there. In what case do we want to allow the parents of the merge_commit_sha not to match to the ones on the PullRequestSCMRevision ?

As I said, you have way more knowledge in this area and I really appreciate your help in drilling into this.

jglick@cloudbees.com (JIRA)

unread,
Feb 25, 2019, 3:10:04 PM2/25/19
to jenkinsc...@googlegroups.com

We have no control over what the parents of merge_commit_sha are. They may well include the PR’s current head and some commit from the base branch. Those are not, in general, going to be identical to what is specified in PullRequestSCMRevision for a given build.

stevengfoster@gmail.com (JIRA)

unread,
Feb 25, 2019, 3:21:03 PM2/25/19
to jenkinsc...@googlegroups.com

Is it feasible to apply this as an optimization, even if it only applies a certain percentage of time, with the current behavior as a fallback? I guess it depends on if the parents of merge_commit_sha are cheaper to inspect than cloning the repo.

jglick@cloudbees.com (JIRA)

unread,
Feb 25, 2019, 4:38:03 PM2/25/19
to jenkinsc...@googlegroups.com

We cannot use merge_commit_sha from this plugin. Just forget it exists.

W.r.t. optimizations: it is possible for GitHubSCMFileSystem.BuilderImpl.build to simply fall back to heavyweight checkouts if its preconditions are not met. In fact this is what it already does—the issue is that it does so for all cases of PullRequestSCMHead.merge, but this is a common case that we want lightweight checkout for. My suggestion from 2017-06-19 was to fail with a meaningful message rather than falling back to heavyweight checkout in case the merge is not fast-forward for the fie in question, since we do not really want the big clone on the master ever and the fix for the user is simple (just bring your PR up to date).

Anyway it seems that quietly falling back to heavyweight checkout would not work without bigger changes. SCMBinder checks for the existence of an SCMFileSystem at this revision and then uses SCMFileSystem.child(String) to load the Jenkinsfile or whatever. This is because scm-api does not allow the client to communicate to the implementation that it only cares about a single file: the decision to return null or not happens before the filename is used. So what we can do is to unconditionally return the SCMFileSystem but then make GitHubSCMFile.content throw the AbortException if that particular file cannot in fact be loaded via straightforward GitHub API calls (i.e., if it has diverged between the two parent commits from their merge base). Now it happens that SCMBinder currently falls back to heavyweight checkout on an IOException as well as a null return value from SCMFileSystem.of, which is something we could consider changing, or making customizable.

bitwiseman@gmail.com (JIRA)

unread,
Feb 25, 2019, 5:02:02 PM2/25/19
to jenkinsc...@googlegroups.com

We cannot use merge_commit_sha from this plugin. Just forget it exists.

Why? I need to understand why this is not possible. I don't need you to spoon feed it to me but I do need more information than you've provided.

The suggestion that we should fail and make the user bring their PR up-to-date is an interesting one. I'll add that to the list of options.

bitwiseman@gmail.com (JIRA)

unread,
Feb 25, 2019, 5:03:04 PM2/25/19
to jenkinsc...@googlegroups.com
Liam Newman edited a comment on Improvement JENKINS-43194
[~jglick]
{quote}We cannot use {{ merge_commit_sha }} from this plugin. Just forget it exists.{quote}
* Why? * I need to understand why this is not possible.   I don't need you to spoon feed it to me but I do need more information than you've provided.


The suggestion that we should fail and make the user bring their PR up-to-date is an interesting one.  I'll add that to the list of options.

bitwiseman@gmail.com (JIRA)

unread,
Mar 27, 2019, 7:22:06 PM3/27/19
to jenkinsc...@googlegroups.com

I've released a beta release of this change in v2.4.6-beta-1:

This is not production ready but anyone wanting to test it and give feedback would be appreciated. It is on the experimental update site.
Also: https://repo.jenkins-ci.org/releases/org/jenkins-ci/plugins/github-branch-source/2.4.6-beta-1/github-branch-source-2.4.6-beta-1.hpi

vivek.pandey@gmail.com (JIRA)

unread,
Apr 3, 2019, 12:04:05 PM4/3/19
to jenkinsc...@googlegroups.com
Vivek Pandey started work on Improvement JENKINS-43194
 
Change By: Vivek Pandey
Status: Open In Progress

vivek.pandey@gmail.com (JIRA)

unread,
Apr 3, 2019, 12:04:06 PM4/3/19
to jenkinsc...@googlegroups.com

bitwiseman@gmail.com (JIRA)

unread,
Apr 26, 2019, 12:46:03 PM4/26/19
to jenkinsc...@googlegroups.com
Liam Newman updated Improvement JENKINS-43194
 

Released in v2.5.0.

Change By: Liam Newman
Status: In Review Resolved
Resolution: Fixed

sysadmin@silverorange.com (JIRA)

unread,
Apr 29, 2019, 2:24:02 PM4/29/19
to jenkinsc...@googlegroups.com
Sys Admin commented on Improvement JENKINS-43194
 
Re: Lightweight checkout for PR merge jobs

I have been experiencing problems since the most recent update of this plugin to 2.5.0.  I'm running the latest Jenkins with all plugins updated.  After updating from 2.4.5 of this plugin to 2.5.0 my tests are failing with the following error.  As soon as I roll the plugin back to 2.4.5 the tests being passing again.  Anyone have any idea what might be causing this?

 

Caused: org.kohsuke.github.GHFileNotFoundException:

{"message":"No commit found for SHA: xxxxxxxxxxxxxxdc556f036","documentation_url":"[https://developer.github.com/v3/repos/commits/#get-a-single-commit]"}

bitwiseman@gmail.com (JIRA)

unread,
Apr 29, 2019, 6:52:07 PM4/29/19
to jenkinsc...@googlegroups.com

Sys Admin
Please take a look at
https://issues.jenkins-ci.org/browse/JENKINS-57195.
If that looks like your issue, please try upgrading again, then close and reopen the PR that is failing.

bitwiseman@gmail.com (JIRA)

unread,
Apr 29, 2019, 6:54:03 PM4/29/19
to jenkinsc...@googlegroups.com
Liam Newman edited a comment on Improvement JENKINS-43194
[~silverorange]
If that looks like your issue, please try upgrading again, then close and reopen the PR that is failing.    This is not a fix - but it will give me more information for debugging.

bitwiseman@gmail.com (JIRA)

unread,
Apr 29, 2019, 6:57:03 PM4/29/19
to jenkinsc...@googlegroups.com
Liam Newman edited a comment on Improvement JENKINS-43194
[~silverorange]
Please take a look at
https://issues.jenkins-ci.org/browse/JENKINS-57195.
If that looks like your issue, please try upgrading again, then close and reopen the PR that is failing.   This is not a fix - but it will give me more information for debugging.

  It would also be useful to have a full stack trace for that failure.

bitwiseman@gmail.com (JIRA)

unread,
Apr 29, 2019, 7:15:03 PM4/29/19
to jenkinsc...@googlegroups.com
Liam Newman updated an issue
 
Change By: Liam Newman
Comment:
[~silverorange]
Please take a look at
https://issues.jenkins-ci.org/browse/JENKINS-57195.
If that looks like your issue, please try upgrading again, then close and reopen the PR that is failing.   This is not a fix - but it will give me more information for debugging.  It would also be useful to have a full stack trace for that failure.

bitwiseman@gmail.com (JIRA)

unread,
Apr 29, 2019, 7:16:01 PM4/29/19
to jenkinsc...@googlegroups.com
Liam Newman commented on Improvement JENKINS-43194
 
Re: Lightweight checkout for PR merge jobs

Sys Admin
Please open a new issue with more details including callstacks.
As a work around please try closing and reopening the PR that is causing the failure.

bitwiseman@gmail.com (JIRA)

unread,
May 14, 2019, 12:07:03 PM5/14/19
to jenkinsc...@googlegroups.com
Liam Newman closed an issue as Fixed
 
Change By: Liam Newman
Status: Resolved Closed
Reply all
Reply to author
Forward
0 new messages