From 9de9652582c96b7047209c6010347fdcce5234b4 Mon Sep 17 00:00:00 2001 From: Joe Onorato Date: Tue, 19 Jan 2021 22:20:09 -0800 Subject: [PATCH] Add class to fork and exec kati, based on the commandline option given. Test: m product-config-test && java -jar out/host/linux-x86/testcases/product-config-test/product-config-test.jar Change-Id: I4706e32ff7ac4424b6835b94fef40a2c838f8492 --- .../src/com/android/build/config/Errors.java | 5 +- .../com/android/build/config/KatiCommand.java | 43 ++++++ .../android/build/config/KatiCommandImpl.java | 139 ++++++++++++++++++ .../src/com/android/build/config/Main.java | 4 +- .../src/com/android/build/config/Options.java | 94 +++++++++++- .../com/android/build/config/OptionsTest.java | 37 ++++- 6 files changed, 305 insertions(+), 17 deletions(-) create mode 100644 tools/product_config/src/com/android/build/config/KatiCommand.java create mode 100644 tools/product_config/src/com/android/build/config/KatiCommandImpl.java diff --git a/tools/product_config/src/com/android/build/config/Errors.java b/tools/product_config/src/com/android/build/config/Errors.java index 63792c8c8f..5424338fba 100644 --- a/tools/product_config/src/com/android/build/config/Errors.java +++ b/tools/product_config/src/com/android/build/config/Errors.java @@ -30,7 +30,7 @@ import java.util.Map; * Naming Convention: * */ @@ -42,4 +42,7 @@ public class Errors extends ErrorReporter { public final Category WARNING_UNKNOWN_COMMAND_LINE_ERROR = new Category(2, true, Level.HIDDEN, "Passing unknown errors on the command line. Hidden by default for\n" + "forward compatibility."); + + public final Category ERROR_KATI = new Category(3, false, Level.ERROR, + "Error executing or reading from Kati."); } diff --git a/tools/product_config/src/com/android/build/config/KatiCommand.java b/tools/product_config/src/com/android/build/config/KatiCommand.java new file mode 100644 index 0000000000..f3c71d23b9 --- /dev/null +++ b/tools/product_config/src/com/android/build/config/KatiCommand.java @@ -0,0 +1,43 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.build.config; + +import java.util.Arrays; +import java.util.List; + +public interface KatiCommand { + public static class KatiException extends Exception { + private String mStderr; + + public KatiException(List cmd, String stderr) { + super("Error running kati: " + Arrays.toString(cmd.toArray())); + mStderr = stderr; + } + + public String getStderr() { + return mStderr; + } + } + + /** + * Run kati directly. Returns stdout data. + * + * @throws KatiException if there is an error. KatiException will contain + * the stderr from the kati invocation. + */ + public String run(String[] args) throws KatiException; +} diff --git a/tools/product_config/src/com/android/build/config/KatiCommandImpl.java b/tools/product_config/src/com/android/build/config/KatiCommandImpl.java new file mode 100644 index 0000000000..53480d4095 --- /dev/null +++ b/tools/product_config/src/com/android/build/config/KatiCommandImpl.java @@ -0,0 +1,139 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.build.config; + +import java.io.InputStream; +import java.io.InputStreamReader; +import java.io.IOException; +import java.util.ArrayList; +import java.nio.charset.StandardCharsets; + +public class KatiCommandImpl implements KatiCommand { + final Errors mErrors; + final Options mOptions; + + /** + * Runnable that consumes all of an InputStream until EOF, writes the contents + * into a StringBuilder, and then closes the stream. + */ + class OutputReader implements Runnable { + private final InputStream mStream; + private final StringBuilder mOutput; + + OutputReader(InputStream stream, StringBuilder output) { + mStream = stream; + mOutput = output; + } + + @Override + public void run() { + final char[] buf = new char[16*1024]; + final InputStreamReader reader = new InputStreamReader(mStream, StandardCharsets.UTF_8); + try { + int amt; + while ((amt = reader.read(buf, 0, buf.length)) >= 0) { + mOutput.append(buf, 0, amt); + } + } catch (IOException ex) { + mErrors.ERROR_KATI.add("Error reading from kati: " + ex.getMessage()); + } finally { + try { + reader.close(); + } catch (IOException ex) { + // Close doesn't throw + } + } + } + } + + public KatiCommandImpl(Errors errors, Options options) { + mErrors = errors; + mOptions = options; + } + + /** + * Run kati directly. Returns stdout data. + * + * @throws KatiException if there is an error. KatiException will contain + * the stderr from the kati invocation. + */ + public String run(String[] args) throws KatiException { + final ArrayList cmd = new ArrayList(); + cmd.add(mOptions.getCKatiBin()); + for (String arg: args) { + cmd.add(arg); + } + + final ProcessBuilder builder = new ProcessBuilder(cmd); + builder.redirectOutput(ProcessBuilder.Redirect.PIPE); + builder.redirectError(ProcessBuilder.Redirect.PIPE); + + Process process = null; + + try { + process = builder.start(); + } catch (IOException ex) { + throw new KatiException(cmd, "IOException running process: " + ex.getMessage()); + } + + final StringBuilder stdout = new StringBuilder(); + final Thread stdoutThread = new Thread(new OutputReader(process.getInputStream(), stdout), + "kati_stdout_reader"); + stdoutThread.start(); + + final StringBuilder stderr = new StringBuilder(); + final Thread stderrThread = new Thread(new OutputReader(process.getErrorStream(), stderr), + "kati_stderr_reader"); + stderrThread.start(); + + int returnCode = waitForProcess(process); + joinThread(stdoutThread); + joinThread(stderrThread); + + if (returnCode != 0) { + throw new KatiException(cmd, stderr.toString()); + } + + return stdout.toString(); + } + + /** + * Wrap Process.waitFor() because it throws InterruptedException. + */ + private static int waitForProcess(Process proc) { + while (true) { + try { + return proc.waitFor(); + } catch (InterruptedException ex) { + } + } + } + + /** + * Wrap Thread.join() because it throws InterruptedException. + */ + private static void joinThread(Thread thread) { + while (true) { + try { + thread.join(); + return; + } catch (InterruptedException ex) { + } + } + } +} + diff --git a/tools/product_config/src/com/android/build/config/Main.java b/tools/product_config/src/com/android/build/config/Main.java index b792193059..e577dfef26 100644 --- a/tools/product_config/src/com/android/build/config/Main.java +++ b/tools/product_config/src/com/android/build/config/Main.java @@ -47,7 +47,7 @@ public class Main { int exitCode = 0; try { - Options options = Options.parse(errors, args); + Options options = Options.parse(errors, args, System.getenv()); if (errors.hadError()) { Options.printHelp(System.err); System.err.println(); @@ -62,7 +62,7 @@ public class Main { Options.printHelp(System.out); return; } - } catch (CommandException ex) { + } catch (CommandException | Errors.FatalException ex) { // These are user errors, so don't show a stack trace exitCode = 1; } catch (Throwable ex) { diff --git a/tools/product_config/src/com/android/build/config/Options.java b/tools/product_config/src/com/android/build/config/Options.java index 48146cb73d..4e6048425e 100644 --- a/tools/product_config/src/com/android/build/config/Options.java +++ b/tools/product_config/src/com/android/build/config/Options.java @@ -17,6 +17,7 @@ package com.android.build.config; import java.io.PrintStream; +import java.util.Map; import java.util.TreeMap; public class Options { @@ -27,19 +28,50 @@ public class Options { private Action mAction = Action.DEFAULT; + private String mProduct; + private String mVariant; + private String mOutDir; + private String mCKatiBin; + public Action getAction() { return mAction; } + public String getProduct() { + return mProduct; + } + + public String getVariant() { + return mVariant; + } + + public String getOutDir() { + return mOutDir != null ? mOutDir : "out"; + } + + public String getCKatiBin() { + return mCKatiBin; + } + public static void printHelp(PrintStream out) { out.println("usage: product_config"); out.println(); - out.println("OPTIONS"); + out.println("REQUIRED FLAGS"); + out.println(" --ckati_bin CKATI Kati binary to use."); + out.println(); + out.println("OPTIONAL FLAGS"); out.println(" --hide ERROR_ID Suppress this error."); out.println(" --error ERROR_ID Make this ERROR_ID a fatal error."); out.println(" --help -h This message."); out.println(" --warning ERROR_ID Make this ERROR_ID a warning."); out.println(); + out.println("REQUIRED ENVIRONMENT"); + out.println(" TARGET_PRODUCT Product to build from lunch command."); + out.println(" TARGET_BUILD_VARIANT Build variant from lunch command."); + out.println(); + out.println("OPTIONAL ENVIRONMENT"); + out.println(" OUT_DIR Build output directory. Defaults to \"out\"."); + out.println(); out.println("ERRORS"); out.println(" The following are the errors that can be controlled on the"); out.println(" commandline with the --hide --warning --error flags."); @@ -63,20 +95,26 @@ public class Options { private Errors mErrors; private String[] mArgs; + private Map mEnv; private Options mResult = new Options(); private int mIndex; + private boolean mSkipRequiredArgValidation; - public Parser(Errors errors, String[] args) { + public Parser(Errors errors, String[] args, Map env) { mErrors = errors; mArgs = args; + mEnv = env; } public Options parse() { + // Args try { while (mIndex < mArgs.length) { final String arg = mArgs[mIndex]; - if ("--hide".equals(arg)) { + if ("--ckati_bin".equals(arg)) { + mResult.mCKatiBin = requireNextStringArg(arg); + } else if ("--hide".equals(arg)) { handleErrorCode(arg, Errors.Level.HIDDEN); } else if ("--error".equals(arg)) { handleErrorCode(arg, Errors.Level.ERROR); @@ -99,11 +137,45 @@ public class Options { mErrors.ERROR_COMMAND_LINE.add(ex.getMessage()); } + // Environment + mResult.mProduct = mEnv.get("TARGET_PRODUCT"); + mResult.mVariant = mEnv.get("TARGET_BUILD_VARIANT"); + mResult.mOutDir = mEnv.get("OUT_DIR"); + + validateArgs(); + return mResult; } - private void addWarning(Errors.Category category, String message) { - category.add(message); + /** + * For testing; don't generate errors about missing arguments + */ + public void setSkipRequiredArgValidation() { + mSkipRequiredArgValidation = true; + } + + private void validateArgs() { + if (!mSkipRequiredArgValidation) { + if (mResult.mCKatiBin == null || "".equals(mResult.mCKatiBin)) { + addMissingArgError("--ckati_bin"); + } + if (mResult.mProduct == null) { + addMissingEnvError("TARGET_PRODUCT"); + } + if (mResult.mVariant == null) { + addMissingEnvError("TARGET_BUILD_VARIANT"); + } + } + } + + private void addMissingArgError(String argName) { + mErrors.ERROR_COMMAND_LINE.add("Required command line argument missing: " + + argName); + } + + private void addMissingEnvError(String envName) { + mErrors.ERROR_COMMAND_LINE.add("Required environment variable missing: " + + envName); } private String getNextNonFlagArg() { @@ -117,6 +189,14 @@ public class Options { return mArgs[mIndex]; } + private String requireNextStringArg(String arg) throws ParseException { + final String val = getNextNonFlagArg(); + if (val == null) { + throw new ParseException(arg + " requires a string argument."); + } + return val; + } + private int requireNextNumberArg(String arg) throws ParseException { final String val = getNextNonFlagArg(); if (val == null) { @@ -151,7 +231,7 @@ public class Options { *

* Adds errors encountered to Errors object. */ - public static Options parse(Errors errors, String[] args) { - return (new Parser(errors, args)).parse(); + public static Options parse(Errors errors, String[] args, Map env) { + return (new Parser(errors, args, env)).parse(); } } diff --git a/tools/product_config/test/com/android/build/config/OptionsTest.java b/tools/product_config/test/com/android/build/config/OptionsTest.java index 2c36322584..459efa53b0 100644 --- a/tools/product_config/test/com/android/build/config/OptionsTest.java +++ b/tools/product_config/test/com/android/build/config/OptionsTest.java @@ -19,12 +19,24 @@ package com.android.build.config; import org.junit.Assert; import org.junit.Test; +import java.util.HashMap; + public class OptionsTest { + + private Options parse(Errors errors, String[] args) { + final HashMap env = new HashMap(); + env.put("TARGET_PRODUCT", "test_product"); + env.put("TARGET_BUILD_VARIANT", "user"); + final Options.Parser parser = new Options.Parser(errors, args, env); + parser.setSkipRequiredArgValidation(); + return parser.parse(); + } + @Test public void testErrorMissingLast() { final Errors errors = new Errors(); - final Options options = Options.parse(errors, new String[] { + final Options options = parse(errors, new String[] { "--error" }); @@ -37,7 +49,7 @@ public class OptionsTest { public void testErrorMissingNotLast() { final Errors errors = new Errors(); - final Options options = Options.parse(errors, new String[] { + final Options options = parse(errors, new String[] { "--error", "--warning", "2" }); @@ -50,7 +62,7 @@ public class OptionsTest { public void testErrorNotNumeric() { final Errors errors = new Errors(); - final Options options = Options.parse(errors, new String[] { + final Options options = parse(errors, new String[] { "--error", "notgood" }); @@ -63,7 +75,7 @@ public class OptionsTest { public void testErrorInvalidError() { final Errors errors = new Errors(); - final Options options = Options.parse(errors, new String[] { + final Options options = parse(errors, new String[] { "--error", "50000" }); @@ -76,7 +88,7 @@ public class OptionsTest { public void testErrorOne() { final Errors errors = new Errors(); - final Options options = Options.parse(errors, new String[] { + final Options options = parse(errors, new String[] { "--error", "2" }); @@ -89,7 +101,7 @@ public class OptionsTest { public void testWarningOne() { final Errors errors = new Errors(); - final Options options = Options.parse(errors, new String[] { + final Options options = parse(errors, new String[] { "--warning", "2" }); @@ -102,7 +114,7 @@ public class OptionsTest { public void testHideOne() { final Errors errors = new Errors(); - final Options options = Options.parse(errors, new String[] { + final Options options = parse(errors, new String[] { "--hide", "2" }); @@ -110,5 +122,16 @@ public class OptionsTest { Assert.assertEquals(Options.Action.DEFAULT, options.getAction()); Assert.assertFalse(errors.hadWarningOrError()); } + + @Test + public void testEnv() { + final Errors errors = new Errors(); + + final Options options = parse(errors, new String[0]); + + Assert.assertEquals("test_product", options.getProduct()); + Assert.assertEquals("user", options.getVariant()); + Assert.assertFalse(errors.hadWarningOrError()); + } }