Implement ChangeRequestSCMHead2 for SCMHead & SCMRevision #382

atl-dyon · 2023-05-11T00:29:18Z

Changes the git SCMHead and SCMRevision to use ChangeRequestSCMHead2.

Submitter checklist

Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
Ensure that the pull request title represents the desired changelog entry
Please describe what you did
Link to relevant issues in GitHub or Jira
Link to relevant pull requests, esp. upstream and downstream changes
Ensure you have provided tests - that demonstrates feature works or fixes the issue

dkjellin · 2023-05-23T00:33:54Z

src/main/java/com/atlassian/bitbucket/jenkins/internal/scm/BitbucketPullRequestSCMRevision.java

+    }
+
+    /**
+     * The commit hash of the head of the change request branch.


Suggested change

* The commit hash of the head of the change request branch.

* The commit hash of the pull request source branch (source ref)

dkjellin · 2023-05-23T00:37:06Z

src/main/java/com/atlassian/bitbucket/jenkins/internal/scm/BitbucketSCMHead.java

+
+    @Override
+    @NonNull
+    public SCMHeadOrigin getOrigin() {


why do we override this just to return the same value as the overridden method?

This is because we need to eventually determine the origin. For now we can copy the default behaviour, but when we add support for branches from forks, we will need to update this method to handle that. I felt like its best keeping it in earlier rather than later. I'm happy to add a comment to the method for when we go back to it

dkjellin · 2023-05-23T00:44:25Z

src/main/java/com/atlassian/bitbucket/jenkins/internal/scm/BitbucketSCMRevision.java

+public class BitbucketSCMRevision extends SCMRevision {
+
+    private static final long serialVersionUID = 1L;
+    private final String hash;


Suggested change

private final String hash;

private final String commitHash;

change name of getter

just to be explicit and avoid confusing ourselves. Up to you though

src/main/java/com/atlassian/bitbucket/jenkins/internal/scm/MinimalPullRequest.java

dkjellin · 2023-05-23T01:42:10Z

src/main/java/com/atlassian/bitbucket/jenkins/internal/trigger/BitbucketWebhookConsumer.java

-            return Collections.singletonMap(new GitBranchSCMHead(fromRef.getDisplayId()),
-                    new GitBranchSCMRevision(new GitBranchSCMHead(fromRef.getDisplayId()), fromRef.getLatestCommit()));
+            BitbucketPullRequestSCMHead prScmHead = new BitbucketPullRequestSCMHead(getPayload().getPullRequest());
+            return Collections.singletonMap(prScmHead,


This is totally fine, but we also have Map.of(k,v) which may be nicer? Up to you.

KalleOlaviNiemitalo · 2023-05-23T09:40:01Z

I guess this would fix [JENKINS-66581] Implement ChangeRequestSCMHead2 for pull requests.

KalleOlaviNiemitalo · 2023-05-23T09:47:55Z

src/main/java/com/atlassian/bitbucket/jenkins/internal/scm/BitbucketPullRequestSCMRevision.java

+        ChangeRequestSCMRevision<?> that = (ChangeRequestSCMRevision<?>) o;
+        if (!equivalent(that)) {
+            return false;
+        }


This looks like infinite recursion to me. I don't see the purpose of the cast either, as the type of o is already ChangeRequestSCMRevision<?>.

KalleOlaviNiemitalo · 2023-05-23T09:55:58Z

src/main/java/com/atlassian/bitbucket/jenkins/internal/scm/BitbucketPullRequestSCMHead.java

+public class BitbucketPullRequestSCMHead extends BitbucketSCMHead implements ChangeRequestSCMHead2 {
+
+    public static final int PR_NAME_BRANCH_MAX_LENGTH = 20;
+    public static final String PR_NAME_TEMPLATE = "pr%s--%s--%s";


What does this name affect in Jenkins?

In a multibranch pipeline project, if Jenkins has generated a branch job for a pull request, and I change the target branch of the PR in Bitbucket Server, then will Jenkins generate a separate job for the new name, or will it continue the preexisting job as the pull request ID still matches?

The name affects the title of the job in the Pull Requests tab. It's a good question of whether we should generate a seperate job for a name change. Any thoughts @dkjellin @jaguilar-atl?

I'd like to keep the same Jenkins job for the same pull request, no matter how it is edited. The URL of the Jenkins job should also stay the same.

then will Jenkins generate a separate job for the new name, or will it continue the preexisting job as the pull request ID still matches?

Good point. I haven't tested it myself, but yes, I think it will generate a separate job as it uses the name as the unique identifier. I do agree that the Job should stay the same for a PR regardless of the target branch.

With that said, we should probably just use the PR id here to ensure that's the case @atl-dyon.

Thanks for raising this @KalleOlaviNiemitalo !

The Bitbucket Branch Source plugin formats names like PR-1234 and this is useful for including/excluding by wildcard in a multibranch project: I can configure the project to include PR-* and release/2.*, and it won't build release/1.* which we no longer support. (For pull requests, including by type might be more reliable than PR-*, but I don't know if that is available from any plugin.)

Include/exclude by wildcard is implemented as WildcardSCMHeadFilterTrait in scm-api-plugin, and it compares SCMHead.getName() to the patterns: https://github.com/jenkinsci/scm-api-plugin/blob/64378ab20c60b237483b1fcec104a6d2dc350d11/src/main/java/jenkins/scm/impl/trait/WildcardSCMHeadFilterTrait.java#L91-L103

Bitbucket Branch Source apparently implements the PR-1234 format here: https://github.com/jenkinsci/bitbucket-branch-source-plugin/blob/abb9aa5035c1fb40ed8638728c1245a3562f95b0/src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketSCMSource.java#L682-L706

and in some constructors of PullRequestSCMHead, but those are all deprecated, presumably because they were not able to support multiple build strategies (source branch or merge) for the same pull request.

KalleOlaviNiemitalo · 2023-05-25T19:04:24Z

On the naming of pull requests: Branch API 2.1105.v472604208c55 was just released with a fix for [JENKINS-55348] Display Pull Request Name instead of ID in the UI. It is now able to read the title of the pull request from an ObjectMetadataAction and display that as the job name. This already works with the Bitbucket Branch Source plugin. The Bitbucket Server Integration plugin however doesn't seem to add an ObjectMetadataAction yet. It is not necessary to implement that as part of this PR but please keep it in mind for the future.

…' into dg/implement-scmHead-scmRevision

atl-dyon added 6 commits April 27, 2023 13:46

Initial implementation of SCMHead & SCMRevision

7dd2ad7

fix checkstyle issues

c8b6722

add proper formatting of PR name

77cdde8

add getter in scmHead for pullRequest

a5eca05

serialize BitbucketPullRequest with minimalPullRequest

96dbdf2

add implements Serializable to minimalPullRequest

f5d41e8

atl-dyon changed the title ~~Dg/implement scm head scm revision~~ Implement ChangeRequestSCMHead2 for SCMHead & SCMRevision May 11, 2023

atl-dyon requested review from dkjellin and mhenschke-atl May 11, 2023 00:30

atl-dyon added the enhancement New feature or request label May 11, 2023

atl-dyon marked this pull request as ready for review May 11, 2023 00:30

dkjellin reviewed May 23, 2023

View reviewed changes

KalleOlaviNiemitalo reviewed May 23, 2023

View reviewed changes

implement review comments

f9f2573

atl-dyon added 2 commits May 26, 2023 10:44

Switch PR name in jenkins to pullRequestId

409541f

Merge branch 'master' into dg/implement-scmHead-scmRevision

56bdca0

jaguilar-atl approved these changes May 30, 2023

View reviewed changes

dkjellin approved these changes Jun 2, 2023

View reviewed changes

jaguilar-atl added 3 commits June 12, 2023 16:09

Merge remote-tracking branch 'origin/feature/JENKINS-66581-PR-support…

29c84f4

…' into dg/implement-scmHead-scmRevision

Return old GitSCMBranchHead in BitbucketSCMHeadPullRequestEvent#heads

386c75b

Add prefix to PR ID

cf6c789

jaguilar-atl merged commit eddb5b5 into feature/JENKINS-66581-PR-support Jun 13, 2023
12 checks passed

jaguilar-atl deleted the dg/implement-scmHead-scmRevision branch June 13, 2023 10:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement ChangeRequestSCMHead2 for SCMHead & SCMRevision #382

Implement ChangeRequestSCMHead2 for SCMHead & SCMRevision #382

atl-dyon commented May 11, 2023

dkjellin May 23, 2023

dkjellin May 23, 2023

atl-dyon May 23, 2023

dkjellin May 23, 2023

dkjellin May 23, 2023

KalleOlaviNiemitalo commented May 23, 2023

KalleOlaviNiemitalo May 23, 2023

KalleOlaviNiemitalo May 23, 2023

atl-dyon May 24, 2023

KalleOlaviNiemitalo May 24, 2023 •

edited

jaguilar-atl May 25, 2023

KalleOlaviNiemitalo May 25, 2023 •

edited

KalleOlaviNiemitalo May 25, 2023

KalleOlaviNiemitalo May 25, 2023

KalleOlaviNiemitalo commented May 25, 2023

	* The commit hash of the head of the change request branch.
	* The commit hash of the pull request source branch (source ref)

Implement ChangeRequestSCMHead2 for SCMHead & SCMRevision #382

Implement ChangeRequestSCMHead2 for SCMHead & SCMRevision #382

Conversation

atl-dyon commented May 11, 2023

Submitter checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KalleOlaviNiemitalo commented May 23, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KalleOlaviNiemitalo May 24, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KalleOlaviNiemitalo May 25, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KalleOlaviNiemitalo commented May 25, 2023

KalleOlaviNiemitalo May 24, 2023 •

edited

KalleOlaviNiemitalo May 25, 2023 •

edited