SCMSourceRetrieverTest.lease highlighted that dir2Jar was improperly deleting its source; anyway it needed to be refactored to be simpler and more efficient by directly archiving the desired files

Author: jglick

src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRetriever.java

@@ -36,7 +36,10 @@
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import edu.umd.cs.findbugs.annotations.NonNull;
 import hudson.slaves.WorkspaceList;
+import hudson.util.DirScanner;
+import hudson.util.FileVisitor;
 import hudson.util.io.ArchiverFactory;
+import java.io.File;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.nio.file.Path;
@@ -84,35 +87,28 @@ public void retrieveJar(@NonNull String name, @NonNull String version, boolean c
      * into a JAR file with Groovy in classpath orientation and {@code resources/} as a ZIP folder.
      */
     static void dir2Jar(@NonNull String name, @NonNull FilePath dir, @NonNull FilePath jar) throws IOException, InterruptedException {
-        // TODO do this more efficiently by packing JAR directly
-        FilePath tmp = jar.sibling(jar.getBaseName() + "-repack");
-        tmp.mkdirs();
+        lookForBadSymlinks(dir, dir);
+        FilePath mf = jar.withSuffix(".mf");
         try {
-            FilePath src = dir.child("src");
-            if (src.isDirectory()) {
-                src.moveAllChildrenTo(tmp);
-            }
-            FilePath vars = dir.child("vars");
-            if (vars.isDirectory()) {
-                vars.moveAllChildrenTo(tmp);
-            }
-            FilePath resources = dir.child("resources");
-            if (resources.isDirectory()) {
-                resources.renameTo(tmp.child("resources"));
-            }
-            lookForBadSymlinks(tmp, tmp);
-            try (OutputStream os = tmp.child(JarFile.MANIFEST_NAME).write()) {
+            try (OutputStream os = mf.write()) {
                 Manifest m = new Manifest();
                 m.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0");
                 // Informational debugging aid, since the hex JAR basename will be meaningless:
                 m.getMainAttributes().putValue(ATTR_LIBRARY_NAME, name);
                 m.write(os);
             }
             try (OutputStream os = jar.write()) {
-                tmp.archive(ArchiverFactory.ZIP, os, "**");
+                dir.archive(ArchiverFactory.ZIP, os, new DirScanner() {
+                    @Override public void scan(File dir, FileVisitor visitor) throws IOException {
+                        scanSingle(new File(mf.getRemote()), JarFile.MANIFEST_NAME, visitor);
+                        new DirScanner.Glob("**/*.groovy", null).scan(new File(dir, "src"), visitor);
+                        new DirScanner.Glob("*.groovy,*.txt", null).scan(new File(dir, "vars"), visitor);
+                        new DirScanner.Glob("resources/", null).scan(dir, visitor);
+                    }
+                });
             }
         } finally {
-            tmp.deleteRecursive();
+            mf.delete();
         }
     }
 

src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryRetrieverTest.java

@@ -33,6 +33,7 @@
 import java.util.jar.JarFile;
 import org.apache.commons.io.IOUtils;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.arrayContainingInAnyOrder;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.is;
 import static org.junit.Assert.assertThrows;
@@ -56,6 +57,10 @@ public class LibraryRetrieverTest {
         assertDir2Jar(Set.of("src/p1/xxx.groovy", "vars/yyy.groovy", "resources/a.txt", "resources/b/c.txt"), Set.of("p1/xxx.groovy", "yyy.groovy", "resources/a.txt", "resources/b/c.txt"));
     }
 
+    @Test public void otherFiles() throws Exception {
+        assertDir2Jar(Set.of("vars/v.groovy", "vars/v.txt", "vars/README.md", "README.md", "docs/usage.png", "src/p1/C.groovy", "src/p1/README.md"), Set.of("v.groovy", "v.txt", "p1/C.groovy"));
+    }
+
     @Test public void safeSymlinks() throws Exception {
         FilePath work = new FilePath(tmp.newFolder());
         FilePath dir = work.child("dir");
@@ -79,16 +84,16 @@ public class LibraryRetrieverTest {
         assertThrows(SecurityException.class, () -> LibraryRetriever.dir2Jar("mylib", dir, jar));
     }
 
-    // TODO assert that other files are not copied
-
     private void assertDir2Jar(Set<String> inputs, Set<String> outputs) throws Exception {
         FilePath work = new FilePath(tmp.newFolder());
         FilePath dir = work.child("dir");
         for (String input : inputs) {
             dir.child(input).write("xxx", null);
         }
         FilePath jar = work.child("x.jar");
+        var before = dir.list("**");
         LibraryRetriever.dir2Jar("mylib", dir, jar);
+        assertThat(dir.list("**"), arrayContainingInAnyOrder(before));
         Set<String> actualOutputs = new TreeSet<>();
         try (JarFile jf = new JarFile(jar.getRemote())) {
             assertThat(jf.getManifest().getMainAttributes().getValue(LibraryRetriever.ATTR_LIBRARY_NAME), is("mylib"));