Only delete empty changelog files whose last modified time has not changed (#40)

Author: kshultzCB

.gitignore

@@ -5,3 +5,7 @@ work
 .classpath
 .project
 .settings
+
+# IntelliJ IDEA
+.idea
+*.iml

Jenkinsfile

@@ -1 +1 @@
-buildPlugin()
+buildPlugin(useAci: true)

src/main/java/org/jenkinsci/plugins/workflow/steps/scm/SCMStep.java

@@ -110,6 +110,7 @@ public final void checkout(Run<?,?> run, FilePath workspace, TaskListener listen
                     changelogFile = Files.createTempFile(run.getRootDir().toPath(), "changelog", ".xml").toFile();
                 }
             }
+            Long changelogOriginalModifiedDate = (changelogFile != null) ? changelogFile.lastModified() : null;
             SCM scm = createSCM();
             SCMRevisionState baseline = null;
             Run<?,?> prev = run.getPreviousBuild();
@@ -122,10 +123,11 @@ public final void checkout(Run<?,?> run, FilePath workspace, TaskListener listen
                 }
             }
             scm.checkout(run, launcher, workspace, listener, changelogFile, baseline);
-            if (changelogFile != null && changelogFile.length() == 0) {
-                // JENKINS-57918/JENKINS-59560: Some SCM plugins don't write anything to the changelog file in some
+            if (changelogFile != null && changelogFile.length() == 0
+                    && changelogOriginalModifiedDate != null && changelogFile.lastModified() == changelogOriginalModifiedDate) {
+                // JENKINS-57918/JENKINS-59560/FakeChangeLogSCM: Some SCMs don't write anything to the changelog file in some
                 // cases. `WorkflowRun.onCheckout` asks the SCM to parse the changelog file if it exists, and
-                // attempting to parse an empty file will cause an error, so we delete empty files before they even get
+                // attempting to parse an empty file will cause an error, so we delete changelog files that were not modified during the checkout before they even get
                 // to `WorkflowRun.onCheckout`.
                 Files.deleteIfExists(changelogFile.toPath());
                 changelogFile = null;

src/test/java/org/jenkinsci/plugins/workflow/steps/scm/SCMStepTest.java

@@ -40,13 +40,15 @@
 import hudson.util.StreamTaskListener;
 import java.io.File;
 import java.io.IOException;
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import jenkins.model.Jenkins;
 
 import jenkins.plugins.git.GitSampleRepoRule;
 import jenkins.scm.impl.subversion.SubversionSampleRepoRule;
 import org.apache.commons.io.FileUtils;
+import org.hamcrest.Matchers;
 import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
 import org.jenkinsci.plugins.workflow.cps.CpsScmFlowDefinition;
 import org.jenkinsci.plugins.workflow.job.WorkflowJob;
@@ -165,7 +167,7 @@ public void scmVars() throws Exception {
     }
 
     @Issue(value = { "JENKINS-57918", "JENKINS-59560" })
-    @Test public void scmParsesUmodifiedChangelogFile() {
+    @Test public void scmParsesUnmodifiedChangelogFile() {
         rr.then(r -> {
             WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p");
             p.setDefinition(new CpsFlowDefinition(
@@ -176,6 +178,46 @@ public void scmVars() throws Exception {
         });
     }
 
+    @Test public void scmParsesChangelogFileFromFakeChangeLogSCM() {
+        rr.then(r -> {
+            WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p");
+            p.setDefinition(new CpsFlowDefinition(
+                    "import org.jvnet.hudson.test.FakeChangeLogSCM\n" +
+                    "def testSCM = new FakeChangeLogSCM()\n" +
+                    "testSCM.addChange().withAuthor(/alice$BUILD_NUMBER/)\n" +
+                    "node() {\n" +
+                    "  checkout(testSCM)\n" +
+                    "}", false));
+            WorkflowRun b = r.buildAndAssertSuccess(p);
+            assertThat(b.getCulpritIds(), Matchers.equalTo(Collections.singleton("alice1")));
+        });
+    }
+
+    @Test public void gitChangelogSmokes() {
+        rr.then(r -> {
+            WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p");
+            p.setDefinition(new CpsFlowDefinition(
+                    "node() {\n" +
+                            "  checkout(scm: [\n" +
+                            "    $class: 'GitSCM',\n" +
+                            "    branches: [[name: '*/master']],\n" +
+                            "    userRemoteConfigs: [[url: '" + sampleGitRepo.fileUrl() + "']]\n" +
+                            "  ])\n" +
+                            "}", true));
+            sampleGitRepo.init(); // GitSampleRepoRule provides default user gits@mplereporule
+            sampleGitRepo.write("foo", "bar");
+            sampleGitRepo.git("add", "foo");
+            sampleGitRepo.git("commit", "-m", "Initial commit");
+            WorkflowRun b1 = r.buildAndAssertSuccess(p);
+            assertThat(b1.getCulpritIds(), Matchers.equalTo(Collections.emptySet()));
+            sampleGitRepo.write("foo", "bar1");
+            sampleGitRepo.git("add", "foo");
+            sampleGitRepo.git("commit", "-m", "Second commit");
+            WorkflowRun b2 = r.buildAndAssertSuccess(p);
+            assertThat(b2.getCulpritIds(), Matchers.equalTo(Collections.singleton("gits")));
+        });
+    }
+
     private static void assertPolling(WorkflowJob p, PollingResult.Change expectedChange) {
         assertEquals(expectedChange, p.poll(StreamTaskListener.fromStdout()).change);
     }
@@ -194,7 +236,7 @@ public ChangeLogSet<? extends ChangeLogSet.Entry> parse(Run build, RepositoryBro
                 }
             };
         }
-        @TestExtension("scmParsesUmodifiedChangelogFile")
+        @TestExtension("scmParsesUnmodifiedChangelogFile")
         public static class DescriptorImpl extends NullSCM.DescriptorImpl { }
     }