Sketch of saving libraries as JAR files rather than unpacked

Author: jglick

pom.xml

@@ -75,6 +75,11 @@
                 <scope>import</scope>
                 <type>pom</type>
             </dependency>
+            <dependency>
+                <groupId>org.jenkins-ci.plugins.workflow</groupId>
+                <artifactId>workflow-cps</artifactId>
+                <version>999999-SNAPSHOT</version> <!-- TODO https://github.com/jenkinsci/workflow-cps-plugin/pull/668 -->
+            </dependency>
         </dependencies>
     </dependencyManagement>
     <dependencies>

src/main/java/org/jenkinsci/plugins/workflow/cps/global/UserDefinedGlobalVariable.java

@@ -22,6 +22,7 @@
  */
 // not @Extension because these are instantiated programmatically
 public class UserDefinedGlobalVariable extends GlobalVariable {
+    // TODO switch to URL
     private final File help;
     private final String name;
 

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

@@ -50,6 +50,8 @@
 import java.util.logging.Logger;
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import edu.umd.cs.findbugs.annotations.NonNull;
+import java.util.jar.JarFile;
+import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import org.apache.commons.io.IOUtils;
 import org.jenkinsci.plugins.workflow.cps.CpsFlowExecution;
@@ -97,6 +99,7 @@
         if (action != null) {
             // Resuming a build, so just look up what we loaded before.
             for (LibraryRecord record : action.getLibraries()) {
+                // TODO call LibraryRetriever.dir2Jar as needed
                 FilePath libDir = new FilePath(execution.getOwner().getRootDir()).child("libs/" + record.getDirectoryName());
                 for (String root : new String[] {"src", "vars"}) {
                     FilePath dir = libDir.child(root);
@@ -147,9 +150,7 @@
         // Now actually try to retrieve the libraries.
         for (LibraryRecord record : librariesAdded.values()) {
             listener.getLogger().println("Loading library " + record.name + "@" + record.version);
-            for (URL u : retrieve(record, retrievers.get(record.name), listener, build, execution)) {
-                additions.add(new Addition(u, record.trusted));
-            }
+            additions.add(new Addition(retrieve(record, retrievers.get(record.name), listener, build, execution), record.trusted));
         }
         return additions;
     }
@@ -169,14 +170,14 @@ private enum CacheStatus {
         EXPIRED;
     }
 
-    private static CacheStatus getCacheStatus(@NonNull LibraryCachingConfiguration cachingConfiguration, @NonNull final FilePath versionCacheDir)
+    private static CacheStatus getCacheStatus(@NonNull LibraryCachingConfiguration cachingConfiguration, @NonNull final FilePath versionCacheJar)
           throws IOException, InterruptedException
     {
         if (cachingConfiguration.isRefreshEnabled()) {
             final long cachingMilliseconds = cachingConfiguration.getRefreshTimeMilliseconds();
 
-            if(versionCacheDir.exists()) {
-                if ((versionCacheDir.lastModified() + cachingMilliseconds) > System.currentTimeMillis()) {
+            if(versionCacheJar.exists()) {
+                if ((versionCacheJar.lastModified() + cachingMilliseconds) > System.currentTimeMillis()) {
                     return CacheStatus.VALID;
                 } else {
                     return CacheStatus.EXPIRED;
@@ -185,7 +186,7 @@ private static CacheStatus getCacheStatus(@NonNull LibraryCachingConfiguration c
                 return CacheStatus.DOES_NOT_EXIST;
             }
         } else {
-            if (versionCacheDir.exists()) {
+            if (versionCacheJar.exists()) {
                 return CacheStatus.VALID;
             } else {
                 return CacheStatus.DOES_NOT_EXIST;
@@ -194,16 +195,16 @@ private static CacheStatus getCacheStatus(@NonNull LibraryCachingConfiguration c
     }
 
     /** Retrieve library files. */
-    static List<URL> retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriever retriever, @NonNull TaskListener listener, @NonNull Run<?,?> run, @NonNull CpsFlowExecution execution) throws Exception {
+    static URL retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriever retriever, @NonNull TaskListener listener, @NonNull Run<?,?> run, @NonNull CpsFlowExecution execution) throws Exception {
         String name = record.name;
         String version = record.version;
         boolean changelog = record.changelog;
         LibraryCachingConfiguration cachingConfiguration = record.cachingConfiguration;
-        FilePath libDir = new FilePath(execution.getOwner().getRootDir()).child("libs/" + record.getDirectoryName());
+        FilePath libJar = new FilePath(execution.getOwner().getRootDir()).child("libs/" + record.getDirectoryName() + ".jar");
         Boolean shouldCache = cachingConfiguration != null;
-        final FilePath versionCacheDir = new FilePath(LibraryCachingConfiguration.getGlobalLibrariesCacheDir(), record.getDirectoryName());
+        final FilePath versionCacheJar = new FilePath(LibraryCachingConfiguration.getGlobalLibrariesCacheDir(), record.getDirectoryName() + ".jar");
         ReentrantReadWriteLock retrieveLock = getReadWriteLockFor(record.getDirectoryName());
-        final FilePath lastReadFile = new FilePath(versionCacheDir, LibraryCachingConfiguration.LAST_READ_FILE);
+        final FilePath lastReadFile = versionCacheJar.sibling(record.getDirectoryName() + "." + LibraryCachingConfiguration.LAST_READ_FILE);
 
         if(shouldCache && cachingConfiguration.isExcluded(version)) {
             listener.getLogger().println("Library " + name + "@" + version + " is excluded from caching.");
@@ -213,13 +214,13 @@ static List<URL> retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriev
         if(shouldCache) {
             retrieveLock.readLock().lockInterruptibly();
             try {
-                CacheStatus cacheStatus = getCacheStatus(cachingConfiguration, versionCacheDir);
+                CacheStatus cacheStatus = getCacheStatus(cachingConfiguration, versionCacheJar);
                 if (cacheStatus == CacheStatus.DOES_NOT_EXIST || cacheStatus == CacheStatus.EXPIRED) {
                     retrieveLock.readLock().unlock();
                     retrieveLock.writeLock().lockInterruptibly();
                     try {
                       boolean retrieve = false;
-                      switch (getCacheStatus(cachingConfiguration, versionCacheDir)) {
+                      switch (getCacheStatus(cachingConfiguration, versionCacheJar)) {
                           case VALID: 
                               listener.getLogger().println("Library " + name + "@" + version + " is cached. Copying from home."); 
                                 break;
@@ -229,18 +230,17 @@ static List<URL> retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriev
                           case EXPIRED:
                               long cachingMinutes = cachingConfiguration.getRefreshTimeMinutes();
                               listener.getLogger().println("Library " + name + "@" + version + " is due for a refresh after " + cachingMinutes + " minutes, clearing.");
-                                if (versionCacheDir.exists()) {
-                                    versionCacheDir.deleteRecursive();
-                                    versionCacheDir.withSuffix("-name.txt").delete();
+                                if (versionCacheJar.exists()) {
+                                    versionCacheJar.delete();
+                                    versionCacheJar.sibling(record.getDirectoryName() + "-name.txt").delete();
                                 }
                                 retrieve = true;
                                 break;
                       }
 
                         if (retrieve) {
                             listener.getLogger().println("Caching library " + name + "@" + version);                            
-                            versionCacheDir.mkdirs();
-                            retriever.retrieve(name, version, changelog, versionCacheDir, run, listener);
+                            retriever.retrieveJar(name, version, changelog, versionCacheJar, run, listener);
                         }
                         retrieveLock.readLock().lock();
                     } finally {
@@ -251,50 +251,44 @@ static List<URL> retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriev
                 }
 
                 lastReadFile.touch(System.currentTimeMillis());
-                versionCacheDir.withSuffix("-name.txt").write(name, "UTF-8");
-                versionCacheDir.copyRecursiveTo(libDir);
+                versionCacheJar.sibling(record.getDirectoryName() + "-name.txt").write(name, "UTF-8");
+                versionCacheJar.copyTo(libJar);
             } finally {
               retrieveLock.readLock().unlock();
             }
         } else {
-            retriever.retrieve(name, version, changelog, libDir, run, listener);
+            retriever.retrieveJar(name, version, changelog, libJar, run, listener);
         }
         // Write the user-provided name to a file as a debugging aid.
-        libDir.withSuffix("-name.txt").write(name, "UTF-8");
+        libJar.sibling(record.getDirectoryName() + "-name.txt").write(name, "UTF-8");
 
         // Replace any classes requested for replay:
         if (!record.trusted) {
             for (String clazz : ReplayAction.replacementsIn(execution)) {
-                for (String root : new String[] {"src", "vars"}) {
-                    String rel = root + "/" + clazz.replace('.', '/') + ".groovy";
-                    FilePath f = libDir.child(rel);
-                    if (f.exists()) {
-                        String replacement = ReplayAction.replace(execution, clazz);
-                        if (replacement != null) {
-                            listener.getLogger().println("Replacing contents of " + rel);
-                            f.write(replacement, null); // TODO as below, unsure of encoding used by Groovy compiler
-                        }
+                String rel = clazz.replace('.', '/') + ".groovy";
+                /* TODO need to unpack & repack I guess
+                FilePath f = libDir.child(rel);
+                if (f.exists()) {
+                    String replacement = ReplayAction.replace(execution, clazz);
+                    if (replacement != null) {
+                        listener.getLogger().println("Replacing contents of " + rel);
+                        f.write(replacement, null); // TODO as below, unsure of encoding used by Groovy compiler
                     }
                 }
+                */
             }
         }
-        List<URL> urls = new ArrayList<>();
-        FilePath srcDir = libDir.child("src");
-        if (srcDir.isDirectory()) {
-            urls.add(srcDir.toURI().toURL());
-        }
-        FilePath varsDir = libDir.child("vars");
-        if (varsDir.isDirectory()) {
-            urls.add(varsDir.toURI().toURL());
-            for (FilePath var : varsDir.list("*.groovy")) {
-                record.variables.add(var.getBaseName());
-            }
-        }
-        if (urls.isEmpty()) {
-            throw new AbortException("Library " + name + " expected to contain at least one of src or vars directories");
+        try (JarFile jf = new JarFile(libJar.getRemote())) {
+            jf.stream().forEach(entry -> {
+                Matcher m = ROOT_GROOVY_SOURCE.matcher(entry.getName());
+                if (m.matches()) {
+                    record.variables.add(m.group(1));
+                }
+            });
         }
-        return urls;
+        return libJar.toURI().toURL();
     }
+    private static final Pattern ROOT_GROOVY_SOURCE = Pattern.compile("([^/]+)[.]groovy");
 
     /**
      * Loads resources for {@link ResourceStep}.
@@ -309,6 +303,7 @@ static List<URL> retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriev
             Run<?,?> run = (Run) executable;
             LibrariesAction action = run.getAction(LibrariesAction.class);
             if (action != null) {
+                // TODO handle *.jar
                 FilePath libs = new FilePath(run.getRootDir()).child("libs");
                 for (LibraryRecord library : action.getLibraries()) {
                     FilePath libResources = libs.child(library.getDirectoryName() + "/resources/");
@@ -347,6 +342,7 @@ private static String readResource(FilePath file, @CheckForNull String encoding)
             List<GlobalVariable> vars = new ArrayList<>();
             for (LibraryRecord library : action.getLibraries()) {
                 for (String variable : library.variables) {
+                    // TODO pass URL of *.jar!/$variable.txt
                     vars.add(new UserDefinedGlobalVariable(variable, new File(run.getRootDir(), "libs/" + library.getDirectoryName() + "/vars/" + variable + ".txt")));
                 }
             }
@@ -367,6 +363,7 @@ private static String readResource(FilePath file, @CheckForNull String encoding)
                     Run<?,?> run = (Run) executable;
                     LibrariesAction action = run.getAction(LibrariesAction.class);
                     if (action != null) {
+                        // TODO handle *.jar
                         FilePath libs = new FilePath(run.getRootDir()).child("libs");
                         for (LibraryRecord library : action.getLibraries()) {
                             if (library.trusted) {

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

@@ -31,7 +31,7 @@ public final class LibraryCachingConfiguration extends AbstractDescribableImpl<L
     private String excludedVersionsStr;
 
     private static final String VERSIONS_SEPARATOR = " ";
-    public static final String GLOBAL_LIBRARIES_DIR = "global-libraries-cache";
+    public static final String GLOBAL_LIBRARIES_DIR = "global-libraries-jar-cache";
     public static final String LAST_READ_FILE = "last_read";
 
     @DataBoundConstructor public LibraryCachingConfiguration(int refreshTimeMinutes, String excludedVersionsStr) {

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

@@ -71,7 +71,7 @@ public String getName() {
     }
 
     /**
-     * Returns a partially unique name that can be safely used as a directory name.
+     * Returns a partially unique name that can be safely used as a directory or JAR base name.
      *
      * Uniqueness is based on the library name, version, whether it is trusted, and the source of the library.
      * {@link LibraryRetriever}-specific information such as the SCM is not used to produce this name.

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

@@ -35,12 +35,68 @@
 import hudson.util.FormValidation;
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import edu.umd.cs.findbugs.annotations.NonNull;
+import hudson.util.io.ArchiverFactory;
+import java.io.IOException;
+import java.io.OutputStream;
 
 /**
  * A way in which a library can be physically obtained for use in a build.
  */
 public abstract class LibraryRetriever extends AbstractDescribableImpl<LibraryRetriever> implements ExtensionPoint {
 
+    /**
+     * Obtains library sources.
+     * @param name the {@link LibraryConfiguration#getName}
+     * @param version the version of the library, such as from {@link LibraryConfiguration#getDefaultVersion} or an override
+     * @param changelog whether to include changesets in the library in jobs using it from {@link LibraryConfiguration#getIncludeInChangesets}
+     * @param target a JAR file in which to stash sources; should contain {@code **}{@code /*.groovy} (sources at top level will be considered vars), and optionally also {@code resources/}
+     * @param run a build which will use the library
+     * @param listener a way to report progress
+     * @throws Exception if there is any problem (use {@link AbortException} for user errors)
+     */
+    public void retrieveJar(@NonNull String name, @NonNull String version, boolean changelog, @NonNull FilePath target, @NonNull Run<?,?> run, @NonNull TaskListener listener) throws Exception {
+        if (Util.isOverridden(LibraryRetriever.class, getClass(), "retrieve", String.class, String.class, boolean.class, FilePath.class, Run.class, TaskListener.class)) {
+            FilePath tmp = target.withSuffix(".checkout");
+            try {
+                retrieve(name, version, changelog, tmp, run, listener);
+                dir2Jar(tmp, target);
+            } finally {
+                tmp.deleteRecursive();
+            }
+        } else {
+            throw new AbstractMethodError("Implement retrieveJar");
+        }
+    }
+
+    /**
+     * Translates a historical directory with {@code src/} and/or {@code vars/} and/or {@code resources/} subdirectories
+     * into a JAR file with Groovy in classpath orientation and {@code resources/} as a ZIP folder.
+     */
+    static void dir2Jar(@NonNull FilePath dir, @NonNull FilePath jar) throws IOException, InterruptedException {
+        // TODO do this more efficiently by packing JAR directly
+        FilePath tmp2 = jar.withSuffix(".tmp2");
+        tmp2.mkdirs();
+        try {
+            FilePath src = dir.child("src");
+            if (src.isDirectory()) {
+                src.moveAllChildrenTo(tmp2);
+            }
+            FilePath vars = dir.child("vars");
+            if (vars.isDirectory()) {
+                vars.moveAllChildrenTo(tmp2);
+            }
+            FilePath resources = dir.child("resources");
+            if (resources.isDirectory()) {
+                resources.renameTo(tmp2.child("resources"));
+            }
+            try (OutputStream os = jar.write()) {
+                tmp2.archive(ArchiverFactory.ZIP, os, "**");
+            }
+        } finally {
+            tmp2.deleteRecursive();
+        }
+    }
+
     /**
      * Obtains library sources.
      * @param name the {@link LibraryConfiguration#getName}
@@ -51,7 +107,14 @@ public abstract class LibraryRetriever extends AbstractDescribableImpl<LibraryRe
      * @param listener a way to report progress
      * @throws Exception if there is any problem (use {@link AbortException} for user errors)
      */
-    public abstract void retrieve(@NonNull String name, @NonNull String version, boolean changelog, @NonNull FilePath target, @NonNull Run<?,?> run, @NonNull TaskListener listener) throws Exception;
+    @Deprecated
+    public void retrieve(@NonNull String name, @NonNull String version, boolean changelog, @NonNull FilePath target, @NonNull Run<?,?> run, @NonNull TaskListener listener) throws Exception {
+        if (Util.isOverridden(LibraryRetriever.class, getClass(), "retrieve", String.class, String.class, FilePath.class, Run.class, TaskListener.class)) {
+            retrieve(name, version, target, run, listener);
+        } else {
+            throw new AbstractMethodError("Implement retrieveJar");
+        }
+    }
 
     /**
      * Obtains library sources.
@@ -62,8 +125,10 @@ public abstract class LibraryRetriever extends AbstractDescribableImpl<LibraryRe
      * @param listener a way to report progress
      * @throws Exception if there is any problem (use {@link AbortException} for user errors)
      */
-    // TODO this should have been made nonabstract and deprecated and delegated to the new version; may be able to use access-modifier to help
-    public abstract void retrieve(@NonNull String name, @NonNull String version, @NonNull FilePath target, @NonNull Run<?,?> run, @NonNull TaskListener listener) throws Exception;
+    @Deprecated
+    public void retrieve(@NonNull String name, @NonNull String version, @NonNull FilePath target, @NonNull Run<?,?> run, @NonNull TaskListener listener) throws Exception {
+        throw new AbstractMethodError("Implement retrieveJar");
+    }
 
     @Deprecated
     public FormValidation validateVersion(@NonNull String name, @NonNull String version) {