fix filtering of web resources using system properties and escaping #451
Conversation
jtnord
changed the title
jtnord
changed the title
| @@ -212,7 +217,6 @@ | |||
| <dependency> | |||
| <groupId>org.codehaus.plexus</groupId> | |||
| <artifactId>plexus-utils</artifactId> | |||
| <!-- generated help mojo has a dependency on plexus utils --> | |||
There was a problem hiding this comment.
the generated file did not contain any plexus-util imports
jtnord
force-pushed
the
replace-filtering-2
branch
2 times, most recently
from
1893d78
to
d8aca59
Compare
replace plexus-util filtering with maven-filtering The code still in directly uses some plexus-util so the dependency is not removed but the comment was no longer correct so that was removed.
jtnord
force-pushed
the
replace-filtering-2
branch
from
d8aca59
to
ddc51f3
Compare
jtnord
changed the title
| or | ||
| \@prop2\@ @prop2@ | ||
| and these will be replaced at build time | ||
| it is even possible to use system properties | ||
| \@java.class.version\@ ${java.class.version} | ||
| \@java.vendor.url\@ ${java.vendor.url} |
There was a problem hiding this comment.
class version is of the JVM not the compiled classes - so this would on java17 etc. replaced with vendor.rul which is very jvm specific - but it is 2023 so if this is not a https:// url then someone should fix the JVM...
| MavenResourcesExecution mavenResourcesExecution = | ||
| new MavenResourcesExecution(resources, webappDirectory, project, StandardCharsets.UTF_8.name(), filters, | ||
| Collections.emptyList(), session); | ||
| mavenResourcesExecution.setEscapeString("\\"); |
There was a problem hiding this comment.
this means we can escape things - which is what you would get with regular maven filtering.
technically this is a breaking change - but looking through projects of the 5 I found adding webResources non used filtering anyway... so making this compatible would appear to be overkill
An argument could be made therefor to remove this code entirely - but as the webResources are of type Resource it would mean we would take options that would be discarded.
| // assert globalConfigHelp.contains("\\@java.class.version\\@ 55.0"); | ||
| assert globalConfigHelp.contains('${prop1} hello'); | ||
| assert globalConfigHelp.contains('@prop2@ goodbye'); | ||
| assert globalConfigHelp.contains('@java.vendor.url@ https://'); |
There was a problem hiding this comment.
showing a System property is now resolved.
| assert globalConfigHelp.contains("\\@prop2\\@ goodbye"); | ||
| // code claims to add System properties but this fails | ||
| // assert globalConfigHelp.contains("\\@java.class.version\\@ 55.0"); | ||
| assert globalConfigHelp.contains('${prop1} hello'); |
There was a problem hiding this comment.
the escapes are now working - so they get replaced with the literal (in other words the escape string is correctly removed.
webResourcesnow consultsSystem.getProperties()(again?)\${project.version}and not have it replaced by the project version. (this matches Mavens' default behaviourReplaces code from plexus-util with maven-filtering.
The code still directly uses some plexus-util so the dependency is not removed but the comment was no longer correct so that was removed.