Skip to content

Commit 05769c7

Browse files
committed
moved VFS mount point sanity check to builder setters
1 parent 9e5b831 commit 05769c7

File tree

2 files changed

+46
-31
lines changed

2 files changed

+46
-31
lines changed

graalpython/com.oracle.graal.python.test.integration/src/org/graalvm/python/embedding/utils/test/VirtualFileSystemTest.java

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,6 @@
7878
import java.util.logging.Level;
7979
import java.util.logging.Logger;
8080

81-
import com.oracle.graal.python.test.integration.Utils;
8281
import org.graalvm.polyglot.Context;
8382
import org.graalvm.polyglot.Context.Builder;
8483
import org.graalvm.polyglot.HostAccess;
@@ -505,7 +504,7 @@ private static void checkException(Class<?> exType, Callable<Object> c, String m
505504
try {
506505
c.call();
507506
} catch (Exception e) {
508-
assert e.getClass() == exType;
507+
assertEquals(e.getClass(), exType);
509508
gotEx = true;
510509
}
511510
assertTrue(msg != null ? msg : "expected " + exType.getName(), gotEx);
@@ -872,16 +871,18 @@ public void externalResourcesBuilderTest() throws IOException {
872871
@Test
873872
public void vfsMountPointTest() {
874873
if (IS_WINDOWS) {
875-
checkException(IllegalArgumentException.class, () -> VirtualFileSystem.newBuilder().windowsMountPoint("test").build());
876-
checkException(IllegalArgumentException.class, () -> VirtualFileSystem.newBuilder().windowsMountPoint("test\\").build());
877-
checkException(IllegalArgumentException.class, () -> VirtualFileSystem.newBuilder().windowsMountPoint("\\test\\").build());
878-
checkException(IllegalArgumentException.class, () -> VirtualFileSystem.newBuilder().windowsMountPoint("\\test").build());
879-
checkException(InvalidPathException.class, () -> VirtualFileSystem.newBuilder().windowsMountPoint("X:\\test|test").build());
874+
checkException(IllegalArgumentException.class, () -> VirtualFileSystem.newBuilder().windowsMountPoint("test"));
875+
checkException(IllegalArgumentException.class, () -> VirtualFileSystem.newBuilder().windowsMountPoint("test\\"));
876+
checkException(IllegalArgumentException.class, () -> VirtualFileSystem.newBuilder().windowsMountPoint("\\test\\"));
877+
checkException(IllegalArgumentException.class, () -> VirtualFileSystem.newBuilder().windowsMountPoint("\\test"));
878+
checkException(IllegalArgumentException.class, () -> VirtualFileSystem.newBuilder().windowsMountPoint("X:\\test\\"));
879+
checkException(InvalidPathException.class, () -> VirtualFileSystem.newBuilder().windowsMountPoint("X:\\test|test"));
880880
} else {
881-
checkException(IllegalArgumentException.class, () -> VirtualFileSystem.newBuilder().unixMountPoint("test").build());
882-
checkException(IllegalArgumentException.class, () -> VirtualFileSystem.newBuilder().unixMountPoint("test/").build());
883-
checkException(IllegalArgumentException.class, () -> VirtualFileSystem.newBuilder().unixMountPoint("/test/").build());
884-
checkException(InvalidPathException.class, () -> VirtualFileSystem.newBuilder().unixMountPoint("/test/\0").build());
881+
checkException(IllegalArgumentException.class, () -> VirtualFileSystem.newBuilder().unixMountPoint("test"));
882+
checkException(IllegalArgumentException.class, () -> VirtualFileSystem.newBuilder().unixMountPoint("test/"));
883+
checkException(IllegalArgumentException.class, () -> VirtualFileSystem.newBuilder().unixMountPoint("/test/"));
884+
checkException(IllegalArgumentException.class, () -> VirtualFileSystem.newBuilder().unixMountPoint("X:\\test"));
885+
checkException(InvalidPathException.class, () -> VirtualFileSystem.newBuilder().unixMountPoint("/test/\0"));
885886
}
886887
}
887888

graalpython/org.graalvm.python.embedding/src/org/graalvm/python/embedding/utils/VirtualFileSystem.java

Lines changed: 34 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,9 @@ public static final class Builder {
135135
return s.endsWith(".so") || s.endsWith(".dylib") || s.endsWith(".pyd") || s.endsWith(".dll") || s.endsWith(".ttf");
136136
};
137137

138-
private String windowsMountPoint = "X:\\graalpy_vfs";
139-
private String unixMountPoint = "/graalpy_vfs";
138+
private static final String DEFAULT_WINDOWS_MOUNT_POINT = "X:\\graalpy_vfs";
139+
private String DEFAULT_UNIX_MOUNT_POINT = "/graalpy_vfs";
140+
private Path mountPoint;
140141
private Predicate<Path> extractFilter = DEFAULT_EXTRACT_FILTER;
141142
private HostIO allowHostIO = HostIO.READ_WRITE;
142143
private boolean caseInsensitive = isWindows();
@@ -172,7 +173,9 @@ public Builder allowHostIO(HostIO b) {
172173
* will not be accessible.
173174
*/
174175
public Builder windowsMountPoint(String s) {
175-
windowsMountPoint = s;
176+
if (isWindows()) {
177+
mountPoint = getMountPointAsPath(s);
178+
}
176179
return this;
177180
}
178181

@@ -185,7 +188,9 @@ public Builder windowsMountPoint(String s) {
185188
* will not be accessible.
186189
*/
187190
public Builder unixMountPoint(String s) {
188-
unixMountPoint = s;
191+
if (!isWindows()) {
192+
mountPoint = getMountPointAsPath(s);
193+
}
189194
return this;
190195
}
191196

@@ -220,10 +225,27 @@ public Builder extractFilter(Predicate<Path> filter) {
220225
}
221226

222227
public VirtualFileSystem build() {
223-
return new VirtualFileSystem(extractFilter, windowsMountPoint, unixMountPoint, allowHostIO, resourceLoadingClass, caseInsensitive);
228+
if (mountPoint == null) {
229+
mountPoint = isWindows() ? Path.of(DEFAULT_WINDOWS_MOUNT_POINT) : Path.of(DEFAULT_UNIX_MOUNT_POINT);
230+
}
231+
return new VirtualFileSystem(extractFilter, mountPoint, allowHostIO, resourceLoadingClass, caseInsensitive);
224232
}
225233
}
226234

235+
private static Path getMountPointAsPath(String mp) {
236+
Path mountPoint = Path.of(mp);
237+
if (mp.endsWith(PLATFORM_SEPARATOR) || !mountPoint.isAbsolute()) {
238+
String msg;
239+
if (System.getenv("GRAALPY_VFS_MOUNT_POINT") != null) {
240+
msg = String.format("Environment variable GRAALPY_VFS_MOUNT_POINT must be set to an absolute path without a trailing separator: '%s'", mp);
241+
} else {
242+
msg = String.format("Virtual filesystem mount point must be set to an absolute path without a trailing separator: '%s'", mp);
243+
}
244+
throw new IllegalArgumentException(msg);
245+
}
246+
return mountPoint;
247+
}
248+
227249
public static Builder newBuilder() {
228250
return new Builder();
229251
}
@@ -354,8 +376,7 @@ private void removeExtractDir() {
354376
* causing that no extraction will happen.
355377
*/
356378
VirtualFileSystem(Predicate<Path> extractFilter,
357-
String windowsMountPoint,
358-
String unixMountPoint,
379+
Path mountPoint,
359380
HostIO allowHostIO,
360381
Class<?> resourceLoadingClass,
361382
boolean caseInsensitive) {
@@ -367,21 +388,14 @@ private void removeExtractDir() {
367388

368389
this.caseInsensitive = caseInsensitive;
369390
String mp = System.getenv("GRAALPY_VFS_MOUNT_POINT");
370-
if (mp == null) {
371-
mp = isWindows() ? windowsMountPoint : unixMountPoint;
372-
}
373-
this.mountPoint = Path.of(mp);
374-
this.mountPointLowerCase = mp.toLowerCase(Locale.ROOT);
375-
if (mp.endsWith(PLATFORM_SEPARATOR) || !mountPoint.isAbsolute()) {
376-
String msg;
377-
if (System.getenv("GRAALPY_VFS_MOUNT_POINT") != null) {
378-
msg = String.format("Environment variable GRAALPY_VFS_MOUNT_POINT must be set to an absolute path without a trailing separator: '%s'", mp);
379-
} else {
380-
msg = String.format("Virtual filesystem mount point must be set to an absolute path without a trailing separator: '%s'", mp);
381-
}
382-
throw new IllegalArgumentException(msg);
391+
if (mp != null) {
392+
this.mountPoint = getMountPointAsPath(mp);
393+
} else {
394+
this.mountPoint = mountPoint;
383395
}
384396

397+
this.mountPointLowerCase = mountPoint.toString().toLowerCase(Locale.ROOT);
398+
385399
fine("VirtualFilesystem %s, allowHostIO: %s, resourceLoadingClass: %s, caseInsensitive: %s, extractOnStartup: %s%s",
386400
mountPoint, allowHostIO.toString(), this.resourceLoadingClass.getName(), caseInsensitive, extractOnStartup, extractFilter != null ? "" : ", extractFilter: null");
387401

0 commit comments

Comments
 (0)