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

fix/#979 fix springboot running plain jar instead of executable jar #980

Conversation

fattila16
Copy link
Contributor

@fattila16 fattila16 commented Sep 25, 2023

This PR solves the following issue: #979

Disclaimer: This drops support of any spring version that does not have a bootJar task, mainly spring v1

  • Adjust the JavaProvider so that it uses bootJar to build an executable JAR instead of build which generates a *plain.jar also when spring and gradle is detected
  • Add e2e test cases for spring v2 and v3, also remove spring v1
  • Update the docs of the JavaProvider so that it has information regarding the spring version support, and the build commands

As per the comments below PR implements a different solution now:

  • Adjust the JavaProvider so that if gradle and spring is detected then the start cmd includes a one line to find the executable jar and blacklists the plain.jar
  • Add e2e test cases for spring v1, v2 and v3
  • Update the docs of the JavaProvider so that it includes description for the new start cmd if gradle and spring is detected
  • Unit tests are added
  • E2E tests are added
  • Docs are updated

@@ -95,11 +96,20 @@ impl JavaProvider {
}
}

fn get_gradle_build_cmd(&self, build_gradle_content: &String, gradle_exe: &String) -> String {
if self.is_using_spring_boot(build_gradle_content) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to detect if Spring v1 is used and only use bootJar if it is version 2+?

Copy link
Contributor Author

@fattila16 fattila16 Sep 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One can read it from the build.gradle file, however I would not go that way. There can be edge cases, where a gradle plugin update or a spring version update changes the versioning scheme. Also one can define it in kotlin and groovy. That makes capturing this a little bit finnicky and I would not want to rely on a bunch of regexp expressions.

If nixpacks would like to support spring v1 as well, there is an alternative solution to what I've provided in this PR. Which is the following:

  • The problem is that a *plain.jar is being generated along with an executable .jar
  • The start command can point to the *plain.jar instead of the executable
  • As a solution one can adjust the start command with the following one-liner:
    java $JAVA_OPTS -jar -Dserver.port=$PORT $(ls -1 build/libs/*jar | grep -v plain)
  • So basically instead of configuring gradle to create only an executable jar, we use ls and grep to find the executable jar instead of the plain one
  • This way it doesn't matter what spring or gradle version is being used, the only thing that matters is that the jars are being generated in the build/libs folder and there is only one executable jar

What do you think about this approach?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that solution works! And yeah, supporting v1 isn't 100% necessary, but backwards compatibility is always nice, especially if the solution is as simple as greping for the executable jar

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the PR with the new solution

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible for me to get a review here? And then I can check whether all the pipelines are green or not. Thank you!

@fattila16 fattila16 force-pushed the fix/979-fix-springboot-running-plain-jar-instead-of-executable-jar branch 6 times, most recently from 7440d78 to f84dea6 Compare September 30, 2023 14:18
@fattila16 fattila16 force-pushed the fix/979-fix-springboot-running-plain-jar-instead-of-executable-jar branch from c451919 to 1235ef6 Compare October 17, 2023 10:38
fattila16 and others added 3 commits October 18, 2023 09:40
…cutable

jar file if gradle is used, and blacklists plain.jar files

Co-authored-by: Tibor Toth <toth.t11@gmail.com>
be sure that the adjusted start cmd is backwards compatible

Co-authored-by: Tibor Toth <toth.t11@gmail.com>
…art cmd for gradle

Co-authored-by: Tibor Toth <toth.t11@gmail.com>
@fattila16 fattila16 force-pushed the fix/979-fix-springboot-running-plain-jar-instead-of-executable-jar branch from 1235ef6 to de94abe Compare October 18, 2023 07:40
@coffee-cup coffee-cup merged commit fdad81f into railwayapp:main Oct 19, 2023
95 checks passed
@coffee-cup
Copy link
Contributor

Thank you 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release/minor Author minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants