[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

[jaikiran]

Github user jaikiran closed the pull request at:

https://github.com/apache/ant/pull/60


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #:

[twogee]

Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/5aa1e8eff81ad6005e62587cdaaa962fce1105ea#commitcomment-27929054
  
In src/tests/junit/org/apache/tools/ant/taskdefs/RmicAdvancedTest.java:
In src/tests/junit/org/apache/tools/ant/taskdefs/RmicAdvancedTest.java on 
line 476:
I meant "instead of `try { ...; fail ... } catch { ... }`"; you can still 
put expected exception rule inside an `if {} else {}` or use assume to get rid 
of that if/else.


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #:

[bodewig]

Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/5aa1e8eff81ad6005e62587cdaaa962fce1105ea#commitcomment-27928590
  
In src/tests/junit/org/apache/tools/ant/taskdefs/RmicAdvancedTest.java:
In src/tests/junit/org/apache/tools/ant/taskdefs/RmicAdvancedTest.java on 
line 476:
combined with "expect this exception only when run on Java 11 and expect no 
exception otherwise"?


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #:

[twogee]

Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/e06168ca78b819a21924f3716b99e3621b4855ef#commitcomment-27927813
  
It's not quite appropriate here, but we need a new release of Ivy... :cry: 


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #:

[twogee]

Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/5aa1e8eff81ad6005e62587cdaaa962fce1105ea#commitcomment-27927763
  
In src/tests/junit/org/apache/tools/ant/taskdefs/RmicAdvancedTest.java:
In src/tests/junit/org/apache/tools/ant/taskdefs/RmicAdvancedTest.java on 
line 476:
This the old way to do it, ExpectedException should be used instead.


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

[jaikiran]

Github user jaikiran commented on a diff in the pull request:

https://github.com/apache/ant/pull/60#discussion_r172044906
  
--- Diff: 
src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/JUnitLauncherTask.java
 ---
@@ -0,0 +1,508 @@
+package org.apache.tools.ant.taskdefs.optional.junitlauncher;
+
+import org.apache.tools.ant.AntClassLoader;
+import org.apache.tools.ant.BuildException;
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.Task;
+import org.apache.tools.ant.types.Path;
+import org.apache.tools.ant.util.FileUtils;
+import org.apache.tools.ant.util.KeepAliveOutputStream;
+import org.junit.platform.launcher.Launcher;
+import org.junit.platform.launcher.LauncherDiscoveryRequest;
+import org.junit.platform.launcher.TestExecutionListener;
+import org.junit.platform.launcher.TestPlan;
+import org.junit.platform.launcher.core.LauncherFactory;
+import org.junit.platform.launcher.listeners.SummaryGeneratingListener;
+import org.junit.platform.launcher.listeners.TestExecutionSummary;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
+import java.io.PrintStream;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * An Ant {@link Task} responsible for launching the JUnit platform for 
running tests.
+ * This requires a minimum of JUnit 5, since that's the version in which 
the JUnit platform launcher
+ * APIs were introduced.
+ * 
+ * This task in itself doesn't run the JUnit tests, instead the sole 
responsibility of
+ * this task is to setup the JUnit platform launcher, build requests, 
launch those requests and then parse the
+ * result of the execution to present in a way that's been configured on 
this Ant task.
+ * 
+ * 
+ * Furthermore, this task allows users control over which classes to 
select for passing on to the JUnit 5
+ * platform for test execution. It however, is solely the JUnit 5 
platform, backed by test engines that
+ * decide and execute the tests.
+ *
+ * @see https://junit.org/junit5/;>JUnit 5 documentation for 
more details
+ * on how JUnit manages the platform and the test engines.
+ */
+public class JUnitLauncherTask extends Task {
+
+private Path classPath;
+private boolean haltOnFailure;
+private String failureProperty;
+private final List tests = new ArrayList<>();
+private final List listeners = new ArrayList<>();
+
+public JUnitLauncherTask() {
+}
+
+@Override
+public void execute() throws BuildException {
+final ClassLoader previousClassLoader = 
Thread.currentThread().getContextClassLoader();
+try {
+final ClassLoader executionCL = 
createClassLoaderForTestExecution();
+Thread.currentThread().setContextClassLoader(executionCL);
+final Launcher launcher = LauncherFactory.create();
+final List requests = buildTestRequests();
+for (final TestRequest testRequest : requests) {
+try {
+final TestDefinition test = testRequest.getOwner();
+final LauncherDiscoveryRequest request = 
testRequest.getDiscoveryRequest().build();
+final List 
testExecutionListeners = new ArrayList<>();
+// a listener that we always put at the front of list 
of listeners
+// for this request.
+final Listener firstListener = new Listener();
+// we always enroll the summary generating listener, 
to the request, so that we
+// get to use some of the details of the summary for 
our further decision making
+testExecutionListeners.add(firstListener);
+
testExecutionListeners.addAll(getListeners(testRequest, executionCL));
+final PrintStream originalSysOut = System.out;
+final PrintStream originalSysErr = System.err;
+try {
+firstListener.switchedSysOutHandle = 
trySwitchSysOut(testRequest);
+firstListener.switchedSysErrHandle = 
trySwitchSysErr(testRequest);
+launcher.execute(request, 

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

[jaikiran]

Github user jaikiran commented on a diff in the pull request:

https://github.com/apache/ant/pull/60#discussion_r172044717
  
--- Diff: 
src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/AbstractJUnitResultFormatter.java
 ---
@@ -0,0 +1,295 @@
+package org.apache.tools.ant.taskdefs.optional.junitlauncher;
+
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.Task;
+import org.apache.tools.ant.util.FileUtils;
+import org.junit.platform.engine.TestSource;
+import org.junit.platform.engine.support.descriptor.ClassSource;
+import org.junit.platform.launcher.TestIdentifier;
+import org.junit.platform.launcher.TestPlan;
+
+import java.io.BufferedReader;
+import java.io.ByteArrayInputStream;
+import java.io.Closeable;
+import java.io.FileOutputStream;
+import java.io.FileReader;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.Reader;
+import java.io.Writer;
+import java.nio.BufferOverflowException;
+import java.nio.ByteBuffer;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Objects;
+import java.util.Optional;
+
+/**
+ * Contains some common behaviour that's used by our internal {@link 
TestResultFormatter}s
+ */
+abstract class AbstractJUnitResultFormatter implements TestResultFormatter 
{
+
+
+protected static String NEW_LINE = 
System.getProperty("line.separator");
+protected Task task;
+
+private SysOutErrContentStore sysOutStore;
+private SysOutErrContentStore sysErrStore;
+
+@Override
+public void sysOutAvailable(final byte[] data) {
+if (this.sysOutStore == null) {
+this.sysOutStore = new SysOutErrContentStore(true);
+}
+try {
+this.sysOutStore.store(data);
+} catch (IOException e) {
+handleException(e);
+return;
+}
+}
+
+@Override
+public void sysErrAvailable(final byte[] data) {
+if (this.sysErrStore == null) {
+this.sysErrStore = new SysOutErrContentStore(false);
+}
+try {
+this.sysErrStore.store(data);
+} catch (IOException e) {
+handleException(e);
+return;
+}
+}
+
+@Override
+public void setExecutingTask(final Task task) {
+this.task = task;
+}
+
+/**
+ * @return Returns true if there's any stdout data, that was generated 
during the
+ * tests, is available for use. Else returns false.
+ */
+boolean hasSysOut() {
+return this.sysOutStore != null && this.sysOutStore.hasData();
+}
+
+/**
+ * @return Returns true if there's any stderr data, that was generated 
during the
+ * tests, is available for use. Else returns false.
+ */
+boolean hasSysErr() {
+return this.sysErrStore != null && this.sysErrStore.hasData();
+}
+
+/**
+ * @return Returns a {@link Reader} for reading any stdout data that 
was generated
+ * during the test execution. It is expected that the {@link 
#hasSysOut()} be first
+ * called to see if any such data is available and only if there is, 
then this method
+ * be called
+ * @throws IOException If there's any I/O problem while creating the 
{@link Reader}
+ */
+Reader getSysOutReader() throws IOException {
+return this.sysOutStore.getReader();
+}
+
+/**
+ * @return Returns a {@link Reader} for reading any stderr data that 
was generated
+ * during the test execution. It is expected that the {@link 
#hasSysErr()} be first
+ * called to see if any such data is available and only if there is, 
then this method
+ * be called
+ * @throws IOException If there's any I/O problem while creating the 
{@link Reader}
+ */
+Reader getSysErrReader() throws IOException {
+return this.sysErrStore.getReader();
+}
+
+/**
+ * Writes out any stdout data that was generated during the
+ * test execution. If there was no such data then this method just 
returns.
+ *
+ * @param writer The {@link Writer} to use. Cannot be null.
+ * @throws IOException If any I/O problem occurs during writing the 
data
+ */
+void writeSysOut(final Writer writer) throws IOException {
+Objects.requireNonNull(writer, "Writer cannot be null");
+this.writeFrom(this.sysOutStore, writer);
+}
+
+/**
+ * Writes out any stderr data that was generated during the
+ * test execution. If there was no such data then this 

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

[jaikiran]

Github user jaikiran commented on a diff in the pull request:

https://github.com/apache/ant/pull/60#discussion_r172044665
  
--- Diff: 
src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/AbstractJUnitResultFormatter.java
 ---
@@ -0,0 +1,295 @@
+package org.apache.tools.ant.taskdefs.optional.junitlauncher;
+
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.Task;
+import org.apache.tools.ant.util.FileUtils;
+import org.junit.platform.engine.TestSource;
+import org.junit.platform.engine.support.descriptor.ClassSource;
+import org.junit.platform.launcher.TestIdentifier;
+import org.junit.platform.launcher.TestPlan;
+
+import java.io.BufferedReader;
+import java.io.ByteArrayInputStream;
+import java.io.Closeable;
+import java.io.FileOutputStream;
+import java.io.FileReader;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.Reader;
+import java.io.Writer;
+import java.nio.BufferOverflowException;
+import java.nio.ByteBuffer;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Objects;
+import java.util.Optional;
+
+/**
+ * Contains some common behaviour that's used by our internal {@link 
TestResultFormatter}s
+ */
+abstract class AbstractJUnitResultFormatter implements TestResultFormatter 
{
+
+
+protected static String NEW_LINE = 
System.getProperty("line.separator");
+protected Task task;
+
+private SysOutErrContentStore sysOutStore;
+private SysOutErrContentStore sysErrStore;
+
+@Override
+public void sysOutAvailable(final byte[] data) {
+if (this.sysOutStore == null) {
+this.sysOutStore = new SysOutErrContentStore(true);
+}
+try {
+this.sysOutStore.store(data);
+} catch (IOException e) {
+handleException(e);
+return;
+}
+}
+
+@Override
+public void sysErrAvailable(final byte[] data) {
+if (this.sysErrStore == null) {
+this.sysErrStore = new SysOutErrContentStore(false);
+}
+try {
+this.sysErrStore.store(data);
+} catch (IOException e) {
+handleException(e);
+return;
+}
+}
+
+@Override
+public void setExecutingTask(final Task task) {
+this.task = task;
+}
+
+/**
+ * @return Returns true if there's any stdout data, that was generated 
during the
+ * tests, is available for use. Else returns false.
+ */
+boolean hasSysOut() {
+return this.sysOutStore != null && this.sysOutStore.hasData();
+}
+
+/**
+ * @return Returns true if there's any stderr data, that was generated 
during the
+ * tests, is available for use. Else returns false.
+ */
+boolean hasSysErr() {
+return this.sysErrStore != null && this.sysErrStore.hasData();
+}
+
+/**
+ * @return Returns a {@link Reader} for reading any stdout data that 
was generated
+ * during the test execution. It is expected that the {@link 
#hasSysOut()} be first
+ * called to see if any such data is available and only if there is, 
then this method
+ * be called
+ * @throws IOException If there's any I/O problem while creating the 
{@link Reader}
+ */
+Reader getSysOutReader() throws IOException {
+return this.sysOutStore.getReader();
+}
+
+/**
+ * @return Returns a {@link Reader} for reading any stderr data that 
was generated
+ * during the test execution. It is expected that the {@link 
#hasSysErr()} be first
+ * called to see if any such data is available and only if there is, 
then this method
+ * be called
+ * @throws IOException If there's any I/O problem while creating the 
{@link Reader}
+ */
+Reader getSysErrReader() throws IOException {
+return this.sysErrStore.getReader();
+}
+
+/**
+ * Writes out any stdout data that was generated during the
+ * test execution. If there was no such data then this method just 
returns.
+ *
+ * @param writer The {@link Writer} to use. Cannot be null.
+ * @throws IOException If any I/O problem occurs during writing the 
data
+ */
+void writeSysOut(final Writer writer) throws IOException {
+Objects.requireNonNull(writer, "Writer cannot be null");
+this.writeFrom(this.sysOutStore, writer);
+}
+
+/**
+ * Writes out any stderr data that was generated during the
+ * test execution. If there was no such data then this 

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

[jaikiran]

Github user jaikiran commented on a diff in the pull request:

https://github.com/apache/ant/pull/60#discussion_r172044509
  
--- Diff: 
src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/AbstractJUnitResultFormatter.java
 ---
@@ -0,0 +1,295 @@
+package org.apache.tools.ant.taskdefs.optional.junitlauncher;
+
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.Task;
+import org.apache.tools.ant.util.FileUtils;
+import org.junit.platform.engine.TestSource;
+import org.junit.platform.engine.support.descriptor.ClassSource;
+import org.junit.platform.launcher.TestIdentifier;
+import org.junit.platform.launcher.TestPlan;
+
+import java.io.BufferedReader;
+import java.io.ByteArrayInputStream;
+import java.io.Closeable;
+import java.io.FileOutputStream;
+import java.io.FileReader;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.Reader;
+import java.io.Writer;
+import java.nio.BufferOverflowException;
+import java.nio.ByteBuffer;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Objects;
+import java.util.Optional;
+
+/**
+ * Contains some common behaviour that's used by our internal {@link 
TestResultFormatter}s
+ */
+abstract class AbstractJUnitResultFormatter implements TestResultFormatter 
{
+
+
+protected static String NEW_LINE = 
System.getProperty("line.separator");
+protected Task task;
+
+private SysOutErrContentStore sysOutStore;
+private SysOutErrContentStore sysErrStore;
+
+@Override
+public void sysOutAvailable(final byte[] data) {
+if (this.sysOutStore == null) {
+this.sysOutStore = new SysOutErrContentStore(true);
+}
+try {
+this.sysOutStore.store(data);
+} catch (IOException e) {
+handleException(e);
+return;
+}
+}
+
+@Override
+public void sysErrAvailable(final byte[] data) {
+if (this.sysErrStore == null) {
+this.sysErrStore = new SysOutErrContentStore(false);
+}
+try {
+this.sysErrStore.store(data);
+} catch (IOException e) {
+handleException(e);
+return;
+}
+}
+
+@Override
+public void setExecutingTask(final Task task) {
+this.task = task;
+}
+
+/**
+ * @return Returns true if there's any stdout data, that was generated 
during the
+ * tests, is available for use. Else returns false.
+ */
+boolean hasSysOut() {
+return this.sysOutStore != null && this.sysOutStore.hasData();
+}
+
+/**
+ * @return Returns true if there's any stderr data, that was generated 
during the
+ * tests, is available for use. Else returns false.
+ */
+boolean hasSysErr() {
+return this.sysErrStore != null && this.sysErrStore.hasData();
+}
+
+/**
+ * @return Returns a {@link Reader} for reading any stdout data that 
was generated
+ * during the test execution. It is expected that the {@link 
#hasSysOut()} be first
+ * called to see if any such data is available and only if there is, 
then this method
+ * be called
+ * @throws IOException If there's any I/O problem while creating the 
{@link Reader}
+ */
+Reader getSysOutReader() throws IOException {
+return this.sysOutStore.getReader();
+}
+
+/**
+ * @return Returns a {@link Reader} for reading any stderr data that 
was generated
+ * during the test execution. It is expected that the {@link 
#hasSysErr()} be first
+ * called to see if any such data is available and only if there is, 
then this method
+ * be called
+ * @throws IOException If there's any I/O problem while creating the 
{@link Reader}
+ */
+Reader getSysErrReader() throws IOException {
+return this.sysErrStore.getReader();
+}
+
+/**
+ * Writes out any stdout data that was generated during the
+ * test execution. If there was no such data then this method just 
returns.
+ *
+ * @param writer The {@link Writer} to use. Cannot be null.
+ * @throws IOException If any I/O problem occurs during writing the 
data
+ */
+void writeSysOut(final Writer writer) throws IOException {
+Objects.requireNonNull(writer, "Writer cannot be null");
+this.writeFrom(this.sysOutStore, writer);
+}
+
+/**
+ * Writes out any stderr data that was generated during the
+ * test execution. If there was no such data then this 

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

[bodewig]

Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/60#discussion_r172043964
  
--- Diff: 
src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/JUnitLauncherTask.java
 ---
@@ -0,0 +1,508 @@
+package org.apache.tools.ant.taskdefs.optional.junitlauncher;
+
+import org.apache.tools.ant.AntClassLoader;
+import org.apache.tools.ant.BuildException;
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.Task;
+import org.apache.tools.ant.types.Path;
+import org.apache.tools.ant.util.FileUtils;
+import org.apache.tools.ant.util.KeepAliveOutputStream;
+import org.junit.platform.launcher.Launcher;
+import org.junit.platform.launcher.LauncherDiscoveryRequest;
+import org.junit.platform.launcher.TestExecutionListener;
+import org.junit.platform.launcher.TestPlan;
+import org.junit.platform.launcher.core.LauncherFactory;
+import org.junit.platform.launcher.listeners.SummaryGeneratingListener;
+import org.junit.platform.launcher.listeners.TestExecutionSummary;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
+import java.io.PrintStream;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * An Ant {@link Task} responsible for launching the JUnit platform for 
running tests.
+ * This requires a minimum of JUnit 5, since that's the version in which 
the JUnit platform launcher
+ * APIs were introduced.
+ * 
+ * This task in itself doesn't run the JUnit tests, instead the sole 
responsibility of
+ * this task is to setup the JUnit platform launcher, build requests, 
launch those requests and then parse the
+ * result of the execution to present in a way that's been configured on 
this Ant task.
+ * 
+ * 
+ * Furthermore, this task allows users control over which classes to 
select for passing on to the JUnit 5
+ * platform for test execution. It however, is solely the JUnit 5 
platform, backed by test engines that
+ * decide and execute the tests.
+ *
+ * @see https://junit.org/junit5/;>JUnit 5 documentation for 
more details
+ * on how JUnit manages the platform and the test engines.
+ */
+public class JUnitLauncherTask extends Task {
+
+private Path classPath;
+private boolean haltOnFailure;
+private String failureProperty;
+private final List tests = new ArrayList<>();
+private final List listeners = new ArrayList<>();
+
+public JUnitLauncherTask() {
+}
+
+@Override
+public void execute() throws BuildException {
+final ClassLoader previousClassLoader = 
Thread.currentThread().getContextClassLoader();
+try {
+final ClassLoader executionCL = 
createClassLoaderForTestExecution();
+Thread.currentThread().setContextClassLoader(executionCL);
+final Launcher launcher = LauncherFactory.create();
+final List requests = buildTestRequests();
+for (final TestRequest testRequest : requests) {
+try {
+final TestDefinition test = testRequest.getOwner();
+final LauncherDiscoveryRequest request = 
testRequest.getDiscoveryRequest().build();
+final List 
testExecutionListeners = new ArrayList<>();
+// a listener that we always put at the front of list 
of listeners
+// for this request.
+final Listener firstListener = new Listener();
+// we always enroll the summary generating listener, 
to the request, so that we
+// get to use some of the details of the summary for 
our further decision making
+testExecutionListeners.add(firstListener);
+
testExecutionListeners.addAll(getListeners(testRequest, executionCL));
+final PrintStream originalSysOut = System.out;
+final PrintStream originalSysErr = System.err;
+try {
+firstListener.switchedSysOutHandle = 
trySwitchSysOut(testRequest);
+firstListener.switchedSysErrHandle = 
trySwitchSysErr(testRequest);
+launcher.execute(request, 

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

[bodewig]

Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/60#discussion_r172043835
  
--- Diff: 
src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/AbstractJUnitResultFormatter.java
 ---
@@ -0,0 +1,295 @@
+package org.apache.tools.ant.taskdefs.optional.junitlauncher;
+
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.Task;
+import org.apache.tools.ant.util.FileUtils;
+import org.junit.platform.engine.TestSource;
+import org.junit.platform.engine.support.descriptor.ClassSource;
+import org.junit.platform.launcher.TestIdentifier;
+import org.junit.platform.launcher.TestPlan;
+
+import java.io.BufferedReader;
+import java.io.ByteArrayInputStream;
+import java.io.Closeable;
+import java.io.FileOutputStream;
+import java.io.FileReader;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.Reader;
+import java.io.Writer;
+import java.nio.BufferOverflowException;
+import java.nio.ByteBuffer;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Objects;
+import java.util.Optional;
+
+/**
+ * Contains some common behaviour that's used by our internal {@link 
TestResultFormatter}s
+ */
+abstract class AbstractJUnitResultFormatter implements TestResultFormatter 
{
+
+
+protected static String NEW_LINE = 
System.getProperty("line.separator");
+protected Task task;
+
+private SysOutErrContentStore sysOutStore;
+private SysOutErrContentStore sysErrStore;
+
+@Override
+public void sysOutAvailable(final byte[] data) {
+if (this.sysOutStore == null) {
+this.sysOutStore = new SysOutErrContentStore(true);
+}
+try {
+this.sysOutStore.store(data);
+} catch (IOException e) {
+handleException(e);
+return;
+}
+}
+
+@Override
+public void sysErrAvailable(final byte[] data) {
+if (this.sysErrStore == null) {
+this.sysErrStore = new SysOutErrContentStore(false);
+}
+try {
+this.sysErrStore.store(data);
+} catch (IOException e) {
+handleException(e);
+return;
+}
+}
+
+@Override
+public void setExecutingTask(final Task task) {
+this.task = task;
+}
+
+/**
+ * @return Returns true if there's any stdout data, that was generated 
during the
+ * tests, is available for use. Else returns false.
+ */
+boolean hasSysOut() {
+return this.sysOutStore != null && this.sysOutStore.hasData();
+}
+
+/**
+ * @return Returns true if there's any stderr data, that was generated 
during the
+ * tests, is available for use. Else returns false.
+ */
+boolean hasSysErr() {
+return this.sysErrStore != null && this.sysErrStore.hasData();
+}
+
+/**
+ * @return Returns a {@link Reader} for reading any stdout data that 
was generated
+ * during the test execution. It is expected that the {@link 
#hasSysOut()} be first
+ * called to see if any such data is available and only if there is, 
then this method
+ * be called
+ * @throws IOException If there's any I/O problem while creating the 
{@link Reader}
+ */
+Reader getSysOutReader() throws IOException {
+return this.sysOutStore.getReader();
+}
+
+/**
+ * @return Returns a {@link Reader} for reading any stderr data that 
was generated
+ * during the test execution. It is expected that the {@link 
#hasSysErr()} be first
+ * called to see if any such data is available and only if there is, 
then this method
+ * be called
+ * @throws IOException If there's any I/O problem while creating the 
{@link Reader}
+ */
+Reader getSysErrReader() throws IOException {
+return this.sysErrStore.getReader();
+}
+
+/**
+ * Writes out any stdout data that was generated during the
+ * test execution. If there was no such data then this method just 
returns.
+ *
+ * @param writer The {@link Writer} to use. Cannot be null.
+ * @throws IOException If any I/O problem occurs during writing the 
data
+ */
+void writeSysOut(final Writer writer) throws IOException {
+Objects.requireNonNull(writer, "Writer cannot be null");
+this.writeFrom(this.sysOutStore, writer);
+}
+
+/**
+ * Writes out any stderr data that was generated during the
+ * test execution. If there was no such data then this 

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

[bodewig]

Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/60#discussion_r172043778
  
--- Diff: 
src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/AbstractJUnitResultFormatter.java
 ---
@@ -0,0 +1,295 @@
+package org.apache.tools.ant.taskdefs.optional.junitlauncher;
+
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.Task;
+import org.apache.tools.ant.util.FileUtils;
+import org.junit.platform.engine.TestSource;
+import org.junit.platform.engine.support.descriptor.ClassSource;
+import org.junit.platform.launcher.TestIdentifier;
+import org.junit.platform.launcher.TestPlan;
+
+import java.io.BufferedReader;
+import java.io.ByteArrayInputStream;
+import java.io.Closeable;
+import java.io.FileOutputStream;
+import java.io.FileReader;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.Reader;
+import java.io.Writer;
+import java.nio.BufferOverflowException;
+import java.nio.ByteBuffer;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Objects;
+import java.util.Optional;
+
+/**
+ * Contains some common behaviour that's used by our internal {@link 
TestResultFormatter}s
+ */
+abstract class AbstractJUnitResultFormatter implements TestResultFormatter 
{
+
+
+protected static String NEW_LINE = 
System.getProperty("line.separator");
+protected Task task;
+
+private SysOutErrContentStore sysOutStore;
+private SysOutErrContentStore sysErrStore;
+
+@Override
+public void sysOutAvailable(final byte[] data) {
+if (this.sysOutStore == null) {
+this.sysOutStore = new SysOutErrContentStore(true);
+}
+try {
+this.sysOutStore.store(data);
+} catch (IOException e) {
+handleException(e);
+return;
+}
+}
+
+@Override
+public void sysErrAvailable(final byte[] data) {
+if (this.sysErrStore == null) {
+this.sysErrStore = new SysOutErrContentStore(false);
+}
+try {
+this.sysErrStore.store(data);
+} catch (IOException e) {
+handleException(e);
+return;
+}
+}
+
+@Override
+public void setExecutingTask(final Task task) {
+this.task = task;
+}
+
+/**
+ * @return Returns true if there's any stdout data, that was generated 
during the
+ * tests, is available for use. Else returns false.
+ */
+boolean hasSysOut() {
+return this.sysOutStore != null && this.sysOutStore.hasData();
+}
+
+/**
+ * @return Returns true if there's any stderr data, that was generated 
during the
+ * tests, is available for use. Else returns false.
+ */
+boolean hasSysErr() {
+return this.sysErrStore != null && this.sysErrStore.hasData();
+}
+
+/**
+ * @return Returns a {@link Reader} for reading any stdout data that 
was generated
+ * during the test execution. It is expected that the {@link 
#hasSysOut()} be first
+ * called to see if any such data is available and only if there is, 
then this method
+ * be called
+ * @throws IOException If there's any I/O problem while creating the 
{@link Reader}
+ */
+Reader getSysOutReader() throws IOException {
+return this.sysOutStore.getReader();
+}
+
+/**
+ * @return Returns a {@link Reader} for reading any stderr data that 
was generated
+ * during the test execution. It is expected that the {@link 
#hasSysErr()} be first
+ * called to see if any such data is available and only if there is, 
then this method
+ * be called
+ * @throws IOException If there's any I/O problem while creating the 
{@link Reader}
+ */
+Reader getSysErrReader() throws IOException {
+return this.sysErrStore.getReader();
+}
+
+/**
+ * Writes out any stdout data that was generated during the
+ * test execution. If there was no such data then this method just 
returns.
+ *
+ * @param writer The {@link Writer} to use. Cannot be null.
+ * @throws IOException If any I/O problem occurs during writing the 
data
+ */
+void writeSysOut(final Writer writer) throws IOException {
+Objects.requireNonNull(writer, "Writer cannot be null");
+this.writeFrom(this.sysOutStore, writer);
+}
+
+/**
+ * Writes out any stderr data that was generated during the
+ * test execution. If there was no such data then this 

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

[jaikiran]

Github user jaikiran commented on a diff in the pull request:

https://github.com/apache/ant/pull/60#discussion_r169626791
  
--- Diff: 
src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/JUnitLauncherTask.java
 ---
@@ -0,0 +1,508 @@
+package org.apache.tools.ant.taskdefs.optional.junitlauncher;
+
+import org.apache.tools.ant.AntClassLoader;
+import org.apache.tools.ant.BuildException;
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.Task;
+import org.apache.tools.ant.types.Path;
+import org.apache.tools.ant.util.KeepAliveOutputStream;
+import org.junit.platform.launcher.Launcher;
+import org.junit.platform.launcher.LauncherDiscoveryRequest;
+import org.junit.platform.launcher.TestExecutionListener;
+import org.junit.platform.launcher.TestPlan;
+import org.junit.platform.launcher.core.LauncherFactory;
+import org.junit.platform.launcher.listeners.SummaryGeneratingListener;
+import org.junit.platform.launcher.listeners.TestExecutionSummary;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
+import java.io.PrintStream;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * An Ant {@link Task} responsible for launching the JUnit platform for 
running tests.
+ * This requires a minimum of JUnit 5, since that's the version in which 
the JUnit platform launcher
+ * APIs were introduced.
+ * 
+ * This task in itself doesn't run the JUnit tests, instead the sole 
responsibility of
+ * this task is to setup the JUnit platform launcher, build requests, 
launch those requests and then parse the
+ * result of the execution to present in a way that's been configured on 
this Ant task.
+ * 
+ * 
+ * Furthermore, this task allows users control over which classes to 
select for passing on to the JUnit 5
+ * platform for test execution. It however, is solely the JUnit 5 
platform, backed by test engines that
+ * decide and execute the tests.
+ *
+ * @see https://junit.org/junit5/;>JUnit 5 documentation for 
more details
+ * on how JUnit manages the platform and the test engines.
+ */
+public class JUnitLauncherTask extends Task {
+
+private Path classPath;
+private boolean haltOnFailure;
+private String failureProperty;
+private final List tests = new ArrayList<>();
+private final List listeners = new ArrayList<>();
+
+public JUnitLauncherTask() {
+}
+
+@Override
+public void execute() throws BuildException {
+final ClassLoader previousClassLoader = 
Thread.currentThread().getContextClassLoader();
+try {
+final ClassLoader executionCL = 
createClassLoaderForTestExecution();
+Thread.currentThread().setContextClassLoader(executionCL);
+final Launcher launcher = LauncherFactory.create();
+final List requests = buildTestRequests();
+for (final TestRequest testRequest : requests) {
+try {
+final TestDefinition test = testRequest.getOwner();
+final LauncherDiscoveryRequest request = 
testRequest.getDiscoveryRequest().build();
+final List 
testExecutionListeners = new ArrayList<>();
+// a listener that we always put at the front of list 
of listeners
+// for this request.
+final Listener firstListener = new Listener();
+// we always enroll the summary generating listener, 
to the request, so that we
+// get to use some of the details of the summary for 
our further decision making
+testExecutionListeners.add(firstListener);
+
testExecutionListeners.addAll(getListeners(testRequest, executionCL));
+final PrintStream originalSysOut = System.out;
+final PrintStream originalSysErr = System.err;
+try {
+firstListener.switchedSysOutHandle = 
trySwitchSysOut(testRequest);
+firstListener.switchedSysErrHandle = 
trySwitchSysErr(testRequest);
+launcher.execute(request, 
testExecutionListeners.toArray(new 

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

[jaikiran]

Github user jaikiran commented on a diff in the pull request:

https://github.com/apache/ant/pull/60#discussion_r169625878
  
--- Diff: 
src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/JUnitLauncherTask.java
 ---
@@ -0,0 +1,508 @@
+package org.apache.tools.ant.taskdefs.optional.junitlauncher;
+
+import org.apache.tools.ant.AntClassLoader;
+import org.apache.tools.ant.BuildException;
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.Task;
+import org.apache.tools.ant.types.Path;
+import org.apache.tools.ant.util.KeepAliveOutputStream;
+import org.junit.platform.launcher.Launcher;
+import org.junit.platform.launcher.LauncherDiscoveryRequest;
+import org.junit.platform.launcher.TestExecutionListener;
+import org.junit.platform.launcher.TestPlan;
+import org.junit.platform.launcher.core.LauncherFactory;
+import org.junit.platform.launcher.listeners.SummaryGeneratingListener;
+import org.junit.platform.launcher.listeners.TestExecutionSummary;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
+import java.io.PrintStream;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * An Ant {@link Task} responsible for launching the JUnit platform for 
running tests.
+ * This requires a minimum of JUnit 5, since that's the version in which 
the JUnit platform launcher
+ * APIs were introduced.
+ * 
+ * This task in itself doesn't run the JUnit tests, instead the sole 
responsibility of
+ * this task is to setup the JUnit platform launcher, build requests, 
launch those requests and then parse the
+ * result of the execution to present in a way that's been configured on 
this Ant task.
+ * 
+ * 
+ * Furthermore, this task allows users control over which classes to 
select for passing on to the JUnit 5
+ * platform for test execution. It however, is solely the JUnit 5 
platform, backed by test engines that
+ * decide and execute the tests.
+ *
+ * @see https://junit.org/junit5/;>JUnit 5 documentation for 
more details
+ * on how JUnit manages the platform and the test engines.
+ */
+public class JUnitLauncherTask extends Task {
+
+private Path classPath;
+private boolean haltOnFailure;
+private String failureProperty;
+private final List tests = new ArrayList<>();
+private final List listeners = new ArrayList<>();
+
+public JUnitLauncherTask() {
+}
+
+@Override
+public void execute() throws BuildException {
+final ClassLoader previousClassLoader = 
Thread.currentThread().getContextClassLoader();
+try {
+final ClassLoader executionCL = 
createClassLoaderForTestExecution();
+Thread.currentThread().setContextClassLoader(executionCL);
+final Launcher launcher = LauncherFactory.create();
+final List requests = buildTestRequests();
+for (final TestRequest testRequest : requests) {
+try {
+final TestDefinition test = testRequest.getOwner();
+final LauncherDiscoveryRequest request = 
testRequest.getDiscoveryRequest().build();
+final List 
testExecutionListeners = new ArrayList<>();
+// a listener that we always put at the front of list 
of listeners
+// for this request.
+final Listener firstListener = new Listener();
+// we always enroll the summary generating listener, 
to the request, so that we
+// get to use some of the details of the summary for 
our further decision making
+testExecutionListeners.add(firstListener);
+
testExecutionListeners.addAll(getListeners(testRequest, executionCL));
+final PrintStream originalSysOut = System.out;
+final PrintStream originalSysErr = System.err;
+try {
+firstListener.switchedSysOutHandle = 
trySwitchSysOut(testRequest);
+firstListener.switchedSysErrHandle = 
trySwitchSysErr(testRequest);
+launcher.execute(request, 
testExecutionListeners.toArray(new 

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

[jaikiran]

Github user jaikiran commented on a diff in the pull request:

https://github.com/apache/ant/pull/60#discussion_r169624865
  
--- Diff: 
src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/JUnitLauncherTask.java
 ---
@@ -0,0 +1,508 @@
+package org.apache.tools.ant.taskdefs.optional.junitlauncher;
+
+import org.apache.tools.ant.AntClassLoader;
+import org.apache.tools.ant.BuildException;
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.Task;
+import org.apache.tools.ant.types.Path;
+import org.apache.tools.ant.util.KeepAliveOutputStream;
+import org.junit.platform.launcher.Launcher;
+import org.junit.platform.launcher.LauncherDiscoveryRequest;
+import org.junit.platform.launcher.TestExecutionListener;
+import org.junit.platform.launcher.TestPlan;
+import org.junit.platform.launcher.core.LauncherFactory;
+import org.junit.platform.launcher.listeners.SummaryGeneratingListener;
+import org.junit.platform.launcher.listeners.TestExecutionSummary;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
+import java.io.PrintStream;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * An Ant {@link Task} responsible for launching the JUnit platform for 
running tests.
+ * This requires a minimum of JUnit 5, since that's the version in which 
the JUnit platform launcher
+ * APIs were introduced.
+ * 
+ * This task in itself doesn't run the JUnit tests, instead the sole 
responsibility of
+ * this task is to setup the JUnit platform launcher, build requests, 
launch those requests and then parse the
+ * result of the execution to present in a way that's been configured on 
this Ant task.
+ * 
+ * 
+ * Furthermore, this task allows users control over which classes to 
select for passing on to the JUnit 5
+ * platform for test execution. It however, is solely the JUnit 5 
platform, backed by test engines that
+ * decide and execute the tests.
+ *
+ * @see https://junit.org/junit5/;>JUnit 5 documentation for 
more details
+ * on how JUnit manages the platform and the test engines.
+ */
+public class JUnitLauncherTask extends Task {
+
+private Path classPath;
+private boolean haltOnFailure;
+private String failureProperty;
+private final List tests = new ArrayList<>();
+private final List listeners = new ArrayList<>();
+
+public JUnitLauncherTask() {
+}
+
+@Override
+public void execute() throws BuildException {
+final ClassLoader previousClassLoader = 
Thread.currentThread().getContextClassLoader();
+try {
+final ClassLoader executionCL = 
createClassLoaderForTestExecution();
+Thread.currentThread().setContextClassLoader(executionCL);
+final Launcher launcher = LauncherFactory.create();
+final List requests = buildTestRequests();
+for (final TestRequest testRequest : requests) {
+try {
+final TestDefinition test = testRequest.getOwner();
+final LauncherDiscoveryRequest request = 
testRequest.getDiscoveryRequest().build();
+final List 
testExecutionListeners = new ArrayList<>();
+// a listener that we always put at the front of list 
of listeners
+// for this request.
+final Listener firstListener = new Listener();
+// we always enroll the summary generating listener, 
to the request, so that we
+// get to use some of the details of the summary for 
our further decision making
+testExecutionListeners.add(firstListener);
+
testExecutionListeners.addAll(getListeners(testRequest, executionCL));
+final PrintStream originalSysOut = System.out;
+final PrintStream originalSysErr = System.err;
+try {
+firstListener.switchedSysOutHandle = 
trySwitchSysOut(testRequest);
+firstListener.switchedSysErrHandle = 
trySwitchSysErr(testRequest);
+launcher.execute(request, 
testExecutionListeners.toArray(new 

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

[jaikiran]

Github user jaikiran commented on a diff in the pull request:

https://github.com/apache/ant/pull/60#discussion_r169624837
  
--- Diff: 
src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/JUnitLauncherTask.java
 ---
@@ -0,0 +1,508 @@
+package org.apache.tools.ant.taskdefs.optional.junitlauncher;
+
+import org.apache.tools.ant.AntClassLoader;
+import org.apache.tools.ant.BuildException;
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.Task;
+import org.apache.tools.ant.types.Path;
+import org.apache.tools.ant.util.KeepAliveOutputStream;
+import org.junit.platform.launcher.Launcher;
+import org.junit.platform.launcher.LauncherDiscoveryRequest;
+import org.junit.platform.launcher.TestExecutionListener;
+import org.junit.platform.launcher.TestPlan;
+import org.junit.platform.launcher.core.LauncherFactory;
+import org.junit.platform.launcher.listeners.SummaryGeneratingListener;
+import org.junit.platform.launcher.listeners.TestExecutionSummary;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
+import java.io.PrintStream;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * An Ant {@link Task} responsible for launching the JUnit platform for 
running tests.
+ * This requires a minimum of JUnit 5, since that's the version in which 
the JUnit platform launcher
+ * APIs were introduced.
+ * 
+ * This task in itself doesn't run the JUnit tests, instead the sole 
responsibility of
+ * this task is to setup the JUnit platform launcher, build requests, 
launch those requests and then parse the
+ * result of the execution to present in a way that's been configured on 
this Ant task.
+ * 
+ * 
+ * Furthermore, this task allows users control over which classes to 
select for passing on to the JUnit 5
+ * platform for test execution. It however, is solely the JUnit 5 
platform, backed by test engines that
+ * decide and execute the tests.
+ *
+ * @see https://junit.org/junit5/;>JUnit 5 documentation for 
more details
+ * on how JUnit manages the platform and the test engines.
+ */
+public class JUnitLauncherTask extends Task {
+
+private Path classPath;
+private boolean haltOnFailure;
+private String failureProperty;
+private final List tests = new ArrayList<>();
+private final List listeners = new ArrayList<>();
+
+public JUnitLauncherTask() {
+}
+
+@Override
+public void execute() throws BuildException {
+final ClassLoader previousClassLoader = 
Thread.currentThread().getContextClassLoader();
+try {
+final ClassLoader executionCL = 
createClassLoaderForTestExecution();
+Thread.currentThread().setContextClassLoader(executionCL);
+final Launcher launcher = LauncherFactory.create();
+final List requests = buildTestRequests();
+for (final TestRequest testRequest : requests) {
+try {
+final TestDefinition test = testRequest.getOwner();
+final LauncherDiscoveryRequest request = 
testRequest.getDiscoveryRequest().build();
+final List 
testExecutionListeners = new ArrayList<>();
+// a listener that we always put at the front of list 
of listeners
+// for this request.
+final Listener firstListener = new Listener();
+// we always enroll the summary generating listener, 
to the request, so that we
+// get to use some of the details of the summary for 
our further decision making
+testExecutionListeners.add(firstListener);
+
testExecutionListeners.addAll(getListeners(testRequest, executionCL));
+final PrintStream originalSysOut = System.out;
+final PrintStream originalSysErr = System.err;
+try {
+firstListener.switchedSysOutHandle = 
trySwitchSysOut(testRequest);
+firstListener.switchedSysErrHandle = 
trySwitchSysErr(testRequest);
+launcher.execute(request, 
testExecutionListeners.toArray(new 

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

[jaikiran]

Github user jaikiran commented on a diff in the pull request:

https://github.com/apache/ant/pull/60#discussion_r169624475
  
--- Diff: 
src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/JUnitLauncherTask.java
 ---
@@ -0,0 +1,508 @@
+package org.apache.tools.ant.taskdefs.optional.junitlauncher;
+
+import org.apache.tools.ant.AntClassLoader;
+import org.apache.tools.ant.BuildException;
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.Task;
+import org.apache.tools.ant.types.Path;
+import org.apache.tools.ant.util.KeepAliveOutputStream;
+import org.junit.platform.launcher.Launcher;
+import org.junit.platform.launcher.LauncherDiscoveryRequest;
+import org.junit.platform.launcher.TestExecutionListener;
+import org.junit.platform.launcher.TestPlan;
+import org.junit.platform.launcher.core.LauncherFactory;
+import org.junit.platform.launcher.listeners.SummaryGeneratingListener;
+import org.junit.platform.launcher.listeners.TestExecutionSummary;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
+import java.io.PrintStream;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * An Ant {@link Task} responsible for launching the JUnit platform for 
running tests.
+ * This requires a minimum of JUnit 5, since that's the version in which 
the JUnit platform launcher
+ * APIs were introduced.
+ * 
+ * This task in itself doesn't run the JUnit tests, instead the sole 
responsibility of
+ * this task is to setup the JUnit platform launcher, build requests, 
launch those requests and then parse the
+ * result of the execution to present in a way that's been configured on 
this Ant task.
+ * 
+ * 
+ * Furthermore, this task allows users control over which classes to 
select for passing on to the JUnit 5
+ * platform for test execution. It however, is solely the JUnit 5 
platform, backed by test engines that
+ * decide and execute the tests.
+ *
+ * @see https://junit.org/junit5/;>JUnit 5 documentation for 
more details
+ * on how JUnit manages the platform and the test engines.
+ */
+public class JUnitLauncherTask extends Task {
+
+private Path classPath;
+private boolean haltOnFailure;
+private String failureProperty;
+private final List tests = new ArrayList<>();
+private final List listeners = new ArrayList<>();
+
+public JUnitLauncherTask() {
+}
+
+@Override
+public void execute() throws BuildException {
+final ClassLoader previousClassLoader = 
Thread.currentThread().getContextClassLoader();
+try {
+final ClassLoader executionCL = 
createClassLoaderForTestExecution();
+Thread.currentThread().setContextClassLoader(executionCL);
+final Launcher launcher = LauncherFactory.create();
+final List requests = buildTestRequests();
+for (final TestRequest testRequest : requests) {
+try {
+final TestDefinition test = testRequest.getOwner();
+final LauncherDiscoveryRequest request = 
testRequest.getDiscoveryRequest().build();
+final List 
testExecutionListeners = new ArrayList<>();
+// a listener that we always put at the front of list 
of listeners
+// for this request.
+final Listener firstListener = new Listener();
+// we always enroll the summary generating listener, 
to the request, so that we
+// get to use some of the details of the summary for 
our further decision making
+testExecutionListeners.add(firstListener);
+
testExecutionListeners.addAll(getListeners(testRequest, executionCL));
+final PrintStream originalSysOut = System.out;
+final PrintStream originalSysErr = System.err;
+try {
+firstListener.switchedSysOutHandle = 
trySwitchSysOut(testRequest);
+firstListener.switchedSysErrHandle = 
trySwitchSysErr(testRequest);
+launcher.execute(request, 
testExecutionListeners.toArray(new 

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

[bodewig]

Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/60#discussion_r169248698
  
--- Diff: 
src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/JUnitLauncherTask.java
 ---
@@ -0,0 +1,508 @@
+package org.apache.tools.ant.taskdefs.optional.junitlauncher;
+
+import org.apache.tools.ant.AntClassLoader;
+import org.apache.tools.ant.BuildException;
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.Task;
+import org.apache.tools.ant.types.Path;
+import org.apache.tools.ant.util.KeepAliveOutputStream;
+import org.junit.platform.launcher.Launcher;
+import org.junit.platform.launcher.LauncherDiscoveryRequest;
+import org.junit.platform.launcher.TestExecutionListener;
+import org.junit.platform.launcher.TestPlan;
+import org.junit.platform.launcher.core.LauncherFactory;
+import org.junit.platform.launcher.listeners.SummaryGeneratingListener;
+import org.junit.platform.launcher.listeners.TestExecutionSummary;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
+import java.io.PrintStream;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * An Ant {@link Task} responsible for launching the JUnit platform for 
running tests.
+ * This requires a minimum of JUnit 5, since that's the version in which 
the JUnit platform launcher
+ * APIs were introduced.
+ * 
+ * This task in itself doesn't run the JUnit tests, instead the sole 
responsibility of
+ * this task is to setup the JUnit platform launcher, build requests, 
launch those requests and then parse the
+ * result of the execution to present in a way that's been configured on 
this Ant task.
+ * 
+ * 
+ * Furthermore, this task allows users control over which classes to 
select for passing on to the JUnit 5
+ * platform for test execution. It however, is solely the JUnit 5 
platform, backed by test engines that
+ * decide and execute the tests.
+ *
+ * @see https://junit.org/junit5/;>JUnit 5 documentation for 
more details
+ * on how JUnit manages the platform and the test engines.
+ */
+public class JUnitLauncherTask extends Task {
+
+private Path classPath;
+private boolean haltOnFailure;
+private String failureProperty;
+private final List tests = new ArrayList<>();
+private final List listeners = new ArrayList<>();
+
+public JUnitLauncherTask() {
+}
+
+@Override
+public void execute() throws BuildException {
+final ClassLoader previousClassLoader = 
Thread.currentThread().getContextClassLoader();
+try {
+final ClassLoader executionCL = 
createClassLoaderForTestExecution();
+Thread.currentThread().setContextClassLoader(executionCL);
+final Launcher launcher = LauncherFactory.create();
+final List requests = buildTestRequests();
+for (final TestRequest testRequest : requests) {
+try {
+final TestDefinition test = testRequest.getOwner();
+final LauncherDiscoveryRequest request = 
testRequest.getDiscoveryRequest().build();
+final List 
testExecutionListeners = new ArrayList<>();
+// a listener that we always put at the front of list 
of listeners
+// for this request.
+final Listener firstListener = new Listener();
+// we always enroll the summary generating listener, 
to the request, so that we
+// get to use some of the details of the summary for 
our further decision making
+testExecutionListeners.add(firstListener);
+
testExecutionListeners.addAll(getListeners(testRequest, executionCL));
+final PrintStream originalSysOut = System.out;
+final PrintStream originalSysErr = System.err;
+try {
+firstListener.switchedSysOutHandle = 
trySwitchSysOut(testRequest);
+firstListener.switchedSysErrHandle = 
trySwitchSysErr(testRequest);
+launcher.execute(request, 
testExecutionListeners.toArray(new 

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

[bodewig]

Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/60#discussion_r169248048
  
--- Diff: 
src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/JUnitLauncherTask.java
 ---
@@ -0,0 +1,508 @@
+package org.apache.tools.ant.taskdefs.optional.junitlauncher;
+
+import org.apache.tools.ant.AntClassLoader;
+import org.apache.tools.ant.BuildException;
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.Task;
+import org.apache.tools.ant.types.Path;
+import org.apache.tools.ant.util.KeepAliveOutputStream;
+import org.junit.platform.launcher.Launcher;
+import org.junit.platform.launcher.LauncherDiscoveryRequest;
+import org.junit.platform.launcher.TestExecutionListener;
+import org.junit.platform.launcher.TestPlan;
+import org.junit.platform.launcher.core.LauncherFactory;
+import org.junit.platform.launcher.listeners.SummaryGeneratingListener;
+import org.junit.platform.launcher.listeners.TestExecutionSummary;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
+import java.io.PrintStream;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * An Ant {@link Task} responsible for launching the JUnit platform for 
running tests.
+ * This requires a minimum of JUnit 5, since that's the version in which 
the JUnit platform launcher
+ * APIs were introduced.
+ * 
+ * This task in itself doesn't run the JUnit tests, instead the sole 
responsibility of
+ * this task is to setup the JUnit platform launcher, build requests, 
launch those requests and then parse the
+ * result of the execution to present in a way that's been configured on 
this Ant task.
+ * 
+ * 
+ * Furthermore, this task allows users control over which classes to 
select for passing on to the JUnit 5
+ * platform for test execution. It however, is solely the JUnit 5 
platform, backed by test engines that
+ * decide and execute the tests.
+ *
+ * @see https://junit.org/junit5/;>JUnit 5 documentation for 
more details
+ * on how JUnit manages the platform and the test engines.
+ */
+public class JUnitLauncherTask extends Task {
+
+private Path classPath;
+private boolean haltOnFailure;
+private String failureProperty;
+private final List tests = new ArrayList<>();
+private final List listeners = new ArrayList<>();
+
+public JUnitLauncherTask() {
+}
+
+@Override
+public void execute() throws BuildException {
+final ClassLoader previousClassLoader = 
Thread.currentThread().getContextClassLoader();
+try {
+final ClassLoader executionCL = 
createClassLoaderForTestExecution();
+Thread.currentThread().setContextClassLoader(executionCL);
+final Launcher launcher = LauncherFactory.create();
+final List requests = buildTestRequests();
+for (final TestRequest testRequest : requests) {
+try {
+final TestDefinition test = testRequest.getOwner();
+final LauncherDiscoveryRequest request = 
testRequest.getDiscoveryRequest().build();
+final List 
testExecutionListeners = new ArrayList<>();
+// a listener that we always put at the front of list 
of listeners
+// for this request.
+final Listener firstListener = new Listener();
+// we always enroll the summary generating listener, 
to the request, so that we
+// get to use some of the details of the summary for 
our further decision making
+testExecutionListeners.add(firstListener);
+
testExecutionListeners.addAll(getListeners(testRequest, executionCL));
+final PrintStream originalSysOut = System.out;
+final PrintStream originalSysErr = System.err;
+try {
+firstListener.switchedSysOutHandle = 
trySwitchSysOut(testRequest);
+firstListener.switchedSysErrHandle = 
trySwitchSysErr(testRequest);
+launcher.execute(request, 
testExecutionListeners.toArray(new 

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

[bodewig]

Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/60#discussion_r169247383
  
--- Diff: 
src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/JUnitLauncherTask.java
 ---
@@ -0,0 +1,508 @@
+package org.apache.tools.ant.taskdefs.optional.junitlauncher;
+
+import org.apache.tools.ant.AntClassLoader;
+import org.apache.tools.ant.BuildException;
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.Task;
+import org.apache.tools.ant.types.Path;
+import org.apache.tools.ant.util.KeepAliveOutputStream;
+import org.junit.platform.launcher.Launcher;
+import org.junit.platform.launcher.LauncherDiscoveryRequest;
+import org.junit.platform.launcher.TestExecutionListener;
+import org.junit.platform.launcher.TestPlan;
+import org.junit.platform.launcher.core.LauncherFactory;
+import org.junit.platform.launcher.listeners.SummaryGeneratingListener;
+import org.junit.platform.launcher.listeners.TestExecutionSummary;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
+import java.io.PrintStream;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * An Ant {@link Task} responsible for launching the JUnit platform for 
running tests.
+ * This requires a minimum of JUnit 5, since that's the version in which 
the JUnit platform launcher
+ * APIs were introduced.
+ * 
+ * This task in itself doesn't run the JUnit tests, instead the sole 
responsibility of
+ * this task is to setup the JUnit platform launcher, build requests, 
launch those requests and then parse the
+ * result of the execution to present in a way that's been configured on 
this Ant task.
+ * 
+ * 
+ * Furthermore, this task allows users control over which classes to 
select for passing on to the JUnit 5
+ * platform for test execution. It however, is solely the JUnit 5 
platform, backed by test engines that
+ * decide and execute the tests.
+ *
+ * @see https://junit.org/junit5/;>JUnit 5 documentation for 
more details
+ * on how JUnit manages the platform and the test engines.
+ */
+public class JUnitLauncherTask extends Task {
+
+private Path classPath;
+private boolean haltOnFailure;
+private String failureProperty;
+private final List tests = new ArrayList<>();
+private final List listeners = new ArrayList<>();
+
+public JUnitLauncherTask() {
+}
+
+@Override
+public void execute() throws BuildException {
+final ClassLoader previousClassLoader = 
Thread.currentThread().getContextClassLoader();
+try {
+final ClassLoader executionCL = 
createClassLoaderForTestExecution();
+Thread.currentThread().setContextClassLoader(executionCL);
+final Launcher launcher = LauncherFactory.create();
+final List requests = buildTestRequests();
+for (final TestRequest testRequest : requests) {
+try {
+final TestDefinition test = testRequest.getOwner();
+final LauncherDiscoveryRequest request = 
testRequest.getDiscoveryRequest().build();
+final List 
testExecutionListeners = new ArrayList<>();
+// a listener that we always put at the front of list 
of listeners
+// for this request.
+final Listener firstListener = new Listener();
+// we always enroll the summary generating listener, 
to the request, so that we
+// get to use some of the details of the summary for 
our further decision making
+testExecutionListeners.add(firstListener);
+
testExecutionListeners.addAll(getListeners(testRequest, executionCL));
+final PrintStream originalSysOut = System.out;
+final PrintStream originalSysErr = System.err;
+try {
+firstListener.switchedSysOutHandle = 
trySwitchSysOut(testRequest);
+firstListener.switchedSysErrHandle = 
trySwitchSysErr(testRequest);
+launcher.execute(request, 
testExecutionListeners.toArray(new 

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

[bodewig]

Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/60#discussion_r169245116
  
--- Diff: 
src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/JUnitLauncherTask.java
 ---
@@ -0,0 +1,508 @@
+package org.apache.tools.ant.taskdefs.optional.junitlauncher;
+
+import org.apache.tools.ant.AntClassLoader;
+import org.apache.tools.ant.BuildException;
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.Task;
+import org.apache.tools.ant.types.Path;
+import org.apache.tools.ant.util.KeepAliveOutputStream;
+import org.junit.platform.launcher.Launcher;
+import org.junit.platform.launcher.LauncherDiscoveryRequest;
+import org.junit.platform.launcher.TestExecutionListener;
+import org.junit.platform.launcher.TestPlan;
+import org.junit.platform.launcher.core.LauncherFactory;
+import org.junit.platform.launcher.listeners.SummaryGeneratingListener;
+import org.junit.platform.launcher.listeners.TestExecutionSummary;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
+import java.io.PrintStream;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * An Ant {@link Task} responsible for launching the JUnit platform for 
running tests.
+ * This requires a minimum of JUnit 5, since that's the version in which 
the JUnit platform launcher
+ * APIs were introduced.
+ * 
+ * This task in itself doesn't run the JUnit tests, instead the sole 
responsibility of
+ * this task is to setup the JUnit platform launcher, build requests, 
launch those requests and then parse the
+ * result of the execution to present in a way that's been configured on 
this Ant task.
+ * 
+ * 
+ * Furthermore, this task allows users control over which classes to 
select for passing on to the JUnit 5
+ * platform for test execution. It however, is solely the JUnit 5 
platform, backed by test engines that
+ * decide and execute the tests.
+ *
+ * @see https://junit.org/junit5/;>JUnit 5 documentation for 
more details
+ * on how JUnit manages the platform and the test engines.
+ */
+public class JUnitLauncherTask extends Task {
+
+private Path classPath;
+private boolean haltOnFailure;
+private String failureProperty;
+private final List tests = new ArrayList<>();
+private final List listeners = new ArrayList<>();
+
+public JUnitLauncherTask() {
+}
+
+@Override
+public void execute() throws BuildException {
+final ClassLoader previousClassLoader = 
Thread.currentThread().getContextClassLoader();
+try {
+final ClassLoader executionCL = 
createClassLoaderForTestExecution();
+Thread.currentThread().setContextClassLoader(executionCL);
+final Launcher launcher = LauncherFactory.create();
+final List requests = buildTestRequests();
+for (final TestRequest testRequest : requests) {
+try {
+final TestDefinition test = testRequest.getOwner();
+final LauncherDiscoveryRequest request = 
testRequest.getDiscoveryRequest().build();
+final List 
testExecutionListeners = new ArrayList<>();
+// a listener that we always put at the front of list 
of listeners
+// for this request.
+final Listener firstListener = new Listener();
+// we always enroll the summary generating listener, 
to the request, so that we
+// get to use some of the details of the summary for 
our further decision making
+testExecutionListeners.add(firstListener);
+
testExecutionListeners.addAll(getListeners(testRequest, executionCL));
+final PrintStream originalSysOut = System.out;
+final PrintStream originalSysErr = System.err;
+try {
+firstListener.switchedSysOutHandle = 
trySwitchSysOut(testRequest);
+firstListener.switchedSysErrHandle = 
trySwitchSysErr(testRequest);
+launcher.execute(request, 
testExecutionListeners.toArray(new 

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

[bodewig]

Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/60#discussion_r169243988
  
--- Diff: 
src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/JUnitLauncherTask.java
 ---
@@ -0,0 +1,508 @@
+package org.apache.tools.ant.taskdefs.optional.junitlauncher;
+
+import org.apache.tools.ant.AntClassLoader;
+import org.apache.tools.ant.BuildException;
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.Task;
+import org.apache.tools.ant.types.Path;
+import org.apache.tools.ant.util.KeepAliveOutputStream;
+import org.junit.platform.launcher.Launcher;
+import org.junit.platform.launcher.LauncherDiscoveryRequest;
+import org.junit.platform.launcher.TestExecutionListener;
+import org.junit.platform.launcher.TestPlan;
+import org.junit.platform.launcher.core.LauncherFactory;
+import org.junit.platform.launcher.listeners.SummaryGeneratingListener;
+import org.junit.platform.launcher.listeners.TestExecutionSummary;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
+import java.io.PrintStream;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * An Ant {@link Task} responsible for launching the JUnit platform for 
running tests.
+ * This requires a minimum of JUnit 5, since that's the version in which 
the JUnit platform launcher
+ * APIs were introduced.
+ * 
+ * This task in itself doesn't run the JUnit tests, instead the sole 
responsibility of
+ * this task is to setup the JUnit platform launcher, build requests, 
launch those requests and then parse the
+ * result of the execution to present in a way that's been configured on 
this Ant task.
+ * 
+ * 
+ * Furthermore, this task allows users control over which classes to 
select for passing on to the JUnit 5
+ * platform for test execution. It however, is solely the JUnit 5 
platform, backed by test engines that
+ * decide and execute the tests.
+ *
+ * @see https://junit.org/junit5/;>JUnit 5 documentation for 
more details
+ * on how JUnit manages the platform and the test engines.
+ */
+public class JUnitLauncherTask extends Task {
+
+private Path classPath;
+private boolean haltOnFailure;
+private String failureProperty;
+private final List tests = new ArrayList<>();
+private final List listeners = new ArrayList<>();
+
+public JUnitLauncherTask() {
+}
+
+@Override
+public void execute() throws BuildException {
+final ClassLoader previousClassLoader = 
Thread.currentThread().getContextClassLoader();
+try {
+final ClassLoader executionCL = 
createClassLoaderForTestExecution();
+Thread.currentThread().setContextClassLoader(executionCL);
+final Launcher launcher = LauncherFactory.create();
+final List requests = buildTestRequests();
+for (final TestRequest testRequest : requests) {
+try {
+final TestDefinition test = testRequest.getOwner();
+final LauncherDiscoveryRequest request = 
testRequest.getDiscoveryRequest().build();
+final List 
testExecutionListeners = new ArrayList<>();
+// a listener that we always put at the front of list 
of listeners
+// for this request.
+final Listener firstListener = new Listener();
+// we always enroll the summary generating listener, 
to the request, so that we
+// get to use some of the details of the summary for 
our further decision making
+testExecutionListeners.add(firstListener);
+
testExecutionListeners.addAll(getListeners(testRequest, executionCL));
+final PrintStream originalSysOut = System.out;
+final PrintStream originalSysErr = System.err;
+try {
+firstListener.switchedSysOutHandle = 
trySwitchSysOut(testRequest);
+firstListener.switchedSysErrHandle = 
trySwitchSysErr(testRequest);
+launcher.execute(request, 
testExecutionListeners.toArray(new 

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

[jaikiran]

Github user jaikiran commented on a diff in the pull request:

https://github.com/apache/ant/pull/60#discussion_r168983237
  
--- Diff: 
src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/AbstractJUnitResultFormatter.java
 ---
@@ -0,0 +1,139 @@
+package org.apache.tools.ant.taskdefs.optional.junitlauncher;
+
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.Task;
+import org.junit.platform.engine.TestSource;
+import org.junit.platform.engine.support.descriptor.ClassSource;
+import org.junit.platform.launcher.TestIdentifier;
+import org.junit.platform.launcher.TestPlan;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.Writer;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Optional;
+
+/**
+ * Contains some common behaviour that's used by our internal {@link 
TestResultFormatter}s
+ */
+abstract class AbstractJUnitResultFormatter implements TestResultFormatter 
{
+
+protected static String NEW_LINE = 
System.getProperty("line.separator");
+protected Path sysOutFilePath;
+protected Path sysErrFilePath;
+protected Task task;
+
+private OutputStream sysOutStream;
+private OutputStream sysErrStream;
+
+@Override
+public void sysOutAvailable(final byte[] data) {
+if (this.sysOutStream == null) {
+try {
+this.sysOutFilePath = Files.createTempFile(null, "sysout");
+this.sysOutFilePath.toFile().deleteOnExit();
+this.sysOutStream = 
Files.newOutputStream(this.sysOutFilePath);
+} catch (IOException e) {
+handleException(e);
+return;
+}
+}
+try {
+this.sysOutStream.write(data);
+} catch (IOException e) {
+handleException(e);
+return;
+}
+}
+
+@Override
+public void sysErrAvailable(final byte[] data) {
+if (this.sysErrStream == null) {
+try {
+this.sysErrFilePath = Files.createTempFile(null, "syserr");
+this.sysErrFilePath.toFile().deleteOnExit();
+this.sysErrStream = 
Files.newOutputStream(this.sysOutFilePath);
+} catch (IOException e) {
+handleException(e);
+return;
+}
+}
+try {
+this.sysErrStream.write(data);
+} catch (IOException e) {
+handleException(e);
+return;
+}
+}
+
+@Override
+public void setExecutingTask(final Task task) {
+this.task = task;
+}
+
+protected void writeSysOut(final Writer writer) throws IOException {
+this.writeFrom(this.sysOutFilePath, writer);
+}
+
+protected void writeSysErr(final Writer writer) throws IOException {
+this.writeFrom(this.sysErrFilePath, writer);
+}
+
+static Optional traverseAndFindTestClass(final 
TestPlan testPlan, final TestIdentifier testIdentifier) {
+if (isTestClass(testIdentifier).isPresent()) {
+return Optional.of(testIdentifier);
+}
+final Optional parent = 
testPlan.getParent(testIdentifier);
+return parent.isPresent() ? traverseAndFindTestClass(testPlan, 
parent.get()) : Optional.empty();
+}
+
+static Optional isTestClass(final TestIdentifier 
testIdentifier) {
+if (testIdentifier == null) {
+return Optional.empty();
+}
+final Optional source = testIdentifier.getSource();
+if (!source.isPresent()) {
+return Optional.empty();
+}
+final TestSource testSource = source.get();
+if (testSource instanceof ClassSource) {
+return Optional.of((ClassSource) testSource);
+}
+return Optional.empty();
+}
+
+private void writeFrom(final Path path, final Writer writer) throws 
IOException {
+final byte[] content = new byte[1024];
+int numBytes;
+try (final InputStream is = Files.newInputStream(path)) {
+while ((numBytes = is.read(content)) != -1) {
+writer.write(new String(content, 0, numBytes));
+}
+}
+}
+
+@Override
+public void close() throws IOException {
+if (this.sysOutStream != null) {
+try {
+this.sysOutStream.close();
+} catch (Exception e) {
+// ignore
+}

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

[jaikiran]

Github user jaikiran commented on a diff in the pull request:

https://github.com/apache/ant/pull/60#discussion_r168983118
  
--- Diff: 
src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/AbstractJUnitResultFormatter.java
 ---
@@ -0,0 +1,139 @@
+package org.apache.tools.ant.taskdefs.optional.junitlauncher;
+
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.Task;
+import org.junit.platform.engine.TestSource;
+import org.junit.platform.engine.support.descriptor.ClassSource;
+import org.junit.platform.launcher.TestIdentifier;
+import org.junit.platform.launcher.TestPlan;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.Writer;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Optional;
+
+/**
+ * Contains some common behaviour that's used by our internal {@link 
TestResultFormatter}s
+ */
+abstract class AbstractJUnitResultFormatter implements TestResultFormatter 
{
+
+protected static String NEW_LINE = 
System.getProperty("line.separator");
+protected Path sysOutFilePath;
+protected Path sysErrFilePath;
+protected Task task;
+
+private OutputStream sysOutStream;
+private OutputStream sysErrStream;
+
+@Override
+public void sysOutAvailable(final byte[] data) {
+if (this.sysOutStream == null) {
+try {
+this.sysOutFilePath = Files.createTempFile(null, "sysout");
+this.sysOutFilePath.toFile().deleteOnExit();
+this.sysOutStream = 
Files.newOutputStream(this.sysOutFilePath);
+} catch (IOException e) {
+handleException(e);
+return;
+}
+}
+try {
+this.sysOutStream.write(data);
+} catch (IOException e) {
+handleException(e);
+return;
+}
+}
+
+@Override
+public void sysErrAvailable(final byte[] data) {
+if (this.sysErrStream == null) {
+try {
+this.sysErrFilePath = Files.createTempFile(null, "syserr");
+this.sysErrFilePath.toFile().deleteOnExit();
+this.sysErrStream = 
Files.newOutputStream(this.sysOutFilePath);
--- End diff --

Indeed :) Fixed.


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

[bodewig]

Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/60#discussion_r168964381
  
--- Diff: 
src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/AbstractJUnitResultFormatter.java
 ---
@@ -0,0 +1,139 @@
+package org.apache.tools.ant.taskdefs.optional.junitlauncher;
+
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.Task;
+import org.junit.platform.engine.TestSource;
+import org.junit.platform.engine.support.descriptor.ClassSource;
+import org.junit.platform.launcher.TestIdentifier;
+import org.junit.platform.launcher.TestPlan;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.Writer;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Optional;
+
+/**
+ * Contains some common behaviour that's used by our internal {@link 
TestResultFormatter}s
+ */
+abstract class AbstractJUnitResultFormatter implements TestResultFormatter 
{
+
+protected static String NEW_LINE = 
System.getProperty("line.separator");
+protected Path sysOutFilePath;
+protected Path sysErrFilePath;
+protected Task task;
+
+private OutputStream sysOutStream;
+private OutputStream sysErrStream;
+
+@Override
+public void sysOutAvailable(final byte[] data) {
+if (this.sysOutStream == null) {
+try {
+this.sysOutFilePath = Files.createTempFile(null, "sysout");
+this.sysOutFilePath.toFile().deleteOnExit();
+this.sysOutStream = 
Files.newOutputStream(this.sysOutFilePath);
+} catch (IOException e) {
+handleException(e);
+return;
+}
+}
+try {
+this.sysOutStream.write(data);
+} catch (IOException e) {
+handleException(e);
+return;
+}
+}
+
+@Override
+public void sysErrAvailable(final byte[] data) {
+if (this.sysErrStream == null) {
+try {
+this.sysErrFilePath = Files.createTempFile(null, "syserr");
+this.sysErrFilePath.toFile().deleteOnExit();
+this.sysErrStream = 
Files.newOutputStream(this.sysOutFilePath);
+} catch (IOException e) {
+handleException(e);
+return;
+}
+}
+try {
+this.sysErrStream.write(data);
+} catch (IOException e) {
+handleException(e);
+return;
+}
+}
+
+@Override
+public void setExecutingTask(final Task task) {
+this.task = task;
+}
+
+protected void writeSysOut(final Writer writer) throws IOException {
+this.writeFrom(this.sysOutFilePath, writer);
+}
+
+protected void writeSysErr(final Writer writer) throws IOException {
+this.writeFrom(this.sysErrFilePath, writer);
+}
+
+static Optional traverseAndFindTestClass(final 
TestPlan testPlan, final TestIdentifier testIdentifier) {
+if (isTestClass(testIdentifier).isPresent()) {
+return Optional.of(testIdentifier);
+}
+final Optional parent = 
testPlan.getParent(testIdentifier);
+return parent.isPresent() ? traverseAndFindTestClass(testPlan, 
parent.get()) : Optional.empty();
+}
+
+static Optional isTestClass(final TestIdentifier 
testIdentifier) {
+if (testIdentifier == null) {
+return Optional.empty();
+}
+final Optional source = testIdentifier.getSource();
+if (!source.isPresent()) {
+return Optional.empty();
+}
+final TestSource testSource = source.get();
+if (testSource instanceof ClassSource) {
+return Optional.of((ClassSource) testSource);
+}
+return Optional.empty();
+}
+
+private void writeFrom(final Path path, final Writer writer) throws 
IOException {
+final byte[] content = new byte[1024];
+int numBytes;
+try (final InputStream is = Files.newInputStream(path)) {
+while ((numBytes = is.read(content)) != -1) {
+writer.write(new String(content, 0, numBytes));
+}
+}
+}
+
+@Override
+public void close() throws IOException {
+if (this.sysOutStream != null) {
+try {
+this.sysOutStream.close();
+} catch (Exception e) {
+// ignore
+}
+ 

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

[bodewig]

Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/60#discussion_r168964295
  
--- Diff: 
src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/AbstractJUnitResultFormatter.java
 ---
@@ -0,0 +1,139 @@
+package org.apache.tools.ant.taskdefs.optional.junitlauncher;
+
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.Task;
+import org.junit.platform.engine.TestSource;
+import org.junit.platform.engine.support.descriptor.ClassSource;
+import org.junit.platform.launcher.TestIdentifier;
+import org.junit.platform.launcher.TestPlan;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.Writer;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Optional;
+
+/**
+ * Contains some common behaviour that's used by our internal {@link 
TestResultFormatter}s
+ */
+abstract class AbstractJUnitResultFormatter implements TestResultFormatter 
{
+
+protected static String NEW_LINE = 
System.getProperty("line.separator");
+protected Path sysOutFilePath;
+protected Path sysErrFilePath;
+protected Task task;
+
+private OutputStream sysOutStream;
+private OutputStream sysErrStream;
+
+@Override
+public void sysOutAvailable(final byte[] data) {
+if (this.sysOutStream == null) {
+try {
+this.sysOutFilePath = Files.createTempFile(null, "sysout");
+this.sysOutFilePath.toFile().deleteOnExit();
+this.sysOutStream = 
Files.newOutputStream(this.sysOutFilePath);
+} catch (IOException e) {
+handleException(e);
+return;
+}
+}
+try {
+this.sysOutStream.write(data);
+} catch (IOException e) {
+handleException(e);
+return;
+}
+}
+
+@Override
+public void sysErrAvailable(final byte[] data) {
+if (this.sysErrStream == null) {
+try {
+this.sysErrFilePath = Files.createTempFile(null, "syserr");
+this.sysErrFilePath.toFile().deleteOnExit();
+this.sysErrStream = 
Files.newOutputStream(this.sysOutFilePath);
+} catch (IOException e) {
+handleException(e);
+return;
+}
+}
+try {
+this.sysErrStream.write(data);
+} catch (IOException e) {
+handleException(e);
+return;
+}
+}
+
+@Override
+public void setExecutingTask(final Task task) {
+this.task = task;
+}
+
+protected void writeSysOut(final Writer writer) throws IOException {
+this.writeFrom(this.sysOutFilePath, writer);
+}
+
+protected void writeSysErr(final Writer writer) throws IOException {
+this.writeFrom(this.sysErrFilePath, writer);
+}
+
+static Optional traverseAndFindTestClass(final 
TestPlan testPlan, final TestIdentifier testIdentifier) {
+if (isTestClass(testIdentifier).isPresent()) {
+return Optional.of(testIdentifier);
+}
+final Optional parent = 
testPlan.getParent(testIdentifier);
+return parent.isPresent() ? traverseAndFindTestClass(testPlan, 
parent.get()) : Optional.empty();
+}
+
+static Optional isTestClass(final TestIdentifier 
testIdentifier) {
+if (testIdentifier == null) {
+return Optional.empty();
+}
+final Optional source = testIdentifier.getSource();
+if (!source.isPresent()) {
+return Optional.empty();
+}
+final TestSource testSource = source.get();
+if (testSource instanceof ClassSource) {
+return Optional.of((ClassSource) testSource);
+}
+return Optional.empty();
+}
+
+private void writeFrom(final Path path, final Writer writer) throws 
IOException {
+final byte[] content = new byte[1024];
+int numBytes;
+try (final InputStream is = Files.newInputStream(path)) {
+while ((numBytes = is.read(content)) != -1) {
+writer.write(new String(content, 0, numBytes));
+}
--- End diff --

I don't think this is going to work reliably. The `read` may have split a 
multi-byte sequence at the end of `content` and then creating a string from it 
is going to break. Is there any reason you want to use a stream when reading 
the temporary file rather than a reader?


---

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

[bodewig]

Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/60#discussion_r168963955
  
--- Diff: 
src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/AbstractJUnitResultFormatter.java
 ---
@@ -0,0 +1,139 @@
+package org.apache.tools.ant.taskdefs.optional.junitlauncher;
+
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.Task;
+import org.junit.platform.engine.TestSource;
+import org.junit.platform.engine.support.descriptor.ClassSource;
+import org.junit.platform.launcher.TestIdentifier;
+import org.junit.platform.launcher.TestPlan;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.Writer;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Optional;
+
+/**
+ * Contains some common behaviour that's used by our internal {@link 
TestResultFormatter}s
+ */
+abstract class AbstractJUnitResultFormatter implements TestResultFormatter 
{
+
+protected static String NEW_LINE = 
System.getProperty("line.separator");
+protected Path sysOutFilePath;
+protected Path sysErrFilePath;
+protected Task task;
+
+private OutputStream sysOutStream;
+private OutputStream sysErrStream;
+
+@Override
+public void sysOutAvailable(final byte[] data) {
+if (this.sysOutStream == null) {
+try {
+this.sysOutFilePath = Files.createTempFile(null, "sysout");
+this.sysOutFilePath.toFile().deleteOnExit();
+this.sysOutStream = 
Files.newOutputStream(this.sysOutFilePath);
+} catch (IOException e) {
+handleException(e);
+return;
+}
+}
+try {
+this.sysOutStream.write(data);
+} catch (IOException e) {
+handleException(e);
+return;
+}
+}
+
+@Override
+public void sysErrAvailable(final byte[] data) {
+if (this.sysErrStream == null) {
+try {
+this.sysErrFilePath = Files.createTempFile(null, "syserr");
+this.sysErrFilePath.toFile().deleteOnExit();
+this.sysErrStream = 
Files.newOutputStream(this.sysOutFilePath);
--- End diff --

copy-paste error, you want that to be `sysErrFilePath` :-)


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #62: Use char notation to represent a character to improve ...

[reudismam]

GitHub user reudismam opened a pull request:

https://github.com/apache/ant/pull/62

Use char notation to represent a character to improve performance.



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/reudismam/ant character

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/ant/pull/62.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #62


commit 4df60d6b21f4c85df039037531aa1627fc40b68d
Author: reudismam 
Date:   2018-02-16T12:49:15Z

Use char notation to represent a character to improve performance.




---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #61: Use the isEmpty method instead of comparing the value ...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/ant/pull/61


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #61: Use the isEmpty method instead of comparing the value ...

[reudismam]

GitHub user reudismam opened a pull request:

https://github.com/apache/ant/pull/61

Use the isEmpty method instead of comparing the value of size() to 0.



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/reudismam/ant size

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/ant/pull/61.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #61


commit 25315d0c28cf81c8c23944d3f368c76c6da02ac2
Author: reudismam 
Date:   2018-02-15T17:30:25Z

Use the isEmpty method instead of comparing the value of size() to 0.




---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #60: JUnit 5 support - A new junitlauncher task

[jaikiran]

GitHub user jaikiran opened a pull request:

https://github.com/apache/ant/pull/60

JUnit 5 support - A new junitlauncher task

This is the initial working version of a new `junitlauncher` task that 
support using JUnit 5 framework for testing, within Ant build files. The commit 
in this PR is the initial set of goals that I had in mind for the first release 
of this task.

The manual for this task can be (temporarily) found at 
https://builds.apache.org/job/Ant-Build-Jaikiran/ws/manual/Tasks/junitlauncher.html

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/jaikiran/ant junit5

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/ant/pull/60.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #60


commit 991bddff07b4fed1db2dc7f6b83f47557e62213b
Author: Jaikiran Pai 
Date:   2017-12-13T13:37:41Z

JUnit 5 support - A new junitlauncher task




---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #57: Make junitreport with Saxon

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/ant/pull/57


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #59: Fix NPE in ChainedMapper.

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/ant/pull/59


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #59: Fix NPE in ChainedMapper.

[jpountz]

GitHub user jpountz opened a pull request:

https://github.com/apache/ant/pull/59

Fix NPE in ChainedMapper.

This NPE happens whenever any of the sub mappers returns `null`, which may
happen eg. with `GlobPatternMapper`.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/jpountz/ant fix/chained_mapper_npe

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/ant/pull/59.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #59


commit e777ee85a5c077949c81e23c359cf857dd13ca83
Author: Adrien Grand 
Date:   2018-02-08T14:22:02Z

Fix NPE in ChainedMapper.

This NPE happens whenever any of the sub mappers returns `null`, which may
happen eg. with `GlobPatternMapper`.




---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #58: Use StringBuilder instead of StringBuffer as it offers...

[bodewig]

Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/58#discussion_r166217627
  
--- Diff: src/main/org/apache/tools/ant/taskdefs/Parallel.java ---
@@ -377,7 +377,7 @@ public synchronized void run() {
 }
 
 // now did any of the threads throw an exception
-exceptionMessage = new StringBuffer();
+exceptionMessage = new StringBuilder();
--- End diff --

Looking through the class I don't think `exceptionMessage` needs to be an 
instance field at all, it could be a local variable in `spinThreads` and get 
passed as an argument to `processExceptions` without doing any harm. To me it 
seems it is only ever used by a single thread.

Most probably a further refactoring could get rid of the `first*` instance 
fields as well and have `processExceptions` return all their values nicely 
encapsulated in a single object - including the accumulated messages.


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #58: Use StringBuilder instead of StringBuffer as it offers...

[bodewig]

Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/58#discussion_r166216151
  
--- Diff: src/main/org/apache/tools/ant/listener/MailLogger.java ---
@@ -102,7 +102,7 @@
 private static final String DEFAULT_MIME_TYPE = "text/plain";
 
 /** Buffer in which the message is constructed prior to sending */
-private StringBuffer buffer = new StringBuffer();
+private StringBuilder buffer = new StringBuilder();
--- End diff --

Given the `parallel` task and things like our execute framework that spawns 
new threads, loggers need to be thread safe, But to be honest I don't think 
we've ever verified `MailLogger` actually is.


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #58: Use StringBuilder instead of StringBuffer as it offers...

[jaikiran]

Github user jaikiran commented on a diff in the pull request:

https://github.com/apache/ant/pull/58#discussion_r166183671
  
--- Diff: src/main/org/apache/tools/ant/taskdefs/Parallel.java ---
@@ -377,7 +377,7 @@ public synchronized void run() {
 }
 
 // now did any of the threads throw an exception
-exceptionMessage = new StringBuffer();
+exceptionMessage = new StringBuilder();
--- End diff --

Same comment as above.


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #58: Use StringBuilder instead of StringBuffer as it offers...

[jaikiran]

Github user jaikiran commented on a diff in the pull request:

https://github.com/apache/ant/pull/58#discussion_r166183611
  
--- Diff: src/main/org/apache/tools/ant/listener/MailLogger.java ---
@@ -102,7 +102,7 @@
 private static final String DEFAULT_MIME_TYPE = "text/plain";
 
 /** Buffer in which the message is constructed prior to sending */
-private StringBuffer buffer = new StringBuffer();
+private StringBuilder buffer = new StringBuilder();
--- End diff --

I'm not too sure this change here, in this class is correct. The 
`StringBuffer` is a thread-safe class whereas the `StringBuilder` isn't. Having 
said that I haven't checked yet whether MailLogger class is expected to be 
thread safe nor have I checked its usage thoroughly.


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #58: Use StringBuilder instead of StringBuffer as it offers...

[reudismam]

GitHub user reudismam opened a pull request:

https://github.com/apache/ant/pull/58

Use StringBuilder instead of StringBuffer as it offers high performan…

…ce in single thread places as it is generally the case.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/reudismam/ant builder

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/ant/pull/58.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #58


commit bf3d0b448ed055eea7dcc036397e8b0f7cbc50fe
Author: reudismam 
Date:   2018-02-05T16:18:05Z

Use StringBuilder instead of StringBuffer as it offers high performance in 
single thread places as it is generally the case.




---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #57: Make junitreport with Saxon

[adamretter]

GitHub user adamretter opened a pull request:

https://github.com/apache/ant/pull/57

Make junitreport with Saxon

Previously the XSLT(s) used in `junitreport` were only compatible with 
Xalan 2. Whilst you could specify a different `TransformerFactory`, and that 
could be Saxon, it would fail as the XSL(s) use Xalan specific extensions.

This PR adds compatibility with Saxon, so that if you specify Saxon as the 
`TransformerFactory`, then pure XSLT2 stylesheets are used without any vendor 
extensions.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/adamretter/ant junitreport-xsl-saxon

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/ant/pull/57.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #57


commit 3ceba3aaf66701aad8fc4a8ad685df4e6645a2c9
Author: Adam Retter 
Date:   2018-02-04T10:09:55Z

[bugfix] Allow Saxon to be used for junitreport XSL transformation




---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #56: Use equals method of a string literal to prevent NPE a...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/ant/pull/56


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #56: Use equals method of a string literal to prevent NPE a...

[reudismam]

GitHub user reudismam opened a pull request:

https://github.com/apache/ant/pull/56

Use equals method of a string literal to prevent NPE and isEmpty() me…

…thod instead of comparing a String object with an empty string.

This change has already been done for some locations in the commit:

https://github.com/apache/ant/commit/b7d1e9bde44cb8e5233d6e70bb96e14cbb2f3e2d

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/reudismam/ant equals

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/ant/pull/56.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #56


commit debf404d170cfd771a517e9c5b408bc9735e2f70
Author: reudismam 
Date:   2018-02-01T17:03:19Z

Use equals method of a string literal to prevent NPE and isEmpty() method 
instead of comparing a String object with an empty string.




---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #55: Use valueOf method instead of constructor since valueO...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/ant/pull/55


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #55: Use valueOf method instead of constructor since valueO...

[reudismam]

GitHub user reudismam opened a pull request:

https://github.com/apache/ant/pull/55

Use valueOf method instead of constructor since valueOf has higher pe…

…rformance.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/reudismam/ant master

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/ant/pull/55.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #55


commit 51acedc82ee0bb7472041825937c4c139c93195e
Author: reudismam 
Date:   2018-01-24T13:57:12Z

Use valueOf method instead of constructor since valueOf has higher 
performance.




---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #:

[bodewig]

Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/1465c4581ace84548ec9dfc487d62cfec14d84ad#commitcomment-27040705
  
we should try whether the bridge between 1.x and 2.x works and recommend 
our users to use that if it does.

I don't think we should change our POMs as it should be a conscious 
decisions of the users whether they want to upgrade their logging dependencies 
or not. They may be running Ant in an environment where the risk of an old 
Log4j version is acceptable to them.


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #:

[twogee]

Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/1465c4581ace84548ec9dfc487d62cfec14d84ad#commitcomment-27040319
  
What about upgrading to 2.x with 1.x compatibility API? (ditto for Commons 
Logging). Any comments, @bodewig?


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #:

[twogee]

Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/538b7c9ffee7a18064f7726c8b20faf681adb218#commitcomment-27018577
  
I'd rather upgrade to Log4j 2.x :smile: 


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #52: [master] Fix BZ-58451 BZ-58833

[jaikiran]

Github user jaikiran closed the pull request at:

https://github.com/apache/ant/pull/52


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #:

[bodewig]

Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/d43d83e0198f86c9233631bcb443da132e394d65#commitcomment-26506626
  
You're welcome.

That's why I try to make changes that are target for both branches just to 
the 1.9.x branch and merge it to master.


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #:

[twogee]

Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/d43d83e0198f86c9233631bcb443da132e394d65#commitcomment-26504265
  
Thanks for catching this one up, a slip of attention while backporting 
changes in master.


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #54: Let’s use Ivy (properly!) and drop Maven Ant tasks, ...

[twogee]

GitHub user twogee opened a pull request:

https://github.com/apache/ant/pull/54

Let’s use Ivy (properly!) and drop Maven Ant tasks, Commons OpenPGP, …

… Python, etc. The implementation is incomplete, please comment.
I put extraneous stuff in `attic` subdirectory for reference.
`release/upload.xml` could very well be integrated in build.xml (because of 
`project.version`).
`fetch.xml` does not implement all targets nor updating Ivy distribution
(could be done by restricting ivy:resolve to a particular conf). 🎅

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/twogee/ant dog-food-ivy

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/ant/pull/54.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #54


commit 86494a660fe9f3d202de3b4b069bf0b65a29c500
Author: twogee 
Date:   2017-12-26T20:51:41Z

Let’s use Ivy (properly!) and drop Maven Ant tasks, Commons OpenPGP, 
Python, etc




---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #53: Optional libraries for Ant 1.10

[twogee]

Github user twogee closed the pull request at:

https://github.com/apache/ant/pull/53


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #:

[twogee]

Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/1a2c33fd0ef9dfc042044277707d7f38b0cfa0b0#commitcomment-26409414
  
Somebody with a sense of humour has a `@Deprecated` account on GitHub 
:rofl: 


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

Re: [GitHub] ant pull request #:

[Jaikiran Pai]

No specific reason, just that I'm more used to using the javadoc variant 
of @deprecated since it allows explaining what's deprecated and why. I 
have now added the @Deprecated annotation too and pushed a commit.


-Jaikiran


On 20/12/17 11:08 PM, twogee wrote:

Github user twogee commented on the pull request:

 
https://github.com/apache/ant/commit/242d661161ddf2b28e6df23847c1471ee788bab7#commitcomment-26396824
   
 In src/main/org/apache/tools/ant/util/SymbolicLinkUtils.java:

 In src/main/org/apache/tools/ant/util/SymbolicLinkUtils.java on line 35:
 Why not annotating the class with `@Deprecated`, too?


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org




-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #:

[twogee]

Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/242d661161ddf2b28e6df23847c1471ee788bab7#commitcomment-26396824
  
In src/main/org/apache/tools/ant/util/SymbolicLinkUtils.java:
In src/main/org/apache/tools/ant/util/SymbolicLinkUtils.java on line 35:
Why not annotating the class with `@Deprecated`, too?


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #53: Optional libraries for Ant 1.10

[twogee]

GitHub user twogee opened a pull request:

https://github.com/apache/ant/pull/53

Optional libraries for Ant 1.10

A bit unsure about JRuby: should we stay with 1.7 for a while or go for 9? 
In any case, would jruby-core be sufficient, or should jruby-stdlib be added, 
too?

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/twogee/ant master

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/ant/pull/53.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #53


commit 9374be377732e3cd3038fc15aa735171f83d13cd
Author: twogee 
Date:   2017-12-19T19:01:28Z

Optional libraries for Ant 1.10




---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #50: Use newer third party libraries

[twogee]

Github user twogee closed the pull request at:

https://github.com/apache/ant/pull/50


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #:

[bodewig]

Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/6f5db5e0689c0bea88f16f018be875ef9840825b#commitcomment-26342106
  
you are certainly right, thank you for catching this.


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #:

[twogee]

Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/6f5db5e0689c0bea88f16f018be875ef9840825b#commitcomment-26338779
  
Shouldn't we use HTML entities (eg )? Is HTML 3 is a requirement, 
too? :confused:


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #52: [master] Fix BZ-58451 BZ-58833

[jaikiran]

GitHub user jaikiran opened a pull request:

https://github.com/apache/ant/pull/52

[master] Fix BZ-58451 BZ-58833

The commit here fixes the issues reported in:

https://bz.apache.org/bugzilla/show_bug.cgi?id=58833
https://bz.apache.org/bugzilla/show_bug.cgi?id=58451

The `PumpStreamHandler` and the `StreamPumper` work together for 
redirecting the streams (reading from inputstream and writing to a 
outputstream). When the `StreamPumper` is asked to stop, it stops reading from 
the input stream and goes on to finish any (non-blocking) reads and finally a 
(potentially blocking) flush of whatever it has read so far. The 
`PumpStreamHandler`, which initiates that `stop` is a bit too impatient (mainly 
because of its inability to know what's going on in `StreamPumper` after the 
`stop` was invoked on it) and starts interrupting the `StreamPumper`'s thread 
very soon (as early as 200 milli seconds after stop). This triggers a bunch of 
issues in `StreamPumper`, which either is in the middle of finishing the 
non-blocking reads or in the middle of actually flushing out whatever it's held 
on to and ultimately leads to a non-clean stop and thus corrupting/truncating 
the output stream as noted in those issues.

The commit here introduces a way, through the `PostStopHandle`, for 
`PumpStreamHandler` to be aware that upon `stop` the `StreamPumper` has 
respected the call to `stop` and is doing certain post-stop finishing acts 
before actually being considered `finished`. The `PumpStreamHandler` then 
allows a bit of time for the post-stop tasks to finish before actually trying 
to kill off the `StreamPumper` using thread interrupts, if the `StreamPumper` 
didn't finish after that grace period.

The commit here intentionally does _not_ introduce a configurable "grace 
period" for the post-stop activities and instead using a hard coded (maximum) 2 
seconds for the following reasons:

* The 2 seconds is the _maxium_ amount of time to let the post-stop to 
complete, so it's not always going to wait that long

* The cleaning up gracefully is an internal implementation detail and 
doesn't have to end up being configured by the user.

* Finally, the `PumpStreamHandler` anyway falls back to the thread 
interrupts if the post-stop (flush() etc...) doesn't complete in those 2 
seconds. So from a end user point of view, there isn't really any behaviour 
change, except that we now give the `StreamPumper` a chance to gracefully stop.

This PR is against master branch. If this is approved I'll backport it to 
1.9.x branch too.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/jaikiran/ant bz-58451-master

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/ant/pull/52.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #52


commit 3bcdac90581cb78e4a49b0b7fe6b01657a6436f1
Author: Jaikiran Pai 
Date:   2017-12-16T09:48:30Z

BZ-58451 BZ-58833 Give StreamPumper a chance to finish cleanly before 
interrupting its thread, to prevent truncated output




---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #51: [master] Changes for BZ-19516

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/ant/pull/51


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #38: Do not merge

[jaikiran]

Github user jaikiran closed the pull request at:

https://github.com/apache/ant/pull/38


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #51: [master] Changes for BZ-19516

[jaikiran]

Github user jaikiran closed the pull request at:

https://github.com/apache/ant/pull/51


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #51: [master] Changes for BZ-19516

[jaikiran]

GitHub user jaikiran reopened a pull request:

https://github.com/apache/ant/pull/51

[master] Changes for BZ-19516 

As suggested in https://bz.apache.org/bugzilla/show_bug.cgi?id=19516, the 
change in this PR uses `java.io.BufferedInputStream` which can take an 
underlying `InputStream` and provide `mark` support on the input stream. This 
should prevent loading a large amount of data into memory, in certain cases, in 
the `Zip` task.

If this PR is approved, I'll then backport this pretty straightforward 
change to 1.9.x branch too.



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/jaikiran/ant bz-19516-master

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/ant/pull/51.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #51


commit 759a9c1263e8bb0a710c638960756af2e0971bc4
Author: Jaikiran Pai 
Date:   2017-12-12T11:22:26Z

BZ-19516 Use BufferedInputStream for reduced memory usage in Zip task, in 
certain cases




---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #51: [master] Changes for BZ-19516

[jaikiran]

Github user jaikiran closed the pull request at:

https://github.com/apache/ant/pull/51


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #51: [master] Changes for BZ-19516

[jaikiran]

GitHub user jaikiran reopened a pull request:

https://github.com/apache/ant/pull/51

[master] Changes for BZ-19516 

As suggested in https://bz.apache.org/bugzilla/show_bug.cgi?id=19516, the 
change in this PR uses `java.io.BufferedInputStream` which can take an 
underlying `InputStream` and provide `mark` support on the input stream. This 
should prevent loading a large amount of data into memory, in certain cases, in 
the `Zip` task.

If this PR is approved, I'll then backport this pretty straightforward 
change to 1.9.x branch too.



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/jaikiran/ant bz-19516-master

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/ant/pull/51.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #51


commit 759a9c1263e8bb0a710c638960756af2e0971bc4
Author: Jaikiran Pai 
Date:   2017-12-12T11:22:26Z

BZ-19516 Use BufferedInputStream for reduced memory usage in Zip task, in 
certain cases




---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #38: Do not merge

[jaikiran]

GitHub user jaikiran reopened a pull request:

https://github.com/apache/ant/pull/38

Do not merge 

Testing Jenkins integration. Do not merge this PR

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/jaikiran/ant dummy

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/ant/pull/38.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #38


commit 7722bab09e2baab9cfb140f537dd045f05aba7f2
Author: Jaikiran Pai 
Date:   2017-09-17T07:45:52Z

do not merge




---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #51: [master] Changes for BZ-19516

[jaikiran]

GitHub user jaikiran opened a pull request:

https://github.com/apache/ant/pull/51

[master] Changes for BZ-19516 

As suggested in https://bz.apache.org/bugzilla/show_bug.cgi?id=19516, the 
change in this PR uses `java.io.BufferedInputStream` which can take an 
underlying `InputStream` and provide `mark` support on the input stream. This 
should prevent loading a large amount of data into memory, in certain cases, in 
the `Zip` task.

If this PR is approved, I'll then backport this pretty straightforward 
change to 1.9.x branch too.



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/jaikiran/ant bz-19516-master

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/ant/pull/51.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #51


commit 759a9c1263e8bb0a710c638960756af2e0971bc4
Author: Jaikiran Pai 
Date:   2017-12-12T11:22:26Z

BZ-19516 Use BufferedInputStream for reduced memory usage in Zip task, in 
certain cases




---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #49: [master branch] - Fix BZ-58683

[jaikiran]

Github user jaikiran closed the pull request at:

https://github.com/apache/ant/pull/49


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #49: [master branch] - Fix BZ-58683

[bodewig]

Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/49#discussion_r155941358
  
--- Diff: src/main/org/apache/tools/ant/util/SymbolicLinkUtils.java ---
@@ -30,6 +30,8 @@
  * a symbolic link based on the absent support for them in Java.
  *
  * @since Ant 1.8.0
+ * @deprecated Starting Ant 1.10.2, this utility is deprecated in favour 
of the symlink
+ *  support introduced in Java {@link java.nio.file.Files} APIs
--- End diff --

sounds good to me, thanks. I prefer small changes myself as well.


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #49: [master branch] - Fix BZ-58683

[jaikiran]

Github user jaikiran commented on a diff in the pull request:

https://github.com/apache/ant/pull/49#discussion_r155941213
  
--- Diff: src/main/org/apache/tools/ant/util/SymbolicLinkUtils.java ---
@@ -30,6 +30,8 @@
  * a symbolic link based on the absent support for them in Java.
  *
  * @since Ant 1.8.0
+ * @deprecated Starting Ant 1.10.2, this utility is deprecated in favour 
of the symlink
+ *  support introduced in Java {@link java.nio.file.Files} APIs
--- End diff --

>> Do you intend to change those other occurrences as well?

I do intend to do necessary changes related to this, in subsequent 
commit(s), when I get a chance. I didn't want to create one large PR with too 
many unrelated changes. I added this deprecation note just so that any new code 
doesn't end up using this class, now that we are on Java 7. But I think, it's 
better to add this deprecation note when I do get to changing references to 
this class in some other commits. So I have undone this change and updated the 
PR.


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #49: [master branch] - Fix BZ-58683

[jaikiran]

Github user jaikiran commented on a diff in the pull request:

https://github.com/apache/ant/pull/49#discussion_r155941155
  
--- Diff: src/main/org/apache/tools/ant/taskdefs/optional/unix/Symlink.java 
---
@@ -500,18 +502,12 @@ private void doLink(String res, String lnk) throws 
BuildException {
 File dir = fs.getDir(getProject());
 
 Stream.of(ds.getIncludedFiles(), ds.getIncludedDirectories())
-.flatMap(Stream::of).forEach(path -> {
-try {
-File f = new File(dir, path);
-File pf = f.getParentFile();
-String name = f.getName();
-if (SYMLINK_UTILS.isSymbolicLink(pf, name)) {
-result.add(new File(pf.getCanonicalFile(), 
name));
-}
-} catch (IOException e) {
-handleError("IOException: " + path + " omitted");
+.flatMap(Stream::of).forEach(path -> {
+final File f = new File(dir, path);
+if (Files.isSymbolicLink(f.toPath())) {
+result.add(f);
--- End diff --

When I initially changed this part, while submitting the PR, I had thought 
about it a bit whether or not to stick with the previous behaviour. Given that 
it was noted as a "limitation" (in the javadoc), I had decided to change the 
behaviour. But thinking about it again, I do agree with you that it isn't worth 
changing the previous behaviour (without knowing the complete impact). So I 
have switched back to the previous semantic, in this part of the code and 
updated the PR


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #49: [master branch] - Fix BZ-58683

[jaikiran]

Github user jaikiran commented on a diff in the pull request:

https://github.com/apache/ant/pull/49#discussion_r155941101
  
--- Diff: src/main/org/apache/tools/ant/taskdefs/optional/unix/Symlink.java 
---
@@ -448,49 +432,67 @@ private void handleError(String msg) {
 /**
  * Conduct the actual construction of a link.
  *
- *  The link is constructed by calling 
Execute.runCommand.
  *
- * @param res   The path of the resource we are linking to.
- * @param lnk   The name of the link we wish to make.
+ * @param res The path of the resource we are linking to.
+ * @param lnk The name of the link we wish to make.
  * @throws BuildException when things go wrong
  */
 private void doLink(String res, String lnk) throws BuildException {
-File linkfil = new File(lnk);
-String options = "-s";
-if (overwrite) {
-options += "f";
-if (linkfil.exists()) {
-try {
-SYMLINK_UTILS.deleteSymbolicLink(linkfil, this);
-} catch (FileNotFoundException fnfe) {
-log("Symlink disappeared before it was deleted: " + 
lnk);
-} catch (IOException ioe) {
-log("Unable to overwrite preexisting link or file: " + 
lnk,
-ioe, Project.MSG_INFO);
+final Path link = Paths.get(lnk);
+final Path target = Paths.get(res);
+final boolean alreadyExists = Files.exists(link);
+if (!alreadyExists) {
+// if the path (at which the link is expected to be created) 
isn't already present
+// then we just go ahead and attempt to symlink
+try {
+Files.createSymbolicLink(link, target);
+} catch (IOException e) {
+if (failonerror) {
+throw new BuildException("Failed to create symlink " + 
lnk + " to target " + res, e);
 }
+log("Unable to create symlink " + lnk + " to target " + 
res, e, Project.MSG_INFO);
 }
+return;
+}
+// file already exists, see if we are allowed to overwrite
+if (!overwrite) {
+log("Skipping symlink creation, since file at " + lnk + " 
already exists and overwrite is set to false", Project.MSG_INFO);
+return;
+}
+// we have been asked to overwrite, so we now do the necessary 
steps
+
+// initiate a deletion *only* if the path is a symlink, else we 
fail with error
+if (!Files.isSymbolicLink(link)) {
+throw new BuildException("Cannot overwrite, as symlink, at " + 
lnk + " since the path already exists and isn't a symlink");
--- End diff --

Yes, that was an oversight. Fixed.


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #49: [master branch] - Fix BZ-58683

[jaikiran]

Github user jaikiran commented on a diff in the pull request:

https://github.com/apache/ant/pull/49#discussion_r155941095
  
--- Diff: src/main/org/apache/tools/ant/taskdefs/optional/unix/Symlink.java 
---
@@ -207,26 +198,30 @@ public void recreate() throws BuildException {
 try {
 if (fileSets.isEmpty()) {
 handleError(
-"File set identifying link file(s) required for action 
recreate");
+"File set identifying link file(s) required for 
action recreate");
 return;
 }
-Properties links = loadLinks(fileSets);
-
-for (String lnk : links.stringPropertyNames()) {
-String res = links.getProperty(lnk);
-// handle the case where lnk points to a directory (bug 
25181)
+final Properties links = loadLinks(fileSets);
+for (final String lnk : links.stringPropertyNames()) {
+final String res = links.getProperty(lnk);
 try {
-File test = new File(lnk);
-if (!SYMLINK_UTILS.isSymbolicLink(lnk)) {
-doLink(res, lnk);
-} else if (!test.getCanonicalPath().equals(
-new File(res).getCanonicalPath())) {
-SYMLINK_UTILS.deleteSymbolicLink(test, this);
-doLink(res, lnk);
-} // else lnk exists, do nothing
-} catch (IOException ioe) {
-handleError("IO exception while creating link");
+if (Files.isSymbolicLink(Paths.get(lnk)) &&
+
Paths.get(lnk).toFile().getCanonicalPath().equals(Paths.get(res).toFile().getCanonicalPath()))
 {
+// it's already a symlink and the symlink target 
is the same
+// as the target noted in the properties file. So 
there's no
+// need to recreate it
+continue;
+}
+} catch (IOException e) {
+if (failonerror) {
+throw new BuildException("Failed to recreate 
symlink " + lnk + " to target " + res, e);
+}
+// log and continue
+log("Failed to recreate symlink " + lnk + " to target 
" + res, Project.MSG_INFO);
+continue;
 }
+// create the link
+this.doLink(res, lnk);
--- End diff --

You are right - the error message was incorrect. I have fixed that and 
updated the PR.


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #49: [master branch] - Fix BZ-58683

[jaikiran]

Github user jaikiran commented on a diff in the pull request:

https://github.com/apache/ant/pull/49#discussion_r155941080
  
--- Diff: manual/Tasks/symlink.html ---
@@ -26,7 +26,7 @@
 
 Symlink
 Description
- Manages symbolic links on Unix based platforms. Can be used to
+ Manages symbolic links on platforms that support symbolic links. Can 
be used to
--- End diff --

Fixed and updated the PR


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #49: [master branch] - Fix BZ-58683

[bodewig]

Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/49#discussion_r155928363
  
--- Diff: src/main/org/apache/tools/ant/util/SymbolicLinkUtils.java ---
@@ -30,6 +30,8 @@
  * a symbolic link based on the absent support for them in Java.
  *
  * @since Ant 1.8.0
+ * @deprecated Starting Ant 1.10.2, this utility is deprecated in favour 
of the symlink
+ *  support introduced in Java {@link java.nio.file.Files} APIs
--- End diff --

the class is used all over the code base (well, at least in a few different 
places :-). Do you intend to change those other occurrences as well?


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #49: [master branch] - Fix BZ-58683

[bodewig]

Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/49#discussion_r155928313
  
--- Diff: src/main/org/apache/tools/ant/taskdefs/optional/unix/Symlink.java 
---
@@ -500,18 +502,12 @@ private void doLink(String res, String lnk) throws 
BuildException {
 File dir = fs.getDir(getProject());
 
 Stream.of(ds.getIncludedFiles(), ds.getIncludedDirectories())
-.flatMap(Stream::of).forEach(path -> {
-try {
-File f = new File(dir, path);
-File pf = f.getParentFile();
-String name = f.getName();
-if (SYMLINK_UTILS.isSymbolicLink(pf, name)) {
-result.add(new File(pf.getCanonicalFile(), 
name));
-}
-} catch (IOException e) {
-handleError("IOException: " + path + " omitted");
+.flatMap(Stream::of).forEach(path -> {
+final File f = new File(dir, path);
+if (Files.isSymbolicLink(f.toPath())) {
+result.add(f);
--- End diff --

The previous code canonicalized the link's parent directory to deal with 
symlinks further up the tree, I think this is important for consumers of the 
returned set like `record` that so merges entries from several "directories" 
that actually are links to the same directory.

To be honest I'm not sure about the real impact but I guess we better keep 
the existing behavior.


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #49: [master branch] - Fix BZ-58683

[bodewig]

Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/49#discussion_r155928143
  
--- Diff: src/main/org/apache/tools/ant/taskdefs/optional/unix/Symlink.java 
---
@@ -448,49 +432,67 @@ private void handleError(String msg) {
 /**
  * Conduct the actual construction of a link.
  *
- *  The link is constructed by calling 
Execute.runCommand.
  *
- * @param res   The path of the resource we are linking to.
- * @param lnk   The name of the link we wish to make.
+ * @param res The path of the resource we are linking to.
+ * @param lnk The name of the link we wish to make.
  * @throws BuildException when things go wrong
  */
 private void doLink(String res, String lnk) throws BuildException {
-File linkfil = new File(lnk);
-String options = "-s";
-if (overwrite) {
-options += "f";
-if (linkfil.exists()) {
-try {
-SYMLINK_UTILS.deleteSymbolicLink(linkfil, this);
-} catch (FileNotFoundException fnfe) {
-log("Symlink disappeared before it was deleted: " + 
lnk);
-} catch (IOException ioe) {
-log("Unable to overwrite preexisting link or file: " + 
lnk,
-ioe, Project.MSG_INFO);
+final Path link = Paths.get(lnk);
+final Path target = Paths.get(res);
+final boolean alreadyExists = Files.exists(link);
+if (!alreadyExists) {
+// if the path (at which the link is expected to be created) 
isn't already present
+// then we just go ahead and attempt to symlink
+try {
+Files.createSymbolicLink(link, target);
+} catch (IOException e) {
+if (failonerror) {
+throw new BuildException("Failed to create symlink " + 
lnk + " to target " + res, e);
 }
+log("Unable to create symlink " + lnk + " to target " + 
res, e, Project.MSG_INFO);
 }
+return;
+}
+// file already exists, see if we are allowed to overwrite
+if (!overwrite) {
+log("Skipping symlink creation, since file at " + lnk + " 
already exists and overwrite is set to false", Project.MSG_INFO);
+return;
+}
+// we have been asked to overwrite, so we now do the necessary 
steps
+
+// initiate a deletion *only* if the path is a symlink, else we 
fail with error
+if (!Files.isSymbolicLink(link)) {
+throw new BuildException("Cannot overwrite, as symlink, at " + 
lnk + " since the path already exists and isn't a symlink");
--- End diff --

should this take `failOnError` into account?


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #49: [master branch] - Fix BZ-58683

[bodewig]

Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/49#discussion_r155928069
  
--- Diff: src/main/org/apache/tools/ant/taskdefs/optional/unix/Symlink.java 
---
@@ -207,26 +198,30 @@ public void recreate() throws BuildException {
 try {
 if (fileSets.isEmpty()) {
 handleError(
-"File set identifying link file(s) required for action 
recreate");
+"File set identifying link file(s) required for 
action recreate");
 return;
 }
-Properties links = loadLinks(fileSets);
-
-for (String lnk : links.stringPropertyNames()) {
-String res = links.getProperty(lnk);
-// handle the case where lnk points to a directory (bug 
25181)
+final Properties links = loadLinks(fileSets);
+for (final String lnk : links.stringPropertyNames()) {
+final String res = links.getProperty(lnk);
 try {
-File test = new File(lnk);
-if (!SYMLINK_UTILS.isSymbolicLink(lnk)) {
-doLink(res, lnk);
-} else if (!test.getCanonicalPath().equals(
-new File(res).getCanonicalPath())) {
-SYMLINK_UTILS.deleteSymbolicLink(test, this);
-doLink(res, lnk);
-} // else lnk exists, do nothing
-} catch (IOException ioe) {
-handleError("IO exception while creating link");
+if (Files.isSymbolicLink(Paths.get(lnk)) &&
+
Paths.get(lnk).toFile().getCanonicalPath().equals(Paths.get(res).toFile().getCanonicalPath()))
 {
+// it's already a symlink and the symlink target 
is the same
+// as the target noted in the properties file. So 
there's no
+// need to recreate it
+continue;
+}
+} catch (IOException e) {
+if (failonerror) {
+throw new BuildException("Failed to recreate 
symlink " + lnk + " to target " + res, e);
+}
+// log and continue
+log("Failed to recreate symlink " + lnk + " to target 
" + res, Project.MSG_INFO);
+continue;
 }
+// create the link
+this.doLink(res, lnk);
--- End diff --

Just now see `doLink` doesn't throw any `IOException`. In this case the 
error message is wrong as the error occured while trying to check whether it is 
a (self-)link.


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #49: [master branch] - Fix BZ-58683

[bodewig]

Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/49#discussion_r155927463
  
--- Diff: src/main/org/apache/tools/ant/taskdefs/optional/unix/Symlink.java 
---
@@ -207,26 +198,30 @@ public void recreate() throws BuildException {
 try {
 if (fileSets.isEmpty()) {
 handleError(
-"File set identifying link file(s) required for action 
recreate");
+"File set identifying link file(s) required for 
action recreate");
 return;
 }
-Properties links = loadLinks(fileSets);
-
-for (String lnk : links.stringPropertyNames()) {
-String res = links.getProperty(lnk);
-// handle the case where lnk points to a directory (bug 
25181)
+final Properties links = loadLinks(fileSets);
+for (final String lnk : links.stringPropertyNames()) {
+final String res = links.getProperty(lnk);
 try {
-File test = new File(lnk);
-if (!SYMLINK_UTILS.isSymbolicLink(lnk)) {
-doLink(res, lnk);
-} else if (!test.getCanonicalPath().equals(
-new File(res).getCanonicalPath())) {
-SYMLINK_UTILS.deleteSymbolicLink(test, this);
-doLink(res, lnk);
-} // else lnk exists, do nothing
-} catch (IOException ioe) {
-handleError("IO exception while creating link");
+if (Files.isSymbolicLink(Paths.get(lnk)) &&
+
Paths.get(lnk).toFile().getCanonicalPath().equals(Paths.get(res).toFile().getCanonicalPath()))
 {
+// it's already a symlink and the symlink target 
is the same
+// as the target noted in the properties file. So 
there's no
+// need to recreate it
+continue;
+}
+} catch (IOException e) {
+if (failonerror) {
+throw new BuildException("Failed to recreate 
symlink " + lnk + " to target " + res, e);
+}
+// log and continue
+log("Failed to recreate symlink " + lnk + " to target 
" + res, Project.MSG_INFO);
+continue;
 }
+// create the link
+this.doLink(res, lnk);
--- End diff --

I think this should be inside of the `try` block.


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #49: [master branch] - Fix BZ-58683

[bodewig]

Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/49#discussion_r155927271
  
--- Diff: manual/Tasks/symlink.html ---
@@ -26,7 +26,7 @@
 
 Symlink
 Description
- Manages symbolic links on Unix based platforms. Can be used to
+ Manages symbolic links on platforms that support symbolic links. Can 
be used to
--- End diff --

rather "platform where Java supports symbolic links"? I'm sure people will 
yell at us as their platform supports some kind of symbolic link but Java 
doesn't support it.


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #49: [master branch] - Fix BZ-58683

[bodewig]

Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/49#discussion_r155927252
  
--- Diff: WHATSNEW ---
@@ -27,6 +27,11 @@ Fixed bugs:
copy happened to be the same source file (symlinked back
to itself).
 
+ * Bugzilla Report 58683 - Fixed the issue where symlink creation
--- End diff --

I overlooked this through your earlier changes to WHATSNEW. We use to add 
the bugzilla issue as last line of the added entry and I think it would be good 
to keep a common format.


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #50: Use newer Maven Ant tasks

[twogee]

GitHub user twogee opened a pull request:

https://github.com/apache/ant/pull/50

Use newer Maven Ant tasks

The initial aim was to avoid NPE when retrieving jasper and automate 
retrieval of ant-antunit (with exclusion of ant/ant-launcher).

Then I started wondering whether it could be possible to forgo JUnit 3 
altogether and update JUnit 4 to 4.12; if there's a good reason for keeping 
things the way they were, I'd be happy to add automated downloads of both 
versions and removal of JUnit 3 classes in JUnit 4.

Finally, jakarta regex is updated to the final version 1.4; AFACS, both 
versions have the same version of bytecode.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/twogee/ant master

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/ant/pull/50.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #50


commit 051f70e9e2bdf674d3216846c67fd3d2a17f4d25
Author: twogee 
Date:   2017-12-09T13:21:33Z

Use newer Maven Ant tasks




---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #49: [master branch] - Fix BZ-58683

[jaikiran]

GitHub user jaikiran opened a pull request:

https://github.com/apache/ant/pull/49

[master branch] - Fix BZ-58683 

The commit here fixes the issue reported at 
https://bz.apache.org/bugzilla/show_bug.cgi?id=58683.

This commit along with fixing the issue reported in that bug, also has 
updated the `Symlink` task to now start relying on the Java 7 support of 
symlinking through the use of `java.nio.file.Files` APIs. This now allows the 
task to move away from spawning process(es) for dealing with symlinks and also 
allows this task to be functional on non-unix systems which support symbolic 
links.

P.S: For 1.9.x branch, I can create a separate commit, without using Java 7 
APIs to fix this bug, if we do want to do that there. Let me know.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/jaikiran/ant bz-58683-master

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/ant/pull/49.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #49


commit 95b38bb8800f7608d8ee94866a0c9c532b06a498
Author: Jaikiran Pai 
Date:   2017-12-09T07:32:56Z

BZ-58683 Honour the overwrite=false for existing symlinks. Plus, use Java 7 
java.nio.file.Files support for symbolinks, in symlink task




---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #:

[bodewig]

Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/68b846a4c27361cce2b6d7e9c60ca697f980913e#commitcomment-25935605
  
In src/main/org/apache/tools/ant/taskdefs/Javac.java:
In src/main/org/apache/tools/ant/taskdefs/Javac.java on line 778:
bigger not bigger or equal :-)

TBH it first read bigger than 5 until I stiopped and had second thoughts.


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #:

[twogee]

Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/68b846a4c27361cce2b6d7e9c60ca697f980913e#commitcomment-25932714
  
In src/main/org/apache/tools/ant/taskdefs/Javac.java:
In src/main/org/apache/tools/ant/taskdefs/Javac.java on line 778:
For jdk 1.4? 😄 


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #48: BZ-61718 Upgrade to 0.1.54 of Jsch library

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/ant/pull/48


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #48: BZ-61718 Upgrade to 0.1.54 of Jsch library

[jaikiran]

GitHub user jaikiran opened a pull request:

https://github.com/apache/ant/pull/48

BZ-61718 Upgrade to 0.1.54 of Jsch library

The commit here upgrades Jsch library dependency to `0.1.54` (which is the 
latest released version) as requested in 
https://bz.apache.org/bugzilla/show_bug.cgi?id=61718. This PR is against 1.9.x 
branch and if approved, I'll merge the change to master branch too.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/jaikiran/ant bz61718

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/ant/pull/48.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #48


commit d6e3296ad1461d2f86616ed35baf577475029ef5
Author: Jaikiran Pai 
Date:   2017-11-05T02:42:56Z

BZ-61718 Upgrade to 0.1.54 of Jsch library




---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #47: Add the "final" modifier to a public static field.

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/ant/pull/47


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #44: Remove the 'protected' modifier to make fields package...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/ant/pull/44


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #46: Remove the redundant nullcheck of non-null values.

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/ant/pull/46


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #47: Add the "final" modifier to a public static field.

[BruceKuiLiu]

GitHub user BruceKuiLiu opened a pull request:

https://github.com/apache/ant/pull/47

Add the "final" modifier to a public static field.

This static field public but not final, and could be changed by malicious 
code or by accident from another package. The field could be made final to 
avoid this vulnerability.
http://findbugs.sourceforge.net/bugDescriptions.html#MS_SHOULD_BE_FINAL

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/BruceKuiLiu/ant master4

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/ant/pull/47.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #47


commit ab72f9071269121d5b9fd7e38130a3ae4a8ed1a9
Author: Kui LIU 
Date:   2017-10-14T14:45:32Z

Add the "final" modifier to a public static field.

This static field public but not final, and could be changed by malicious 
code or by accident from another package. The field could be made final to 
avoid this vulnerability.
http://findbugs.sourceforge.net/bugDescriptions.html#MS_SHOULD_BE_FINAL




---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #46: Remove the redundant nullcheck of non-null values.

[BruceKuiLiu]

GitHub user BruceKuiLiu opened a pull request:

https://github.com/apache/ant/pull/46

Remove the redundant nullcheck of non-null values.


These statements contain redundant check of known non-null values against 
the constant null.

http://findbugs.sourceforge.net/bugDescriptions.html#RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/BruceKuiLiu/ant master7

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/ant/pull/46.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #46


commit 81c97dbbc5350d39ed349662cb19b621aee55e34
Author: Kui LIU 
Date:   2017-10-14T15:20:29Z

Remove the redundant nullcheck of value known to be non-null.

This statement contains a redundant check of a known non-null s against the 
constant null.

http://findbugs.sourceforge.net/bugDescriptions.html#RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE

commit c1172b574a7b5a3832bd317b4e76e49e1303b78c
Author: Kui LIU 
Date:   2017-10-14T15:24:54Z

Remove the redundant nullcheck statements of non-null values.

These statements contain redundant check of known non-null values against 
the constant null.

http://findbugs.sourceforge.net/bugDescriptions.html#RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE




---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #45: Consider returning a zero length array rather than nul...

[BruceKuiLiu]

GitHub user BruceKuiLiu opened a pull request:

https://github.com/apache/ant/pull/45

Consider returning a zero length array rather than null.

It is often a better design to return a length zero array rather than a 
null reference to indicate that there are no results (i.e., an empty list of 
results).
This way, no explicit check for null is needed by clients of the method.
On the other hand, using null to indicate "there is no answer to this 
question" is probably appropriate.

http://findbugs.sourceforge.net/bugDescriptions.html#PZLA_PREFER_ZERO_LENGTH_ARRAYS

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/BruceKuiLiu/ant master6

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/ant/pull/45.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #45


commit ca2fbf36183b82457fe6d161b77a6b10ae2e457f
Author: Kui LIU 
Date:   2017-10-14T15:06:08Z

Consider returning a zero length array rather than null.

It is often a better design to return a length zero array rather than a 
null reference to indicate that there are no results (i.e., an empty list of 
results).
This way, no explicit check for null is needed by clients of the method.
On the other hand, using null to indicate "there is no answer to this 
question" is probably appropriate.

http://findbugs.sourceforge.net/bugDescriptions.html#PZLA_PREFER_ZERO_LENGTH_ARRAYS




---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #44: Remove the 'protected' modifier to make fields package...

[BruceKuiLiu]

GitHub user BruceKuiLiu opened a pull request:

https://github.com/apache/ant/pull/44

Remove the 'protected' modifier to make fields package protected.

The three mutable static fields could be changed by malicious code or by 
accident. These fields could be made package protected to avoid this 
vulnerability.
http://findbugs.sourceforge.net/bugDescriptions.html#MS_PKGPROTECT

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/BruceKuiLiu/ant master5

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/ant/pull/44.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #44


commit a48234563c7a82642e0e079bc8aedec68fd3c4c2
Author: Kui LIU 
Date:   2017-10-14T14:29:11Z

Remove the 'protected' modifier to make fields package protected.

The three mutable static fields could be changed by malicious code or by 
accident. These fields could be made package protected to avoid this 
vulnerability.
http://findbugs.sourceforge.net/bugDescriptions.html#MS_PKGPROTECT




---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #43: Fix the problem of using '+=' operator to concatenate ...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/ant/pull/43


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #42: Fix the inefficient use of keySet iterator with entryS...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/ant/pull/42


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #43: Fix the problem of using '+=' operator to concatenate ...

[BruceKuiLiu]

GitHub user BruceKuiLiu opened a pull request:

https://github.com/apache/ant/pull/43

Fix the problem of using '+=' operator to concatenate strings in a loop

The method is building a String using concatenation in a loop.
In each iteration, the String is converted to a StringBuilder, appended to, 
and converted back to a String.
This can lead to a cost quadratic in the number of iterations, as the 
growing string is recopied in each iteration.
Better performance can be obtained by using a StringBuilder explicitly.

http://findbugs.sourceforge.net/bugDescriptions.html#SBSC_USE_STRINGBUFFER_CONCATENATION

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/BruceKuiLiu/ant master3

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/ant/pull/43.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #43


commit cc37652438f3ff345af78b6dc45e8108f02bd402
Author: Kui LIU 
Date:   2017-10-11T13:01:48Z

Fix the problem of using '+=' operator to concatenate strings a in a loop.

The method is building a String using concatenation in a loop.
In each iteration, the String is converted to a StringBuilder, appended to, 
and converted back to a String.
This can lead to a cost quadratic in the number of iterations, as the 
growing string is recopied in each iteration.
Better performance can be obtained by using a StringBuilder explicitly.

http://findbugs.sourceforge.net/bugDescriptions.html#SBSC_USE_STRINGBUFFER_CONCATENATION




---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #42: Fix the inefficient use of keySet iterator with entryS...

[BruceKuiLiu]

GitHub user BruceKuiLiu opened a pull request:

https://github.com/apache/ant/pull/42

Fix the inefficient use of keySet iterator with entrySet iterator.


The current source code accesses the key and value of a Hashtable entry, 
using a key that is retrieved from a keySet iterator.
It is more efficient to use an iterator on the entrySet of the Hashtable, 
to avoid the Map.get(key) lookup.
http://findbugs.sourceforge.net/bugDescriptions.html#WMI_WRONG_MAP_ITERATOR

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/BruceKuiLiu/ant master2

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/ant/pull/42.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #42


commit fb817dd2c4b01a1840f1c06940c71de3ec60ce0a
Author: Kui LIU 
Date:   2017-10-11T12:18:19Z

Fix the inefficient use of keySet iterator with entrySet iterator.

The current source code accesses the key and value of a Hashtable entry, 
using a key that is retrieved from a keySet iterator.
It is more efficient to use an iterator on the entrySet of the Hashtable, 
to avoid the Map.get(key) lookup.

commit ab65a09dc00d9baa55a088dbffeec414fbda9bc9
Author: Kui LIU 
Date:   2017-10-11T12:42:46Z

Fix the inefficient use of keySet iterator with entrySet iterator.

The current source code accesses the key and value of a Hashtable entry, 
using a key that is retrieved from a keySet iterator.
It is more efficient to use an iterator on the entrySet of the Hashtable, 
to avoid the Map.get(key) lookup.
http://findbugs.sourceforge.net/bugDescriptions.html#WMI_WRONG_MAP_ITERATOR

commit 6a9a466b275b553637277b980139b6297a166483
Author: Kui LIU 
Date:   2017-10-11T12:51:12Z

Fix the inefficient use of keySet iterator with entrySet iterator.

The current source code accesses the key and value of a Hashtable entry, 
using a key that is retrieved from a keySet iterator.
It is more efficient to use an iterator on the entrySet of the Hashtable, 
to avoid the Map.get(key) lookup.
http://findbugs.sourceforge.net/bugDescriptions.html#WMI_WRONG_MAP_ITERATOR




---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] ant pull request #41: Fix the problem of instanceof test always return true.

[BruceKuiLiu]

Github user BruceKuiLiu closed the pull request at:

https://github.com/apache/ant/pull/41


---

-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org