[tomcat] branch master updated: Allow casual runtime use of the migration tool

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[tomcat] branch master updated: Allow casual runtime use of the migration tool

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 abd1dea  Allow casual runtime use of the migration tool
abd1dea is described below

commit abd1dead7804e99e3215e8e01a6cf7448a6b9f36
Author: remm <[hidden email]>
AuthorDate: Tue Feb 16 17:06:27 2021 +0100

    Allow casual runtime use of the migration tool
   
    This allows specifying a profile which will be used for a
    ClassFileTransformer. Nothing much to report, it does basic things but
    does not do classloader resources or static files.
---
 .../apache/catalina/loader/LocalStrings.properties |  1 +
 java/org/apache/catalina/loader/WebappLoader.java  | 57 ++++++++++++++++++++++
 webapps/docs/changelog.xml                         |  7 +++
 webapps/docs/config/loader.xml                     | 11 +++++
 4 files changed, 76 insertions(+)

diff --git a/java/org/apache/catalina/loader/LocalStrings.properties b/java/org/apache/catalina/loader/LocalStrings.properties
index 7c6c976..dee7e2c 100644
--- a/java/org/apache/catalina/loader/LocalStrings.properties
+++ b/java/org/apache/catalina/loader/LocalStrings.properties
@@ -59,6 +59,7 @@ webappClassLoader.wrongVersion=(unable to load class [{0}])
 webappClassLoaderParallel.registrationFailed=Registration of org.apache.catalina.loader.ParallelWebappClassLoader as capable of loading classes in parallel failed
 
 webappLoader.deploy=Deploying class repositories to work directory [{0}]
+webappLoader.noJakartaConverter=The Jakarta converter provided by the Tomcat migration tool could not be loaded
 webappLoader.noResources=No resources found for context [{0}]
 webappLoader.reloadable=Cannot set reloadable property to [{0}]
 webappLoader.setContext.ise=Setting the Context is not permitted while the loader is started.
diff --git a/java/org/apache/catalina/loader/WebappLoader.java b/java/org/apache/catalina/loader/WebappLoader.java
index 636c741..b262db9 100644
--- a/java/org/apache/catalina/loader/WebappLoader.java
+++ b/java/org/apache/catalina/loader/WebappLoader.java
@@ -21,7 +21,10 @@ import java.beans.PropertyChangeSupport;
 import java.io.File;
 import java.io.FilePermission;
 import java.io.IOException;
+import java.lang.instrument.ClassFileTransformer;
 import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
 import java.net.URL;
 import java.net.URLClassLoader;
 import java.nio.charset.StandardCharsets;
