[xmppclient] Update smack to 4.4.4 and fix dependencies #11670
Conversation
Signed-off-by: Fabian Wolter <[email protected]>
fwolter
added
bug
|
I fear that I can not help much with OSGi and OpenHAB specifics, but I want to make one remark:
This is not correct: |
|
Do you actually need You also don't want
That's not correct: bnd tries to be 'smart' by default and scans for Class.forName usages, resulting in an OSGi import for android.os. To remove that, the next minidns version should probably either exclude android.os or add the bnd instruction |
Signed-off-by: Fabian Wolter <[email protected]>
|
I removed smack-resolver-minidns and its transitive dependencies, which seems to work. Thanks for the hint! However, minidns-core seems to be necessary during compile time: https://github.com/igniterealtime/Smack/blob/48f5e349b9a318ba2a1d82aef9fa069e62da10bb/smack-core/src/main/java/org/jivesoftware/smack/ConnectionConfiguration.java#L83 Now, the same problem with |
|
minidns-core is required both at compile and at runtime. But that's really the only minidns dependency. The last resolving issue boils down to: |
The advantage of minidns-client is that it allows Smack to use DANE to authenticate the server certificate. But for now, OpenHAB could probably live well without MiniDNS and instead using smack-resolver-javax.
I see. I wonder if there is a way to disable this for certain Java packages, e.g. org.minidns, besides the artifact declaring -noclassforname? |
|
For the paper trail: MiniDNS/minidns#123 should allow you to use minidns-client in an OSGi environment (assuming that I did everything correctly and this works). |
I don't know. I discovered that "feature" the hard way when upgrading maven-bundle-plugin. IMO the exclusion you mention should be android, i.e. never declare an import for this package, not disabled it for a project. But in any case, the default should be to import such packages only optional. |
…l; wrap smack-core Signed-off-by: Fabian Wolter <[email protected]>
|
Adding When installing the xmppclient feature, the entire OSGi environment needs to be refreshed due to That leads eventually to: After a restart, strange things happen like NPEs deep in Jetty and freezing karaf console. When I remove the I couldn't find any documentation what exactly |
The PKCS11 usage is about smartcard authentication to an XMPP server, so most likely not anything that's used in OpenHAB. I already have sun.security.pkcs in .extra in Jitsi, so again something I didn't notice and which should become an optional import in Smack.
Well, SpiFly is a framework extension, so I'm not suprised by that. Can't you add SpiFly as a system package (instead of inside this plugin)?
Hmm, good question, I think the framework extension used to be mentioned on that doc page. |
There was a problem hiding this comment.
You added a lot of dependencies that are either unnecessary or transitive dependency of the main entry dependencies of Smack. In OpenHAB case, such a main entry point would be smack-java8, or maybe even smack-java8-full if you don't care about fine grained dependency control.
In a normal pom.xml, you would not add things like smack-core, as it is such a transitive dependency. However, this is maybe the right thing to do due to a particularity of OpenHABs build system, which I am unfamiliar with.
| </dependency> | ||
| <dependency> | ||
| <groupId>org.igniterealtime.smack</groupId> | ||
| <artifactId>smack-xmlparser</artifactId> |
There was a problem hiding this comment.
You usually don't want to add this dependency explicitly.
| </properties> | ||
|
|
||
| <dependencies> | ||
| <dependency> | ||
| <groupId>org.igniterealtime.smack</groupId> | ||
| <artifactId>smack-java7</artifactId> | ||
| <artifactId>smack-core</artifactId> |
There was a problem hiding this comment.
You usually don't want to add this dependency explicitly.
| </dependency> | ||
| <dependency> | ||
| <groupId>org.igniterealtime.smack</groupId> | ||
| <artifactId>smack-xmlparser-xpp3</artifactId> |
There was a problem hiding this comment.
You usually don't want to add this dependency explicitly.
| <version>${smack.version}</version> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>xpp3</groupId> |
There was a problem hiding this comment.
Smack on Java SE does not use xpp3, so it should be possible to drop this.
| </dependency> | ||
| <dependency> | ||
| <groupId>org.jxmpp</groupId> | ||
| <artifactId>jxmpp-core</artifactId> |
There was a problem hiding this comment.
You usually don't want to add this dependency explicitly.
| </dependency> | ||
| <dependency> | ||
| <groupId>org.jxmpp</groupId> | ||
| <artifactId>jxmpp-jid</artifactId> |
There was a problem hiding this comment.
You usually don't want to add this dependency explicitly.
| </dependency> | ||
| <dependency> | ||
| <groupId>org.igniterealtime.smack</groupId> | ||
| <artifactId>smack-sasl-javax</artifactId> |
There was a problem hiding this comment.
You usually don't want to add this dependency explicitly.
| </dependency> | ||
| <dependency> | ||
| <groupId>org.igniterealtime.smack</groupId> | ||
| <artifactId>smack-resolver-javax</artifactId> |
There was a problem hiding this comment.
You want to add this dependency explicitly if you want to use the javax resolver instead of MiniDNS.
| </dependency> | ||
| <dependency> | ||
| <groupId>org.igniterealtime.smack</groupId> | ||
| <artifactId>smack-streammanagement</artifactId> |
There was a problem hiding this comment.
You usually don't want to add this dependency explicitly.
| </dependency> | ||
| <dependency> | ||
| <groupId>org.hsluv</groupId> | ||
| <artifactId>hsluv</artifactId> |
There was a problem hiding this comment.
You usually don't want to add this dependency explicitly.
|
Thanks for your review! Indeed, openHAB needs all transitive dependencies specified in the pom.xml. I'll probably continue to work on this after the holidays. |
|
While you're at it, would it make sense to move the XMPP dependencies to -core, managed as transport feature ( |
|
Seems like my priorities shifted a bit. I played with the xmppclient upgrade to learn more about OSGi and karaf (which worked), but I don't use xmppclient in my openHAB setup. I'd be happy if someone could pick up this PR. Before adding the latest xmppclient library version to OH,
Sounds like a reasonable approach. |
I updated smack to 4.4.4 and updated also all transitive dependencies. A connection to a XMPP server is working when letting it run with eclipse/bndtools and the karaf feature verification succeeds, too. But when building a kar and deploying it in an openHAB instance, I get the following error:
The transitive dependency minidns-client imports
android.os. I tried to wrap the minidns-client bundle and add!android.*toImport-Package, but that doesn't seem to have any effect to the manifest file in the minidns-client JAR in the KAR file.I think I came to an end with my OSGi/karaf knowledge. Any help appreciated. @wborn @Flowdalic @ibauersachs
Fixes #11560