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

[WFLY-19629] set bootableJarName in bootable-jar profile #946

Closed
wants to merge 1 commit into from

Conversation

kstekovi
Copy link
Contributor

@kstekovi kstekovi commented Aug 9, 2024

@kstekovi kstekovi requested a review from emmartins as a code owner August 9, 2024 12:46
@emmartins
Copy link
Contributor

emmartins commented Aug 12, 2024

@kstekovi for this kind of solution you need to change the quickstart_ci.yaml lines 137 and 244 to include -Dbootable-jar-name=${{ inputs.QUICKSTART_PATH }}-bootable.jar , this is why all CI is failing on bootable jar testing... and also there is an instruction at https://github.com/wildfly/quickstart/blob/main/shared-doc/run-integration-tests-with-bootable-jar.adoc?plain=1#L22 where you need to do similar. You also need to revert the removal of <name/> lines, cause those are used to define the web path of the quickstart (from root, being ROOT.war same as "/").

There is an alternative tho, which is to keep the plugin default config i.e. name all bootable jars "server-bootable.jar", and change instead the README instructions at https://github.com/wildfly/quickstart/blob/main/shared-doc/build-and-run-the-quickstart-with-bootable-jar.adoc lines 65 and 68, to use server-bootable.jar instead of {artifactId}-bootable-jar. Personally I prefer this alternative since it means less pom.xml content, but maybe it is just me.

@jfdenise @jamezp any opinion on what should be the default wildfly maven plugin config we should use wrt bootable jars?

Copy link
Contributor

@emmartins emmartins left a comment

Choose a reason for hiding this comment

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

See comment above

@jamezp
Copy link
Member

jamezp commented Aug 12, 2024

I need to think about this. The bootablJarName really should have been defaulting to ${project.artifactId}-bootable.jar. It would be an easy thing to change, but isn't backwards compatible. However, it might not really be a big deal either.

@jamezp
Copy link
Member

jamezp commented Aug 12, 2024

@emmartins
Copy link
Contributor

emmartins commented Aug 12, 2024

@jamezp considering that we didn't yet release anything on EAP yet I agree it shouldn't be a big issue to change to ${project-artifactId}-bootable.jar, and that should mean all QS needs to do is move to the new plugin version

PS: please note that this affects not only package goal, also start-jar

@jamezp
Copy link
Member

jamezp commented Aug 12, 2024

@jamezp considering that we didn't yet release anything on EAP yet I agree it shouldn't be a big issue to change to ${project-artifactId}-bootable.jar, and that should mean all QS needs to do is move to the new plugin version

PS: please note that this affects not only package goal, also start-jar

We've had it supported in WildFly since 5.0, which is the current release. It would be a breaking change for 5.0, but might be okay. I'll have to think if that's something we want to do or if it should go into 5.1.

@kstekovi
Copy link
Contributor Author

kstekovi commented Aug 16, 2024

@kstekovi for this kind of solution you need to change the quickstart_ci.yaml lines 137 and 244 to include -Dbootable-jar-name=${{ inputs.QUICKSTART_PATH }}-bootable.jar , this is why all CI is failing on bootable jar testing... and also there is an instruction at https://github.com/wildfly/quickstart/blob/main/shared-doc/run-integration-tests-with-bootable-jar.adoc?plain=1#L22 where you need to do similar. You also need to revert the removal of <name/> lines, cause those are used to define the web path of the quickstart (from root, being ROOT.war same as "/").

There is an alternative tho, which is to keep the plugin default config i.e. name all bootable jars "server-bootable.jar", and change instead the README instructions at https://github.com/wildfly/quickstart/blob/main/shared-doc/build-and-run-the-quickstart-with-bootable-jar.adoc lines 65 and 68, to use server-bootable.jar instead of {artifactId}-bootable-jar. Personally I prefer this alternative since it means less pom.xml content, but maybe it is just me.

@jfdenise @jamezp any opinion on what should be the default wildfly maven plugin config we should use wrt bootable jars?

Hi @emmartins requested change done. I did the first approach even it means one more line in the pom.xml

@jamezp regarding to https://issues.redhat.com/browse/WFMP-265 using the <bootableJarName> is just a workaround till the WFMP-265 will be done?

@emmartins
Copy link
Contributor

@jamezp any idea when we can have WFMP-265 done? Should we proceed with this one till the plugin is updated?

@kstekovi
Copy link
Contributor Author

@jamezp does is WFMP-265 only about a default value? So with the -Dbootable-jar-name=microprofile-config-bootable.jar parameter it should look for microprofile-config-bootable.jar?

@emmartins
Copy link
Contributor

@kstekovi maybe that is a second issue, @jamezp can you please look at that one too?

@emmartins
Copy link
Contributor

no longer needed since the plugin is now using to the old wildfly-jar behavior, producing ${artifactId}-bootable.jar

@emmartins emmartins closed this Nov 15, 2024
@jamezp
Copy link
Member

jamezp commented Nov 15, 2024

Oh wow. I don't know how I missed this. I'm sorry about that @emmartins. I think it's time to clean up my inbox :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants