Support commas embedded in command line arguments from Maven plugin #18711

nosan · 2019-10-23T15:24:46Z

snicoll

Thanks for the PR @nosan. I am not sure we want to tokenize on ,--. I'd like to make sure we are consistent with other core maven plugins before making this change and I haven't researched yet if there is one.

snicoll · 2019-10-25T08:57:32Z

...ing-boot-maven-plugin/src/main/java/org/springframework/boot/maven/ApplicationArguments.java

+		}
+		List<String> result = new ArrayList<>();
+		for (String line : new RunArguments(String.join(",", args)).getArgs()) {
+			String[] tokens = line.split(",--");


I think that's a bit fragile. I think the plan is more to stop binding the property as a String[] but rather introduce another one that we would map to the command-line property. Existing plugins use a space-delimited feature and I haven't checked how they skip spaces within valuesL

snicoll · 2019-10-25T08:58:06Z

...ring-boot-tools/spring-boot-maven-plugin/src/it/run-arguments-commandline/invoker.properties

@@ -0,0 +1 @@
+invoker.mavenOpts=-Dspring-boot.run.arguments='--management.endpoints.web.exposure.include=prometheus,info,health,metrics,--spring.profiles.active=foo,bar'


I think it would be more natural if the arguments were separated by a space rather than a comma.

nosan · 2019-10-25T10:35:52Z

Thanks, @snicoll

We can't change the type String[] to String at least in the current version:

[ERROR] Failed to execute goal org.springframework.boot:spring-boot-maven-plugin:2.2.1.BUILD-SNAPSHOT:run (default) on project run-arguments: Unable to parse configuration of mojo org.springframework.boot:spring-boot-maven-plugin:2.2.1.BUILD-SNAPSHOT:run: Basic element 'arguments' must not contain child elements -> [Help 1]

Steps to reproduce:

<plugin>
	<groupId>org.springframework.boot</groupId>
	<artifactId>spring-boot-maven-plugin</artifactId>
	<version>${version}</version>
	<executions>
		<execution>
			<phase>package</phase>
			<goals>
				<goal>run</goal>
			</goals>
			<configuration>
				<arguments>
					<argument>--management.endpoints.web.exposure.include=prometheus,info</argument>
					<argument>--spring.profiles.active=foo,bar</argument>
				</arguments>
			</configuration>
		</execution>
	</executions>
</plugin>

UPD:

$ mvn spring-boot:run -Dspring-boot.run.arguments='--management.endpoints.web.exposure.include=prometheus,info,health,metrics --spring.profiles.active=foo,bar'

Works fine if we change the type for arguments field from array to String or just introduce a separate field to map system property.

	/**
	 * Arguments from command line that should be passed to the application.
	 * @since 2.2.1
	 */
	@Parameter(property = "spring-boot.run.arguments", readonly = true)
	private String args;

snicoll · 2019-10-25T11:14:50Z

We can't change the type String[] to String at least in the current version:

Thanks for the feedback but that's not what I was suggesting

I think the plan is more to stop binding the property as a String[] but rather introduce another one that we would map to the command-line property.

In other words, keep the property that we have but remove the binding to command line and introduce a new property of type String that we would bound. Anyway, I haven't looked in details so just sharing what I would do without looking in more details.

See spring-projectsgh-18688

…erty.

nosan · 2019-10-25T19:49:25Z

I've pushed an additional commit with @snicoll suggestions.

luvarqpp · 2019-12-19T15:01:16Z

...-maven-plugin/src/it/run-arguments-commandline/src/main/java/org/test/SampleApplication.java

+		}
+		String endpoints = args[0].split("=")[1];
+		String profile = args[1].split("=")[1];
+		System.out.println("I haz been run with profile(s) '" + profile + "' and endpoint(s) '" + endpoints + "'");


one typo, haz -> has

Thanks, it was done on purpose.

luvarqpp · 2019-12-19T15:02:02Z

...g-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/it/run-arguments/verify.groovy

@@ -0,0 +1,2 @@
+def file = new File(basedir, "build.log")
+return file.text.contains("I haz been run with profile(s) 'foo,bar' and endpoint(s) 'prometheus,info'")


typo, haz -> has

Thanks, it was done on purpose.

See gh-18711

snicoll · 2019-12-26T10:19:34Z

Well done once again @nosan!

nosan · 2019-12-26T10:26:43Z

Thank you @snicoll

spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 23, 2019

nosan changed the title ~~Add support commas embedded in command line arguments from Maven plugin~~ Support commas embedded in command line arguments from Maven plugin Oct 23, 2019

nosan force-pushed the gh-18688 branch from 058c50d to ef06c2f Compare October 23, 2019 18:29

philwebb added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 23, 2019

philwebb added this to the 2.2.x milestone Oct 23, 2019

philwebb mentioned this pull request Oct 23, 2019

Support commas embedded in command line arguments from Maven plugin #18688

Closed

nosan force-pushed the gh-18688 branch from ef06c2f to a356d6a Compare October 24, 2019 09:19

snicoll reviewed Oct 25, 2019

View reviewed changes

nosan added 2 commits October 25, 2019 22:45

Support commas embedded in command line arguments from Maven plugin

79377a6

See spring-projectsgh-18688

Introduce args field to map spring-boot.run.arguments system prop…

5de47fb

…erty.

nosan force-pushed the gh-18688 branch from a356d6a to 5de47fb Compare October 25, 2019 19:48

luvarqpp reviewed Dec 19, 2019

View reviewed changes

snicoll self-assigned this Dec 26, 2019

snicoll modified the milestones: 2.2.x, 2.2.3 Dec 26, 2019

snicoll mentioned this pull request Dec 26, 2019

Support commas embedded in command line arguments from Maven plugin #19463

Closed

snicoll pushed a commit that referenced this pull request Dec 26, 2019

Support commas embedded in command line arguments from Maven plugin

1fb904a

See gh-18711

snicoll added a commit that referenced this pull request Dec 26, 2019

Polish contribution

6c01666

See gh-18711

snicoll closed this in 83e594a Dec 26, 2019

nosan deleted the gh-18688 branch December 26, 2019 10:26

snicoll mentioned this pull request Jan 31, 2020

Document spring-boot.run.arguments behaviour with multiple arguments #19998

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support commas embedded in command line arguments from Maven plugin #18711

Support commas embedded in command line arguments from Maven plugin #18711

nosan commented Oct 23, 2019

snicoll left a comment

snicoll Oct 25, 2019

snicoll Oct 25, 2019

nosan commented Oct 25, 2019 •

edited

Loading

snicoll commented Oct 25, 2019

nosan commented Oct 25, 2019

luvarqpp Dec 19, 2019

nosan Dec 19, 2019

luvarqpp Dec 19, 2019

nosan Dec 19, 2019

snicoll commented Dec 26, 2019

nosan commented Dec 26, 2019

		@@ -0,0 +1 @@
		invoker.mavenOpts=-Dspring-boot.run.arguments='--management.endpoints.web.exposure.include=prometheus,info,health,metrics,--spring.profiles.active=foo,bar'

		@@ -0,0 +1,2 @@
		def file = new File(basedir, "build.log")
		return file.text.contains("I haz been run with profile(s) 'foo,bar' and endpoint(s) 'prometheus,info'")

Support commas embedded in command line arguments from Maven plugin #18711

Support commas embedded in command line arguments from Maven plugin #18711

Conversation

nosan commented Oct 23, 2019

snicoll left a comment

Choose a reason for hiding this comment

snicoll Oct 25, 2019

Choose a reason for hiding this comment

snicoll Oct 25, 2019

Choose a reason for hiding this comment

nosan commented Oct 25, 2019 • edited Loading

snicoll commented Oct 25, 2019

nosan commented Oct 25, 2019

luvarqpp Dec 19, 2019

Choose a reason for hiding this comment

nosan Dec 19, 2019

Choose a reason for hiding this comment

luvarqpp Dec 19, 2019

Choose a reason for hiding this comment

nosan Dec 19, 2019

Choose a reason for hiding this comment

snicoll commented Dec 26, 2019

nosan commented Dec 26, 2019

nosan commented Oct 25, 2019 •

edited

Loading