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

[Maven-Target] OSGi-metadata generation fails for artifact without MANIFEST.MF #2394

Open
HannesWell opened this issue May 10, 2023 · 6 comments

Comments

@HannesWell
Copy link
Member

For Maven artifacts used in a Maven target location that don't contain any MANIFEST.MF the Target-Platform resolution fails.

The reason seems to be:

BundleDescription bundleDescription = BundlesAction.createBundleDescription(bundleLocation);
if (bundleDescription == null) {
if (logger.isDebugEnabled()) {
logger.debug("Bundle Location: " + bundleLocation + " (Filesize "
+ (bundleLocation != null ? bundleLocation.length() : -1) + ")");
boolean isFile = bundleLocation != null && bundleLocation.isFile();
logger.debug("File isFile: " + isFile);
if (isFile) {
try (JarFile jarFile = new JarFile(bundleLocation)) {
Enumeration<JarEntry> entries = jarFile.entries();
while (entries.hasMoreElements()) {
JarEntry jarEntry = entries.nextElement();
logger.debug(" Entry: " + jarEntry.getName());
}
} catch (Exception e) {
logger.debug("Reading as jar failed: " + e);
}
}
}
throw new TargetDefinitionResolutionException("Artifact " + mavenArtifact + " of location "
+ location + " is not a valid jar file");
} else {

I wonder why the case of BundlesAction.createBundleDescription(bundleLocation) returning null is not just handled like if the symbolicName of that description is null (i.e. just generated the OSGi-metadata)?

One example where this happens is:

    <location includeDependencyDepth="none" includeDependencyScopes="" includeSource="true" missingManifest="generate" type="Maven">
      <dependencies>
        <dependency>
          <groupId>javax.inject</groupId>
          <artifactId>javax.inject</artifactId>
          <version>1</version>
          <type>jar</type>
        </dependency>
    </location>
  </locations>
@laeubi
Copy link
Member

laeubi commented May 11, 2023

For Maven artifacts used in a Maven target location that don't contain any MANIFEST.MF the Target-Platform resolution fails.

Correct.

I wonder why the case of BundlesAction.createBundleDescription(bundleLocation) returning null is not just handled like if the symbolicName of that description is null (i.e. just generated the OSGi-metadata)?

Because Maven target location only supports:

  1. jar Files
  2. pom files
  3. feature files

One example where this happens is:

This is sadly not a jar file but a zip file that is named jar...

@HannesWell
Copy link
Member Author

One example where this happens is:

This is sadly not a jar file but a zip file that is named jar...

But Oracle's Java-17 JAR File Specification says:

A JAR file is essentially a zip file that contains an **optional** META-INF directory.

Furthermore the javadoc of JarFile.getManifest() says:

@return the jar file manifest, or {@code null} if none

With all of that I think just a zip, which is named jar, with properly located class-files is already a valid jar, even if it does not have a Manifest.

@laeubi
Copy link
Member

laeubi commented May 11, 2023

The BundlesAction.createBundleDescription(bundleLocation) assumes that a jar without a manifest is not a bundle, but also returns null if the manifest is invalid, in both cases null is returned, also some other places assume that, so you need to be aware that maybe things that a actually invalid are also considered "not a bundle" and get wrapped.

But as wrapping is actually something that should be avoided, it is not completely "forbidden" to wrap such content (beside that javax.inject is outdated), we just should made sure that both Tycho+m2e can wrap such "strange" jars (I'm not aware of any else than javax.inject on central without a manifest).

@laeubi
Copy link
Member

laeubi commented May 16, 2023

@HannesWell I plan Tycho 3.0.5 release so if you want to contribute a fix it might be worth to include it there as well.

@HannesWell
Copy link
Member Author

@HannesWell I plan Tycho 3.0.5 release so if you want to contribute a fix it might be worth to include it there as well.

I saw that #2431 changes a lot in the affected area and I have the impression that it already fixes this case?

@laeubi
Copy link
Member

laeubi commented May 24, 2023

I have the impression that it already fixes this case?

Ye I did this for 4.x, but you might want to include it in 3.x as well (only the manifest part), but I would leave this up to you :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants