[GR-65623] Extend java.lang.module.ModuleReader#list with the ability to follow symlinks for directory modulepath entries during digest computation.

Author: Tudor-Stefan Magirescu

compiler/ci/ci_common/benchmark-suites.libsonnet

@@ -91,15 +91,15 @@
 
   barista_template(suite_version=null, suite_name="barista", max_jdk_version=null, cmd_app_prefix=["hwloc-bind --cpubind node:0.core:0-3.pu:0 --membind node:0"], non_prefix_barista_args=[]):: cc.compiler_benchmark + {
     suite:: suite_name,
-    local barista_version = "v0.4.7",
+    local barista_version = "v0.4.8",
     local suite_version_args = if suite_version != null then ["--bench-suite-version=" + suite_version] else [],
     local prefix_barista_arg = if std.length(cmd_app_prefix) > 0 then [std.format("--cmd-app-prefix=%s", std.join(" ", cmd_app_prefix))] else [],
     local all_barista_args = prefix_barista_arg + non_prefix_barista_args,
     local barista_args_with_separator = if std.length(all_barista_args) > 0 then ["--"] + all_barista_args else [],
     downloads+: {
       "WRK": { "name": "wrk", "version": "a211dd5", platformspecific: true},
       "WRK2": { "name": "wrk2", "version": "2.1", platformspecific: true},
-      "BARISTA_BENCHMARKS": { "name": "barista", "version": "0.4.7"}
+      "BARISTA_BENCHMARKS": { "name": "barista", "version": "0.4.8"}
     },
     packages+: {
       maven: "==3.8.6",

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/imagelayer/NativeImageLayers.md

@@ -115,6 +115,8 @@ Args = --layer-create=@layer-create.args
 The `layer-create.args` file-path is relative to the directory that contains the `native-image.properties` file and might look like this:
 ```
 base-layer.nil
+# ignore this classpath/modulepath entry during layer compatibility checks
+digest-ignore
 module=java.base
 # micronaut and dependencies
 package=io.micronaut.*
@@ -129,6 +131,13 @@ package=org.reactivestreams.*
 Each line corresponds to one entry in the list of comma-separated entries that can usually be found in a regular `--layer-create` argument.
 Lines starting with `#` are ignored and can therefore be used to provide comments in such an option argument file.
 
+Note the use of the `digest-ignore` suboption in the `layer-create.args` example file above.
+This suboption _only takes effect_ when the `--layer-create` option is specified _within a file_, and is ignored if provided via the command line.
+When defining `--layer-create` in a file, always include this suboption (see [compatibility rules](#compatibility-rules)).
+
+Avoid placing other class or resource files in the same classpath/modulepath entry as the file(s) defining `--layer-create`.
+These files would be excluded from compatibility checks, potentially leading to subtle, difficult-to-debug issues.
+
 ### Option `--layer-use` consumes a shared layer, and can extend it or create a final executable:
 
 ```
@@ -195,16 +204,31 @@ native-image --module-path target/AwesomeLib-1.0-SNAPSHOT.jar --shared
 
 ### Compatibility rules
 
-Currently, layer build compatibility checking is very limited and is only performed for the image builder arguments.
+Layer build compatibility checks are performed to ensure the consistency of _image builder arguments_ and _classpath/modulepath entries_.
+These will be covered in the following subsections.
+
+#### Image builder arguments compatibility
+
 The list below gives a few examples of checks that are already implemented.
 
 - Module system options `--add-exports`, `--add-opens`, `--add-reads` that were passed in the previous image build also need to be passed in the current image build.
   Note that additional module system options not found in the previous image build are allowed to be used in the current image build.
 - Builder options of the form `-H:NeverInline=<pattern>` follow the same logic as the module system options above.
 - If debug option `-g` was passed in the previous image build it must also be passed in the current image build at the same position.
 - Other options like `-H:EntryPointNamePrefix=...`, `-H:APIFunctionPrefix=...`, ... follow the same logic as the `-g` option.
+- Environment variables provided using `-E` to the previous image build must be passed to the current image build, with their values remaining unchanged. Additional environment variables may be supplied to the current build if needed.
+
+#### Classpath & modulepath compatibility
+
+The classpath/modulepath entries used in the previous image layer build must also be included in the current image layer build.
+These entries must retain the same content. If any of the shared entries are modified (.e.g, updated jar files), the previous image layer must be rebuilt with the updated versions.
+
+For example, assume the previous layer build used the classpath: `/path/to/foo.jar:/path/to/bar.jar`.
+Then the current layer build must include both `/path/to/foo.jar` and `/path/to/bar.jar`, possibly alongside additional entries: `path/to/foo.jar:/path/to/bar.jar:/path/to/extra.jar`.
+Additionally, the contents of `foo.jar` and `bar.jar` should remain unchanged between the two builds.
 
-The number of checks is subject to change and will be further improved in the future.
+**Exception**: Classpath or modulepath entries that include a `native-image.properties` file specifying the `--layer-create` option with the `digest-ignore` suboption are exempt from this rule.
+These entries should be _excluded_ from subsequent layer builds.
 
 ### Limitations
 

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageClassLoaderSupport.java

@@ -74,13 +74,15 @@
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.LongAdder;
 import java.util.function.BiConsumer;
+import java.util.function.Predicate;
 import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
 import java.util.stream.Collector;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import java.util.zip.ZipFile;
 
+import jdk.internal.module.Resources;
 import org.graalvm.collections.EconomicMap;
 import org.graalvm.collections.EconomicSet;
 import org.graalvm.collections.MapCursor;
@@ -126,7 +128,9 @@ public final class NativeImageClassLoaderSupport {
     private final List<Path> buildmp;
 
     private final Set<Path> imageProvidedJars;
+    /** Cleared by {@link #computePathEntryDigests()} on first call. */
     private PathDigests pathDigests;
+    private final Class<?> explodedModuleReaderClass;
 
     private final EconomicMap<URI, EconomicSet<String>> classes;
     private final EconomicMap<URI, EconomicSet<String>> packages;
@@ -289,6 +293,8 @@ protected NativeImageClassLoaderSupport(ClassLoader defaultSystemClassLoader, St
         annotationExtractor = new SubstrateAnnotationExtractor();
 
         includeConfigSealed = false;
+
+        explodedModuleReaderClass = ReflectionUtil.lookupClass(false, "jdk.internal.module.ModuleReferences$ExplodedModuleReader");
     }
 
     private static Stream<Path> toRealPath(Path p) {
@@ -311,11 +317,14 @@ public NativeImageClassLoader getClassLoader() {
         return classLoader;
     }
 
-    public Optional<PathDigests> getPathDigests(boolean free) {
-        Optional<PathDigests> res = Optional.ofNullable(pathDigests);
-        if (free) {
-            pathDigests = null;
-        }
+    /**
+     * The temporary {@link PathDigestEntry} object is used to create the list of
+     * {@link PathDigestEntry} tuples. Note that the {@code pathDigests} field is cleared after the
+     * first time this method is called.
+     */
+    public List<PathDigestEntry> computePathEntryDigests() {
+        List<PathDigestEntry> res = PathDigestEntry.aggregatePathDigests(pathDigests);
+        pathDigests = null;
         return res;
     }
 
@@ -986,23 +995,49 @@ private void initModule(ModuleReference moduleReference, boolean moduleRequiresI
                 }
                 final boolean isInImageModulePathOfLayeredBuild = pathDigests != null && pathDigests.mpDigests.containsKey(container);
                 final boolean isJar = ClasspathUtils.isJar(Path.of(container));
-                moduleReader.list().forEach(moduleResource -> {
-                    char fileSystemSeparatorChar = '/';
-                    String className = extractClassName(moduleResource, fileSystemSeparatorChar);
-                    if (className != null) {
-                        currentlyProcessedEntry = moduleReferenceLocation + fileSystemSeparatorChar + moduleResource;
-                        executor.execute(() -> handleClassFileName(container, module, className, includeUnconditionally, moduleRequiresInit, preserveModule));
-                    }
-                    if (isInImageModulePathOfLayeredBuild) {
-                        executor.execute(() -> PathDigests.storePathFileDigest(container, moduleResource, isJar, pathDigests.mpDigests));
-                    }
-                    entriesProcessed.increment();
-                });
+                try (Stream<String> moduleResources = moduleReaderListFollowSymlinks(moduleReader, container)) {
+                    moduleResources.forEach(moduleResource -> {
+                        char fileSystemSeparatorChar = '/';
+                        String className = extractClassName(moduleResource, fileSystemSeparatorChar);
+                        if (className != null) {
+                            currentlyProcessedEntry = moduleReferenceLocation + fileSystemSeparatorChar + moduleResource;
+                            executor.execute(() -> handleClassFileName(container, module, className, includeUnconditionally, moduleRequiresInit, preserveModule));
+                        }
+                        if (isInImageModulePathOfLayeredBuild) {
+                            executor.execute(() -> PathDigests.storePathFileDigest(container, moduleResource, isJar, pathDigests.mpDigests));
+                        }
+                        entriesProcessed.increment();
+                    });
+                }
             } catch (IOException e) {
                 throw new RuntimeException("Unable get list of resources in module" + moduleReference.descriptor().name(), e);
             }
         }
 
+        /**
+         * This method extends {@link ModuleReader#list} with the ability to follow symlinks when
+         * the given {@link ModuleReader} corresponds to a
+         * {@code jdk.internal.module.ModuleReferences$ExplodedModuleReader} (i.e.,
+         * {@code container} is a directory which contains the contents of the module).
+         *
+         * Classloaders (e.g., {@link NativeImageClassLoader#findResource(String, String)}) can load
+         * resources pointed to by a symlink present on the modulepath, meaning the extension below
+         * is necessary if we want to keep track of such resources (e.g., for digest computation in
+         * layered builds).
+         */
+        @SuppressWarnings("resource")
+        private Stream<String> moduleReaderListFollowSymlinks(ModuleReader reader, URI container) throws IOException {
+            if (explodedModuleReaderClass == null || !reader.getClass().isAssignableFrom(explodedModuleReaderClass)) {
+                return reader.list();
+            }
+
+            Path dir = Path.of(container);
+            // See {@code jdk.internal.module.ModuleReferences$ExplodedModuleReader#list}.
+            return Files.walk(dir, Integer.MAX_VALUE, FileVisitOption.FOLLOW_LINKS)
+                            .map(f -> Resources.toResourceName(dir, f))
+                            .filter(Predicate.not(String::isEmpty));
+        }
+
         private void loadClassesFromPath(Path path) {
             final boolean includeUnconditionally = layerSelectors.classpathEntries().contains(path);
             final boolean includeAllMetadata = preserveSelectors.classpathEntries().contains(path);
@@ -1434,7 +1469,16 @@ public Set<IncludeOptionsSupport.PackageOptionValue> packages() {
         }
     }
 
-    public static final class PathDigests {
+    /**
+     * Stores a temporary collection of individual class/resource file digests that is updated
+     * during class loading. In particular, {@code PathDigests} objects store and update two
+     * {@link EconomicMap}s, one for classpath entries and the other for modulepath entries. Each
+     * {@link EconomicMap} maps a path entry (i.e., directory/jar) with the list of individual
+     * digests for the files it contains. The order of the individual digests is non-deterministic.
+     * After class loading, the {@code PathDigests} should be aggregated into a list of
+     * {@link PathDigestEntry}.
+     */
+    private static final class PathDigests {
         private final EconomicMap<URI, List<String>> cpDigests = EconomicMap.create();
         private final EconomicMap<URI, List<String>> mpDigests = EconomicMap.create();
 
@@ -1475,12 +1519,73 @@ private static void storePathFileDigest(URI container, String resource, boolean
             }
         }
 
-        public EconomicMap<URI, List<String>> getCpDigests() {
+        private EconomicMap<URI, List<String>> getCpDigests() {
             return cpDigests;
         }
 
-        public EconomicMap<URI, List<String>> getMpDigests() {
+        private EconomicMap<URI, List<String>> getMpDigests() {
             return mpDigests;
         }
     }
+
+    /**
+     * The record type {@link PathDigestEntry} encodes tuples of the form (type, digest, path),
+     * where: - type: is either {@code PathType.cp} or {@code PathType.mp} - digest: is a checksum
+     * which encodes information about every class/resource file part of that path (or reachable
+     * from a symlink part of that path) - path: is the absolute path ponting to the directory/jar
+     * included on the class/module-path. To obtain a list of all {@code PathDigestEntry} objects
+     * corresponding to a particular Native Image build, use the method
+     * {@link NativeImageClassLoaderSupport#computePathEntryDigests()}.
+     */
+    public record PathDigestEntry(PathType type, String digest, String path) {
+
+        /**
+         * Aggregate the contents of {@link PathDigests} to create a list of
+         * {@link PathDigestEntry}. The individual file digests of a path entry are aggregated into
+         * a final digest that encodes information about all the contents of that path entry.
+         */
+        private static List<PathDigestEntry> aggregatePathDigests(PathDigests pathDigests) {
+            Objects.requireNonNull(pathDigests, "NativeImageClassLoaderSupport#pathDigests should not be empty for a layered build.");
+
+            List<PathDigestEntry> aggregatedDigests = new ArrayList<>();
+            aggregatedDigests.addAll(aggregatePathDigests(pathDigests.getCpDigests(), PathType.cp));
+            aggregatedDigests.addAll(aggregatePathDigests(pathDigests.getMpDigests(), PathType.mp));
+            return aggregatedDigests;
+        }
+
+        private static List<PathDigestEntry> aggregatePathDigests(EconomicMap<URI, List<String>> pathDigests, PathType type) {
+            List<PathDigestEntry> aggregatedDigests = new ArrayList<>();
+            var cursor = pathDigests.getEntries();
+            while (cursor.advance()) {
+                aggregatedDigests.add(PathDigestEntry.of(type, cursor.getKey().getPath(), cursor.getValue()));
+            }
+            return aggregatedDigests;
+        }
+
+        private static PathDigestEntry of(PathType type, String path, List<String> digests) {
+            DigestBuilder db = new DigestBuilder();
+            digests.stream()
+                            .sorted()
+                            .map(d -> d.getBytes(StandardCharsets.UTF_8))
+                            .forEach(db::update);
+
+            String aggregatedDigest = new String(db.digest(), StandardCharsets.UTF_8);
+            return new PathDigestEntry(type, aggregatedDigest, path);
+        }
+
+        public static PathDigestEntry of(String digestEntry) {
+            String[] envVarArr = digestEntry.split(":", 3);
+            return new PathDigestEntry(PathType.valueOf(envVarArr[0]), envVarArr[1], envVarArr[2]);
+        }
+
+        @Override
+        public String toString() {
+            return type + ":" + digest + ":" + path;
+        }
+
+        public enum PathType {
+            cp,
+            mp
+        }
+    }
 }

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/HostedImageLayerBuildingSupport.java

@@ -100,6 +100,15 @@ private static String layerCreatePossibleOptions() {
      * {@link #initialize}.
      */
     private final Function<Class<?>, SingletonTrait[]> singletonTraitInjector;
+    /**
+     * Optional suboption of the {@link SubstrateOptions#LayerCreate} option. If the `LayerCreate`
+     * option is specified inside a `native-image.properties` file and this suboption is enabled,
+     * the classpath/modulepath entry containing the `native-image.properties` file will be excluded
+     * from the classpath/modulepath layered compatibility check. This suboption has no effect if
+     * it's specified from the command line. See
+     * {@link #processLayerOptions(EconomicMap, NativeImageClassLoaderSupport)} for more details.
+     */
+    private static final String DIGEST_IGNORE = "digest-ignore";
 
     private HostedImageLayerBuildingSupport(ImageClassLoader imageClassLoader,
                     Reader snapshot, List<FileChannel> graphsChannels,
@@ -206,7 +215,7 @@ public static void processLayerOptions(EconomicMap<OptionKey<?>, Object> values,
             classLoaderSupport.setLayerFile(layerFile);
 
             NativeImageClassLoaderSupport.IncludeSelectors layerSelectors = classLoaderSupport.getLayerSelectors();
-            IncludeOptionsSupport.ExtendedOption digestIgnoreExtendedOption = new IncludeOptionsSupport.ExtendedOption("digest-ignore", null);
+            IncludeOptionsSupport.ExtendedOption digestIgnoreExtendedOption = new IncludeOptionsSupport.ExtendedOption(DIGEST_IGNORE, null);
             for (IncludeOptionsSupport.ExtendedOption option : layerOption.extendedOptions()) {
                 if (option.equals(digestIgnoreExtendedOption)) {
                     digestIgnorePath = valueWithOrigin.origin().location();
@@ -295,7 +304,7 @@ private static String getLayerCreateValue(ValueWithOrigin<String> valueWithOrigi
         return String.join(",", OptionUtils.resolveOptionValuesRedirection(SubstrateOptions.LayerCreate, valueWithOrigin));
     }
 
-    public static boolean isLayerUseOptionEnabled(OptionValues values) {
+    private static boolean isLayerUseOptionEnabled(OptionValues values) {
         if (SubstrateOptions.LayerUse.hasBeenSet(values)) {
             return !getLayerUseValue(values).toString().isEmpty();
         }