[GR-43971] Support native-image environment variable capturing in bundles.

olpaw · olpaw · commit b2265d5abafa · 2023-03-07T06:07:07.000Z
PullRequest: graal/13914
diff --git a/substratevm/CHANGELOG.md b/substratevm/CHANGELOG.md
@@ -26,6 +26,7 @@ This changelog summarizes major changes to GraalVM Native Image.
 * (GR-40641) Dynamic linking of AWT libraries on Linux.
 * (GR-40463) Red Hat added experimental support for JMX, which can be enabled with the `--enable-monitoring` option (e.g. `--enable-monitoring=jmxclient,jmxserver`).
 * (GR-44110) Native Image now targets `x86-64-v3` by default on AMD64 and supports a new `-march` option. Use `-march=compatibility` for best compatibility (previous default) or `-march=native` for best performance if the native executable is deployed on the same machine or on a machine with the same CPU features. To list all available machine types, use `-march=list`.
+* (GR-43971) Add native-image option `-E<env-var-key>[=<env-var-value>]` and support environment variable capturing in bundles. Previously almost all environment variables were available in the builder. To temporarily revert back to the old behaviour, env setting `NATIVE_IMAGE_SLOPPY_BUILDER_SANITATION=true` can be used. The old behaviour will be removed in a future release.
 
 ## Version 22.3.0
 * (GR-35721) Remove old build output style and the `-H:±BuildOutputUseNewStyle` option.
diff --git a/substratevm/src/com.oracle.svm.driver/resources/HelpExtra.txt b/substratevm/src/com.oracle.svm.driver/resources/HelpExtra.txt
@@ -34,5 +34,11 @@ Non-standard options help:
                           creates a new bundle app_dbg.nib based on the given app.nib bundle.
                           Both bundles are the same except the new one also uses the -g option.
 
+    -E<env-var-key>[=<env-var-value>]
+                          allow native-image to access the given environment variable during
+                          image build. If the optional <env-var-value> is not given, the value
+                          of the environment variable will be taken from the environment
+                          native-image was invoked from.
+
     -V<key>=<value>       provide values for placeholders in native-image.properties files
 
diff --git a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/BundleSupport.java b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/BundleSupport.java
@@ -40,11 +40,11 @@
 import java.time.format.FormatStyle;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.ListIterator;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
