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

mvnd native executable is not passing -Dkey=val to the daemon #157

Closed
thiagolocatelli opened this issue Oct 26, 2020 · 14 comments · Fixed by #159
Closed

mvnd native executable is not passing -Dkey=val to the daemon #157

thiagolocatelli opened this issue Oct 26, 2020 · 14 comments · Fixed by #159
Assignees
Milestone

Comments

@thiagolocatelli
Copy link

thiagolocatelli commented Oct 26, 2020

I do have the following configuration for my docker-maven-plugin in the parent pom.xml for my multi module project:

	<plugin>
		<groupId>com.spotify</groupId>
		<artifactId>dockerfile-maven-plugin</artifactId>
		<version>1.4.13</version>
		<executions>
			<execution>
				<id>default</id>
				<goals>
					<goal>build</goal>
				</goals>
			</execution>
		</executions>
		<configuration>
			<repository>${docker.user}/${project.artifactId}</repository>
			<tag>${project.version}</tag>
			<buildArgs>
				<JAR_FILE>${project.build.finalName}.jar</JAR_FILE>
			</buildArgs>
		</configuration>
	</plugin>

each of my modules import this configuration using the following directive:

	<profiles>
		<profile>
			<id>docker</id>
			<activation>
				<property>
					<name>docker</name>
					<value>build</value>
				</property>
			</activation>
			<build>
				<plugins>
					<plugin>
						<groupId>com.spotify</groupId>
						<artifactId>dockerfile-maven-plugin</artifactId>
					</plugin>
				</plugins>
			</build>
		</profile>
	</profiles>	

With the command mvn clean package -DskipTests -P docker -Ddocker.user=thiagolocatelli project builds successfully and my docker images are created, however when I use the command mvnd clean package -DskipTests -P docker -Ddocker.user=thiagolocatelli the project fail with the following error messages:

[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for homeflix-parent 1.0.0-SNAPSHOT:
[INFO]
[INFO] homeflix-parent .................................... SUCCESS [  0.182 s]
[INFO] user-service ....................................... FAILURE [ 10.839 s]
[INFO] rental-service ..................................... FAILURE [ 10.839 s]
[INFO] cart-service ....................................... FAILURE [ 10.840 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  15.523 s (Wall Clock)
[INFO] Finished at: 2020-10-26T10:20:39-04:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal com.spotify:dockerfile-maven-plugin:1.4.13:build (default) on project user-service: Repo name "${docker.user}/user-service" must contain only lowercase, numbers, '-', '_' or '.'. -> [Help 1]
[ERROR] Failed to execute goal com.spotify:dockerfile-maven-plugin:1.4.13:build (default) on project rental-service: Repo name "${docker.user}/rental-service" must contain only lowercase, numbers, '-', '_' or '.'. -> [Help 1]
[ERROR] Failed to execute goal com.spotify:dockerfile-maven-plugin:1.4.13:build (default) on project cart-service: Repo name "${docker.user}/cart-service" must contain only lowercase, numbers, '-', '_' or '.'. -> [Help 1]

Not sure this is a bug related to mvnd but it is only happening with mvnd on my local environment. The same error happen with other projects as well.

@ppalaga
Copy link
Contributor

ppalaga commented Oct 26, 2020

Thanks for the report, @thiagolocatell. I guess we need to set the properties in the Daemon JVM programmatically when starting a build.

@ppalaga ppalaga self-assigned this Oct 26, 2020
@ppalaga ppalaga changed the title Error replacing variable with dockerfile-maven-plugin mvnd native executable is not passing -Dkey=val to the daemon Oct 27, 2020
@ppalaga
Copy link
Contributor

ppalaga commented Oct 27, 2020

Debugging a bit, it turns out that -Dkey=val style args are not passed to the main(String[] argv) method, while -Dkey are. This also explains why -DskipTests works and -Ddocker.user=thiagolocatelli does not.

@gnodet
Copy link
Contributor

gnodet commented Oct 27, 2020

Debugging a bit, it turns out that -Dkey=val style args are not passed to the main(String[] argv) method, while -Dkey are. This also explains why -DskipTests works and -Ddocker.user=thiagolocatelli does not.

Ah, I was suspecting something that. I think I've been hit by that already but had no time to investigate yet.

@thiagolocatelli
Copy link
Author

thiagolocatelli commented Oct 27, 2020

just wrote this simple example, -Dkey=val and -Dkey are indeed available in the argv array in the main method if they are used as application arguments, but not if they are used as JVM arguments (aka system properties).

class Main {
  public static void main(String[] args) {
    System.out.println("Command-Line arguments are");

    // loop through all arguments
    for(String str: args) {
      System.out.println(str);
    }
  }
}

execution:

[thiago@dropzone ~] java -DskipTests -Ddocker.user=thiagolocatelli Main clean package
Command-Line arguments are
clean
package

[thiago@dropzone ~] java Main clean package -DskipTests  -Ddocker.user=thiagolocatelli
Command-Line arguments are
clean
package
-DskipTests
-Ddocker.user=thiagolocatelli

@ppalaga
Copy link
Contributor

ppalaga commented Oct 27, 2020

Yeah, @thiagolocatelli this is how stock java works. mvnd is a native executable built with GraalVM and apparently, its behavior is slightly different here. Note that with mvnd the ordering of the args plays no role.

@thiagolocatelli
Copy link
Author

thiagolocatelli commented Oct 27, 2020

@ppalaga understood. Should the parameters then somehow be parsed in the client to identify which ones are system properties format (-Dkey=val) and then convert them to system properties when sending the execution to the daemon? Would this be a valid approach?

@ppalaga
Copy link
Contributor

ppalaga commented Oct 27, 2020

It is not easy. We have to figure out.

@ppalaga
Copy link
Contributor

ppalaga commented Oct 27, 2020

-H:-ParseRuntimeOptions looks very promissing oracle/graal#779 (comment)

@thiagolocatelli
Copy link
Author

@thiagolocatelli
Copy link
Author

Thank you for the fix @ppalaga , once the new build is available on sdkman I will pull it and test it here.

@ppalaga
Copy link
Contributor

ppalaga commented Oct 27, 2020

You can also test with the zip attached to the build of the PR https://github.com/mvndaemon/mvnd/pull/159/checks - it is under Artifacts in the top right corner

@ppalaga
Copy link
Contributor

ppalaga commented Oct 27, 2020

Thanks for the report, this is a very good catch!

@thiagolocatelli
Copy link
Author

thiagolocatelli commented Oct 27, 2020

ok, I ran using the shell script

Building gracenote-parent  threads: 7  time: 20s  progress: 5/8 62%
:gracenote-csv-exporter-605:com.spotify:dockerfile-maven-plugin:1.4.13:build {execution: default}
:gracenote-history-loader-605:com.spotify:dockerfile-maven-plugin:1.4.13:build {execution: default}
:gracenote-data-loader-605:com.spotify:dockerfile-maven-plugin:1.4.13:build {execution: default}

it worked but I guess the shell script is calling java directly and not using the native binary, which I can not run at the time.

@thiagolocatelli
Copy link
Author

thiagolocatelli commented Oct 27, 2020

@ppalaga I was able to run the native binary and it worked. Maven build successful and docker images created. thank you for your fix.

@ppalaga ppalaga added this to the 0.0.11 milestone Oct 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants