Allow making an executable-jar image by only providing an executable JAR

[sap-ali]

In this PR, I'm enabling the capability to only provide a standalone executable JAR without the need to have a file in META-INF/MANIFEST.MF

[Fixes #264]

[linux-foundation-easycla[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: sap-ali / name: Ali Rabiee (824b6a9b9c0f730b33a1671833a70a459440ee68, b1ab7991de346845a724d8276045e0b3972da59f, 40ed839c161ddbe59b1a2071f41e1b6a2554c9e8)

[dmikusa]

Can you expand more on the use case here? I saw you other issue, but it's not clear to me exactly what you're trying to do and why. Can you provide some more details? A working reproduction including code/steps to produce a JAR file that fits what you're trying to do would be helpful.

[sap-ali]

Can you expand more on the use case here? I saw you other issue, but it's not clear to me exactly what you're trying to do and why. Can you provide some more details? A working reproduction including code/steps to produce a JAR file that fits what you're trying to do would be helpful.

Let's say we have build a fat JAR from our project: example-fat.jar.

We can apply the buildpack to create an image that runs the jar using:

pack build example:latest \
  --path example-fat.jar
  --builder paketobuildpacks/builder:base \
  --buildpack gcr.io/paketo-buildpacks/sap-machine \
  --buildpack gcr.io/paketo-buildpacks/syft \
  --buildpack gcr.io/paketo-buildpacks/executable-jar

However, this has the downside that the JAR is exploded in the image, which can cause some side-effects (as it has in our case using Quarkus).

Specifying --path . would also not work because there is no META-INF/MANIFEST.MF in the path.

Let's say we create such file identical to what already is there in the jar.

Now, we can build an image by running:

pack build example:latest \
  --path .
  --builder paketobuildpacks/builder:base \
  --buildpack gcr.io/paketo-buildpacks/sap-machine \
  --buildpack gcr.io/paketo-buildpacks/syft \
  --buildpack gcr.io/paketo-buildpacks/executable-jar

In this case, the buildpack is participating (because the META-INF/MANIFEST.MF exists) and can find the JAR by searching in the path. So, it can create a docker image in our desired way; an image with the exact jar that we had built before, not exploded or modified in any way.

However, now when we try to run the image we get a Class not found error for the defined entry point in the META-INF/MANIFEST.MF.

To fix this, first, we need to get rid of the requirement for a META-INF/MANIFEST.MF when we have a stand-alone JAR file that can run on its own. The rest of the code already handles the case very well. It is only in the detection phase, when it does not let the buildpack proceed without a manual META-INF/MANIFEST.MF.

With the proposed changes, when there is a stand-alone JAR in the path, the buildpack can find and place that JAR in the image without requiring a separate META-INF/MANIFEST.MF and without requiring to explode the JAR, and the produced image runs perfectly.

[sap-ali]

@dmikusa I have gathered the following shell-script that show cases the use-case where the buildpack is not working as expected without the proposed changes:

#!/usr/bin/env bash
gh repo clone jreleaser/helloworld-java-jar
cd helloworld-java-jar
./mvnw verify
pack build helloworld-java:latest \
  --path target/ \
  --builder paketobuildpacks/builder:base \
  --buildpack gcr.io/paketo-buildpacks/sap-machine \
  --buildpack gcr.io/paketo-buildpacks/syft \
  --buildpack gcr.io/paketo-buildpacks/executable-jar &&
docker run --rm helloworld-java:latest
cd -

[dmikusa]

Sorry for the delay. I got a chance to take a look at this and what you're doing with the target isn't really something that's supported presently.

pack build helloworld-java:latest \
  --path target/ 

You're pointing it to a whole folder of stuff and expecting the buildpack to pick through all that. The Java buildpacks expect one of two things: a.) an exploded JAR file that you've compiled in advance b.) Java source code that it can build.

If you take the demo app here and give it either of those things, then it produces a functional app image.

I think what you're proposing here is basically a fix for https://github.com/paketo-buildpacks/executable-jar/issues/132, and if I'm remembering this one correctly it came out of the work with Quarkus. You're hitting it in a slightly different use case, but it amounts to the same thing. You want to be able to set a path with pack build to a folder of one or more JAR files (in your case it's one, in the #132, it's multiple).

How it's implemented really isn't that different, with the exception that your PR doesn't handle the case where other JARs (or resources) in the folder need to be on the classpath. That works in your case, cause you have only the one FAT jar, but I believe Quarkus' "fast-jar" format has the executable JAR plus some other JARs that need to be on the classpath. In that case, this PR wouldn't work.

Would you be interested in submitting code changes to support that use case as well? (i.e. both #132 and #264)

There is a lot of testing and verification that's required for a change like this because it impacts the build plan and there are a lot of different scenarios to cover. It would be helpful if we could do it all at once.

[sap-ali]

@dmikusa Thanks for getting back to me. I would be happy to help with the other case, but since I don't have a good overview on all the possible cases that you mentioned, maybe we could somehow work on it together?

For example, you could write down all the tests that you think are necessary, and then I could see how I can implement the required functionality, or alternatively we could also schedule a 1h pairing session. What do you think?

[pbusko]

You're pointing it to a whole folder of stuff and expecting the buildpack to pick through all that. The Java buildpacks expect one of two things: a.) an exploded JAR file that you've compiled in advance b.) Java source code that it can build.

I think what you're proposing here is basically a fix for https://github.com/paketo-buildpacks/executable-jar/issues/132, and if I'm remembering this one correctly it came out of the work with Quarkus. You're hitting it in a slightly different use case, but it amounts to the same thing. You want to be able to set a path with pack build to a folder of one or more JAR files (in your case it's one, in the https://github.com/paketo-buildpacks/executable-jar/issues/132, it's multiple).

@dmikusa At the moment there's basically no way to build an image using executable jar and not exploding it. The use-case should be supported, given the following in the readme:

This buildpack will participate if all the following conditions are met:

• /**/*.jar exists and that JAR has a /META-INF/MANIFEST.MF file which contains a Main-Class entry

and

$BP_EXECUTABLE_JAR_LOCATION | An optional glob to specify the JAR used as an entrypoint. Defaults to "", which causes the buildpack to do a breadth-first search for the first executable JAR it finds.

https://github.com/paketo-buildpacks/executable-jar/blob/bba2e76c45e18e40d76181c40ba3682a91e9d12f/executable/build.go#L83-L87

The following is not working:

$ tree .
.
└── build
    └── runner.jar

2 directories, 1 file

$ pack build --verbose --trust-builder --builder paketobuildpacks/builder-jammy-base -b gcr.io/paketo-buildpacks/bellsoft-liberica -b gcr.io/paketo-buildpacks/syft -b gcr.io/paketo-buildpacks/executable-jar -e 'BP_EXECUTABLE_JAR_LOCATION=build/runner.jar' my-executable-jar

This scenario fails because the executable-jar buildpack completely ignores the BP_EXECUTABLE_JAR_LOCATION during detection and only reads it during build, which I would consider a bug in the current implementation.

[dmikusa]

@pbusko Yes, what you're referencing is exactly why #132 is needed. Those settings only apply when building from source code because detect hasn't been updated to support multiple JARs.

To be clear, I'd 100% like to see #132 implemented (I think it addresses #264 as well). This PR needs a little more though, see my previous notes.

[pbusko]

How it's implemented really isn't that different, with the exception that your PR doesn't handle the case where other JARs (or resources) in the folder need to be on the classpath. That works in your case, cause you have only the one FAT jar, but I believe Quarkus' "fast-jar" format has the executable JAR plus some other JARs that need to be on the classpath. In that case, this PR wouldn't work.

Do you have an example at hand? I just tried with Quarkus fatjar and the scenario described in https://github.com/paketo-buildpacks/executable-jar/pull/265#issuecomment-2108043124 and everything works as expected.

Also the https://github.com/paketo-buildpacks/executable-jar/issues/132 states:

This enhancement request is to have detect also look at the present workspace and see if either a.) the workspace itself has a manifest or if b.) it can identify a single executable JAR file in the workspace.

Which is exactly what this PR does in my opinion.

[c0d1ngm0nk3y]

Which is exactly what this PR does in my opinion.

I agree that this pr looks good as it is. Imho it is rather a bugfix than a enhancement. The build logic handled the case well (by adding -jar), but it could never be detected.

Also this supports the most simplest use case - only a single executable jar present.

[c0d1ngm0nk3y]

@paketo-buildpacks/java-maintainers This fixes the use case that gives the buildpack its name (#264). This does not yet fix #132 , but imho it is a valuable improvement over the current state. Or do you see something missing?

[loewenstein]

@paketo-buildpacks/java-maintainers This fixes the use case that gives the buildpack its name (#264). This does not yet fix #132 , but imho it is a valuable improvement over the current state. Or do you see something missing?

@paketo-buildpacks/java-maintainers Would you mind to have a look?

[loewenstein]

@anthonydahanne & @pivotal-david-osullivan what's your take on this? I'd be much in favor of getting this merged as is. It's fixing a bug and that should not be coupled to feature development like #132 imho.

[anthonydahanne]

what's your take on this? I'd be much in favor of getting this merged as is. It's fixing a bug and that should not be coupled to feature development like https://github.com/paketo-buildpacks/executable-jar/issues/132 imho.

Hello! merging this contribution would require to run a decent number of tests; such as all the Java samples; and make sure the detection and plan are working properly for all use cases. I'd very much prefer to perform this effort after https://github.com/paketo-buildpacks/executable-jar/issues/132 is also fixed / included in the fix so that we squash 2 related issues during 1 big regression testing session

[c0d1ngm0nk3y]

How can we continue with that? @dmikusa Could you give some details on the buildplan changes you see critical? Actually, there is no change in the buildplan at all, right, "Only" one additional case is detected. And fwiw, the case that gives the buildpack its name (it is not called "exploded executable JAR").

For me, this is a clear bugfix, since the behavior was already documented and even implemented in build. There was a bug in the detect. I think when we do not feel comfortable in merging bugfixes since they might have side effects, it is a clear indication that some tests are missing and I would like to better understand what is seen as critical here.

Imho, this is a good contribution to enable an additional use case.

[pbusko]

The integration tests from https://github.com/paketo-buildpacks/java passed successfully with the change from this PR. @dmikusa is there anything else blocking this change from being merged?

[loewenstein]

cc @paketo-buildpacks/java-maintainers

[loewenstein-sap]

@anthonydahanne WDYT, ready to merge? If not, what do you think is still missing?

[loewenstein-sap]

@paketo-buildpacks/java-maintainers If I recall correctly, the only concern left with this was its effect on the build plan and the lack of coverage in tests for the kind of problems that can occur in case of changes to the build plan. With https://github.com/paketo-buildpacks/java/pull/1399 being merged, is there anything left to do or could we get this approved and merged?

[loewenstein-sap]

@paketo-buildpacks/java-maintainers If I recall correctly, the only concern left with this was its effect on the build plan and the lack of coverage in tests for the kind of problems that can occur in case of changes to the build plan. With paketo-buildpacks/java#1399 being merged, is there anything left to do or could we get this approved and merged?

@paketo-buildpacks/java-maintainers Would be nice to get a comment on that. Can we merge it or what is left to do before we can?

[anthonydahanne]

rebasing on top of main

[anthonydahanne]

thanks for your patience all! detection of executable jar is very sensitive, hopefully everything will be smooth and it will expand use cases!