openrewrite / rewrite

Automated mass refactoring of source code.
https://docs.openrewrite.org
Apache License 2.0
2.03k stars 300 forks source link

"validating active recipes..." runs forever. #1562

Closed erdincay closed 2 years ago

erdincay commented 2 years ago

I could not make OpenRewrite work properly, I added following code to my pom.xml:

<build>
    <plugins>
        <plugin>
            <groupId>org.openrewrite.maven</groupId>
            <artifactId>rewrite-maven-plugin</artifactId>
            <version>4.21.0</version>
            <configuration>
                <activeRecipes>
                    <recipe>org.openrewrite.java.migrate.JavaxMigrationToJakarta</recipe>
                    <recipe>org.openrewrite.java.migrate.Java8toJava11</recipe>
                </activeRecipes>
            </configuration>
        </plugin>
    </plugins>
</build>

And then executed rewrite > run in the Maven menu in IntelliJ.

The job run forever, with the following log message, and does not seem to do anything:

[INFO] --- rewrite-maven-plugin:4.21.0:run (default-cli) @ fb-viva-core --- [INFO] Using active recipe(s) [org.openrewrite.java.migrate.Java8toJava11, org.openrewrite.java.migrate.JavaxMigrationToJakarta] [INFO] Using active styles(s) [] [INFO] Validating active recipes...

Can someone hint me to the right direction, what the issue might be?

timtebeek commented 2 years ago

Your plugin configuration appears to be missing the dependency on rewrite-migrate-java; See here.

erdincay commented 2 years ago

@timtebeek

even after adding the dependency it still runs forever:

<build>
    <plugins>
        <plugin>
            <groupId>org.openrewrite.maven</groupId>
            <artifactId>rewrite-maven-plugin</artifactId>
            <version>4.21.0</version>
            <configuration>
                <activeRecipes>
                    <recipe>org.openrewrite.java.migrate.JavaxMigrationToJakarta</recipe>
                    <recipe>org.openrewrite.java.migrate.Java8toJava11</recipe>
                </activeRecipes>
            </configuration>
            <dependencies>
                <dependency>
                    <groupId>org.openrewrite.recipe</groupId>
                    <artifactId>rewrite-migrate-java</artifactId>
                    <version>1.3.0</version>
                </dependency>
            </dependencies>
        </plugin>
    </plugins>
</build>

I have tried to run org.openrewrite.maven:rewrite-maven-plugin:4.21.0:run with JDK 8 and with JDK 11, in both cases it just runs forever. Even with version 1.1.0 of the rewrite-migrate-java - recipe collection it does not terminate - and runs forever.

As far as I can say, it hangs at Validating active recipes in the https://github.com/openrewrite/rewrite-maven-plugin/blob/main/src/main/java/org/openrewrite/maven/AbstractRewriteMojo.java - File.

timtebeek commented 2 years ago

Do you still see the same issue with rewrite-maven-plugin 4.22.0 and rewrite-migrate-java 1.4.0?

tkvangorder commented 2 years ago

@erdincay can you share the pom.xml file that is hanging?

Please note that rewrite has its own maven resolution and is therefore going to be downloading pom.xml and metadata.xml from remote repositories. The resolution logic will cache those downloaded artifacts under ~/.rewrite-cache

I am wondering if there is something in your maven hierarchy that may be causing the delay.

erdincay commented 2 years ago

Do you still see the same issue with rewrite-maven-plugin 4.22.0 and rewrite-migrate-java 1.4.0?

Thanks, I tested it, it failed with the following message:

[ERROR] Recipe validation error in org.openrewrite.java.migrate.Java8toJava11.recipeList[12] (in jar:file:/C:/Entwicklung/open_source/maven/repository/org/openrewrite/recipe/rewrite-migrate-java/1.4.0/rewrite-migrate-java-1.4.0.jar!/META-INF/rewrite/java8-to-java11.yml): recipe 'org.openrewrite.java.migrate.jacoco.UpgradeJaCoCoVersion' does not exist.

Which is good, because now I have at least an Error Message, and I could track that down to the following location:

https://github.com/openrewrite/rewrite-migrate-java/blob/main/src/main/resources/META-INF/rewrite/java8-to-java11.yml

The last line: - org.openrewrite.java.migrate.jacoco.UpgradeJaCoCoMavenPluginVersion has been modified, instead of UpgradeJaCoCoVersion the recipe is now named UpgradeJaCoCoMavenPluginVersion. Unfortunately this is not the case in the JAR, that I have obtained through Maven. I assume, that this change somewhat has been pushed after building version 1.4.0 or that there was a naming issue at buildtime, or an Git update issue on the buildserver on your side.

