[GitHub] ant pull request #:

2018-12-16 Thread twogee
Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/343dff90f2a06a14a5e3d8f787de87d4446af3d9#commitcomment-31692461
  
In src/main/org/apache/tools/ant/types/ModuleVersion.java:
In src/main/org/apache/tools/ant/types/ModuleVersion.java on line 51:
no @param for number


---

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



[GitHub] ant pull request #:

2018-12-16 Thread bodewig
Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/343dff90f2a06a14a5e3d8f787de87d4446af3d9#commitcomment-31692386
  
fixed with 876376a8d - many thanks for catching this @twogee 


---

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



[GitHub] ant pull request #:

2018-12-16 Thread twogee
Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/343dff90f2a06a14a5e3d8f787de87d4446af3d9#commitcomment-31692367
  
Thanks, perhaps it's time for [multi-release 
jars](https://maven.apache.org/plugins/maven-compiler-plugin/multirelease.html)?


---

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



[GitHub] ant pull request #:

2018-12-16 Thread bodewig
Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/343dff90f2a06a14a5e3d8f787de87d4446af3d9#commitcomment-31692317
  
I'll take care of it.


---

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



[GitHub] ant pull request #:

2018-12-15 Thread twogee
Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/343dff90f2a06a14a5e3d8f787de87d4446af3d9#commitcomment-31691241
  
Yes, that's the idea.


---

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



[GitHub] ant pull request #:

2018-12-15 Thread craigpell
Github user craigpell commented on the pull request:


https://github.com/apache/ant/commit/343dff90f2a06a14a5e3d8f787de87d4446af3d9#commitcomment-31691208
  
It appears I can just add 
`org/apache/tools/ant/taskdefs/modules/` to the excludes and 
testExcludes elements to do that, correct?  (I don't particularly want to 
install Maven, but if I need to in order to test this, I will.)


---

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



[GitHub] ant pull request #:

2018-12-15 Thread twogee
Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/343dff90f2a06a14a5e3d8f787de87d4446af3d9#commitcomment-31691133
  
`src/etc/poms/ant/pom.xml` should exclude the new tasks from compilation; 
Maven builds are Java 8 only.


---

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



[GitHub] ant pull request #80: Added tasks for JDK's jmod and jlink tools.

2018-12-15 Thread asfgit
Github user asfgit closed the pull request at:

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


---

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



[GitHub] ant pull request #80: Added tasks for JDK's jmod and jlink tools.

2018-12-12 Thread bodewig
Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/80#discussion_r241292280
  
--- Diff: src/main/org/apache/tools/ant/taskdefs/modules/Jmod.java ---
@@ -0,0 +1,1282 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You under the Apache License, Version 2.0
+ *  (the "License"); you may not use this file except in compliance with
+ *  the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or 
implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ *
+ */
+
+package org.apache.tools.ant.taskdefs.modules;
+
+import java.io.File;
+import java.io.ByteArrayOutputStream;
+import java.io.PrintStream;
+import java.io.IOException;
+
+import java.nio.file.Files;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.ArrayList;
+
+import java.util.Map;
+import java.util.LinkedHashMap;
+
+import java.util.Collections;
+
+import java.util.spi.ToolProvider;
+
+import org.apache.tools.ant.BuildException;
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.Task;
+
+import org.apache.tools.ant.util.MergingMapper;
+import org.apache.tools.ant.util.FileUtils;
+import org.apache.tools.ant.util.ResourceUtils;
+
+import org.apache.tools.ant.types.EnumeratedAttribute;
+import org.apache.tools.ant.types.FileSet;
+import org.apache.tools.ant.types.ModuleVersion;
+import org.apache.tools.ant.types.Path;
+import org.apache.tools.ant.types.Reference;
+import org.apache.tools.ant.types.Resource;
+import org.apache.tools.ant.types.ResourceCollection;
+
+import org.apache.tools.ant.types.resources.FileResource;
+import org.apache.tools.ant.types.resources.Union;
+
+/**
+ * Creates a linkable .jmod file from a modular jar file, and optionally 
from
+ * other resource files such as native libraries and documents.  Equivalent
+ * to the JDK's
+ * https://docs.oracle.com/en/java/javase/11/tools/jmod.html;>jmod
+ * tool.
+ * 
+ * Supported attributes:
+ * 
+ * {@code destFile}
+ * Required, jmod file to create.
+ * {@code classpath}
+ * {@code classpathref}
+ * Where to locate files to be placed in the jmod file.
+ * {@code modulepath}
+ * {@code modulepathref}
+ * Where to locate dependencies.
+ * {@code commandpath}
+ * {@code commandpathref}
+ * Directories containing native commands to include in jmod.
+ * {@code headerpath}
+ * {@code headerpathref}
+ * Directories containing header files to include in jmod.
+ * {@code configpath}
+ * {@code configpathref}
+ * Directories containing user-editable configuration files
+ * to include in jmod.
+ * {@code legalpath}
+ * {@code legalpathref}
+ * Directories containing legal licenses and notices to include in 
jmod.
+ * {@code nativelibpath}
+ * {@code nativelibpathref}
+ * Directories containing native libraries to include in jmod.
+ * {@code manpath}
+ * {@code manpathref}
+ * Directories containing man pages to include in jmod.
+ * {@code version}
+ * Module https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/module/ModuleDescriptor.Version.html;>version.
+ * {@code mainclass}
+ * Main class of module.
+ * {@code platform}
+ * The target platform for the jmod.  A particular JDK's platform
+ * can be seen by running
+ * jmod describe $JDK_HOME/jmods/java.base.jmod | grep -i 
platform.
+ * {@code hashModulesPattern}
+ * Regular expression for names of modules in the module path
+ * which depend on the jmod being created, and which should have
+ * hashes generated for them and included in the new jmod.
+ * {@code resolveByDefault}
+ * Boolean indicating whether the jmod should be one of
+ * the default resolved modules in an application.  Default is true.
+ * {@code moduleWarnings}
+ * Whether to emit warnings when resolving modules which are
+ * not recommended for use.  Comma-separated list of one of more of
+ * the following:
+ * 
+ * {@code deprecated}
+ * Warn if module is deprecated
+ * {@code leaving}
+ * Warn if module is deprecated for 

[GitHub] ant pull request #80: Added tasks for JDK's jmod and jlink tools.

2018-12-12 Thread craigpell
Github user craigpell commented on a diff in the pull request:

https://github.com/apache/ant/pull/80#discussion_r241010770
  
--- Diff: src/main/org/apache/tools/ant/taskdefs/modules/Jmod.java ---
@@ -0,0 +1,1282 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You under the Apache License, Version 2.0
+ *  (the "License"); you may not use this file except in compliance with
+ *  the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or 
implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ *
+ */
+
+package org.apache.tools.ant.taskdefs.modules;
+
+import java.io.File;
+import java.io.ByteArrayOutputStream;
+import java.io.PrintStream;
+import java.io.IOException;
+
+import java.nio.file.Files;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.ArrayList;
+
+import java.util.Map;
+import java.util.LinkedHashMap;
+
+import java.util.Collections;
+
+import java.util.spi.ToolProvider;
+
+import org.apache.tools.ant.BuildException;
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.Task;
+
+import org.apache.tools.ant.util.MergingMapper;
+import org.apache.tools.ant.util.FileUtils;
+import org.apache.tools.ant.util.ResourceUtils;
+
+import org.apache.tools.ant.types.EnumeratedAttribute;
+import org.apache.tools.ant.types.FileSet;
+import org.apache.tools.ant.types.ModuleVersion;
+import org.apache.tools.ant.types.Path;
+import org.apache.tools.ant.types.Reference;
+import org.apache.tools.ant.types.Resource;
+import org.apache.tools.ant.types.ResourceCollection;
+
+import org.apache.tools.ant.types.resources.FileResource;
+import org.apache.tools.ant.types.resources.Union;
+
+/**
+ * Creates a linkable .jmod file from a modular jar file, and optionally 
from
+ * other resource files such as native libraries and documents.  Equivalent
+ * to the JDK's
+ * https://docs.oracle.com/en/java/javase/11/tools/jmod.html;>jmod
+ * tool.
+ * 
+ * Supported attributes:
+ * 
+ * {@code destFile}
+ * Required, jmod file to create.
+ * {@code classpath}
+ * {@code classpathref}
+ * Where to locate files to be placed in the jmod file.
+ * {@code modulepath}
+ * {@code modulepathref}
+ * Where to locate dependencies.
+ * {@code commandpath}
+ * {@code commandpathref}
+ * Directories containing native commands to include in jmod.
+ * {@code headerpath}
+ * {@code headerpathref}
+ * Directories containing header files to include in jmod.
+ * {@code configpath}
+ * {@code configpathref}
+ * Directories containing user-editable configuration files
+ * to include in jmod.
+ * {@code legalpath}
+ * {@code legalpathref}
+ * Directories containing legal licenses and notices to include in 
jmod.
+ * {@code nativelibpath}
+ * {@code nativelibpathref}
+ * Directories containing native libraries to include in jmod.
+ * {@code manpath}
+ * {@code manpathref}
+ * Directories containing man pages to include in jmod.
+ * {@code version}
+ * Module https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/module/ModuleDescriptor.Version.html;>version.
+ * {@code mainclass}
+ * Main class of module.
+ * {@code platform}
+ * The target platform for the jmod.  A particular JDK's platform
+ * can be seen by running
+ * jmod describe $JDK_HOME/jmods/java.base.jmod | grep -i 
platform.
+ * {@code hashModulesPattern}
+ * Regular expression for names of modules in the module path
+ * which depend on the jmod being created, and which should have
+ * hashes generated for them and included in the new jmod.
+ * {@code resolveByDefault}
+ * Boolean indicating whether the jmod should be one of
+ * the default resolved modules in an application.  Default is true.
+ * {@code moduleWarnings}
+ * Whether to emit warnings when resolving modules which are
+ * not recommended for use.  Comma-separated list of one of more of
+ * the following:
+ * 
+ * {@code deprecated}
+ * Warn if module is deprecated
+ * {@code leaving}
+ * Warn if module is deprecated for 

[GitHub] ant pull request #81: Fix rare ConcurrentModificationException when running ...

2018-12-11 Thread asfgit
Github user asfgit closed the pull request at:

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


---

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



[GitHub] ant pull request #80: Added tasks for JDK's jmod and jlink tools.

2018-12-11 Thread jaikiran
Github user jaikiran commented on a diff in the pull request:

https://github.com/apache/ant/pull/80#discussion_r240889855
  
--- Diff: src/main/org/apache/tools/ant/taskdefs/modules/Jmod.java ---
@@ -0,0 +1,1282 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You under the Apache License, Version 2.0
+ *  (the "License"); you may not use this file except in compliance with
+ *  the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or 
implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ *
+ */
+
+package org.apache.tools.ant.taskdefs.modules;
+
+import java.io.File;
+import java.io.ByteArrayOutputStream;
+import java.io.PrintStream;
+import java.io.IOException;
+
+import java.nio.file.Files;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.ArrayList;
+
+import java.util.Map;
+import java.util.LinkedHashMap;
+
+import java.util.Collections;
+
+import java.util.spi.ToolProvider;
+
+import org.apache.tools.ant.BuildException;
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.Task;
+
+import org.apache.tools.ant.util.MergingMapper;
+import org.apache.tools.ant.util.FileUtils;
+import org.apache.tools.ant.util.ResourceUtils;
+
+import org.apache.tools.ant.types.EnumeratedAttribute;
+import org.apache.tools.ant.types.FileSet;
+import org.apache.tools.ant.types.ModuleVersion;
+import org.apache.tools.ant.types.Path;
+import org.apache.tools.ant.types.Reference;
+import org.apache.tools.ant.types.Resource;
+import org.apache.tools.ant.types.ResourceCollection;
+
+import org.apache.tools.ant.types.resources.FileResource;
+import org.apache.tools.ant.types.resources.Union;
+
+/**
+ * Creates a linkable .jmod file from a modular jar file, and optionally 
from
+ * other resource files such as native libraries and documents.  Equivalent
+ * to the JDK's
+ * https://docs.oracle.com/en/java/javase/11/tools/jmod.html;>jmod
+ * tool.
+ * 
+ * Supported attributes:
+ * 
+ * {@code destFile}
+ * Required, jmod file to create.
+ * {@code classpath}
+ * {@code classpathref}
+ * Where to locate files to be placed in the jmod file.
+ * {@code modulepath}
+ * {@code modulepathref}
+ * Where to locate dependencies.
+ * {@code commandpath}
+ * {@code commandpathref}
+ * Directories containing native commands to include in jmod.
+ * {@code headerpath}
+ * {@code headerpathref}
+ * Directories containing header files to include in jmod.
+ * {@code configpath}
+ * {@code configpathref}
+ * Directories containing user-editable configuration files
+ * to include in jmod.
+ * {@code legalpath}
+ * {@code legalpathref}
+ * Directories containing legal licenses and notices to include in 
jmod.
+ * {@code nativelibpath}
+ * {@code nativelibpathref}
+ * Directories containing native libraries to include in jmod.
+ * {@code manpath}
+ * {@code manpathref}
+ * Directories containing man pages to include in jmod.
+ * {@code version}
+ * Module https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/module/ModuleDescriptor.Version.html;>version.
+ * {@code mainclass}
+ * Main class of module.
+ * {@code platform}
+ * The target platform for the jmod.  A particular JDK's platform
+ * can be seen by running
+ * jmod describe $JDK_HOME/jmods/java.base.jmod | grep -i 
platform.
+ * {@code hashModulesPattern}
+ * Regular expression for names of modules in the module path
+ * which depend on the jmod being created, and which should have
+ * hashes generated for them and included in the new jmod.
+ * {@code resolveByDefault}
+ * Boolean indicating whether the jmod should be one of
+ * the default resolved modules in an application.  Default is true.
+ * {@code moduleWarnings}
+ * Whether to emit warnings when resolving modules which are
+ * not recommended for use.  Comma-separated list of one of more of
+ * the following:
+ * 
+ * {@code deprecated}
+ * Warn if module is deprecated
+ * {@code leaving}
+ * Warn if module is deprecated for 

[GitHub] ant pull request #81: Fix rare ConcurrentModificationException when running ...

2018-12-11 Thread mharmer
GitHub user mharmer opened a pull request:

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

Fix rare ConcurrentModificationException when running with Parallel-Ant 
executor.

This resolves a rare race condition when running with the 
[Parallel-Ant](https://github.com/codeaholics/parallel-ant) executor.

This seems to be triggered concurrently when a reference was being added to 
the project at the same time the references were being copied through 
Project.getCopyOfReferences(). The stack trace observed in this case was:
```
java.util.ConcurrentModificationException
at java.util.Hashtable$Enumerator.next(Hashtable.java:1387)
at java.util.HashMap.putMapEntries(HashMap.java:512)
at java.util.HashMap.(HashMap.java:490)
at 
org.apache.tools.ant.Project.getCopyOfReferences(Project.java:2038)
at 
org.apache.tools.ant.util.ScriptRunnerBase.bindToComponent(ScriptRunnerBase.java:307)
at 
org.apache.tools.ant.util.ScriptRunnerHelper.getScriptRunner(ScriptRunnerHelper.java:66)
at 
org.apache.tools.ant.taskdefs.optional.Script.execute(Script.java:53)
at 
org.apache.tools.ant.UnknownElement.execute(UnknownElement.java:293)
at sun.reflect.GeneratedMethodAccessor4.invoke(Unknown Source)
at 
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at 
org.apache.tools.ant.dispatch.DispatchUtils.execute(DispatchUtils.java:106)
at org.apache.tools.ant.Task.perform(Task.java:348)
at org.apache.tools.ant.Target.execute(Target.java:435)
at org.apache.tools.ant.Target.performTasks(Target.java:456)
at 
org.codeaholics.tools.build.pant.AntWrapperImpl.executeTarget(Unknown Source)
at 
org.codeaholics.tools.build.pant.DependencyGraphEntry.run(Unknown Source)
at 
java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
at java.util.concurrent.FutureTask.run(FutureTask.java:266)
at 
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
at 
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
at java.lang.Thread.run(Thread.java:748)
```

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

$ git pull https://github.com/pc-doctor/ant ConcurrentModificationReferences

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

https://github.com/apache/ant/pull/81.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 #81


commit 01b9073218c9b2f067539bc9f5b21340bf6abd6f
Author: mharmer 
Date:   2018-12-11T22:02:37Z

Fixing a potential ConcurrentModificationException that could occur when 
running Ant with the Parallel-Ant executor.




---

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



[GitHub] ant pull request #80: Added tasks for JDK's jmod and jlink tools.

2018-12-06 Thread craigpell
GitHub user craigpell opened a pull request:

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

Added tasks for JDK's jmod and jlink tools.

Support for the jmod and jlink tools present in the JDK since Java 9.  Now 
that Java 11 has no standalone JRE, officially, these tools are the only way to 
distribute client-side Java applications.

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

$ git pull https://github.com/craigpell/ant jmod-and-jlink

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

https://github.com/apache/ant/pull/80.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 #80


commit 3bc80754d5063b88c3a8b044e9774c0a791ac2ff
Author: VGR 
Date:   2018-12-07T04:32:57Z

Added tasks for JDK's jmod and jlink tools.




---

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



[GitHub] ant pull request #79: Make DataType and Reference generic

2018-11-19 Thread twogee
Github user twogee closed the pull request at:

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


---

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



[GitHub] ant pull request #78: A new CharSet type to hold available Charset names

2018-11-14 Thread twogee
Github user twogee closed the pull request at:

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


---

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



[GitHub] ant pull request #77: Avoid FileInputStream and FileOutputStream.

2018-11-14 Thread asfgit
Github user asfgit closed the pull request at:

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


---

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



[GitHub] ant pull request #78: A new CharSet type to hold available Charset names

2018-11-12 Thread twogee
Github user twogee commented on a diff in the pull request:

https://github.com/apache/ant/pull/78#discussion_r232795363
  
--- Diff: src/tests/junit/org/apache/tools/ant/types/CharSetTest.java ---
@@ -0,0 +1,20 @@
+package org.apache.tools.ant.types;
+
+import org.apache.tools.ant.BuildException;
+import org.junit.Test;
+
+import java.util.Arrays;
+
+public class CharSetTest {
+@Test
+public void testCorrectNames() {
+String[] expected = {"UTF-8", "ISO-8859-1", "037", "us", "IBM500"};
+Arrays.stream(expected).forEach(new CharSet()::setValue);
+}
+
+@Test(expected = BuildException.class)
+public void testNonExistentNames() {
+String[] nonexistent = {"mojibake", "dummy"};
+Arrays.stream(nonexistent).forEach(new CharSet()::setValue);
--- End diff --

I was lazy... 😁 the tests are properly parameterised now


---

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



[GitHub] ant pull request #78: A new CharSet type to hold available Charset names

2018-11-12 Thread bodewig
Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/78#discussion_r232673446
  
--- Diff: src/tests/junit/org/apache/tools/ant/types/CharSetTest.java ---
@@ -0,0 +1,20 @@
+package org.apache.tools.ant.types;
+
+import org.apache.tools.ant.BuildException;
+import org.junit.Test;
+
+import java.util.Arrays;
+
+public class CharSetTest {
+@Test
+public void testCorrectNames() {
+String[] expected = {"UTF-8", "ISO-8859-1", "037", "us", "IBM500"};
+Arrays.stream(expected).forEach(new CharSet()::setValue);
+}
+
+@Test(expected = BuildException.class)
+public void testNonExistentNames() {
+String[] nonexistent = {"mojibake", "dummy"};
+Arrays.stream(nonexistent).forEach(new CharSet()::setValue);
--- End diff --

is the test for "dummy" ever going to be run?


---

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



[GitHub] ant pull request #79: Make DataType and Reference generic

2018-11-12 Thread bodewig
Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/79#discussion_r232669831
  
--- Diff: src/main/org/apache/tools/ant/types/TarFileSet.java ---
@@ -263,9 +273,7 @@ public Object clone() {
 private void checkTarFileSetAttributesAllowed() {
 if (getProject() == null
 || (isReference()
-&& (getRefid().getReferencedObject(
--- End diff --

please keep whitespace changes separate from PRs (or better just don't 
create them at all)


---

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



[GitHub] ant pull request #79: Make DataType and Reference generic

2018-11-12 Thread bodewig
Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/79#discussion_r232669396
  
--- Diff: src/main/org/apache/tools/ant/types/PropertySet.java ---
@@ -424,8 +424,8 @@ private void addPropertyNames(Set names, 
Map props) {
  * referenced PropertySet.
  * @return the referenced PropertySet.
  */
-protected PropertySet getRef() {
-return getCheckedRef(PropertySet.class, "propertyset");
+private PropertySet getRef() {
--- End diff --

this is not backwards compatible


---

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



[GitHub] ant pull request #79: Make DataType and Reference generic

2018-11-12 Thread bodewig
Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/79#discussion_r232669259
  
--- Diff: src/main/org/apache/tools/ant/types/Path.java ---
@@ -721,7 +721,7 @@ public synchronized boolean isFilesystemOnly() {
  * @return the passed in ResourceCollection.
  */
 protected ResourceCollection assertFilesystemOnly(ResourceCollection 
rc) {
-if (rc != null && !(rc.isFilesystemOnly())) {
+if (rc != null && !rc.isFilesystemOnly()) {
--- End diff --

please keep style changes separate from PRs (or better just don't create 
them at all)


---

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



[GitHub] ant pull request #79: Make DataType and Reference generic

2018-11-12 Thread bodewig
Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/79#discussion_r232669102
  
--- Diff: src/main/org/apache/tools/ant/types/FilterSet.java ---
@@ -235,8 +235,8 @@ protected FilterSet(FilterSet filterset) {
  *
  * @return the filterset from the reference.
  */
-protected FilterSet getRef() {
--- End diff --

this is not backwards compatible


---

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



[GitHub] ant pull request #79: Make DataType and Reference generic

2018-11-12 Thread bodewig
Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/79#discussion_r232668570
  
--- Diff: src/main/org/apache/tools/ant/types/FileList.java ---
@@ -137,13 +144,10 @@ public void setFiles(String filenames) {
 }
 
 /**
- * Performs the check for circular references and returns the
- * referenced FileList.
- * @param p the current project
- * @return the FileList represented by a referenced filelist.
+ * @return the list of files represented by this FileList.
  */
-protected FileList getRef(Project p) {
-return (FileList) getCheckedRef(p);
+public String[] getFiles() {
--- End diff --

please remove the unrelated new method


---

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



[GitHub] ant pull request #79: Make DataType and Reference generic

2018-11-12 Thread bodewig
Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/79#discussion_r232668389
  
--- Diff: src/main/org/apache/tools/ant/types/FileList.java ---
@@ -98,6 +98,13 @@ public File getDir(Project p) {
 return dir;
 }
 
+/**
--- End diff --

please remove the unrelated new method


---

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



[GitHub] ant pull request #79: Make DataType and Reference generic

2018-11-12 Thread bodewig
Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/79#discussion_r232667496
  
--- Diff: src/main/org/apache/tools/ant/types/ArchiveFileSet.java ---
@@ -574,11 +570,8 @@ public int getDirMode() {
  * fileset if the project has been set).
  */
 private void checkArchiveAttributesAllowed() {
-if (getProject() == null
-|| (isReference()
-&& (getRefid().getReferencedObject(
-getProject())
-instanceof ArchiveFileSet))) {
+if (getProject() == null || (isReference()
+&& getRefid().getReferencedObject(getProject()) instanceof 
ArchiveFileSet)) {
--- End diff --

please keep whitespace changes separate from PRs (or better just don't 
create them at all)


---

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



[GitHub] ant pull request #79: Make DataType and Reference generic

2018-11-12 Thread bodewig
Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/79#discussion_r232667773
  
--- Diff: src/main/org/apache/tools/ant/types/DataType.java ---
@@ -196,24 +196,44 @@ public static void 
pushAndInvokeCircularReferenceCheck(DataType dt,
 /**
  * Performs the check for circular references and returns the
  * referenced object.
+ * @param  required reference type
  * @return the dereferenced object.
  * @throws BuildException if the reference is invalid (circular ref, 
wrong class, etc).
  * @since Ant 1.7
+ * @deprecated use getCheckedRef(Class)
  */
-protected Object getCheckedRef() {
+@Deprecated
+protected  T getCheckedRef() {
 return getCheckedRef(getProject());
 }
 
 /**
  * Performs the check for circular references and returns the
  * referenced object.
+ * @param  required reference type
+ * @param requiredClass the class that this reference should be a 
subclass of.
+ * @return the dereferenced object.
+ * @throws BuildException if the reference is invalid (circular ref, 
wrong class, etc).
+ * @since Ant 1.10
--- End diff --

needs the patch version


---

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



[GitHub] ant pull request #79: Make DataType and Reference generic

2018-11-12 Thread bodewig
Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/79#discussion_r232666118
  
--- Diff: src/main/org/apache/tools/ant/types/AbstractFileSet.java ---
@@ -915,7 +920,7 @@ public String toString() {
 @Override
 public synchronized Object clone() {
 if (isReference()) {
-return (getRef(getProject())).clone();
+return getRef(getProject()).clone();
--- End diff --

please keep style changes separate from PRs (or better just don't create 
them at all)


---

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



[GitHub] ant pull request #79: Make DataType and Reference generic

2018-11-12 Thread bodewig
Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/79#discussion_r232665835
  
--- Diff: 
src/main/org/apache/tools/ant/taskdefs/optional/extension/ExtensionSet.java ---
@@ -34,8 +34,7 @@
  *
  * @ant.datatype name="extension-set"
  */
-public class ExtensionSet
-extends DataType {
+public class ExtensionSet extends DataType {
--- End diff --

please keep whitespace changes separate from PRs (or better just don't 
create them at all)



---

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



[GitHub] ant pull request #79: Make DataType and Reference generic

2018-11-06 Thread twogee
GitHub user twogee opened a pull request:

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

Make DataType and Reference generic



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

$ git pull https://github.com/twogee/ant checked-reference

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

https://github.com/apache/ant/pull/79.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 #79


commit 92fa54249840615df5658c5eb4b27bdb0700810f
Author: twogee 
Date:   2018-08-26T05:42:26Z

Make DataType and Reference generic

commit 53dfa2cd3168cd3bba10ef4dd526add2b7169721
Author: twogee 
Date:   2018-11-07T06:48:44Z

Deprecate the old API




---

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



[GitHub] ant pull request #78: A new CharSet type to hold available Charset names

2018-11-06 Thread twogee
GitHub user twogee opened a pull request:

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

A new CharSet type to hold available Charset names

I believe that might be useful when validating "encoding" (or "charset") 
attributes

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

$ git pull https://github.com/twogee/ant charset-type

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

https://github.com/apache/ant/pull/78.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 #78


commit 033d68c7f6ecf382f84eec5ab0c2bb91eb9bd6fb
Author: twogee 
Date:   2018-11-06T21:27:55Z

A new CharSet type to hold available Charset names




---

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



[GitHub] ant pull request #77: Avoid FileInputStream and FileOutputStream.

2018-11-04 Thread reudismam
GitHub user reudismam opened a pull request:

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

Avoid FileInputStream and FileOutputStream.

Avoid FileInputStream and FileOutputStream. These classes override the 
finalize method. As a result, their objects are only cleaned when the garbage 
collector performs a sweep. Since Java 7, programmers can use 
Files.newInputStream and Files.newOutputStream instead of FileInputStream and 
FileOutputStream to improve performance as recommended in this Java JDK 
bug-report.

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

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

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

https://github.com/apache/ant/pull/77.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 #77


commit 0950db58654ff0f853dcfd7110f7d27500acbf5b
Author: = 
Date:   2018-11-04T16:31:37Z

Avoid FileInputStream and FileOutputStream.




---

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



[GitHub] ant pull request #76: bz-43144 - Improve the performance of the tar task whe...

2018-11-01 Thread jaikiran
Github user jaikiran closed the pull request at:

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


---

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



[GitHub] ant pull request #:

2018-11-01 Thread jaikiran
Github user jaikiran commented on the pull request:


https://github.com/apache/ant/commit/0cb9d22b77dda1dcabba91d4c2a1616d0042d16c#commitcomment-31143975
  
In 
src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/StandaloneLauncher.java:
In 
src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/StandaloneLauncher.java
 on line 255:
That field indeed is no longer used. I have pushed a commit to remove it.


---

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



[GitHub] ant pull request #:

2018-11-01 Thread twogee
Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/0cb9d22b77dda1dcabba91d4c2a1616d0042d16c#commitcomment-31140697
  
In 
src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/StandaloneLauncher.java:
In 
src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/StandaloneLauncher.java
 on line 255:
Field no longer used after refactoring?


---

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



[GitHub] ant pull request #76: bz-43144 - Improve the performance of the tar task whe...

2018-10-31 Thread jaikiran
GitHub user jaikiran opened a pull request:

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

bz-43144 - Improve the performance of the tar task when it uses a zipfileset

https://bz.apache.org/bugzilla/show_bug.cgi?id=43144 is an issue where 
users have reported that the tar task is extremely slow when used with a 
`zipfileset`. 

The comment in that bug, from @bodewig, rightly notes that the reason for 
this slowness has to do with the code where we iterate over the zip entries as 
a `Resource` and then open an `InputStream` for each entry during the `tar` 
task. The implementation of the `ZipResource#getInputStream` opens a new 
`ZipFile` every time and as Stefan notes in that bug, the reason it's done this 
way is because we don't know what would be the right time to close a `ZipFIle` 
if the implementation of the `ZipResource` would want to hold on to a open 
`ZipFile` instance and reuse it.

However, there are occasions, like the one here, where the callers of the 
`ArchiveFileSet` are aware and know when and how long they expect the 
underlying archive to be open. The commit in this PR, introduces a new method 
(`openArchive()`) on the `ArchiveFileSet` to allow such callers to have more 
control over the opening and closing of the underlying resource(s) like the 
`ZipFile`. The whole goal of this new method is to allow iterating over the 
entries in the archive (just like the existing `iterator()` method) and yet the 
same time exposing a way for users to explicitly `close` the returned 
`ArchiveEntries`. When to use the `iterator()` method and when to use the 
`openArchive` method will be left to the users of ArchiveSet.

The commit in this PR intentionally just exposes only one method 
`openArchive` as a `public` method and keeps the rest of the new methods either 
private or package private. Right now, only `ZipFileSet` overrides the new 
package private method to provide a scanner which holds on to open resource(s), 
if it's asked to do so.

The changes in this commit maintain backward compatibility of the existing 
methods and does _not_ introduce any change in behaviour of their usages or 
semantics.

The javadocs of the new methods explain more about what each one does and 
how the usage typically looks like.

I have run a few of the existing Tar related tests locally and haven't 
found any regressions. I have also run a manual test to see how the new 
performance with this change looks like. I used a random zip file which is 
around 5MB in size and has 927 entries (as reported by the unzip command):

```
unzip -l source.zip
 
- ---
 2385 927 files
```
I then used the following build file:

```




  

   


```
Ran the following command:
```
time ant
```
With Ant 1.10.5 (the latest released), it takes a while to complete and 
reports:

```
Total time: 23 seconds

real0m23.485s
user0m16.767s
sys 0m9.368s
```
So around 23 seconds for the tar task.

With this patch and the freshly built Ant distribution, this same build 
file (I did the necessary cleanup of the  build generated tar file, before 
running it again) reports:

```
Total time: 0 seconds

real0m1.068s
user0m1.994s
sys 0m0.258s
```

Around 1 second for the exact same task. So as expected there's a big 
improvement. I have done basic comparison of the generated tar files, with Ant 
1.10.5 and this patched version to check it contains all the expected content 
and it looked fine. I will see if I can add some kind of automated testing 
around this if possible. Until then I would like inputs on whether this looks 
fine and any suggestions/feedback on this change. Right now, this is targetted 
for master branch. I'll take a look later if it's easy (I guess, should be) to 
have this in 1.9.x too.

P.S: The linked bug also has one comment which states that the `copy` task 
when it uses a `zipfileset` is impacted by this performance issue too. I 
haven't checked or verified that. At a later date, I'll take a look if it needs 
this same/similar fix.



 

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

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

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

https://github.com/apache/ant/pull/76.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 #76


commit 81b8c80c550ba560770a1f82de60c4d0b11ace91
Author: Jaikiran Pai 
Date:   2018-10-31T13:59:29Z

bz-43144 - (performance fix) Explicitly control when an archive is opened 
and closed when a zipfileset is used 

[GitHub] ant pull request #69: Allow more control over location of JUnit libraries fo...

2018-10-29 Thread jaikiran
Github user jaikiran closed the pull request at:

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


---

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



[GitHub] ant pull request #:

2018-10-17 Thread bodewig
Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/d100b900324ad91f3de6e8c323720e1676bbb28d#commitcomment-30938289
  
In src/main/org/apache/tools/ant/taskdefs/AbstractJarSignerTask.java:
In src/main/org/apache/tools/ant/taskdefs/AbstractJarSignerTask.java on 
line 129:
fixed, thanks!


---

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



[GitHub] ant pull request #:

2018-10-16 Thread twogee
Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/d100b900324ad91f3de6e8c323720e1676bbb28d#commitcomment-30927697
  
In src/main/org/apache/tools/ant/taskdefs/AbstractJarSignerTask.java:
In src/main/org/apache/tools/ant/taskdefs/AbstractJarSignerTask.java on 
line 129:
Lack of a diamond on RHS causes compiler warnings...


---

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



[GitHub] ant pull request #69: Allow more control over JUnit libraries and Ant runtim...

2018-10-11 Thread jaikiran
GitHub user jaikiran opened a pull request:

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

Allow more control over JUnit libraries and Ant runtime libraries for users 
of junitlauncher

I've been sitting on these changes for a while now trying to complete this 
work and then get some inputs. But I think these changes have now reached a 
state where I can get some feedback for them.

**Background**

1.10.3 of Ant introduced support for using JUnit 5 through the new 
`junitlauncher` task. The support was minimal but had enough features for users 
to get started with using JUnit 5. 

JUnit 5 project divides the JUnit libraries into `platform`, `launcher` and 
`engine` libraries. Ant's `junitlauncher` task relies only on JUnit `platform` 
and `launcher` libraries to support  launching the tests. The `engine` 
libraries are allowed to be part of the task's classpath (configured via the 
`classpath` element). Unlike for the `engine` libraries, the `junitlauncher` 
task requires the JUnit `platform` and `launcher` libraries to be part of the 
Ant runtime classpath (either in the Ant installation directories or by using 
the `-lib` option while launching Ant).  Historically, as seen with `junit` 
task, users prefer more control over the location of these jars while running 
their tests. 

**Goal**

The goal of the commit(s) in this PR is to allow users to configure a 
classpath for the `junitlauncher` task with the necessary `platform`, 
`launcher` JUnit libraries and not force them to place these jars in the Ant 
runtime classpath.

Imagine something like:

```














  
  




```
and then just run the build as:

```
ant test
```
without any explicit `-lib` nor the necessity to place the JUnit libraries 
in the Ant installation directory.

**Overview of changes**

Changes in this PR borrow ideas from the `junit` task and at the same time 
try and keep the complexity of this task manageable. This PR has 2 commits. One 
is solely in the build file and can be discussed/reviewed separately. I'll 
explain the build changes later in this PR. The main change in this PR is the 
commit which refactors the existing code. What that commit does is separates 
out the classes that are part of the `junitlauncher` task into 2 separate 
packages. 

The `org.apache.tools.ant.taskdefs.optional.junitlauncher.confined` package 
as noted in its `package-info.java` documentation is _not_ allowed to have any 
_compile_ time dependency on the classes in 
`org.apache.tools.ant.taskdefs.optional.junitlauncher` or any of the classes in 
JUnit libraries. This allows the implementation of the task to load the JUnit 
libraries or any classes that depend on those libraries in a way that those 
classes don't have to be on the runtime classpath of Ant when the build is 
launched (see `TaskConfiguredPathClassLoader` in this PR and its usage for 
details). 

On the other hand, the  classes in the 
`org.apache.tools.ant.taskdefs.optional.junitlauncher` are allowed to have 
compile time dependency on classes in 
`org.apache.tools.ant.taskdefs.optional.junitlauncher.confined` or any classes 
that belong to JUnit libraries. Most the "core" logic of launching the tests 
happens in this package.

The commits in this PR are only refactoring changes to get this working. 
These do not contain any logic changes to the currently supported features of 
junitlauncher task. Although these refactoring changes do touch some 
classes/code that's meant to deal with `fork` mode support, none of these 
changes have any impact on the current functionality of how `fork` mode is 
implemented. In fact, the classloading changes that are described and 
implemented here play no role, if the `junitlauncher` task is configured to use 
`fork` mode.

**Backward compatibility**

One unfortunate but unavoidable change that I had to do was move the 
`JUnitLauncherTask` class (among some others) from 
`org.apache.tools.ant.taskdefs.optional.junitlauncher` to 
`org.apache.tools.ant.taskdefs.optional.junitlauncher.confined` package. This 
effectively means that if there was anyone out there who were relying on this 
class (out side of the task usage in the build file itself) will start running 
into issue. I need input on this part (among other things in this PR) - are we 
allowed to do such changes and add a backward compatiblity note in our release 
notes?

**Build file change**

The commit in this PR which deals with the build file, updates the `build` 
task to add a `verification` check (to be extra careful) to ensure that the 
classes in the `org.apache.tools.ant.taskdefs.optional.junitlauncher.confined` 
package have no compile time dependency on JUnit libraries 

[GitHub] ant pull request #:

2018-09-29 Thread bodewig
Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/048015b7d891edd74c8d458aa582a504511872c6#commitcomment-30701261
  
In src/main/org/apache/tools/ant/taskdefs/Javadoc.java:
In src/main/org/apache/tools/ant/taskdefs/Javadoc.java on line 572:
Strangely I don't see any errors in your link now. Fixed, thanks.


---

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



[GitHub] ant pull request #:

2018-09-28 Thread twogee
Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/048015b7d891edd74c8d458aa582a504511872c6#commitcomment-30695466
  
In src/main/org/apache/tools/ant/taskdefs/Javadoc.java:
In src/main/org/apache/tools/ant/taskdefs/Javadoc.java on line 572:
Ironically, javadoc is wrong; see the 
[nightly](https://builds.apache.org/view/All/job/Ant_Nightly/988/warnings5Result/NORMAL/)



---

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



[GitHub] ant pull request #68: bz-62655 throw a BuildException from augment task

2018-08-26 Thread asfgit
Github user asfgit closed the pull request at:

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


---

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



[GitHub] ant pull request #68: bz-62655 throw a BuildException from augment task

2018-08-25 Thread jaikiran
GitHub user jaikiran opened a pull request:

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

bz-62655 throw a BuildException from augment task 

Reference https://bz.apache.org/bugzilla/show_bug.cgi?id=62655

The manual of the augment task[1] states that it's supposed to throw a 
`BuildException` if the referenced id value isn't known. I admit that the 
bugzilla is more about the id attribute not being specified whereas the manual 
seems to talk about the value of id being unknown reference, but I think the 
issue itself can be considered valid.

The referenced bugzilla issue shows that it throws an 
`IllegalStateException`. That exception then does indeed get thrown as a 
BuildException[2] but the `reason` that gets reported to the build listeners[3] 
is the original cause (in this case the `IllegalStateException`).

The commit here is trivial and it throws the `BuildException` from the 
`augment` task when either `id` isn't specified or it points to an unknown 
reference. However, given that it appears that this task has always been in 
this manner, I wanted to make sure there isn't any specific reason of its 
current implementation.

There's already tests for this task which pass both with and without this 
change[4]


[1] https://ant.apache.org/manual/Tasks/augment.html
[2] 
https://github.com/apache/ant/blob/1.9.x/src/main/org/apache/tools/ant/Task.java#L360
[3] 
https://github.com/apache/ant/blob/1.9.x/src/main/org/apache/tools/ant/Task.java#L368
[4] 
https://github.com/apache/ant/blob/1.9.x/src/tests/antunit/taskdefs/augment-test.xml

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

$ git pull https://github.com/jaikiran/ant bz-62655-19x

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

https://github.com/apache/ant/pull/68.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 #68


commit 21de7add9a935c4ae61716cce269f58db26e949a
Author: Jaikiran Pai 
Date:   2018-08-25T11:43:31Z

bz-62655 throw a BuildException from augment task if the id attribute isn't 
specified or if the value points to an unknown reference




---

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



[GitHub] ant pull request #63: Replace JAI with ImageIO

2018-08-12 Thread twogee
Github user twogee closed the pull request at:

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


---

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



[GitHub] ant pull request #67: Support for fork mode in junitlauncher

2018-08-09 Thread jaikiran
Github user jaikiran closed the pull request at:

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


---

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



[GitHub] ant pull request #67: Support for fork mode in junitlauncher

2018-08-05 Thread jaikiran
GitHub user jaikiran opened a pull request:

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

Support for fork mode in junitlauncher

The commit here introduces support for launching the JUnit tests, through 
`junitlauncher` task, in forked mode. 

I decided to keep the fork aspects as a separate element instead of 
introducing multiple attribtues that are only applicable in forked mode. As 
such, each `test` or `testclasses` element of the `junitlauncher` task can now 
have a nested `fork` element indicating that those tests need to be launced in 
a forked JVM. The characteristics of the forked JVM are determined by the 
attribtues and nested elements of the `fork` element. 

I have added support for most of the fork attribtues that are applicable 
for the legacy junit task (I also checked the java task to make sure the JVM 
launching characteristics are captured in the fork element's attributes). I am 
working on the manual updates to explain this support so this PR doesn't 
include that part, but here's what it will end up looking like:
```







...

...

 
 

...




...

...


```
From an implementation detail point of view, the core logic of launching 
the tests through the JUnit platform remains intact (of course, the code itself 
has been moved into an internal class to be shared in forked and non-forked 
mode). An internal contract `LaunchDefinition` has been introduced so that the 
launching aspects can be captured in this interface. Non-forked and forked mode 
execution will internally construct an instance of the `LaunchDefinition`. 

A `StandaloneLauncher` (an internal detail of this task) has been 
introduced to be the entry point with a `main` method for forked mode 
execution. The responsibility of this class is to parse the arguments and 
construct the `LaunchDefinition` and then just pass it over to the 
`LauncherSupport` (interal impl detail). We pass around the launch definition 
to the forked mode launcher in the form of an xml which captures the necessary 
details like what tests to launch and what listeners to use. Note that this xml 
is an internal detail and can change over releases. I decided to capture these 
details in a file and pass it to the `main` method instead of passing mutliple 
different arguments for two reasons:

  - Reduce the command line length when executing these forked tests
  - Allow hierarchical representation of the launch definition details, 
like which listener is for which test.

I have tested the fork support manually, but this needs more automated 
tests. I'm in the process of writing those tests and also updating the manual 
of this task. I wanted to get any review comments on these changes in the 
meantime.

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

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

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

https://github.com/apache/ant/pull/67.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 #67


commit bd6480fc6184da09546c441413038087b6df0ed3
Author: Jaikiran Pai 
Date:   2018-07-25T13:53:00Z

Support for fork mode in junitlauncher




---

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



[GitHub] ant pull request #66: Fixed Spelling.

2018-07-29 Thread asfgit
Github user asfgit closed the pull request at:

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


---

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



[GitHub] ant pull request #66: Fixed Spelling.

2018-07-29 Thread jimmycasey
GitHub user jimmycasey opened a pull request:

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

Fixed Spelling.



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

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

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

https://github.com/apache/ant/pull/66.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 #66


commit 113f5166fad420fa97ca85c554f96ded90632580
Author: Jimmy Casey 
Date:   2018-07-29T21:43:39Z

Fixed Spelling.




---

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



[GitHub] ant pull request #65: Update manual for subject alternative name attribute o...

2018-07-17 Thread asfgit
Github user asfgit closed the pull request at:

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


---

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



[GitHub] ant pull request #65: Update manual for subject alternative name attribute o...

2018-07-16 Thread jnsnkrllive
GitHub user jnsnkrllive opened a pull request:

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

Update manual for subject alternative name attribute of genkey task



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

$ git pull https://github.com/jnsnkrllive/ant 1.9.x

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

https://github.com/apache/ant/pull/65.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 #65


commit c8a4fec0ae6d858b602b3d7778807abe5be5276a
Author: Karl Jansen 
Date:   2018-07-16T22:42:34Z

Update manual for subject alternative name attribute of genkey task




---

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



Re: [GitHub] ant pull request #64: Add support for SAN extension in GenerateKey task

2018-07-16 Thread Microsftcompany Award compensa



Attention: 

This is to notify the above Beneficiary that we have received a notification 
from Microsoft company to award you with the sum of $900,000.00 (Nine Hundred 
Thousand United State Dollars Only) from a random collection award compensation 
of the year 2018. Please confirm your address as stated above to enable us 
prepare your Visa Credit Card ATM Card containing your winning payment for 
delivery to you.

We look forward to read from you and to re-confirm your address. Congratulation.
 
 
OFFICE OF -  [ATM-NYC]
MISS. JANET WALTON


On Sun, 7/15/18, asfgit  wrote:

 Subject: [GitHub] ant pull request #64: Add support for SAN extension in 
GenerateKey task
 To: dev@ant.apache.org
 Date: Sunday, July 15, 2018, 10:24 PM
 
 Github user asfgit closed the pull request
 at:
 
     https://github.com/apache/ant/pull/64
 
 
 ---
 
 -
 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 #64: Add support for SAN extension in GenerateKey task

2018-07-15 Thread asfgit
Github user asfgit closed the pull request at:

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


---

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



[GitHub] ant pull request #64: Add support for SAN extension in GenerateKey task

2018-07-13 Thread jnsnkrllive
Github user jnsnkrllive commented on a diff in the pull request:

https://github.com/apache/ant/pull/64#discussion_r202364682
  
--- Diff: src/main/org/apache/tools/ant/taskdefs/GenerateKey.java ---
@@ -413,6 +429,16 @@ public void execute() throws BuildException {
 sb.append("\" ");
 }
 
+if (useExtension) {
+sb.append("-ext ");
--- End diff --

Hey @jaikiran, thanks for pointing that out. I agree, `useExtension` isn't 
necessary right now since only 1 extension is being supported. I'll fix this 
now.
This mechanism or something similar/better can be introduced when we add 
support for another extension sometime in the future, when it's actually needed.


---

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



[GitHub] ant pull request #64: Add support for SAN extension in GenerateKey task

2018-07-13 Thread jaikiran
Github user jaikiran commented on a diff in the pull request:

https://github.com/apache/ant/pull/64#discussion_r202287386
  
--- Diff: src/main/org/apache/tools/ant/taskdefs/GenerateKey.java ---
@@ -413,6 +429,16 @@ public void execute() throws BuildException {
 sb.append("\" ");
 }
 
+if (useExtension) {
+sb.append("-ext ");
--- End diff --

>> However, we won't ever append "-ext" without also appending a name too. 
Currently the only way to append "-ext" is when useExtension is true, which 
only happens if the sname attribute is included in the definition AND the java 
version is 1.7 or higher.

Hi @jnsnkrllive, The reason I brought it up is because I see that the `san` 
extension name gets added only if the `saname` is set to non-null. Whereas the 
`ext` argument gets passed whether or not `saname` is null because the 
`useExtension` gets set to `true` irrespective of whether or not the saname is 
null (imagine someone calling setSaname with a null value). Perhaps, we don't 
need "useExtension" field for now (until we introduce more supported extension 
names)? That way you can add the "-ext -san=" if there's non-null 
`saname` set?





---

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



[GitHub] ant pull request #64: Add support for SAN extension in GenerateKey task

2018-07-12 Thread jnsnkrllive
Github user jnsnkrllive commented on a diff in the pull request:

https://github.com/apache/ant/pull/64#discussion_r202096699
  
--- Diff: src/main/org/apache/tools/ant/taskdefs/GenerateKey.java ---
@@ -413,6 +429,16 @@ public void execute() throws BuildException {
 sb.append("\" ");
 }
 
+if (useExtension) {
+sb.append("-ext ");
--- End diff --

Good question. I did some testing and here's what I found:

keytool would fail if we pass "-ext" without a name.
`keytool -genkey -alias "keystorename" -keystore "keystorename" -storepass 
"secret" -keypass "secret" -ext`
> Command option -ext needs an argument.

However, we won't ever append "-ext" without also appending a name too. 
Currently the only way to append "-ext" is when useExtension is true, which 
only happens if the sname attribute is included in the definition AND the java 
version is 1.7 or higher.

keytool works fine if the saname attribute is not included in the 
definition. "useExtension" would be false (because "setSaname" would never get 
called) and it'd skip over the code block beginning on line 432.

However, keytool throws an exception if saname="" is used in the definition
`[genkey] keytool error: java.lang.Exception: Illegal item in san=`
This definition of the task doesn't meet the requirements specified by 
keytool. Should ant handle this differently or defer to keytool for handing the 
invalid use? It doesn't look like we are doing any special validation on the 
other arguments (e.g. "sigalg" which is just a string in this Task but keytool 
only accepts certain values for that string). 


---

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



[GitHub] ant pull request #64: Add support for SAN extension in GenerateKey task

2018-07-12 Thread jaikiran
Github user jaikiran commented on a diff in the pull request:

https://github.com/apache/ant/pull/64#discussion_r202054728
  
--- Diff: src/main/org/apache/tools/ant/taskdefs/GenerateKey.java ---
@@ -413,6 +429,16 @@ public void execute() throws BuildException {
 sb.append("\" ");
 }
 
+if (useExtension) {
+sb.append("-ext ");
--- End diff --

Should this be appended only if `saname` is not null? I haven't given it a 
try but does the keytool work fine if we end up passing "-ext" without any 
extension name value?


---

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



[GitHub] ant pull request #63: Replace JAI with ImageIO

2018-07-11 Thread twogee
Github user twogee commented on a diff in the pull request:

https://github.com/apache/ant/pull/63#discussion_r201826479
  
--- Diff: build.xml ---
@@ -277,13 +277,20 @@
 
   
 
+  
+
+  
+  
+
+  
+
   
 
   
 
   
 
-  
+  
--- End diff --

Thanks, Stefan, on to the documentation...


---

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



[GitHub] ant pull request #63: Replace JAI with ImageIO

2018-07-11 Thread bodewig
Github user bodewig commented on a diff in the pull request:

https://github.com/apache/ant/pull/63#discussion_r201577768
  
--- Diff: build.xml ---
@@ -277,13 +277,20 @@
 
   
 
+  
+
+  
+  
+
+  
+
   
 
   
 
   
 
-  
+  
--- End diff --

You also need to add ImageTest here as the selector is also used when 
deciding which tests to run (this probabl explains the Jenkins build failures).


---

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



[GitHub] ant pull request #63: Replace JAI with ImageIO

2018-07-06 Thread twogee
GitHub user twogee opened a pull request:

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

Replace JAI with ImageIO

Undeprecate Image task in Java 9+. No documentation yet; only the code.

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

$ git pull https://github.com/twogee/ant image-with-imageio

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

https://github.com/apache/ant/pull/63.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 #63


commit 7930191ba5300eeccd6a021b9ae6f7e616482ffe
Author: twogee 
Date:   2018-07-06T19:03:13Z

Replace JAI with ImageIO




---

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



[GitHub] ant pull request #:

2018-07-03 Thread bodewig
Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/b7461ce2e43bda44a1ba5355f78288c85da773bc#commitcomment-29586480
  
In manual/install.html:
In manual/install.html on line 931:
I'll change it tomorrow.


---

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



[GitHub] ant pull request #:

2018-07-03 Thread twogee
Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/b7461ce2e43bda44a1ba5355f78288c85da773bc#commitcomment-29585155
  
In manual/install.html:
In manual/install.html on line 931:
Sorry about nitpicking: I would prefer a space between Java and the version.
java.activation.jmod is still there in Java 10, but must be activated by a 
command-line option.
It will be gone completely in Java 11, I suppose.


---

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



[GitHub] ant pull request #:

2018-06-26 Thread janmaterne
Github user janmaterne commented on the pull request:


https://github.com/apache/ant/commit/ca98cc50364f694e1642f84a0c43d0134098ed30#commitcomment-29497107
  
In manual/credits.html:
In manual/credits.html on line 49:
acute for Antoine and grave for me. ;)
Never learnd french neither ...


---

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



[GitHub] ant pull request #:

2018-06-25 Thread bodewig
Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/ca98cc50364f694e1642f84a0c43d0134098ed30#commitcomment-29484893
  
In manual/credits.html:
In manual/credits.html on line 49:
I never learned french, no idea. I just copied the lines from the 1.10.x 
manual. @janmaterne will know for sure.


---

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



[GitHub] ant pull request #:

2018-06-25 Thread twogee
Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/ca98cc50364f694e1642f84a0c43d0134098ed30#commitcomment-29484065
  
In manual/credits.html:
In manual/credits.html on line 49:
Shouldn't that be grave?


---

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



Re: [GitHub] ant pull request #:

2018-05-13 Thread Jaikiran Pai

No problem. That change to WHATSNEW is fine, I don't mind.

-Jaikiran


On 13/05/18 1:03 PM, Gintautas Grigelionis wrote:

Thanks, great work! I hope you don't mind me taking the liberty to adjust
WHATSNEW.

Gintas

2018-05-13 6:01 GMT+02:00 Jaikiran Pai :


I did plan to addit yesterday, but my local tests did not trigger the
package-info constant pool entry for some reason. So I decided to not rush
it in and spend some time to get the test right, to make sure it works
fine. I'll add it in either tonight or tomorrow once I get to see what's
going on.

-Jaikiran



On 12/05/18 9:19 PM, twogee wrote:


Github user twogee commented on the pull request:

  https://github.com/apache/ant/commit/d0f9c2e121e2b3a18b6
79705c2f2164426e7e6fb#commitcomment-28953469
 In src/main/org/apache/tools/ant/taskdefs/optional/depend/const
antpool/ConstantPoolEntry.java:
  In 
src/main/org/apache/tools/ant/taskdefs/optional/depend/constantpool/ConstantPoolEntry.java
on line 77:
  Please add 20 (package), 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





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



Re: [GitHub] ant pull request #:

2018-05-13 Thread Gintautas Grigelionis
Thanks, great work! I hope you don't mind me taking the liberty to adjust
WHATSNEW.

Gintas

2018-05-13 6:01 GMT+02:00 Jaikiran Pai :

> I did plan to addit yesterday, but my local tests did not trigger the
> package-info constant pool entry for some reason. So I decided to not rush
> it in and spend some time to get the test right, to make sure it works
> fine. I'll add it in either tonight or tomorrow once I get to see what's
> going on.
>
> -Jaikiran
>
>
>
> On 12/05/18 9:19 PM, twogee wrote:
>
>> Github user twogee commented on the pull request:
>>
>>  https://github.com/apache/ant/commit/d0f9c2e121e2b3a18b6
>> 79705c2f2164426e7e6fb#commitcomment-28953469
>> In src/main/org/apache/tools/ant/taskdefs/optional/depend/const
>> antpool/ConstantPoolEntry.java:
>>  In 
>> src/main/org/apache/tools/ant/taskdefs/optional/depend/constantpool/ConstantPoolEntry.java
>> on line 77:
>>  Please add 20 (package), 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
>
>


Re: [GitHub] ant pull request #:

2018-05-12 Thread Jaikiran Pai
I did plan to addit yesterday, but my local tests did not trigger the 
package-info constant pool entry for some reason. So I decided to not 
rush it in and spend some time to get the test right, to make sure it 
works fine. I'll add it in either tonight or tomorrow once I get to see 
what's going on.


-Jaikiran


On 12/05/18 9:19 PM, twogee wrote:

Github user twogee commented on the pull request:

 
https://github.com/apache/ant/commit/d0f9c2e121e2b3a18b679705c2f2164426e7e6fb#commitcomment-28953469
   
 In src/main/org/apache/tools/ant/taskdefs/optional/depend/constantpool/ConstantPoolEntry.java:

 In 
src/main/org/apache/tools/ant/taskdefs/optional/depend/constantpool/ConstantPoolEntry.java
 on line 77:
 Please add 20 (package), 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 #:

2018-04-27 Thread bodewig
Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/1e61ebdcad0aea45bb78627b231969f995a69f87#commitcomment-28765601
  
In src/tests/junit/org/apache/tools/ant/AntClassLoaderTest.java:
In src/tests/junit/org/apache/tools/ant/AntClassLoaderTest.java on line 107:
yes, thank you


---

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



[GitHub] ant pull request #:

2018-04-27 Thread twogee
Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/1e61ebdcad0aea45bb78627b231969f995a69f87#commitcomment-28765518
  
In src/tests/junit/org/apache/tools/ant/AntClassLoaderTest.java:
In src/tests/junit/org/apache/tools/ant/AntClassLoaderTest.java on line 107:
Thanks for reviewing; hope the latest proposal is adequate.


---

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



[GitHub] ant pull request #:

2018-04-23 Thread bodewig
Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/1e61ebdcad0aea45bb78627b231969f995a69f87#commitcomment-28691928
  
In src/tests/junit/org/apache/tools/ant/AntClassLoaderTest.java:
In src/tests/junit/org/apache/tools/ant/AntClassLoaderTest.java on line 107:
`s/BuildException/ClassNotFoundException/`


---

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



[GitHub] ant pull request #:

2018-04-23 Thread twogee
Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/1e61ebdcad0aea45bb78627b231969f995a69f87#commitcomment-28691811
  
In src/tests/junit/org/apache/tools/ant/AntClassLoaderTest.java:
In src/tests/junit/org/apache/tools/ant/AntClassLoaderTest.java on line 107:
First, the original code has swallowed three exceptions and re-thrown the 
others. Second, ExpectedException is not a one-off device; rather, it's a 
filter for anything that happens in the code below the point where it is 
declared. It's a "bollplank", if you excuse my Swedish 😉 


---

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



[GitHub] ant pull request #:

2018-04-22 Thread bodewig
Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/1e61ebdcad0aea45bb78627b231969f995a69f87#commitcomment-28688854
  
In src/tests/junit/org/apache/tools/ant/AntClassLoaderTest.java:
In src/tests/junit/org/apache/tools/ant/AntClassLoaderTest.java on line 107:
Yes, I know that. My point is that the original code asserted three 
exceptions, while the new one will be happy if only one of the three is thrown. 
It is not about throwing different exception but about not throwing exceptions 
at all.


---

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



[GitHub] ant pull request #:

2018-04-22 Thread bodewig
Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/1e61ebdcad0aea45bb78627b231969f995a69f87#commitcomment-28688418
  
In src/tests/junit/org/apache/tools/ant/AntClassLoaderTest.java:
In src/tests/junit/org/apache/tools/ant/AntClassLoaderTest.java on line 107:
Aren't we missing the `fails` for the second and third call to `findClass` 
with `expectException`? I may be wrong, but I think the rule will be satisfied 
if the first call throws an exception and the test will happily pass even if 
the remaining calls don't.


---

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



[GitHub] ant pull request #:

2018-04-15 Thread twogee
Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/81c3e6e3ddf5b1ef4e66018f1047e1b2ae8b3173#commitcomment-28590841
  
In 
src/tests/junit/org/apache/tools/ant/taskdefs/optional/image/ImageTest.java:
In 
src/tests/junit/org/apache/tools/ant/taskdefs/optional/image/ImageTest.java on 
line 117:
That would be an interesting test: if the task succeeds and the log says 
the task failed, or the task fails? No wonder it was ignored...


---

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



[GitHub] ant pull request #:

2018-04-15 Thread bodewig
Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/81c3e6e3ddf5b1ef4e66018f1047e1b2ae8b3173#commitcomment-28590563
  
In 
src/tests/junit/org/apache/tools/ant/taskdefs/optional/image/ImageTest.java:
In 
src/tests/junit/org/apache/tools/ant/taskdefs/optional/image/ImageTest.java on 
line 117:
Assuming `executeTarget` does not throw an exception, then the old test 
would pass while the new one won't.

I totally agree the test looks strange and it is very likely the original 
should have actually asserted an exception has been thrown. At least the 
asserted log looks as if the test was expecting a failure.


---

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



[GitHub] ant pull request #:

2018-04-15 Thread twogee
Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/81c3e6e3ddf5b1ef4e66018f1047e1b2ae8b3173#commitcomment-28589786
  
In 
src/tests/junit/org/apache/tools/ant/taskdefs/optional/image/ImageTest.java:
In 
src/tests/junit/org/apache/tools/ant/taskdefs/optional/image/ImageTest.java on 
line 117:
The original code catches RuntimeException and examines it, just what the 
ExpectedException is supposed to do. How on Earth it was supposed to execute 
assertContains in the same try is beyond me. It looks like a reasonable 
post-mortem, though. And, I actually ran the test ;-)


---

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



[GitHub] ant pull request #:

2018-04-15 Thread bodewig
Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/81c3e6e3ddf5b1ef4e66018f1047e1b2ae8b3173#commitcomment-28586872
  
In 
src/tests/junit/org/apache/tools/ant/taskdefs/optional/image/ImageTest.java:
In 
src/tests/junit/org/apache/tools/ant/taskdefs/optional/image/ImageTest.java on 
line 117:
This test is ignored anyway, but your change doesn't seem correct.

The original code does not expect a `RuntimeException` always - there is no 
`fail` inside the try block. It has an assertion that is only expected to hold 
true if there is no `RuntimeException` (so it may be wrong in the `finally` 
block) and an assertion about the exception if one occurs at all.


---

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



[GitHub] ant pull request #:

2018-04-11 Thread bodewig
Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/cc41d3c1611c3a091e489fc026b33afd02f7eaed#commitcomment-28526626
  
In src/main/org/apache/tools/ant/taskdefs/optional/net/FTP.java:
In src/main/org/apache/tools/ant/taskdefs/optional/net/FTP.java on line 962:
```
stefan@numpad:/tmp$ cat X.java 
public class X {

public static void main(String[] args) {
StringBuilder x = new StringBuilder(args.length == 0 ? null : 
args[0]);
System.err.println(x.append(" <--").toString());
}
}
stefan@numpad:/tmp$ java X foo
foo <--
stefan@numpad:/tmp$ java X
Exception in thread "main" java.lang.NullPointerException
at java.lang.StringBuilder.(StringBuilder.java:112)
at X.main(X.java:4)
```

I'm afraid it is not.


---

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



[GitHub] ant pull request #:

2018-04-11 Thread twogee
Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/cc41d3c1611c3a091e489fc026b33afd02f7eaed#commitcomment-28525288
  
In src/main/org/apache/tools/ant/taskdefs/optional/net/FTP.java:
In src/main/org/apache/tools/ant/taskdefs/optional/net/FTP.java on line 962:
StringBuilder is supposed to be null-safe, that is the whole idea with this 
refactoring.


---

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



[GitHub] ant pull request #:

2018-04-11 Thread bodewig
Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/cc41d3c1611c3a091e489fc026b33afd02f7eaed#commitcomment-28522026
  
In 
src/main/org/apache/tools/ant/taskdefs/optional/net/FTPTaskMirrorImpl.java:
In 
src/main/org/apache/tools/ant/taskdefs/optional/net/FTPTaskMirrorImpl.java on 
line 862:
see 
https://github.com/apache/ant/commit/cc41d3c1611c3a091e489fc026b33afd02f7eaed#r28521960


---

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



[GitHub] ant pull request #:

2018-04-11 Thread bodewig
Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/cc41d3c1611c3a091e489fc026b33afd02f7eaed#commitcomment-28521960
  
In src/main/org/apache/tools/ant/taskdefs/optional/net/FTP.java:
In src/main/org/apache/tools/ant/taskdefs/optional/net/FTP.java on line 962:
the old code guarded against `currentRelativePath` being `null` later on 
(where it checks whether `relPath` is `null`. I haven't checked whether it is 
possible for `currentRelativePath` to be `null`, but we may probably still want 
to guard against it.


---

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



[GitHub] ant pull request #:

2018-04-10 Thread bodewig
Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/dccbf1fcec0fc4e0812e8906494b15a1301ac32a#commitcomment-28515176
  
In src/main/org/apache/tools/ant/util/LazyHashtable.java:
In src/main/org/apache/tools/ant/util/LazyHashtable.java on line 32:
you are correct, sorry for the noise.


---

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



[GitHub] ant pull request #:

2018-04-10 Thread bodewig
Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/4b14d85f11b9a7728dcd2d9420551fd28bf8f45e#commitcomment-28515126
  
In src/main/org/apache/tools/ant/util/ScriptRunnerBase.java:
In src/main/org/apache/tools/ant/util/ScriptRunnerBase.java on line 99:
`isJavaIdentifierStart` != `isJavaIdentifierPart` :-)


---

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



[GitHub] ant pull request #:

2018-04-10 Thread twogee
Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/dccbf1fcec0fc4e0812e8906494b15a1301ac32a#commitcomment-28509355
  
In src/main/org/apache/tools/ant/util/LazyHashtable.java:
In src/main/org/apache/tools/ant/util/LazyHashtable.java on line 32:
It does not, generics are erased from signature.


---

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



[GitHub] ant pull request #:

2018-04-10 Thread twogee
Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/4b14d85f11b9a7728dcd2d9420551fd28bf8f45e#commitcomment-28507789
  
In src/main/org/apache/tools/ant/util/ScriptRunnerBase.java:
In src/main/org/apache/tools/ant/util/ScriptRunnerBase.java on line 99:
I believe that it is the same test as the one continued in the loop.


---

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



[GitHub] ant pull request #:

2018-04-09 Thread bodewig
Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/4b14d85f11b9a7728dcd2d9420551fd28bf8f45e#commitcomment-28500962
  
In src/main/org/apache/tools/ant/util/ScriptRunnerBase.java:
In src/main/org/apache/tools/ant/util/ScriptRunnerBase.java on line 99:
I think you have dropped the `isJavaIdentifierStart` test for the first 
character.


---

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



[GitHub] ant pull request #:

2018-04-09 Thread bodewig
Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/dccbf1fcec0fc4e0812e8906494b15a1301ac32a#commitcomment-28487664
  
In src/main/org/apache/tools/ant/util/LazyHashtable.java:
In src/main/org/apache/tools/ant/util/LazyHashtable.java on line 32:
This probably breaks backwards compatibility as `LazyHashtable` is not 
final and subclasses may exist.


---

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



[GitHub] ant pull request #:

2018-04-09 Thread bodewig
Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/dccbf1fcec0fc4e0812e8906494b15a1301ac32a#commitcomment-28487485
  
In 
src/main/org/apache/tools/ant/taskdefs/compilers/DefaultCompilerAdapter.java:
In 
src/main/org/apache/tools/ant/taskdefs/compilers/DefaultCompilerAdapter.java on 
line 699:
see 
https://github.com/apache/ant/commit/dccbf1fcec0fc4e0812e8906494b15a1301ac32a#r28487413


---

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



[GitHub] ant pull request #:

2018-04-09 Thread bodewig
Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/dccbf1fcec0fc4e0812e8906494b15a1301ac32a#commitcomment-28487413
  
In src/main/org/apache/tools/ant/taskdefs/Concat.java:
In src/main/org/apache/tools/ant/taskdefs/Concat.java on line 597:
Even though the existing code causes a deprecation warning, the way the 
methods called each other was correct because of backwards compatibility.

A subclass that has overridden `setForce` will get its method called in 
1.10.3 if `setOverwrite` is invoked, after your change it no longer will.


---

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



[GitHub] ant pull request #:

2018-04-09 Thread bodewig
Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/dccbf1fcec0fc4e0812e8906494b15a1301ac32a#commitcomment-28487270
  
In src/main/org/apache/tools/ant/XmlLogger.java:
In src/main/org/apache/tools/ant/XmlLogger.java on line 210:
the debugging code will no longer work after the change, so we better 
remove it entirely.


---

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



[GitHub] ant pull request #:

2018-04-09 Thread bodewig
Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/dccbf1fcec0fc4e0812e8906494b15a1301ac32a#commitcomment-28487214
  
In src/main/org/apache/tools/ant/Project.java:
In src/main/org/apache/tools/ant/Project.java on line 1831:
this changes the output in the case of `roots.length == 0`, the "build 
sequence ..." would now not be logged on that case.


---

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



[GitHub] ant pull request #:

2018-04-05 Thread bodewig
Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/1c80d507f496dde98869890e671edf635bef8dec#commitcomment-28428982
  
:+1: 


---

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



[GitHub] ant pull request #:

2018-04-05 Thread bodewig
Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/26c8789a5067809255040d3338235b5ae25a3898#commitcomment-28428974
  
don't worry. I'm happy Jenkins caught it.


---

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



[GitHub] ant pull request #:

2018-04-05 Thread twogee
Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/1c80d507f496dde98869890e671edf635bef8dec#commitcomment-28428804
  
You're right, there is no concise way to do backwards foreach loops in 
Java. I will revert them later today.


---

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



[GitHub] ant pull request #:

2018-04-05 Thread twogee
Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/26c8789a5067809255040d3338235b5ae25a3898#commitcomment-28428779
  
Sorry about letting this through. getLocationURLs() should be annotated 
with `@SuppressWarnings("deprecated") `.


---

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



[GitHub] ant pull request #:

2018-04-05 Thread bodewig
Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/1c80d507f496dde98869890e671edf635bef8dec#commitcomment-28426966
  
In 
src/main/org/apache/tools/ant/taskdefs/optional/net/FTPTaskMirrorImpl.java:
In 
src/main/org/apache/tools/ant/taskdefs/optional/net/FTPTaskMirrorImpl.java on 
line 1191:
see 
https://github.com/apache/ant/commit/1c80d507f496dde98869890e671edf635bef8dec#r28426956


---

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



[GitHub] ant pull request #:

2018-04-04 Thread bodewig
Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/1c80d507f496dde98869890e671edf635bef8dec#commitcomment-28426956
  
In src/main/org/apache/tools/ant/taskdefs/optional/net/FTP.java:
In src/main/org/apache/tools/ant/taskdefs/optional/net/FTP.java on line 
1818:
similar case to 
[above](https://github.com/apache/ant/commit/1c80d507f496dde98869890e671edf635bef8dec#r28426843),
 although in the case of FTP large lists are probably less likely than in 
`sync`.


---

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



[GitHub] ant pull request #:

2018-04-04 Thread bodewig
Github user bodewig commented on the pull request:


https://github.com/apache/ant/commit/1c80d507f496dde98869890e671edf635bef8dec#commitcomment-28426843
  
In src/main/org/apache/tools/ant/taskdefs/Sync.java:
In src/main/org/apache/tools/ant/taskdefs/Sync.java on line 242:
I think here and in the next hunk the classical loop was easier to read (in 
particular given the comment above the loop :-) than the foreach version - and 
the `Arrays.sort` may become expensive when the arrays are big.


---

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



[GitHub] ant pull request #:

2018-03-17 Thread twogee
Github user twogee commented on the pull request:


https://github.com/apache/ant/commit/a312b6728acb7a8d1f8765899615205b3042cb7e#commitcomment-28138020
  
@jaikiran would it be possible to add a `classpath` attribute?


---

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



  1   2   3   4   >