-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
We do not handle existing MANIFEST.MF files properly #1443
Comments
Oh, that's the issue! But why did Eclipse try to create a |
So, basically the logic would be to check for the existence of |
(when I say inject, read replace if already set, with a warning) |
How about replacing the old file with a fresh copy on each build? |
I don't know why Eclipse IDE/m2e generates MANIFEST.MF files, but in any case, having some application builds that generate some MANIFEST.MF is not so unusual and this case should be embraced by Quarkus rather than rejected. |
From what I understand, it seems there various steps that are run :
|
We probably want a build item (optional) for the input manifest, and something for the output. The key is to ensure that we don't replace the manifest in-place, as cumulative changes would prevent build repeatability. |
@dmlloyd I'm failing to see how that would prevent the IDE from generating its own MANIFEST.MF? I think I would just update What would be interesting would be to have an example of this generated MANIFEST.MF so that we can see if it defines the classpath or main class. @Riduidel would you like to give it a try? |
@gsmet I would be very happy to attempt a solution as a PR. I will try to do that this week-end |
Howevern strangely, quarkus dev mode works correctly, so it seems to me the dev mode is not impacted (but I would love to have a way to make sure dev mode is not impacted) |
I'm quite confident now that I understand how So, in final Manifest manifest = new Manifest();
manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0");
manifest.getMainAttributes().put(Attributes.Name.CLASS_PATH, classPath.toString());
manifest.getMainAttributes().put(Attributes.Name.MAIN_CLASS, mainClass);
try (OutputStream os = Files.newOutputStream(runnerZipFs.getPath("META-INF", "MANIFEST.MF"))) {
manifest.write(os);
}
copyFiles(augmentOutcome.getAppClassesDir(), runnerZipFs);
copyFiles(augmentOutcome.getTransformedClassesDir(), runnerZipFs);
for (Map.Entry<String, List<byte[]>> entry : services.entrySet()) {
try (OutputStream os = Files.newOutputStream(runnerZipFs.getPath(entry.getKey()))) {
for (byte[] i : entry.getValue()) {
os.write(i);
os.write('\n');
}
}
} What I understant is that quarkus first create its own manifest, then copy existing files. It explains how previously existing manifest will overwrite quarkus one (hence my bug). How to fix that ? I will move the manifest generation code in its own method, move call to that method after file copy and in that method add declarations to existing manifest instead of creating a new one. Is it ok for you ? @gsmet I took time to answer, but yes, I'll give it a try ;-) |
We should be modifying the manifest via build items IMO. Maybe extensions should produce manifest entry items, which are then merged into the final product. |
it could
Something like copyFiles(augmentOutcome.getAppClassesDir(), runnerZipFs);
copyFiles(augmentOutcome.getTransformedClassesDir(), runnerZipFs);
for (Map.Entry<String, List<byte[]>> entry : services.entrySet()) {
try (OutputStream os = Files.newOutputStream(runnerZipFs.getPath(entry.getKey()))) {
for (byte[] i : entry.getValue()) {
os.write(i);
os.write('\n');
}
}
}
final Manifest manifest = new Manifest();
Path targetManifestPath = runnerZipFs.getPath("META-INF", "MANIFEST.MF"));
if (Files.exists(targetManifestPath)) {
try (InputStream stream = Files.newInputStream(targetManifestPath)) {
manifest.read(stream);
}
}
manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0");
manifest.getMainAttributes().put(Attributes.Name.CLASS_PATH, classPath.toString());
manifest.getMainAttributes().put(Attributes.Name.MAIN_CLASS, mainClass);
try (OutputStream os = Files.newOutputStream(targetManifestPath) {
manifest.write(os);
} |
…od, invoked after copying all files The goal of this method is to make sure the entries we write in manfiest do not exist already or, if they exist, are compatible with quarkus requirements. This goal is reached for MAIN_CLASS attribute, but not totally for CLASSPATH
Well, I suggest you take a look at the pull request #1540 :-) |
Fixes #1443 by moving manifest generation in a specific method
See https://stackoverflow.com/questions/55124900/how-to-create-native-executable-using-quarkus#
In this case we should probably modify the existing manifest to add the Main-Class entry
The text was updated successfully, but these errors were encountered: