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

Provide GraalVM metadata repository support with Buildpacks #31782

Closed
5 of 6 tasks
sdeleuze opened this issue Jul 18, 2022 · 22 comments
Closed
5 of 6 tasks

Provide GraalVM metadata repository support with Buildpacks #31782

sdeleuze opened this issue Jul 18, 2022 · 22 comments
Assignees
Labels
status: superseded An issue that has been superseded by another theme: aot An issue related to Ahead-of-time processing type: enhancement A general enhancement

Comments

@sdeleuze
Copy link
Contributor

sdeleuze commented Jul 18, 2022

(edit)

Splitting into sub-tasks:

External issues:


As discussed with @dmikusa-pivotal, @mhalbritter and @scottfrederick, we need to integrate GraalVM metadata repository in Buildpacks in addition to Native Build Tools current support (see #31687) in order to allow compiling Spring Boot application with third-party dependencies to a native executable. 2 implementations have been discussed.

Based on our discussions, integration at Spring Boot Maven and Gradle plugin AOT generation level is the recommended way to implement such integration because:

  • It allows a single and consistent way to provide third-party native configuration for Spring Boot applications
  • It can leverage graalvm-reachability-metadata Java library and its GraalVMReachabilityMetadataRepository class maintained by GraalVM team.
  • Reference implementation in NBT Gradle and Maven plugins can help us for the Spring Boot implementation.
  • Conceptually a good fit to put third party hints along to Spring ones
  • Better DevXP (all the JSON hints generated at the same location at the same time)
  • Better build reproducibility
  • Gives more control to Spring Boot on the reachability metadata repository release a certain version of Spring Boot uses

The alternative way is to have 2 distinct implementations : the one at NBT plugins level and another at Buildpacks level, but the lack of consistency, poorer DevXP, the fact that we would likely have to reimplement graalvm-reachability-metadata in Go and maintaining it seems to indicate this should not be the way to move forward.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 18, 2022
@philwebb philwebb added the for: team-meeting An issue we'd like to discuss as a team to make progress label Jul 18, 2022
@mhalbritter mhalbritter added the theme: aot An issue related to Ahead-of-time processing label Jul 19, 2022
@wilkinsona
Copy link
Member

On the Gradle side, we may be able to use GraalVMReachabilityMetadataService which is a build service that implements GraalVMReachabilityMetadataRepository.

@snicoll
Copy link
Member

snicoll commented Aug 11, 2022

On the Maven Side, things aren't as easy to reuse I am afraid. We've discussed this and we think that extracting that capability in a dedicated goal of the Native Build Plugins would be a nice option. We don't want to redefine the attributes that this feature need in our own plugin and rather add an extra execution of an existing goal in our parent.

For instance, the existing compile goal could have an additional attribute that points to the directory where the metadata has been processed. And the new goal would write to this directory, supporting the existing attributes that the unique goal has at the moment.

@wilkinsona
Copy link
Member

I've been thinking about this some more, and I'm starting to wonder if relying on capabilities of the native build tools' Gradle and Maven plugins is a good idea.

If buildpacks are creating the native image, will the user have even configured/applied the native build tools plugin? For those that haven't, I think it would feel quite strange to have to apply/configure the plugin locally solely for the purpose of providing reachability metadata to the buildpack.

I know it's more work on the buildpacks side, but it feels to me that if the buildpacks are creating the native image, they should be capable of retrieving and using the reachability metadata. We could provide GAV metadata to the buildpacks to facilitate retrieving the reachability metadata without requiring the user to configure anything.

@sdeleuze
Copy link
Contributor Author

sdeleuze commented Aug 16, 2022

I've been thinking about this some more, and I'm starting to wonder if relying on capabilities of the native build tools' Gradle and Maven plugins is a good idea.

I think the right level of reuse is the graalvm-reachability-metadata dependency. Notice that if you feel GraalVMReachabilityMetadataRepository is not enough and more could be in the shared logic, we can refactor Native Build Tools accordingly but I think we would need a concrete proposal for that.

I know it's more work on the buildpacks side, but it feels to me that if the buildpacks are creating the native image, they should be capable of retrieving and using the reachability metadata. We could provide GAV metadata to the buildpacks to facilitate retrieving the reachability metadata without requiring the user to configure anything.

One of the main issue discussed with @dmikusa-pivotal during our last meeting related to implementing that at Buildpack level is that it will be hard to leverage GraalVMReachabilityMetadataRepository via graalvm-reachability-metadata JVM dependency from Buildpacks infra which is in Go, at least that's what I remember.

I think any solution not based on graalvm-reachability-metadata will be a maintenance nightmare given the fact that the repository format will evolve.

@dmikusa
Copy link

dmikusa commented Aug 17, 2022

I know it's more work on the buildpacks side, but it feels to me that if the buildpacks are creating the native image, they should be capable of retrieving and using the reachability metadata. We could provide GAV metadata to the buildpacks to facilitate retrieving the reachability metadata without requiring the user to configure anything.

One of the main issue discussed with @dmikusa-pivotal during our last meeting related to implementing that at Buildpack level is that it will be hard to leverage GraalVMReachabilityMetadataRepository via graalvm-reachability-metadata JVM dependency from Buildpacks infra which is in Go, at least that's what I remember.

I think any solution not based on graalvm-reachability-metadata will be a maintenance nightmare given the fact that the repository format will evolve.

100%. The big challenge is that we can't reuse any of their library code. The only path forward would be if that project were to release a CLI that we could call from the buildpack, either java -jar foo.jar or a native-image binary. If we can invoke the logic from that repo through a CLI, that would be the first hurdle cleared.

The second hurdle is that buildpacks have to account for two scenarios: 1.) building from source and 2.) building from precompiled assets. I think we could make building from source work (assuming we have the CLI to call) but I'm very skeptical that we could make the second scenario, building from a precompiled asset, work. There's just a lack of information since all we'd have is a JAR file, and we cannot assume it's a Spring Boot JAR. The buildpack has to work with any kind of JAR.

