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

Inject MANIFEST.MF also for source jar files #406

Merged
merged 1 commit into from
Mar 21, 2024

Conversation

jerboaa
Copy link
Collaborator

@jerboaa jerboaa commented Mar 20, 2024

For Quarkus productization source jars are expected to have META-INF/MANIFEST.MF files in them. Inject them when they're not already there like we do for the actual jar artefact.

Diff ignoring whitespace looks like this:

diff --git a/build.java b/build.java
index a8e5c16..be805b8 100644
--- a/build.java
+++ b/build.java
@@ -657,8 +657,12 @@ class SequentialBuild
     {
         return (artifact, paths) ->
         {
-            final Path jarPath = PathFinder.getFirstExisting(fs.mandrelRepo().resolve(paths[0]).toString(), artifact);
-            try (java.nio.file.FileSystem jarfs = FileSystems.newFileSystem(jarPath, this.getClass().getClassLoader()))
+            final String jarF = PathFinder.getFirstExisting(fs.mandrelRepo().resolve(paths[0]).toString(), artifact).toString();
+            final Path jarPath = Path.of(jarF);
+            final Path sourceJarPath = Path.of(jarF.replace(".jar", ".src.zip"));
+            for (Path jar: List.of( jarPath, sourceJarPath ))
+            {
+                try (java.nio.file.FileSystem jarfs = FileSystems.newFileSystem(jar, this.getClass().getClassLoader()))
                 {
                     Path metaInfPath = jarfs.getPath("/META-INF");
                     Path manifestPath = metaInfPath.resolve("MANIFEST.MF");
@@ -676,6 +680,7 @@ class SequentialBuild
                 {
                     throw new RuntimeException(e);
                 }
+            }
         };
     }
 

For Quarkus productization source jars are expected to have
META-INF/MANIFEST.MF files in them. Inject them when they're
not already there like we do for the actual jar artefact.
@jerboaa jerboaa requested a review from zakkak March 20, 2024 16:38
@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Mar 20, 2024
@jerboaa
Copy link
Collaborator Author

jerboaa commented Mar 20, 2024

I've tested this locally with --maven-deploy-local.

Before this patch (no manifests in source jars):

$ for i in word collections nativeimage; do echo "----------------------- $i ------------------------"; echo $i.jar=$(jar -tf mandrel-build/lib/jvmci/$i.jar | grep MANIFEST); echo $i.src.zip=$(jar -tf mandrel-build/lib/jvmci/$i.src.zip | grep MANIFEST); done
----------------------- word ------------------------
word.jar=META-INF/MANIFEST.MF
word.src.zip=
----------------------- collections ------------------------
collections.jar=META-INF/MANIFEST.MF
collections.src.zip=
----------------------- nativeimage ------------------------
nativeimage.jar=META-INF/MANIFEST.MF
nativeimage.src.zip=

After this patch (manifests in both, the regular jars and the source jar):

$ for i in word collections nativeimage; do echo "----------------------- $i ------------------------"; echo $i.jar=$(jar -tf mandrel-build/lib/jvmci/$i.jar | grep MANIFEST); echo $i.src.zip=$(jar -tf mandrel-build/lib/jvmci/$i.src.zip | grep MANIFEST); done
----------------------- word ------------------------
word.jar=META-INF/MANIFEST.MF
word.src.zip=META-INF/MANIFEST.MF
----------------------- collections ------------------------
collections.jar=META-INF/MANIFEST.MF
collections.src.zip=META-INF/MANIFEST.MF
----------------------- nativeimage ------------------------
nativeimage.jar=META-INF/MANIFEST.MF
nativeimage.src.zip=META-INF/MANIFEST.MF

@jerboaa
Copy link
Collaborator Author

jerboaa commented Mar 21, 2024

Merging as this is needed for a release coming up.

@jerboaa jerboaa merged commit bd13e6a into graalvm:master Mar 21, 2024
5 checks passed
@zakkak
Copy link
Collaborator

zakkak commented Mar 26, 2024

Thanks for taking care of this @jerboaa

Diff ignoring whitespace looks like this:

FYI you can get this diff on github by appending the URL with ?w=1 or by clicking the gear box in the "Files Changed" tab of the PR and ticking "Hide whitespace"

image

@jerboaa
Copy link
Collaborator Author

jerboaa commented Mar 26, 2024

OK. Thanks for the tip!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants