jenkinsci / gitlab-plugin

A Jenkins plugin for interfacing with GitLab
https://plugins.jenkins.io/gitlab-plugin/
GNU General Public License v2.0
1.43k stars 613 forks source link

Latest commit is not set in gitlabAfter param if multiple commits while build is running #1200

Open marcusphi opened 2 years ago

marcusphi commented 2 years ago

Version report

Jenkins and plugins versions report:

Jenkins: 2.303.1
OS: Linux - 3.10.0-1160.6.1.el7.x86_64
---
...
git:4.8.2
git-changelog:3.11
git-client:3.9.0
git-server:1.10
...
gitlab-api:1.0.6
gitlab-branch-source:1.5.9
gitlab-oauth:1.12
gitlab-plugin:1.5.24
...
scm-api:2.6.5
...

Reproduction steps

Results

Expected result:

If multiple pushes happens while a build is running, only one build is queued, but when it runs the gitlabAfter param is set to commit of last push. That is, the latter trigger should supersede the former.

Actual result:

gitlabAfter is set to commit of first push. No build at all is triggered for second push. That change is "pending" until a build is triggered next time.

I believe this is incorrect behaviour.

I also commented in https://github.com/jenkinsci/gitlab-plugin/issues/841 which sounds similar, but I wanted to have a new clean issue for this problem.

marcusphi commented 2 years ago

Here is the Jenkins XML for the build when it's finished (somewhat shorted):

An interesting observation is that the BuildData action contains the last commit and the changeSet contains the penultimate, the first commit of the two.

gitlabAfter in build is f8b182c...

Another interesting observation is that there are two GitLabWebHookCause actions. These correspond to the two pushes.

<freeStyleBuild _class="hudson.model.FreeStyleBuild">
    <action _class="hudson.model.CauseAction">
        <cause _class="com.dabsquared.gitlabjenkins.cause.GitLabWebHookCause">
            <shortDescription>Started by GitLab push by Marcus Philip</shortDescription>
        </cause>
        <cause _class="com.dabsquared.gitlabjenkins.cause.GitLabWebHookCause">
            <shortDescription>Started by GitLab push by Marcus Philip</shortDescription>
        </cause>
    </action>
    ...
    <action _class="hudson.plugins.git.util.BuildData">
        <buildsByBranchName>
            <refsremotesoriginmaster _class="hudson.plugins.git.util.Build">
                <buildNumber>14</buildNumber>
                <marked>
                    <SHA1>bc97d0223a60c66f5463d3668463386658c93f0a</SHA1>
                    <branch>
                        <SHA1>bc97d0223a60c66f5463d3668463386658c93f0a</SHA1>
                        <name>refs/remotes/origin/master</name>
                    </branch>
                </marked>
                <revision>
                    <SHA1>bc97d0223a60c66f5463d3668463386658c93f0a</SHA1>
                    <branch>
                        <SHA1>bc97d0223a60c66f5463d3668463386658c93f0a</SHA1>
                        <name>refs/remotes/origin/master</name>
                    </branch>
                </revision>
            </refsremotesoriginmaster>
        </buildsByBranchName>
        <lastBuiltRevision>
            <SHA1>bc97d0223a60c66f5463d3668463386658c93f0a</SHA1>
            <branch>
                <SHA1>bc97d0223a60c66f5463d3668463386658c93f0a</SHA1>
                <name>refs/remotes/origin/master</name>
            </branch>
        </lastBuiltRevision>
        <remoteUrl>http://git.xyz.nu/TeamLP/adept-load-test.git</remoteUrl>
        <scmName/>
    </action>
    <building>false</building>
    <description>Started by GitLab push by Marcus Philip</description>
    <fullDisplayName>TeamLP-playground » test-sleep-githook-freestyle #14</fullDisplayName>
    <id>14</id>
    <timestamp>1638807693106</timestamp>
    <url>http://jenkins-xx.xyz.nu/view/Team%20LP/job/TeamLP-playground/job/test-sleep-githook-freestyle/14/</url>
    ...
    <changeSet _class="hudson.plugins.git.GitChangeSetList">
        <item _class="hudson.plugins.git.GitChangeSet">
            <affectedPath>README.md</affectedPath>
            <commitId>f8b182c515e28b0e53a052a6b089459497f84b74</commitId>
            <timestamp>1638807611000</timestamp>
            <author>
                <absoluteUrl>http://jenkins-xx.xyz.nu/user/marphi</absoluteUrl>
                <fullName>Marcus Philip</fullName>
            </author>
            <authorEmail>marcus.philip@xyz.se</authorEmail>
            <comment>Dummy commit 5 </comment>
            <date>2021-12-06 17:20:11 +0100</date>
            <id>f8b182c515e28b0e53a052a6b089459497f84b74</id>
            <msg>Dummy commit 5</msg>
            <path>
                <editType>edit</editType>
                <file>README.md</file>
            </path>
        </item>
        <kind>git</kind>
    </changeSet>
    <culprit>...</culprit>
</freeStyleBuild>
marcusphi commented 2 years ago

I see that PushHookTriggerHandlerImpl#createRevisionParameter() creates a RevisionParameterAction with combineCommits = false, which is used in shouldSchedule() and foldIntoExisting() in this class.

Maybe this is the error?

marcusphi commented 2 years ago

I see that PushHookTriggerHandlerImpl#createRevisionParameter() creates a RevisionParameterAction with combineCommits = false ...

Maybe this is the error?

Manual testing in our Jenkins instance with modified plugin suggests it was not this easy unfortunately. The behaviour is the same in tis regard.