Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support additional recipe artifacts in quarkus update #41663

Merged
merged 1 commit into from
Jul 4, 2024

Conversation

vsevel
Copy link
Contributor

@vsevel vsevel commented Jul 3, 2024

This is adding support for a updateRecipeArtifactCoordinates property in maven/gradle/cli for quarkus update, which allows to specify several artifacts containing update recipes.

this as been tested so far with mvn io.quarkus:quarkus-maven-plugin:999-SNAPSHOT:update -N -DupdateRecipeArtifactCoordinates=org.acme:quarkusupdate:1.0.0-SNAPSHOT,io.quarkus:quarkus-update-recipes:LATEST -DrewriteDryRun=true

with a 3.12.yaml recipe file in module org.acme:quarkusupdate containing:

---
type: [specs.openrewrite.org/v1beta/recipe](http://specs.openrewrite.org/v1beta/recipe)
name: add property foo
recipeList:
  - org.openrewrite.maven.AddProperty:
      key: foo
      value: bar
      preserveExistingValue: true

when running against a 3.8.5 project, this produces the following build log:

[INFO] Running recipe(s)...
[WARNING] These recipes would make changes to pom.xml:
[WARNING]     io.quarkus.openrewrite.Quarkus
[WARNING]         org.openrewrite.maven.UpgradeDependencyVersion: {groupId=io.quarkus.platform, artifactId=quarkus-bom, newVersion=3.12.0}
[WARNING]         io.quarkus.updates.core.quarkus39.Relocations
[WARNING]             org.openrewrite.java.dependencies.ChangeDependency: {oldGroupId=io.quarkus, oldArtifactId=quarkus-resteasy-reactive, newArtifactId=quarkus-rest}
[WARNING]         add property foo
[WARNING]             org.openrewrite.maven.AddProperty: {key=foo, value=bar, preserveExistingValue=true}
[WARNING]         io.quarkus.updates.core.quarkus310.UpdateConfigPackagePom
[WARNING]             io.quarkus.updates.core.quarkus310.AdjustPackageProperty
[WARNING] Patch file available:
[WARNING]     /Users/vsevel/dev/tmp/quarkusupdate/target/rewrite/rewrite.patch
[WARNING] Estimate time saved: 5m
[WARNING] Run 'mvn rewrite:run' to apply the recipes.

and the diff looks like:

% cat /Users/vsevel/dev/tmp/quarkusupdate/target/rewrite/rewrite.patch
diff --git a/pom.xml b/pom.xml
index 8560607..ffffd24 100644
--- a/pom.xml
+++ b/pom.xml
@@ -7,12 +7,13 @@ org.openrewrite.config.CompositeRecipe
   <version>1.0.2-SNAPSHOT</version>
   <properties>
     <compiler-plugin.version>3.13.0</compiler-plugin.version>
+    <foo>bar</foo>
     <maven.compiler.release>17</maven.compiler.release>
     <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
     <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
     <quarkus.platform.artifact-id>quarkus-bom</quarkus.platform.artifact-id>
     <quarkus.platform.group-id>io.quarkus.platform</quarkus.platform.group-id>
-    <quarkus.platform.version>3.8.5</quarkus.platform.version>
+    <quarkus.platform.version>3.12.0</quarkus.platform.version>
     <skipITs>true</skipITs>
     <surefire-plugin.version>3.2.5</surefire-plugin.version>
   </properties>
@@ -34,7 +35,7 @@
     </dependency>
     <dependency>
       <groupId>io.quarkus</groupId>
-      <artifactId>quarkus-resteasy-reactive</artifactId>
+      <artifactId>quarkus-rest</artifactId>
     </dependency>
     <dependency>
       <groupId>io.quarkus</groupId>
@@ -114,7 +115,8 @@
       </activation>
       <properties>
         <skipITs>false</skipITs>
-        <quarkus.package.type>native</quarkus.package.type>
+        <quarkus.native.enabled>true</quarkus.native.enabled>
+        <quarkus.package.jar.enabled>false</quarkus.package.jar.enabled>
       </properties>
     </profile>
   </profiles>

gradle support has not been done yet.
this has not been rebased onto the other PR (wondering if we really need to).
some logging has been added for debugging purposes, but will be removed before merge.
this is created as a draft to get some early feedback from @ia3andy

@quarkus-bot quarkus-bot bot added area/cli Related to quarkus cli (not maven/gradle/etc.) area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/maven area/platform Issues related to definition and interaction with Quarkus Platform labels Jul 3, 2024
@ia3andy
Copy link
Contributor

ia3andy commented Jul 3, 2024

If you keep the other PR and rename the param to additionalRecipeCoords, then you don't need to specify the quarkus core one

@vsevel
Copy link
Contributor Author

vsevel commented Jul 3, 2024

we can change the attribute to be additionalRecipeCoords, and always include the io.quarkus:quarkus-update-recipes. this is a good idea. I will go ahead and change that.
the other PR is not required unless we want to offer the ability to not use the default quarkus-update-recipes. this is not a requirement I have (but others might?).

@vsevel
Copy link
Contributor Author

vsevel commented Jul 3, 2024

changed naming to additionalUpdateRecipeCoords

@quarkus-bot quarkus-bot bot added the area/gradle Gradle label Jul 3, 2024
@vsevel
Copy link
Contributor Author

vsevel commented Jul 3, 2024

should be ready for review.
tested with mvn io.quarkus:quarkus-maven-plugin:999-SNAPSHOT:update -N -DadditionalUpdateRecipeCoords=org.acme:quarkusupdate:1.0.3-SNAPSHOT -DrewriteDryRun=true
if no comments, I need to get rid of the extra logs, plus squash/rebase

@vsevel vsevel requested a review from ia3andy July 3, 2024 16:08
Copy link
Contributor

@ia3andy ia3andy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks good !

@vsevel vsevel marked this pull request as ready for review July 3, 2024 17:50

This comment has been minimized.

@vsevel vsevel force-pushed the feature/recipeArtifactCoordinates branch from 10c1ef8 to b65c70c Compare July 4, 2024 04:07
@vsevel vsevel force-pushed the feature/recipeArtifactCoordinates branch from b65c70c to 0ae63d5 Compare July 4, 2024 04:08
@vsevel
Copy link
Contributor Author

vsevel commented Jul 4, 2024

I cleaned the logs, squashed and rebased. I am wondering if we should keep the clean in the process-sources command.

@ia3andy ia3andy changed the title Support multiple recipe artifacts in quarkus update Support additional recipe artifacts in quarkus update Jul 4, 2024
Copy link

quarkus-bot bot commented Jul 4, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 0ae63d5.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@vsevel
Copy link
Contributor Author

vsevel commented Jul 4, 2024

CI finally green @gsmet

@ia3andy
Copy link
Contributor

ia3andy commented Jul 4, 2024

@vsevel wait we might need to rename again following @maxandersen review:

And I suggest it becomes:
updateRecipe and additionalUpdateRecipes to signal one is singular the other plural/list.

@ia3andy
Copy link
Contributor

ia3andy commented Jul 4, 2024

@vsevel ok since there is no release planned, let's merge this and I'll rename both in my following PR

Thanks for this!!

@ia3andy ia3andy merged commit deff9c6 into quarkusio:main Jul 4, 2024
52 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.13 - main milestone Jul 4, 2024
@vsevel vsevel deleted the feature/recipeArtifactCoordinates branch July 4, 2024 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cli Related to quarkus cli (not maven/gradle/etc.) area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/gradle Gradle area/maven area/platform Issues related to definition and interaction with Quarkus Platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants