CC hrmpw. My latest suggestion was to check if the file had been modified in at most one of the branches to be merged, and if so, load it via API; otherwise simply throw an AbortException requesting that the user push a merge commit, even when there is not necessarily a line-by-line conflict.

jglick I assume you mean content of the Jenkinsfile right? (which most of the time is not changed in a PR compared to other code, so chances of this just working are hight) - do I have that right? 

michaelneale typically Jenkinsfile, though the name of that script is now customizable, and other files may be loaded with the same semantics from readTrusted; also from CpsScmFlowDefinition we could load any requested script.

Worth checking whether this API can be used to actually perform the merge on the server side and then access file contents from that (unreferenced?) commit.

jglick perhaps - would want to create a new branch to merge into (so it doesn't do a merge about a branch anyone cares about) - however, it would likely then trigger a build... 

vivek any progress on this? We have many repos which are quite large, and do merge PR builds. The full workspace checkout is taxing our masters significantly.

would want to create a new branch to merge into

Depending on what the behavior of the endpoint is. If it actually modifies the base ref, you cannot use it. It is only useful if it creates a new dangling commit. The API does not seem to specify—it shows the merge commit hash being returned, but that is all.

Just tested it and it does update the remote base branch reference. So this API is useless, ignore it. Back to my original suggestion:

check if the file had been modified in at most one of the branches to be merged, and if so, load it via API; otherwise simply throw an AbortException requesting that the user push a merge commit, even when there is not necessarily a line-by-line conflict

jglick 

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.

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.

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.

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.

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

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.

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.

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.

jglick

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.

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

Released in v2.5.0.

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:

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