From cf6209484d859e79b98c2cecdac1c5beb8f6a398 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Champeau?= Date: Wed, 26 Nov 2025 14:44:19 +0100 Subject: [PATCH 1/2] Use a ClassLoader for locating resources Before this change, the VirtualFileSystem was using a `Class` as a beacon to find resources. I suppose that the intent was to use `Class#getResource`, but in practice, it's always `clazz.getClassLoader().getResource()` which is used, which makes it redundant to use a class. This commit refactors the code to allow declaring which _classloader_ to use for loading. This is important because in some cases, we don't necessarily have a `Class` to pass, but we _do_ have a classloader. This should also allow more advanced scenarios with filtering classloaders for example. Since the `Class` was actually never directly used, but only its classloader, this change should have 0 impact for backwards compatibility. --- .../python/embedding/VirtualFileSystem.java | 32 ++++++++++---- .../embedding/VirtualFileSystemImpl.java | 42 ++++++++++--------- 2 files changed, 46 insertions(+), 28 deletions(-) diff --git a/org.graalvm.python.embedding/src/main/java/org/graalvm/python/embedding/VirtualFileSystem.java b/org.graalvm.python.embedding/src/main/java/org/graalvm/python/embedding/VirtualFileSystem.java index f74ab3f..0c0f2fb 100644 --- a/org.graalvm.python.embedding/src/main/java/org/graalvm/python/embedding/VirtualFileSystem.java +++ b/org.graalvm.python.embedding/src/main/java/org/graalvm/python/embedding/VirtualFileSystem.java @@ -107,7 +107,8 @@ public static final class Builder { private HostIO allowHostIO = HostIO.READ_WRITE; private boolean caseInsensitive = VirtualFileSystemImpl.isWindows(); - private Class resourceLoadingClass; + private ClassLoader resourceClassLoader; + private String resourceDirectory; private Builder() { @@ -238,21 +239,36 @@ public Builder unixMountPoint(String unixMountPoint) { /** * By default, virtual filesystem resources are loaded by delegating to - * VirtualFileSystem.class.getResource(name). Use + * VirtualFileSystem.class.getClassLoader().getResource(name). Use * resourceLoadingClass to determine where to locate resources in * cases when for example VirtualFileSystem is on module path and * the jar containing the resources is on class path. * - * @param c - * the class for loading the resources + * @param c the class for loading the resources * @return the builder * @since 24.2.0 */ public Builder resourceLoadingClass(Class c) { - resourceLoadingClass = c; + resourceClassLoader = c.getClassLoader(); return this; } + /** + * By default, virtual filesystem resources are loaded by delegating to + * VirtualFileSystem.class.getClassLoader().getResource(name). Use + * resourceClassLoader to determine where to locate resources in + * cases when for example VirtualFileSystem is on module path and + * the jar containing the resources is on class path. + * + * @param cl the classloader used to load resources + * @return this builder + * @since 26.0.0 + */ + public Builder resourceClassLoader(ClassLoader cl) { + resourceClassLoader = cl; + return this; + } + /** * This filter applied to files in the virtual filesystem treats them as * symlinks to real files in the host filesystem. This is useful, for example, @@ -292,7 +308,7 @@ public VirtualFileSystem build() { ? Path.of(DEFAULT_WINDOWS_MOUNT_POINT) : Path.of(DEFAULT_UNIX_MOUNT_POINT); } - return new VirtualFileSystem(extractFilter, mountPoint, allowHostIO, resourceLoadingClass, + return new VirtualFileSystem(extractFilter, mountPoint, allowHostIO, resourceClassLoader, resourceDirectory, caseInsensitive); } } @@ -308,10 +324,10 @@ private static Path getMountPointAsPath(String mp) { } private VirtualFileSystem(Predicate extractFilter, Path mountPoint, HostIO allowHostIO, - Class resourceLoadingClass, String resourceDirectory, boolean caseInsensitive) { + ClassLoader resourceClassLoader, String resourceDirectory, boolean caseInsensitive) { this.impl = new VirtualFileSystemImpl(extractFilter, mountPoint, resourceDirectory, allowHostIO, - resourceLoadingClass, caseInsensitive); + resourceClassLoader, caseInsensitive); this.delegatingFileSystem = VirtualFileSystemImpl.createDelegatingFileSystem(impl); } diff --git a/org.graalvm.python.embedding/src/main/java/org/graalvm/python/embedding/VirtualFileSystemImpl.java b/org.graalvm.python.embedding/src/main/java/org/graalvm/python/embedding/VirtualFileSystemImpl.java index 1bfa25c..5c6359a 100644 --- a/org.graalvm.python.embedding/src/main/java/org/graalvm/python/embedding/VirtualFileSystemImpl.java +++ b/org.graalvm.python.embedding/src/main/java/org/graalvm/python/embedding/VirtualFileSystemImpl.java @@ -169,12 +169,12 @@ private static String absoluteResourcePath(String... components) { * Maps platform-specific paths to entries. */ private final Map vfsEntries = new HashMap<>(); - - /** - * Class used to read resources with getResource(name). By default - * VirtualFileSystem.class. - */ - private Class resourceLoadingClass; + + /** + * Classloader used to read resources. By defaut, the classloader of + * VirtualFileSystem.class. + */ + private final ClassLoader resourceClassLoader; static final String PLATFORM_SEPARATOR = Paths.get("").getFileSystem().getSeparator(); private static final char RESOURCE_SEPARATOR_CHAR = '/'; @@ -310,12 +310,12 @@ private void removeExtractDir() { * argument may be {@code null} causing that no extraction will happen. */ VirtualFileSystemImpl(Predicate extractFilter, Path mountPoint, String resourceDirectory, HostIO allowHostIO, - Class resourceLoadingClass, boolean caseInsensitive) { - if (resourceLoadingClass != null) { - this.resourceLoadingClass = resourceLoadingClass; - } else { - this.resourceLoadingClass = VirtualFileSystem.class; - } + ClassLoader resourceClassLoader, boolean caseInsensitive) { + if (resourceClassLoader != null) { + this.resourceClassLoader = resourceClassLoader; + } else { + this.resourceClassLoader = VirtualFileSystem.class.getClassLoader(); + } this.caseInsensitive = caseInsensitive; this.mountPoint = mountPoint; this.mountPointLowerCase = mountPoint.toString().toLowerCase(Locale.ROOT); @@ -323,10 +323,12 @@ private void removeExtractDir() { this.platformVenvPath = resourcePathToPlatformPath(absoluteResourcePath(vfsRoot, VFS_VENV)); this.platformSrcPath = resourcePathToPlatformPath(absoluteResourcePath(vfsRoot, VFS_SRC)); - fine("VirtualFilesystem %s, allowHostIO: %s, resourceLoadingClass: %s, caseInsensitive: %s, extractOnStartup: %s%s", - mountPoint, allowHostIO.toString(), this.resourceLoadingClass.getName(), caseInsensitive, - extractOnStartup, extractFilter != null ? "" : ", extractFilter: null"); - + if (LOGGER.isLoggable(Level.FINE)) { + var classLoaderLabel = this.resourceClassLoader == VirtualFileSystem.class.getClassLoader() ? "VirtualFileSystem" : "custom"; + fine("VirtualFilesystem %s, allowHostIO: %s, resourceClassLoader: %s, caseInsensitive: %s, extractOnStartup: %s%s", + mountPoint, allowHostIO.toString(), classLoaderLabel, caseInsensitive, + extractOnStartup, extractFilter != null ? "" : ", extractFilter: null"); + } this.extractFilter = extractFilter; if (extractFilter != null) { try { @@ -670,7 +672,7 @@ public boolean equals(Object obj) { private List getFilelistURLs(String filelistPath) { List filelistUrls; try { - filelistUrls = Collections.list(this.resourceLoadingClass.getClassLoader().getResources(filelistPath)); + filelistUrls = Collections.list(resourceClassLoader.getResources(filelistPath)); } catch (IOException e) { throw new IllegalStateException("IO error during reading the VirtualFileSystem metadata", e); } @@ -746,7 +748,7 @@ private void validateMultipleVFSLocations(List filelistUrls) { ArrayList installedUrls; try { installedUrls = Collections.list( - this.resourceLoadingClass.getClassLoader().getResources(resourcePath(vfsRoot, INSTALLED_FILE))); + resourceClassLoader.getResources(resourcePath(vfsRoot, INSTALLED_FILE))); } catch (IOException e) { warn("Cannot check compatibility of the merged virtual environments. Cannot read list of packages installed in the virtual environments. IOException: " + e.getMessage()); @@ -791,7 +793,7 @@ private void validateMultipleVFSLocations(List filelistUrls) { ArrayList contentsUrls; try { contentsUrls = Collections.list( - this.resourceLoadingClass.getClassLoader().getResources(resourcePath(vfsRoot, CONTENTS_FILE))); + resourceClassLoader.getResources(resourcePath(vfsRoot, CONTENTS_FILE))); } catch (IOException e) { warn("Cannot check compatibility of the merged virtual environments. Cannot read GraalPy version of the virtual environments. IOException: " + e.getMessage()); @@ -831,7 +833,7 @@ private void validateMultipleVFSLocations(List filelistUrls) { } private URL getResourceUrl(String path) throws IOException { - List urls = Collections.list(this.resourceLoadingClass.getClassLoader().getResources(path.substring(1))); + List urls = Collections.list(resourceClassLoader.getResources(path.substring(1))); if (vfsRootURL != null) { urls = getURLInRoot(urls); } From 7a8a7bb2e3a5687e732a7e4cf5afce1996940496 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Champeau?= Date: Wed, 26 Nov 2025 16:39:58 +0100 Subject: [PATCH 2/2] Make spotless happy --- .../python/embedding/VirtualFileSystem.java | 42 ++++++++++--------- .../embedding/VirtualFileSystemImpl.java | 42 +++++++++---------- 2 files changed, 43 insertions(+), 41 deletions(-) diff --git a/org.graalvm.python.embedding/src/main/java/org/graalvm/python/embedding/VirtualFileSystem.java b/org.graalvm.python.embedding/src/main/java/org/graalvm/python/embedding/VirtualFileSystem.java index 0c0f2fb..b5c4392 100644 --- a/org.graalvm.python.embedding/src/main/java/org/graalvm/python/embedding/VirtualFileSystem.java +++ b/org.graalvm.python.embedding/src/main/java/org/graalvm/python/embedding/VirtualFileSystem.java @@ -107,7 +107,7 @@ public static final class Builder { private HostIO allowHostIO = HostIO.READ_WRITE; private boolean caseInsensitive = VirtualFileSystemImpl.isWindows(); - private ClassLoader resourceClassLoader; + private ClassLoader resourceClassLoader; private String resourceDirectory; @@ -244,7 +244,8 @@ public Builder unixMountPoint(String unixMountPoint) { * cases when for example VirtualFileSystem is on module path and * the jar containing the resources is on class path. * - * @param c the class for loading the resources + * @param c + * the class for loading the resources * @return the builder * @since 24.2.0 */ @@ -253,21 +254,22 @@ public Builder resourceLoadingClass(Class c) { return this; } - /** - * By default, virtual filesystem resources are loaded by delegating to - * VirtualFileSystem.class.getClassLoader().getResource(name). Use - * resourceClassLoader to determine where to locate resources in - * cases when for example VirtualFileSystem is on module path and - * the jar containing the resources is on class path. - * - * @param cl the classloader used to load resources - * @return this builder - * @since 26.0.0 - */ - public Builder resourceClassLoader(ClassLoader cl) { - resourceClassLoader = cl; - return this; - } + /** + * By default, virtual filesystem resources are loaded by delegating to + * VirtualFileSystem.class.getClassLoader().getResource(name). Use + * resourceClassLoader to determine where to locate resources in + * cases when for example VirtualFileSystem is on module path and + * the jar containing the resources is on class path. + * + * @param cl + * the classloader used to load resources + * @return this builder + * @since 26.0.0 + */ + public Builder resourceClassLoader(ClassLoader cl) { + resourceClassLoader = cl; + return this; + } /** * This filter applied to files in the virtual filesystem treats them as @@ -308,8 +310,8 @@ public VirtualFileSystem build() { ? Path.of(DEFAULT_WINDOWS_MOUNT_POINT) : Path.of(DEFAULT_UNIX_MOUNT_POINT); } - return new VirtualFileSystem(extractFilter, mountPoint, allowHostIO, resourceClassLoader, - resourceDirectory, caseInsensitive); + return new VirtualFileSystem(extractFilter, mountPoint, allowHostIO, resourceClassLoader, resourceDirectory, + caseInsensitive); } } @@ -327,7 +329,7 @@ private VirtualFileSystem(Predicate extractFilter, Path mountPoint, HostIO ClassLoader resourceClassLoader, String resourceDirectory, boolean caseInsensitive) { this.impl = new VirtualFileSystemImpl(extractFilter, mountPoint, resourceDirectory, allowHostIO, - resourceClassLoader, caseInsensitive); + resourceClassLoader, caseInsensitive); this.delegatingFileSystem = VirtualFileSystemImpl.createDelegatingFileSystem(impl); } diff --git a/org.graalvm.python.embedding/src/main/java/org/graalvm/python/embedding/VirtualFileSystemImpl.java b/org.graalvm.python.embedding/src/main/java/org/graalvm/python/embedding/VirtualFileSystemImpl.java index 5c6359a..8f5a8f6 100644 --- a/org.graalvm.python.embedding/src/main/java/org/graalvm/python/embedding/VirtualFileSystemImpl.java +++ b/org.graalvm.python.embedding/src/main/java/org/graalvm/python/embedding/VirtualFileSystemImpl.java @@ -169,12 +169,12 @@ private static String absoluteResourcePath(String... components) { * Maps platform-specific paths to entries. */ private final Map vfsEntries = new HashMap<>(); - - /** - * Classloader used to read resources. By defaut, the classloader of - * VirtualFileSystem.class. - */ - private final ClassLoader resourceClassLoader; + + /** + * Classloader used to read resources. By defaut, the classloader of + * VirtualFileSystem.class. + */ + private final ClassLoader resourceClassLoader; static final String PLATFORM_SEPARATOR = Paths.get("").getFileSystem().getSeparator(); private static final char RESOURCE_SEPARATOR_CHAR = '/'; @@ -311,11 +311,11 @@ private void removeExtractDir() { */ VirtualFileSystemImpl(Predicate extractFilter, Path mountPoint, String resourceDirectory, HostIO allowHostIO, ClassLoader resourceClassLoader, boolean caseInsensitive) { - if (resourceClassLoader != null) { - this.resourceClassLoader = resourceClassLoader; - } else { - this.resourceClassLoader = VirtualFileSystem.class.getClassLoader(); - } + if (resourceClassLoader != null) { + this.resourceClassLoader = resourceClassLoader; + } else { + this.resourceClassLoader = VirtualFileSystem.class.getClassLoader(); + } this.caseInsensitive = caseInsensitive; this.mountPoint = mountPoint; this.mountPointLowerCase = mountPoint.toString().toLowerCase(Locale.ROOT); @@ -323,12 +323,14 @@ private void removeExtractDir() { this.platformVenvPath = resourcePathToPlatformPath(absoluteResourcePath(vfsRoot, VFS_VENV)); this.platformSrcPath = resourcePathToPlatformPath(absoluteResourcePath(vfsRoot, VFS_SRC)); - if (LOGGER.isLoggable(Level.FINE)) { - var classLoaderLabel = this.resourceClassLoader == VirtualFileSystem.class.getClassLoader() ? "VirtualFileSystem" : "custom"; - fine("VirtualFilesystem %s, allowHostIO: %s, resourceClassLoader: %s, caseInsensitive: %s, extractOnStartup: %s%s", - mountPoint, allowHostIO.toString(), classLoaderLabel, caseInsensitive, - extractOnStartup, extractFilter != null ? "" : ", extractFilter: null"); - } + if (LOGGER.isLoggable(Level.FINE)) { + var classLoaderLabel = this.resourceClassLoader == VirtualFileSystem.class.getClassLoader() + ? "VirtualFileSystem" + : "custom"; + fine("VirtualFilesystem %s, allowHostIO: %s, resourceClassLoader: %s, caseInsensitive: %s, extractOnStartup: %s%s", + mountPoint, allowHostIO.toString(), classLoaderLabel, caseInsensitive, extractOnStartup, + extractFilter != null ? "" : ", extractFilter: null"); + } this.extractFilter = extractFilter; if (extractFilter != null) { try { @@ -747,8 +749,7 @@ private void validateMultipleVFSLocations(List filelistUrls) { // by the Maven/Gradle plugin and should contain "pip freeze" of the venv ArrayList installedUrls; try { - installedUrls = Collections.list( - resourceClassLoader.getResources(resourcePath(vfsRoot, INSTALLED_FILE))); + installedUrls = Collections.list(resourceClassLoader.getResources(resourcePath(vfsRoot, INSTALLED_FILE))); } catch (IOException e) { warn("Cannot check compatibility of the merged virtual environments. Cannot read list of packages installed in the virtual environments. IOException: " + e.getMessage()); @@ -792,8 +793,7 @@ private void validateMultipleVFSLocations(List filelistUrls) { // Check compatibility of GraalPy versions that were used to create the VFSs ArrayList contentsUrls; try { - contentsUrls = Collections.list( - resourceClassLoader.getResources(resourcePath(vfsRoot, CONTENTS_FILE))); + contentsUrls = Collections.list(resourceClassLoader.getResources(resourcePath(vfsRoot, CONTENTS_FILE))); } catch (IOException e) { warn("Cannot check compatibility of the merged virtual environments. Cannot read GraalPy version of the virtual environments. IOException: " + e.getMessage());