[GitHub] [tomcat] larsgrefer commented on a change in pull request #272: Loop can be terminated after condition is met

[GitBox]

larsgrefer commented on a change in pull request #272: Loop can be terminated 
after condition is met
URL: https://github.com/apache/tomcat/pull/272#discussion_r405148102
 
 

 ##
 File path: java/org/apache/catalina/realm/RealmBase.java
 ##
 @@ -756,10 +756,11 @@ public void backgroundProcess() {
 }
 
 boolean matched = false;
-for(int k=0; k < patterns.length && !matched; k++) {
+for(int k = 0; k < patterns.length; k++) {
 
 Review comment:
   I wanted to make a separate PR for all possible foreach loops later


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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

[GitHub] [tomcat] michael-o commented on a change in pull request #272: Loop can be terminated after condition is met

[GitBox]

michael-o commented on a change in pull request #272: Loop can be terminated 
after condition is met
URL: https://github.com/apache/tomcat/pull/272#discussion_r405110647
 
 

 ##
 File path: java/org/apache/catalina/tribes/tipis/AbstractReplicatedMap.java
 ##
 @@ -824,17 +824,23 @@ public void mapMemberAdded(Member member) {
 public boolean inSet(Member m, Member[] set) {
 if ( set == null ) return false;
 boolean result = false;
-for (int i=0; i result = new ArrayList<>();
 for (int i=0; i

[GitHub] [tomcat] michael-o commented on a change in pull request #272: Loop can be terminated after condition is met

[GitBox]

michael-o commented on a change in pull request #272: Loop can be terminated 
after condition is met
URL: https://github.com/apache/tomcat/pull/272#discussion_r405110590
 
 

 ##
 File path: java/org/apache/catalina/tribes/tipis/AbstractReplicatedMap.java
 ##
 @@ -824,17 +824,23 @@ public void mapMemberAdded(Member member) {
 public boolean inSet(Member m, Member[] set) {
 if ( set == null ) return false;
 boolean result = false;
-for (int i=0; i

[GitHub] [tomcat] michael-o commented on a change in pull request #272: Loop can be terminated after condition is met

[GitBox]

michael-o commented on a change in pull request #272: Loop can be terminated 
after condition is met
URL: https://github.com/apache/tomcat/pull/272#discussion_r405110511
 
 

 ##
 File path: java/org/apache/catalina/realm/RealmBase.java
 ##
 @@ -756,10 +756,11 @@ public void backgroundProcess() {
 }
 
 boolean matched = false;
-for(int k=0; k < patterns.length && !matched; k++) {
+for(int k = 0; k < patterns.length; k++) {
 
 Review comment:
   Why not make it foreach directly?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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

buildbot success in on tomcat-trunk

[buildbot]

The Buildbot has detected a restored build on builder tomcat-trunk while 
building tomcat. Full details are available at:
https://ci.apache.org/builders/tomcat-trunk/builds/5099

Buildbot URL: https://ci.apache.org/

Buildslave for this Build: asf946_ubuntu

Build Reason: The AnyBranchScheduler scheduler named 'on-tomcat-commit' 
triggered this build
Build Source Stamp: [branch master] 1eabf4e3d2059cd684b1f075059ba1e31a69ecf3
Blamelist: remm 

Build succeeded!

Sincerely,
 -The Buildbot




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

buildbot success in on tomcat-9-trunk

[buildbot]

The Buildbot has detected a restored build on builder tomcat-9-trunk while 
building tomcat. Full details are available at:
https://ci.apache.org/builders/tomcat-9-trunk/builds/158

Buildbot URL: https://ci.apache.org/

Buildslave for this Build: asf946_ubuntu

Build Reason: The AnyBranchScheduler scheduler named 'on-tomcat-9-commit' 
triggered this build
Build Source Stamp: [branch 9.0.x] e540983e76998b7b41c4ad230a2cc219239f7800
Blamelist: remm 

Build succeeded!

Sincerely,
 -The Buildbot




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

Re: API Change - Connector.java Constructor

[Rémy Maucherat]

On Tue, Apr 7, 2020 at 8:42 PM Mark Thomas  wrote:

> On 07/04/2020 19:03, Filip Hanik wrote:
> >
> >
> > On Tue, Apr 7, 2020 at 9:35 AM Rémy Maucherat  > > wrote:
> >
> >
> > Does the connector need to know about the actual implementations?
> >
> >
> > Ideally no, but it removes the reflection you say is bad for Graal.
> >
> >
> > Correct. Turns out that the connectors use setProperty/getProperty via
> > reflection (IntrospectionUtils.setProperty/getProperty), so changing
> > only this constructor would achieve a mini step.
> > Before we commit any changes, I'd like to evaluate the scope of
> > reflection we're dealing with.
> >
> > Then I can come back. I'll close the PR for now, as it only touches the
> > surface.
> >
> > sound fair?
>
> Sounds reasonable. I'm happy for any obvious clean-up to stay though.
>
> On a similar note, the ProtocolHandler calls setProperty() on the
> Endpoint which then also uses reflection.
>
> I think I have a way around this but it is not great for maintenance.
>

Yes, I don't see how it could be a good move.

The Connector creation cleanup sounded like a good refactoring however,
especially the [horrible] AJP part of it.

Rémy

[tomcat] branch 9.0.x updated: Improve Connector creation

[remm]

This is an automated email from the ASF dual-hosted git repository.

remm pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/9.0.x by this push:
 new e540983  Improve Connector creation
e540983 is described below

commit e540983e76998b7b41c4ad230a2cc219239f7800
Author: remm 
AuthorDate: Tue Apr 7 20:58:54 2020 +0200

Improve Connector creation

Remove unneeded reflection. More importantly remove AJP specific hack in
favor of a new default method in ProtocolHandler. No behavior change for
existing API.
---
 java/org/apache/catalina/connector/Connector.java  | 66 +++-
 java/org/apache/coyote/ProtocolHandler.java| 71 ++
 .../org/apache/coyote/ajp/AbstractAjpProtocol.java |  6 ++
 webapps/docs/changelog.xml |  8 +++
 4 files changed, 106 insertions(+), 45 deletions(-)

diff --git a/java/org/apache/catalina/connector/Connector.java 
b/java/org/apache/catalina/connector/Connector.java
index 534246f..f30f26e 100644
--- a/java/org/apache/catalina/connector/Connector.java
+++ b/java/org/apache/catalina/connector/Connector.java
@@ -35,7 +35,6 @@ import org.apache.coyote.AbstractProtocol;
 import org.apache.coyote.Adapter;
 import org.apache.coyote.ProtocolHandler;
 import org.apache.coyote.UpgradeProtocol;
-import org.apache.coyote.ajp.AbstractAjpProtocol;
 import org.apache.coyote.http11.AbstractHttp11JsseProtocol;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
@@ -75,49 +74,37 @@ public class Connector extends LifecycleMBeanBase  {
  * Defaults to using HTTP/1.1 NIO implementation.
  */
 public Connector() {
-this("org.apache.coyote.http11.Http11NioProtocol");
+this("HTTP/1.1");
 }
 
 
 public Connector(String protocol) {
-boolean aprConnector = AprLifecycleListener.isAprAvailable() &&
+boolean apr = AprLifecycleListener.isAprAvailable() &&
 AprLifecycleListener.getUseAprConnector();
-
-if ("HTTP/1.1".equals(protocol) || protocol == null) {
-if (aprConnector) {
-protocolHandlerClassName = 
"org.apache.coyote.http11.Http11AprProtocol";
-} else {
-protocolHandlerClassName = 
"org.apache.coyote.http11.Http11NioProtocol";
-}
-} else if ("AJP/1.3".equals(protocol)) {
-if (aprConnector) {
-protocolHandlerClassName = 
"org.apache.coyote.ajp.AjpAprProtocol";
-} else {
-protocolHandlerClassName = 
"org.apache.coyote.ajp.AjpNioProtocol";
-}
-} else {
-protocolHandlerClassName = protocol;
-}
-
-// Instantiate protocol handler
 ProtocolHandler p = null;
 try {
-Class clazz = Class.forName(protocolHandlerClassName);
-p = (ProtocolHandler) clazz.getConstructor().newInstance();
+p = ProtocolHandler.create(protocol, apr);
 } catch (Exception e) {
 log.error(sm.getString(
 "coyoteConnector.protocolHandlerInstantiationFailed"), e);
-} finally {
-this.protocolHandler = p;
 }
-
+if (p != null) {
+protocolHandler = p;
+protocolHandlerClassName = protocolHandler.getClass().getName();
+} else {
+protocolHandler = null;
+protocolHandlerClassName = protocol;
+}
 // Default for Connector depends on this system property
 
setThrowOnFailure(Boolean.getBoolean("org.apache.catalina.startup.EXIT_ON_INIT_FAILURE"));
+}
 
-// Default for Connector depends on this (deprecated) system property
-if 
(Boolean.parseBoolean(System.getProperty("org.apache.tomcat.util.buf.UDecoder.ALLOW_ENCODED_SLASH",
 "false"))) {
-encodedSolidusHandling = EncodedSolidusHandling.DECODE;
-}
+
+public Connector(ProtocolHandler protocolHandler) {
+protocolHandlerClassName = protocolHandler.getClass().getName();
+this.protocolHandler = protocolHandler;
+// Default for Connector depends on this system property
+
setThrowOnFailure(Boolean.getBoolean("org.apache.catalina.startup.EXIT_ON_INIT_FAILURE"));
 }
 
 
@@ -621,18 +608,7 @@ public class Connector extends LifecycleMBeanBase  {
  * @return the Coyote protocol handler in use.
  */
 public String getProtocol() {
-if 
(("org.apache.coyote.http11.Http11NioProtocol".equals(getProtocolHandlerClassName())
 &&
-(!AprLifecycleListener.isAprAvailable() || 
!AprLifecycleListener.getUseAprConnector())) ||
-
"org.apache.coyote.http11.Http11AprProtocol".equals(getProtocolHandlerClassName())
 &&
-AprLifecycleListener.getUseAprConnector()) {
-return "HTTP/1.1";
-} else if 
(("org.apache.coyote.

[tomcat] branch master updated: Move getProtocol method to be consistent

[remm]

This is an automated email from the ASF dual-hosted git repository.

remm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/master by this push:
 new 1eabf4e  Move getProtocol method to be consistent
1eabf4e is described below

commit 1eabf4e3d2059cd684b1f075059ba1e31a69ecf3
Author: remm 
AuthorDate: Tue Apr 7 20:56:57 2020 +0200

Move getProtocol method to be consistent
---
 java/org/apache/catalina/connector/Connector.java |  7 +--
 java/org/apache/coyote/ProtocolHandler.java   | 14 ++
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/java/org/apache/catalina/connector/Connector.java 
b/java/org/apache/catalina/connector/Connector.java
index ca7b909..f529161 100644
--- a/java/org/apache/catalina/connector/Connector.java
+++ b/java/org/apache/catalina/connector/Connector.java
@@ -651,12 +651,7 @@ public class Connector extends LifecycleMBeanBase  {
  * @return the Coyote protocol handler in use.
  */
 public String getProtocol() {
-if 
("org.apache.coyote.http11.Http11NioProtocol".equals(getProtocolHandlerClassName()))
 {
-return "HTTP/1.1";
-} else if 
("org.apache.coyote.ajp.AjpNioProtocol".equals(getProtocolHandlerClassName())) {
-return "AJP/1.3";
-}
-return getProtocolHandlerClassName();
+return ProtocolHandler.getProtocol(getProtocolHandlerClassName());
 }
 
 
diff --git a/java/org/apache/coyote/ProtocolHandler.java 
b/java/org/apache/coyote/ProtocolHandler.java
index c6e1565..902e86e 100644
--- a/java/org/apache/coyote/ProtocolHandler.java
+++ b/java/org/apache/coyote/ProtocolHandler.java
@@ -222,4 +222,18 @@ public interface ProtocolHandler {
 }
 
 
+/**
+ * Get the protocol name associated with the protocol class.
+ * @param protocolClassName the protocol class name
+ * @return the protocol name
+ */
+public static String getProtocol(String protocolClassName) {
+if 
(org.apache.coyote.http11.Http11NioProtocol.class.getName().equals(protocolClassName))
 {
+return "HTTP/1.1";
+} else if 
(org.apache.coyote.ajp.AjpNioProtocol.class.getName().equals(protocolClassName))
 {
+return "AJP/1.3";
+}
+return protocolClassName;
+}
+
 }


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

Re: API Change - Connector.java Constructor

[Mark Thomas]

On 07/04/2020 19:03, Filip Hanik wrote:
> 
> 
> On Tue, Apr 7, 2020 at 9:35 AM Rémy Maucherat  > wrote:
> 
> 
> Does the connector need to know about the actual implementations?
> 
> 
> Ideally no, but it removes the reflection you say is bad for Graal.
> 
> 
> Correct. Turns out that the connectors use setProperty/getProperty via
> reflection (IntrospectionUtils.setProperty/getProperty), so changing
> only this constructor would achieve a mini step.
> Before we commit any changes, I'd like to evaluate the scope of
> reflection we're dealing with. 
> 
> Then I can come back. I'll close the PR for now, as it only touches the
> surface.
> 
> sound fair?

Sounds reasonable. I'm happy for any obvious clean-up to stay though.

On a similar note, the ProtocolHandler calls setProperty() on the
Endpoint which then also uses reflection.

I think I have a way around this but it is not great for maintenance.

Mark

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

[tomcat] branch master updated: Add boilerplate javadoc

[remm]

This is an automated email from the ASF dual-hosted git repository.

remm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/master by this push:
 new 8f3f98b  Add boilerplate javadoc
8f3f98b is described below

commit 8f3f98bb3f804159c1561da3fa333ed67de4
Author: remm 
AuthorDate: Tue Apr 7 20:35:00 2020 +0200

Add boilerplate javadoc
---
 java/org/apache/coyote/ProtocolHandler.java | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/java/org/apache/coyote/ProtocolHandler.java 
b/java/org/apache/coyote/ProtocolHandler.java
index 57a3086..c6e1565 100644
--- a/java/org/apache/coyote/ProtocolHandler.java
+++ b/java/org/apache/coyote/ProtocolHandler.java
@@ -197,13 +197,13 @@ public interface ProtocolHandler {
  * Create a new ProtocolHandler for the given protocol.
  * @param protocol the protocol
  * @return the newly instantiated protocol handler
- * @throws ClassNotFoundException
- * @throws InstantiationException
- * @throws IllegalAccessException
- * @throws IllegalArgumentException
- * @throws InvocationTargetException
- * @throws NoSuchMethodException
- * @throws SecurityException
+ * @throws ClassNotFoundException Specified protocol was not found
+ * @throws InstantiationException Specified protocol could not be 
instantiated
+ * @throws IllegalAccessException Exception occurred
+ * @throws IllegalArgumentException Exception occurred
+ * @throws InvocationTargetException Exception occurred
+ * @throws NoSuchMethodException Exception occurred
+ * @throws SecurityException Exception occurred
  */
 public static ProtocolHandler create(String protocol)
 throws ClassNotFoundException, InstantiationException, 
IllegalAccessException,


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

[GitHub] [tomcat] fhanik commented on issue #267: Remove reflection from Connector.java

[GitBox]

fhanik commented on issue #267: Remove reflection from Connector.java
URL: https://github.com/apache/tomcat/pull/267#issuecomment-610548217
 
 
   AbstractProtocol and AbstractEndpoint use reflection for all property 
setters. This change is a step in the right direction, but only has minimal 
impact.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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

Re: API Change - Connector.java Constructor

[Filip Hanik]

On Tue, Apr 7, 2020 at 9:35 AM Rémy Maucherat  wrote:

>
>> Does the connector need to know about the actual implementations?
>>
>
> Ideally no, but it removes the reflection you say is bad for Graal.
>

Correct. Turns out that the connectors use setProperty/getProperty via
reflection (IntrospectionUtils.setProperty/getProperty), so changing only
this constructor would achieve a mini step.
Before we commit any changes, I'd like to evaluate the scope of reflection
we're dealing with.

Then I can come back. I'll close the PR for now, as it only touches the
surface.

sound fair?

Filip

[Bug 64311] org.apache.tomcat.jni.TestSocketServerAnyLocalAddress locks entire testbed run under certain conditions

[bugzilla]

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

--- Comment #2 from Michael Osipov  ---
(In reply to Mark Thomas from comment #1)
> I'm struggling to recreate this.
> 
> I have APR 1.7.x compiled with --disable-threads
> I have Tomcat-Native at 57da160 (just before the fix for BZ 63671)
> I have tomcat master (current HEAD)
> 
> The tests fail because of this test:
> https://github.com/apache/tomcat/blob/master/java/org/apache/tomcat/jni/
> Library.java#L251
> 
> which has been present for 14+ years.
> 
> How did you manage to run the test with APR threading disabled?

My bad, I forgot to mention this. I have removed the if clause to see whether
Tomcat really requires this. It obviously don't. Only OpenSSL < 1.1.0 requires
the threading support.

Please remove that check and try again.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 64311] org.apache.tomcat.jni.TestSocketServerAnyLocalAddress locks entire testbed run under certain conditions

[bugzilla]

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

--- Comment #1 from Mark Thomas  ---
I'm struggling to recreate this.

I have APR 1.7.x compiled with --disable-threads
I have Tomcat-Native at 57da160 (just before the fix for BZ 63671)
I have tomcat master (current HEAD)

The tests fail because of this test:
https://github.com/apache/tomcat/blob/master/java/org/apache/tomcat/jni/Library.java#L251

which has been present for 14+ years.

How did you manage to run the test with APR threading disabled?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

buildbot failure in on tomcat-trunk

[buildbot]

The Buildbot has detected a new failure on builder tomcat-trunk while building 
tomcat. Full details are available at:
https://ci.apache.org/builders/tomcat-trunk/builds/5097

Buildbot URL: https://ci.apache.org/

Buildslave for this Build: asf946_ubuntu

Build Reason: The AnyBranchScheduler scheduler named 'on-tomcat-commit' 
triggered this build
Build Source Stamp: [branch master] 15540305ab97b12156536d26e17895b750f12a38
Blamelist: remm 

BUILD FAILED: failed compile

Sincerely,
 -The Buildbot




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

[tomcat] branch master updated: Improve Connector creation

[remm]

This is an automated email from the ASF dual-hosted git repository.

remm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/master by this push:
 new 1554030  Improve Connector creation
1554030 is described below

commit 15540305ab97b12156536d26e17895b750f12a38
Author: remm 
AuthorDate: Tue Apr 7 18:56:08 2020 +0200

Improve Connector creation

Remove unneeded reflection. More importantly remove AJP specific hack in
favor of a new default method in ProtocolHandler. No behavior change for
existing API.
---
 java/org/apache/catalina/connector/Connector.java  | 39 ++--
 java/org/apache/coyote/ProtocolHandler.java| 43 ++
 .../org/apache/coyote/ajp/AbstractAjpProtocol.java |  8 +++-
 webapps/docs/changelog.xml |  4 ++
 4 files changed, 74 insertions(+), 20 deletions(-)

diff --git a/java/org/apache/catalina/connector/Connector.java 
b/java/org/apache/catalina/connector/Connector.java
index 20118f7..ca7b909 100644
--- a/java/org/apache/catalina/connector/Connector.java
+++ b/java/org/apache/catalina/connector/Connector.java
@@ -35,7 +35,6 @@ import org.apache.coyote.AbstractProtocol;
 import org.apache.coyote.Adapter;
 import org.apache.coyote.ProtocolHandler;
 import org.apache.coyote.UpgradeProtocol;
-import org.apache.coyote.ajp.AbstractAjpProtocol;
 import org.apache.coyote.http11.AbstractHttp11JsseProtocol;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
@@ -64,40 +63,42 @@ public class Connector extends LifecycleMBeanBase  {
 
 //  Constructor
 
+
 /**
  * Defaults to using HTTP/1.1 NIO implementation.
  */
 public Connector() {
-this("org.apache.coyote.http11.Http11NioProtocol");
+this("HTTP/1.1");
 }
 
 
 public Connector(String protocol) {
-if ("HTTP/1.1".equals(protocol) || protocol == null) {
-protocolHandlerClassName = 
"org.apache.coyote.http11.Http11NioProtocol";
-} else if ("AJP/1.3".equals(protocol)) {
-protocolHandlerClassName = "org.apache.coyote.ajp.AjpNioProtocol";
-} else {
-protocolHandlerClassName = protocol;
-}
-
-// Instantiate protocol handler
 ProtocolHandler p = null;
 try {
-Class clazz = Class.forName(protocolHandlerClassName);
-p = (ProtocolHandler) clazz.getConstructor().newInstance();
+p = ProtocolHandler.create(protocol);
 } catch (Exception e) {
 log.error(sm.getString(
 "coyoteConnector.protocolHandlerInstantiationFailed"), e);
-} finally {
-this.protocolHandler = p;
 }
-
+if (p != null) {
+protocolHandler = p;
+protocolHandlerClassName = protocolHandler.getClass().getName();
+} else {
+protocolHandler = null;
+protocolHandlerClassName = protocol;
+}
 // Default for Connector depends on this system property
 
setThrowOnFailure(Boolean.getBoolean("org.apache.catalina.startup.EXIT_ON_INIT_FAILURE"));
 }
 
 
+public Connector(ProtocolHandler protocolHandler) {
+protocolHandlerClassName = protocolHandler.getClass().getName();
+this.protocolHandler = protocolHandler;
+// Default for Connector depends on this system property
+
setThrowOnFailure(Boolean.getBoolean("org.apache.catalina.startup.EXIT_ON_INIT_FAILURE"));
+}
+
 // - Instance Variables
 
 
@@ -946,9 +947,9 @@ public class Connector extends LifecycleMBeanBase  {
  * @return a new Servlet response object
  */
 public Response createResponse() {
-if (protocolHandler instanceof AbstractAjpProtocol) {
-int packetSize = ((AbstractAjpProtocol) 
protocolHandler).getPacketSize();
-return new Response(packetSize - 
org.apache.coyote.ajp.Constants.SEND_HEAD_LEN);
+int size = protocolHandler.getDesiredBufferSize();
+if (size > 0) {
+return new Response(size);
 } else {
 return new Response();
 }
diff --git a/java/org/apache/coyote/ProtocolHandler.java 
b/java/org/apache/coyote/ProtocolHandler.java
index b467558..57a3086 100644
--- a/java/org/apache/coyote/ProtocolHandler.java
+++ b/java/org/apache/coyote/ProtocolHandler.java
@@ -16,6 +16,7 @@
  */
 package org.apache.coyote;
 
+import java.lang.reflect.InvocationTargetException;
 import java.util.concurrent.Executor;
 import java.util.concurrent.ScheduledExecutorService;
 
@@ -179,4 +180,46 @@ public interface ProtocolHandler {
  * @return the protocols
  */
 public UpgradeProtocol[] findUpgradeProtocols();
+
+
+/**
+ * Some protocols, like AJP, have a packet length that
+

Re: API Change - Connector.java Constructor

[Rémy Maucherat]

On Tue, Apr 7, 2020 at 5:22 PM Filip Hanik  wrote:

> On Tue, Apr 7, 2020 at 8:15 AM Rémy Maucherat  wrote:
>
>> On Tue, Apr 7, 2020 at 10:16 AM Mark Thomas  wrote:
>>
>>> > Noted, I think a compromise may be in order. Where we simply add a
>>> > constructor that avoids the Class.forName
>>> > and that allows the developer to explicitly invoke a constructor that
>>> > avoids it.
>>>
>>> My thinking was more along the following lines...
>>>
>>> Rewrite the Connector (and possibly some/all other components) so that
>>> if a known class name was specified, rather than using the specified
>>> name directly via reflection we could do something like:
>>>
>>> if ("org.apache.coyote.http11.Http11NioProtocol".equals(className)) {
>>> protocol = new Http11NioProtocol();
>>> } else if ("org...
>>> ...
>>> } else {
>>> // OK. Unrecognised class name. Use reflection
>>> ...
>>> }
>>>
>>> Would that style of approach help at all?
>>>
>>
>> Proposed patch:
>> https://pastebin.com/ydCYnBD9
>>
>
> That patch works, I would include the check for the full classname too
>
> Was: if ("HTTP/1.1".equals(protocol) || protocol == null)
> Could be: if (protocol == null || "HTTP/1.1".equals(protocol) ||
> "HTTP/1.1".equals(Http11NioProtocol.class.getName))
>
> However, what still is awkward is that Connector.java deals with the
> interface ProtocolHandler.
> The loading of both the Http11NioConnector and the Ajp13NioConnector seems
> to defy the purpose of having that interface being present
>
> Is: protected final ProtocolHandler protocolHandler;
>
> Does the connector need to know about the actual implementations?
>

Ideally no, but it removes the reflection you say is bad for Graal.

Rémy


>
> Filip
>
>
>
>
>>
>> Rémy
>>
>>
>>>
>>> Mark
>>>
>>> -
>>> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
>>> For additional commands, e-mail: dev-h...@tomcat.apache.org
>>>
>>>

[GitHub] [tomcat] larsgrefer opened a new pull request #272: Loop can be terminated after condition is met

[GitBox]

larsgrefer opened a new pull request #272: Loop can be terminated after 
condition is met
URL: https://github.com/apache/tomcat/pull/272
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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

[GitHub] [tomcat] larsgrefer opened a new pull request #271: Use empty arrays for Collections.toArray()

[GitBox]

larsgrefer opened a new pull request #271: Use empty arrays for 
Collections.toArray()
URL: https://github.com/apache/tomcat/pull/271
 
 
   There are two styles to convert a collection to an array:
   either using a pre-sized array (like `c.toArray(new String[c.size()])`)
   or using an empty array (like `c.toArray(new String[0])`).
   
   In older Java versions using pre-sized array was recommended,
   as the reflection call which is necessary to create an array of proper size 
was quite slow.
   However since late updates of OpenJDK 6 this call was intrinsified,
   making the performance of the empty array version the same and sometimes 
even better,
   compared to the pre-sized version.
   
   See also: https://shipilev.net/blog/2016/arrays-wisdom-ancients/


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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

[GitHub] [tomcat] larsgrefer opened a new pull request #270: Bulk operation can be used instead of iteration

[GitBox]

larsgrefer opened a new pull request #270: Bulk operation can be used instead 
of iteration
URL: https://github.com/apache/tomcat/pull/270
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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

[GitHub] [tomcat] larsgrefer opened a new pull request #269: Use parameterized collection constructors where possible

[GitBox]

larsgrefer opened a new pull request #269: Use parameterized collection 
constructors where possible
URL: https://github.com/apache/tomcat/pull/269
 
 
   This allows collections like ArrayList and HashSet to initialize their 
backing arrays
   with the correct size.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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

Re: API Change - Connector.java Constructor

[Filip Hanik]

On Tue, Apr 7, 2020 at 8:15 AM Rémy Maucherat  wrote:

> On Tue, Apr 7, 2020 at 10:16 AM Mark Thomas  wrote:
>
>> > Noted, I think a compromise may be in order. Where we simply add a
>> > constructor that avoids the Class.forName
>> > and that allows the developer to explicitly invoke a constructor that
>> > avoids it.
>>
>> My thinking was more along the following lines...
>>
>> Rewrite the Connector (and possibly some/all other components) so that
>> if a known class name was specified, rather than using the specified
>> name directly via reflection we could do something like:
>>
>> if ("org.apache.coyote.http11.Http11NioProtocol".equals(className)) {
>> protocol = new Http11NioProtocol();
>> } else if ("org...
>> ...
>> } else {
>> // OK. Unrecognised class name. Use reflection
>> ...
>> }
>>
>> Would that style of approach help at all?
>>
>
> Proposed patch:
> https://pastebin.com/ydCYnBD9
>

That patch works, I would include the check for the full classname too

Was: if ("HTTP/1.1".equals(protocol) || protocol == null)
Could be: if (protocol == null || "HTTP/1.1".equals(protocol) ||
"HTTP/1.1".equals(Http11NioProtocol.class.getName))

However, what still is awkward is that Connector.java deals with the
interface ProtocolHandler.
The loading of both the Http11NioConnector and the Ajp13NioConnector seems
to defy the purpose of having that interface being present

Is: protected final ProtocolHandler protocolHandler;

Does the connector need to know about the actual implementations?

Filip




>
> Rémy
>
>
>>
>> Mark
>>
>> -
>> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
>> For additional commands, e-mail: dev-h...@tomcat.apache.org
>>
>>

Re: API Change - Connector.java Constructor

[Rémy Maucherat]

On Tue, Apr 7, 2020 at 10:16 AM Mark Thomas  wrote:

> > Noted, I think a compromise may be in order. Where we simply add a
> > constructor that avoids the Class.forName
> > and that allows the developer to explicitly invoke a constructor that
> > avoids it.
>
> My thinking was more along the following lines...
>
> Rewrite the Connector (and possibly some/all other components) so that
> if a known class name was specified, rather than using the specified
> name directly via reflection we could do something like:
>
> if ("org.apache.coyote.http11.Http11NioProtocol".equals(className)) {
> protocol = new Http11NioProtocol();
> } else if ("org...
> ...
> } else {
> // OK. Unrecognised class name. Use reflection
> ...
> }
>
> Would that style of approach help at all?
>

Proposed patch:
https://pastebin.com/ydCYnBD9

Rémy


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

[GitHub] [tomcat] biya-bi opened a new pull request #268: Improve Catalina class loader repositories regular expression

[GitBox]

biya-bi opened a new pull request #268: Improve Catalina class loader 
repositories regular expression
URL: https://github.com/apache/tomcat/pull/268
 
 
   The goal of this enhancement is to improve the regular expression used
   for searching class loader repositories when bootstrapping Catalina.
   
   With the Java regular expression engine which is regex-directed, we
   gain in performance by using the negated character class [^\"]* rather
   than the lazy quantifier .*? in the regular expression used for
   searching class loader repositories when bootstrapping Catalina.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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

[tomcat-jakartaee-migration] branch master updated: Avoid deprecation warning

[remm]

This is an automated email from the ASF dual-hosted git repository.

remm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat-jakartaee-migration.git


The following commit(s) were added to refs/heads/master by this push:
 new 54c1fe1  Avoid deprecation warning
54c1fe1 is described below

commit 54c1fe1cc2da889c45412312a103e809b52e0664
Author: remm 
AuthorDate: Tue Apr 7 15:20:12 2020 +0200

Avoid deprecation warning
---
 src/test/java/org/apache/tomcat/jakartaee/MigrationTest.java | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/test/java/org/apache/tomcat/jakartaee/MigrationTest.java 
b/src/test/java/org/apache/tomcat/jakartaee/MigrationTest.java
index 8ed7f81..c41a735 100644
--- a/src/test/java/org/apache/tomcat/jakartaee/MigrationTest.java
+++ b/src/test/java/org/apache/tomcat/jakartaee/MigrationTest.java
@@ -18,6 +18,7 @@
 package org.apache.tomcat.jakartaee;
 
 import java.io.File;
+import java.nio.charset.StandardCharsets;
 
 import org.apache.commons.io.FileUtils;
 import org.junit.After;
@@ -45,7 +46,7 @@ public class MigrationTest {
 
 assertTrue("Migrated file not found", migratedFile.exists());
 
-String migratedSource = FileUtils.readFileToString(migratedFile);
+String migratedSource = FileUtils.readFileToString(migratedFile, 
StandardCharsets.UTF_8);
 assertFalse("Imports not migrated", migratedSource.contains("import 
javax.servlet"));
 assertTrue("Migrated imports not found", 
migratedSource.contains("import jakarta.servlet"));
 }
@@ -57,7 +58,7 @@ public class MigrationTest {
 
 assertTrue("Migrated file not found", migratedFile.exists());
 
-String migratedSource = FileUtils.readFileToString(migratedFile);
+String migratedSource = FileUtils.readFileToString(migratedFile, 
StandardCharsets.UTF_8);
 assertFalse("Imports not migrated", migratedSource.contains("import 
javax.servlet"));
 assertTrue("Migrated imports not found", 
migratedSource.contains("import jakarta.servlet"));
 }
@@ -72,7 +73,7 @@ public class MigrationTest {
 
 assertTrue("Migrated file not found", migratedFile.exists());
 
-String migratedSource = FileUtils.readFileToString(migratedFile);
+String migratedSource = FileUtils.readFileToString(migratedFile, 
StandardCharsets.UTF_8);
 assertFalse("Imports not migrated", migratedSource.contains("import 
javax.servlet"));
 assertTrue("Migrated imports not found", 
migratedSource.contains("import jakarta.servlet"));
 }


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

Re: [Bug 64309] Improve repository regular expression performance used for bootstrapping Catalina

[Paul Muriel Biya-Bi]

Yes, I also think the same. I will push the pull request for the enhancement.

Paul

> On Apr 7, 2020, at 8:48 AM, Mark Thomas  wrote:
> 
> On 07/04/2020 13:39, Paul Muriel Biya-Bi wrote:
>> Hi Mark,
>> 
>> In our case, the performance difference is indeed very small. On my Ubuntu 
>> system, I did a basic performance measurement (using 
>> System.currentTimeMillis()) on my common.loader path. With the 
>> Bootstrap.getPaths() method, I gained only 2 illiseconds on a repository 
>> path of 340 characters. The performance difference would have been more 
>> important if the repository path was a longer.
>> 
>> So should we forget about this enhancement?
> 
> Up to you. I've no objection to it. A couple of milliseconds might
> matter to some users.
> 
> Mark
> 
> 
>> 
>> Thanks,
>> Paul
>> 
 On Apr 6, 2020, at 10:46 AM, bugzi...@apache.org wrote:
>>> 
>>> https://bz.apache.org/bugzilla/show_bug.cgi?id=64309
>>> 
>>> --- Comment #1 from Mark Thomas  ---
>>> No objections but I do wonder if the performance difference is noticeable or
>>> even measurable given that this happens once on start-up.
>>> 
>>> -- 
>>> You are receiving this mail because:
>>> You are the assignee for the bug.
>>> -
>>> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
>>> For additional commands, e-mail: dev-h...@tomcat.apache.org
>> 
>> -
>> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
>> For additional commands, e-mail: dev-h...@tomcat.apache.org
>> 
> 
> 
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
> 

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

Re: [Bug 64309] Improve repository regular expression performance used for bootstrapping Catalina

[Mark Thomas]

On 07/04/2020 13:39, Paul Muriel Biya-Bi wrote:
> Hi Mark,
> 
> In our case, the performance difference is indeed very small. On my Ubuntu 
> system, I did a basic performance measurement (using 
> System.currentTimeMillis()) on my common.loader path. With the 
> Bootstrap.getPaths() method, I gained only 2 illiseconds on a repository path 
> of 340 characters. The performance difference would have been more important 
> if the repository path was a longer.
> 
> So should we forget about this enhancement?

Up to you. I've no objection to it. A couple of milliseconds might
matter to some users.

Mark


> 
> Thanks,
> Paul
> 
>> On Apr 6, 2020, at 10:46 AM, bugzi...@apache.org wrote:
>>
>> https://bz.apache.org/bugzilla/show_bug.cgi?id=64309
>>
>> --- Comment #1 from Mark Thomas  ---
>> No objections but I do wonder if the performance difference is noticeable or
>> even measurable given that this happens once on start-up.
>>
>> -- 
>> You are receiving this mail because:
>> You are the assignee for the bug.
>> -
>> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
>> For additional commands, e-mail: dev-h...@tomcat.apache.org
> 
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
> 


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

Re: [Bug 64309] Improve repository regular expression performance used for bootstrapping Catalina

[Paul Muriel Biya-Bi]

Hi Mark,

In our case, the performance difference is indeed very small. On my Ubuntu 
system, I did a basic performance measurement (using 
System.currentTimeMillis()) on my common.loader path. With the 
Bootstrap.getPaths() method, I gained only 2 illiseconds on a repository path 
of 340 characters. The performance difference would have been more important if 
the repository path was a longer.

So should we forget about this enhancement?

Thanks,
Paul

> On Apr 6, 2020, at 10:46 AM, bugzi...@apache.org wrote:
> 
> https://bz.apache.org/bugzilla/show_bug.cgi?id=64309
> 
> --- Comment #1 from Mark Thomas  ---
> No objections but I do wonder if the performance difference is noticeable or
> even measurable given that this happens once on start-up.
> 
> -- 
> You are receiving this mail because:
> You are the assignee for the bug.
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org

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

Re: Jakarta EE migration tool improvements

[Emmanuel Bourg]

Le 07/04/2020 à 13:50, Rémy Maucherat a écrit :

> I would want to keep the basic tool available.

Of course, this is in addition to the command line tool.


> Ideally it is nice to avoid having too many options.

I hardly imagine more than 10 options for this kind of tool.

Emmanuel Bourg

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

Re: API Change - Connector.java Constructor

[Rémy Maucherat]

On Tue, Apr 7, 2020 at 10:16 AM Mark Thomas  wrote:

> My thinking was more along the following lines...
>
> Rewrite the Connector (and possibly some/all other components) so that
>

If we're talking about embedded, then the other components don't have the
problem. For example to create a context you can do new Context and add it
to a Host, there's no reflection. That's my basic understanding at least.
Personally, I dislike true embedded in favor of using server.xml and in
that case it's all reflection. Although the binary is "large" I haven't
noticed this was a problem.


> if a known class name was specified, rather than using the specified
> name directly via reflection we could do something like:
>
> if ("org.apache.coyote.http11.Http11NioProtocol".equals(className)) {
> protocol = new Http11NioProtocol();
> } else if ("org...
> ...
> } else {
> // OK. Unrecognised class name. Use reflection
> ...
> }
>
> Would that style of approach help at all?
>

I like it, so I'll work on that. I'll also clear up the AJP hack.

Rémy


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

Re: Jakarta EE migration tool improvements

[Rémy Maucherat]

On Tue, Apr 7, 2020 at 11:54 AM Emmanuel Bourg  wrote:

> Hi,
>
> So I've started working on the Jakarta EE migration tool. I've some
> enhancements in mind but I'd like to ensure there is a consensus before
> proceeding.
>
> Here are my suggestions:
>
> - Turn the tool into a plugin for the main build systems (Maven, Gradle,
> Ant) to easily post process the jar/war files generated by a project.
> I've some experience on this side with my jsign project
> (https://github.com/ebourg/jsign) and I suggest adopting a similar multi
> module structure with separate artifacts.
>

I would want to keep the basic tool available.


>
> - Use a CLI parsing library (picocli, jcommander or commons-cli) to ease
> the addition of command line options. It may also bring help/manpage
> generation, tab completion, colored messages.
>

Ideally it is nice to avoid having too many options.


>
> - Parallel processing of directories, and maybe of archives too but
> that's trickier.
>
> - Support in-place migration of directories. I've implemented it for
> single files but it looks a bit dangerous for whole directories. I think
> it'd be safer to ask the user to confirm first.
>
> - In memory buffering. The converters read and write directly to the
> disk, writing first to memory and then to the disk speeds up the
> migration (on my system it halved the migration time of the Tomcat
> source tree).
>

Often the memory requirements become hard to control and or predict.

Rémy


>
> What do you think?
>
> Emmanuel Bourg
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>

Re: Jakarta EE migration tool improvements

[Emmanuel Bourg]

Le 07/04/2020 à 12:41, Mark Thomas a écrit :

>> - Use a CLI parsing library (picocli, jcommander or commons-cli) to ease
>> the addition of command line options. It may also bring help/manpage
>> generation, tab completion, colored messages.

> Are we at the point where we need one?

The lack of proper command line parsing will quickly become a deterrent
to the addition of new features. I hesitated to add the -verbose option
due to this. We'll probably need a -force flag to confirm inplace
migration, a -quiet flag wouldn't be unusual. The main method will
quickly turn into spaghetti code without a library.


> I'd opt for Commons CLI if I had to choose but I'm not really familiar 
> with any of the options to judge relative pros/cons.

Having maintained Commons CLI some time ago I know it well, it's quite
compact but it shows its age now compared to more recent alternatives.
JCommander is annotation based and more concise. Picocli is the most
advanced I think, it supports help usage and manpage generation, tab
completion and ANSI coloring. But it's a 350K dependency, so harder to
sell I guess.


>> - Support in-place migration of directories. I've implemented it for
>> single files but it looks a bit dangerous for whole directories. I think
>> it'd be safer to ask the user to confirm first.
> 
> I'd be happy to allow this for a build system without confirmation. I'm
> not sure I'd want to allow it at all outside of that (including single
> files). The opportunity for shooting yourself in the foot if the
> migration fails is significant. I'd rather the tool forced a safe
> approach of writing the migrated content to a new location.

What about safe by default, and a command line option to allow it?


>> - In memory buffering. The converters read and write directly to the
>> disk, writing first to memory and then to the disk speeds up the
>> migration (on my system it halved the migration time of the Tomcat
>> source tree).
> 
> I'm wondering about buffer size vs performance benefit. Is there a point
> where a larger buffer doesn't help much more? I'm thinking about memory
> constrained systems and whether there is a sensible fixed buffer size we
> can use, or whether the buffer size should be configurable and if so,
> with what default?

I'm thinking about a mixed approach, with buffering for small files
(let's say under 1MB) and direct write for the others, so the heap usage
remains moderate.

Emmmanuel Bourg

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

[tomcat-jakartaee-migration] branch master updated: Fix an IDE nag

[markt]

This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat-jakartaee-migration.git


The following commit(s) were added to refs/heads/master by this push:
 new 8c95d25  Fix an IDE nag
8c95d25 is described below

commit 8c95d252e724948e5e02a4f7b818d4ce1e2e6c4b
Author: Mark Thomas 
AuthorDate: Tue Apr 7 12:11:09 2020 +0100

Fix an IDE nag
---
 src/main/java/org/apache/tomcat/jakartaee/EESpecProfile.java | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/main/java/org/apache/tomcat/jakartaee/EESpecProfile.java 
b/src/main/java/org/apache/tomcat/jakartaee/EESpecProfile.java
index 45aed81..1eb6a41 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/EESpecProfile.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/EESpecProfile.java
@@ -37,7 +37,7 @@ public enum EESpecProfile {
 }
 
 /**
- * Return the replacement pattern for this profile.
+ * @return the replacement pattern for this profile.
  */
 public Pattern getPattern() {
 return pattern;


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

[tomcat-jakartaee-migration] branch master updated: Really use the charset parameter in the Util.toString(InputStream, Charset) method

[ebourg]

This is an automated email from the ASF dual-hosted git repository.

ebourg pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat-jakartaee-migration.git


The following commit(s) were added to refs/heads/master by this push:
 new d3c805d  Really use the charset parameter in the 
Util.toString(InputStream, Charset) method
d3c805d is described below

commit d3c805df5d38e60420f7b51b1f02d1de53cf2d94
Author: Emmanuel Bourg 
AuthorDate: Tue Apr 7 12:48:39 2020 +0200

Really use the charset parameter in the Util.toString(InputStream, Charset) 
method
---
 src/main/java/org/apache/tomcat/jakartaee/Util.java | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/main/java/org/apache/tomcat/jakartaee/Util.java 
b/src/main/java/org/apache/tomcat/jakartaee/Util.java
index 7189d8a..1fbca92 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/Util.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/Util.java
@@ -21,7 +21,6 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.nio.charset.Charset;
-import java.nio.charset.StandardCharsets;
 import java.util.Locale;
 
 public class Util {
@@ -54,7 +53,7 @@ public class Util {
 ByteArrayOutputStream baos = new ByteArrayOutputStream();
 Util.copy(is, baos);
 
-return new String(baos.toByteArray(), StandardCharsets.ISO_8859_1);
+return new String(baos.toByteArray(), charset);
 }
 
 private Util() {


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

Re: [tomcat-jakartaee-migration] branch master updated (397ff8a -> c4e5c18)

[Emmanuel Bourg]

Le 07/04/2020 à 12:24, Mark Thomas a écrit :

> +1 to all of the above.

Thank you for the review.


>>  new 3618367  Renamed NoOpConverter to PassThroughConverter
> 
> What problem does this commit address and/or what new feature does it
> add? It looks more like a personal preference for a different name to me.

I've found "NoOp" to be confusing, at first glance based on the name I
thought it was an empty implementation of the Converter interface, and I
was wrong since it actually copies the source unmodified. I think the
term "pass-through" better describes the behavior of the converter.


> I think there is less likelihood of conflict if there is a technical
> justification for a change, and, if the justification is not / might not
> be obvious then mention it in the commit message.

Ok, I tend to write concise commit messages but I'll try to elaborate in
this kind of situation.


> This tests creates a file but doesn't remove it afterwards. Tests that
> create files should remove those files on completion. It might be worth
> considering creating such files in a temporary location rather than in
> the source tree.

I agree this can be improved.


> Generally, the code was clean (no IDE warnings) and the aim is to keep
> it this way as it enables errors to be spotted more easily. This is no
> longer the case after the above changes. The unused charset parameter is
> flagged as a warning.

Sorry, for some reason my IDE was misconfigured and no longer
highlighted unused code. I'll fix that.

Emmanuel Bourg

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

Re: Jakarta EE migration tool improvements

[Mark Thomas]

On 07/04/2020 10:54, Emmanuel Bourg wrote:
> Hi,
> 
> So I've started working on the Jakarta EE migration tool. I've some
> enhancements in mind but I'd like to ensure there is a consensus before
> proceeding.
> 
> Here are my suggestions:
> 
> - Turn the tool into a plugin for the main build systems (Maven, Gradle,
> Ant) to easily post process the jar/war files generated by a project.
> I've some experience on this side with my jsign project
> (https://github.com/ebourg/jsign) and I suggest adopting a similar multi
> module structure with separate artifacts.

Build time solutions (or at least pre-deployment solutions) strike me as
a better option than converting at deploy time (meaning deployment takes
longer) or on-the-fly conversion (which just makes me nervous) so +1 to
this approach.

> - Use a CLI parsing library (picocli, jcommander or commons-cli) to ease
> the addition of command line options. It may also bring help/manpage
> generation, tab completion, colored messages.

Are we at the point where we need one? I'd opt for Commons CLI if I had
to choose but I'm not really familiar with any of the options to judge
relative pros/cons.

> - Parallel processing of directories, and maybe of archives too but
> that's trickier.

For speed of conversion I assume. There is always the "unpack, convert,
repack" option for archives as a quick(ish) and dirty solution. Parallel
processing of archives looks like a fun problem. First impressions are
it would involve a lot of in-memory caching.

> - Support in-place migration of directories. I've implemented it for
> single files but it looks a bit dangerous for whole directories. I think
> it'd be safer to ask the user to confirm first.

I'd be happy to allow this for a build system without confirmation. I'm
not sure I'd want to allow it at all outside of that (including single
files). The opportunity for shooting yourself in the foot if the
migration fails is significant. I'd rather the tool forced a safe
approach of writing the migrated content to a new location.

> - In memory buffering. The converters read and write directly to the
> disk, writing first to memory and then to the disk speeds up the
> migration (on my system it halved the migration time of the Tomcat
> source tree).

I'm wondering about buffer size vs performance benefit. Is there a point
where a larger buffer doesn't help much more? I'm thinking about memory
constrained systems and whether there is a sensible fixed buffer size we
can use, or whether the buffer size should be configurable and if so,
with what default?

Mark

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

Re: [tomcat-jakartaee-migration] branch master updated (397ff8a -> c4e5c18)

[Mark Thomas]

On 07/04/2020 01:56, ebo...@apache.org wrote:
> This is an automated email from the ASF dual-hosted git repository.
> 
> ebourg pushed a change to branch master
> in repository 
> https://gitbox.apache.org/repos/asf/tomcat-jakartaee-migration.git.
> 
> 
> from 397ff8a  Travis integration
>  new c8afe9b  One line log messages when running from the command line
>  new 07b9ad1  Test with an alternative profile
>  new 703262f  Moved EESpecProfile to the top level
>  new e58d4c5  Renamed getEESpecLevel() to getEESpecProfile()
>  new 39ada70  Turned the migration profile into an instance field of the 
> Migration class
>  new ec70314  Added a -verbose option

+1 to all of the above.

>  new 3618367  Renamed NoOpConverter to PassThroughConverter

What problem does this commit address and/or what new feature does it
add? It looks more like a personal preference for a different name to me.

My main concern here is if two (or more) committers feel strongly about
an issue of personal preference the situation can escalate.

I think there is less likelihood of conflict if there is a technical
justification for a change, and, if the justification is not / might not
be obvious then mention it in the commit message.

>  new f69a7d9  Support inplace file migration

This tests creates a file but doesn't remove it afterwards. Tests that
create files should remove those files on completion. It might be worth
considering creating such files in a temporary location rather than in
the source tree.

>  new 884aa46  Moved common stream operation methods to the Util class

The charset parameter on Util.toString() appears to be unused.

>  new c4e5c18  Test invalid options

Generally, the code was clean (no IDE warnings) and the aim is to keep
it this way as it enables errors to be spotted more easily. This is no
longer the case after the above changes. The unused charset parameter is
flagged as a warning.

Mark

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

Re: Jakarta EE migration tool release

[Mark Thomas]

On 07/04/2020 10:15, Emmanuel Bourg wrote:
> Hi all,
> 
> I'd like to start packaging the Jakarta EE migration tool for Debian
> since it's now a prerequisite to build Tomcat 10. For that I'd need a
> recent tag, do you think we could tag the current code as 0.0.2? Do we
> need a formal vote at this stage?

Using it outside of Tomcat implies a release so I think a vote would be
the right thing to do even if the release doesn't progress beyond a tag.

You could argue that ASF processes don't require a vote in this instance
but I'd rather stick to the spirit of the process rather than use a
strict reading to avoid doing something.

Mark

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

Jakarta EE migration tool improvements

[Emmanuel Bourg]

Hi,

So I've started working on the Jakarta EE migration tool. I've some
enhancements in mind but I'd like to ensure there is a consensus before
proceeding.

Here are my suggestions:

- Turn the tool into a plugin for the main build systems (Maven, Gradle,
Ant) to easily post process the jar/war files generated by a project.
I've some experience on this side with my jsign project
(https://github.com/ebourg/jsign) and I suggest adopting a similar multi
module structure with separate artifacts.

- Use a CLI parsing library (picocli, jcommander or commons-cli) to ease
the addition of command line options. It may also bring help/manpage
generation, tab completion, colored messages.

- Parallel processing of directories, and maybe of archives too but
that's trickier.

- Support in-place migration of directories. I've implemented it for
single files but it looks a bit dangerous for whole directories. I think
it'd be safer to ask the user to confirm first.

- In memory buffering. The converters read and write directly to the
disk, writing first to memory and then to the disk speeds up the
migration (on my system it halved the migration time of the Tomcat
source tree).

What do you think?

Emmanuel Bourg

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

[tomcat-jakartaee-migration] branch master updated: Implicit phase binding for the shade plugin

[ebourg]

This is an automated email from the ASF dual-hosted git repository.

ebourg pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat-jakartaee-migration.git


The following commit(s) were added to refs/heads/master by this push:
 new c9bec12  Implicit phase binding for the shade plugin
c9bec12 is described below

commit c9bec123381df2e9ee1829a1a4f6cad5b21e91cb
Author: Emmanuel Bourg 
AuthorDate: Mon Apr 6 22:00:58 2020 +0200

Implicit phase binding for the shade plugin
---
 pom.xml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/pom.xml b/pom.xml
index adc57b4..2d4ddd8 100644
--- a/pom.xml
+++ b/pom.xml
@@ -121,7 +121,6 @@
 maven-shade-plugin
 
   
-package
 
   shade
 


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

Jakarta EE migration tool release

[Emmanuel Bourg]

Hi all,

I'd like to start packaging the Jakarta EE migration tool for Debian
since it's now a prerequisite to build Tomcat 10. For that I'd need a
recent tag, do you think we could tag the current code as 0.0.2? Do we
need a formal vote at this stage?

Emmanuel Bourg

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

Re: [tomcat-jakartaee-migration] 01/03: Replaced NonClosing{In, Out}putStream with the equivalent classes from Commons IO

[Rémy Maucherat]

On Tue, Apr 7, 2020 at 11:05 AM Emmanuel Bourg  wrote:

> Le 07/04/2020 à 09:41, Rémy Maucherat a écrit :
>
> > Reimplemeting ... It was already done, and this was a better trivial
> > solution. This is a veto (unlike Mark I will not withdraw it), so please
> > revert this commit.
>
> Remy, this is honestly more a rant on an insignificant detail than a
> reasonably justified veto. It doesn't affect the correctness, the
> performance or the maintainability of the tool. I'm tempted to translate
> this as "Don't touch my project" and an incitation to go work on
> something else.
>
> The dependency graph of this tool is very likely to grow as its features
> expand (think about adding a CLI parser, providing the tool as a
> Maven/Gradle plugin or an Ant task), and I wouldn't be surprised to see
> Commons IO brought to the classpath sooner or later anyway (or the
> overhead diluted).
>

This is exactly the reason why we don't use Maven in Tomcat, dependencies
creep up because it is "normal".

Rémy

Re: [tomcat-jakartaee-migration] 01/03: Replaced NonClosing{In, Out}putStream with the equivalent classes from Commons IO

[Emmanuel Bourg]

Le 07/04/2020 à 09:41, Rémy Maucherat a écrit :

> Reimplemeting ... It was already done, and this was a better trivial
> solution. This is a veto (unlike Mark I will not withdraw it), so please
> revert this commit.

Remy, this is honestly more a rant on an insignificant detail than a
reasonably justified veto. It doesn't affect the correctness, the
performance or the maintainability of the tool. I'm tempted to translate
this as "Don't touch my project" and an incitation to go work on
something else.

The dependency graph of this tool is very likely to grow as its features
expand (think about adding a CLI parser, providing the tool as a
Maven/Gradle plugin or an Ant task), and I wouldn't be surprised to see
Commons IO brought to the classpath sooner or later anyway (or the
overhead diluted).

Emmanuel Bourg

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

Re: [tomcat-jakartaee-migration] branch master updated (5c96c0b -> 61ba095)

[Mark Thomas]

On 07/04/2020 09:40, Emmanuel Bourg wrote:
> Le 07/04/2020 à 10:12, Mark Thomas a écrit :
> 
>> The dependencies are only embedded to create a single JAR to make it
>> easier for users to use on the command line. If the code was re-used I'd
>> expect the "standard" JAR to be used and leave the decision on how to
>> handle the dependencies up to the integrator.
> 
> Ok, I assumed the shaded jar was the main artifact. As long as it isn't
> published to Maven Central I'm fine with removing the relocation.

Working out how this could be formally released is still on the TODO list.

I don't view the shaded JAR as the main artefact but I can see it being
pushed to Maven Central at some point. If that does happen it would be
along side the "standard" JAR which would have a POM with the correct
dependencies.

Mark

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

Re: [tomcat-jakartaee-migration] branch master updated (5c96c0b -> 61ba095)

[Emmanuel Bourg]

Le 07/04/2020 à 10:12, Mark Thomas a écrit :

> The dependencies are only embedded to create a single JAR to make it
> easier for users to use on the command line. If the code was re-used I'd
> expect the "standard" JAR to be used and leave the decision on how to
> handle the dependencies up to the integrator.

Ok, I assumed the shaded jar was the main artifact. As long as it isn't
published to Maven Central I'm fine with removing the relocation.

Emmanuel Bourg

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

[tomcat-jakartaee-migration] branch master updated: Revert "Relocated the dependencies in the shaded jar"

[ebourg]

This is an automated email from the ASF dual-hosted git repository.

ebourg pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat-jakartaee-migration.git


The following commit(s) were added to refs/heads/master by this push:
 new 7106838  Revert "Relocated the dependencies in the shaded jar"
7106838 is described below

commit 710683849be32afd4d5f16466e59976b46ca39ca
Author: Emmanuel Bourg 
AuthorDate: Tue Apr 7 10:36:07 2020 +0200

Revert "Relocated the dependencies in the shaded jar"

This reverts commit 61ba09577afc9fecf542d19c5cad6e79f1008ebd.
---
 pom.xml | 10 --
 1 file changed, 10 deletions(-)

diff --git a/pom.xml b/pom.xml
index 3da57ee..adc57b4 100644
--- a/pom.xml
+++ b/pom.xml
@@ -143,16 +143,6 @@
   
 
   
-  
-
-  org.apache.bcel
-  
org.apache.tomcat.jakartaee.bcel
-
-
-  org.apache.commons
-  
org.apache.tomcat.jakartaee.commons
-
-  
 
   
 


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

Re: sources repository for taglibs-standard-impl/specs-1.2.5-migrated-0.0.1

[Mark Thomas]

On 07/04/2020 00:44, Olivier Lamy wrote:
> Hi
> This weekend I have started running this on a server
> 
> svn2git https://svn.apache.org/repos/asf/tomcat/taglibs/standard/
> 
> It may take few days to finish...
> 
> Working with a svndump might be easier. Do you know if INFRA can provide
> this?

http://svn-dump.apache.org/

Before you go that route, let me explore doing a dump locally on one of
the svn servers.

Mark


> 
> 
> On Sat, 28 Mar 2020 at 20:58, Olivier Lamy  > wrote:
> 
> 
> 
> On Fri, 27 Mar 2020 at 20:01, Mark Thomas  > wrote:
> 
> On 27/03/2020 04:58, Olivier Lamy wrote:
> > Hi
> > We have to move
> this https://svn.apache.org/repos/asf/tomcat/taglibs/ 
> > to git.
> > As it's few separate Maven projects, maybe we could create one
> git repo
> > per project: 
> > - https://github.com/apache/tomcat-taglibs-site
> > - https://github.com/apache/tomcat-taglibs-standard
> (standard-examples
> > can be integrated in standard source tree but not part of the
> release
> > just here for documentation purpose)
> > - https://github.com/apache/tomcat-taglibs-parent
> >
> > - https://github.com/apache/tomcat-taglibs can contain some
> modules to
> > ease clone of only git repo (or maybe we simply put the site
> here) I
> > don't have strong opinion for this
> >
> > Do we still need rdc?
> 
> The above seems reasonable to me but I have never worked on
> taglibs so
> my opinion shouldn't count for much.
> 
> > during the migration each tags/branches (if any) will be
> integrated in
> > the created git repo
> 
> This is the bit that interests me. I know from experience with
> previous
> Tomcat projects that the svn -> git migration for a project that has
> previously gone through an cvs -> svn migration is very slow. It
> took
> over a week to process the Tomcat Connectors project. This is
> because
> the migration tool ends up searching back through the history
> repeatedly
> for each tag.
> 
> I might be able to help put by running the migration directly on
> one of
> the ASF servers. I'll need to talk to infra if we want to try
> that route.
> 
> 
> Great. Let me know if you need any help?
>  
> 
> 
> Mark
> 
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> 
> For additional commands, e-mail: dev-h...@tomcat.apache.org
> 
> 
> 
> 
> -- 
> Olivier Lamy
> http://twitter.com/olamy | http://linkedin.com/in/olamy
> 
> 
> 
> -- 
> Olivier Lamy
> http://twitter.com/olamy | http://linkedin.com/in/olamy


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

Re: API Change - Connector.java Constructor

[Mark Thomas]

On 06/04/2020 20:50, Filip Hanik wrote:
> On Mon, Apr 6, 2020 at 11:20 AM Mark Thomas  > wrote:
> 
> On 06/04/2020 17:56, Filip Hanik wrote:
> > Team,
> >
> > As I'm slowly transitioning between projects, Apache Tomcat has once
> > again showed up in my workspace. I'm currently working on
> improving the
> > embedded experience for native images.
> >
> > I have a pull request,[1] ,
> > that I'd like to open up a discussion about
> >
> > Goal: Able to start embedded Apache Tomcat without using reflection.
> 
> Do you have a test case you are working to? It would be helpful to see
> exactly where the problems are in terms of what is considered reflection
> and what isn't.
> 
> 
> Yes, I'm working on a sample that should demonstrate it.

Excellent.

> On that point, can we tell the tooling that reflection isn't used even
> if the automated analysis can't tell that? I'm thinking of something
> along the lines of ensuring that most execution paths - including
> embedded - avoid reflection. We should, hopefully, be able to document
> the scenarios where reflection is triggered so users know to avoid them.
> 
> > This PR: Changes the constructor for Connector to not use reflection.
> > Instead the calling components will instantiate the ProtocolHandler
> >
> > Besides the constructor change, functionality should remain backwards
> > compatible.
> 
> I am very uncomfortable changing the constructor for a fundamental
> component like Connector this far into a stable release - hence wanting
> to explore options.
> 
> 
> Noted, I think a compromise may be in order. Where we simply add a
> constructor that avoids the Class.forName
> and that allows the developer to explicitly invoke a constructor that
> avoids it.

My thinking was more along the following lines...

Rewrite the Connector (and possibly some/all other components) so that
if a known class name was specified, rather than using the specified
name directly via reflection we could do something like:

if ("org.apache.coyote.http11.Http11NioProtocol".equals(className)) {
protocol = new Http11NioProtocol();
} else if ("org...
...
} else {
// OK. Unrecognised class name. Use reflection
...
}

Would that style of approach help at all?

Mark

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

Re: [tomcat-jakartaee-migration] branch master updated (5c96c0b -> 61ba095)

[Mark Thomas]

On 06/04/2020 17:45, Emmanuel Bourg wrote:
> Le 06/04/2020 à 18:33, Mark Thomas a écrit :
> 
>> OK. But that is still 7.2k of classes rather than 2.4k of classes for
>> zero benefit.
>>
>> I'll withdraw my -1 because we are approaching the point where the
>> differences aren't worth the time spent discussing them but I still
>> don't like this change.
> 
> I've removed the duplicated Apache license in the shaded jar, so the
> difference is now near zero.
> 
> I agree this change is trivial and doesn't bring significant changes.
> Apache Commons was created to share and reuse common code used at the
> ASF, I always feel a bit sad when the code produced there isn't reused.
> 
> 
>> And the relocation of the dependencies? At the moment, I only see a
>> downside (harder debugging). What is the benefit?
> 
> Relocating embedded dependencies is a best practice to prevent classpath
> conflicts. If the tools ends up being integrated in a bigger software it
> could clash with another version of BCEL on the classpath.

The dependencies are only embedded to create a single JAR to make it
easier for users to use on the command line. If the code was re-used I'd
expect the "standard" JAR to be used and leave the decision on how to
handle the dependencies up to the integrator.

Mark

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

[tomcat-jakartaee-migration] branch master updated: Remove broken variable

[remm]

This is an automated email from the ASF dual-hosted git repository.

remm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat-jakartaee-migration.git


The following commit(s) were added to refs/heads/master by this push:
 new 0a021b3  Remove broken variable
0a021b3 is described below

commit 0a021b35841443d8431bb8876aff5c910c4aee55
Author: remm 
AuthorDate: Tue Apr 7 09:58:21 2020 +0200

Remove broken variable
---
 src/main/java/org/apache/tomcat/jakartaee/Migration.java | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/main/java/org/apache/tomcat/jakartaee/Migration.java 
b/src/main/java/org/apache/tomcat/jakartaee/Migration.java
index 4ba79bd..68a8fc2 100644
--- a/src/main/java/org/apache/tomcat/jakartaee/Migration.java
+++ b/src/main/java/org/apache/tomcat/jakartaee/Migration.java
@@ -154,8 +154,6 @@ public class Migration {
 result = migrateStream(src.getName(), is, os);
 }
 } else {
-File tmp = new File(dest.getParentFile(), dest.getName() + ".tmp");
-
 ByteArrayOutputStream buffer = new ByteArrayOutputStream((int) 
(src.length() * 1.05));
 
 try (InputStream is = new FileInputStream(src)) {


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

Re: [tomcat-jakartaee-migration] 01/03: Replaced NonClosing{In, Out}putStream with the equivalent classes from Commons IO

[Rémy Maucherat]

On Mon, Apr 6, 2020 at 3:45 PM Emmanuel Bourg  wrote:

> Le 06/04/2020 à 15:38, Rémy Maucherat a écrit :
>
> > commit 29ea1891489060f3b3e40259098def5ae47645ef
> > Author: Emmanuel Bourg mailto:ebo...@apache.org
> >>
> > AuthorDate: Mon Apr 6 15:24:35 2020 +0200
> >
> > Replaced NonClosing{In,Out}putStream with the equivalent classes
> > from Commons IO
> >
> >
> > Ah, the beauty of having Maven. Actual benefit: zero. Trouble with
> > updating dependencies: one. Possible incompatibilities: one.
> > So I think it's a total of -2 for the commit.
>
> Sorry but I'm not sure to understand your objection. Commons IO is dead
> stable, these classes have been around for over 10 years and haven't
> changed much since. There is really no point reimplementing them.
>

Reimplemeting ... It was already done, and this was a better trivial
solution. This is a veto (unlike Mark I will not withdraw it), so please
revert this commit.

Rémy