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

Add graalvm versions to grails-bom, will be used in providedRuntime #13971

Merged
merged 2 commits into from
Jan 17, 2025

Conversation

jamesfredley
Copy link
Contributor

@jamesfredley jamesfredley commented Jan 14, 2025

to exclude jars from war and reduce size significantly.

This is the first part of the following solution to reduce the war file size when using asset-pipeline by excluding the graalvm jars from the war using Gradle's War Plugin providedRuntime configuration scope. Asset-pipeline-core requires the graalvm jars and some transitive dependencies during compile, test and bootRun, but not in the war. Since they are needed during bootRun and Asset-pipeline-core is used in the non-grails Asset-pipeline distributions, using the war plugin scope is a direct path to solve this particular situation.
https://docs.gradle.org/current/userguide/war_plugin.html#sec:war_dependency_management

jamesfredley/asset-pipeline-small-jar-testing@426cd61#diff-49a96e7eea8a94af862798a45174e6ac43eb4f8b4bd40759b5da63ba31ec3ef7R68

…o exclude jars from war to reduce size significantly.
@jdaugherty
Copy link
Contributor

What about the jar file? Is the target to only exclude these from the war?

@jamesfredley
Copy link
Contributor Author

@jamesfredley
Copy link
Contributor Author

@jdaugherty between the 3 graalvm dependencies there are are few transitive dependencies. They are included in 7.0.0-M1 currently, since asset-pipeline-core 5.0.5 provides them. The version of graalvm-sdk/js specified in this PR is newer then the version provided by asset-pipeline-core. https://repo1.maven.org/maven2/com/bertramlabs/plugins/asset-pipeline-core/5.0.5/asset-pipeline-core-5.0.5.pom

Yes, the goal is the exclude these 3 and their transitive dependencies from the war, since it increases the base war size from ~85MB to > 140MB.

I also built an example project against asset-pipeline 5.0.4, which only exposes graal-sdk as a compile dependency https://repo1.maven.org/maven2/com/bertramlabs/plugins/asset-pipeline-core/5.0.4/asset-pipeline-core-5.0.4.pom using developmentOnly: jamesfredley/asset-pipeline-small-jar-testing@ae30390

Of those two, I prefer the version using providedRuntime, since asset-pipeline-core needs the three graal-sdk/js dependencies for the runtime bootRun, but not war execution and providedRuntime is the Gradle War Plugin's mechanism for excluding dependencies from the war. The root issue: #13913 (comment). Both approaches would benefit from this PR so that the graalvm versions live in grails-bom instead of the end application.

@jdaugherty
Copy link
Contributor

What about the forge cli project - won't this cause conflicts? It uses graal and will likely be on the latest going forward / a different version than the asset pipeline?

@jamesfredley
Copy link
Contributor Author

I don't think forge CLI will be an issue, since it does not run the generated application, it just generates the files.

@codeconsole
Copy link
Contributor

Shouldn't these versions really be managed by asset pipeline? Aren't non-grails apps also affected by the same dependency resolution?

It also seems to be environment specific? I have yet to experience this exception on any of my apps and I have been running asset pipeline 5.0.4. Seems overkill to add something for everyone that doesn't affect everyone.

# Conflicts:
#	grails-bom/build.gradle
@jamesfredley
Copy link
Contributor Author

@codeconsole If you want to try to eliminate the need for these dependencies on bootRun, #13913 (comment) has the list of where to look.

I am proceeding with the solution which reduces the war size against 5.0.5 for now and moving on to other priorities.

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 this pull request may close these issues.

3 participants