This is the content of my java8-to-java11.yml of the rewrite-migrate-java:1.4.0 package:

#
# Copyright 2021 the original author or authors.
# <p>
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
# <p>
# https://www.apache.org/licenses/LICENSE-2.0
# <p>
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#
---
type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.java.migrate.Java8toJava11
displayName: Migrate Java 8 to Java 11
description: This recipe will apply changes commonly needed when migrating from Java 8 to Java 11.
tags:
  - java11
  - jaxb
  - jaxws
  - deprecated
  - jakarta

recipeList:
  # Add javax dependencies if they are used by a project.
  - org.openrewrite.java.migrate.javax.AddJaxwsDependencies
  - org.openrewrite.java.migrate.javax.AddJaxbDependencies
  - org.openrewrite.java.migrate.javax.AddInjectDependencies
  # Add jdeprscan plugin to a maven-based build.
  - org.openrewrite.java.migrate.AddJDeprScanPlugin
  # Remediate deprecations
  - org.openrewrite.java.cleanup.BigDecimalRoundingConstantsToEnums
  - org.openrewrite.java.cleanup.PrimitiveWrapperClassConstructorToValueOf
  - org.openrewrite.java.migrate.concurrent.JavaConcurrentAPIs
  - org.openrewrite.java.migrate.lang.JavaLangAPIs
  - org.openrewrite.java.migrate.logging.JavaLoggingAPIs
  - org.openrewrite.java.migrate.net.JavaNetAPIs
  - org.openrewrite.java.migrate.sql.JavaSqlAPIs
  - org.openrewrite.java.migrate.javax.JavaxLangModelUtil
  - org.openrewrite.java.migrate.javax.JavaxManagementMonitorAPIs
  - org.openrewrite.java.migrate.javax.JavaxXmlStreamAPIs
  - org.openrewrite.java.migrate.wro4j.UpgradeWro4jMavenPluginVersion
  - org.openrewrite.java.migrate.jacoco.UpgradeJaCoCoVersion
timtebeek commented 2 years ago

@erdincay Good to hear the timeout issues you were having are resolved; The issue you mentioned relating to JaCoCo has been fixed in this commit. We just need to wait for a new release of rewrite-migrate-java for that message to go away.

Am I correct in thinking there's not much holding you back anymore then? Can this issue be closed with the suggestion of adopting the latest releases?

erdincay commented 2 years ago

@erdincay can you share the pom.xml file that is hanging?

Please note that rewrite has its own maven resolution and is therefore going to be downloading pom.xml and metadata.xml from remote repositories. The resolution logic will cache those downloaded artifacts under ~/.rewrite-cache

I am wondering if there is something in your maven hierarchy that may be causing the delay.

I tried it on several projects, the most compact one is my xml-tags - project, which consists only of these few lines:

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
    <modelVersion>4.0.0</modelVersion>

    <groupId>justa.small.company</groupId>
    <artifactId>xmltags</artifactId>
    <version>1.0-SNAPSHOT</version>

    <properties>
        <maven.compiler.source>8</maven.compiler.source>
        <maven.compiler.target>8</maven.compiler.target>
    </properties>

    <dependencies>
        <dependency>
            <groupId>com.googlecode.json-simple</groupId>
            <artifactId>json-simple</artifactId>
            <version>1.1.1</version>
        </dependency>
    </dependencies>

    <build>
        <plugins>
            <plugin>
                <groupId>org.openrewrite.maven</groupId>
                <artifactId>rewrite-maven-plugin</artifactId>
                <version>4.22.0</version>
                <configuration>
                    <activeRecipes>
                        <recipe>org.openrewrite.java.migrate.Java8toJava11</recipe>
                    </activeRecipes>
                    <failOnInvalidActiveRecipes>true</failOnInvalidActiveRecipes>
                </configuration>
                <dependencies>
                    <dependency>
                        <groupId>org.openrewrite.recipe</groupId>
                        <artifactId>rewrite-migrate-java</artifactId>
                        <version>1.4.0</version>
                    </dependency>
                </dependencies>
            </plugin>
        </plugins>
    </build>
</project>

Although I fixed the Jacoco - MavenPlugin recipe issue, by changing the last line of the yaml - File and repackaged it as a new zip and renamed it to JAR in the Maven repository. I was only able to remove the Error Message, but now I get the same behaviour that the Validation of the Active Recipes is running forever.

