Skip to content

Fix sun.nio.ch import#598

Merged
lukehutch merged 1 commit into
classgraph:latestfrom
Roman-Skripka:latest
Nov 16, 2021
Merged

Fix sun.nio.ch import#598
lukehutch merged 1 commit into
classgraph:latestfrom
Roman-Skripka:latest

Conversation

@Roman-Skripka

@Roman-Skripka Roman-Skripka commented Nov 16, 2021

Copy link
Copy Markdown

Was broken in #597 @lukehutch

@lukehutch lukehutch merged commit 0d92bbb into classgraph:latest Nov 16, 2021
@lukehutch

lukehutch commented Nov 16, 2021

Copy link
Copy Markdown
Member

Sorry about that! I should have been more careful...

Thanks for the pull request. Fixed and released in 4.8.133.

I don't have any OSGi tests right now, so I wasn't able to catch this issue. How hard would it be to add an OSGi testcase? I don't know anything about OSGi.

@Roman-Skripka

Roman-Skripka commented Nov 16, 2021

Copy link
Copy Markdown
Author

@lukehutch not sure about tests, but I was able to see this problem in real OSGi environment after the deployment. For my own bundles I use maven-bundle-plugin where you can specify an instruction like

<Import-Package>
    *
</Import-Package>

and the manifest file will be generated automatically based on the used libs on classpath. Optional deps are automatically imported as optional ones. Not sure whether the plugin you're using can do this automatically, but my suggestion is to just automate the manifest generation)

@lukehutch

lukehutch commented Nov 16, 2021

Copy link
Copy Markdown
Member

Thanks for the tip, that's good to know. This wouldn't work here though, because the optional packages are all referred to in the ClassGraph code by name (using a string constant), using reflection at runtime. (That's the only way I could find to make these dependencies truly optional, without link errors etc.) I have had a couple of users inform me that some of the optional packages are not available in their OSGi runtime.

I'm a big fan of automating everything that can be automated, but in this case, automation is also made more difficult because the name of the module containing an optional package needs to be listed in module-info.java as a requires static dependency. But there is no easy way to look up the module name given a package name. So that information has to be determined manually anyway.

@Roman-Skripka

Roman-Skripka commented Nov 16, 2021

Copy link
Copy Markdown
Author

@lukehutch out of curiosity I tried replacing jar plugin to build the manifest file

<plugin>
                    <groupId>org.apache.maven.plugins</groupId>
                    <artifactId>maven-jar-plugin</artifactId>
                    <version>3.2.0</version>
                    <configuration>
                        <archive>
                            <manifestFile>${project.build.outputDirectory}/META-INF/MANIFEST.MF</manifestFile>
                        </archive>
                    </configuration>
                </plugin>
                <plugin>
                    <groupId>org.apache.felix</groupId>
                    <artifactId>maven-bundle-plugin</artifactId>
                    <version>5.1.2</version>
                    <executions>
                        <execution>
                            <id>scr-metadata</id>
                            <goals>
                                <goal>manifest</goal>
                            </goals>
                        </execution>
                    </executions>
                    <configuration>
                        <unpackBundle>true</unpackBundle>
                        <instructions>
                            <!-- Add OSGi bundle attributes to manifest -->
                            <Bundle-Category>Utilities</Bundle-Category>
                            <Bundle-License>http://opensource.org/licenses/MIT</Bundle-License>
                            <Bundle-ManifestVersion>2</Bundle-ManifestVersion>
                            <Bundle-Name>ClassGraph</Bundle-Name>
                            <Bundle-SymbolicName>${project.groupId}.${project.artifactId}</Bundle-SymbolicName>
                            <Bundle-Vendor>Luke Hutchison</Bundle-Vendor>
                            <Bundle-Description>${project.description}</Bundle-Description>
                            <Bundle-Version>${project.version}</Bundle-Version>
                            <Require-Capability>osgi.ee;filter:="(&amp;(osgi.ee=JavaSE)(version=1.7))"</Require-Capability>
                            <Export-Package>io.github.classgraph;version="${project.version}"</Export-Package>
                            <!-- Make sure "requires" entries in the module descriptor match these imports: -->
                            <!-- src/main/module-info/io.github.classgraph/module-info.java -->
                            <Import-Package>
                                jdk.internal.misc;resolution:="optional",
                                sun.misc;resolution:="optional",
                                sun.nio.ch;resolution:="optional",
                                io.github.toolfactory.narcissus;resolution:="optional",
                                io.github.toolfactory.jvm;resolution:="optional",
                                org.apache.openejb;resolution:="optional",
                                org.apache.openejb.server.cxf.transport.util;resolution:="optional",
                                *
                            </Import-Package>

                        <!-- This is a multi-release jar, since module-info.class will be added to -->
                            <!-- META-INF/versions/9 , so we need to set the Multi-Release entry to true -->
                            <Multi-Release>true</Multi-Release>
                            <Include-Resource>{maven-resources}</Include-Resource>
                        </instructions>
                    </configuration>
                </plugin>

and got the following manifest

Manifest-Version: 1.0
Bnd-LastModified: 1637103293462
Build-Jdk: 11.0.13
Built-By: -
Bundle-Category: Utilities
Bundle-Description: The uber-fast, ultra-lightweight classpath and modul
 e scanner for JVM languages.
Bundle-License: http://opensource.org/licenses/MIT
Bundle-ManifestVersion: 2
Bundle-Name: ClassGraph
Bundle-SymbolicName: io.github.classgraph.classgraph
Bundle-Vendor: Luke Hutchison
Bundle-Version: 4.8.133.SNAPSHOT
Created-By: Apache Maven Bundle Plugin
Export-Package: io.github.classgraph;version="4.8.133.SNAPSHOT"
Import-Package: sun.misc;resolution:=optional,sun.nio.ch;resolution:=opt
 ional,org.apache.openejb;resolution:=optional,org.apache.openejb.server
 .cxf.transport.util;resolution:=optional,javax.xml.namespace,javax.xml.
 parsers,javax.xml.xpath,org.w3c.dom,jdk.internal.misc;resolution:=optio
 nal,io.github.toolfactory.narcissus;resolution:=optional;version="[1.0,
 2)",io.github.toolfactory.jvm;resolution:=optional
Multi-Release: true
Require-Capability: osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=1.7))"
Tool: Bnd-5.1.1.202006162103

Pay attention that in the process I found a few missing optional imports used in forName function(its usage is detected by the plugin automatically). '*' includes it as a required dep if not marked otherwise. May be this will help somehow )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Roman-Skripka @lukehutch