[SECURITY] CVE-2017-5645: Apache Ant 1.9.9 and 1.10.1 - Apache Log4j 1.2.13 security vulnerability

2018-02-06 Thread jhm
CVE-2017-5645: Apache Ant 1.9.9 and 1.10.1 - Apache Log4j 1.2.13 security
vulnerability

 

Severity: low

Vendor: The Apache Software Foundation

Versions Affected:

  Apache Ant 1.9.0 - 1.9.9

  Apache Ant 1.10.0 - 1.10.1

  The unsupported Apache Ant 1.8 and lower versions are also affected.

Description:

  When using Apache Ants Log4jListener there could be a security issue with
the

  underlying Apache Log4j library in version 1.x. 

  Please note that Log4j 1.x has reached its end of life and is no longer
maintained. 

  For details about migrating away from Log4j 1.x please consult with the
Apache Log4j team.

Mitigation:

  Users should not use the Log4JListener or use the log4j2-bridge.

  (Using the bridge requires Ant 1.9.10+ or Ant 1.10.2+.)

Credit: 

  This issue was discovered by Wade Schwarz of Oracle.

 

 

-Jan Matèrne

on behalf of the Apache Ant PMC



[ANN] Apache Ant 1.9.10 and 1.10.2 Released

2018-02-06 Thread Stefan Bodewig
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

The Apache Ant Team is pleased to announce the releases of Apache Ant
1.9.10 and 1.10.2.

Apache Ant is a Java library and command-line tool that helps building
software.

The Apache Ant team currently maintains two lines of development. The
1.9.x releases require Java5 at runtime and 1.10.x requires Java8 at
runtime. Both lines are based off of Ant 1.9.7 and the 1.9.x releases
are mostly bug fix releases while additional new features are developed
for 1.10.x. We recommend using 1.10.2 unless you are required to use
versions of Java prior to Java8 during the build process.

Ant 1.10.2 contains a superset of 1.9.10 - with the exception of a few
tasks and features that no longer work with Java8 anyway (like the apt
task).

Both releases are mostly bug fix releases with a few new features being
added. A new javaversion condition can be used to detect the version of
the JVM running Ant.

The log4j listener has been deprecated as log4j 1.x is no longer
actively developed and we've made sure the listener works when using the
log4j2 logging bridge. In 1.10.2 the imaging tasks have been deprecated
as JAI doesn't seem to work with Java9 anymore.

Source and binary distributions are available for download from the
Apache Ant download site:

http://ant.apache.org/bindownload.cgi
http://ant.apache.org/srcdownload.cgi

When downloading, please verify signatures using the KEYS file available
at the above location when downloading the release.

Changes in 1.10.2 include:
==

Changes that could break older environments:
- ---

 * updated the dependency of BCEL to 6.2.
   Bugzilla Report 61196

 * delete task previously would silently accept wildcard (*)
   value for the "file" attribute. That's no longer the case
   and an exception could get thrown by the underlying filesystem
   for such use. Usage like:

   

   should instead be changed to use resource collections like:

   
 
   

 * Commons Net 3.6 is binary-code, but not source compatible;
   see change list of Commons Net 3.0 for details

 * The Log4jListener is marked as deprecated as the required log4j library
   (in version 1.x) is not maintained any more.

 * Image task is marked as deprecated as the required JAI library is not
   maintained any more and internal APIs that JAI depended on are no longer
   available in Java 9.

Fixed bugs:
- ---

 * 's  child now skips s that lack a key or
   value.
   Bugzilla Report 60767

 * bootstrapping Ant on Windows failed
   Bugzilla Report 61027

 * Fixed the issue where the SCP based tasks would try to change
   the permissions on the parent directory of a transferred file,
   instead of changing it on the transferred file itself.
   Bugzilla Reports 59648 and 43271

 * Fixed the issue where the source file being copied could end
   up being corrupted if the target of the copy happened to be
   the same source file (symlinked back to itself).
   Bugzilla Report 60644

 * Fixed the issue where symlink creation with "overwrite=false",
   on existing symlink whose target was a directory, would end
   up creating a new symlink under the target directory.
   Bugzilla Report 58683

 * Improvement to the Zip task for reduced memory usage in certain
   cases. Thanks to Glen Lewis for reporting the issue and
   suggesting the fix.
   Bugzilla Report 19516

 * Fixed an issue where the content redirected from output/error
   streams of a process, could end up being truncated.
   Bugzilla Report 58833, 58451

 * // will now throw an exception
   with a more useful error message when setFile is called twice on
   the same instance.
   Bugzilla Report 62071

Other changes:
- --

 * Added forceCsvQuoteChar option to  task. When enabled the
   values always get quoted.
   Github Pull Request #32

 * Added  attributes to various script related tasks and a
   compiled attribute to scriptdef.
   Github Pull Request #30

 * Added support for jarsigner's -tsadigestalg to .
   Bugzilla Report 60665

 * added "regexp" attribute to 
   Bugzilla Report 60968

 * reduced GC pressure by replacing all usage of FileInputStream and
   FileOutputStream.

 * Task can now also use attribute setters that expect a
   java.nio.file.Path argument.
   Bugzilla Report 61042

 * added a new magic property ant.tstamp.now that can be used to
   override the current time/date used by .
   Bugzilla Report 61079

 * added Orion support to ejbjar
   Github Pull Request #33

 * SCP task, when configured to use SFTP protocol, now preserves last
   modified timestamp on files that it uploads, if the
   preserveLastModified attribute is set to true for that task
   Bugzilla Report 58589

 * zip and the related tasks can now set the modification time of all
   entries to a fixed timestamp.
   Github Pull Request #36

 * Jsch library dependency has now been upgraded to 0.1.54. Jsch is
   the library behind the sshexec and scp Ant 

Re: Release Notes for 1.10.2 and 1.9.10

2018-02-06 Thread Jaikiran Pai
Maybe the new  task is worth highlighting. But other than 
that I don't have anything specific.


-Jaikiran


On 06/02/18 11:18 PM, Stefan Bodewig wrote:

Hi all

as you can witness in
https://svn.apache.org/viewvc/ant/site/ant/sources/antnews.xml?r1=1823368=1823367=1823368
I don't really know which change to highlight specifically.

The announcement is going to contain all changes, is there any one or
two changes you think we should highlight in the news page and the
initial paragraphs of the announcement mail (most of it is boilerplate
anyway).

Stefan

-
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: Release Notes for 1.10.2 and 1.9.10

2018-02-06 Thread Gintautas Grigelionis
Please highlight JAI removal in Java 9+ and deprecation of Log4j.

Gintas

2018-02-06 18:48 GMT+01:00 Stefan Bodewig :

> Hi all
>
> as you can witness in
> https://svn.apache.org/viewvc/ant/site/ant/sources/antnews.
> xml?r1=1823368=1823367=1823368
> I don't really know which change to highlight specifically.
>
> The announcement is going to contain all changes, is there any one or
> two changes you think we should highlight in the news page and the
> initial paragraphs of the announcement mail (most of it is boilerplate
> anyway).
>
> Stefan
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
> For additional commands, e-mail: dev-h...@ant.apache.org
>
>


Re: ant git commit: Generate manifest files and add automatic module names for JPMS

2018-02-06 Thread Gintautas Grigelionis
2018-02-06 11:05 GMT+01:00 Stefan Bodewig :

> On 2018-02-05, Gintautas Grigelionis wrote:
>
> > I adjusted the proposal and I hope that I addressed your criticism.
>
> Unfortunately it doesn't.
>
> I'm afraid that we would be sending a wrong signal here. On top of that
> I don't think Ant would actually work if parts of it are used as
> modules. We use classloaders and Class.forName all over the place, not
> ServiceLoader, for example.
>
> If the taskdef/typedef implementation classes are loaded via a module
> path and a custom task lives on the CLASSPATH will taskdef be able to
> load it at all?
>

Anything on the CLASSPATH is in the unnamed module.
The worst that can happen is the situation where a package split between
module path and classpath.
That's what --patch-module is for.


