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

Goal 'compile-jsp' uses wrong source/target #1812

Closed
bmarwell opened this issue May 3, 2024 · 11 comments · Fixed by #1813
Closed

Goal 'compile-jsp' uses wrong source/target #1812

bmarwell opened this issue May 3, 2024 · 11 comments · Fixed by #1813
Assignees
Labels

Comments

@bmarwell
Copy link
Contributor

bmarwell commented May 3, 2024

Hi,

when migrating to 11/17, we caught an error in the CompileJspMojo logic:

Potential issues

it tries to read "source" from the maven war plugin. However:

  • Source Level and Target level are two different things - I do not see a target level specified
  • maven.compiler.release seems ignored which is the better option, if set.
  • It completely ignores the server.xml properties which are both valid:
    <jspEngine javaSourceLevel="21" /> or <jspEngine jdkSourceLevel="18" />

While both work, the current docs only mention "javaSourceLevel" to avoid confusion: jdkSourceLevel uses "18" for "1.8" or simply "8" and does not go beyond that.

Recommended changes

  1. use server.xml (location: configurable) when present.
    a. prefer javaSourceLevel
    b. then the next best option is jdkSourceLevel
  2. Otherwise use pom.xml settings
    a. Prefer "release" paramter of Maven compiler Plugin
    b. Otherwise use source (as of now)

Other changes

Breaking changes

  • Potentially fixes builds :) Previously, you could not really set the target properly.
  • source should have been target all along, as far as I can tell

Other things to be aware of

I found that jsp-level (2.2/2.3) could have been read from server.xml as well. You might want to open another ticket for this.

@cherylking
Copy link
Member

@bmarwell Thank you for bringing this to our attention. We will have to look into this further in order to answer your specific questions.

@cherylking cherylking added the bug label May 3, 2024
@cherylking
Copy link
Member

cherylking commented May 3, 2024

It completely ignores the server.xml properties which are both valid:
<jspEngine javaSourceLevel="21" /> or

@bmarwell Was there a second part to this comment?

@scottkurz
Copy link
Member

It completely ignores the server.xml properties which are both valid:
<jspEngine javaSourceLevel="21" /> or

@bmarwell Was there a second part to this comment?

I fixed the formatting to reveal this..

@cherylking
Copy link
Member

Related issue we may want to consider when fixing this issue - #1467

@bmarwell
Copy link
Contributor Author

bmarwell commented May 3, 2024

Thanks for fixing the formatting.
Quick question: can I somehow use another plugin to compile JSPs?

@cherylking
Copy link
Member

Quick question: can I somehow use another plugin to compile JSPs?

I don't think so. I have a fix up for review and can create a snapshot by EOB today. Will you be able to test it @bmarwell ?

@cherylking
Copy link
Member

Does the plugin actually update the web.xml file as other plugins do?

No, it only copies the compiled jsp class files into the application binary.

Do JSP files compiled with compile-jsp work on other application servers or does it compile against the OpenLiberty API?

It compiles with the jsp-2.2 / jsp-2.3 feature from OpenLiberty.

@bmarwell
Copy link
Contributor Author

bmarwell commented May 6, 2024

Quick question: can I somehow use another plugin to compile JSPs?

I don't think so. I have a fix up for review and can create a snapshot by EOB today. Will you be able to test it @bmarwell ?

Sadly no, I don't have access to the source code of that app. I'll check with my colleagues tomorrow if they can do it.

@cherylking
Copy link
Member

The fix is in the 3.10.3-SNAPSHOT. If I don't receive any feedback on the fix, I will go ahead and publish a minor fix release including the fix on Wednesday most likely.

@bmarwell
Copy link
Contributor Author

bmarwell commented May 7, 2024

The fix is in the 3.10.3-SNAPSHOT. If I don't receive any feedback on the fix, I will go ahead and publish a minor fix release including the fix on Wednesday most likely.

will build and distribute here momentarily

@cherylking
Copy link
Member

@bmarwell I saw some feedback in the support case that maven.compiler.source no longer works? I have a test case specific for that and it is passing. Can the developer of the app post a pom.xml snippet of how the maven.compiler.source is getting specified so I can try to recreate?

Couple of points:

  1. We will not be adding logic to get the info from the server.xml.
  2. Only LTS releases are supported by the javaSourceLevel attribute on the jspEngine element. So if something other than a LTS release is specified in the maven.compiler.source or maven.compiler.release, the compile-jsp goal is not going to work correctly. Do we need to add hard-coded logic to adjust the java version specified to the nearest LTS? For example, if you specify maven.compiler.release = 22 should we automatically use javaSourceLevel="21"? This seems like it would be problematic which is why I did not address it in my PR.

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

Successfully merging a pull request may close this issue.

3 participants