I have to inform you, that we are using an VPN and are behind an firewall, even though I disabled the VPN connection I was not able to make it work. I assume, that I might have to change or remove the Maven settings, which is pointing to an server within the VPN of the company I am working at.

So the issue is still there.

erdincay commented 2 years ago

@erdincay Good to hear the timeout issues you were having are resolved; The issue you mentioned relating to JaCoCo has been fixed in this commit. We just need to wait for a new release of rewrite-migrate-java for that message to go away.

Am I correct in thinking there's not much holding you back anymore then? Can this issue be closed with the suggestion of adopting the latest releases?

Unfortunately it is still not working. I checked the folder you pointed me to: ~/.rewrite-cache, I emptied it and restarted IntelliJ and retried it by using the Maven menu on the right side of the IntelliJ, and clicked on: rewrite:run, which executes following command in the console below:

C:\Entwicklung\java\ibm_sdk80u191-b26x64\bin\java.exe -Dmaven.multiModuleProjectDirectory=C:\Entwicklung\workspace-experimental\xml-tags -Xms1024m -Xmx2048m -Djavax.net.ssl.trustStore=C:\Entwicklung\open_source\maven\maven-truststore.jks -Djavax.net.ssl.trustStorePassword=justaPassword -Dhttps.protocols=TLSv1.2 -Xms1024m -Xmx2048m -Djavax.net.ssl.trustStore=C:\Entwicklung\open_source\maven\maven-truststore.jks -Djavax.net.ssl.trustStorePassword=justaPassword -Dhttps.protocols=TLSv1.2 -Dmaven.home=C:\Entwicklung\open_source\maven\apache-maven-3.5.4 -Dclassworlds.conf=C:\Entwicklung\open_source\maven\apache-maven-3.5.4\bin\m2.conf -Dmaven.ext.class.path=C:\Users\justaUser\AppData\Local\JetBrains\Toolbox\apps\IDEA-U\ch-0\211.7142.45\plugins\maven\lib\maven-event-listener.jar -javaagent:C:\Users\justaUser\AppData\Local\JetBrains\Toolbox\apps\IDEA-U\ch-0\211.7142.45\lib\idea_rt.jar=50994:C:\Users\justaUser\AppData\Local\JetBrains\Toolbox\apps\IDEA-U\ch-0\211.7142.45\bin -Dfile.encoding=UTF-8 -classpath C:\Entwicklung\open_source\maven\apache-maven-3.5.4\boot\plexus-classworlds-2.5.2.jar org.codehaus.classworlds.Launcher -Didea.version=2021.1.1 -s C:\Users\justaUser\.m2\settings.xml -Dmaven.repo.local=C:\Entwicklung\open_source\maven\repository org.openrewrite.maven:rewrite-maven-plugin:4.22.0:run

Here is the ~/.rewrite-cache - folders's content, is something missing? I couldn't see any POM files, that you were referring to: openrewrite-pluginissue

tkvangorder commented 2 years ago

Hi @erdincay

We are going to do a point release of all open rewrite projects today, so this will include the fix for the recipe name.

I took your pom.xml (obviously with no other sources) :

[INFO] Using active recipe(s) [org.openrewrite.java.migrate.Java8toJava11]
[INFO] Using active styles(s) []
[INFO] Validating active recipes...
[INFO] Parsing Java main files...
[INFO] Parsing Java test files...
[INFO] Running recipe(s)...
[WARNING] These recipes would make changes to pom.xml:
[WARNING]     org.openrewrite.java.migrate.AddJDeprScanPlugin
[WARNING] Patch file available:
[WARNING]     /Users/tyler.vangorder/work/sample-repos/what/target/site/rewrite/rewrite.patch
[WARNING] Run 'mvn rewrite:run' to apply the recipes.
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  4.972 s
[INFO] Finished at: 2022-04-05T09:12:41-07:00
[INFO] ------------------------------------------------------------------------

The two environmental things I would like to rule out on your end:

tkvangorder commented 2 years ago

There are now point releases for the rewrite maven plugin and rewrite-migrate-java

So you can update your pom as follows:

    <build>
        <plugins>
            <plugin>
                <groupId>org.openrewrite.maven</groupId>
                <artifactId>rewrite-maven-plugin</artifactId>
                <version>4.22.1</version>
                <configuration>
                    <activeRecipes>
                        <recipe>org.openrewrite.java.migrate.Java8toJava11</recipe>
                    </activeRecipes>
                    <failOnInvalidActiveRecipes>true</failOnInvalidActiveRecipes>
                </configuration>
                <dependencies>
                    <dependency>
                        <groupId>org.openrewrite.recipe</groupId>
                        <artifactId>rewrite-migrate-java</artifactId>
                        <version>1.4.2</version>
                    </dependency>
                </dependencies>
            </plugin>
        </plugins>
    </build>