> As for moving classes and adding stubs for backwards compatibility -
> let's please evaluate what we'd gain with such a move.
>
> Ant is used as a code dependency by way more things than we know. We do
> know Gradle and SBT directly call into Ant classes under the
> covers. Lots and lots of custom tasks have been written that rely on our
> API. If you use some kind of Maven antrun construct then moving classes
> around and adding a new jar means you have to add a new dependency when
> you want to upgrade to a new version. This is inconvenient and may turn
> out to cause difficult to diagnose problems.
>

Modules is the way to take back control. There are switches in Java 9 to
enable access.
We should rather be doing now when JRE is still lenient. Who knows what to
expect from Java 11 LTS (6 months from now)?

Gintas


Release Notes for 1.10.2 and 1.9.10

2018-02-06 Thread Stefan Bodewig
Hi all

as you can witness in
https://svn.apache.org/viewvc/ant/site/ant/sources/antnews.xml?r1=1823368=1823367=1823368
I don't really know which change to highlight specifically.

The announcement is going to contain all changes, is there any one or
two changes you think we should highlight in the news page and the
initial paragraphs of the announcement mail (most of it is boilerplate
anyway).

Stefan

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



[RESULT] Release Apache Ant 1.10.2 based on RC1

2018-02-06 Thread Stefan Bodewig
On 2018-02-03, Stefan Bodewig wrote:

> Hi all

> I've created a release candidate for 1.10.2:

Jan Matèrne
Jaikiran Pai
Stefan Bodewig

and no other votes the vote has passed.

I'll publish the artifacts and give the mirrors a few hours time to
catch up before updating the site and send out the release
announcement(s).

Stefan

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



[RESULT] Release Ant 1.9.10 based on RC1

2018-02-06 Thread Stefan Bodewig
On 2018-02-03, Stefan Bodewig wrote:

> I've created a release candidate for 1.9.10:

with +1s by

Jaikiran Pai
Jan Matèrne
Maarten Coene
Stefan Bodewig

and no other votes the vote has passed.

Stefan

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



Re: github PR builds

2018-02-06 Thread Stefan Bodewig
On 2018-02-06, Jaikiran Pai wrote:

> The PR build on Jenkins is backed by the github PR integration
> plugin[1]. One of the features of that plugin is to prevent some
> malicious/rogue PR (imagine someone creating a PR with code which does
> some odd things with the host on which it runs) being auto-triggered
> against the Jenkins hosts.

I see. It's just that I never saw this in use before - and to be honest
am a bit annoyed by the bot asking for confirmation on every pull
request :-)

> From what I remember, our Ant job is configured to consider ASF
> members as whitelisted and admins, so if some ASF members opens a PR,
> it auto-triggers a job and also lets ASF members to put in a "this is
> ok to test" message to trigger the job for users who aren't part of
> the whitelist or aren't ASF members themselves. I'll re-check the Ant
> job to make sure that indeed is how it is configured.

> Note that when I say ASF members, I'm talking about github users who
> belong to the apache organization.

Thanks. And don't worry about not providing details in your initial
email - I actually don't recall the mail at all and most probably you
provided everything that was needed.

Stefan

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



[GitHub] ant-ivy pull request #62: IVY-1572 - Check for Thread interruption while res...

2018-02-06 Thread apupier
Github user apupier commented on a diff in the pull request:

https://github.com/apache/ant-ivy/pull/62#discussion_r166349423
  
