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

Issue #3514 interpolation of mod files to avoid hardcoding version in mod files #6839

Merged
merged 7 commits into from Sep 22, 2021

Conversation

olamy
Copy link
Member

@olamy olamy commented Sep 15, 2021

to prevent issues with the current usage of ${} in mod files. Interpolation of values from pom will be based on the delimiter @....@ such @log4j2.version@

Signed-off-by: Olivier Lamy oliver.lamy@gmail.com

@olamy olamy added the Build label Sep 15, 2021
@olamy olamy force-pushed the jetty-10.0.x_modfiles_filtering branch 3 times, most recently from 0550774 to d0cf79d Compare September 16, 2021 02:24
@olamy olamy changed the title Issue #3514 start work on interpolation of mod files Issue #3514 interpolation of mod files Sep 16, 2021
@olamy olamy changed the title Issue #3514 interpolation of mod files Issue #3514 interpolation of mod files to avoid hardcoding version in mod files Sep 16, 2021
@olamy olamy marked this pull request as ready for review September 16, 2021 04:25
Copy link
Contributor

@janbartel janbartel left a comment

Choose a reason for hiding this comment

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

Need to also do: memcached and mongo.

@@ -15,9 +15,12 @@ jmx
etc/jolokia.xml

[files]
maven://org.jolokia/jolokia-war/1.3.3/war|lib/jolokia/jolokia.war
maven://org.jolokia/jolokia-war/${jolokia.version}/war|lib/jolokia/jolokia.war
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this correct? Why is this ${jolokia.version} here, but @jolokia.version@ on line 22?

Copy link
Member Author

Choose a reason for hiding this comment

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

as explained in the PR description ;)

to prevent issues with the current usage of ${} in mod files. 
Interpolation of values from pom will be based on the delimiter @....@ such @log4j2.version@

this file have the following section

[ini]
jolokia.version?=@jolokia.version@

this section will be transformed to

[ini]
jolokia.version?=3.5.4 (or whatever the value coming from the pom)

The idea is to keep the the option to run jetty using such parameter to test new jolokia version

java -jar start.jar jolokia.version=6.4.4

Copy link
Contributor

Choose a reason for hiding this comment

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

To me the use of @blah@ is processed by the resource plugin filtering resulting in a "hardcoded" value at build time.
The use of ${blah} is left alone.

Copy link
Member Author

Choose a reason for hiding this comment

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

yes I didn't want to touch anything about ${bla} because already used in mod files.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this difference is documented anywhere? So you're saying that if the substitution happens at runtime, like when the start mechanism processes the .mod/.ini file then you use ${var}, but if it happens at build time, then you use @var@??? That's a pretty subtle difference, think that needs a good explanation somewhere.

Copy link
Contributor

@joakime joakime Sep 21, 2021

Choose a reason for hiding this comment

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

@janbartel The @var@ syntax is standard Maven Resource handling with filtering.

  • The @var@ syntax is compile time expanded during the compile/package steps.
  • The ${var} syntax is runtime expanded, and in the specific cases on this PR, only by jetty-start.

Copy link
Contributor

Choose a reason for hiding this comment

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

Btw, we use the same @var@ syntax in our jetty-maven-plugin/src/it/* and jetty-jspc-maven-plugin/src/it/* test cases.
Also, the @var@ syntax is the most common form of maven resource filtering (during the various resource phases of the build).

jetty-home/pom.xml Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
@olamy
Copy link
Member Author

olamy commented Sep 20, 2021

memcached and mongo done

Copy link
Contributor

@joakime joakime left a comment

Choose a reason for hiding this comment

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

2 more mod files need conversion.

./demos/demo-jndi-webapp/src/main/config/modules/demo-jndi.mod:maven://org.eclipse.jetty.orbit/javax.mail.glassfish/1.4.1.v201005082020/jar|lib/ext/javax.mail.glassfish-1.4.1.v201005082020.jar
./demos/demo-jndi-webapp/src/main/config/modules/demo-jndi.mod:maven://jakarta.transaction/jakarta.transaction-api/1.3.2/jar|lib/ext/jakarta.transaction-api-1.3.2.jar
./jetty-home/src/main/resources/modules/jminix.mod:maven://org.jminix/jminix/1.1.0|lib/jminix/jminix-1.1.0.jar
./jetty-home/src/main/resources/modules/jminix.mod:https://maven.restlet.talend.com/org/restlet/org.restlet/1.1.5/org.restlet-1.1.5.jar|lib/jminix/org.restlet-1.1.5.jar
./jetty-home/src/main/resources/modules/jminix.mod:https://maven.restlet.talend.com/org/restlet/org.restlet.ext.velocity/1.1.5/org.restlet.ext.velocity-1.1.5.jar|lib/jminix/org.restlet.ext.velocity-1.1.5.jar
./jetty-home/src/main/resources/modules/jminix.mod:maven://org.apache.velocity/velocity/1.5|lib/jminix/velocity-1.5.jar
./jetty-home/src/main/resources/modules/jminix.mod:maven://oro/oro/2.0.8|lib/jminix/oro-2.0.8.jar
./jetty-home/src/main/resources/modules/jminix.mod:https://maven.restlet.talend.com/com/noelios/restlet/com.noelios.restlet/1.1.5/com.noelios.restlet-1.1.5.jar|lib/jminix/com.noelios.restlet-1.1.5.jar
./jetty-home/src/main/resources/modules/jminix.mod:https://maven.restlet.talend.com/com/noelios/restlet/com.noelios.restlet.ext.servlet/1.1.5/com.noelios.restlet.ext.servlet-1.1.5.jar|lib/jminix/com.noelios.restlet.ext.servlet-1.1.5.jar
./jetty-home/src/main/resources/modules/jminix.mod:maven://net.sf.json-lib/json-lib/2.2.3/jar/jdk15|lib/jminix/json-lib-2.2.3-jdk15.jar
./jetty-home/src/main/resources/modules/jminix.mod:maven://commons-lang/commons-lang/2.4|lib/jminix/commons-lang-2.4.jar
./jetty-home/src/main/resources/modules/jminix.mod:maven://commons-beanutils/commons-beanutils/1.7.0|lib/jminix/commons-beanutils-1.7.0.jar
./jetty-home/src/main/resources/modules/jminix.mod:maven://commons-collections/commons-collections/3.2|lib/jminix/commons-collections-3.2.jar
./jetty-home/src/main/resources/modules/jminix.mod:maven://net.sf.ezmorph/ezmorph/1.0.6|lib/jminix/ezmorph-1.0.6.jar
./jetty-home/src/main/resources/modules/jminix.mod:maven://org.jgroups/jgroups/2.12.1.3.Final|lib/jminix/jgroups-2.12.1.3.Final.jar
./jetty-home/src/main/resources/modules/jminix.mod:maven://org.jasypt/jasypt/1.8|lib/jminix/jasypt-1.8.jar

pom.xml Show resolved Hide resolved
Signed-off-by: Olivier Lamy <oliver.lamy@gmail.com>
Signed-off-by: Olivier Lamy <oliver.lamy@gmail.com>
Signed-off-by: Olivier Lamy <oliver.lamy@gmail.com>
@olamy olamy force-pushed the jetty-10.0.x_modfiles_filtering branch from 666e855 to 13400b3 Compare September 20, 2021 23:20
@olamy
Copy link
Member Author

olamy commented Sep 20, 2021

@joakime to be honest I would even prefer remove jminix as it's not really maintained and hawtio is providing the same features (and it's maintained)

joakime
joakime previously approved these changes Sep 21, 2021
Copy link
Contributor

@joakime joakime left a comment

Choose a reason for hiding this comment

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

I'm happy with this as it sits.

I'll go ahead and merge up the conflicts from the infinispan PR into this.

Signed-off-by: Olivier Lamy <oliver.lamy@gmail.com>
Signed-off-by: Olivier Lamy <oliver.lamy@gmail.com>
@olamy
Copy link
Member Author

olamy commented Sep 21, 2021

demo-jndi.mod fixed.
not sure we want to fix jminix but deprecate/remove it. (see #6879)

@joakime joakime added this to In progress in Jetty 10.0.7/11.0.7 FROZEN via automation Sep 22, 2021
@joakime joakime dismissed janbartel’s stale review September 22, 2021 12:26

Change already applied, Old review no longer relevant

Jetty 10.0.7/11.0.7 FROZEN automation moved this from In progress to Review in progress Sep 22, 2021
Jetty 10.0.7/11.0.7 FROZEN automation moved this from Review in progress to Reviewer approved Sep 22, 2021
@joakime joakime merged commit 9c06dc2 into jetty-10.0.x Sep 22, 2021
Jetty 10.0.7/11.0.7 FROZEN automation moved this from Reviewer approved to Done Sep 22, 2021
@joakime joakime deleted the jetty-10.0.x_modfiles_filtering branch September 22, 2021 12:27
@joakime joakime linked an issue Sep 22, 2021 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Use interpolation of versions from pom in mod files
3 participants