@@ -86,8 +86,8 @@ final class BundleSupport {
     Map<Path, Path> pathCanonicalizations = new HashMap<>();
     Map<Path, Path> pathSubstitutions = new HashMap<>();
 
-    private final List<String> buildArgs;
-    private Collection<String> updatedBuildArgs;
+    private final List<String> nativeImageArgs;
+    private List<String> updatedNativeImageArgs;
 
     boolean loadBundle;
     boolean writeBundle;
@@ -110,9 +110,13 @@ final class BundleSupport {
     static final String BUNDLE_OPTION = "--bundle";
     static final String BUNDLE_FILE_EXTENSION = ".nib";
 
-    private enum BundleOptionVariants {
+    enum BundleOptionVariants {
         create(),
-        apply()
+        apply();
+
+        String optionName() {
+            return BUNDLE_OPTION + "-" + this;
+        }
     }
 
     static BundleSupport create(NativeImage nativeImage, String bundleArg, NativeImage.ArgumentQueue args) {
@@ -121,10 +125,6 @@ static BundleSupport create(NativeImage nativeImage, String bundleArg, NativeIma
                             "Bundle support is still experimental and needs to be unlocked with '" + UNLOCK_BUNDLE_SUPPORT_OPTION + "'. The unlock option must precede '" + bundleArg + "'.");
         }
 
-        if (!nativeImage.userConfigProperties.isEmpty()) {
-            throw NativeImage.showError("Bundle support cannot be combined with " + NativeImage.CONFIG_FILE_ENV_VAR_KEY + " environment variable use.");
-        }
-
         try {
             String variant = bundleArg.substring(BUNDLE_OPTION.length() + 1);
             String bundleFilename = null;
@@ -133,31 +133,31 @@ static BundleSupport create(NativeImage nativeImage, String bundleArg, NativeIma
                 variant = variantParts[0];
                 bundleFilename = variantParts[1];
             }
-            String applyOptionStr = BUNDLE_OPTION + "-" + BundleOptionVariants.apply;
-            String createOptionStr = BUNDLE_OPTION + "-" + BundleOptionVariants.create;
+            String applyOptionName = BundleOptionVariants.apply.optionName();
+            String createOptionName = BundleOptionVariants.create.optionName();
             BundleSupport bundleSupport;
             switch (BundleOptionVariants.valueOf(variant)) {
                 case apply:
                     if (nativeImage.useBundle()) {
                         if (nativeImage.bundleSupport.loadBundle) {
-                            throw NativeImage.showError(String.format("native-image allows option %s to be specified only once.", applyOptionStr));
+                            throw NativeImage.showError(String.format("native-image allows option %s to be specified only once.", applyOptionName));
                         }
                         if (nativeImage.bundleSupport.writeBundle) {
-                            throw NativeImage.showError(String.format("native-image option %s is not allowed to be used after option %s.", applyOptionStr, createOptionStr));
+                            throw NativeImage.showError(String.format("native-image option %s is not allowed to be used after option %s.", applyOptionName, createOptionName));
                         }
                     }
                     if (bundleFilename == null) {
-                        throw NativeImage.showError(String.format("native-image option %s requires a bundle file argument. E.g. %s=bundle-file.nib.", applyOptionStr, applyOptionStr));
+                        throw NativeImage.showError(String.format("native-image option %s requires a bundle file argument. E.g. %s=bundle-file.nib.", applyOptionName, applyOptionName));
                     }
                     bundleSupport = new BundleSupport(nativeImage, bundleFilename);
                     /* Inject the command line args from the loaded bundle in-place */
-                    List<String> buildArgs = bundleSupport.getBuildArgs();
+                    List<String> buildArgs = bundleSupport.getNativeImageArgs();
                     for (int i = buildArgs.size() - 1; i >= 0; i--) {
                         args.push(buildArgs.get(i));
                     }
                     nativeImage.showVerboseMessage(nativeImage.isVerbose(), BUNDLE_INFO_MESSAGE_PREFIX + "Inject args: '" + String.join(" ", buildArgs) + "'");
                     /* Snapshot args after in-place expansion (includes also args after this one) */
-                    bundleSupport.updatedBuildArgs = args.snapshot();
+                    bundleSupport.updatedNativeImageArgs = args.snapshot();
                     break;
                 case create:
                     if (nativeImage.useBundle()) {
@@ -209,7 +209,7 @@ private BundleSupport(NativeImage nativeImage) {
         } catch (IOException e) {
             throw NativeImage.showError("Unable to create bundle directory layout", e);
         }
-        this.buildArgs = Collections.unmodifiableList(nativeImage.config.getBuildArgs());
+        this.nativeImageArgs = nativeImage.getNativeImageArgs();
     }
 
     private BundleSupport(NativeImage nativeImage, String bundleFilenameArg) {
@@ -279,18 +279,25 @@ private BundleSupport(NativeImage nativeImage, String bundleFilenameArg) {
         } catch (IOException e) {
             throw NativeImage.showError("Failed to read bundle-file " + pathSubstitutionsFile, e);
         }
+        Path environmentFile = stageDir.resolve("environment.json");
+        try (Reader reader = Files.newBufferedReader(environmentFile)) {
+            new EnvironmentParser(nativeImage.imageBuilderEnvironment).parseAndRegister(reader);
+        } catch (IOException e) {
+            throw NativeImage.showError("Failed to read bundle-file " + environmentFile, e);
+        }
+
         Path buildArgsFile = stageDir.resolve("build.json");
         try (Reader reader = Files.newBufferedReader(buildArgsFile)) {
             List<String> buildArgsFromFile = new ArrayList<>();
             new BuildArgsParser(buildArgsFromFile).parseAndRegister(reader);
-            buildArgs = Collections.unmodifiableList(buildArgsFromFile);
+            nativeImageArgs = Collections.unmodifiableList(buildArgsFromFile);
         } catch (IOException e) {
-            throw NativeImage.showError("Failed to read bundle-file " + pathSubstitutionsFile, e);
+            throw NativeImage.showError("Failed to read bundle-file " + buildArgsFile, e);
         }
     }
 
-    public List<String> getBuildArgs() {
-        return buildArgs;
+    public List<String> getNativeImageArgs() {
+        return nativeImageArgs;
     }
 
     Path recordCanonicalization(Path before, Path after) {
@@ -409,8 +416,8 @@ private Path substitutePath(Path origPath, Path destinationDir) {
             return origPath;
         }
 
-        // TODO Report error if overlapping dir-trees are passed in
-        // TODO add .endsWith(ClasspathUtils.cpWildcardSubstitute) handling (copy whole directory)
+        // TODO: Report error if overlapping dir-trees are passed in
+
         String origFileName = origPath.getFileName().toString();
         int extensionPos = origFileName.lastIndexOf('.');
         String baseName;
@@ -440,6 +447,16 @@ private Path substitutePath(Path origPath, Path destinationDir) {
         return substitutedPath;
     }
 
+    Path originalPath(Path substitutedPath) {
+        Path relativeSubstitutedPath = rootDir.relativize(substitutedPath);
+        for (Map.Entry<Path, Path> entry : pathSubstitutions.entrySet()) {
+            if (entry.getValue().equals(relativeSubstitutedPath)) {
+                return entry.getKey();
+            }
+        }
+        return null;
+    }
+
     private void copyFiles(Path source, Path target, boolean overwrite) {
         nativeImage.showVerboseMessage(nativeImage.isVVerbose(), "> Copy files from " + source + " to " + target);
         if (Files.isDirectory(source)) {
@@ -567,39 +584,36 @@ private Path writeBundle() {
         } catch (IOException e) {
             throw NativeImage.showError("Failed to write bundle-file " + pathSubstitutionsFile, e);
         }
+        Path environmentFile = stageDir.resolve("environment.json");
+        try (JsonWriter writer = new JsonWriter(environmentFile)) {
+            /* Printing as list with defined sort-order ensures useful diffs are possible */
+            JsonPrinter.printCollection(writer, nativeImage.imageBuilderEnvironment.entrySet(), Map.Entry.comparingByKey(), BundleSupport::printEnvironmentVariable);
+        } catch (IOException e) {
+            throw NativeImage.showError("Failed to write bundle-file " + environmentFile, e);
+        }
 
         Path buildArgsFile = stageDir.resolve("build.json");
         try (JsonWriter writer = new JsonWriter(buildArgsFile)) {
-            ArrayList<String> cleanBuildArgs = new ArrayList<>();
-            for (String buildArg : updatedBuildArgs != null ? updatedBuildArgs : buildArgs) {
-                if (buildArg.equals(UNLOCK_BUNDLE_SUPPORT_OPTION)) {
-                    continue;
-                }
-                if (buildArg.startsWith(BUNDLE_OPTION)) {
-                    continue;
-                }
-                if (buildArg.startsWith(nativeImage.oHPath)) {
-                    continue;
-                }
-                if (buildArg.equals(CmdLineOptionHandler.VERBOSE_OPTION)) {
-                    continue;
-                }
-                if (buildArg.equals(CmdLineOptionHandler.DRY_RUN_OPTION)) {
-                    continue;
-                }
-                if (buildArg.startsWith("-Dllvm.bin.dir=")) {
-                    Optional<String> existing = nativeImage.config.getBuildArgs().stream().filter(arg -> arg.startsWith("-Dllvm.bin.dir=")).findFirst();
-                    if (existing.isPresent() && !existing.get().equals(buildArg)) {
-                        throw NativeImage.showError("Bundle native-image argument '" + buildArg + "' conflicts with existing '" + existing.get() + "'.");
+            List<String> equalsNonBundleOptions = List.of(UNLOCK_BUNDLE_SUPPORT_OPTION, CmdLineOptionHandler.VERBOSE_OPTION, CmdLineOptionHandler.DRY_RUN_OPTION);
+            List<String> startsWithNonBundleOptions = List.of(BUNDLE_OPTION, DefaultOptionHandler.ADD_ENV_VAR_OPTION, nativeImage.oHPath);
+            ArrayList<String> bundleArgs = new ArrayList<>(updatedNativeImageArgs != null ? updatedNativeImageArgs : nativeImageArgs);
+            ListIterator<String> bundleArgsIterator = bundleArgs.listIterator();
+            while (bundleArgsIterator.hasNext()) {
+                String arg = bundleArgsIterator.next();
+                if (equalsNonBundleOptions.contains(arg) || startsWithNonBundleOptions.stream().anyMatch(arg::startsWith)) {
+                    bundleArgsIterator.remove();
+                } else if (arg.startsWith("-Dllvm.bin.dir=")) {
+                    Optional<String> existing = nativeImage.config.getBuildArgs().stream().filter(a -> a.startsWith("-Dllvm.bin.dir=")).findFirst();
+                    if (existing.isPresent() && !existing.get().equals(arg)) {
+                        throw NativeImage.showError("Bundle native-image argument '" + arg + "' conflicts with existing '" + existing.get() + "'.");
                     }
-                    continue;
+                    bundleArgsIterator.remove();
                 }
-                cleanBuildArgs.add(buildArg);
             }
             /* Printing as list with defined sort-order ensures useful diffs are possible */
-            JsonPrinter.printCollection(writer, cleanBuildArgs, null, BundleSupport::printBuildArg);
+            JsonPrinter.printCollection(writer, bundleArgs, null, BundleSupport::printBuildArg);
         } catch (IOException e) {
-            throw NativeImage.showError("Failed to write bundle-file " + pathSubstitutionsFile, e);
+            throw NativeImage.showError("Failed to write bundle-file " + buildArgsFile, e);
         }
 
         bundleProperties.write();
@@ -639,7 +653,7 @@ private static Manifest createManifest() {
     private static final String substitutionMapDstField = "dst";
 
     private static void printPathMapping(Map.Entry<Path, Path> entry, JsonWriter w) throws IOException {
-        w.append('{').quote(substitutionMapSrcField).append(" : ").quote(entry.getKey());
+        w.append('{').quote(substitutionMapSrcField).append(':').quote(entry.getKey());
         w.append(',').quote(substitutionMapDstField).append(':').quote(entry.getValue());
         w.append('}');
     }
@@ -648,6 +662,18 @@ private static void printBuildArg(String entry, JsonWriter w) throws IOException
         w.quote(entry);
     }
 
+    private static final String environmentKeyField = "key";
+    private static final String environmentValueField = "val";
+
+    private static void printEnvironmentVariable(Map.Entry<String, String> entry, JsonWriter w) throws IOException {
+        if (entry.getValue() == null) {
+            throw NativeImage.showError("Storing environment variable '" + entry.getKey() + "' in bundle requires to have its value defined.");
+        }
+        w.append('{').quote(environmentKeyField).append(':').quote(entry.getKey());
+        w.append(',').quote(environmentValueField).append(':').quote(entry.getValue());
+        w.append('}');
+    }
+
     private static final class PathMapParser extends ConfigurationParser {
 
         private final Map<Path, Path> pathMap;
@@ -674,6 +700,33 @@ public void parseAndRegister(Object json, URI origin) {
         }
     }
 
+    private static final class EnvironmentParser extends ConfigurationParser {
+
+        private final Map<String, String> environment;
+
+        private EnvironmentParser(Map<String, String> environment) {
+            super(true);
+            environment.clear();
+            this.environment = environment;
+        }
+
+        @Override
+        public void parseAndRegister(Object json, URI origin) {
+            for (var rawEntry : asList(json, "Expected a list of environment variable objects")) {
+                var entry = asMap(rawEntry, "Expected a environment variable object");
+                Object envVarKeyString = entry.get(environmentKeyField);
+                if (envVarKeyString == null) {
+                    throw new JSONParserException("Expected " + environmentKeyField + "-field in environment variable object");
+                }
+                Object envVarValueString = entry.get(environmentValueField);
+                if (envVarValueString == null) {
+                    throw new JSONParserException("Expected " + environmentValueField + "-field in environment variable object");
+                }
+                environment.put(envVarKeyString.toString(), envVarValueString.toString());
+            }
+        }
+    }
+
     private static final class BuildArgsParser extends ConfigurationParser {
 
         private final List<String> args;
diff --git a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/DefaultOptionHandler.java b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/DefaultOptionHandler.java
@@ -45,6 +45,8 @@ class DefaultOptionHandler extends NativeImage.OptionHandler<NativeImage> {
     static final String addModulesOption = "--add-modules";
     private static final String addModulesErrorMessage = " requires modules to be specified";
 
+    static final String ADD_ENV_VAR_OPTION = "-E";
+
     /* Defunct legacy options that we have to accept to maintain backward compatibility */
     private static final String noServerOption = "--no-server";
 
@@ -166,6 +168,14 @@ public boolean consume(ArgumentQueue args) {
             nativeImage.addOptionKeyValue(keyValue[0], keyValue[1]);
             return true;
         }
+        if (headArg.startsWith(ADD_ENV_VAR_OPTION)) {
+            args.poll();
+            String envVarSetting = headArg.substring(ADD_ENV_VAR_OPTION.length());
+            String[] keyValue = envVarSetting.split("=", 2);
+            String valueDefinedOrInherited = keyValue.length > 1 ? keyValue[1] : null;
+            nativeImage.imageBuilderEnvironment.put(keyValue[0], valueDefinedOrInherited);
+            return true;
+        }
         if (headArg.startsWith("-J")) {
             args.poll();
             if (headArg.equals("-J")) {
@@ -191,7 +201,7 @@ public boolean consume(ArgumentQueue args) {
             Path origArgFile = Paths.get(headArg);
             Path argFile = nativeImage.bundleSupport != null ? nativeImage.bundleSupport.substituteAuxiliaryPath(origArgFile, BundleMember.Role.Input) : origArgFile;
             NativeImage.NativeImageArgsProcessor processor = nativeImage.new NativeImageArgsProcessor(OptionOrigin.argFilePrefix + argFile);
-            readArgFile(argFile).forEach(processor::accept);
+            readArgFile(argFile).forEach(processor);
             List<String> leftoverArgs = processor.apply(false);
             if (leftoverArgs.size() > 0) {
                 NativeImage.showError(String.format("Found unrecognized options while parsing argument file '%s':%n%s", argFile, String.join(System.lineSeparator(), leftoverArgs)));
@@ -211,7 +221,7 @@ enum PARSER_STATE {
         IN_TOKEN
     }
 
-    class CTX_ARGS {
+    static class CTX_ARGS {
         PARSER_STATE state;
         int cptr;
         int eob;
@@ -221,7 +231,7 @@ class CTX_ARGS {
     }
 
     // Ported from JDK11's java.base/share/native/libjli/args.c
-    private List<String> readArgFile(Path file) {
+    private static List<String> readArgFile(Path file) {
         List<String> arguments = new ArrayList<>();
         // Use of the at sign (@) to recursively interpret files isn't supported.
         arguments.add("--disable-@files");
diff --git a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java
