Skip to content

Commit

Permalink
* Refactor Builder a little to work around issues with Gradle
Browse files Browse the repository at this point in the history
  • Loading branch information
saudet committed May 25, 2020
1 parent 9762f14 commit a970348
Show file tree
Hide file tree
Showing 3 changed files with 133 additions and 59 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@

* Refactor `Builder` a little to work around issues with Gradle
* Log as warnings `SecurityException` thrown on `Loader.getCacheDir()` instead of swallowing them
* Fix memory leak that occurs with "org.bytedeco.javacpp.nopointergc" ([issue bytedeco/javacpp-presets#878](https://github.com/bytedeco/javacpp-presets/issues/878))
* Take into account `platform.library.path` when extracting executables and their libraries on `Loader.load()` ([issue bytedeco/javacv#1410](https://github.com/bytedeco/javacv/issues/1410))
Expand Down
94 changes: 35 additions & 59 deletions src/main/java/org/bytedeco/javacpp/tools/Builder.java
Original file line number Diff line number Diff line change
Expand Up @@ -190,61 +190,6 @@ void includeJavaPaths(ClassProperties properties, boolean header) {
}
}

/**
* Executes a command with {@link ProcessBuilder}, but also logs the call
* and redirects its input and output to our process.
*
* @param command to have {@link ProcessBuilder} execute
* @param workingDirectory to pass to {@link ProcessBuilder#directory()}
* @param environmentVariables to put in {@link ProcessBuilder#environment()}
* @return the exit value of the command
* @throws IOException
* @throws InterruptedException
*/
int executeCommand(List<String> command, File workingDirectory,
Map<String,String> environmentVariables) throws IOException, InterruptedException {
String platform = Loader.getPlatform();
boolean windows = platform.startsWith("windows");
for (int i = 0; i < command.size(); i++) {
String arg = command.get(i);
if (arg == null) {
arg = "";
}
if (arg.trim().isEmpty() && windows) {
// seems to be the only way to pass empty arguments on Windows?
arg = "\"\"";
}
command.set(i, arg);
}

String text = "";
for (String s : command) {
boolean hasSpaces = s.indexOf(" ") > 0 || s.isEmpty();
if (hasSpaces) {
text += windows ? "\"" : "'";
}
text += s;
if (hasSpaces) {
text += windows ? "\"" : "'";
}
text += " ";
}
logger.info(text);

ProcessBuilder pb = new ProcessBuilder(command);
if (workingDirectory != null) {
pb.directory(workingDirectory);
}
if (environmentVariables != null) {
for (Map.Entry<String,String> e : environmentVariables.entrySet()) {
if (e.getKey() != null && e.getValue() != null) {
pb.environment().put(e.getKey(), e.getValue());
}
}
}
return pb.inheritIO().start().waitFor();
}

/**
* Launches and waits for the native compiler to produce a native shared library.
*
Expand Down Expand Up @@ -491,7 +436,7 @@ int compile(String[] sourceFilenames, String outputFilename, ClassProperties pro

// Use the library output path as the working directory so that all
// build files, including intermediate ones from MSVC, are dumped there
return executeCommand(command, workingDirectory, environmentVariables);
return commandExecutor.executeCommand(command, workingDirectory, environmentVariables);
}

/**
Expand Down Expand Up @@ -734,6 +679,7 @@ public Builder(Logger logger) {
classScanner = new ClassScanner(logger, new ArrayList<Class>(),
new UserClassLoader(Thread.currentThread().getContextClassLoader()));
compilerOptions = new ArrayList<String>();
commandExecutor = new CommandExecutor(logger);
}

/** Logger where to send debug, info, warning, and error messages. */
Expand Down Expand Up @@ -774,6 +720,8 @@ public Builder(Logger logger) {
Map<String,String> environmentVariables = null;
/** Contains additional command line options from the user for the native compiler. */
Collection<String> compilerOptions = null;
/** An alternative CommandExecutor to use to execute commands. */
CommandExecutor commandExecutor = null;

/** Splits argument with {@link File#pathSeparator} and appends result to paths of the {@link #classScanner}. */
public Builder classPaths(String classPaths) {
Expand Down Expand Up @@ -898,6 +846,26 @@ public Builder property(String key, String value) {
}
return this;
}
/** Returns {@code properties}. */
public Properties getProperties() {
return properties;
}
/** Returns {@code properties.getProperty(key)}. */
public String getProperty(String key) {
return properties.getProperty(key);
}
/** Adds values to a given property key, seperating them with "platform.path.separator". */
public Builder addProperty(String key, String... values) {
if (values != null && values.length > 0) {
String separator = properties.getProperty("platform.path.separator");
String v = properties.getProperty(key, "");
for (String s : values) {
v += v.length() == 0 || v.endsWith(separator) ? v + s : v + separator + s;
}
properties.setProperty(key, v);
}
return this;
}
/** Requests the {@link #classScanner} to add a class or all classes from a package.
* A {@code null} argument indicates the unnamed package. */
public Builder classesOrPackages(String ... classesOrPackages) throws IOException, ClassNotFoundException, NoClassDefFoundError {
Expand Down Expand Up @@ -935,6 +903,11 @@ public Builder compilerOptions(String ... options) {
}
return this;
}
/** Sets the {@link #commandExecutor} field to the argument. */
public Builder commandExecutor(CommandExecutor commandExecutor) {
this.commandExecutor = commandExecutor;
return this;
}

/**
* Starts the build process and returns an array of {@link File} produced.
Expand Down Expand Up @@ -971,6 +944,9 @@ public File[] build() throws IOException, InterruptedException, ParserException
libProperties = new ClassProperties(properties);
}
includeJavaPaths(libProperties, header);
if (environmentVariables == null) {
environmentVariables = new HashMap<String,String>();
}
for (Map.Entry<String, List<String>> entry : libProperties.entrySet()) {
String key = entry.getKey();
key = key.toUpperCase().replace('.', '_');
Expand Down Expand Up @@ -1021,7 +997,7 @@ public File[] build() throws IOException, InterruptedException, ParserException
environmentVariables.put("BUILD_PATH_SEPARATOR", File.pathSeparator);
}
}
int exitValue = executeCommand(command, workingDirectory, environmentVariables);
int exitValue = commandExecutor.executeCommand(command, workingDirectory, environmentVariables);
if (exitValue != 0) {
throw new RuntimeException("Process exited with an error: " + exitValue);
}
Expand Down Expand Up @@ -1346,7 +1322,7 @@ public static void main(String[] args) throws Exception {
}
command.add(paths);
command.add(arg);
int exitValue = builder.executeCommand(command, builder.workingDirectory, builder.environmentVariables);
int exitValue = builder.commandExecutor.executeCommand(command, builder.workingDirectory, builder.environmentVariables);
if (exitValue != 0) {
throw new RuntimeException("Could not compile " + arg + ": " + exitValue);
}
Expand Down Expand Up @@ -1393,7 +1369,7 @@ public static void main(String[] args) throws Exception {
command.add(paths);
command.add(c.getCanonicalName());
command.addAll(Arrays.asList(execArgs));
System.exit(builder.executeCommand(command, builder.workingDirectory, builder.environmentVariables));
System.exit(builder.commandExecutor.executeCommand(command, builder.workingDirectory, builder.environmentVariables));
}
}
}
97 changes: 97 additions & 0 deletions src/main/java/org/bytedeco/javacpp/tools/CommandExecutor.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
/*
* Copyright (C) 2020 Samuel Audet
*
* Licensed either under the Apache License, Version 2.0, or (at your option)
* under the terms of the GNU General Public License as published by
* the Free Software Foundation (subject to the "Classpath" exception),
* either version 2, or any later version (collectively, 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
* http://www.gnu.org/licenses/
* http://www.gnu.org/software/classpath/license.html
*
* or as provided in the LICENSE.txt file that accompanied this code.
* 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 org.bytedeco.javacpp.tools;

import java.io.File;
import java.io.IOException;
import java.util.List;
import java.util.Map;
import org.bytedeco.javacpp.Loader;

/**
* A wrapper for ProcessBuilder that can be overridden easily for frameworks like Gradle that don't support it well.
*
* @author Samuel Audet
*/
public class CommandExecutor {
final Logger logger;

public CommandExecutor(Logger logger) {
this.logger = logger;
}

/**
* Executes a command with {@link ProcessBuilder}, but also logs the call
* and redirects its input and output to our process.
*
* @param command to have {@link ProcessBuilder} execute
* @param workingDirectory to pass to {@link ProcessBuilder#directory()}
* @param environmentVariables to put in {@link ProcessBuilder#environment()}
* @return the exit value of the command
* @throws IOException
* @throws InterruptedException
*/
public int executeCommand(List<String> command, File workingDirectory,
Map<String,String> environmentVariables) throws IOException, InterruptedException {
String platform = Loader.getPlatform();
boolean windows = platform.startsWith("windows");
for (int i = 0; i < command.size(); i++) {
String arg = command.get(i);
if (arg == null) {
arg = "";
}
if (arg.trim().isEmpty() && windows) {
// seems to be the only way to pass empty arguments on Windows?
arg = "\"\"";
}
command.set(i, arg);
}

String text = "";
for (String s : command) {
boolean hasSpaces = s.indexOf(" ") > 0 || s.isEmpty();
if (hasSpaces) {
text += windows ? "\"" : "'";
}
text += s;
if (hasSpaces) {
text += windows ? "\"" : "'";
}
text += " ";
}
logger.info(text);

ProcessBuilder pb = new ProcessBuilder(command);
if (workingDirectory != null) {
pb.directory(workingDirectory);
}
if (environmentVariables != null) {
for (Map.Entry<String,String> e : environmentVariables.entrySet()) {
if (e.getKey() != null && e.getValue() != null) {
pb.environment().put(e.getKey(), e.getValue());
}
}
}
return pb.inheritIO().start().waitFor();
}
}

0 comments on commit a970348

Please sign in to comment.