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

exclude org.graalvm dependencies from the deployable war/jar #503

Merged
merged 2 commits into from
Jan 29, 2025

Conversation

jamesfredley
Copy link
Contributor

exclude org.graalvm dependencies which are required by asset-pipeline-core for bootRun but are not required in the deployed war/jar

exclude org.graalvm dependencies which are required by asset-pipeline-core for bootRun but are not required in the deployed war/jar
Comment on lines 75 to 82
// exclude org.graalvm dependencies which are required by asset-pipeline-core
// for bootRun but are not required in the deployed war/jar
tasks.named { it in ['bootWar', 'bootJar', 'war'] }.configureEach {
classpath = classpath.findAll {
!it.toString().contains('org.graalvm.')
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Should this block only be included on condition that the asset-pipeline-grails feature is active?

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we put this in the gradle plugin?

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we put this in the gradle plugin?

I also thought about that 😄
But, then we are hiding stuff that will be hard to understand/debug for the end user, if it becomes a problem.
Better to keep it in sight and remove it when we find a better solution.
Maybe include a link to the issue in the comment?

Copy link
Contributor

Choose a reason for hiding this comment

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

We could always add a feature flag for opt out. Documentation for the gradle plugin is how we avoid hiding this functionality and it's probably important to keep the end app size small. I imagine this is one of those enhancements that can result in a significant cost savings for people that run in cloud environments. We know the dependencies are only required in dev/locally, so I would push for it to be in the gradle plugin and we can add documentation about it so its known.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I agree, it would definitely be nicer to hide it in the Gradle Plugin 👍

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 agree on making it conditioned on the asset-pipeline-grails feature being active and will add a link to one of the tickets noting the file size difference.

@jamesfredley jamesfredley merged commit 16f50d8 into 7.0.x Jan 29, 2025
9 checks passed
@jamesfredley jamesfredley deleted the exclude-org.graalvm-dependencies branch January 29, 2025 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants