-
Notifications
You must be signed in to change notification settings - Fork 41.5k
Invalid variable reference in maven-shade-plugin configuration #20143
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
Invalid variable reference in maven-shade-plugin configuration #20143
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the follow-up. It's my mistake not properly retesting the outcome. ${project.version}
looked legit to me and I missed the context of the plugin dependency.
Unfortunately, what we need there is to expand the token rather than writing it as is. Your proposal on an empty project leads to:
[WARNING]
[WARNING] Some problems were encountered while building the effective model for com.example:demo:jar:0.0.1-SNAPSHOT
[WARNING] The expression ${version} is deprecated. Please use ${project.version} instead.
[WARNING]
[WARNING] It is highly recommended to fix these problems because they threaten the stability of your build.
[WARNING]
[WARNING] For this reason, future Maven versions might no longer support building such malformed projects.
[WARNING]
and
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-shade-plugin:3.2.1:shade (default-cli) on project demo: Execution default-cli of goal org.apache.maven.plugins:maven-shade-plugin:3.2.1:shade failed: Plugin org.apache.maven.plugins:maven-shade-plugin:3.2.1 or one of its dependencies could not be resolved: Could not find artifact org.springframework.boot:spring-boot-maven-plugin:jar:0.0.1-SNAPSHOT in spring-milestones (https://repo.spring.io/milestone)
I don't know yet what we need to add there to expand the version but I'll investigate tomorrow if nobody else is beating me to it.
@@ -191,7 +191,7 @@ publishing.publications.withType(MavenPublication) { | |||
dependency { | |||
delegate.groupId('org.springframework.boot') | |||
delegate.artifactId('spring-boot-maven-plugin') | |||
delegate.version('${project.version}') | |||
delegate.version('${version}') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gradle publishToMavenLocal
generates the following for me:
<dependencies>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-maven-plugin</artifactId>
<version>${version}</version>
</dependency>
</dependencies>
I believe it should generate the following:
<dependencies>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-maven-plugin</artifactId>
<version>2.3.0.BUILD-SNAPSHOT</version>
</dependency>
</dependencies>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following will work:
delegate.version("${project.version}")
This aligns with how the parent version is configured earlier in the same build.gradle
file:
parent {
delegate.groupId("${project.group}")
delegate.artifactId("spring-boot-dependencies")
delegate.version("${project.version}")
}
The change can be verified with the following command:
./gradlew spring-boot-project:spring-boot-starters:spring-boot-starter-parent:generatePom
This will write the generated pom to spring-boot-project/spring-boot-starters/spring-boot-starter-parent/build/publications/maven/pom-default.xml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wilkinsona If I'm understanding you correctly does that mean this PR is simply not needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I think it's still needed, but in a modified form. We currently have '${project.version}'
. We need "${project.version}"
instead. Note the use of "
rather than '
.
@@ -191,7 +191,7 @@ publishing.publications.withType(MavenPublication) { | |||
dependency { | |||
delegate.groupId('org.springframework.boot') | |||
delegate.artifactId('spring-boot-maven-plugin') | |||
delegate.version('${project.version}') | |||
delegate.version('${version}') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following will work:
delegate.version("${project.version}")
This aligns with how the parent version is configured earlier in the same build.gradle
file:
parent {
delegate.groupId("${project.group}")
delegate.artifactId("spring-boot-dependencies")
delegate.version("${project.version}")
}
The change can be verified with the following command:
./gradlew spring-boot-project:spring-boot-starters:spring-boot-starter-parent:generatePom
This will write the generated pom to spring-boot-project/spring-boot-starters/spring-boot-starter-parent/build/publications/maven/pom-default.xml
@scheerer thanks for letting us know about the issue. I took the liberty of applying Andy's suggestion myself as we are releasing M2 today and I'd like to make sure the starter has a valid POM. |
This should fix my prior PR #20052. I am pretty sure this is correct, but sadly I seem to be unable to fully build the project to verify for sure. Might need a couple eyes from build masters to ensure. I am so sorry for the bad fix in #20052.
History:
${revision}
${project.version}
2.3.0.BUILD-SNAPSHOT
build with my change I discovered ${project.version} always resolves to0.0.1-SNAPSHOT
.${version}
for the current SpringBoot build version. So I used that. :)In order to know for sure a simple child project inheriting from the spring-boot-starter-parent 2.3.0.BUILD-SNAPSHOT must use the
shade
plugin like so:Then simply execute
./mvnw package
to see the error.