Apparently some Revision.getSha1() is returning null. This field has unclear nullability; various methods in Revision imply that it may be null, though I am not sure what that would mean. Seems to date at least to this commit but may be older. Suggest making it be @Nonnull, enforced at runtime, and if there any resulting test failures then track down the root issue.

See the JENKINS-59008 verify job for another example of the null pointer exception in the JGit pre-build merge.

 I have this issue on 3 of 5 jobs... I just don't understand why it sometimes works and sometimes it doesn't...

Any chance we can get this fixed in the next release?

malice00 it is not likely to be fixed in the next release or any time very soon.

The bug only affects the JGit implementation, not the command line git implementation. Change your job to use the command line git implementation instead of the JGit implementation.

FWIW we started encountering this after upgrading from Git 3.12.1 + Git Client 2.9.0 to Git 4.0.0 + Git Client 3.0.0. We also use the JGit implementation.

I downgraded to no avail; builds against the same workspace clones continued to NPE.

I deleted the workspace clones, stayed on the older versions of Git and Git Client, and now have cleared the issue. I'm wondering if there is some sporadic edge case subtly corrupting the workspace clone and contributing to nulls per jglick's comment above. But that's speculation based on it clearing after I forced the job to re-clone.

I wouldn't rate this as a Minor bug. Today this started hitting us as well and it affected a lot of jobs at once. Could be rather caused by the git client plugin, but in the end the NPE occurs in the git plugin code.

malice00, markewaite, mag, what versions of the Git and Git Client plugins were you using when you ran into the NPE? I am trying to pin down the ultimate cause of the problem.

I made another attempt today to upgrade our production plugins to versions 4.0.0 and 3.0.0 respectively, first deleting the cloned repository from the build workspace. But met with no success, even after downgrading the plugins. The merge NPEed again with both the new and old versions.

I had to delete the clone once more and use the downgraded plugins to get a clean merge.

Before deleting it, however, I copied over the clone so as to poke around with the a regular Git client. Between it and plugin commits perhaps a clue will pop out.

I first detected the problem with a git client plugin 3.0.0 pre-release as far as I recall.

The simplest workaround is likely to switch from JGit to command line git for the job that is performing the merge.

Thanks for the info. CLI Git may be where I'm ultimately headed. One more try before then.

Here's some very coarse bisect-style testing. It suggests JGit 5.x might have some sort of breaking API change or perhaps a bug.

These first two tests were just replication controls. The third featured a doctored Git Client plugin.

All runs started with a clean workspace. Fresh repo clone each time.

G 4.0.0 + GC 3.0.0 with stock JGit 5.5.1 jars
  NPE replicated
G 3.12.1 + GC 3.0.0 with stock JGit 5.5.1 jars
  NPE replicated
G 3.12.1 + GC 3.0.0 with downgraded JGit 4.5.7*
  No NPE

G == Git plugin
GC == Git Client plugin
* == The JGit vintage bundled with GC 2.9.0.

Maybe 5.5.1 is returning a null commit hash for certain references. I could not yield one by wiring JGit 5.5.1 into a Groovy shell and pointing to to the problem repo, then fetching branches and tags (and the detached HEAD). Nor did anything jump out by spelunking JGit source. But I'm no expert.

Thanks very much, brianeray!

In our case it was failing even with the git client plugin 2.9.0 (git plugin 3.12.1) in some cases. However I noticed that after creating a copy of one problematic jenkins job the new one didn't exhibit the issue. Both (old&new) job configs were identical, therefore I cleared the old job's workspace (at all jenkins slaves) and it started working again. You may try that if you encounter the issue.

mag that is interesting. I'm wondering if we experienced a distinct NPE as we ran into it only upon upgrading. But it would be a good idea for me to try cloning the job as well.

This one made us revert version to Git/GitClient 3.11/2.8.

Lots of our pipeline-jobs with merge in first step were breaking after few seconds.

Please increase priority! 

Log:
11:59:44 Merging Revision f000ac5075553c119e286b18a561fdd01b50364a (origin/development) to origin/master, UserMergeOptions{mergeRemote='origin', mergeTarget='master', mergeStrategy='DEFAULT', fastForwardMode='FF'}
java.lang.NullPointerException at hudson.plugins.git.util.GitUtils.getRevisionForSHA1(GitUtils.java:165) at hudson.plugins.git.extensions.impl.PreBuildMerge.decorateRevisionToBuild(PreBuildMerge.java:120) at hudson.plugins.git.GitSCM.determineRevisionToBuild(GitSCM.java:1053) at hudson.plugins.git.GitSCM.checkout(GitSCM.java:1146) at org.jenkinsci.plugins.workflow.steps.scm.SCMStep.checkout(SCMStep.java:124) at org.jenkinsci.plugins.workflow.steps.scm.SCMStep$StepExecutionImpl.run(SCMStep.java:93) at org.jenkinsci.plugins.workflow.steps.scm.SCMStep$StepExecutionImpl.run(SCMStep.java:80) at org.jenkinsci.plugins.workflow.steps.SynchronousNonBlockingStepExecution.lambda$start$0(SynchronousNonBlockingStepExecution.java:47) at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511) at java.util.concurrent.FutureTask.run(FutureTask.java:266) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) at java.lang.Thread.run(Thread.java:745)

I got some time to do a finer-grained bisect, swapping different JGit release versions into Git Client 3.1.1. The first and last versions are from my coarse bisect several comments above.

The breaking behavior appears to originate in JGit 4.9.0.

N = Tested, NPE thrown
T = Tested, no NPE thrown

5.5.1.201910021850-r N

5.5.0.201909110433-r 

5.4.3.201909031940-r 
  ...
5.0.0.201806131550-r 

4.11.9.201909030838-r 
4.11.8.201904181247-r N
  ...
4.11.0.201803080745-r 

4.10.0.201712302008-r 

4.9.10.201904181027-r 
  ...
4.9.2.201712150930-r N
4.9.1.201712030800-r 
4.9.0.201710071750-r N

4.8.0.201706111038-r T

4.7.9.201904161809-r T
4.7.8.201903121755-r 
4.7.7.201812240805-r 
4.7.6.201810191618-r 
4.7.5.201810051826-r T
  ...
4.7.0.201704051617-r 

4.6.1.201703071140-r 
4.6.0.201612231935-r 

4.5.7.201904151645-r T

Let me see if diffing those two adjacent release tags yields any more clues.

Diffing JGit code was unproductive. I rebuilt Git 4.1.1 and Git Client 3.1.1 with some more logging spliced in and seem to be getting closer. A lot of tags are ultimately resolving to null commit hashes. PreBuildMerge scoops up branches and tags at the outset.

Feb 10, 2020 4:10:59 PM org.jenkinsci.plugins.gitclient.JGitAPIImpl getTags WARNING: CICD-128 tagName=1.6.18.1, raw tagRef=Ref[refs/tags/1.6.18.1=c617fb6b71266adb9ca6bff59a89ad7e06638864(-1)]
Feb 10, 2020 4:10:59 PM org.jenkinsci.plugins.gitclient.JGitAPIImpl getTags WARNING: CICD-128 adding tagRef.getPeeledObjectId()=AnyObjectId[9935fd11f9a3c6a93bf3120ef7d7e15a5d130ed8]
Feb 10, 2020 4:10:59 PM org.jenkinsci.plugins.gitclient.JGitAPIImpl getTags WARNING: CICD-128 tagName=3.1.10, raw tagRef=Ref[refs/tags/3.1.10=94adcf654c5ae0d4d2bb88ec01bdc43ed0bea442(-1)]
Feb 10, 2020 4:10:59 PM org.jenkinsci.plugins.gitclient.JGitAPIImpl getTags WARNING: CICD-128 adding tagRef.getPeeledObjectId()=null
Feb 10, 2020 4:10:59 PM org.jenkinsci.plugins.gitclient.JGitAPIImpl getTags WARNING: CICD-128 tagName=3.1.10.1, raw tagRef=Ref[refs/tags/3.1.10.1=be995402548cad1220fb4ed20c083323bb50d303(-1)]
Feb 10, 2020 4:10:59 PM org.jenkinsci.plugins.gitclient.JGitAPIImpl getTags WARNING: CICD-128 adding tagRef.getPeeledObjectId()=null
Feb 10, 2020 4:10:59 PM org.jenkinsci.plugins.gitclient.JGitAPIImpl getTags WARNING: CICD-128 tagName=3.1.2.2, raw tagRef=Ref[refs/tags/3.1.2.2=fdc33c0ca48ba47afe2ba36d2016d346f42120df(-1)]
Feb 10, 2020 4:10:59 PM org.jenkinsci.plugins.gitclient.JGitAPIImpl getTags WARNING: CICD-128 adding tagRef.getPeeledObjectId()=AnyObjectId[263187a0f9d5ed3747520c230728f276f6fd848d]
Feb 10, 2020 4:10:59 PM org.jenkinsci.plugins.gitclient.JGitAPIImpl getTags WARNING: CICD-128 tagName=3.1.6, raw tagRef=Ref[refs/tags/3.1.6=7e9ed43e4838b31eacdbd60d25908f1566bfe7cd(-1)]
Feb 10, 2020 4:10:59 PM org.jenkinsci.plugins.gitclient.JGitAPIImpl getTags WARNING: CICD-128 adding tagRef.getPeeledObjectId()=AnyObjectId[3d686b8d9bc8c02c7dab25f16b8a79217e568b1d]
 ...

That's JGitAPIImpl.getTags() here. My suspicion is still on JGit 4.9.0 but rigging up the plugins looked a little less intimidating than JGit.

With aid of a Groovy shell and JGit looking more closely at the first two tags logged above,

//Local repo clone instantiation ommitted
tags = repo.tags
twoTags = tags.findAll {
  k, _ -> [ "1.6.18.1", "3.1.10" ].contains(k)
}
twoTags.collectEntries {
  k, v -> [
    k, [ v.class.simpleName, v.peeled, v.objectId, v.peeledObjectId ]
  ]
}

yields this

===> [1.6.18.1:[PeeledTag, true, AnyObjectId[c617fb6b71266adb9ca6bff59a89ad7e06638864], AnyObjectId[9935fd11f9a3c6a93bf3120ef7d7e15a
5d130ed8]], 3.1.10:[PeeledNonTag, true, AnyObjectId[94adcf654c5ae0d4d2bb88ec01bdc43ed0bea442], null]]

A PeeledNonTag always returns a null peeledObjectId. Why the reference is not regarded as a tag is a little weird; it's clearly a tag in the upstream BitBucket repo.

Windows Git 2.23.0 CLI reflects differences in the two tags.

> git show-ref --dereference "1.6.18.1" "3.1.10"
c617fb6b71266adb9ca6bff59a89ad7e06638864 refs/tags/1.6.18.1
9935fd11f9a3c6a93bf3120ef7d7e15a5d130ed8 refs/tags/1.6.18.1^{}
94adcf654c5ae0d4d2bb88ec01bdc43ed0bea442 refs/tags/3.1.10

I'm trying to figure out whether the two tags really are structurally different in the upstream BitBucket repo, or something amiss in JGit's clone, or something else.

If the differences between the two tags are legitimate maybe JGitAPIImpl.getTags() will need to process PeeledNonTag types differently, returning the objectId instead of peeledObjectId.

Cloning the repo with Git CLI yields different tag types via git show-ref --dereference as above.

And the following makes me relatively certain that JGit 4.9.0+ is just now smarter in discerning the two. This is how JGit 4.8.0 sees the tags.

===> [1.6.18.1:[LooseUnpeeled, false, AnyObjectId[c617fb6b71266adb9ca6bff59a89ad7e06638864], null], 3.1.10:[LooseUnpeeled, false
, AnyObjectId[94adcf654c5ae0d4d2bb88ec01bdc43ed0bea442], null]]

As 4.8.0 treats both tags as LooseUnpeeled, a subtype of Unpeeled, with isPeeled() unconditionally false then JGitAPIImpl.getTags() will return the objectId for both. No nulls.

markewaite I could attempt a PR to teach JGitAPIImpl.getTags() about the 4.9.0 change in behavior. My initial guess would be something like

if (tagRef.isPeeled() && tagRef.getPeeledObjectId() != null ) {
    peeledTags.add(new GitObject(tagName, tagRef.getPeeledObjectId()));
} else if (!tagNames.contains(tagName)) {
    peeledTags.add(new GitObject(tagName, tagRef.getObjectId()));
}

It's also possible that JGit is not just behaving differently but behaving incorrectly now, though I'm much less confident in trying to troubleshoot that.

What do you think?

And finally, the two different tag types are annotated and lightweight tags. That makes sense, though the difference is not visible in BitBucket AFAICT.

Thanks for the excellent investigation brianeray! I think JGit 4.9.0 and later are handling things correctly. The JGitAPIImpl needs to be adapted as you've recommended. It previously assumed there was only a single type of tag. JGit extended itself to know about the two different tag types but the git client plugin implementation did not adapt.

Please submit the pull request with tests to show the broken behavior and with a code change to correct the broken behavior.

Thanks again for your great investigation into the problem!

Yep, happy to track this one down. Between other duties and being a first time contributor, I'm hoping to file a PR by early next week.

markewaite Could you take a quick look at this test in my fork?

Two concerns:

1. Confusingly the test doesn't yield the null for the lightweight tag. My patch is not yet in place so I'm expecting test failure. But it seems to resolve both tags to valid hashes, as if it were using JGit 4.8.0. This branch is a few commits behind master so it has JGit 5.6.0. Here's some logging spliced into JGitAPIImpl and GitAPITestCase amongst the Maven + Surefire logging:

  [INFO]  T E S T S
  [INFO] -------------------------------------------------------
  [INFO] Running org.jenkinsci.plugins.gitclient.JGitAPIImplTest
  Feb 18, 2020 8:51:12 AM org.jenkinsci.plugins.gitclient.JGitAPIImpl getTags WARNING: CICD-128 tagName=annotated_tag [LooseUnpeeled], raw tagRef=Ref[refs/tags/annotated_tag=1f11c640ba0d2e3a0547ed8a54a9a4a192f7ee78(-1)]
  Feb 18, 2020 8:51:12 AM org.jenkinsci.plugins.gitclient.JGitAPIImpl getTags WARNING: CICD-128 peeledRef=Ref[refs/tags/annotated_tag=1f11c640ba0d2e3a0547ed8a54a9a4a192f7ee78(-1)]
  Feb 18, 2020 8:51:12 AM org.jenkinsci.plugins.gitclient.JGitAPIImpl getTags WARNING: CICD-128 adding tagRef.getPeeledObjectId()=AnyObjectId[25c459ecee2cefc43b63a9be9e04ca3865da78b6]
  Feb 18, 2020 8:51:12 AM org.jenkinsci.plugins.gitclient.JGitAPIImpl getTags WARNING: CICD-128 tagName=lightweight_tag [LooseUnpeeled], raw tagRef=Ref[refs/tags/lightweight_tag=520d5510b59d216176e01982343ba579a96e2b32(-1)]
  Feb 18, 2020 8:51:12 AM org.jenkinsci.plugins.gitclient.JGitAPIImpl getTags WARNING: CICD-128 peeledRef=Ref[refs/tags/lightweight_tag=520d5510b59d216176e01982343ba579a96e2b32(-1)]
  Feb 18, 2020 8:51:12 AM org.jenkinsci.plugins.gitclient.JGitAPIImpl getTags WARNING: CICD-128 adding tagRef.getObjectId()=AnyObjectId[520d5510b59d216176e01982343ba579a96e2b32]
  DEBUG: tag=lightweight_tag, AnyObjectId[520d5510b59d216176e01982343ba579a96e2b32]
  DEBUG: tag=annotated_tag, AnyObjectId[25c459ecee2cefc43b63a9be9e04ca3865da78b6]
  [INFO] Tests run: 1 Failures: 0 Errors: 0 Skipped: 0 Time elapsed: 4.657 s - in org.jenkinsci.plugins.gitclient.JGitAPIImplTest
  [INFO]
  [INFO] Results:
  [INFO]
  [INFO] Tests run: 1 Failures: 0 Errors: 0 Skipped: 0

