SHA for pull request is incorrect

[ppg]

The various environment variables set involving the current SHA, as well any code that uses CommitInfo eventually comes to this code for finding the SHA from the githook payload:

DotCi/src/main/java/com/groupon/jenkins/github/Payload.java

Lines 61 to 66 in 85673ee

	public String getSha() {
	if (isPullRequest()) {
	return getPullRequest().getJSONObject("head").getString("sha");
	}
	return payloadJson.getJSONObject("head_commit").getString("id");
	}

. However, on a PR build it pulls the code at FETCH_HEAD (i.e.

DotCi/src/main/java/com/groupon/jenkins/buildtype/dockercompose/BuildConfiguration.java

Line 167 in 85673ee

shellCommands.add("git reset --hard FETCH_HEAD");

) to correctly build the PR merged into its base (note since I didn't know until I studied up some, the SHA from that PR's tip is built separately by the push build); this means that its not really building the pull_request.head.sha code (that's handled by the push build), it's building the pull_request.merged_commit_sha (i.e. FETCH_HEAD).

An example of how this causes me problems is my build process pushes the artifacts (i.e. packages) to S3 and 'tags' them (i.e. names them) with the SHA; similar things I've done in the past might push docker images and tag with the SHA. Github is set to trigger both PR and code changes, so on a PR update dotci sees both the PR and push triggers and goes and builds both, but they both clash on pushing artifacts since they both think think they're the same SHA. The retrieval method is via a custom build type that is similar to the docker compose one and gets that info via:

BuildCause cause = build.getCause();
String gitShaShort = cause.getCommitInfo().getShortSha();`

At the very least it'd be nice (necessary?) to expose the merged commit SHA via Payload/CommitInfo so I could fix the problem in my custom build code. However, I think arguably the getSha call in Payload should be changed to pull the merged commit sha; that is much farther reaching, but I think it will make corner cases that are probably broken now but just undetected work correctly.

Side question, why doesn't Payload expose a getter for the payloadJson and pipe that through CommitInfo so consumers could lookup additional payload information if they desired?

If either (or both) of those paths sound alright I can make a PR, but I wanted to check on the desired path first before going down one.

Thanks,
\Peter

[suryagaddipati]

hi Peter, Thanks for reporting this. I will research this sometime this week.

[ppg]

@suryagaddipati any thoughts on this? I can make a PR for the work, but I wasn't sure which path I can choose; I'd prefer fixing getSha so its the real, local sha being built (i.e. in the PR build the merged commit sha, not the PR tip sha).

Thanks,
\Peter