[GitHub] ant pull request #:
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 #:
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 #:
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 #:
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 #:
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 #:
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 #:
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.
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.
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 r
[GitHub] ant pull request #80: Added tasks for JDK's jmod and jlink tools.
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 ...
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.
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 ...
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.
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
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
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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...
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 #:
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 #:
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...
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...
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 #:
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 #:
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...
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 o
[GitHub] ant pull request #:
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 #:
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
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
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
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
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
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.
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.
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...
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...
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
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
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
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
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
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
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 #64: Add support for SAN extension in GenerateKey task
GitHub user jnsnkrllive opened a pull request: https://github.com/apache/ant/pull/64 Add support for SAN extension in GenerateKey task Beginning with Java 7, keytool supports the SubjectAlternativeName extension on X.509 certificates. [Java 7 Keytool Documentation](https://docs.oracle.com/javase/7/docs/technotes/tools/windows/keytool.html) This enhancement will be useful to users who use Ant to issue certificates for their project. Especially, for example, if the project is a web application that is expected to be accessed through modern browsers who are beginning to drop support for Common Name. [Chrome 58 Deprecations - Remove support for commonName matching in certificates](https://developers.google.com/web/updates/2017/03/chrome-58-deprecations#remove_support_for_commonname_matching_in_certificates) Targeting 1.9.x branch now, this can be merged into master too. 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/64.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 #64 commit e291c451995d1f89a2d22e0b4f71058ef453e1bc Author: Karl Jansen Date: 2018-07-11T21:19:29Z Add support for SAN extension in GenerateKey task --- - 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
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
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
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 #:
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 #:
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 #:
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 #:
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 #:
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 #:
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 #:
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 #:
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 #:
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
[GitHub] ant pull request #:
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 #:
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 #:
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 #:
Github user bodewig commented on the pull request: https://github.com/apache/ant/commit/1e61ebdcad0aea45bb78627b231969f995a69f87#commitcomment-28691921 In src/tests/junit/org/apache/tools/ant/AntClassLoaderTest.java: In src/tests/junit/org/apache/tools/ant/AntClassLoaderTest.java on line 107: I'm afraid my Swedish is not up to the task :-) Let's make this concrete and then I'll shut up. Assuming the first call to `findClass` in the original line 97 throws an exception. Now if the second `findClass` in the original line 106 which is now in line 111 does not throw a `BuildException` will the new code fail? The old one would because of the original line 107. --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant pull request #:
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 #:
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 #:
Github user twogee commented on the pull request: https://github.com/apache/ant/commit/1e61ebdcad0aea45bb78627b231969f995a69f87#commitcomment-28688738 In src/tests/junit/org/apache/tools/ant/AntClassLoaderTest.java: In src/tests/junit/org/apache/tools/ant/AntClassLoaderTest.java on line 107: finally ensures all calls execute. This works by virtue of all thrown exceptions being identical. Try throwing something else and it explodes ð --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant pull request #:
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 #:
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 #:
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 #:
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 #:
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 #:
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 #:
Github user twogee commented on the pull request: https://github.com/apache/ant/commit/cc41d3c1611c3a091e489fc026b33afd02f7eaed#commitcomment-28525323 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: In Java 8+, that is... --- - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
[GitHub] ant pull request #:
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 #:
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 #:
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 #:
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 #:
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 #:
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 #:
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 #:
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 #:
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 #:
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 #:
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 #:
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 #:
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 #:
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 #:
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 #:
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