Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-67281

Commits since last build is wrong when last build commit no longer exists

    XMLWordPrintable

Details

    Description

      The "Commits since last build" shows 200 when it can not determine the commits since last build.

      Steps to reproduce:

      1. Push new branch with 1 commit
      2. Amend commit (replace commit) and force-push

      Instead of nothing or unable to determine, it shows 200 - which is wrong.

      • Commits since last build: 200

      Attachments

        Activity

          jk Jan Klass added a comment -

          I see now that this is probably not a forensics plugin issue, but git SCM instead.

          jk Jan Klass added a comment - I see now that this is probably not a forensics plugin issue, but git SCM instead.
          drulli Ulli Hafner added a comment -

          I think the component was correct. The output is from my plugin.

          The algorithm currently scans the log until it finds a matching commit (it stops after a configurable limit of 200 commits).

          Do know a way to detect that we have a rewriting of the commit history?

          drulli Ulli Hafner added a comment - I think the component was correct. The output is from my plugin. The algorithm currently scans the log until it finds a matching commit (it stops after a configurable limit of 200 commits). Do know a way to detect that we have a rewriting of the commit history?
          jk Jan Klass added a comment - - edited

          I don’t think identifying discarded commits is viable. With a force push, the commit is discarded from the ref/branch. I am not sure how Git hosting reflog and discarded objects may be accessible in general or between platforms (GitHub vs GitLab etc).

          I think a simple "try to find" approach where a force-push would mean no "x commits since" can be determined is fine. A force push is a rewriting of history, so determining a commit number/scalar diff value requires a well thought out concept which I would only do as a second step, if at all.

          If I force-push IMO it should display either no "commits since last build" or "could not determine commits since last build because last build commit was not found within the last 200 commits". I think given how long the second is, that may be reasonable as a log message rather than a displayed short info.

          If you really wanted to determine e.g. "discarded x commits, added y commits since last build", this may work for repositories already cloned and reused on a node, as it could still refer to and log-diff to the old object. You open a whole bunch of complexity there though with handling this, and communicating to the user what the dependency or platform conditions and consequences are, and how to display this and/or discard+add adequately.

          I am not that familiar with how Jenkins master tracks refs to determine diffs, so I can not really make guesses there. Maybe it would be reasonably possible with that - at least to a degree where it is viable.

          jk Jan Klass added a comment - - edited I don’t think identifying discarded commits is viable. With a force push, the commit is discarded from the ref/branch. I am not sure how Git hosting reflog and discarded objects may be accessible in general or between platforms (GitHub vs GitLab etc). I think a simple "try to find" approach where a force-push would mean no "x commits since" can be determined is fine. A force push is a rewriting of history, so determining a commit number/scalar diff value requires a well thought out concept which I would only do as a second step, if at all. If I force-push IMO it should display either no "commits since last build" or "could not determine commits since last build because last build commit was not found within the last 200 commits". I think given how long the second is, that may be reasonable as a log message rather than a displayed short info. If you really wanted to determine e.g. "discarded x commits, added y commits since last build", this may work for repositories already cloned and reused on a node, as it could still refer to and log-diff to the old object. You open a whole bunch of complexity there though with handling this, and communicating to the user what the dependency or platform conditions and consequences are, and how to display this and/or discard+add adequately. I am not that familiar with how Jenkins master tracks refs to determine diffs, so I can not really make guesses there. Maybe it would be reasonably possible with that - at least to a degree where it is viable.
          jk Jan Klass added a comment -

          To summarize: To fix this bug I would not show any "since last build" number/info if the previous build commit can not be located. Given that there is a limit (200), a log message informing about this and the limit would be helpful.

          jk Jan Klass added a comment - To summarize: To fix this bug I would not show any "since last build" number/info if the previous build commit can not be located. Given that there is a limit (200), a log message informing about this and the limit would be helpful.
          drulli Ulli Hafner added a comment -

          Yes, that makes sense! We need to change that in three different steps but it shouldn't be hard to improve that. One idea is to toggle a flag in BuildCommits if the max number of commits has been reached.

          If you are interested in helping with that fix feel free to provide a PR.

          drulli Ulli Hafner added a comment - Yes, that makes sense! We need to change that in three different steps but it shouldn't be hard to improve that. One idea is to toggle a flag in BuildCommits if the max number of commits has been reached. If you are interested in helping with that fix feel free to provide a PR.
          jk Jan Klass added a comment -

          Not currently.

          jk Jan Klass added a comment - Not currently.

          People

            Unassigned Unassigned
            jk Jan Klass
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated: