From e3ba6489eab6e409997a9862e42cf17931b408c7 Mon Sep 17 00:00:00 2001 From: CJ Steiner Date: Thu, 22 Jan 2026 21:06:11 -0600 Subject: [PATCH 1/3] Honor tag creation date properly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * "Ignore tags older than" field was using the commit creation date rather than the tag creation date. * Change correctly distinguishes between: - Annotated tags → Uses tag creation time (fixes the bug) - Lightweight tags → Uses commit time (backward compatible) This enables the "Ignore tags older than" feature to work correctly when new tags are created on old commits. Fixes: * https://github.com/jenkinsci/git-plugin/issues/3731 * https://github.com/jenkinsci/basic-branch-build-strategies-plugin/issues/268 --- .../plugins/git/AbstractGitSCMSource.java | 37 ++++++++++++++++--- .../git/AbstractGitSCMSourceWantTagsTest.java | 20 ++++++++++ 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java b/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java index 1504297965..268ec30e2c 100644 --- a/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java +++ b/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java @@ -110,6 +110,7 @@ import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevTag; import org.eclipse.jgit.revwalk.RevTree; import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.transport.RefSpec; @@ -425,6 +426,34 @@ private , R extends GitSCMSourceRequest> } } + private long getTagTimestamp(RevWalk walk, ObjectId objectId) throws IOException { + try { + // Annotated tag object + RevTag tag = walk.parseTag(objectId); + + if (tag.getTaggerIdent() != null + && tag.getTaggerIdent().getWhen() != null) { + return tag.getTaggerIdent().getWhen().getTime(); + } + + // No tagger ident (or weird tag) — walk the tag chain to a commit, if any + Object target = tag.getObject(); + for (int i = 0; i < 32 && target instanceof RevTag; i++) { + target = ((RevTag) target).getObject(); + } + + if (target instanceof RevCommit) { + return TimeUnit.SECONDS.toMillis(((RevCommit) target).getCommitTime()); + } + + throw new IOException("Tag does not ultimately reference a commit: " + objectId.name()); + } catch (org.eclipse.jgit.errors.IncorrectObjectTypeException e) { + // Lightweight tag (or direct commit id) + RevCommit commit = walk.parseCommit(objectId); + return TimeUnit.SECONDS.toMillis(commit.getCommitTime()); + } + } + /** * {@inheritDoc} */ @@ -786,8 +815,7 @@ private void discoverTags(final Repository repository, } count++; final String tagName = StringUtils.removeStart(ref.getKey(), Constants.R_TAGS); - RevCommit commit = walk.parseCommit(ref.getValue()); - final long lastModified = TimeUnit.SECONDS.toMillis(commit.getCommitTime()); + final long lastModified = getTagTimestamp(walk, ref.getValue()); if (request.process(new GitTagSCMHead(tagName, lastModified), new SCMSourceRequest.IntermediateLambda() { @Nullable @@ -804,7 +832,7 @@ public SCMSourceCriteria.Probe create(@NonNull GitTagSCMHead head, @Nullable ObjectId revisionInfo) throws IOException, InterruptedException { RevCommit commit = walk.parseCommit(revisionInfo); - final long lastModified = TimeUnit.SECONDS.toMillis(commit.getCommitTime()); + final long lastModified = getTagTimestamp(walk, revisionInfo); final RevTree tree = commit.getTree(); return new TreeWalkingSCMProbe(tagName, lastModified, repository, tree); } @@ -1012,8 +1040,7 @@ public SCMRevision run(GitClient client, String remoteName) throws GitException, final Repository repository = client.getRepository(); RevWalk walk = new RevWalk(repository)) { ObjectId ref = client.revParse(tagRef); - RevCommit commit = walk.parseCommit(ref); - long lastModified = TimeUnit.SECONDS.toMillis(commit.getCommitTime()); + long lastModified = getTagTimestamp(walk, ref); listener.getLogger().printf("Resolved tag %s revision %s%n", revision, ref.getName()); return new GitTagSCMRevision(new GitTagSCMHead(revision, lastModified), diff --git a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceWantTagsTest.java b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceWantTagsTest.java index 2a7b87ca76..18140f1f90 100644 --- a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceWantTagsTest.java +++ b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceWantTagsTest.java @@ -3,6 +3,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -169,6 +170,25 @@ void indexingHasBranchAndTagDiscoveryTraitIgnoreTagDiscoveryTrait() throws Excep assertTrue(tagsFetched); } + @Test + public void tagTimestampsAreValid() throws Exception { + source.setTraits(Collections.singletonList(new TagDiscoveryTrait())); + Set heads = source.fetch(LISTENER); + + Set tags = heads.stream() + .filter(h -> h instanceof GitTagSCMHead) + .map(h -> (GitTagSCMHead) h) + .collect(Collectors.toSet()); + + assertThat("Should discover both tags", tags.size(), is(2)); + + long year2000 = 946684800000L; // Jan 1, 2000 + for (GitTagSCMHead tag : tags) { + assertThat("Tag " + tag.getName() + " should have valid timestamp", + tag.getTimestamp(), greaterThan(year2000)); + } + } + static boolean tagsFetched; public static class MockGitClientForTags extends TestJGitAPIImpl { From 2b5227017acb2ca9af355cb1a7ec6594fc3a4b6d Mon Sep 17 00:00:00 2001 From: CJ Steiner Date: Thu, 22 Jan 2026 22:29:35 -0600 Subject: [PATCH 2/3] pr comments --- .../plugins/git/AbstractGitSCMSource.java | 29 ++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java b/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java index 268ec30e2c..d9b9599cd8 100644 --- a/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java +++ b/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java @@ -109,7 +109,9 @@ import org.eclipse.jgit.lib.FileMode; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevObject; import org.eclipse.jgit.revwalk.RevTag; import org.eclipse.jgit.revwalk.RevTree; import org.eclipse.jgit.revwalk.RevWalk; @@ -428,18 +430,24 @@ private , R extends GitSCMSourceRequest> private long getTagTimestamp(RevWalk walk, ObjectId objectId) throws IOException { try { - // Annotated tag object - RevTag tag = walk.parseTag(objectId); - - if (tag.getTaggerIdent() != null - && tag.getTaggerIdent().getWhen() != null) { - return tag.getTaggerIdent().getWhen().getTime(); + RevObject target = walk.parseAny(objectId); + + // If the first hash is an annotated tag, prefer its tag time + if (target instanceof RevTag) { + RevTag tag = walk.parseTag((RevTag) target); + PersonIdent tagger = tag.getTaggerIdent(); + if (tagger != null && tagger.getWhen() != null) { + return tagger.getWhen().getTime(); + } + target = tag.getObject(); // walk to commit if needed } - // No tagger ident (or weird tag) — walk the tag chain to a commit, if any - Object target = tag.getObject(); - for (int i = 0; i < 32 && target instanceof RevTag; i++) { - target = ((RevTag) target).getObject(); + // Walk until we reach a commit (or give up) + target = walk.parseAny(target); + for (int i = 0; i < 32 && !(target instanceof RevCommit); i++) { //32 is to guard against inf loop + if (!(target instanceof RevTag)) break; + RevTag tag = walk.parseTag((RevTag) target); + target = walk.parseAny(tag.getObject()); } if (target instanceof RevCommit) { @@ -454,6 +462,7 @@ private long getTagTimestamp(RevWalk walk, ObjectId objectId) throws IOException } } + /** * {@inheritDoc} */ From d2c7e7b2b784449c7e0c2630741ded1ffdced2dc Mon Sep 17 00:00:00 2001 From: CJ Steiner Date: Fri, 23 Jan 2026 03:31:31 -0600 Subject: [PATCH 3/3] AbstractGitSCMSourceWantTagsTest.java: add tests over getTagTimestamp --- .../git/AbstractGitSCMSourceWantTagsTest.java | 124 ++++++++++++++++++ 1 file changed, 124 insertions(+) diff --git a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceWantTagsTest.java b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceWantTagsTest.java index 18140f1f90..5cf73dab30 100644 --- a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceWantTagsTest.java +++ b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceWantTagsTest.java @@ -5,8 +5,11 @@ import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.lessThanOrEqualTo; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import hudson.EnvVars; import hudson.model.TaskListener; @@ -63,6 +66,8 @@ static void beforeAll(GitSampleRepoRule repo) throws Exception { sampleRepo.write("file", "modified"); sampleRepo.git("commit", "--all", "--message=" + BRANCH_NAME + "-commit-1"); sampleRepo.git("tag", LIGHTWEIGHT_TAG_NAME); + // Sleep to ensure annotated tag has a different timestamp than lightweight tag + Thread.sleep(1100); sampleRepo.write("file", "modified2"); sampleRepo.git("commit", "--all", "--message=" + BRANCH_NAME + "-commit-2"); sampleRepo.git("tag", "-a", ANNOTATED_TAG_NAME, "-m", "annotated-tag-message"); @@ -189,6 +194,125 @@ public void tagTimestampsAreValid() throws Exception { } } + @Test + public void lightweightTagHasCommitTimestamp() throws Exception { + source.setTraits(Collections.singletonList(new TagDiscoveryTrait())); + Set heads = source.fetch(LISTENER); + + GitTagSCMHead lightweightTag = heads.stream() + .filter(h -> h instanceof GitTagSCMHead && h.getName().equals(LIGHTWEIGHT_TAG_NAME)) + .map(h -> (GitTagSCMHead) h) + .findFirst() + .orElse(null); + + assertNotNull(lightweightTag, "Lightweight tag should be discovered"); + assertThat("Lightweight tag should have a timestamp", lightweightTag.getTimestamp(), greaterThan(0L)); + } + + @Test + public void annotatedTagHasValidTimestamp() throws Exception { + source.setTraits(Collections.singletonList(new TagDiscoveryTrait())); + Set heads = source.fetch(LISTENER); + + GitTagSCMHead annotatedTag = heads.stream() + .filter(h -> h instanceof GitTagSCMHead && h.getName().equals(ANNOTATED_TAG_NAME)) + .map(h -> (GitTagSCMHead) h) + .findFirst() + .orElse(null); + + assertNotNull(annotatedTag, "Annotated tag should be discovered"); + assertThat("Annotated tag should have a timestamp", annotatedTag.getTimestamp(), greaterThan(0L)); + } + + @Test + public void lightweightAndAnnotatedTagsHaveDifferentCharacteristics() throws Exception { + source.setTraits(Collections.singletonList(new TagDiscoveryTrait())); + Set heads = source.fetch(LISTENER); + + Set tags = heads.stream() + .filter(h -> h instanceof GitTagSCMHead) + .map(h -> (GitTagSCMHead) h) + .collect(Collectors.toSet()); + + assertThat("Should discover both tags", tags.size(), is(2)); + + // Both tags should have valid timestamps + for (GitTagSCMHead tag : tags) { + long timestamp = tag.getTimestamp(); + assertThat("Tag " + tag.getName() + " timestamp should be positive", timestamp, greaterThan(0L)); + // Timestamps should be in milliseconds (modern times are > 1.5 billion ms since epoch) + assertThat("Tag " + tag.getName() + " timestamp should be in milliseconds", + timestamp, greaterThan(1500000000000L)); + } + } + + @Test + public void allDiscoveredTagsHaveValidTimestamps() throws Exception { + source.setTraits(Collections.singletonList(new TagDiscoveryTrait())); + Set heads = source.fetch(LISTENER); + + Set tags = heads.stream() + .filter(h -> h instanceof GitTagSCMHead) + .map(h -> (GitTagSCMHead) h) + .collect(Collectors.toSet()); + + assertThat("Should discover both tags", tags.size(), is(2)); + + // All tags should have timestamps that are: + // 1. Greater than year 2000 in milliseconds (946684800000) + // 2. Less than or equal to current time + long year2000Millis = 946684800000L; + long currentTimeMillis = System.currentTimeMillis(); + + for (GitTagSCMHead tag : tags) { + long timestamp = tag.getTimestamp(); + assertThat("Tag " + tag.getName() + " timestamp should be after year 2000", + timestamp, greaterThan(year2000Millis)); + assertThat("Tag " + tag.getName() + " timestamp should not be in the future", + timestamp, lessThanOrEqualTo(currentTimeMillis)); + } + } + + @Test + public void annotatedTagHasDifferentTimestampFromLightweightTag() throws Exception { + source.setTraits(Collections.singletonList(new TagDiscoveryTrait())); + Set heads = source.fetch(LISTENER); + + GitTagSCMHead lightweightTag = heads.stream() + .filter(h -> h instanceof GitTagSCMHead && h.getName().equals(LIGHTWEIGHT_TAG_NAME)) + .map(h -> (GitTagSCMHead) h) + .findFirst() + .orElse(null); + + GitTagSCMHead annotatedTag = heads.stream() + .filter(h -> h instanceof GitTagSCMHead && h.getName().equals(ANNOTATED_TAG_NAME)) + .map(h -> (GitTagSCMHead) h) + .findFirst() + .orElse(null); + + assertNotNull(lightweightTag, "Lightweight tag should be discovered"); + assertNotNull(annotatedTag, "Annotated tag should be discovered"); + + long lightweightTimestamp = lightweightTag.getTimestamp(); + long annotatedTimestamp = annotatedTag.getTimestamp(); + + // Lightweight tag uses the commit's timestamp (commit-1) + // Annotated tag uses the tagger's timestamp (when the tag was created, after a 1.1 second sleep) + // They should have different timestamps due to the sleep in beforeAll + assertFalse( + lightweightTimestamp == annotatedTimestamp, + "Annotated tag timestamp (" + annotatedTimestamp + ") should differ from " + + "lightweight tag timestamp (" + lightweightTimestamp + ") " + + "since annotated tag uses tagger's timestamp while lightweight uses commit timestamp" + ); + + // Annotated tag should be newer (created after the sleep) + assertThat( + "Annotated tag timestamp should be greater than lightweight tag timestamp", + annotatedTimestamp, greaterThan(lightweightTimestamp) + ); + } + static boolean tagsFetched; public static class MockGitClientForTags extends TestJGitAPIImpl {