Skip to content

Commit 02c3f1e

Browse files
olpawolpaw
committed
Improve bundle support for output paths
1 parent 3517fd3 commit 02c3f1e

File tree

9 files changed

+112
-59
lines changed

9 files changed

+112
-59
lines changed

‎substratevm/src/com.oracle.svm.common/src/com/oracle/svm/common/option/MultiOptionValue.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
package com.oracle.svm.common.option;
2727

2828
import java.util.List;
29+
import java.util.Optional;
2930

3031
public interface MultiOptionValue<T> {
3132

@@ -38,6 +39,8 @@ public interface MultiOptionValue<T> {
3839
*/
3940
List<T> values();
4041

42+
Optional<T> lastValue();
43+
4144
void valueUpdate(Object value);
4245

4346
MultiOptionValue<T> createCopy();

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -437,10 +437,11 @@ public Boolean getValue(OptionValues values) {
437437
@Option(help = "Print GC warnings as part of build output", type = OptionType.User)//
438438
public static final HostedOptionKey<Boolean> BuildOutputGCWarnings = new HostedOptionKey<>(true);
439439

440+
@BundleMember(role = BundleMember.Role.Output)//
440441
@Option(help = "Print build output statistics as JSON to the specified file. " +
441442
"The output conforms to the JSON schema located at: " +
442443
"docs/reference-manual/native-image/assets/build-output-schema-v0.9.1.json", type = OptionType.User)//
443-
public static final HostedOptionKey<String> BuildOutputJSONFile = new HostedOptionKey<>("");
444+
public static final HostedOptionKey<LocatableMultiOptionValue.Paths> BuildOutputJSONFile = new HostedOptionKey<>(LocatableMultiOptionValue.Paths.build());
444445

445446
/*
446447
* Object and array allocation options.

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/configure/ConfigurationFiles.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
import java.net.URL;
2929
import java.nio.file.Files;
3030
import java.nio.file.Path;
31-
import java.nio.file.Paths;
3231
import java.util.ArrayList;
3332
import java.util.Enumeration;
3433
import java.util.List;
@@ -51,7 +50,8 @@ public final class ConfigurationFiles {
5150

5251
public static final class Options {
5352
@Option(help = "Directories directly containing configuration files for dynamic features at runtime.", type = OptionType.User)//
54-
static final HostedOptionKey<LocatableMultiOptionValue.Strings> ConfigurationFileDirectories = new HostedOptionKey<>(LocatableMultiOptionValue.Strings.buildWithCommaDelimiter());
53+
@BundleMember(role = BundleMember.Role.Input)//
54+
static final HostedOptionKey<LocatableMultiOptionValue.Paths> ConfigurationFileDirectories = new HostedOptionKey<>(LocatableMultiOptionValue.Paths.buildWithCommaDelimiter());
5555

5656
@Option(help = "Resource path above configuration resources for dynamic features at runtime.", type = OptionType.User)//
5757
public static final HostedOptionKey<LocatableMultiOptionValue.Strings> ConfigurationResourceRoots = new HostedOptionKey<>(LocatableMultiOptionValue.Strings.buildWithCommaDelimiter());
@@ -106,11 +106,11 @@ public static final class Options {
106106

107107
public static List<Path> findConfigurationFiles(String fileName) {
108108
List<Path> files = new ArrayList<>();
109-
for (String directory : Options.ConfigurationFileDirectories.getValue().values()) {
110-
if (Files.exists(Paths.get(directory, ConfigurationFile.LOCK_FILE_NAME))) {
111-
throw foundLockFile("Configuration file directory '" + directory + "'");
109+
for (Path configDir : Options.ConfigurationFileDirectories.getValue().values()) {
110+
if (Files.exists(configDir.resolve(ConfigurationFile.LOCK_FILE_NAME))) {
111+
throw foundLockFile("Configuration file directory '" + configDir + "'");
112112
}
113-
Path path = Paths.get(directory, fileName);
113+
Path path = configDir.resolve(fileName);
114114
if (Files.exists(path)) {
115115
files.add(path);
116116
}

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/option/LocatableMultiOptionValue.java‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import java.nio.file.Path;
2828
import java.util.ArrayList;
2929
import java.util.List;
30+
import java.util.Optional;
3031
import java.util.stream.Collectors;
3132
import java.util.stream.Stream;
3233

@@ -89,10 +90,20 @@ public void valueUpdate(Object value) {
8990

9091
@Override
9192
public List<T> values() {
93+
return getValuesWithOrigins().map(Pair::getLeft).collect(Collectors.toList());
94+
}
95+
96+
@Override
97+
public Optional<T> lastValue() {
98+
return lastValueWithOrigin().map(Pair::getLeft);
99+
}
100+
101+
public Optional<Pair<T, OptionOrigin>> lastValueWithOrigin() {
92102
if (values.isEmpty()) {
93-
return List.of();
103+
return Optional.empty();
94104
}
95-
return values.stream().map(Pair::getLeft).collect(Collectors.toList());
105+
Pair<T, String> pair = values.get(values.size() - 1);
106+
return Optional.of(Pair.create(pair.getLeft(), OptionOrigin.from(pair.getRight())));
96107
}
97108

98109
public Stream<Pair<T, OptionOrigin>> getValuesWithOrigins() {

‎substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/APIOptionHandler.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,8 @@ static final class PathsOptionInfo {
118118
if (NativeImage.IS_AOT) {
119119
APIOptionSupport support = ImageSingletons.lookup(APIOptionSupport.class);
120120
groupInfos = support.groupInfos;
121-
apiOptions = support.options;
122121
pathOptions = support.pathOptions;
122+
apiOptions = support.options;
123123
} else {
124124
groupInfos = new HashMap<>();
125125
pathOptions = new HashMap<>();

‎substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/BundleSupport.java‎

Lines changed: 70 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@
4848
import org.graalvm.util.json.JSONParserException;
4949

5050
import com.oracle.svm.core.OS;
51-
import com.oracle.svm.core.SubstrateUtil;
5251
import com.oracle.svm.core.configure.ConfigurationParser;
5352
import com.oracle.svm.core.option.BundleMember;
5453
import com.oracle.svm.core.util.json.JsonPrinter;
@@ -87,6 +86,8 @@ boolean show() {
8786
final Path modulePathDir;
8887
final Path auxiliaryDir;
8988
final Path outputDir;
89+
final Path imagePathOutputDir;
90+
final Path auxiliaryOutputDir;
9091

9192
Map<Path, Path> pathCanonicalizations = new HashMap<>();
9293
Map<Path, Path> pathSubstitutions = new HashMap<>();
@@ -141,7 +142,6 @@ static BundleSupport create(NativeImage nativeImage, String bundleArg, NativeIma
141142
} else {
142143
bundleSupport = new BundleSupport(nativeImage, bundleStatus);
143144
}
144-
bundleSupport.bundleFile = nativeImage.config.getWorkingDirectory().resolve("unnamed.nib");
145145
return bundleSupport;
146146
}
147147

@@ -159,6 +159,8 @@ private BundleSupport(NativeImage nativeImage, BundleStatus status) {
159159
classPathDir = Files.createDirectories(classesDir.resolve("cp"));
160160
modulePathDir = Files.createDirectories(classesDir.resolve("p"));
161161
outputDir = Files.createDirectories(rootDir.resolve("output"));
162+
imagePathOutputDir = Files.createDirectories(outputDir.resolve("default"));
163+
auxiliaryOutputDir = Files.createDirectories(outputDir.resolve("other"));
162164
} catch (IOException e) {
163165
throw NativeImage.showError("Unable to create bundle directory layout", e);
164166
}
@@ -171,32 +173,32 @@ private BundleSupport(NativeImage nativeImage, BundleStatus status, String bundl
171173
this.nativeImage = nativeImage;
172174
this.status = status;
173175

174-
Path bundlePath = Path.of(bundleFilename);
175-
if (!Files.isReadable(bundlePath)) {
176+
bundleFile = Path.of(bundleFilename);
177+
if (!Files.isReadable(bundleFile)) {
176178
throw NativeImage.showError("The given bundle file " + bundleFilename + " cannot be read");
177179
}
178180

179-
if (Files.isDirectory(bundlePath)) {
180-
rootDir = bundlePath;
181+
if (Files.isDirectory(bundleFile)) {
182+
throw NativeImage.showError("The given bundle file " + bundleFilename + " is a directory and not a file");
181183
} else {
182184
try {
183185
rootDir = Files.createTempDirectory(bundleTempDirPrefix);
184-
try (JarFile archive = new JarFile(bundlePath.toFile())) {
186+
try (JarFile archive = new JarFile(bundleFile.toFile())) {
185187
archive.stream().forEach(jarEntry -> {
186-
Path bundleFile = rootDir.resolve(jarEntry.getName());
188+
Path bundleEntry = rootDir.resolve(jarEntry.getName());
187189
try {
188-
Path bundleFileParent = bundleFile.getParent();
190+
Path bundleFileParent = bundleEntry.getParent();
189191
if (bundleFileParent != null) {
190192
Files.createDirectories(bundleFileParent);
191193
}
192-
Files.copy(archive.getInputStream(jarEntry), bundleFile);
194+
Files.copy(archive.getInputStream(jarEntry), bundleEntry);
193195
} catch (IOException e) {
194-
throw NativeImage.showError("Unable to copy " + jarEntry.getName() + " from bundle " + bundlePath + " to " + bundleFile, e);
196+
throw NativeImage.showError("Unable to copy " + jarEntry.getName() + " from bundle " + bundleEntry + " to " + bundleEntry, e);
195197
}
196198
});
197199
}
198200
} catch (IOException e) {
199-
throw NativeImage.showError("Unable to create bundle directory layout from file " + bundlePath, e);
201+
throw NativeImage.showError("Unable to create bundle directory layout from file " + bundleFile, e);
200202
}
201203
}
202204

@@ -207,6 +209,8 @@ private BundleSupport(NativeImage nativeImage, BundleStatus status, String bundl
207209
classPathDir = classesDir.resolve("cp");
208210
modulePathDir = classesDir.resolve("p");
209211
outputDir = rootDir.resolve("output");
212+
imagePathOutputDir = outputDir.resolve("default");
213+
auxiliaryOutputDir = outputDir.resolve("other");
210214

211215
Path pathCanonicalizationsFile = stageDir.resolve("path_canonicalizations.json");
212216
try (Reader reader = Files.newBufferedReader(pathCanonicalizationsFile)) {
@@ -230,6 +234,10 @@ private BundleSupport(NativeImage nativeImage, BundleStatus status, String bundl
230234
}
231235
}
232236

237+
public boolean isBundleCreation() {
238+
return !status.loadBundle;
239+
}
240+
233241
public List<String> getBuildArgs() {
234242
return buildArgs;
235243
}
@@ -266,14 +274,19 @@ Path substituteAuxiliaryPath(Path origPath, BundleMember.Role bundleMemberRole)
266274
destinationDir = auxiliaryDir;
267275
break;
268276
case Output:
269-
destinationDir = outputDir;
277+
destinationDir = auxiliaryOutputDir;
270278
break;
271279
default:
272280
return origPath;
273281
}
274282
return substitutePath(origPath, destinationDir);
275283
}
276284

285+
Path substituteImagePath(Path origPath) {
286+
pathSubstitutions.put(origPath, imagePathOutputDir);
287+
return imagePathOutputDir;
288+
}
289+
277290
Path substituteClassPath(Path origPath) {
278291
try {
279292
return substitutePath(origPath, classPathDir);
@@ -298,11 +311,6 @@ static final class BundlePathSubstitutionError extends Error {
298311
super(message);
299312
this.origPath = origPath;
300313
}
301-
302-
BundlePathSubstitutionError(String message, Path origPath, Throwable cause) {
303-
super(message, cause);
304-
this.origPath = origPath;
305-
}
306314
}
307315

308316
@SuppressWarnings("try")
@@ -367,23 +375,16 @@ private Path substitutePath(Path origPath, Path destinationDir) {
367375
baseName = origFileName;
368376
extension = "";
369377
}
370-
String substitutedPathFilename = baseName + "_" + SubstrateUtil.digest(origPath.toString()) + extension;
371-
Path substitutedPath = destinationDir.resolve(substitutedPathFilename);
372-
if (Files.exists(substitutedPath)) {
373-
/* If we ever see this, we have to implement substitutedPath collision-handling */
374-
throw new BundlePathSubstitutionError("Failed to create a unique path-name in " + destinationDir + ". " + substitutedPath + " already exists", origPath);
378+
379+
Path substitutedPath = destinationDir.resolve(baseName + extension);
380+
int collisionIndex = 0;
381+
while (Files.exists(substitutedPath)) {
382+
collisionIndex += 1;
383+
substitutedPath = destinationDir.resolve(baseName + "_" + collisionIndex + extension);
375384
}
376385

377386
if (!destinationDir.startsWith(outputDir)) {
378-
if (Files.isDirectory(origPath)) {
379-
try (Stream<Path> walk = Files.walk(origPath)) {
380-
walk.forEach(sourcePath -> copyFile(sourcePath, substitutedPath.resolve(origPath.relativize(sourcePath))));
381-
} catch (IOException e) {
382-
throw new BundlePathSubstitutionError("Failed to iterate through directory " + origPath, origPath, e);
383-
}
384-
} else {
385-
copyFile(origPath, substitutedPath);
386-
}
387+
copyFiles(origPath, substitutedPath);
387388
}
388389

389390
Path relativeSubstitutedPath = rootDir.relativize(substitutedPath);
@@ -394,25 +395,45 @@ private Path substitutePath(Path origPath, Path destinationDir) {
394395
return substitutedPath;
395396
}
396397

397-
private void copyFile(Path source, Path target) {
398+
private void copyFiles(Path source, Path target) {
399+
if (Files.isDirectory(source)) {
400+
try (Stream<Path> walk = Files.walk(source)) {
401+
walk.forEach(sourcePath -> copyFile(sourcePath, target.resolve(source.relativize(sourcePath))));
402+
} catch (IOException e) {
403+
throw NativeImage.showError("Failed to iterate through directory " + source, e);
404+
}
405+
} else {
406+
copyFile(source, target);
407+
}
408+
}
409+
410+
private void copyFile(Path sourceFile, Path target) {
398411
try {
399-
if (nativeImage.isVerbose()) {
400-
System.out.println("> Copy to bundle: " + nativeImage.config.workDir.relativize(source));
412+
if (nativeImage.isVerbose() && target.startsWith(rootDir)) {
413+
System.out.println("> Copy to bundle: " + nativeImage.config.workDir.relativize(sourceFile));
401414
}
402-
Files.copy(source, target);
415+
Files.copy(sourceFile, target);
403416
} catch (IOException e) {
404-
throw NativeImage.showError("Failed to copy " + source + " to " + target, e);
417+
throw NativeImage.showError("Failed to copy " + sourceFile + " to " + target, e);
405418
}
406419
}
407420

408421
void shutdown() {
409-
if (!status.loadBundle) {
410-
writeBundle();
422+
Path originalImagePath = bundleFile.getParent();
423+
copyFiles(outputDir, originalImagePath.resolve(outputDir.getFileName()));
424+
425+
try {
426+
if (isBundleCreation()) {
427+
writeBundle();
428+
}
429+
} finally {
430+
nativeImage.deleteAllFiles(rootDir);
411431
}
412-
nativeImage.deleteAllFiles(rootDir);
413432
}
414433

415434
void writeBundle() {
435+
assert isBundleCreation();
436+
416437
Path pathCanonicalizationsFile = stageDir.resolve("path_canonicalizations.json");
417438
try (JsonWriter writer = new JsonWriter(pathCanonicalizationsFile)) {
418439
/* Printing as list with defined sort-order ensures useful diffs are possible */
@@ -436,6 +457,15 @@ void writeBundle() {
436457
throw NativeImage.showError("Failed to write bundle-file " + pathSubstitutionsFile, e);
437458
}
438459

460+
/*
461+
* Provide a fallback to ensure we even get a bundle if there are errors before we are able
462+
* to determine the final bundle name (see use of BundleSupport.isBundleCreation() in
463+
* NativeImage.completeImageBuild() to know where this happens).
464+
*/
465+
if (bundleFile == null) {
466+
bundleFile = nativeImage.config.getWorkingDirectory().resolve("unnamed.nib");
467+
}
468+
439469
try (JarOutputStream jarOutStream = new JarOutputStream(Files.newOutputStream(bundleFile), new Manifest())) {
440470
try (Stream<Path> walk = Files.walk(rootDir)) {
441471
walk.forEach(bundleEntry -> {

‎substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1129,8 +1129,16 @@ private int completeImageBuild() {
11291129
updateArgumentEntryValue(imageBuilderArgs, imagePathEntry, imagePath.toString());
11301130
}
11311131
if (useBundle()) {
1132-
bundleSupport.bundleFile = imagePath.resolve(imageName + ".nib");
1133-
imagePath = bundleSupport.substituteAuxiliaryPath(imagePath, BundleMember.Role.Output);
1132+
if (bundleSupport.isBundleCreation()) {
1133+
/*
1134+
* If we are in bundle creation mode, we are at the point where we know the final
1135+
* imagePath and imageName that we can now use to derive the new bundle name from.
1136+
*/
1137+
bundleSupport.bundleFile = imagePath.resolve(imageName + ".nib");
1138+
}
1139+
/* In bundle mode the imagePath has to be redirected to be within the bundle */
1140+
imagePath = bundleSupport.substituteImagePath(imagePath);
1141+
/* and we need to adjust the argument that passes the imagePath to the builder */
11341142
updateArgumentEntryValue(imageBuilderArgs, imagePathEntry, imagePath.toString());
11351143
}
11361144

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ProgressReporter.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import java.util.List;
4242
import java.util.Map;
4343
import java.util.Map.Entry;
44+
import java.util.Optional;
4445
import java.util.TreeMap;
4546
import java.util.TreeSet;
4647
import java.util.concurrent.Executors;
@@ -190,8 +191,9 @@ public ProgressReporter(OptionValues options) {
190191
builderIO = NativeImageSystemIOWrappers.singleton();
191192
}
192193

193-
if (SubstrateOptions.BuildOutputJSONFile.hasBeenSet(options)) {
194-
jsonHelper = new ProgressReporterJsonHelper(SubstrateOptions.BuildOutputJSONFile.getValue(options));
194+
Optional<Path> buildOutputJSONFile = SubstrateOptions.BuildOutputJSONFile.getValue(options).lastValue();
195+
if (buildOutputJSONFile.isPresent()) {
196+
jsonHelper = new ProgressReporterJsonHelper(buildOutputJSONFile.get());
195197
} else {
196198
jsonHelper = null;
197199
}

0 commit comments

Comments
 (0)