2. Partway through this test I realized there's already some method coverage higher up (?) in GitClientTest. Should my testing be there instead of GitAPITestCase?

Thanks.

Understood and will do. To be clear, the log from above is from a brand new test for the current issue. (Before I realized there was already some related coverage elsewhere.)

I'm at a loss as to why it's returning friendly LooseUnpeeled as opposed to the problematic PeeledNonTag for the lightweight tag. This suggests the test won't actually replicate the NPE use case. But I'll take another run at it with the JUnit 4 base. Knock on wood, maybe it will replicate the scenario then.

LooseUnpeeled objects are, naturally, loose refs. Peeled[Non]Tag objects are packed refs.

JGit 4.9.0+ packs all refs upon cloning. 4.8.0 and prior leaves them loose. That explains the variation in types.

I'm close to having a valid test and then the patch. But the upcoming two days are booked; it may be next week before this lands in a PR.

Thanks so much for your efforts brianeray!

Yep. Finally have a test that replicates the scenario available for a sneak peak here in my fork.

[ERROR] Failures:
[ERROR]   JGitClientTest.testGetTags_packedRefs:143
Expected: a collection containing (hasProperty("name", "lightweight_tag") and hasProperty("SHA1", <AnyObjectId[45809c4f5d218192b94cb668964ecb7f0a3aeebc]>))
     but: mismatches were: [hasProperty("name", "lightweight_tag")  property 'name' was "annotated_tag", hasProperty("SHA1", <AnyObjectId[45809c4f5d218192b94cb668964ecb7f0a3aeebc]>)  property 'SHA1' was null]
[INFO]
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0

That was almost as tricky to write as the NPE was to track down. (I wish the tests were Spock specs. ) Fixing the bug might be the easiest part!

Time to rebase, then the actual patch, then the PR ... maybe by the end of the week.

Thanks for doing that. Your experience matches mine when exploring a bug. By the time I've duplicated the bug in a unit test, I've understood it well enough that the bug fix is simple compared to all the learning that preceded it.

brianeray I'm starting the planning so that we can release a new version of the git plugin in the next 1-2 weeks. I'd love to include this fix in a git client plugin release at the same time, if it is ready. Thanks again for your work on it!

There are 3 pull requests to the git plugin that need to be evaluated and merged before the release. Wanted you to know the plans for the release in case that information helps you.

Thanks markewaite. PR is imminent, Monday at the latest but more likely this afternoon.

The full Git Client test suite just finished up here on my local, rebased-to-fresh-master branch. 100% success and and coverage on getTags() via Jacoco (largely in thanks to existing tests to be sure).

After grabbing some lunch I should be able slip the plugin into my test Jenkins and confirm the downstream NPE is squashed.

Expected to release in git client plugin 3.1.2 within the next 2 weeks

Release in git client plugin 3.2.0 March 1, 2020.