--- Diff: 
src/java/org/apache/ivy/plugins/resolver/AbstractPatternsBasedResolver.java ---
@@ -91,6 +91,9 @@ protected ResolvedResource 
findResourceUsingPatterns(ModuleRevisionId moduleRevi
 Set foundRevisions = new HashSet<>();
 boolean dynamic = 
getSettings().getVersionMatcher().isDynamic(moduleRevision);
 for (String pattern : patternList) {
+if (Thread.currentThread().isInterrupted()) {
+return null;
--- End diff --

the specific exception in this case is in theory 
https://docs.oracle.com/javase/7/docs/api/java/lang/InterruptedException.html 
which is a checked Exception.
Do you have another runtime exception in mind? Or you want to create a 
specific Runtime Exception? I'm wondering what would be the effect in this case 
to Thread in which it has been launched. Will it finished also?


---

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



Re: ant git commit: Do not add automatic module names

2018-02-06 Thread Gintautas Grigelionis
I put things back as they were, but we're breaking specs [1] by not having
unique Extension-Name (not that anybody cares).

Gintas

[1]
https://docs.oracle.com/javase/8/docs/technotes/guides/jar/jar.html#Main_Attributes
(point 4)

2018-02-06 13:09 GMT+01:00 Stefan Bodewig :

> On 2018-02-06,  wrote:
>
> > Do not add automatic module names
>
> thank you
>
> Stefan
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
> For additional commands, e-mail: dev-h...@ant.apache.org
>
>


[GitHub] ant-ivy pull request #62: IVY-1572 - Check for Thread interruption while res...

2018-02-06 Thread jaikiran
Github user jaikiran commented on a diff in the pull request:

https://github.com/apache/ant-ivy/pull/62#discussion_r166282397
  
--- Diff: 
src/java/org/apache/ivy/plugins/resolver/AbstractPatternsBasedResolver.java ---
@@ -91,6 +91,9 @@ protected ResolvedResource 
findResourceUsingPatterns(ModuleRevisionId moduleRevi
 Set foundRevisions = new HashSet<>();
 boolean dynamic = 
getSettings().getVersionMatcher().isDynamic(moduleRevision);
 for (String pattern : patternList) {
+if (Thread.currentThread().isInterrupted()) {
+return null;
--- End diff --

@apupier , I don't think this is the best place to check the thread 
interruption, but I don't have a better suggestion right now, so unless someone 
objects, this change is probably fine. However, instead of returning null, can 
you please throw a (runtime) exception instead.


---

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



Re: ant git commit: Do not add automatic module names

2018-02-06 Thread Stefan Bodewig
On 2018-02-06,  wrote:

> Do not add automatic module names

thank you

Stefan

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



Re: github PR builds

2018-02-06 Thread Jaikiran Pai
So it looks like, although I did send a mail about this PR integration a 
while back[1], I did not include the details about this plugin which was 
used for the integration. Sorry about that.


[1] https://www.mail-archive.com/dev@ant.apache.org/msg46284.html

-Jaikiran


On 06/02/18 5:12 PM, Jaikiran Pai wrote:
The PR build on Jenkins is backed by the github PR integration 
plugin[1]. One of the features of that plugin is to prevent some 
malicious/rogue PR (imagine someone creating a PR with code which does 
some odd things with the host on which it runs) being auto-triggered 
against the Jenkins hosts. The plugin can be configured to disable 
this feature or (like now) be configured to allow a whitelist of users 
for whom the job gets triggered when they open a PR. If a user not 
belonging to that whitelist opens a PR, then one of the admins (also 
configurable in the plugin) can add a "this is ok to test" message (of 
course after doing some basic checks about the content of the PR 
itself) so that the job gets triggered.


From what I remember, our Ant job is configured to consider ASF 
members as whitelisted and admins, so if some ASF members opens a PR, 
it auto-triggers a job and also lets ASF members to put in a "this is 
ok to test" message to trigger the job for users who aren't part of 
the whitelist or aren't ASF members themselves. I'll re-check the Ant 
job to make sure that indeed is how it is configured.


Note that when I say ASF members, I'm talking about github users who 
belong to the apache organization.


P.S: There are a few other keywords that the plugin recognizes and is 
documented at [1].


[1] 
https://wiki.jenkins.io/display/JENKINS/GitHub+pull+request+builder+plugin


-Jaikiran


On 06/02/18 4:59 PM, Stefan Bodewig wrote:

Hi

if I understand correctly our current PR build setup with Jenkins
requires somebody to comment on the issue in order to have Jenkins build
it.

Do we really want this extra step? For Commons Compress I never thought
about something like that.

If we do, how does Jenkins know who is allowed to trigger builds? Is
this a list or derived from membership in the apache github organization
or how does it work?

Sorry if this has been discussed before, I simply don't recall it.

Stefan

-
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 PR builds

2018-02-06 Thread Jaikiran Pai
The PR build on Jenkins is backed by the github PR integration 
plugin[1]. One of the features of that plugin is to prevent some 
malicious/rogue PR (imagine someone creating a PR with code which does 
some odd things with the host on which it runs) being auto-triggered 
against the Jenkins hosts. The plugin can be configured to disable this 
feature or (like now) be configured to allow a whitelist of users for 
whom the job gets triggered when they open a PR. If a user not belonging 
to that whitelist opens a PR, then one of the admins (also configurable 
in the plugin) can add a "this is ok to test" message (of course after 
doing some basic checks about the content of the PR itself) so that the 
job gets triggered.


From what I remember, our Ant job is configured to consider ASF members 
as whitelisted and admins, so if some ASF members opens a PR, it 
auto-triggers a job and also lets ASF members to put in a "this is ok to 
test" message to trigger the job for users who aren't part of the 
whitelist or aren't ASF members themselves. I'll re-check the Ant job to 
make sure that indeed is how it is configured.


Note that when I say ASF members, I'm talking about github users who 
belong to the apache organization.


P.S: There are a few other keywords that the plugin recognizes and is 
documented at [1].


[1] 
https://wiki.jenkins.io/display/JENKINS/GitHub+pull+request+builder+plugin


-Jaikiran


On 06/02/18 4:59 PM, Stefan Bodewig wrote:

Hi

if I understand correctly our current PR build setup with Jenkins
requires somebody to comment on the issue in order to have Jenkins build
it.

Do we really want this extra step? For Commons Compress I never thought
about something like that.

If we do, how does Jenkins know who is allowed to trigger builds? Is
this a list or derived from membership in the apache github organization
or how does it work?

Sorry if this has been discussed before, I simply don't recall it.

Stefan

-
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-ivy pull request #65: IVY-1485 Ensure dependency is applicable to all co...

2018-02-06 Thread twogee
Github user twogee closed the pull request at:

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


---

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



github PR builds

2018-02-06 Thread Stefan Bodewig
Hi

if I understand correctly our current PR build setup with Jenkins
requires somebody to comment on the issue in order to have Jenkins build
it.

Do we really want this extra step? For Commons Compress I never thought
about something like that.

If we do, how does Jenkins know who is allowed to trigger builds? Is
this a list or derived from membership in the apache github organization
or how does it work?

Sorry if this has been discussed before, I simply don't recall it.

Stefan

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



Re: [VOTE] Release Apache Ant 1.10.2 based on RC1

2018-02-06 Thread Stefan Bodewig
On 2018-02-03, Stefan Bodewig wrote:

> git tag: ANT_1.10.2_RC1
>  on commit: 0eba7eb

+1

Stefan

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



Re: [VOTE] Release Ant 1.9.10 based on RC1

2018-02-06 Thread Stefan Bodewig
On 2018-02-03, Stefan Bodewig wrote:

> git tag: ANT_1_9_10_RC1
>  on commit: 528c94e

+1

Stefan

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



Re: ant git commit: Generate manifest files and add automatic module names for JPMS

2018-02-06 Thread Stefan Bodewig
On 2018-02-06, Jaikiran Pai wrote:

>  don't think we should be doing any of these commits especially when
> there's a RC out which we plan to release.

In general we live in a commit then review world, so I think it is kind
of OK to perform bigger changes without discussion upfront. It would
have been good to get some kind of heads-up, tough.

As for commits while a vote is going on, personally I don't care too
much. The RC has been cut from a detached head and if some changes would
be required for a second RC they can be added and merged back to master
by turning the head into a proper branch.

What will happen though (assuming the vote passes) is that we get a
non-linear history where users may assume some changes to master were
part of the release because they happened before the tag was merged back
if they only look at the timestamps rather than the commit graph.

Stefan

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



Re: ant git commit: Generate manifest files and add automatic module names for JPMS

2018-02-06 Thread Stefan Bodewig
On 2018-02-05, Gintautas Grigelionis wrote:

> I adjusted the proposal and I hope that I addressed your criticism.

Unfortunately it doesn't.

I'm afraid that we would be sending a wrong signal here. On top of that
I don't think Ant would actually work if parts of it are used as
modules. We use classloaders and Class.forName all over the place, not
ServiceLoader, for example.

If the taskdef/typedef implementation classes are loaded via a module
path and a custom task lives on the CLASSPATH will taskdef be able to
load it at all?

Without really having tried a setup of some of our jars coming from a
module loader and the rest living on a CLASSPATH and trying out all the
different scenarios of custom tasks/types/loggers, taking into account
classes that do something like

public void add(SomeInterface foo) { ,,, }

in order to allow any typedeffed implementation of the interface to be
used as a nested child element and all the other edge cases, I wouldn't
want to send the slightest signal that Ant was supporting the module
system in any way. It simply has been designed for a classloader world
and heavily relies on it.

As for moving classes and adding stubs for backwards compatibility -
let's please evaluate what we'd gain with such a move.

Ant is used as a code dependency by way more things than we know. We do
know Gradle and SBT directly call into Ant classes under the
covers. Lots and lots of custom tasks have been written that rely on our
API. If you use some kind of Maven antrun construct then moving classes
around and adding a new jar means you have to add a new dependency when
you want to upgrade to a new version. This is inconvenient and may turn
out to cause difficult to diagnose problems.

Stefan

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



[GitHub] ant-ivy issue #65: IVY-1485 Ensure dependency is applicable to all configura...

2018-02-06 Thread jaikiran
Github user jaikiran commented on the issue:

https://github.com/apache/ant-ivy/pull/65
  
These changes don't solve the real issue. The details of this issue are 
part of the discussion in ant dev mailing list, where I explained what the real 
issue is. I do have a completely different code with tests that's not yet ready 
to be merged. 


---

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



Re: ant git commit: Generate manifest files and add automatic module names for JPMS

2018-02-06 Thread Gintautas Grigelionis
I understand the intent of your votes. I do experiment with using module
path in a real world application, so it's not an art for art's sake.
I will move the proposal to a PR. Talking of which, have you any comments
on IVY-1485 (aka PR-65)?

Gintas

2018-02-06 9:46 GMT+01:00 Jaikiran Pai :

> Just to be clear, my -1 was meant for both this commit as well as a
> subsequent commit where some specific jars have been tagged as JPMS
> modules. I think adding this automatic module names just for the sake of it
> isn't a good thing.
>
> -Jaikiran
>
>
>
> On 06/02/18 9:38 AM, Jaikiran Pai wrote:
>
>> I agree. -1.
>>
>> On a related note, I don't think we should be doing any of these commits
>> especially when there's a RC out which we plan to release. IMO, only
>> blocker issues need to be addressed when the RC is out.
>>
>> -Jaikiran
>>
>>
>> On 06/02/18 1:41 AM, Stefan Bodewig wrote:
>>
>>> Generate manifest files and add automatic module names for JPMS

>>> -1
>>>
>>> please remove the Automatic-Module-Name attributes as our jars are no
>>> proper JPMS modules.
>>>
>>> Stefan
>>>
>>> -
>>> 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: ant git commit: Generate manifest files and add automatic module names for JPMS

2018-02-06 Thread Jaikiran Pai
Just to be clear, my -1 was meant for both this commit as well as a 
subsequent commit where some specific jars have been tagged as JPMS 
modules. I think adding this automatic module names just for the sake of 
it isn't a good thing.


-Jaikiran


On 06/02/18 9:38 AM, Jaikiran Pai wrote:

I agree. -1.

On a related note, I don't think we should be doing any of these 
commits especially when there's a RC out which we plan to release. 
IMO, only blocker issues need to be addressed when the RC is out.


-Jaikiran


On 06/02/18 1:41 AM, Stefan Bodewig wrote:

Generate manifest files and add automatic module names for JPMS

-1

please remove the Automatic-Module-Name attributes as our jars are no
proper JPMS modules.

Stefan

-
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 #58: Use StringBuilder instead of StringBuffer as it offers...

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

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

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

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


---

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



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

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

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

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


---

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