erdincay commented 2 years ago

Hi @erdincay

We are going to do a point release of all open rewrite projects today, so this will include the fix for the recipe name.

I took your pom.xml (obviously with no other sources) :

[INFO] Using active recipe(s) [org.openrewrite.java.migrate.Java8toJava11]
[INFO] Using active styles(s) []
[INFO] Validating active recipes...
[INFO] Parsing Java main files...
[INFO] Parsing Java test files...
[INFO] Running recipe(s)...
[WARNING] These recipes would make changes to pom.xml:
[WARNING]     org.openrewrite.java.migrate.AddJDeprScanPlugin
[WARNING] Patch file available:
[WARNING]     /Users/tyler.vangorder/work/sample-repos/what/target/site/rewrite/rewrite.patch
[WARNING] Run 'mvn rewrite:run' to apply the recipes.
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  4.972 s
[INFO] Finished at: 2022-04-05T09:12:41-07:00
[INFO] ------------------------------------------------------------------------

The two environmental things I would like to rule out on your end:

* Do you have the ability to run rewrite on that sample pom outside your company VPN?

* Do you have any virus scanning software that might interfere with the disk operations to your `~/.rewrite-cache` folder?

Thanks for your efforts @tkvangorder.

After serioulsy digging deeper into this issue by fully changing our Maven settings, which have been in place for at least 15 years, I was able to clear the issue. In my opinion, the issue is fully on our side, and especially on the Artifactory side. The same settings.xml - File that was completely rewritten by me yesterday, which did not work then, worked this morning and was fully functional without any issues. As for now I got some information on why this could be an issue on the Artifactory side, every night the Artifactory instance is being restarted, I suspect some volatile error, that cannot be traced down on operations side.

I was able to run the execution, which delivered some results, but I am somewhat disappointed, maybe I am doing something wrong. I thought Java DateTime classes would be updated, which was not the case. I only had multiple instance creation changes on basic wrapper classes like Integer, Long, Boolean, etc. and the Javax to Jakarta - Upgrade.

Is there a recipe for DateTime classes?

erdincay commented 2 years ago

Problem solved. Execution works. Issue was on our side on our Artifactory instance.

tkvangorder commented 2 years ago

@erdincay

Glad you were able to solve your initial issue. As for the Date/Time conversions, what specifically were you hoping would be converted? Conversion java.util.Date and java.util.Calendar to the newer JSR310 classes is likely to be non-trivial, but we are definitely open to exploring options. I have opened this issue to discuss further:

https://github.com/openrewrite/rewrite-migrate-java/issues/86

erdincay commented 2 years ago

@erdincay

Glad you were able to solve your initial issue. As for the Date/Time conversions, what specifically were you hoping would be converted? Conversion java.util.Date and java.util.Calendar to the newer JSR310 classes is likely to be non-trivial, but we are definitely open to exploring options. I have opened this issue to discuss further:

openrewrite/rewrite-migrate-java#86

I was hoping exactly what you have created a ticket for. That Java 1-6 version style classes, should be updated to Java 8-11 style classes. Meaning, to migrate also non-future proof stuff like the old DateTime classes to more standarised and future-proof versions. In terms of basic understanding: The code should look more like a Java 11 application, that "has been created recently", than an application that "has been just barely migrated".

The code-base of the application should be in one of the big version-domains and leaning towards the newer version of that version-domain. Means a Java 6 application that is going to be migrated to Java 8 should use classes of the domain of {Java 8-11} that are future-proof, so that they are available in Java 11 too and if possible will be available in the next domain of {Java 17-XX}. A graphical grouping of Java class and feature consistency:

{Java 1-6} ----------------- {Java 8-11} ----------------- {Java 17-XX}

Even though the Java 8 application is going to migrate only within it's domain, the classes of the newer Java 11 application should lean towards the next domain {Java 17-XX} and not the old domain {Java 1-6}. In a trivial situation, this could mean, spoken in "Specification language": You SHOULD use JSR310 - DateTime classes, even though the old DateTime classes are available on the given domain. Reason: If you don't use JSR310 - DateTime classes, the old DateTime classes might not be available in the next domain, and the migration process to the next version-domain would be harder.

Means: Migrate as early as possible, and as much as possible.