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

Unlike all other Maven properties, spring-boot.run.arguments on the command line takes precedence over the pom #20024

Closed
rawfg opened this issue Feb 3, 2020 · 8 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@rawfg
Copy link

rawfg commented Feb 3, 2020

Description: the spring-boot.run.jvmArguments does not take precedence over the < systemPropertyVariables> map when there is a <jvmArguments> section defined for the spring-boot-maven-plugin.

Documented behavior:
According to the https://docs.spring.io/spring-boot/docs/current/maven-plugin/examples/run-system-properties.html when the spring-boot.run.jvmArguments parameter is provided for the spring-boot:run command the parameter takes precedence and overwrites the system properties defined within the <systemPropertyVariables> XML configuration. I would expect that the presence of the jvmArguments section does not influence the documented behavior.

Steps to reproduce:
There is a sample application which demonstrates non-documented (faulty?) behavior located at https://github.com/rawfg/maven-jvmargs.

  1. The sample application prints the system properties starting with the xyz and by default there are two system properties configured: one comes from the <jvmArguments> section and the other one comes from the < systemPropertyVariables> section.

  2. When the application is run from the command line via the command:

mvn spring-boot:run -Dspring-boot.run.jvmArguments="-Dxyz.MavenSystemProperty='cmd-system-property'" 

the output is (system properties configured in the pom.xml):

xyz.MavenSystemProperty=maven-system-property
xyz.MavenJvmProperty=maven-jvm-property
  1. When the <jvmArguments> section is commented out the application behaves as it was documented, so running the command:
mvn spring-boot:run -Dspring-boot.run.jvmArguments="-Dxyz.MavenSystemProperty='cmd-system-property'" 

results in the output:

xyz.MavenSystemProperty=cmd-system-property
  1. I tested it with the Spring Boot version 2.2.4 but I've downgraded to the 2.1.7 version and I observed the same outcome.

Expected behavior:

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 3, 2020
@scottfrederick
Copy link
Contributor

scottfrederick commented Feb 7, 2020

A value provided by <jvmArguments> in the POM configuration takes precedence over a value provided by the spring-boot.run.jvmArguments system property. So in the case of your sample app as provided without changes, the -Dspring-boot.run.jvmArguments=-Dxyz.MavenSystemProperty=cmd-system-property will be ignored. When the <jvmArguments> POM configuration is removed, then the spring-boot.run.jvmArguments value is used.

Expected behavior:

This isn't so much an exception as it is an order of precedence. The documentation states that jvmArguments takes precedence over <systemPropertyVariables>, which is always the case. What isn't documented is which manner of providing jvmArguments takes precedence. A quick test shows that the order of precedence isn't consistent between configuration properties like jvmArguments and arguments.

I've marked this for team attention so we can discuss the proper order of precedence between POM configuration and spring-boot.run system properties, adding tests to confirm the proper behavior, and documenting the behavior.

@scottfrederick scottfrederick added for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged and removed status: waiting-for-triage An issue we've not yet triaged labels Feb 7, 2020
@wilkinsona
Copy link
Member

For consistency with configuration precedence in other area of Spring Boot, I think that system properties provided on the command line should take precedence over something that's hardcoded in the pom.xml file.

@scottfrederick scottfrederick self-assigned this Feb 12, 2020
@scottfrederick scottfrederick changed the title spring-boot.run.jvmArguments system properties issue Ensure Maven command-line properties take precedence over POM configuration Feb 12, 2020
@scottfrederick scottfrederick changed the title Ensure Maven command-line properties take precedence over POM configuration Ensure precedence of Maven command-line properties and POM configuration is consistent Feb 12, 2020
@scottfrederick
Copy link
Contributor

Maven documents the fact that Mojo @Parameters can be set via POM configuration or from system properties (provided the @Parameter has a property attribute) but I can't find the order of precedence documented anywhere. However, testing shows that POM configuration takes precedence over system properties.

spring-boot.run.arguments is the exception to the rule because Boot's Mojo reverses the default Maven order by declaring two separate parameters and resolving them explicitly.

@wilkinsona
Copy link
Member

However, testing shows that POM configuration takes precedence over system properties.

Just to make sure I've understood, that's the opposite of what would align with Boot's configuration precedence in other areas?

@scottfrederick
Copy link
Contributor

Just to make sure I've understood, that's the opposite of what would align with Boot's configuration precedence in other areas?

Yes, exactly the opposite.

@wilkinsona
Copy link
Member

We're going to align with Maven's defaults.

@wilkinsona wilkinsona added this to the 2.2.x milestone Feb 19, 2020
@wilkinsona wilkinsona added type: bug A general bug and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Feb 19, 2020
@wilkinsona wilkinsona changed the title Ensure precedence of Maven command-line properties and POM configuration is consistent Unlike all other Maven properties, spring-boot.run.arguments on the command line takes precedence over the pom Feb 19, 2020
@mbhave mbhave modified the milestones: 2.2.x, 2.2.5 Feb 20, 2020
@wilkinsona
Copy link
Member

Re-opening so that we can investigate the test failures on Windows: https://ge.spring.io/s/m62r4fmeh6y6k/tests/failed.

@wilkinsona wilkinsona reopened this Feb 25, 2020
@wilkinsona
Copy link
Member

2.2.x is unaffected by the test failures.

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

No branches or pull requests

5 participants