This might seem like a weird use case, who's going to do half the build locally and then half with buildpacks, but that's exactly what the Spring Boot tools do, ./mvnw spring-boot:build-image generates the JAR locally and builds the native-image in the container.

@dmikusa
Copy link

dmikusa commented Aug 17, 2022

We could provide GAV metadata to the buildpacks to facilitate retrieving the reachability metadata without requiring the user to configure anything.

Can you expand on this? What type of information would this provide to buildpacks? Basically, just a full list of all the dependencies installed?

In particular, for the case where ./mvnw spring-boot:build-image so all the buildpack gets is a Spring Boot JAR file. How do you see this working? Something in /META-INF/?

@philwebb philwebb added the status: pending-design-work Needs design work before any code can be developed label Aug 22, 2022
@mhalbritter
Copy link
Contributor

mhalbritter commented Aug 23, 2022

My 2 cents: right now the NBT plugins (Maven & Gradle) download the metadata and invoke the native-image with it. When we add this logic into our plugins or into the buildpack, there's a third place where the repository logic is implemented.

My take would be to talk to the GraalVM team if the metadata repository support could be implemented in native-image itself. We could then invoke the native-image tool like this:

native-image -jar /path/to/spring-boot.jar --dependencies /file/which/lists/dependencies.json --metadata-version=0.1.1

and native-image would fetch the metadata needed for the given dependencies and build a native image with this. With this change the repository fetching could be removed from NBT maven and gradle plugins and we don't need to implement the repository fetching for every build tool out there.

This doesn't solve the problem how exactly the dependencies get passed into the build pack, but maybe we could put it under META-INF somewhere. Maybe even a location which native-image would read by default. This way you could build self-contained JAR files which are executable on the JVM and could then be transformed into a native-image without additional information.

What do you think?

@sdeleuze
Copy link
Contributor Author

sdeleuze commented Aug 23, 2022

We have already discussed this possibility with the GraalVM team in the past which has been refused, no hope to see this happening short/middle term.

@mhalbritter
Copy link
Contributor

mhalbritter commented Aug 23, 2022

If buildpacks are creating the native image, will the user have even configured/applied the native build tools plugin? For those that haven't, I think it would feel quite strange to have to apply/configure the plugin locally solely for the purpose of providing reachability metadata to the buildpack.

That popped into my mind, too. Right now, the org.graalvm.buildtools.native Gradle plugin serves as a trigger to apply our AOT plugin to generate all the AOT code. When the user is using buildpacks, they may not have the NBT plugin applied.

In terms of UX, what is the signal to apply AOT processing + native? gradle bootBuildImage -Pnative=true? gradle bootBuildNativeImage? My guess is that users want to have the option to build a "normal" OCI image and a native OCI image without fiddling with build.gradle.

@sdeleuze
Copy link
Contributor Author

+1, on Spring Native we have seen a higher than expected amount of users just using Buildpack support without Native Build Tools configured so that seems to be a strong use case.

@sdeleuze
Copy link
Contributor Author

sdeleuze commented Aug 30, 2022

In terms of UX, what is the signal to apply AOT processing + native? gradle bootBuildImage -Pnative=true? gradle bootBuildNativeImage? My guess is that users want to have the option to build a "normal" OCI image and a native OCI image without fiddling with build.gradle.

I also think it would help to get some clarity about the big picture to determine what will be "the signal" (Maven/Gradle build configuration and/or command line parameter) to build a native container (AOT transformations + native compilation + tiny container) versus a JVM based one in Spring Boot 3 GA via mvn spring-boot:build-image / gradle bootBuildImage.

@scottfrederick
Copy link
Contributor

scottfrederick commented Sep 16, 2022

#32408 will update the Gradle plugin to provide reachability metadata in the Spring Boot fat jar when built with Gradle. We'll leave this issue open to decide what to do with Maven.

@sdeleuze
Copy link
Contributor Author

sdeleuze commented Sep 20, 2022

We need to add support for the override flag and pass the related --exclude-config command to Buildpacks, see oracle/graalvm-reachability-metadata#9 and graalvm/native-build-tools#268 flags. I can potentially provide a PR for that.

@wilkinsona wilkinsona changed the title Provide GraalVM metadata repository support in Buildpacks Provide GraalVM metadata repository support in Buildpacks when using Maven Sep 30, 2022
@philwebb
Copy link
Member

philwebb commented Oct 3, 2022

I've submitted a PR to add a Maven goal and a Gradle task to NBT. If override is set for any of the metadata then a native-image.properties file is generated with the --exclude-config set in Args.

@sdeleuze
Copy link
Contributor Author

sdeleuze commented Oct 4, 2022

Thanks for working on that @philwebb.

I think the override support will need to be refined since as of GraalVM 22.2 it is not possible to use --exclude-config in native-image.properties anymore (before there were other limitations in term of parameter ordering), see oracle/graal#3749 (comment) related comment. I pushed strongly to remove this limitation but it is by design and we have to live with that.

The only way that I am aware to make it work is to get the exclude info from related Gradle/Maven properties and append that to the BP_NATIVE_IMAGE_BUILD_ARGUMENTS on Boot side to pass it to native-image arguments.

The best way to validate it works is to build and run a WebFlux based application and see if that works on Buildpack since it is using Netty 4.1 where --exclude-config is mandatory.

@philwebb
Copy link
Member

philwebb commented Oct 4, 2022

Oh great! I've no idea how we'd get the information out of the Gradle/Maven plugin about which modules need to be excluded. Perhaps for Maven we can stash something in a property that we can pick up later, but I think that might be considered bad practice.

@philwebb
Copy link
Member

philwebb commented Oct 5, 2022

I've updated the NBT PR so that if there's an override flag when the metadata is copied an additional reachability-metadata.properties is written containing override=true. I think this means that when we construct the fat jar we can check for that file and construct the appropriate pattern. For example, if we're adding the nested jar for io.netty:netty-buffer we'd look for an existing META-INF/native-image/io.netty/netty-buffer/reachability-metadata.properties file and if indicates an override we can add a pattern for .*/lib/netty-buffer-123.jar.