@@ -87,6 +90,14 @@ public class WebappLoader extends LifecycleMBeanBase implements Loader{
 
 
     /**
+     * The profile name which will be used by the converter, or null if not used.
+     * Any invalid profile value will default to the TOMCAT profile, which
+     * converts all packages used by Tomcat.
+     */
+    private String jakartaConverter = null;
+
+
+    /**
      * The Java class name of the ClassLoader implementation to be used.
      * This class should extend WebappClassLoaderBase, otherwise, a different
      * loader implementation must be used.
@@ -173,6 +184,32 @@ public class WebappLoader extends LifecycleMBeanBase implements Loader{
 
 
     /**
+     * @return a non null String if the loader will attempt to use the
+     *  Jakarta converter. The String is the name of the profile
+     *  used for conversion.
+     */
+    public String getJakartaConverter() {
+        return jakartaConverter;
+    }
+
+
+    /**
+     * Set the Jakarta converter.
+     *
+     * @param jakartaConverter The profile name which will be used by the converter
+     *   Any invalid profile value will default to the TOMCAT profile, which
+     *   converts all packages used by Tomcat.
+     */
+    public void setJakartaConverter(String jakartaConverter) {
+        String oldJakartaConverter = this.jakartaConverter;
+        this.jakartaConverter = jakartaConverter;
+        support.firePropertyChange("jakartaConverter",
+                oldJakartaConverter,
+                this.jakartaConverter);
+    }
+
+
+    /**
      * @return the ClassLoader class name.
      */
     public String getLoaderClass() {
@@ -327,6 +364,26 @@ public class WebappLoader extends LifecycleMBeanBase implements Loader{
             classLoader.setResources(context.getResources());
             classLoader.setDelegate(this.delegate);
 
+            // Set Jakarta class converter
+            if (getJakartaConverter() != null) {
+                try {
+                    Class<?> jakartaEnumClass = Class.forName("org.apache.tomcat.jakartaee.EESpecProfile");
+                    Method valueOf = jakartaEnumClass.getMethod("valueOf", String.class);
+                    Object profile = null;
+                    try {
+                        profile = valueOf.invoke(null, getJakartaConverter());
+                    } catch (InvocationTargetException ignored) {
+                        profile = valueOf.invoke(null, "TOMCAT");
+                    }
+                    ClassFileTransformer transformer =
+                            (ClassFileTransformer) Class.forName("org.apache.tomcat.jakartaee.ClassConverter")
+                            .getConstructor(jakartaEnumClass).newInstance(profile);
+                    classLoader.addTransformer(transformer);
+                } catch (InstantiationException | IllegalAccessException | ClassNotFoundException e) {
+                    log.warn(sm.getString("webappLoader.noJakartaConverter"), e);
+                }
+            }
+
             // Configure our repositories
             setClassPath();
 
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index d120802..4f3fa1c 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -120,6 +120,13 @@
         <code>getParallelAnnotationScanning</code> for consistency and ease
         of use in JMX descriptors. (remm)
       </fix>
+      <update>
+        Allow the loader to directly use the JakartaEE migration tool as a
+        <code>ClassFileTransformer</code> using the
+        <code>jakartaConverter</code> attribute. This only supports javax to
+        jakarta conversion for classes, not for classloader resources or
+        static files. (remm)
+      </update>
     </changelog>
   </subsection>
   <subsection name="Coyote">
diff --git a/webapps/docs/config/loader.xml b/webapps/docs/config/loader.xml
index 16cb521..ddd0d62 100644
--- a/webapps/docs/config/loader.xml
+++ b/webapps/docs/config/loader.xml
@@ -103,6 +103,17 @@
 
     <attributes>
 
+      <attribute name="jakartaConverter" required="false">
+        <p>If the Tomcat JakartaEE Migration Tool JAR is placed in the Catalina
+        <code>lib</code> folder, this attribute allows using it. The value of
+        the attribute should be the desired profile name for the conversion.
+        If an invalid value is specified, the <code>TOMCAT</code> profile will
+        be used, which includes all javax classes which are used by Tomcat.
+        Only classes will be converted, classloader resources (loaded using
+        <code>getResource</code> or <code>getResourceAsStream</code> will not
+        be processed).</p>
+      </attribute>
+
       <attribute name="loaderClass" required="false">
         <p>Java class name of the <code>java.lang.ClassLoader</code>
         implementation class to use. Custom implementations must extend


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [tomcat] branch master updated: Allow casual runtime use of the migration tool

markt
On 16/02/2021 16:07, [hidden email] wrote:

> 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 abd1dea  Allow casual runtime use of the migration tool
> abd1dea is described below
>
> commit abd1dead7804e99e3215e8e01a6cf7448a6b9f36
> Author: remm <[hidden email]>
> AuthorDate: Tue Feb 16 17:06:27 2021 +0100
>
>     Allow casual runtime use of the migration tool
>    
>     This allows specifying a profile which will be used for a
>     ClassFileTransformer. Nothing much to report, it does basic things but
>     does not do classloader resources or static files.

I was about to query why you are using reflection when I remembered that
the library isn't availaable by default. I was planning on changing that
as soon as 0.2.0 was released as part of my implementation of migration
at deployment time.

Once the library is available by default we can simplify this some.

I was also planning on adding migrate.[sh|bat] scripts to the bin
directory so the migration tool was available on the command line.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [tomcat] branch master updated: Allow casual runtime use of the migration tool

remm
On Wed, Feb 17, 2021 at 10:23 AM Mark Thomas <[hidden email]> wrote:

> On 16/02/2021 16:07, [hidden email] wrote:
> > 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 abd1dea  Allow casual runtime use of the migration tool
> > abd1dea is described below
> >
> > commit abd1dead7804e99e3215e8e01a6cf7448a6b9f36
> > Author: remm <[hidden email]>
> > AuthorDate: Tue Feb 16 17:06:27 2021 +0100
> >
> >     Allow casual runtime use of the migration tool
> >
> >     This allows specifying a profile which will be used for a
> >     ClassFileTransformer. Nothing much to report, it does basic things
> but
> >     does not do classloader resources or static files.
>
> I was about to query why you are using reflection when I remembered that
>

Will remove it obviously, just testing ...


> the library isn't availaable by default. I was planning on changing that
> as soon as 0.2.0 was released as part of my implementation of migration
> at deployment time.
>
> Once the library is available by default we can simplify this some.
>

If you want it available by default "soon", then a new tag is needed as the
0.2.0 shaded JAR causes an error (the classpath attribute of the manifest
causes a failure as it tries to lookup for the dependent JARs).


>
> I was also planning on adding migrate.[sh|bat] scripts to the bin
> directory so the migration tool was available on the command line.
>

 Good idea !

Rémy
Reply | Threaded
Open this post in threaded view
|

Re: [tomcat] branch master updated: Allow casual runtime use of the migration tool

markt
On 17/02/2021 09:49, Rémy Maucherat wrote:

> On Wed, Feb 17, 2021 at 10:23 AM Mark Thomas <[hidden email]> wrote:
>
>> On 16/02/2021 16:07, [hidden email] wrote:
>>> 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 abd1dea  Allow casual runtime use of the migration tool
>>> abd1dea is described below
>>>
>>> commit abd1dead7804e99e3215e8e01a6cf7448a6b9f36
>>> Author: remm <[hidden email]>
>>> AuthorDate: Tue Feb 16 17:06:27 2021 +0100
>>>
>>>     Allow casual runtime use of the migration tool
>>>
>>>     This allows specifying a profile which will be used for a
>>>     ClassFileTransformer. Nothing much to report, it does basic things
>> but
>>>     does not do classloader resources or static files.
>>
>> I was about to query why you are using reflection when I remembered that
>>
>
> Will remove it obviously, just testing ...

Cool.

>> the library isn't availaable by default. I was planning on changing that
>> as soon as 0.2.0 was released as part of my implementation of migration
>> at deployment time.
>>
>> Once the library is available by default we can simplify this some.
>>
>
> If you want it available by default "soon", then a new tag is needed as the
> 0.2.0 shaded JAR causes an error (the classpath attribute of the manifest
> causes a failure as it tries to lookup for the dependent JARs).

We should be able to avoid that by adding it to the list of JARs not to
scan (which we would want to do anyway as there is no need to scan it).

I agree the classpath needed fixing for other use cases.

>> I was also planning on adding migrate.[sh|bat] scripts to the bin
>> directory so the migration tool was available on the command line.
>>
>
>  Good idea !

Thanks.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [tomcat] branch master updated: Allow casual runtime use of the migration tool

remm
On Wed, Feb 17, 2021 at 10:52 AM Mark Thomas <[hidden email]> wrote:

> On 17/02/2021 09:49, Rémy Maucherat wrote:
> > On Wed, Feb 17, 2021 at 10:23 AM Mark Thomas <[hidden email]> wrote:
> >
> >> On 16/02/2021 16:07, [hidden email] wrote:
> >>> 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 abd1dea  Allow casual runtime use of the migration tool
> >>> abd1dea is described below
> >>>
> >>> commit abd1dead7804e99e3215e8e01a6cf7448a6b9f36
> >>> Author: remm <[hidden email]>
> >>> AuthorDate: Tue Feb 16 17:06:27 2021 +0100
> >>>
> >>>     Allow casual runtime use of the migration tool
> >>>
> >>>     This allows specifying a profile which will be used for a
> >>>     ClassFileTransformer. Nothing much to report, it does basic things
> >> but
> >>>     does not do classloader resources or static files.
> >>
> >> I was about to query why you are using reflection when I remembered that
> >>
> >
> > Will remove it obviously, just testing ...
>
> Cool.
>
> >> the library isn't availaable by default. I was planning on changing that
> >> as soon as 0.2.0 was released as part of my implementation of migration
> >> at deployment time.
> >>
> >> Once the library is available by default we can simplify this some.
> >>
> >
> > If you want it available by default "soon", then a new tag is needed as
> the
> > 0.2.0 shaded JAR causes an error (the classpath attribute of the manifest
> > causes a failure as it tries to lookup for the dependent JARs).
>
> We should be able to avoid that by adding it to the list of JARs not to
> scan (which we would want to do anyway as there is no need to scan it).
>

Oops, I had forgotten that, "problem solved" since it has to be done
anyway, I agree :)
The runtime hack works well enough to play but is obviously unreliable.
Example: with the Tomcat 9 examples webapp, the included JSTL impl won't
work since the classes there are not statically converted to jakarta. A
deploy time tool is much more appropriate to resolve that kind of situation
and move any EE impl portions to the new namespace.

Rémy


>
> I agree the classpath needed fixing for other use cases.
>
> >> I was also planning on adding migrate.[sh|bat] scripts to the bin
> >> directory so the migration tool was available on the command line.
> >>
> >
> >  Good idea !
>
> Thanks.
>
> Mark
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>