This is actually a better solution I think because it puts the pattern generation back in the hands of the thing that's packaging the fat jar.

The problem we have is that once we have those set of patterns, we've got to somehow pass them to the buildpack. We could update the BP_NATIVE_IMAGE_BUILD_ARGUMENTS variable if the buildpack is being triggered from our Maven or Gradle pluging, but I think might be better if we could somehow package those details inside the JAR. That way the JAR is the complete source of truth.

We have some prior art here with things like the classpath.idx files that we ship and the buildpack uses. Perhaps we could package a META-INF/native-image/bp-native-exclude-config file that the buildpack can use if it finds?

Even better would be if we could get --exclude-config to work again with Graal when loaded from META-INF/native-image/native-image.properties. That would mean the buildpack wouldn't need to do anything and creating an image from the fat jar directly would be pretty easy. @sdeleuze Would you be able to see if there's any appetite for that?

@sdeleuze
Copy link
Contributor Author

sdeleuze commented Oct 5, 2022

I can try to discuss that again with @olpaw and @vjovanov but I have little hope since last time I did I think the feedback was that limitation for no --exclude-config in META-INF/native-image/native-image.properties is by design and for good reasons due to constraints in argument processing by native-image.

But conceptually, what we try to achieve here is that Buildpacks automatically recognize a @argFile and passes it to native-image.

So maybe based on those reachability-metadata.properties files, Spring Boot plugin can generates a META-INF/native-image/bp-argfile with the --exclude-config that Buildpacks can recognize and pass via native-image @META-INF/native-image/bp-argfile .... I admit this whole "reachability-metadata.properties -> bp-argfile generation -> Buildpacks special handling" makes me a little bit nervous and looks pretty involved, but if we have no choice ...

Another solution would be to work with @pivotal-david-osullivan to directly generates the --exclude-config based on the reachability-metadata.properties at Buildpacks level. Not sure if that's doable or not.

@sdeleuze
Copy link
Contributor Author

sdeleuze commented Oct 5, 2022

@philwebb So my discussion with @vjovanov confirmed there is no hope for bringing back support for --exclude-config in META-INF/native-image/native-image.properties. So I think that means we should move forward with the solution described above.

@philwebb philwebb changed the title Provide GraalVM metadata repository support in Buildpacks when using Maven Provide GraalVM metadata repository support with Buildpacks Oct 6, 2022
@philwebb philwebb added this to the 3.0.x milestone Oct 6, 2022
@philwebb philwebb removed the status: pending-design-work Needs design work before any code can be developed label Oct 6, 2022
@philwebb philwebb added type: enhancement A general enhancement and removed for: team-meeting An issue we'd like to discuss as a team to make progress status: waiting-for-triage An issue we've not yet triaged labels Oct 6, 2022
@philwebb philwebb self-assigned this Oct 6, 2022
@philwebb
Copy link
Member

philwebb commented Oct 7, 2022

https://github.com/philwebb/spring-boot/tree/gh-31782 contains some prototype code that generates a META-INF/native-image/exclude-config-args file. This file contains lines that can be passed as --exclude-config arguments. I've opened paketo-buildpacks/native-image#196 to see how the buildpack team feel about the approach.

@philwebb philwebb added the status: blocked An issue that's blocked on an external project change label Oct 12, 2022
@sdeleuze
Copy link
Contributor Author

See my related comment on paketo-buildpacks/native-image#196 (comment) where Buidpack team would like to know if it is possible to directly generate the argfile to be passed to the native image compiler via native-image @ @/path/to/META-INF/native-image/argfile.

@philwebb
Copy link
Member

We've done all we can on our end. I'm going to close this one for now but we can reopen if the buildpack support PR isn't merged.

@philwebb philwebb closed this as not planned Won't fix, can't repro, duplicate, stale Oct 20, 2022
@philwebb philwebb added status: superseded An issue that has been superseded by another and removed status: blocked An issue that's blocked on an external project change labels Oct 20, 2022
@philwebb philwebb removed this from the 3.0.x milestone Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: superseded An issue that has been superseded by another theme: aot An issue related to Ahead-of-time processing type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

8 participants